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

[Simon Glass]

Hi Heinrich,

On Tue, 24 Oct 2023 at 18:02, Simon Glass <sjg at google.com> wrote:
>
> 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?

Did you see this comment?

Regards,
Simon

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