On Tue, Oct 16, 2018 at 03:15:13PM +0200, Alexander Graf wrote:
>
>
> On 12.10.18 07:07, AKASHI Takahiro wrote:
> > The current scenario for default UEFI booting, scan_dev_for_efi, has
> > several issues:
> > * invoke 'bootmgr' only if BOOTEFI_NAME binary does exit even though
> > 'bootmgr' can and should work independently whether or not the binary
> > exist,
> > * always assume that a 'fdtfile' variable is defined,
> > * redundantly check for 'fdt_addr_r' in boot_efi_binary
> >
> > In this patch, all the issues above are sorted out.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org>
> > ---
> > include/config_distro_bootcmd.h | 43 ++++++++++++++++-----------------
> > 1 file changed, 21 insertions(+), 22 deletions(-)
> >
> > diff --git a/include/config_distro_bootcmd.h
> > b/include/config_distro_bootcmd.h
> > index 373fee78a999..76e12b7bf4ee 100644
> > --- a/include/config_distro_bootcmd.h
> > +++ b/include/config_distro_bootcmd.h
> > @@ -124,42 +124,41 @@
> >
> > #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" \
> > + "${fdt_addr_r} ${prefix}${efi_fdtfile};" \
> > + "if fdt addr ${fdt_addr_r}; then " \
> > + "setenv efi_fdt_addr ${fdt_addr_r}; " \
> > + "else; " \
> > + "setenv efi_fdt_addr ${fdtcontroladdr}; " \
> > + "fi;\0" \
> > \
> > - "efi_dtb_prefixes=/ /dtb/ /dtb/current/\0" \
> > - "scan_dev_for_efi=" \
> > + "set_efi_fdt_addr=" \
> > "setenv efi_fdtfile ${fdtfile}; " \
> > BOOTENV_EFI_SET_FDTFILE_FALLBACK \
> > - "for prefix in ${efi_dtb_prefixes}; do " \
>
> I fail to see where the prefix logic went? Without that, our distro's
> dtb loading will break.
Oops, I missed it.
>
> > - "if test -e ${devtype} " \
> > - "${devnum}:${distro_bootpart} " \
> > - "${prefix}${efi_fdtfile}; then " \
> > - "run load_efi_dtb; " \
> > - "fi;" \
> > - "done;" \
> > + "if test x${efi_fdtfile} != x -a x{$fdt_addr_r} != x ; then " \
> > + "run load_efi_dtb; " \
> > + "else; " \
> > + "setenv efi_fdt_addr ${fdtcontroladdr}; " \
> > + "fi; " \
>
> Just unconditionally set efi_fdt_addr=$fdtcontrolladdr before the check
> (and invocation of load_efi_dtb).
OK.
> That way you don't need to check for
> the failure case in load_efi_dtb again.
Well, I think that we need a check for validity with "fdt addr" command
just in case that a fdt file exist but its content be corrupted.
My modified version looks like:
===8<===
#define BOOTENV_SHARED_EFI \
"boot_efi_binary=" \
"load ${devtype} ${devnum}:${distro_bootpart} " \
"${kernel_addr_r} efi/boot/"BOOTEFI_NAME"; " \
"bootefi ${kernel_addr_r} ${efi_fdt_addr};\0" \
\
"load_efi_dtb=" \
"efi_dtb_prefixes=/ /dtb/ /dtb/current/" \
"for prefix in ${efi_dtb_prefixes}; do " \
"load ${devtype} ${devnum}:${distro_bootpart} " \
"${fdt_addr_r} ${prefix}${efi_fdtfile};" \
"if fdt addr ${fdt_addr_r}; then " \
"setenv efi_fdt_addr ${fdt_addr_r}; " \
"fi; " \
"done;\0" \
\
"set_efi_fdt_addr=" \
"efi_fdtfile=${fdtfile}; " \
BOOTENV_EFI_SET_FDTFILE_FALLBACK \
"setenv efi_fdt_addr ${fdtcontroladdr}; " \
"if test x${efi_fdtfile} != x -a x{$fdt_addr_r} != x ; then " \
"run load_efi_dtb; " \
"fi;\0" \
"setenv efi_fdtfile\0" \
\
"scan_dev_for_efi=" \
"run set_efi_fdt_addr; " \
"bootefi bootmgr ${efi_fdt_addr};" \
"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; " \
"echo EFI LOAD FAILED: continuing...; " \
"fi; " \
"setenv efi_fdt_addr\0"
===>8===
(not tested yet)
-Takahiro Akashi
> Alex
>
> > + "setenv efi_fdtfile\0" \
> > + \
> > + "efi_dtb_prefixes=/ /dtb/ /dtb/current/\0" \
> > + "scan_dev_for_efi=" \
> > + "run set_efi_fdt_addr; " \
> > + "bootefi bootmgr ${efi_fdt_addr};" \
> > "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; " \
> > "echo EFI LOAD FAILED: continuing...; " \
> > - "fi; " \
> > - "setenv efi_fdtfile\0"
> > + "fi; " \
> > + "setenv efi_fdt_addr\0"
> > #define SCAN_DEV_FOR_EFI "run scan_dev_for_efi;"
> > #else
> > #define BOOTENV_SHARED_EFI
> >
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
