[PATCH 3/4] efi_loader: bootmgr: install the firmware-owned devicetree

[Simon Glass]

Hi Carlo,

On 2026-07-06T15:06:41, Carlo Caione <ccaione at baylibre.com> wrote:
> efi_loader: bootmgr: install the firmware-owned devicetree
>
> SystemReady IR boots through the UEFI boot manager, which does not go
> through the per-device EFI bootmeth, so hooking the bootmeth alone
> would leave the boot-manager path (including autoboot) on the control
> devicetree.
>
> Consume the firmware-FDT source in efi_bootmgr_run() too, so the
> firmware-owned devicetree is installed via efi_install_fdt() regardless
> of which EFI launch path is taken.
>
> The firmware-owned devicetree is tried before the Boot#### load-option
> FDT, so a stale or broken load option can neither override nor block a
> configured (and possibly signed) firmware devicetree. As a side effect,
> a devicetree passed directly to efi_bootmgr_run() now also outranks the
> load-option FDT, which previously overrode it: an explicitly supplied
> devicetree is the operator's immediate intent, and stored boot options
> must not replace it.
>
> Signed-off-by: Carlo Caione <ccaione at baylibre.com>
>
> lib/efi_loader/efi_bootmgr.c | 50 +++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 43 insertions(+), 7 deletions(-)

> As a side effect,
> a devicetree passed directly to efi_bootmgr_run() now also outranks the
> load-option FDT, which previously overrode it: an explicitly supplied
> devicetree is the operator's immediate intent, and stored boot options
> must not replace it.

This behaviour change is independent of the firmware-FDT work -
please split it into its own patch so the two can be reviewed (and,
if needed, reverted) separately. That patch can go first, then this
one becomes purely additive.

> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> @@ -1305,7 +1308,8 @@ efi_status_t efi_bootmgr_run(void *fdt)
>               if (!fdt) {
> +                     ret = load_fdt_from_load_option(&fdt_lo);
> +                     if (ret != EFI_SUCCESS)
> +                             return ret;
> +                     if (fdt_lo)
> +                             fdt = fdt_lo;
> +             }

Now that you have an out: label with cleanup for fw, this early
return ret; is inconsistent - it skips firmware_fdt_free(&fw) and
the load_options / handle cleanup below. In practice fw is zeroed on
this path so nothing leaks today, but please route through goto out;
so the function has a single exit for error paths.

> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> @@ -1320,11 +1324,40 @@ efi_status_t efi_bootmgr_run(void *fdt)
> +                     } else if (err != -ENOENT) {
> +                             log_err("Failed to assemble the firmware devicetree (err %d)\n",
> +                                     err);
> +                             ret = EFI_LOAD_ERROR;
> +                             goto out;
> +                     }

EFI_LOAD_ERROR is really about failing to load an image; for a
devicetree that couldn't be assembled, EFI_DEVICE_ERROR (or
EFI_NOT_FOUND, when appropriate) reads more accurately.

You are adding quite a bit of code here, so please split it into a
separate function if you can.

> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> @@ -1296,7 +1297,9 @@ out:
>   *
>   * Invoke EFI boot manager and execute a binary depending on
>   * boot options. If @fdt is not NULL, it will be passed to
> - * the executed binary.
> + * the executed binary. Otherwise a configured firmware-owned
> + * devicetree source takes precedence over both a Boot#### load
> + * option FDT and the distro one.
>   *
>   * Return:   status code
>   */

Please spell out the full precedence order here (passed @fdt >
firmware-owned > Boot#### load option > distro/ESP) - that is now
the interesting contract of the function and 'Otherwise' hides half
of it.

Regards,
Simon