Re: [PATCH] distro_bootcmd: change the default dtb search path to include default kernel dtbs directory

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

On 2021/9/17 22:25, Mark Kettenis wrote:
>> Date: Fri, 17 Sep 2021 19:42:29 +0800
>> Content-Language: en-US
>>
>> On 2021/9/17 19:34, Mark Kettenis wrote:
>>>> From: Qu Wenruo <wqu@suse.com>
>>>> Date: Fri, 17 Sep 2021 19:02:35 +0800
>>>>
>>>> When booting using U-boot -> systemd-boot (EFI payload) -> kernel on
>>>> RK3399, systemd-boot by some bug can't execute its "devicetree" key
>>>> correctly to load its proper dtb from files.
>>>>
>>>> In that case, it will use fallback dtb from U-boot, which can be
>>>> out-of-date, and on RK3399, even the latest U-boot contains out-of-date
>>>> dtb which can cause problems like invalid opp tables.
>>>>
>>>> And for systemd-boot, it doesn't provide any board specific dtb, but
>>>> completely relies on the EFI environment provided by U-boot, thus if we
>>>> can't find a good dtb, the fallback one will be used anyway.
>>>>
>>>> So this patch will workaround the problem by appending common linux dtbs
>>>> directory to the existing "efi_dtb_prefixes" so that for systemd-boot it
>>>> can use the existing fdt and boot properly.
>>>
>>> Why isn't the dtb installed in the standard location?
>>
>> Isn't standard location "/dtbs" other than "/dtb"?
>>
>> At least kernel puts its dtbs into "/dtbs" by default.
>
> Hmm, well, when I Alexander Graf introduced the code to load an
> updated DTB the intention was clearly to load it from the EFI System
> Partition (ESP).  I doubt that's where "the kernel" sticks them by
> default.  So I suspect the intention is that you copy the DTB to the
> /dtb directory on the ESP if needed.  Clearly that isn't what you've
> done on your system.

Well, for this U-boot > systemd-boot > kernel loading scheme,
systemd-boot is responsible to load the fdt.

But for now (the latest stable release), it doesn't support fdt loading
at all, and only recently get the ability:
https://github.com/systemd/systemd/pull/19417

So in theory, it doesn't need any fdt for systemd-boot.

It's more like a workaround (and not really needed now).

>
> I suspect things have moved on a bit as the discussion in the systemd
> thread shows.  One of the new developments is the EFI_DT_FIXUP_PROCOL
> feature that is clearly targeted at EFI bootloaders picking and
> loading the DT instead of U-Boot.

Yep, I didn't notice the development in systemd part.

>
> In this light I would be somewhat reluctant adding more directories to
> efi_dtb_prefixes.

Yeah, please discard the patch, it's really a workaround and not needed
anymore.

Thanks,
Qu

>
>>> Slowing the boot process down because distro's can't agree where to
>>> put the files is a bit lame...
>>
>> Would it be better to make the search path configurable at config time?
>>
>> By that each distro should config their dtbs search path, which could
>> further optimize the boot sequence by removing all other unnecessary
>> prefixes.
>
> Not in my opinion.  I consider it a bad thing if people configure
> U-Boot specifically for their distro.  The whole idea of distroboot
> and EFI in U-Boot is to be able to boot just any OS with a generic
> U-Boot binary.
>
>> Thanks,
>> Qu
>>
>>>
>>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>>> ---
>>>>    include/config_distro_bootcmd.h | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
>>>> index 2627c2a6a541..4ec87483eb65 100644
>>>> --- a/include/config_distro_bootcmd.h
>>>> +++ b/include/config_distro_bootcmd.h
>>>> @@ -151,7 +151,7 @@
>>>>    		"load ${devtype} ${devnum}:${distro_bootpart} "           \
>>>>    			"${fdt_addr_r} ${prefix}${efi_fdtfile}\0"         \
>>>>    	\
>>>> -	"efi_dtb_prefixes=/ /dtb/ /dtb/current/\0"                        \
>>>> +	"efi_dtb_prefixes=/ /dtb/ /dtb/current/ /dtbs/ /dtbs/current/\0"  \
>>>>    	"scan_dev_for_efi="                                               \
>>>>    		"setenv efi_fdtfile ${fdtfile}; "                         \
>>>>    		BOOTENV_EFI_SET_FDTFILE_FALLBACK                          \
>>>> --
>>>> 2.33.0
>>>>
>>>>
>>