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

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 4500 bytes --]

On Wed, Oct 25, 2023 at 09:57:44PM +0200, Heinrich Schuchardt wrote:
> On 10/25/23 20:23, Simon Glass wrote:
> > Hi Heinrich,
> > 
> > On Tue, 24 Oct 2023 at 18:02, Simon Glass <sjg@google.com> wrote:
> > > 
> > > Hi Heinrich,
> > > 
> > > On Mon, 23 Oct 2023 at 23:20, Heinrich Schuchardt
> > > <heinrich.schuchardt@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@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?
> 
> It would have been nice to put the person which made that comment on copy.
> 
> The truth lies in the world "supposed":
> 
> The idea of a device-tree that never needs to change is quite old and never
> became true on ARM devices.
> 
> We all know Linux tends to break both forward and backward compatibility of
> device-trees. Here is a nice example:
> 
> d0c6707ca423 ("arm64: dts: allwinner: H5: NanoPi Neo Plus2: phy-mode
> rgmii-id")
> 
> Driver changes broke forward and backwards compatibility of a lot of
> Allwinner boards.
> 
> Distros will continue to load the device-tree that matches the kernel to get
> the best possible board support and need to do this efficiently.

Right, OK. And I think we want to try and have things phrased in a more
neutral and less confrontational manner is part of the issue. Maybe:

In ideal circumstances, U-Boot will be able to expose the device-tree it
is using to boot any operation system. However, in some cases operating
systems need to load a specific device-tree rather than utilize the same
one that U-Boot is currently using. In this case there is a need to load
a specific device-tree binary from another location.

And as a more general concern I see right now, "fdt_file" and "fdtfile"
are both used today, including new rather than older platforms that
might avoid EFI_LOADER all the same, perhaps we should check for both?
Or do you instead want to get board maintainers to switch over, as
fdt_file isn't listed under doc/ today.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]