* [U-Boot] [PATCH] distro: load FDT from any partition on boot device
@ 2017-10-04 20:16 Rob Clark
2017-10-05 12:39 ` Alexander Graf
0 siblings, 1 reply; 4+ messages in thread
From: Rob Clark @ 2017-10-04 20:16 UTC (permalink / raw)
To: u-boot
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@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; " \
+ "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" \
+ "scan_dev_for_efi=" \
"if test -e ${devtype} ${devnum}:${distro_bootpart} " \
"efi/boot/"BOOTEFI_NAME"; then " \
"echo Found EFI removable media binary " \
"efi/boot/"BOOTEFI_NAME"; " \
- "run boot_efi_binary; " \
+ "run scan_dev_for_dtb; " \
"echo EFI LOAD FAILED: continuing...; " \
"fi; " \
"setenv efi_fdtfile\0"
--
2.13.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] distro: load FDT from any partition on boot device
2017-10-04 20:16 [U-Boot] [PATCH] distro: load FDT from any partition on boot device Rob Clark
@ 2017-10-05 12:39 ` Alexander Graf
2017-10-05 13:27 ` Rob Clark
0 siblings, 1 reply; 4+ messages in thread
From: Alexander Graf @ 2017-10-05 12:39 UTC (permalink / raw)
To: u-boot
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@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.
> + "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 :).
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.
Alex
^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] distro: load FDT from any partition on boot device
2017-10-05 12:39 ` Alexander Graf
@ 2017-10-05 13:27 ` Rob Clark
2017-10-05 13:38 ` Alexander Graf
0 siblings, 1 reply; 4+ messages in thread
From: Rob Clark @ 2017-10-05 13:27 UTC (permalink / raw)
To: u-boot
On Thu, Oct 5, 2017 at 8:39 AM, Alexander Graf <agraf@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@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 ;-)
>> + "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
That said, if grub scripting supported "break", this would make this much easier
> 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
BR,
-R
^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] distro: load FDT from any partition on boot device
2017-10-05 13:27 ` Rob Clark
@ 2017-10-05 13:38 ` Alexander Graf
0 siblings, 0 replies; 4+ messages in thread
From: Alexander Graf @ 2017-10-05 13:38 UTC (permalink / raw)
To: u-boot
On 05.10.17 15:27, Rob Clark wrote:
> On Thu, Oct 5, 2017 at 8:39 AM, Alexander Graf <agraf@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@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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-10-05 13:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-04 20:16 [U-Boot] [PATCH] distro: load FDT from any partition on boot device Rob Clark
2017-10-05 12:39 ` Alexander Graf
2017-10-05 13:27 ` Rob Clark
2017-10-05 13:38 ` Alexander Graf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox