[PATCH v4 1/1] efi_loader: expose the device-tree file name

[Simon Glass]

Hi Heinrich,

On Mon, 23 Oct 2023 at 23:20, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
> Forward and backward compatibility of Linux kernel device-trees is
> sometimes missing. One solution approach is to load a kernel specific
> device-tree. This can either be done via a U-Boot scripts (like the one
> generated by Debian package flash-kernel or by a boot loader like GRUB.
> The boot loader approach currently requires to know the device-tree name
> before first boot which makes it unusable for generic images.
>
> Expose the device-tree file name as EFI variable FdtFile.
> This will allow bootloaders to load a kernel specific device-tree.

kernel-specific

>
> The variable will not be exposed on ACPI based systems or if the
> environment variable fdtfile is not defined.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> ---
> v4:
>         Generalize the description of the content of $fdtfile.
> v3:
>         Add documentation
> v2:
>         Use a unique GUID to enable future U-Boot independent
>         standardization.
>         Do not try to add the variable on ACPI based systems.
> ---
>  doc/develop/uefi/uefi.rst  | 14 ++++++++++++++
>  include/efi_loader.h       |  5 +++++
>  lib/efi_loader/efi_setup.c | 30 ++++++++++++++++++++++++++++++
>  3 files changed, 49 insertions(+)
>
> diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
> index fb16ac743a..702c490831 100644
> --- a/doc/develop/uefi/uefi.rst
> +++ b/doc/develop/uefi/uefi.rst
> @@ -916,6 +916,20 @@ So our final format of the FilePathList[] is::
>
>      Loaded image - end node (0xff) - VenMedia - initrd_1 - [end node (0x01) - initrd_n ...] - end node (0xff)
>
> +EFI variable FdtFile
> +~~~~~~~~~~~~~~~~~~~~
> +
> +Ideally U-Boot would always expose a device-tree that can be used for booting
> +any operating systems. Unfortunately operating systems like Linux sometimes
> +break forward and backward compatibility. In this case there is a need to load
> +an operating system version specific device-tree.

This seems to be a strong statement. Given the effort that goes into
the DT, changes are supposed to be backwards-compatible. Is this
generally true, or is it just that we want an up-to-date DT for the
kernel to enable new features?

> +
> +U-Boot has an environment variable fdtfile identifying the device-tree file to
> +load. The content of this variable is exposed as EFI variable Fdtfile, vendor
> +GUID d45dde69-3bd6-40e0-90d5-6b606aa57730. It contains the device-tree path
> +name as a NUL terminated ASCII string. On many architectures the file name is

NUL-terminated

> +preceded by a vendor directory ('vendor-directory/board-name.dtb').
> +
>  Links
>  -----
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index e24410505f..146e7f1bce 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -152,6 +152,11 @@ static inline efi_status_t efi_launch_capsules(void)
>         EFI_GUID(0x8108ac4e, 0x9f11, 0x4d59, \
>                  0x85, 0x0e, 0xe2, 0x1a, 0x52, 0x2c, 0x59, 0xb2)
>
> +/* Vendor GUID for the FdtFile variable */
> +#define VENDOR_FDTFILE_GUID \
> +       EFI_GUID(0xd45dde69, 0x3bd6, 0x40e0, \
> +                0x90, 0xd5, 0x6b, 0x60, 0x6a, 0xa5, 0x77, 0x30)
> +
>  /* Use internal device tree when starting UEFI application */
>  #define EFI_FDT_USE_INTERNAL NULL
>
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index e6de685e87..71bcde645b 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -17,6 +17,8 @@
>
>  efi_status_t efi_obj_list_initialized = OBJ_LIST_NOT_INITIALIZED;
>
> +efi_guid_t vendor_fdtfile_guid = VENDOR_FDTFILE_GUID;
> +
>  /*
>   * Allow unaligned memory access.
>   *
> @@ -26,6 +28,27 @@ void __weak allow_unaligned(void)
>  {
>  }
>
> +/**
> + * efi_init_fdtfile() - set EFI variable FdtFile
> + *
> + * Return:     status code
> + */
> +static efi_status_t efi_init_fdtfile(void)
> +{
> +       char *val;
> +
> +       val = env_get("fdtfile");
> +       if (!val)
> +               return EFI_SUCCESS;
> +
> +       return efi_set_variable_int(u"FdtFile",
> +                                   &vendor_fdtfile_guid,
> +                                   EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +                                   EFI_VARIABLE_RUNTIME_ACCESS |
> +                                   EFI_VARIABLE_READ_ONLY,
> +                                   strlen(val) + 1, val, false);
> +}
> +
>  /**
>   * efi_init_platform_lang() - define supported languages
>   *
> @@ -250,6 +273,13 @@ efi_status_t efi_init_obj_list(void)
>         if (ret != EFI_SUCCESS)
>                 goto out;
>
> +       /* Define EFI variable FdtFile */
> +       if (!CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)) {
> +               ret = efi_init_fdtfile();
> +               if (ret != EFI_SUCCESS)
> +                       goto out;
> +       }
> +
>         /* Indicate supported features */
>         ret = efi_init_os_indications();
>         if (ret != EFI_SUCCESS)
> --
> 2.40.1
>

Assuming my concerns above are figured out:

Reviewed-by: Simon Glass <sjg at chromium.org>

Regards,
SImon