All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v2 0/3] efi_loader: improve boot sequence in distro_bootcmd
@ 2018-10-22  4:40 AKASHI Takahiro
  2018-10-22  4:40 ` [U-Boot] [PATCH v2 1/3] efi_loader: rework fdt handling in distro boot script AKASHI Takahiro
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: AKASHI Takahiro @ 2018-10-22  4:40 UTC (permalink / raw)
  To: u-boot

The current distro_bootcmd has several issues regarding efi boot.
(See the patch#1 for details.)

Patch#1: fix distro's issues and make its intent clear
Patch#2,#3: address related issues on qemu-arm

Please note that patch#2 is now rebased on Bin's patch[1].

[1] https://lists.denx.de/pipermail/u-boot/2018-October/344421.html

Changes in v2 (Oct 22, 2018):
* rewrite my previous changes after Alex's comments, including
     - boot bootmgr only once before searching for boot binary
     - dtb must be loaded from the same device with boot binary's
* add patch#2 as part of this patch set, in particular adding CONFIG_SYS

Thanks,
-Takahio Akashi

AKASHI Takahiro (3):
  efi_loader: rework fdt handling in distro boot script
  ARM: qemu-arm: rework Kconfig
  ARM: qemu-arm: define fdt_addr_r

 arch/arm/mach-qemu/Kconfig       | 18 ++++++++-------
 board/emulation/qemu-arm/Kconfig |  6 +++++
 include/config_distro_bootcmd.h  | 38 +++++++++++++++++---------------
 include/configs/qemu-arm.h       |  1 +
 4 files changed, 37 insertions(+), 26 deletions(-)

-- 
2.19.0

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v2 1/3] efi_loader: rework fdt handling in distro boot script
  2018-10-22  4:40 [U-Boot] [PATCH v2 0/3] efi_loader: improve boot sequence in distro_bootcmd AKASHI Takahiro
@ 2018-10-22  4:40 ` AKASHI Takahiro
  2018-10-22  7:22   ` AKASHI Takahiro
  2018-10-22  4:40 ` [U-Boot] [PATCH v2 2/3] ARM: qemu-arm: rework Kconfig AKASHI Takahiro
  2018-10-22  4:40 ` [U-Boot] [PATCH v2 3/3] ARM: qemu-arm: define fdt_addr_r AKASHI Takahiro
  2 siblings, 1 reply; 10+ messages in thread
From: AKASHI Takahiro @ 2018-10-22  4:40 UTC (permalink / raw)
  To: u-boot

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"
+		"fi;\0"
 #define SCAN_DEV_FOR_EFI "run scan_dev_for_efi;"
+#define BOOT_EFI_BOOT_MANAGER "bootefi bootmgr;"
 #else
 #define BOOTENV_SHARED_EFI
 #define SCAN_DEV_FOR_EFI
+#define BOOT_EFI_BOOT_MANAGER
 #endif
 
 #ifdef CONFIG_SATA
@@ -409,6 +410,7 @@
 	BOOT_TARGET_DEVICES(BOOTENV_DEV)                                  \
 	\
 	"distro_bootcmd=" BOOTENV_SET_SCSI_NEED_INIT                      \
+		BOOT_EFI_BOOT_MANAGER					  \
 		"for target in ${boot_targets}; do "                      \
 			"run bootcmd_${target}; "                         \
 		"done\0"
-- 
2.19.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v2 2/3] ARM: qemu-arm: rework Kconfig
  2018-10-22  4:40 [U-Boot] [PATCH v2 0/3] efi_loader: improve boot sequence in distro_bootcmd AKASHI Takahiro
  2018-10-22  4:40 ` [U-Boot] [PATCH v2 1/3] efi_loader: rework fdt handling in distro boot script AKASHI Takahiro
@ 2018-10-22  4:40 ` AKASHI Takahiro
  2018-10-22  4:40 ` [U-Boot] [PATCH v2 3/3] ARM: qemu-arm: define fdt_addr_r AKASHI Takahiro
  2 siblings, 0 replies; 10+ messages in thread
