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

[Tom Rini <trini@konsulko.com>]

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

On Sun, Mar 30, 2025 at 04:38:12PM +0200, Caleb Connolly wrote:
> Hi Heinrich,
> 
> On 3/28/25 15:18, Heinrich Schuchardt wrote:
> > On 28.03.25 14:00, Caleb Connolly wrote:
> > > 
> > > 
> > > On 3/28/25 13:01, Simon Glass wrote:
> > > > Hi Caleb,
> > > > 
> > > > On Sun, 23 Mar 2025 at 12:39, Caleb Connolly
> > > > <caleb.connolly@linaro.org> wrote:
> > > > > 
> > > > > Hi all,
> > > > > 
> > > > > Reviving this as it is still very much an issue, and
> > > > > especially relevant
> > > > > for Qualcomm platforms.
> > > > > 
> > > > > On 11/3/23 20:44, Simon Glass wrote:
> > > > > > Hi Heinrich,
> > > > > > 
> > > > > > On Wed, 25 Oct 2023 at 15:22, Heinrich Schuchardt
> > > > > > <heinrich.schuchardt@canonical.com> wrote:
> > > > > > > 
> > > > > > > On 10/25/23 23:13, Tom Rini wrote:
> > > > > > > > On Wed, Oct 25, 2023 at 10:28:05PM +0200, Mark Kettenis wrote:
> > > > > > > > > > Date: Wed, 25 Oct 2023 21:57:44 +0200
> > > > > > > > > > From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > > > > > > > > > 
> > > > > > > > > > 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.
> > > > > > > > > 
> > > > > > > > > Well, that happened in 2020.  Things have gotten better over time.
> > > > > 
> > > > > (kinda off-topic context on DT version compat)
> > > > > 
> > > > >   From what I've seen, there is not yet very much infrastructure or
> > > > > common practise in place in the kernel to handle parsing DTB in a
> > > > > backwards compatible way. For example there has been efforts
> > > > > to simplify
> > > > > the dwc3 devicetree for Qualcomm platforms with a series dating back
> > > > > about as far as this U-Boot patch!
> > > > > 
> > > > > https://lore.kernel.org/linux-arm-msm/20250318-dwc3-refactor-
> > > > > v5-1-90ea6e5b3ba4@oss.qualcomm.com/
> > > > > 
> > > > > Earlier versions attempted to convert the older DTS to the newer format
> > > > > internally, but after much discussion it was decided that this wasn't
> > > > > really feasible, instead the new approach is to duplicate the entire
> > > > > dwc3 driver to maintain DT compatibility.
> > > > > 
> > > > > It's obviously good to see compatibility taken seriously,
> > > > > since it seems
> > > > > clear that if we ever want to treat DT as firmware, the
> > > > > kernel will need
> > > > > to do this (and eventually vendors could ship laptops with
> > > > > DT out of the
> > > > > box which works with upstream -- crazier things have happened).
> > > > > 
> > > > > But I think in the mean time we still want to be able to drive distro
> > > > > adoption, and the way I see it teaching GRUB and systemd-boot about a
> > > > > new FdtFile EFI variable is going to make that way simpler...
> > 
> > There was a discussion in the context of EBBR if this is the right way
> > forward. And the general opinion was that the compatible string should
> > be used for matching the dtb files.
> 
> Ideally, the file name should contain the same information as the compatible
> string, or more. Currently in upstream there are devices with different
> display variants that have identical compatible strings but different dtb
> files.

I've asked before for clarification on if that's allowed or a bug and
not really gotten an answer. It sure feels wrong and I think the
argument is that devices doing things like that aren't likely to be
general purpose anyhow.

-- 
Tom

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