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

[Simon Glass]

Hi Heinrich,

On Wed, 25 Oct 2023 at 20:51, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
> On 10/25/23 22:28, Mark Kettenis wrote:
> >> Date: Wed, 25 Oct 2023 21:57:44 +0200
> >> From: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> >>
> >> On 10/25/23 20:23, Simon Glass wrote:
> >>> 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?
> >>
> >> 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.
>
> The 2020 event struck me because a Linux kernel would not even be
> compatible with the device-tree of the same kernel release.
>
> This year I once again had issues with booting an Allwinner board with a
> device-tree from a different Debian kernel version. Both backward and
> forward compatibility were broken. But at least I could boot with a
> matching device-tree.

Oh dear.

>
> >
> >> 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.  Even if there is full backward/forward compatibility, you
> > probably want the latest device-tree to make sure you get the most
> > complete hardware support.
> >
> > But this shouldn't be used as an argument to not care about
> > backward/forward compatibility.
>
> Nobody in this thread suggested to not care about forward and backward
> compatibility.

The key thing we need to agree on is that the compatible string is the
source of truth for booting. You check the model compatible and search
for a matching DT with the best match, taking into account board
revisions, etc. It is defined to be correct. If for some reason this
process is wrong, then the board *should not boot*.

So anything which suggests that the above is not correct, or doesn't
always work, or doesn't work well with EFI...is really going in the
wrong direction.

Of course there will be workarounds, like this one, but the 'correct'
way of doing it must still work. Nor is it 'slow' to do this, as I
have demonstrated.

Perhaps we should add a compatible check into U-Boot, to make sure we
are not choosing the wrong DT? At least for me, trying to get the
filename right is a bit of a pain, with so many characters looking the
same.

Regards,
Simon