From: AKASHI Takahiro @ 2018-10-22  4:40 UTC (permalink / raw)
  To: u-boot

Define a missing CONFIG_SYS_SOC and move some CONFIG_SYS_* to a more
canonical place (i.e. under board).

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 arch/arm/mach-qemu/Kconfig       | 18 ++++++++++--------
 board/emulation/qemu-arm/Kconfig |  6 ++++++
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-qemu/Kconfig b/arch/arm/mach-qemu/Kconfig
index a2e4b98b8887..d75a95183a75 100644
--- a/arch/arm/mach-qemu/Kconfig
+++ b/arch/arm/mach-qemu/Kconfig
@@ -3,22 +3,24 @@ if ARCH_QEMU
 config SYS_VENDOR
 	default "emulation"
 
-config SYS_BOARD
-	default "qemu-arm"
+config SYS_SOC
+	default "qemu"
 
-config SYS_CONFIG_NAME
-	default "qemu-arm"
-
-endif
+choice
+	prompt "QEMU cpu type"
 
 config TARGET_QEMU_ARM_32BIT
-	bool "Support qemu_arm"
+	bool "Arm"
 	depends on ARCH_QEMU
 	select ARCH_SUPPORT_PSCI
 	select CPU_V7A
 	select SYS_ARCH_TIMER
 
 config TARGET_QEMU_ARM_64BIT
-	bool "Support qemu_arm64"
+	bool "AArch64"
 	depends on ARCH_QEMU
 	select ARM64
+
+endchoice
+
+endif
diff --git a/board/emulation/qemu-arm/Kconfig b/board/emulation/qemu-arm/Kconfig
index d1c08c2f6a80..ef49e4e85f04 100644
--- a/board/emulation/qemu-arm/Kconfig
+++ b/board/emulation/qemu-arm/Kconfig
@@ -1,5 +1,11 @@
 if TARGET_QEMU_ARM_32BIT || TARGET_QEMU_ARM_64BIT
 
+config SYS_BOARD
+	default "qemu-arm"
+
+config SYS_CONFIG_NAME
+	default "qemu-arm"
+
 config SYS_TEXT_BASE
 	default 0x00000000
 
-- 
2.19.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v2 3/3] ARM: qemu-arm: define fdt_addr_r
  2018-10-22  4:40 [U-Boot] [PATCH v2 0/3] efi_loader: improve boot sequence in distro_bootcmd AKASHI Takahiro
  2018-10-22  4:40 ` [U-Boot] [PATCH v2 1/3] efi_loader: rework fdt handling in distro boot script AKASHI Takahiro
  2018-10-22  4:40 ` [U-Boot] [PATCH v2 2/3] ARM: qemu-arm: rework Kconfig AKASHI Takahiro
@ 2018-10-22  4:40 ` AKASHI Takahiro
  2018-10-24 10:43   ` Tuomas Tynkkynen
  2 siblings, 1 reply; 10+ messages in thread
From: AKASHI Takahiro @ 2018-10-22  4:40 UTC (permalink / raw)
  To: u-boot

This variable, fdt_addr_t, is missing in the current qemu-arm.h while it
seems to be mandatory, at least, to run distro_bootcmd as expected.
So just add its definition. A size of 1MB would be enough.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 include/configs/qemu-arm.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h
index 91fb8d47edf8..0e66f946dde5 100644
--- a/include/configs/qemu-arm.h
+++ b/include/configs/qemu-arm.h
@@ -55,6 +55,7 @@
 	"fdt_high=0xffffffff\0" \
 	"initrd_high=0xffffffff\0" \
 	"fdt_addr=0x40000000\0" \
+	"fdt_addr_r=0x40100000\0" \
 	"scriptaddr=0x40200000\0" \
 	"pxefile_addr_r=0x40300000\0" \
 	"kernel_addr_r=0x40400000\0" \
