Re: [U-Boot] [PATCH v3 1/1] efi_loader: rework fdt handling in distro boot script

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.akashi@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?

Thanks, -Takahiro Akashi

Alex