[U-Boot] [PATCH 3/6] efi_loader: Search .dtb on non-EFI partitions

[Andreas Färber]

Am 12.07.2016 um 09:25 schrieb Alexander Graf:
> 
> 
> On 12.07.16 06:21, Andreas Färber wrote:
>> A UEFI setup will typically have an EFI FAT partition, but device trees
>> are more likely to be located on, e.g., Linux volumes. By convention the
>> EFI partition will usually be placed first, so that we would find the
>> EFI removable media binary and enter it without device tree from disk.
>>
>> Therefore after finding an EFI removable media binary, probe other
>> partitions on the same device for .dtb files.
>>
>> In the default case the behavior will be unchanged in that only the
>> first partition is searched.
>> In the case that there are two partitions marked bootable and the first
>> being EFI, we will first check the first partition, then the second
>> partition for the .dtb.
>> Duplicate loading will only result from non-first EFI partition order.
>>
>> Cc: Alexander Graf <agraf at suse.de>
>> Signed-off-by: Andreas Färber <afaerber at suse.de>
>> ---
>>  include/config_distro_bootcmd.h | 16 ++++++++++++----
>>  1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
>> index 087f576..eadec2e 100644
>> --- a/include/config_distro_bootcmd.h
>> +++ b/include/config_distro_bootcmd.h
>> @@ -124,8 +124,7 @@
>>  		"fi\0"                                                    \
>>  	\
>>  	"load_efi_dtb="                                                   \
>> -		"load ${devtype} ${devnum}:${distro_bootpart} "           \
>> -			"${fdt_addr_r} "                                  \
>> +		"load ${devtype} ${devnum}:${efi_bootpart} ${fdt_addr_r} "\
>>  			"${prefix}${dtb_prefix}${efi_fdtfile}\0"          \
>>  	\
>>  	"efi_dtb_prefixes=\"\" dtb/ dtb/current/\0"                       \
>> @@ -133,7 +132,7 @@
>>  		"for prefix in ${boot_prefixes}; do "                     \
>>  			"for dtb_prefix in ${efi_dtb_prefixes}; do "      \
>>  				"if test -e ${devtype} "                  \
>> -					"${devnum}:${distro_bootpart} "   \
>> +					"${devnum}:${efi_bootpart} "      \
>>  					"${prefix}${dtb_prefix}"          \
>>  					"${efi_fdtfile}; then "           \
>>  					"run load_efi_dtb; "              \
>> @@ -143,13 +142,22 @@
>>  	"scan_dev_for_efi="                                               \
>>  		"setenv efi_fdtfile ${fdtfile}; "                         \
>>  		BOOTENV_EFI_SET_FDTFILE_FALLBACK                          \
>> -		"run scan_dev_for_efi_fdt; "                              \
>>  		"if test -e ${devtype} ${devnum}:${distro_bootpart} "     \
>>  					"efi/boot/"BOOTEFI_NAME"; then "  \
>>  				"echo Found EFI removable media binary "  \
>>  					"efi/boot/"BOOTEFI_NAME"; "       \
>> +				"for efi_bootpart in ${devplist}; do "    \
>> +					"echo Scanning for device tree "  \
>> +						"on ${devtype} ${devnum}" \
>> +						":${efi_bootpart}...; "   \
> 
> Printing can be quite slow on certain devices (think LCDs with uncached
> vram access), but have a *lot* of partitions (think Android partition
> scheme). I don't think we should print out every partition we're
> searching in, only the ones we find something at.
> 
> Maybe something like
> 
>   Found EFI removable media binary in efi/boot/bootaa64.efi.
>   Searching for dtb on mmc 0 ...
>   Found dtb on mmc 0:1 boot/dtb/foobar.dtb
> 
> would be a sensible amount of debug output :).

No, please read again! I am not printing every partition we're
searching, only in the new inner loop, which will likely be exactly
twice, once for FAT, once for ext.

For the "else" below we already get that output from the regular distro
framework.

In particular "mmc 0" is not enough and absolutely not helping. I need
to see that we're searching the right partition if we don't find a .dtb,
i.e. mmc 0:2.

> Also given that we're now running 3 cascaded for loops, it might be
> about time to think about how to break out of the loops in case we
> actually found something, so that the "normal" case is faster.

True point, I noticed that as well. :)

Note that I thought about adding the vendor to $efi_dtb_prefixes but
couldn't think of a way other than looping at runtime.

Regards,
Andreas

> 
> 
> Alex
> 
>> +					"run scan_dev_for_efi_fdt; "      \
>> +				"done; "                                  \
>>  				"run boot_efi_binary; "                   \
>>  				"echo EFI LOAD FAILED: continuing...; "   \
>> +		"else "                                                   \
>> +			"setenv efi_bootpart ${distro_bootpart}; "        \
>> +			"run scan_dev_for_efi_fdt; "                      \
>> +			"setenv efi_bootpart; "                           \
>>  		"fi; "                                                    \
>>  		"setenv efi_fdtfile\0"
>>  #define SCAN_DEV_FOR_EFI "run scan_dev_for_efi;"
>>


-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)