-- 
2.19.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v2 1/3] efi_loader: rework fdt handling in distro boot script
  2018-10-22  4:40 ` [U-Boot] [PATCH v2 1/3] efi_loader: rework fdt handling in distro boot script AKASHI Takahiro
@ 2018-10-22  7:22   ` AKASHI Takahiro
  2018-10-22  7:42     ` Alexander Graf
  0 siblings, 1 reply; 10+ messages in thread
From: AKASHI Takahiro @ 2018-10-22  7:22 UTC (permalink / raw)
  To: u-boot

On Mon, Oct 22, 2018 at 01:40:05PM +0900, 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"
> +		"fi;\0"
>  #define SCAN_DEV_FOR_EFI "run scan_dev_for_efi;"
> +#define BOOT_EFI_BOOT_MANAGER "bootefi bootmgr;"
>  #else
>  #define BOOTENV_SHARED_EFI
>  #define SCAN_DEV_FOR_EFI
> +#define BOOT_EFI_BOOT_MANAGER
>  #endif
>  
>  #ifdef CONFIG_SATA
> @@ -409,6 +410,7 @@
>  	BOOT_TARGET_DEVICES(BOOTENV_DEV)                                  \
>  	\
>  	"distro_bootcmd=" BOOTENV_SET_SCSI_NEED_INIT                      \
> +		BOOT_EFI_BOOT_MANAGER					  \

The last-minute change may have introduced a problem.
By moving "bootmgr" from scan_dev_for_efi" to here,
grub fails to start a grub.cfg menu, saying
  "error: no such device: /.disk/info."

Do you have any ideas?

-Takahiro Akashi


>  		"for target in ${boot_targets}; do "                      \
>  			"run bootcmd_${target}; "                         \
>  		"done\0"
> -- 
> 2.19.0
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v2 1/3] efi_loader: rework fdt handling in distro boot script
  2018-10-22  7:22   ` AKASHI Takahiro
@ 2018-10-22  7:42     ` Alexander Graf
  2018-10-23  2:08       ` AKASHI Takahiro
  0 siblings, 1 reply; 10+ messages in thread
From: Alexander Graf @ 2018-10-22  7:42 UTC (permalink / raw)
  To: u-boot



On 22.10.18 08:22, AKASHI Takahiro wrote:
> On Mon, Oct 22, 2018 at 01:40:05PM +0900, 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"
>> +		"fi;\0"
>>  #define SCAN_DEV_FOR_EFI "run scan_dev_for_efi;"
>> +#define BOOT_EFI_BOOT_MANAGER "bootefi bootmgr;"
>>  #else
>>  #define BOOTENV_SHARED_EFI
>>  #define SCAN_DEV_FOR_EFI
>> +#define BOOT_EFI_BOOT_MANAGER
>>  #endif
>>  
>>  #ifdef CONFIG_SATA
>> @@ -409,6 +410,7 @@
>>  	BOOT_TARGET_DEVICES(BOOTENV_DEV)                                  \
>>  	\
>>  	"distro_bootcmd=" BOOTENV_SET_SCSI_NEED_INIT                      \
>> +		BOOT_EFI_BOOT_MANAGER					  \
> 
> The last-minute change may have introduced a problem.
> By moving "bootmgr" from scan_dev_for_efi" to here,
> grub fails to start a grub.cfg menu, saying
>   "error: no such device: /.disk/info."
> 
> Do you have any ideas?

I would guess that bootmgr doesn't set the loaded image target correctly
and before it just happened to work because there was a load command
before the bootmgr command?

https://github.com/u-boot/u-boot/blob/master/cmd/fs.c#L26


