[U-Boot] [PATCH] distro: load FDT from any partition on boot device

[Alexander Graf]

On 05.10.17 15:27, Rob Clark wrote:
> On Thu, Oct 5, 2017 at 8:39 AM, Alexander Graf <agraf at suse.de> wrote:
>>
>>
>> On 04.10.17 22:16, Rob Clark wrote:
>>>
>>> In the EFI_LOADER boot path, we were only checking the FAT partition
>>> containing the EFI payload for dtb files.  But this is somewhat of a
>>> fiction.  In reality there will be one small (V)FAT partition containing
>>> grub (or whatever the payload may be), and a second boot partition
>>> containing kernel/initrd/fdt (typically ext4).  It is this second
>>> partition where we should be looking for a FDT to load.
>>>
>>> So instead scan all the partitions of the disk containing the EFI
>>> payload.  This matches where grub looks for kernel/initrd (barring
>>> custom grub.cfg, in which case the user can use grub's 'devicetree'
>>> command to load the correct FDT).
>>>
>>> The other option is somehow passing the ${fdtfile} to grub so that it
>>> can load the FDT based on selected kernel version location (which grub
>>> knows) and SoC/board specific ${fdtfile} (which grub does not know).
>>>
>>> Signed-off-by: Rob Clark <robdclark at gmail.com>
>>> ---
>>>    include/config_distro_bootcmd.h | 34 +++++++++++++++++++++++-----------
>>>    1 file changed, 23 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/include/config_distro_bootcmd.h
>>> b/include/config_distro_bootcmd.h
>>> index e232a62996..58b2fe3371 100644
>>> --- a/include/config_distro_bootcmd.h
>>> +++ b/include/config_distro_bootcmd.h
>>> @@ -126,25 +126,37 @@
>>>                  "fi\0"
>>> \
>>>          \
>>>          "load_efi_dtb="
>>> \
>>> -               "load ${devtype} ${devnum}:${distro_bootpart} "
>>> \
>>> -                       "${fdt_addr_r} ${prefix}${efi_fdtfile}\0"
>>> \
>>> +               "load ${devtype} ${devnum}:${dtb_devp} "
>>> \
>>> +                       "${fdt_addr_r} ${prefix}${efi_fdtfile} && "
>>> \
>>> +               "run boot_efi_binary\0"
>>> \
>>>          \
>>>          "efi_dtb_prefixes=/ /dtb/ /dtb/current/\0"
>>> \
>>> -       "scan_dev_for_efi="
>>> \
>>> +       "scan_dev_for_dtb="
>>> \
>>>                  "setenv efi_fdtfile ${fdtfile}; "
>>> \
>>>                  BOOTENV_EFI_SET_FDTFILE_FALLBACK
>>> \
>>> -               "for prefix in ${efi_dtb_prefixes}; do "
>>> \
>>> -                       "if test -e ${devtype} "
>>> \
>>> -                                       "${devnum}:${distro_bootpart} "
>>> \
>>> -                                       "${prefix}${efi_fdtfile}; then "
>>> \
>>> -                               "run load_efi_dtb; "
>>> \
>>> -                       "fi;"
>>> \
>>> -               "done;"
>>> \
>>> +               "part list ${devtype} ${devnum} dtb_devplist; "
>>> \
>>
>>
>> part list spawns 128 error messages for me on a USB stick with an iso dd'ed
>> onto it. I'm not sure we want to do that twice during boot.
> 
> I'm not sure how to avoid doing two 'part list's since one we want to
> find *all* the partitions, not just the -bootable ones..
> 
> I'd suggest you might be better off fixing the root problem here ;-)

Most probably, yes.

> 
>>> +               "env exists dtb_devplist || setenv dtb_devplist "
>>> \
>>> +                       "${distro_bootpart}; "
>>> \
>>> +               "for dtb_devp in ${dtb_devplist}; do "
>>> \
>>> +                       "for prefix in ${efi_dtb_prefixes}; do "
>>> \
>>> +                               "if test -e ${devtype} "
>>> \
>>> +                                               "${devnum}:${dtb_devp} "
>>> \
>>> +
>>> "${prefix}${efi_fdtfile};"\
>>> +                                               " then "
>>> \
>>> +                                       "echo Found DTB ${devtype} "
>>> \
>>> +                                               "${devnum}:${dtb_devp} "
>>> \
>>> +
>>> "${prefix}${efi_fdtfile};"\
>>> +                                       "run load_efi_dtb; "
>>> \
>>> +                               "fi;"
>>> \
>>> +                       "done; "
>>> \
>>> +               "done; "
>>> \
>>> +               "run boot_efi_binary\0"
>>> \
>>
>>
>> This will run the EFI binary twice if we find a DT on disk. Or really
>> nr_dtb_parts_found + 1 times :).
> 
> only if the EFI binary returns, which isn't usually going to be the
> case for distro boot

In a naive view, yes :). All our grub configs on iso have a "Boot local 
disk" entry which is simply "exit" in UEFI. For those it definitely can 
occur. And I make quite heavy use of it too ;).

> That said, if grub scripting supported "break", this would make this much easier

Or alternatively set a variable that skips efi execution after you 
executed once. Then reset the variable at the end of the loop.

> 
>> We also don't want to loop through 50 other partitions if the 1st one
>> already contained a dtb. Instead the loop really should end after the first
>> successful boot attempt on that device.
>>
>> (other distro targets should still get boot attempts, you may want to exit
>> grub from dhcp to enter grub on scsi).
>>
>> Furthermore I remember that Andreas worked in that area too before, let's
>> make sure to CC him.
> 
> If someone has a better patch, then I'm fine to go with that.  I just
> needed something to get fedora booting

I'm not saying better patch, I want to get his input because he worked 
on the same thing before, so he cares and knows which makes him an ideal 
reviewer :).


Alex