[RFC PATCH 1/1] efi_loader: Call efi_init_early() even earlier

[Heinrich Schuchardt]

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 at 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