| Message ID | 20200427094829.1140-3-takahiro.akashi@linaro.org |
|---|---|
| State | Superseded, archived |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | efi_loader: add capsule update support | expand |
On 4/27/20 11:48 AM, AKASHI Takahiro wrote: > If this option is enabled, the initialisation of UEFI subsystem will be > done as part of U-Boot initialisation. > > This feature will be utilised in implementing capsule-on-disk feature. This would mean that we allow unaligned access very early. Something Siarhei was against: https://lists.denx.de/pipermail/u-boot/2018-March/324242.html https://patchwork.ozlabs.org/project/uboot/patch/20180329213350.7868-1-xypron.glpk@gmx.de/ Why can't you wait with the capsule update until any command initializes the UEFI sub-system. Best regards Heinrich > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > common/board_r.c | 6 ++++++ > lib/efi_loader/Kconfig | 4 ++++ > 2 files changed, 10 insertions(+) > > diff --git a/common/board_r.c b/common/board_r.c > index 0bbeaa7594c6..7cf21a6078f9 100644 > --- a/common/board_r.c > +++ b/common/board_r.c > @@ -64,6 +64,9 @@ > #if defined(CONFIG_GPIO_HOG) > #include <asm/gpio.h> > #endif > +#ifdef CONFIG_EFI_SETUP_EARLY > +#include <efi_loader.h> > +#endif > > DECLARE_GLOBAL_DATA_PTR; > > @@ -867,6 +870,9 @@ static init_fnc_t init_sequence_r[] = { > #endif > #if defined(CONFIG_M68K) && defined(CONFIG_BLOCK_CACHE) > blkcache_init, > +#endif > +#ifdef CONFIG_EFI_SETUP_EARLY > + (init_fnc_t)efi_init_obj_list, > #endif > run_main_loop, > }; > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > index 1cfa24ffcf72..7cc2d940f848 100644 > --- a/lib/efi_loader/Kconfig > +++ b/lib/efi_loader/Kconfig > @@ -25,6 +25,10 @@ config EFI_LOADER > > if EFI_LOADER > > +config EFI_SETUP_EARLY > + bool > + default n > + > config EFI_GET_TIME > bool "GetTime() runtime service" > depends on DM_RTC >
Heinrich, On Mon, Apr 27, 2020 at 10:09:11PM +0200, Heinrich Schuchardt wrote: > On 4/27/20 11:48 AM, AKASHI Takahiro wrote: > > If this option is enabled, the initialisation of UEFI subsystem will be > > done as part of U-Boot initialisation. > > > > This feature will be utilised in implementing capsule-on-disk feature. > > This would mean that we allow unaligned access very early. Something > Siarhei was against: ? Even with CONFIG_EFI_CAPSULE_ON_DISK_EARLY enabled, efi_init_obj_list() is called at the last of "init" list and efi_launch_capsules() is called just before the main command loop. So "unalignment" issue won't happen. > https://lists.denx.de/pipermail/u-boot/2018-March/324242.html > https://patchwork.ozlabs.org/project/uboot/patch/20180329213350.7868-1-xypron.glpk@gmx.de/ > > Why can't you wait with the capsule update until any command initializes > the UEFI sub-system. This topic is the one the I mentioned in RFC's cover letter and asked you for comments several time. Anyway, there are a couple of reasons: 1. Updated firmware may have some effects on not only UEFI subsystem but also U-Boot's other features. 2. Firmware update should surely take place after reboot as UEFI specification expects. 3. Firmware update should not rely on user's interactions or whatever "bootcmd" is set to. 4. In case of failure of firmware update, some recovery should be automatically taken "before" the command line is handed over to users. (The feature is not implemented yet though.) -Takahiro Akashi > Best regards > > Heinrich > > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > --- > > common/board_r.c | 6 ++++++ > > lib/efi_loader/Kconfig | 4 ++++ > > 2 files changed, 10 insertions(+) > > > > diff --git a/common/board_r.c b/common/board_r.c > > index 0bbeaa7594c6..7cf21a6078f9 100644 > > --- a/common/board_r.c > > +++ b/common/board_r.c > > @@ -64,6 +64,9 @@ > > #if defined(CONFIG_GPIO_HOG) > > #include <asm/gpio.h> > > #endif > > +#ifdef CONFIG_EFI_SETUP_EARLY > > +#include <efi_loader.h> > > +#endif > > > > DECLARE_GLOBAL_DATA_PTR; > > > > @@ -867,6 +870,9 @@ static init_fnc_t init_sequence_r[] = { > > #endif > > #if defined(CONFIG_M68K) && defined(CONFIG_BLOCK_CACHE) > > blkcache_init, > > +#endif > > +#ifdef CONFIG_EFI_SETUP_EARLY > > + (init_fnc_t)efi_init_obj_list, > > #endif > > run_main_loop, > > }; > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > index 1cfa24ffcf72..7cc2d940f848 100644 > > --- a/lib/efi_loader/Kconfig > > +++ b/lib/efi_loader/Kconfig > > @@ -25,6 +25,10 @@ config EFI_LOADER > > > > if EFI_LOADER > > > > +config EFI_SETUP_EARLY > > + bool > > + default n > > + > > config EFI_GET_TIME > > bool "GetTime() runtime service" > > depends on DM_RTC > > >
On 4/28/20 2:16 AM, AKASHI Takahiro wrote: > Heinrich, > > On Mon, Apr 27, 2020 at 10:09:11PM +0200, Heinrich Schuchardt wrote: >> On 4/27/20 11:48 AM, AKASHI Takahiro wrote: >>> If this option is enabled, the initialisation of UEFI subsystem will be >>> done as part of U-Boot initialisation. >>> >>> This feature will be utilised in implementing capsule-on-disk feature. >> >> This would mean that we allow unaligned access very early. Something >> Siarhei was against: > > ? > Even with CONFIG_EFI_CAPSULE_ON_DISK_EARLY enabled, > efi_init_obj_list() is called at the last of "init" list > and efi_launch_capsules() is called just before the main > command loop. > So "unalignment" issue won't happen. efi_init_obj_list() is even called when booting via booti and therefore before a lot of other code. Best regards Heinrich > >> https://lists.denx.de/pipermail/u-boot/2018-March/324242.html >> https://patchwork.ozlabs.org/project/uboot/patch/20180329213350.7868-1-xypron.glpk@gmx.de/ >> >> Why can't you wait with the capsule update until any command initializes >> the UEFI sub-system. > > This topic is the one the I mentioned in RFC's cover letter > and asked you for comments several time. > Anyway, there are a couple of reasons: > 1. Updated firmware may have some effects on not only UEFI > subsystem but also U-Boot's other features. > 2. Firmware update should surely take place after reboot > as UEFI specification expects. > 3. Firmware update should not rely on user's interactions > or whatever "bootcmd" is set to. > 4. In case of failure of firmware update, some recovery should > be automatically taken "before" the command line is handed over > to users. (The feature is not implemented yet though.) > > -Takahiro Akashi > > >> Best regards >> >> Heinrich >> >>> >>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> >>> --- >>> common/board_r.c | 6 ++++++ >>> lib/efi_loader/Kconfig | 4 ++++ >>> 2 files changed, 10 insertions(+) >>> >>> diff --git a/common/board_r.c b/common/board_r.c >>> index 0bbeaa7594c6..7cf21a6078f9 100644 >>> --- a/common/board_r.c >>> +++ b/common/board_r.c >>> @@ -64,6 +64,9 @@ >>> #if defined(CONFIG_GPIO_HOG) >>> #include <asm/gpio.h> >>> #endif >>> +#ifdef CONFIG_EFI_SETUP_EARLY >>> +#include <efi_loader.h> >>> +#endif >>> >>> DECLARE_GLOBAL_DATA_PTR; >>> >>> @@ -867,6 +870,9 @@ static init_fnc_t init_sequence_r[] = { >>> #endif >>> #if defined(CONFIG_M68K) && defined(CONFIG_BLOCK_CACHE) >>> blkcache_init, >>> +#endif >>> +#ifdef CONFIG_EFI_SETUP_EARLY >>> + (init_fnc_t)efi_init_obj_list, >>> #endif >>> run_main_loop, >>> }; >>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig >>> index 1cfa24ffcf72..7cc2d940f848 100644 >>> --- a/lib/efi_loader/Kconfig >>> +++ b/lib/efi_loader/Kconfig >>> @@ -25,6 +25,10 @@ config EFI_LOADER >>> >>> if EFI_LOADER >>> >>> +config EFI_SETUP_EARLY >>> + bool >>> + default n >>> + >>> config EFI_GET_TIME >>> bool "GetTime() runtime service" >>> depends on DM_RTC >>> >>
Heinrich, On Sun, May 17, 2020 at 09:29:44AM +0200, Heinrich Schuchardt wrote: > On 4/28/20 2:16 AM, AKASHI Takahiro wrote: > > Heinrich, > > > > On Mon, Apr 27, 2020 at 10:09:11PM +0200, Heinrich Schuchardt wrote: > >> On 4/27/20 11:48 AM, AKASHI Takahiro wrote: > >>> If this option is enabled, the initialisation of UEFI subsystem will be > >>> done as part of U-Boot initialisation. > >>> > >>> This feature will be utilised in implementing capsule-on-disk feature. > >> > >> This would mean that we allow unaligned access very early. Something > >> Siarhei was against: > > > > ? > > Even with CONFIG_EFI_CAPSULE_ON_DISK_EARLY enabled, > > efi_init_obj_list() is called at the last of "init" list > > and efi_launch_capsules() is called just before the main > > command loop. > > So "unalignment" issue won't happen. > > efi_init_obj_list() is even called when booting via booti and therefore > before a lot of other code. How is booti related to "unalignment" issue? -Takahiro Akashi > Best regards > > Heinrich > > > > >> https://lists.denx.de/pipermail/u-boot/2018-March/324242.html > >> https://patchwork.ozlabs.org/project/uboot/patch/20180329213350.7868-1-xypron.glpk@gmx.de/ > >> > >> Why can't you wait with the capsule update until any command initializes > >> the UEFI sub-system. > > > > This topic is the one the I mentioned in RFC's cover letter > > and asked you for comments several time. > > Anyway, there are a couple of reasons: > > 1. Updated firmware may have some effects on not only UEFI > > subsystem but also U-Boot's other features. > > 2. Firmware update should surely take place after reboot > > as UEFI specification expects. > > 3. Firmware update should not rely on user's interactions > > or whatever "bootcmd" is set to. > > 4. In case of failure of firmware update, some recovery should > > be automatically taken "before" the command line is handed over > > to users. (The feature is not implemented yet though.) > > > > -Takahiro Akashi > > > > > >> Best regards > >> > >> Heinrich > >> > >>> > >>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > >>> --- > >>> common/board_r.c | 6 ++++++ > >>> lib/efi_loader/Kconfig | 4 ++++ > >>> 2 files changed, 10 insertions(+) > >>> > >>> diff --git a/common/board_r.c b/common/board_r.c > >>> index 0bbeaa7594c6..7cf21a6078f9 100644 > >>> --- a/common/board_r.c > >>> +++ b/common/board_r.c > >>> @@ -64,6 +64,9 @@ > >>> #if defined(CONFIG_GPIO_HOG) > >>> #include <asm/gpio.h> > >>> #endif > >>> +#ifdef CONFIG_EFI_SETUP_EARLY > >>> +#include <efi_loader.h> > >>> +#endif > >>> > >>> DECLARE_GLOBAL_DATA_PTR; > >>> > >>> @@ -867,6 +870,9 @@ static init_fnc_t init_sequence_r[] = { > >>> #endif > >>> #if defined(CONFIG_M68K) && defined(CONFIG_BLOCK_CACHE) > >>> blkcache_init, > >>> +#endif > >>> +#ifdef CONFIG_EFI_SETUP_EARLY > >>> + (init_fnc_t)efi_init_obj_list, > >>> #endif > >>> run_main_loop, > >>> }; > >>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > >>> index 1cfa24ffcf72..7cc2d940f848 100644 > >>> --- a/lib/efi_loader/Kconfig > >>> +++ b/lib/efi_loader/Kconfig > >>> @@ -25,6 +25,10 @@ config EFI_LOADER > >>> > >>> if EFI_LOADER > >>> > >>> +config EFI_SETUP_EARLY > >>> + bool > >>> + default n > >>> + > >>> config EFI_GET_TIME > >>> bool "GetTime() runtime service" > >>> depends on DM_RTC > >>> > >> >
diff --git a/common/board_r.c b/common/board_r.c index 0bbeaa7594c6..7cf21a6078f9 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -64,6 +64,9 @@ #if defined(CONFIG_GPIO_HOG) #include <asm/gpio.h> #endif +#ifdef CONFIG_EFI_SETUP_EARLY +#include <efi_loader.h> +#endif DECLARE_GLOBAL_DATA_PTR; @@ -867,6 +870,9 @@ static init_fnc_t init_sequence_r[] = { #endif #if defined(CONFIG_M68K) && defined(CONFIG_BLOCK_CACHE) blkcache_init, +#endif +#ifdef CONFIG_EFI_SETUP_EARLY + (init_fnc_t)efi_init_obj_list, #endif run_main_loop, }; diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index 1cfa24ffcf72..7cc2d940f848 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -25,6 +25,10 @@ config EFI_LOADER if EFI_LOADER +config EFI_SETUP_EARLY + bool + default n + config EFI_GET_TIME bool "GetTime() runtime service" depends on DM_RTC
If this option is enabled, the initialisation of UEFI subsystem will be done as part of U-Boot initialisation. This feature will be utilised in implementing capsule-on-disk feature. Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- common/board_r.c | 6 ++++++ lib/efi_loader/Kconfig | 4 ++++ 2 files changed, 10 insertions(+)