Alex

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v2 1/3] efi_loader: rework fdt handling in distro boot script
  2018-10-22  7:42     ` Alexander Graf
@ 2018-10-23  2:08       ` AKASHI Takahiro
  2018-10-23  7:36         ` Alexander Graf
  0 siblings, 1 reply; 10+ messages in thread
From: AKASHI Takahiro @ 2018-10-23  2:08 UTC (permalink / raw)
  To: u-boot

On Mon, Oct 22, 2018 at 08:42:32AM +0100, Alexander Graf wrote:
> 
> 
> On 22.10.18 08:22, AKASHI Takahiro wrote:
> > On Mon, Oct 22, 2018 at 01:40:05PM +0900, 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"
> >> +		"fi;\0"
> >>  #define SCAN_DEV_FOR_EFI "run scan_dev_for_efi;"
> >> +#define BOOT_EFI_BOOT_MANAGER "bootefi bootmgr;"
> >>  #else
> >>  #define BOOTENV_SHARED_EFI
> >>  #define SCAN_DEV_FOR_EFI
> >> +#define BOOT_EFI_BOOT_MANAGER
> >>  #endif
> >>  
> >>  #ifdef CONFIG_SATA
> >> @@ -409,6 +410,7 @@
> >>  	BOOT_TARGET_DEVICES(BOOTENV_DEV)                                  \
> >>  	\
> >>  	"distro_bootcmd=" BOOTENV_SET_SCSI_NEED_INIT                      \
> >> +		BOOT_EFI_BOOT_MANAGER					  \
> > 
> > The last-minute change may have introduced a problem.
> > By moving "bootmgr" from scan_dev_for_efi" to here,
> > grub fails to start a grub.cfg menu, saying
> >   "error: no such device: /.disk/info."
> > 
> > Do you have any ideas?
> 
> I would guess that bootmgr doesn't set the loaded image target correctly
> and before it just happened to work because there was a load command
> before the bootmgr command?

The root cause was that efi_init_obj_list(), particularly efi_disk_register(),
is executed only once.
In my case, efi bootmgr is called before "usb start" in scan_dev_for_efi,
and any USB devices will not be added to efi object list forever.
Therefore, later on, grub itself can start but cannot detect/access its own
boot device (USB mass storage) via efi interface.

I think we can fix this problem in several ways:
1. run all 'device-scanning' commands, like "usb start" and "scsi scan,"
   before calling bootmgr in distro_bootcmd,
2. modify efi_obj_init_list() to check and register newly-detected devices,
3. add a device to efi object list dynamically whenever a 'device-scanning'
   command detects one,
4. search and check for a device on-the-fly with each efi device path
   (or handle?) specified in efi interface

What do you think?

-Takahiro Akashi

> https://github.com/u-boot/u-boot/blob/master/cmd/fs.c#L26
> 
> 
> Alex

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v2 1/3] efi_loader: rework fdt handling in distro boot script
  2018-10-23  2:08       ` AKASHI Takahiro
@ 2018-10-23  7:36         ` Alexander Graf
  2018-10-23  8:01           ` AKASHI Takahiro
  0 siblings, 1 reply; 10+ messages in thread
From: Alexander Graf @ 2018-10-23  7:36 UTC (permalink / raw)
  To: u-boot



On 23.10.18 03:08, AKASHI Takahiro wrote:
> On Mon, Oct 22, 2018 at 08:42:32AM +0100, Alexander Graf wrote:
>>
>>
>> On 22.10.18 08:22, AKASHI Takahiro wrote:
>>> On Mon, Oct 22, 2018 at 01:40:05PM +0900, 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"
>>>> +		"fi;\0"
>>>>  #define SCAN_DEV_FOR_EFI "run scan_dev_for_efi;"
>>>> +#define BOOT_EFI_BOOT_MANAGER "bootefi bootmgr;"
>>>>  #else
>>>>  #define BOOTENV_SHARED_EFI
>>>>  #define SCAN_DEV_FOR_EFI
>>>> +#define BOOT_EFI_BOOT_MANAGER
>>>>  #endif
>>>>  
>>>>  #ifdef CONFIG_SATA
>>>> @@ -409,6 +410,7 @@
>>>>  	BOOT_TARGET_DEVICES(BOOTENV_DEV)                                  \
>>>>  	\
>>>>  	"distro_bootcmd=" BOOTENV_SET_SCSI_NEED_INIT                      \
>>>> +		BOOT_EFI_BOOT_MANAGER					  \
>>>
>>> The last-minute change may have introduced a problem.
>>> By moving "bootmgr" from scan_dev_for_efi" to here,
>>> grub fails to start a grub.cfg menu, saying
>>>   "error: no such device: /.disk/info."
>>>
>>> Do you have any ideas?
>>
>> I would guess that bootmgr doesn't set the loaded image target correctly
>> and before it just happened to work because there was a load command
>> before the bootmgr command?
> 
> The root cause was that efi_init_obj_list(), particularly efi_disk_register(),
> is executed only once.
> In my case, efi bootmgr is called before "usb start" in scan_dev_for_efi,
> and any USB devices will not be added to efi object list forever.
> Therefore, later on, grub itself can start but cannot detect/access its own
> boot device (USB mass storage) via efi interface.
> 
> I think we can fix this problem in several ways:
> 1. run all 'device-scanning' commands, like "usb start" and "scsi scan,"
>    before calling bootmgr in distro_bootcmd,

