From: AKASHI Takahiro <takahiro.akashi@linaro.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/2] efi_loader: rework fdt handling in distro boot script
Date: Thu, 18 Oct 2018 11:07:32 +0900 [thread overview]
Message-ID: <20181018020731.GF32578@linaro.org> (raw)
In-Reply-To: <25b492ef-0c06-8675-c87a-bbf5432165e9@suse.de>
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.akashi@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
> >
next prev parent reply other threads:[~2018-10-18 2:07 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-12 5:07 [U-Boot] [PATCH 1/2] efi_loader: rework fdt handling in distro boot script AKASHI Takahiro
2018-10-12 5:07 ` [U-Boot] [PATCH 2/2] ARM: qemu-arm: define fdt_addr_r AKASHI Takahiro
2018-10-15 1:01 ` Tuomas Tynkkynen
2018-10-15 5:14 ` AKASHI Takahiro
2018-10-16 13:04 ` Alexander Graf
2018-10-17 22:25 ` Tuomas Tynkkynen
2018-10-18 7:25 ` Alexander Graf
2018-10-19 6:33 ` AKASHI Takahiro
2018-10-19 7:46 ` Alexander Graf
2018-10-19 8:17 ` AKASHI Takahiro
2018-10-24 10:36 ` Tuomas Tynkkynen
2018-10-16 13:15 ` [U-Boot] [PATCH 1/2] efi_loader: rework fdt handling in distro boot script Alexander Graf
2018-10-18 2:07 ` AKASHI Takahiro [this message]
2018-10-18 7:31 ` Alexander Graf
2018-10-19 7:20 ` AKASHI Takahiro
2018-10-19 7:31 ` Alexander Graf
2018-10-19 8:32 ` AKASHI Takahiro
2018-10-19 8:50 ` Alexander Graf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181018020731.GF32578@linaro.org \
--to=takahiro.akashi@linaro.org \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.