[RFC PATCH 1/1] efi_loader: Call efi_init_early() even earlier
At least in case of the E850-96 board all DM devices are probed in initr_dm(), before efi_init_early() call. Eventually efi_bl_init() registers the EVT_DM_POST_PROBE_EVENT event, but all DM devices (including eMMC) are already probed by then, so no events are caught. Because of that efi_disk_probe() is never called, so EFI disk objects are not created and EFI subsystem can't function correctly, showing the next symptoms: - 'efidebug dh' output shows nothing - attempt to add boot devices in 'eficonfig' shows this message: "No block device found!" - 'bootefi selftest $fdtcontroladdr' shows this warning: "Cannot persist EFI variables without system partition" - booting GRUB with 'bootefi' runs minimal GRUB shell which doesn't see any block devices as well, probably because EFI vars weren't passed
Move efi_init_early() call before initr_dm() in init_sequence_r[] so that EVT_DM_POST_PROBE_EVENT in efi_bl_init() can actually catch all DM probe events and create corresponding EFI objects. That fixes the described problem and makes it possible to run EFI apps like GRUB correctly, add entries in 'eficonfig', and makes 'efivar --list' command in Linux rootfs actually show EFI variables.
This patch doubles down on the approach taken in commit 5e847f7729b3 ("efi_loader: call efi_init_early() earlier"), taking a different path than another fix proposed earlier in "Pull-request efi-2025-01-rc4" [1]. There was also a related discussion in the "efi_loader: more tightly integrate UEFI disks to driver model" patch series [2].
[1] https://lore.kernel.org/u-boot/8910d434-2d77-425f-aa81-8eb803078aef@gmx.de/ [2] https://lists.denx.de/pipermail/u-boot/2022-April/481753.html
Fixes: 5e847f7729b3 ("efi_loader: call efi_init_early() earlier") Signed-off-by: Sam Protsenko semen.protsenko@linaro.org --- common/board_r.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/common/board_r.c b/common/board_r.c index f63c6aed4d5d..323a56c2018c 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -617,6 +617,11 @@ static init_fnc_t init_sequence_r[] = { #endif #ifdef CONFIG_SYS_NONCACHED_MEMORY noncached_init, +#endif + initr_lmb, +#ifdef CONFIG_EFI_LOADER + efi_memory_init, + efi_init_early, #endif initr_of_live, #ifdef CONFIG_DM @@ -636,10 +641,6 @@ static init_fnc_t init_sequence_r[] = { */ #ifdef CONFIG_CLOCKS set_cpu_clk_info, /* Setup clock information */ -#endif - initr_lmb, -#ifdef CONFIG_EFI_LOADER - efi_memory_init, #endif #ifdef CONFIG_BINMAN_FDT initr_binman, @@ -684,9 +685,6 @@ static init_fnc_t init_sequence_r[] = { /* initialize higher level parts of CPU like time base and timers */ cpu_init_r, #endif -#ifdef CONFIG_EFI_LOADER - efi_init_early, -#endif #ifdef CONFIG_CMD_NAND initr_nand, #endif
On 1/13/25 00:58, Sam Protsenko wrote:
At least in case of the E850-96 board all DM devices are probed in initr_dm(), before efi_init_early() call. Eventually efi_bl_init() registers the EVT_DM_POST_PROBE_EVENT event, but all DM devices (including eMMC) are already probed by then, so no events are caught. Because of that efi_disk_probe() is never called, so EFI disk objects are not created and EFI subsystem can't function correctly, showing the next symptoms:
- 'efidebug dh' output shows nothing
- attempt to add boot devices in 'eficonfig' shows this message: "No block device found!"
- 'bootefi selftest $fdtcontroladdr' shows this warning: "Cannot persist EFI variables without system partition"
- booting GRUB with 'bootefi' runs minimal GRUB shell which doesn't see any block devices as well, probably because EFI vars weren't passed
Move efi_init_early() call before initr_dm() in init_sequence_r[] so that EVT_DM_POST_PROBE_EVENT in efi_bl_init() can actually catch all DM probe events and create corresponding EFI objects. That fixes the described problem and makes it possible to run EFI apps like GRUB correctly, add entries in 'eficonfig', and makes 'efivar --list' command in Linux rootfs actually show EFI variables.
This patch doubles down on the approach taken in commit 5e847f7729b3 ("efi_loader: call efi_init_early() earlier"), taking a different path than another fix proposed earlier in "Pull-request efi-2025-01-rc4" [1]. There was also a related discussion in the "efi_loader: more tightly integrate UEFI disks to driver model" patch series [2].
There are two DM functions:
- dm_initr - dm_initr_dm_devices
Before dm_initr() gd->dm_root points to the pre-relocation tree. So I guess we should not register the DM events for EFI before that point.
Shouldn't the DM devices only be scanned in initr_dm_devices? Why is E850-96 doing so in dm_initr?
@Simon We really need to document the dm_initr* functions. Could you, please, suggest code comments to be added common/board_r.c.
Please, also describe enum bootstage_id including BOOTSTAGE_ID_ACCUM_DM_R. I am clueless why the value exists.
Do you have an idea why dm_init_and_scan() would cause probing? Why is this function called in dm_initr() and not in initr_dm_devices()?
Best regards
Heinrich
[1] https://lore.kernel.org/u-boot/8910d434-2d77-425f-aa81-8eb803078aef@gmx.de/ [2] https://lists.denx.de/pipermail/u-boot/2022-April/481753.html
Fixes: 5e847f7729b3 ("efi_loader: call efi_init_early() earlier") Signed-off-by: Sam Protsenko semen.protsenko@linaro.org
common/board_r.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/common/board_r.c b/common/board_r.c index f63c6aed4d5d..323a56c2018c 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -617,6 +617,11 @@ static init_fnc_t init_sequence_r[] = { #endif #ifdef CONFIG_SYS_NONCACHED_MEMORY noncached_init, +#endif
- initr_lmb,
+#ifdef CONFIG_EFI_LOADER
- efi_memory_init,
- efi_init_early, #endif initr_of_live, #ifdef CONFIG_DM
@@ -636,10 +641,6 @@ static init_fnc_t init_sequence_r[] = { */ #ifdef CONFIG_CLOCKS set_cpu_clk_info, /* Setup clock information */ -#endif
- initr_lmb,
-#ifdef CONFIG_EFI_LOADER
- efi_memory_init, #endif #ifdef CONFIG_BINMAN_FDT initr_binman,
@@ -684,9 +685,6 @@ static init_fnc_t init_sequence_r[] = { /* initialize higher level parts of CPU like time base and timers */ cpu_init_r, #endif -#ifdef CONFIG_EFI_LOADER
- efi_init_early,
-#endif #ifdef CONFIG_CMD_NAND initr_nand, #endif
Hi Heinrich,
On Mon, 13 Jan 2025 at 00:29, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 1/13/25 00:58, Sam Protsenko wrote:
At least in case of the E850-96 board all DM devices are probed in initr_dm(), before efi_init_early() call. Eventually efi_bl_init() registers the EVT_DM_POST_PROBE_EVENT event, but all DM devices (including eMMC) are already probed by then, so no events are caught. Because of that efi_disk_probe() is never called, so EFI disk objects are not created and EFI subsystem can't function correctly, showing the next symptoms:
- 'efidebug dh' output shows nothing
- attempt to add boot devices in 'eficonfig' shows this message: "No block device found!"
- 'bootefi selftest $fdtcontroladdr' shows this warning: "Cannot persist EFI variables without system partition"
- booting GRUB with 'bootefi' runs minimal GRUB shell which doesn't see any block devices as well, probably because EFI vars weren't passed
Move efi_init_early() call before initr_dm() in init_sequence_r[] so that EVT_DM_POST_PROBE_EVENT in efi_bl_init() can actually catch all DM probe events and create corresponding EFI objects. That fixes the described problem and makes it possible to run EFI apps like GRUB correctly, add entries in 'eficonfig', and makes 'efivar --list' command in Linux rootfs actually show EFI variables.
This patch doubles down on the approach taken in commit 5e847f7729b3 ("efi_loader: call efi_init_early() earlier"), taking a different path than another fix proposed earlier in "Pull-request efi-2025-01-rc4" [1]. There was also a related discussion in the "efi_loader: more tightly integrate UEFI disks to driver model" patch series [2].
There are two DM functions:
- dm_initr
- dm_initr_dm_devices
Before dm_initr() gd->dm_root points to the pre-relocation tree. So I guess we should not register the DM events for EFI before that point.
Shouldn't the DM devices only be scanned in initr_dm_devices? Why is E850-96 doing so in dm_initr?
@Simon We really need to document the dm_initr* functions. Could you, please, suggest code comments to be added common/board_r.c.
Once [1] lands, sure. At the moment, dm_init() is doing the wrong thing.
Please, also describe enum bootstage_id including BOOTSTAGE_ID_ACCUM_DM_R. I am clueless why the value exists.
It provides the time taken by driver model to init, after relocation. It is printed with the bootstage report before the kernel boots. BTW it should be printed before the EFI app boots too, assuming bootstage is enabled.
Do you have an idea why dm_init_and_scan() would cause probing? Why is this function called in dm_initr() and not in initr_dm_devices()?
No, that is just wrong. I hope it isn't something to do with Marek's change.
As noted above I did send something to try to restore the driver model 'invariants'[1]. If that is applied then I could take a look at a test for this, to make sure other boards don't have this problem.
As I mentioned elsewhere, EFI_LOADER should be using U-Boot's internal data where possible, rather than having its own parallel data structures, as that is always up-to-date. I also wonder if there is a clever way to create ephemeral structures which are created as needed from the DM data. Not sure.
Regards, Simon
[1] https://patchwork.ozlabs.org/project/uboot/list/?series=433606
participants (3)
-
Heinrich Schuchardt -
Sam Protsenko -
Simon Glass