This would work, though it's slightly complicated - hm :/.

> 2. modify efi_obj_init_list() to check and register newly-detected devices,

That won't work, because then efibootmgr won't be able to boot from USB, no?

> 3. add a device to efi object list dynamically whenever a 'device-scanning'
>    command detects one,

Same here I assume.

> 4. search and check for a device on-the-fly with each efi device path
>    (or handle?) specified in efi interface

That won't work either, because we simply don't initialize USB in the
efibootmgr case, so there won't be anything on the fly ;).

> 
> What do you think?

I guess one of your options above mentions this and I just missed it,
but I think we'll have to find out what to initialize in the efibootmgr
command based on the device path we're trying to execute and then
initialize that respective subsystem.

So if we see that the device path involves USB, we need to run "usb
start" from within the efibootmgr code.


Alex

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v2 1/3] efi_loader: rework fdt handling in distro boot script
  2018-10-23  7:36         ` Alexander Graf
@ 2018-10-23  8:01           ` AKASHI Takahiro
  0 siblings, 0 replies; 10+ messages in thread
From: AKASHI Takahiro @ 2018-10-23  8:01 UTC (permalink / raw)
  To: u-boot

On Tue, Oct 23, 2018 at 08:36:58AM +0100, Alexander Graf wrote:
> 
> 
> On 23.10.18 03:08, AKASHI Takahiro wrote:
> > On Mon, Oct 22, 2018 at 08:42:32AM +0100, Alexander Graf wrote:
> >>
> >>
> >> On 22.10.18 08:22, AKASHI Takahiro wrote:
> >>> On Mon, Oct 22, 2018 at 01:40:05PM +0900, 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"
> >>>> +		"fi;\0"
> >>>>  #define SCAN_DEV_FOR_EFI "run scan_dev_for_efi;"
> >>>> +#define BOOT_EFI_BOOT_MANAGER "bootefi bootmgr;"
> >>>>  #else
> >>>>  #define BOOTENV_SHARED_EFI
> >>>>  #define SCAN_DEV_FOR_EFI
> >>>> +#define BOOT_EFI_BOOT_MANAGER
> >>>>  #endif
> >>>>  
> >>>>  #ifdef CONFIG_SATA
> >>>> @@ -409,6 +410,7 @@
> >>>>  	BOOT_TARGET_DEVICES(BOOTENV_DEV)                                  \
> >>>>  	\
> >>>>  	"distro_bootcmd=" BOOTENV_SET_SCSI_NEED_INIT                      \
> >>>> +		BOOT_EFI_BOOT_MANAGER					  \
> >>>
> >>> The last-minute change may have introduced a problem.
> >>> By moving "bootmgr" from scan_dev_for_efi" to here,
> >>> grub fails to start a grub.cfg menu, saying
> >>>   "error: no such device: /.disk/info."
> >>>
> >>> Do you have any ideas?
> >>
> >> I would guess that bootmgr doesn't set the loaded image target correctly
> >> and before it just happened to work because there was a load command
> >> before the bootmgr command?
> > 
> > The root cause was that efi_init_obj_list(), particularly efi_disk_register(),
> > is executed only once.
> > In my case, efi bootmgr is called before "usb start" in scan_dev_for_efi,
> > and any USB devices will not be added to efi object list forever.
> > Therefore, later on, grub itself can start but cannot detect/access its own
> > boot device (USB mass storage) via efi interface.
> > 
> > I think we can fix this problem in several ways:
> > 1. run all 'device-scanning' commands, like "usb start" and "scsi scan,"
> >    before calling bootmgr in distro_bootcmd,
> 
> This would work, though it's slightly complicated - hm :/.
> 
> > 2. modify efi_obj_init_list() to check and register newly-detected devices,
> 
> That won't work, because then efibootmgr won't be able to boot from USB, no?
> 
> > 3. add a device to efi object list dynamically whenever a 'device-scanning'
> >    command detects one,
> 
> Same here I assume.
> 
> > 4. search and check for a device on-the-fly with each efi device path
> >    (or handle?) specified in efi interface
> 
> That won't work either, because we simply don't initialize USB in the
> efibootmgr case, so there won't be anything on the fly ;).
> 
> > 
> > What do you think?
> 
> I guess one of your options above mentions this and I just missed it,
> but I think we'll have to find out what to initialize in the efibootmgr
> command based on the device path we're trying to execute and then
> initialize that respective subsystem.
> 
> So if we see that the device path involves USB, we need to run "usb
> start" from within the efibootmgr code.

