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

[Caleb Connolly]

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 at 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 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.

(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...
>>>
>>> Well, yes and no.  Given the brief summary here, I bet this was just
>>> like when phy-mode and am335x platforms had DT compatibility broken and
>>> the answer was that it was OK because the DT was incorrectly describing
>>> hardware. So this is the reminder that there are cases of breaking DT
>>> compatibility that are allowed. Even if the DT has been out (and wrong)
>>> for several years.
>>>
>>> That's not the main point of this thread and I don't want to derail
>>> things further along this point, I just want to note that the details
>>> here reminded me of when things are allowed to be incompatible with
>>> previous trees.
>>>
>>
> 
> Unfortunately I was off the list for a week or so, but I see this one
> so will reply here.
> 
>> You are conflating things:
>>
>> The EFI variable is a hint to the GRUB OS to find the correct
>> device-tree file without scanning hundreds of device-tree. You can not
>> build a compatibility check for it into U-Boot.
> 
> Actually we can...and I think that is what we should do.

No, how and where an EFI bootloader app chooses to load the FDT from is 
entirely implementation dependent. It may be extremely costly to compare 
compatible strings for every single dtb, and whether or not to do so is 
the decision of the distro anyways, all we can do in U-Boot is try to 
promote best practise (which clearly isn't compatible prop comparison 
for hundreds of DTBs...)

I'll note that systemd-uki does actually do this compatible prop check, 
but this would not be feasible for regular systemd-boot.
> 
>>
>> In distro-boot we are using the same value from environment variable
>> $fdtfile. Here you could check the compatible string but this might
>> break booting .
> 
> We need to figure that point out. The compatible string is
> programmatically is defined to be correct, so it is the filename that
> may/may not be useful. What am I missing?

Current distro images don't have a way to map from compatible strings to 
a filename, but they ship DTBs as normal files on the ESP.. They often 
have multiple kernel versions installed, and therefore multiple DTB 
directories which are versioned.

U-Boot doesn't know what boot entry is picked (and should not NEED to know).

Providing FdtFile as an EFI variable will make it possible for grub and 
systemd-boot to support the same kind of "fdtdir" property that extlinux 
does, this would simply enable versioned DTB loading for distros and 
make booting wayyyyy easier.

With regards to the design, I think a good follow-up patch would set a 
default value for $fdtfile (and consequently FdtFile) from the 
DEVICE_TREE build variable as a constant (maybe only if OF_UPSTREAM is 
enabled).

This would be correct in almost all cases (and wayyy better than the 
hacky fdtfile generating code we have in mach-snapdragon right now).
> 
> For distro-boot, do you mean the scripts? We are trying to deprecate
> those. Of course we have brought in all the same work-arounds, etc.,
> but with bootstd we can start to clean things up, I hope.
> 
> So let's think about how we can have U-Boot choose the right DT to boot with.
> 
> Regards,
> Simon

Heinrich, since it's been a while, if you aren't super interested in 
re-sending this patch I'd be happy to address the wording feedback and 
re-spin it, along with a patch setting the default $fdtfile (else I can 
send that as a follow-up).

I'll also open an issue for getting support for this added to systemd-boot.

-- 
Caleb (they/them)