> Am 03.12.2018 um 03:22 schrieb AKASHI Takahiro <takahiro.aka...@linaro.org>:
>
>> On Sun, Dec 02, 2018 at 11:58:45PM +0100, Alexander Graf wrote:
>>
>>
>>> On 27.11.18 01:33, AKASHI Takahiro wrote:
>>> The current scenario for default UEFI booting, scan_dev_for_efi, has
>>> several issues:
>>> * load dtb dynamically even if its loacation (device) is not the same
>>> as BOOTEFI_NAME binary's, (reported by Alex)
>>> * invoke 'bootmgr' only if BOOTEFI_NAME binary does exit even though
>>> 'bootmgr' can and should work independently whether or not the binary
>>> exist,
>>> * in addition, invoke 'bootmgr' with dynamically-loaded dtb.
>>> This behavior is not expected. (reported by Alex)
>>> * always assume that a 'fdtfile' variable is defined,
>>> ("test -e ${devtype} ${devnum}:${distro_bootpart} "${prefix}${efi_fdtfile}"
>>> always returns true even if fdtfile is NULL with prefix=="/".)
>>> * redundantly check for 'fdt_addr_r' in boot_efi_binary
>>>
>>> In this patch, all the issues above are sorted out.
>>> Please note that the default behavior can be customized with:
>>> fdtfile: a dtb file name
>>> efi_dtb_prefixes: a list of paths for searching for a dtb file
>>>
>>> (this feature does work even without this patch.)
>>>
>>> Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org>
>>> ---
>>> include/config_distro_bootcmd.h | 38 +++++++++++++++++----------------
>>> 1 file changed, 20 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/include/config_distro_bootcmd.h
>>> b/include/config_distro_bootcmd.h
>>> index 373fee78a999..256698309eb9 100644
>>> --- a/include/config_distro_bootcmd.h
>>> +++ b/include/config_distro_bootcmd.h
>>> @@ -115,7 +115,7 @@
>>> */
>>> #define BOOTENV_EFI_SET_FDTFILE_FALLBACK \
>>> "if test -z \"${fdtfile}\" -a -n \"${soc}\"; then " \
>>> - "setenv efi_fdtfile ${soc}-${board}${boardver}.dtb; " \
>>> + "efi_fdtfile=${soc}-${board}${boardver}.dtb; " \
>>> "fi; "
>>> #else
>>> #define BOOTENV_EFI_SET_FDTFILE_FALLBACK
>>> @@ -124,26 +124,20 @@
>>>
>>> #define BOOTENV_SHARED_EFI \
>>> "boot_efi_binary=" \
>>> - "if fdt addr ${fdt_addr_r}; then " \
>>> - "bootefi bootmgr ${fdt_addr_r};" \
>>> - "else " \
>>> - "bootefi bootmgr ${fdtcontroladdr};" \
>>> - "fi;" \
>>> "load ${devtype} ${devnum}:${distro_bootpart} " \
>>> "${kernel_addr_r} efi/boot/"BOOTEFI_NAME"; " \
>>> - "if fdt addr ${fdt_addr_r}; then " \
>>> - "bootefi ${kernel_addr_r} ${fdt_addr_r};" \
>>> - "else " \
>>> - "bootefi ${kernel_addr_r} ${fdtcontroladdr};" \
>>> - "fi\0" \
>>> + "bootefi ${kernel_addr_r} ${efi_fdt_addr};\0" \
>>> \
>>> "load_efi_dtb=" \
>>> - "load ${devtype} ${devnum}:${distro_bootpart} " \
>>> - "${fdt_addr_r} ${prefix}${efi_fdtfile}\0" \
>>> + "load ${devtype} ${devnum}:${distro_bootpart} " \
>>> + "${fdt_addr_r} ${prefix}${efi_fdtfile}; " \
>>> + "if fdt addr ${fdt_addr_r}; then " \
>>> + "efi_fdt_addr=${fdt_addr_r}; " \
>>> + "fi;\0" \
>>> \
>>> "efi_dtb_prefixes=/ /dtb/ /dtb/current/\0" \
>>> - "scan_dev_for_efi=" \
>>> - "setenv efi_fdtfile ${fdtfile}; " \
>>> + "set_efi_fdt_addr=" \
>>> + "efi_fdtfile=${fdtfile}; " \
>>> BOOTENV_EFI_SET_FDTFILE_FALLBACK \
>>> "for prefix in ${efi_dtb_prefixes}; do " \
>>> "if test -e ${devtype} " \
>>> @@ -151,19 +145,26 @@
>>> "${prefix}${efi_fdtfile}; then " \
>>> "run load_efi_dtb; " \
>>> "fi;" \
>>> - "done;" \
>>> + "done;\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"; " \
>>> + "efi_fdt_addr=${fdtcontroladdr}; " \
>>> + "if test -n \"${fdtfile}\"; then " \
>>> + "run set_efi_fdt_addr; " \
>>> + "fi; " \
>>> "run boot_efi_binary; " \
>>> "echo EFI LOAD FAILED: continuing...; " \
>>> - "fi; " \
>>> - "setenv efi_fdtfile\0"
>>
>> This drops the bit that unsets the variable. So after the script, your
>> env will be different from before the script.
>
> Yeah, but
>
>> Please run
>>
>> U-Boot# printenv
>> U-Boot# boot (w/o any efi payload to boot)
>> U-Boot# printenv
>>
>> Then diff the two environments. They should be identical.
>
> Are you sure? When I try this on unmodified v2018.11,
> I also see some difference:
>
> 18a19
>> devnum=0
> 37a39
>> scsi_need_init=false
> 44c46
> < Environment size: 3614/65532 bytes
> ---
>> Environment size: 3644/65532 bytes
>
> It seems that 'devnum' and 'scsi_need_init' come from 'bootcmd_scsi0.'
> Can you clarify your point a bit more, please?
That is a bug :). Thanks for catching it? Can you please send a patch to fix it?
The problem is that a saveenv after a fsiled boot attempt should not contain
intermediate variables from that attempt.
Alex
>
> Thanks,
> -Takahiro Akashi
>
>>
>>
>> Alex
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