I don't think that this behavior is consistent with other U-boot commands.
For example, "load usb 0:1 ..." doesn't work unless we run "usb start"
first. Efi bootmgr should not be an exception.

-Takahiro Akashi

> 
> Alex

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v2 3/3] ARM: qemu-arm: define fdt_addr_r
  2018-10-22  4:40 ` [U-Boot] [PATCH v2 3/3] ARM: qemu-arm: define fdt_addr_r AKASHI Takahiro
@ 2018-10-24 10:43   ` Tuomas Tynkkynen
  0 siblings, 0 replies; 10+ messages in thread
From: Tuomas Tynkkynen @ 2018-10-24 10:43 UTC (permalink / raw)
  To: u-boot

Hi Takahiro,

On Mon, 22 Oct 2018 13:40:07 +0900
AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:

> This variable, fdt_addr_t, is missing in the current qemu-arm.h while
> it seems to be mandatory, at least, to run distro_bootcmd as expected.
> So just add its definition. A size of 1MB would be enough.
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  include/configs/qemu-arm.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h
> index 91fb8d47edf8..0e66f946dde5 100644
> --- a/include/configs/qemu-arm.h
> +++ b/include/configs/qemu-arm.h
> @@ -55,6 +55,7 @@
>  	"fdt_high=0xffffffff\0" \
>  	"initrd_high=0xffffffff\0" \
>  	"fdt_addr=0x40000000\0" \
> +	"fdt_addr_r=0x40100000\0" \
>  	"scriptaddr=0x40200000\0" \
>  	"pxefile_addr_r=0x40300000\0" \
>  	"kernel_addr_r=0x40400000\0" \

The same problem as in the previous version still exists; applying
these as-is would regress any boot entries in PXE files that use the
FDTDIR directive since it'd try to load a non-existent .dtb and fail.

We need some way to avoid that; maybe some magic value
(e.g. fdtfile=none) or a separate environment variable or CONFIG_
option.

- Tuomas

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-10-24 10:43 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-22  4:40 [U-Boot] [PATCH v2 0/3] efi_loader: improve boot sequence in distro_bootcmd AKASHI Takahiro
2018-10-22  4:40 ` [U-Boot] [PATCH v2 1/3] efi_loader: rework fdt handling in distro boot script AKASHI Takahiro
2018-10-22  7:22   ` AKASHI Takahiro
2018-10-22  7:42     ` Alexander Graf
2018-10-23  2:08       ` AKASHI Takahiro
2018-10-23  7:36         ` Alexander Graf
2018-10-23  8:01           ` AKASHI Takahiro
2018-10-22  4:40 ` [U-Boot] [PATCH v2 2/3] ARM: qemu-arm: rework Kconfig AKASHI Takahiro
2018-10-22  4:40 ` [U-Boot] [PATCH v2 3/3] ARM: qemu-arm: define fdt_addr_r AKASHI Takahiro
2018-10-24 10:43   ` Tuomas Tynkkynen

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.