[PATCH v5 0/8] bootstd: sunxi: Migrate to standard boot

[PATCH v5 0/8] bootstd: sunxi: Migrate to standard boot

[Simon Glass]

This series attempts to migrate all sunxi boards to use standard boot,
along with a text environment.

Unfortunately a late-breaking problem has come up with bootmgr. There
are two ways to fix this, both included in this series:

   - Drop bootmgr for sunxi
   - Change efi_mgr bootmeth back to sandbox-only

Based on the mailing-list discussion I have selected the first one and
marked the second as RFC.

Changes in v5:
- Add new patch to drop bootmgr for sunxi

Changes in v4:
- Reword Kconfig
- Add a name for the bootflow
- Drop UUID_GPT_SYSTEM and UUID_GPT_ESP definitions
- Drop special case for ARM64's devicetree subdir
- Drop unwanted quotations and indentation in KERNEL_COMPAT path

Changes in v3:
- Drop wip (work-in-progress) comment in commit
- Fix 'supressed' typo
- Use backquotes to highlight DEFAULT_DEVICE_TREE

Changes in v2:
- Add new patch to resolve BOOTSTD->BLK recursion with Kconfig
- Put the FEL bootmeth before all other global bootmeths
- Convert the other DISTRO_DEFAULTS in the Kconfig too
- Keep BOOTCMD_SUNXI_COMPAT
- Keep bootcmd_sunxi_compat if OLD_SUNXI_KERNEL_COMPAT is enabled

Simon Glass (8):
  bootstd: Avoid depending on BLK
  sunxi: Add a bootmeth for FEL
  efi_loader: bootstd: Drop bootmgr for sunxi
  RFC: Revert "bootstd: Make efi_mgr bootmeth work for non-sandbox
    setups"
  sunxi: Move to bootstd
  sunxi: Drop old distro boot variables
  env: Provide a work-around for unquoting fdtfile
  sunxi: Move to text environment

 Makefile                                      |   1 +
 arch/arm/Kconfig                              |  10 +-
 board/sunxi/sunxi.env                         | 140 +++++++++++
 boot/Kconfig                                  |  18 +-
 boot/Makefile                                 |   1 +
 boot/bootmeth_efi_mgr.c                       |  18 +-
 boot/bootmeth_fel.c                           |  84 +++++++
 .../gardena-smart-gateway-mt7688_defconfig    |   1 +
 doc/usage/environment.rst                     |  12 +
 include/configs/sunxi-common.h                | 238 ------------------
 10 files changed, 261 insertions(+), 262 deletions(-)
 create mode 100644 board/sunxi/sunxi.env
 create mode 100644 boot/bootmeth_fel.c

-- 
2.34.1

[PATCH v5 1/8] bootstd: Avoid depending on BLK

[Simon Glass]

In principle bootstd can work without block devices, even if it does
require driver model to be enabled in that case.

The use of a 'depends on BLK' for BOOTSTD conflicts with the way 'BLK'
is now defined, producing recursive errors through multiple different
paths, one of which is this (with Linksprite_pcDuino3 and
BOOTSTD_DEFAULTS enabled):

  arch/arm/Kconfig:7:error: recursive dependency detected!
  arch/arm/Kconfig:7: symbol ARM64 is selected by ARCH_UNIPHIER_V8_MULTI
  arch/arm/mach-uniphier/Kconfig:17: symbol ARCH_UNIPHIER_V8_MULTI is
     part of choice <choice>
  arch/arm/mach-uniphier/Kconfig:6: choice <choice> contains symbol
     ARCH_UNIPHIER_V8_MULTI
  arch/arm/mach-uniphier/Kconfig:17: symbol ARCH_UNIPHIER_V8_MULTI is
     part of choice SPL
  arch/arm/mach-stm32mp/Kconfig:3: symbol SPL depends on SUPPORT_SPL
  common/spl/Kconfig:1: symbol SUPPORT_SPL is selected by ASPEED_AST2600
  arch/arm/mach-aspeed/Kconfig:26: symbol ASPEED_AST2600 is part of
     choice <choice>
  arch/arm/mach-aspeed/Kconfig:12: choice <choice> contains symbol
     ASPEED_AST2500
  arch/arm/mach-aspeed/Kconfig:17: symbol ASPEED_AST2500 is part of
     choice DM_RESET
  arch/arm/mach-renesas/Kconfig.rcar3:197: symbol DM_RESET is selected
     by CLK_RCAR_GEN3
  drivers/clk/renesas/Kconfig:53: symbol CLK_RCAR_GEN3 depends on
     CLK_RENESAS
  drivers/clk/renesas/Kconfig:1: symbol CLK_RENESAS depends on CLK
  drivers/clk/Kconfig:3: symbol CLK is selected by IMX8M_POWER_DOMAIN
  drivers/power/domain/Kconfig:35: symbol IMX8M_POWER_DOMAIN depends on
     POWER_DOMAIN
  drivers/power/domain/Kconfig:3: symbol POWER_DOMAIN is selected by
     BCM6318_USBH_PHY
  drivers/phy/Kconfig:83: symbol BCM6318_USBH_PHY depends on PHY
  drivers/phy/Kconfig:4: symbol PHY is selected by USB_EHCI_MX7
  drivers/usb/host/Kconfig:211: symbol USB_EHCI_MX7 depends on USB
  drivers/usb/Kconfig:1: symbol USB is selected by BOOTSTD_DEFAULTS
  boot/Kconfig:455: symbol BOOTSTD_DEFAULTS depends on BOOTSTD
  boot/Kconfig:398: symbol BOOTSTD depends on BLK
  drivers/block/Kconfig:1: symbol BLK is selected by PVBLOCK
  drivers/xen/Kconfig:1: symbol PVBLOCK depends on XEN
  Kconfig:176: symbol XEN depends on ARM64

We don't want to revert the change to BLK, which has been in place for
a year now. We don't want to select BLK in BOOTSTD since it should
support booting without block devices. The only realistic option is to
remove BOOTSTD's dependency on BLK.

Disable standard boot on the one board which fails.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v3)

Changes in v3:
- Drop wip (work-in-progress) comment in commit

Changes in v2:
- Add new patch to resolve BOOTSTD->BLK recursion with Kconfig

 boot/Kconfig                                   | 2 +-
 configs/gardena-smart-gateway-mt7688_defconfig | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/boot/Kconfig b/boot/Kconfig
index 7dd30a030e3..b5433e88f10 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -393,7 +393,7 @@ config BOOT_DEFAULTS
 menuconfig BOOTSTD
 	bool "Standard boot"
 	default y
-	depends on DM && OF_CONTROL && BLK
+	depends on DM && OF_CONTROL
 	help
 	  U-Boot supports a standard way of locating something to boot,
 	  typically an Operating System such as Linux, provided by a distro such
diff --git a/configs/gardena-smart-gateway-mt7688_defconfig b/configs/gardena-smart-gateway-mt7688_defconfig
index b7f4a76ce68..85f42a3ab6d 100644
--- a/configs/gardena-smart-gateway-mt7688_defconfig
+++ b/configs/gardena-smart-gateway-mt7688_defconfig
@@ -30,6 +30,7 @@ CONFIG_HAS_BOARD_SIZE_LIMIT=y
 CONFIG_BOARD_SIZE_LIMIT=655360
 CONFIG_FIT=y
 CONFIG_FIT_SIGNATURE=y
+# CONFIG_BOOTSTD is not set
 CONFIG_LEGACY_IMAGE_FORMAT=y
 CONFIG_AUTOBOOT_KEYED=y
 CONFIG_AUTOBOOT_STOP_STR="x"
-- 
2.34.1

[PATCH v5 2/8] sunxi: Add a bootmeth for FEL

[Simon Glass]

Add support for booting from a script loaded over FEL. This mirrors the
bootcmd_fel provided by distro boot.

Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
---

(no changes since v4)

Changes in v4:
- Reword Kconfig
- Add a name for the bootflow

Changes in v2:
- Put the FEL bootmeth before all other global bootmeths

 boot/Kconfig        | 14 ++++++++
 boot/Makefile       |  1 +
 boot/bootmeth_fel.c | 84 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 99 insertions(+)
 create mode 100644 boot/bootmeth_fel.c

diff --git a/boot/Kconfig b/boot/Kconfig
index b5433e88f10..b612360ca55 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -589,6 +589,20 @@ config BOOTMETH_EFI_BOOTMGR
 	  the EFI binary to be launched is determined. To set the EFI variables
 	  use the eficonfig command.
 
+config BOOTMETH_FEL
+	bool "Bootdev support for Sunxi FEL"
+	depends on ARCH_SUNXI
+	default y
+	help
+	  Enables support for executing an explicit boot script uploaded before
+	  via the USB FEL protocol. The 'fel_scriptaddr' environment variable
+	  holds the address of this script, taken from the SPL header.
+
+	  This method is only available if booting from FEL, i.e. the
+	  'fel_booted' environment variable is set.
+
+	  See https://linux-sunxi.org/FEL/Protocol for more information.
+
 config BOOTMETH_QFW
 	bool "Boot method using QEMU parameters"
 	depends on QFW
diff --git a/boot/Makefile b/boot/Makefile
index 43def7c33d7..b03d628187b 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_$(PHASE_)BOOTMETH_EXTLINUX) += bootmeth_extlinux.o
 obj-$(CONFIG_$(PHASE_)BOOTMETH_EXTLINUX_PXE) += bootmeth_pxe.o
 obj-$(CONFIG_$(PHASE_)BOOTMETH_EFILOADER) += bootmeth_efi.o
 obj-$(CONFIG_$(PHASE_)BOOTMETH_CROS) += bootm.o bootm_os.o bootmeth_cros.o
+obj-$(CONFIG_$(PHASE_)BOOTMETH_FEL) += bootmeth_fel.o
 obj-$(CONFIG_$(PHASE_)BOOTMETH_QFW) += bootmeth_qfw.o
 obj-$(CONFIG_$(PHASE_)BOOTMETH_SANDBOX) += bootmeth_sandbox.o
 obj-$(CONFIG_$(PHASE_)BOOTMETH_SCRIPT) += bootmeth_script.o
diff --git a/boot/bootmeth_fel.c b/boot/bootmeth_fel.c
new file mode 100644
index 00000000000..61332d5a3b4
--- /dev/null
+++ b/boot/bootmeth_fel.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Bootmethod for sunxi FEL loading
+ *
+ * Copyright 2024 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#define LOG_CATEGORY UCLASS_BOOTSTD
+
+#include <bootdev.h>
+#include <bootflow.h>
+#include <bootmeth.h>
+#include <command.h>
+#include <dm.h>
+#include <env.h>
+
+static int fel_check(struct udevice *dev, struct bootflow_iter *iter)
+{
+	return 0;
+}
+
+static int fel_read_bootflow(struct udevice *dev, struct bootflow *bflow)
+{
+	if (!env_get("fel_booted") || !env_get("fel_scriptaddr"))
+		return -ENOENT;
+
+	bflow->name = strdup("fel-script");
+	if (!bflow->name)
+		return log_msg_ret("fel", -ENOMEM);
+	bflow->state = BOOTFLOWST_READY;
+
+	return 0;
+}
+
+static int fel_read_file(struct udevice *dev, struct bootflow *bflow,
+			 const char *file_path, ulong addr, ulong *sizep)
+{
+	return -ENOSYS;
+}
+
+static int fel_boot(struct udevice *dev, struct bootflow *bflow)
+{
+	ulong addr;
+	int ret;
+
+	addr = env_get_hex("fel_scriptaddr", 0);
+	ret = cmd_source_script(addr, NULL, NULL);
+	if (ret)
+		return log_msg_ret("boot", ret);
+
+	return 0;
+}
+
+static int fel_bootmeth_bind(struct udevice *dev)
+{
+	struct bootmeth_uc_plat *plat = dev_get_uclass_plat(dev);
+
+	plat->desc = IS_ENABLED(CONFIG_BOOTSTD_FULL) ?
+	   "Sunxi FEL boot over USB" : "FEL";
+	plat->flags = BOOTMETHF_GLOBAL;
+
+	return 0;
+}
+
+static struct bootmeth_ops fel_bootmeth_ops = {
+	.check		= fel_check,
+	.read_bootflow	= fel_read_bootflow,
+	.read_file	= fel_read_file,
+	.boot		= fel_boot,
+};
+
+static const struct udevice_id fel_bootmeth_ids[] = {
+	{ .compatible = "u-boot,fel-bootmeth" },
+	{ }
+};
+
+U_BOOT_DRIVER(bootmeth_2fel) = {
+	.name		= "bootmeth_fel",
+	.id		= UCLASS_BOOTMETH,
+	.of_match	= fel_bootmeth_ids,
+	.ops		= &fel_bootmeth_ops,
+	.bind		= fel_bootmeth_bind,
+};
-- 
2.34.1

[PATCH v5 3/8] efi_loader: bootstd: Drop bootmgr for sunxi

[Simon Glass]

This causes problems with the boot order, so drop it until we can figure
out a better way to know when bootmgr should be used.

Link: https://lore.kernel.org/u-boot/20241112171205.4e80548d@donnerap.manchester.arm.com/

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v5:
- Add new patch to drop bootmgr for sunxi

 boot/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/boot/Kconfig b/boot/Kconfig
index b612360ca55..42c5ed288c5 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -583,7 +583,7 @@ config BOOTMETH_EFI_BOOTMGR
 	bool "Bootdev support for EFI boot manager"
 	depends on EFI_BOOTMGR
 	select BOOTMETH_GLOBAL
-	default y
+	default y if !ARCH_SUNXI  # Temporary work-around for sunxi
 	help
 	  Enable booting via the UEFI boot manager. Based on the EFI variables
 	  the EFI binary to be launched is determined. To set the EFI variables
-- 
2.34.1

[PATCH v5 4/8] RFC: Revert "bootstd: Make efi_mgr bootmeth work for non-sandbox setups"

[Simon Glass]

This is another option to fix sunxi booting with bootstd, which may be
better since it will work for all boards. We can then figure out how to
automatically and deterministicaly decide when bootmgr should be used.

This reverts commit f2bfa0cb17948aa4a0fa20fdf9014296b9c4d9c7.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
If this patch is applied, we don't need to drop bootmgr for sunxi

(no changes since v1)

 boot/bootmeth_efi_mgr.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/boot/bootmeth_efi_mgr.c b/boot/bootmeth_efi_mgr.c
index 23ae1e610ac..095fa74fc60 100644
--- a/boot/bootmeth_efi_mgr.c
+++ b/boot/bootmeth_efi_mgr.c
@@ -14,8 +14,6 @@
 #include <command.h>
 #include <dm.h>
 #include <efi_loader.h>
-#include <efi_variable.h>
-#include <malloc.h>
 
 /**
  * struct efi_mgr_priv - private info for the efi-mgr driver
@@ -48,27 +46,13 @@ static int efi_mgr_check(struct udevice *dev, struct bootflow_iter *iter)
 static int efi_mgr_read_bootflow(struct udevice *dev, struct bootflow *bflow)
 {
 	struct efi_mgr_priv *priv = dev_get_priv(dev);
-	efi_status_t ret;
-	efi_uintn_t size;
-	u16 *bootorder;
 
 	if (priv->fake_dev) {
 		bflow->state = BOOTFLOWST_READY;
 		return 0;
 	}
 
-	ret = efi_init_obj_list();
-	if (ret)
-		return log_msg_ret("init", ret);
-
-	/* Enable this method if the "BootOrder" UEFI exists. */
-	bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid,
-				&size);
-	if (bootorder) {
-		free(bootorder);
-		bflow->state = BOOTFLOWST_READY;
-		return 0;
-	}
+	/* To be implemented */
 
 	return -EINVAL;
 }
-- 
2.34.1

[PATCH v5 5/8] sunxi: Move to bootstd

[Simon Glass]

Drop support for distroboot and move to using bootstd instead.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v2)

Changes in v2:
- Convert the other DISTRO_DEFAULTS in the Kconfig too

 arch/arm/Kconfig | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 7282c4123b0..b08d8d4ff27 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1155,7 +1155,7 @@ config ARCH_SUNXI
 	select BINMAN
 	select CMD_GPIO
 	select CMD_MMC if MMC
-	select CMD_USB if DISTRO_DEFAULTS && USB_HOST
+	select CMD_USB if BOOTSTD_DEFAULTS && USB_HOST
 	select CLK
 	select DM
 	select DM_GPIO
@@ -1177,9 +1177,9 @@ config ARCH_SUNXI
 	select SUNXI_GPIO
 	select SYS_NS16550
 	select SYS_THUMB_BUILD if !ARM64
-	select USB if DISTRO_DEFAULTS
-	select USB_KEYBOARD if DISTRO_DEFAULTS && USB_HOST
-	select USB_STORAGE if DISTRO_DEFAULTS && USB_HOST
+	select USB if BOOTSTD_DEFAULTS
+	select USB_KEYBOARD if BOOTSTD_DEFAULTS && USB_HOST
+	select USB_STORAGE if BOOTSTD_DEFAULTS && USB_HOST
 	select SPL_USE_TINY_PRINTF if SPL
 	select USE_PREBOOT
 	select SYS_RELOC_GD_ENV_ADDR
@@ -1187,7 +1187,7 @@ config ARCH_SUNXI
 	imply CMD_DM
 	imply CMD_GPT
 	imply CMD_UBI if MTD_RAW_NAND
-	imply DISTRO_DEFAULTS
+	imply BOOTSTD_DEFAULTS
 	imply DM_REGULATOR
 	imply DM_REGULATOR_FIXED
 	imply FAT_WRITE
-- 
2.34.1

[PATCH v5 6/8] sunxi: Drop old distro boot variables

[Simon Glass]

These are not needed as bootstd handles the boot now. Drop them.

Keep BOOTCMD_SUNXI_COMPAT for now since it does not relate to
distro boot.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v2)

Changes in v2:
- Keep BOOTCMD_SUNXI_COMPAT

 include/configs/sunxi-common.h | 75 +---------------------------------
 1 file changed, 1 insertion(+), 74 deletions(-)

diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
index b29a25d5617..666553ccadb 100644
--- a/include/configs/sunxi-common.h
+++ b/include/configs/sunxi-common.h
@@ -156,76 +156,6 @@
 	"fdt ram " FDT_ADDR_R " 0x100000;" \
 	"ramdisk ram " RAMDISK_ADDR_R " 0x4000000\0"
 
-/****************************************************************************
- *                  definitions for the distro boot system                  *
- ****************************************************************************/
-#ifdef CONFIG_MMC
-#if CONFIG_MMC_SUNXI_SLOT_EXTRA != -1
-#define BOOTENV_DEV_MMC_AUTO(devtypeu, devtypel, instance)		\
-	BOOTENV_DEV_MMC(MMC, mmc, 0)					\
-	BOOTENV_DEV_MMC(MMC, mmc, 1)					\
-	"bootcmd_mmc_auto="						\
-		"if test ${mmc_bootdev} -eq 1; then "			\
-			"run bootcmd_mmc1; "				\
-			"run bootcmd_mmc0; "				\
-		"elif test ${mmc_bootdev} -eq 0; then "			\
-			"run bootcmd_mmc0; "				\
-			"run bootcmd_mmc1; "				\
-		"fi\0"
-
-#define BOOTENV_DEV_NAME_MMC_AUTO(devtypeu, devtypel, instance) \
-	"mmc_auto "
-
-#define BOOT_TARGET_DEVICES_MMC(func) func(MMC_AUTO, mmc_auto, na)
-#else
-#define BOOT_TARGET_DEVICES_MMC(func) func(MMC, mmc, 0)
-#endif
-#else
-#define BOOT_TARGET_DEVICES_MMC(func)
-#endif
-
-#ifdef CONFIG_AHCI
-#define BOOT_TARGET_DEVICES_SCSI(func) func(SCSI, scsi, 0)
-#else
-#define BOOT_TARGET_DEVICES_SCSI(func)
-#endif
-
-#ifdef CONFIG_USB_STORAGE
-#define BOOT_TARGET_DEVICES_USB(func) func(USB, usb, 0)
-#else
-#define BOOT_TARGET_DEVICES_USB(func)
-#endif
-
-#ifdef CONFIG_CMD_PXE
-#define BOOT_TARGET_DEVICES_PXE(func) func(PXE, pxe, na)
-#else
-#define BOOT_TARGET_DEVICES_PXE(func)
-#endif
-
-#ifdef CONFIG_CMD_DHCP
-#define BOOT_TARGET_DEVICES_DHCP(func) func(DHCP, dhcp, na)
-#else
-#define BOOT_TARGET_DEVICES_DHCP(func)
-#endif
-
-/* FEL boot support, auto-execute boot.scr if a script address was provided */
-#define BOOTENV_DEV_FEL(devtypeu, devtypel, instance) \
-	"bootcmd_fel=" \
-		"if test -n ${fel_booted} && test -n ${fel_scriptaddr}; then " \
-			"echo '(FEL boot)'; " \
-			"source ${fel_scriptaddr}; " \
-		"fi\0"
-#define BOOTENV_DEV_NAME_FEL(devtypeu, devtypel, instance) \
-	"fel "
-
-#define BOOT_TARGET_DEVICES(func) \
-	func(FEL, fel, na) \
-	BOOT_TARGET_DEVICES_MMC(func) \
-	BOOT_TARGET_DEVICES_SCSI(func) \
-	BOOT_TARGET_DEVICES_USB(func) \
-	BOOT_TARGET_DEVICES_PXE(func) \
-	BOOT_TARGET_DEVICES_DHCP(func)
-
 #ifdef CONFIG_OLD_SUNXI_KERNEL_COMPAT
 #define BOOTCMD_SUNXI_COMPAT \
 	"bootcmd_sunxi_compat=" \
@@ -242,8 +172,6 @@
 #define BOOTCMD_SUNXI_COMPAT
 #endif
 
-#include <config_distro_bootcmd.h>
-
 #ifdef CONFIG_USB_KEYBOARD
 #define CONSOLE_STDIN_SETTINGS \
 	"stdin=serial,usbkbd\0"
@@ -296,7 +224,6 @@
 	"uuid_gpt_esp=" UUID_GPT_ESP "\0" \
 	"uuid_gpt_system=" UUID_GPT_SYSTEM "\0" \
 	"partitions=" PARTS_DEFAULT "\0" \
-	BOOTCMD_SUNXI_COMPAT \
-	BOOTENV
+	BOOTCMD_SUNXI_COMPAT
 
 #endif /* _SUNXI_COMMON_CONFIG_H */
-- 
2.34.1

[PATCH v5 7/8] env: Provide a work-around for unquoting fdtfile

[Simon Glass]

Some boards use a CONFIG option to specify the value of this variable.
This is normally handled by efi_get_distro_fdt_name() but in the case
of sunxi this does not work, since 'soc' is sunxi, but the files are
in the allwinner directory.

Provide a work-around for this particular case.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v3)

Changes in v3:
- Fix 'supressed' typo
- Use backquotes to highlight DEFAULT_DEVICE_TREE

 Makefile                  |  1 +
 doc/usage/environment.rst | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/Makefile b/Makefile
index 81b73a1d8d9..07529e0bc4f 100644
--- a/Makefile
+++ b/Makefile
@@ -1861,6 +1861,7 @@ quiet_cmd_gen_envp = ENVP    $@
 		$(CPP) -P $(cpp_flags) -x assembler-with-cpp -undef \
 			-D__ASSEMBLY__ \
 			-D__UBOOT_CONFIG__ \
+			-DDEFAULT_DEVICE_TREE=$(subst ",,$(CONFIG_DEFAULT_DEVICE_TREE)) \
 			-I . -I include -I $(srctree)/include \
 			-include linux/kconfig.h -include include/config.h \
 			-I$(srctree)/arch/$(ARCH)/include \
diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
index 7bd9ffce8d8..1ab3ee7ced9 100644
--- a/doc/usage/environment.rst
+++ b/doc/usage/environment.rst
@@ -87,6 +87,18 @@ settings. For example::
 
    #include <env/ti/mmc.env>
 
+Quotes are not suppressed, for example::
+
+    fdtfile=CONFIG_DEFAULT_DEVICE_TREE.dtb
+    # produces: fdtfile="sun7i-a20-pcduino3.dtb"
+
+For this particular issue you can use ``DEFAULT_DEVICE_TREE`` instead::
+
+    fdtfile=DEFAULT_DEVICE_TREE.dtb
+    # produces: fdtfile=sun7i-a20-pcduino3.dtb
+
+There is no general way to remove quotes.
+
 If CONFIG_ENV_SOURCE_FILE is empty and the default filename is not present, then
 the old-style C environment is used instead. See below.
 
-- 
2.34.1

[PATCH v5 8/8] sunxi: Move to text environment

[Simon Glass]

Convert these boards to use a text environment.

For the boards check, the only differences are extra spaces after the
semicolons in 'dfu_alt_info_ram' and 'partitions', both of which are
permitted.

Add in the special boot command for old kernels, dropping the
unnecessary and confusing hex prefixes.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v4)

Changes in v4:
- Drop UUID_GPT_SYSTEM and UUID_GPT_ESP definitions
- Drop special case for ARM64's devicetree subdir
- Drop unwanted quotations and indentation in KERNEL_COMPAT path

Changes in v2:
- Keep bootcmd_sunxi_compat if OLD_SUNXI_KERNEL_COMPAT is enabled

 board/sunxi/sunxi.env          | 140 ++++++++++++++++++++++++++++
 include/configs/sunxi-common.h | 165 ---------------------------------
 2 files changed, 140 insertions(+), 165 deletions(-)
 create mode 100644 board/sunxi/sunxi.env

diff --git a/board/sunxi/sunxi.env b/board/sunxi/sunxi.env
new file mode 100644
index 00000000000..ec6e07c228f
--- /dev/null
+++ b/board/sunxi/sunxi.env
@@ -0,0 +1,140 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Common sunxi environment
+ *
+ * Copyright 2024 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ /
+
+/****************************************************************************
+ *           environment variables holding default load addresses           *
+ ****************************************************************************/
+/*
+ * We cannot use expressions here, because expressions won't be evaluated in
+ * autoconf.mk.
+ */
+#ifdef CONFIG_ARM64
+/*
+ * Boards seem to come with at least 512MB of DRAM.
+ * The kernel should go at 512K, which is the default text offset (that will
+ * be adjusted at runtime if needed).
+ * There is no compression for arm64 kernels (yet), so leave some space
+ * for really big kernels, say 256MB for now.
+ * Scripts, PXE and DTBs should go afterwards, leaving the rest for the initrd.
+ */
+#define BOOTM_SIZE        0xa000000
+#define KERNEL_ADDR_R     SDRAM_OFFSET(0080000)
+#define KERNEL_COMP_ADDR_R SDRAM_OFFSET(4000000)
+#define KERNEL_COMP_SIZE  0xb000000
+#define FDT_ADDR_R        SDRAM_OFFSET(FA00000)
+#define SCRIPT_ADDR_R     SDRAM_OFFSET(FC00000)
+#define PXEFILE_ADDR_R    SDRAM_OFFSET(FD00000)
+#define FDTOVERLAY_ADDR_R SDRAM_OFFSET(FE00000)
+#define RAMDISK_ADDR_R    SDRAM_OFFSET(FF00000)
+
+#elif (CONFIG_SUNXI_MINIMUM_DRAM_MB >= 256)
+/*
+ * 160M RAM (256M minimum minus 64MB heap + 32MB for u-boot, stack, fb, etc.
+ * 32M uncompressed kernel, 16M compressed kernel, 1M fdt,
+ * 1M script, 1M pxe, 1M dt overlay and the ramdisk at the end.
+ */
+#define BOOTM_SIZE        0xa000000
+#define KERNEL_ADDR_R     SDRAM_OFFSET(2000000)
+#define FDT_ADDR_R        SDRAM_OFFSET(3000000)
+#define SCRIPT_ADDR_R     SDRAM_OFFSET(3100000)
+#define PXEFILE_ADDR_R    SDRAM_OFFSET(3200000)
+#define FDTOVERLAY_ADDR_R SDRAM_OFFSET(3300000)
+#define RAMDISK_ADDR_R    SDRAM_OFFSET(3400000)
+
+#elif (CONFIG_SUNXI_MINIMUM_DRAM_MB >= 64)
+/*
+ * 64M RAM minus 2MB heap + 16MB for u-boot, stack, fb, etc.
+ * 16M uncompressed kernel, 8M compressed kernel, 1M fdt,
+ * 1M script, 1M pxe, 1M dt overlay and the ramdisk at the end.
+ */
+#define BOOTM_SIZE        0x2e00000
+#define KERNEL_ADDR_R     SDRAM_OFFSET(1000000)
+#define FDT_ADDR_R        SDRAM_OFFSET(1800000)
+#define SCRIPT_ADDR_R     SDRAM_OFFSET(1900000)
+#define PXEFILE_ADDR_R    SDRAM_OFFSET(1A00000)
+#define FDTOVERLAY_ADDR_R SDRAM_OFFSET(1B00000)
+#define RAMDISK_ADDR_R    SDRAM_OFFSET(1C00000)
+
+#elif (CONFIG_SUNXI_MINIMUM_DRAM_MB >= 32)
+/*
+ * 32M RAM minus 2.5MB for u-boot, heap, stack, etc.
+ * 16M uncompressed kernel, 7M compressed kernel, 128K fdt, 64K script,
+ * 128K DT overlay, 128K PXE and the ramdisk in the rest (max. 5MB)
+ */
+#define BOOTM_SIZE        0x1700000
+#define KERNEL_ADDR_R     SDRAM_OFFSET(1000000)
+#define FDT_ADDR_R        SDRAM_OFFSET(1d50000)
+#define SCRIPT_ADDR_R     SDRAM_OFFSET(1d40000)
+#define PXEFILE_ADDR_R    SDRAM_OFFSET(1d00000)
+#define FDTOVERLAY_ADDR_R SDRAM_OFFSET(1d20000)
+#define RAMDISK_ADDR_R    SDRAM_OFFSET(1800000)
+
+#else
+#error Need at least 32MB of DRAM. Please adjust load addresses.
+#endif
+
+stdin=serial
+#ifdef CONFIG_USB_KEYBOARD
+stdin+=,usbkbd
+#endif
+
+stdout=serial
+stderr=serial
+#ifdef CONFIG_VIDEO
+stdout+=,vidconsole
+stderr+=,vidconsole
+#endif
+
+bootm_size=BOOTM_SIZE
+kernel_addr_r=KERNEL_ADDR_R
+fdt_addr_r=FDT_ADDR_R
+scriptaddr=SCRIPT_ADDR_R
+pxefile_addr_r=PXEFILE_ADDR_R
+fdtoverlay_addr_r=FDTOVERLAY_ADDR_R
+ramdisk_addr_r=RAMDISK_ADDR_R
+
+#ifdef CONFIG_ARM64
+kernel_comp_addr_r=KERNEL_COMP_ADDR_R
+kernel_comp_size=KERNEL_COMP_SIZE
+#endif
+
+dfu_alt_info_ram=
+  kernel ram KERNEL_ADDR_R 0x1000000;
+  fdt ram FDT_ADDR_R 0x100000;
+  ramdisk ram RAMDISK_ADDR_R 0x4000000
+
+fdtfile=DEFAULT_DEVICE_TREE.dtb
+
+console=ttyS0,115200
+
+uuid_gpt_esp=c12a7328-f81f-11d2-ba4b-00a0c93ec93b
+#ifdef CONFIG_ARM64
+uuid_gpt_system=b921b045-1df0-41c3-af44-4c6f280d3fae
+#else
+uuid_gpt_system=69dad710-2ce4-4e3c-b16c-21a1d49abed3
+#endif
+
+partitions=
+  name=loader1,start=8k,size=32k,uuid=${uuid_gpt_loader1};
+  name=loader2,size=984k,uuid=${uuid_gpt_loader2};
+  name=esp,size=128M,bootable,uuid=${uuid_gpt_esp};
+  name=system,size=-,uuid=${uuid_gpt_system};
+
+/* support booting a very old kernel */
+#ifdef CONFIG_OLD_SUNXI_KERNEL_COMPAT
+bootcmd_sunxi_compat=
+  setenv root /dev/mmcblk0p3 rootwait;
+  if ext2load mmc 0 44000000 uEnv.txt; then
+    echo Loaded environment from uEnv.txt;
+    env import -t 44000000 ${filesize};
+  fi;
+  setenv bootargs console=${console} root=${root} ${extraargs};
+  ext2load mmc 0 43000000 script.bin &&
+  ext2load mmc 0 48000000 uImage &&
+  bootm 48000000
+#endif
diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
index 666553ccadb..ceea26494ad 100644
--- a/include/configs/sunxi-common.h
+++ b/include/configs/sunxi-common.h
@@ -61,169 +61,4 @@
 #define PHYS_SDRAM_0			CFG_SYS_SDRAM_BASE
 #define PHYS_SDRAM_0_SIZE		0x80000000 /* 2 GiB */
 
-/****************************************************************************
- *           environment variables holding default load addresses           *
- ****************************************************************************/
-/*
- * We cannot use expressions here, because expressions won't be evaluated in
- * autoconf.mk.
- */
-#ifdef CONFIG_ARM64
-/*
- * Boards seem to come with at least 512MB of DRAM.
- * The kernel should go at 512K, which is the default text offset (that will
- * be adjusted at runtime if needed).
- * There is no compression for arm64 kernels (yet), so leave some space
- * for really big kernels, say 256MB for now.
- * Scripts, PXE and DTBs should go afterwards, leaving the rest for the initrd.
- */
-#define BOOTM_SIZE        __stringify(0xa000000)
-#define KERNEL_ADDR_R     __stringify(SDRAM_OFFSET(0080000))
-#define KERNEL_COMP_ADDR_R __stringify(SDRAM_OFFSET(4000000))
-#define KERNEL_COMP_SIZE  __stringify(0xb000000)
-#define FDT_ADDR_R        __stringify(SDRAM_OFFSET(FA00000))
-#define SCRIPT_ADDR_R     __stringify(SDRAM_OFFSET(FC00000))
-#define PXEFILE_ADDR_R    __stringify(SDRAM_OFFSET(FD00000))
-#define FDTOVERLAY_ADDR_R __stringify(SDRAM_OFFSET(FE00000))
-#define RAMDISK_ADDR_R    __stringify(SDRAM_OFFSET(FF00000))
-
-#elif (CONFIG_SUNXI_MINIMUM_DRAM_MB >= 256)
-/*
- * 160M RAM (256M minimum minus 64MB heap + 32MB for u-boot, stack, fb, etc.
- * 32M uncompressed kernel, 16M compressed kernel, 1M fdt,
- * 1M script, 1M pxe, 1M dt overlay and the ramdisk at the end.
- */
-#define BOOTM_SIZE        __stringify(0xa000000)
-#define KERNEL_ADDR_R     __stringify(SDRAM_OFFSET(2000000))
-#define FDT_ADDR_R        __stringify(SDRAM_OFFSET(3000000))
-#define SCRIPT_ADDR_R     __stringify(SDRAM_OFFSET(3100000))
-#define PXEFILE_ADDR_R    __stringify(SDRAM_OFFSET(3200000))
-#define FDTOVERLAY_ADDR_R __stringify(SDRAM_OFFSET(3300000))
-#define RAMDISK_ADDR_R    __stringify(SDRAM_OFFSET(3400000))
-
-#elif (CONFIG_SUNXI_MINIMUM_DRAM_MB >= 64)
-/*
- * 64M RAM minus 2MB heap + 16MB for u-boot, stack, fb, etc.
- * 16M uncompressed kernel, 8M compressed kernel, 1M fdt,
- * 1M script, 1M pxe, 1M dt overlay and the ramdisk at the end.
- */
-#define BOOTM_SIZE        __stringify(0x2e00000)
-#define KERNEL_ADDR_R     __stringify(SDRAM_OFFSET(1000000))
-#define FDT_ADDR_R        __stringify(SDRAM_OFFSET(1800000))
-#define SCRIPT_ADDR_R     __stringify(SDRAM_OFFSET(1900000))
-#define PXEFILE_ADDR_R    __stringify(SDRAM_OFFSET(1A00000))
-#define FDTOVERLAY_ADDR_R __stringify(SDRAM_OFFSET(1B00000))
-#define RAMDISK_ADDR_R    __stringify(SDRAM_OFFSET(1C00000))
-
-#elif (CONFIG_SUNXI_MINIMUM_DRAM_MB >= 32)
-/*
- * 32M RAM minus 2.5MB for u-boot, heap, stack, etc.
- * 16M uncompressed kernel, 7M compressed kernel, 128K fdt, 64K script,
- * 128K DT overlay, 128K PXE and the ramdisk in the rest (max. 5MB)
- */
-#define BOOTM_SIZE        __stringify(0x1700000)
-#define KERNEL_ADDR_R     __stringify(SDRAM_OFFSET(1000000))
-#define FDT_ADDR_R        __stringify(SDRAM_OFFSET(1d50000))
-#define SCRIPT_ADDR_R     __stringify(SDRAM_OFFSET(1d40000))
-#define PXEFILE_ADDR_R    __stringify(SDRAM_OFFSET(1d00000))
-#define FDTOVERLAY_ADDR_R __stringify(SDRAM_OFFSET(1d20000))
-#define RAMDISK_ADDR_R    __stringify(SDRAM_OFFSET(1800000))
-
-#else
-#error Need at least 32MB of DRAM. Please adjust load addresses.
-#endif
-
-#define MEM_LAYOUT_ENV_SETTINGS \
-	"bootm_size=" BOOTM_SIZE "\0" \
-	"kernel_addr_r=" KERNEL_ADDR_R "\0" \
-	"fdt_addr_r=" FDT_ADDR_R "\0" \
-	"scriptaddr=" SCRIPT_ADDR_R "\0" \
-	"pxefile_addr_r=" PXEFILE_ADDR_R "\0" \
-	"fdtoverlay_addr_r=" FDTOVERLAY_ADDR_R "\0" \
-	"ramdisk_addr_r=" RAMDISK_ADDR_R "\0"
-
-#ifdef CONFIG_ARM64
-#define MEM_LAYOUT_ENV_EXTRA_SETTINGS \
-	"kernel_comp_addr_r=" KERNEL_COMP_ADDR_R "\0" \
-	"kernel_comp_size=" KERNEL_COMP_SIZE "\0"
-#else
-#define MEM_LAYOUT_ENV_EXTRA_SETTINGS ""
-#endif
-
-#define DFU_ALT_INFO_RAM \
-	"dfu_alt_info_ram=" \
-	"kernel ram " KERNEL_ADDR_R " 0x1000000;" \
-	"fdt ram " FDT_ADDR_R " 0x100000;" \
-	"ramdisk ram " RAMDISK_ADDR_R " 0x4000000\0"
-
-#ifdef CONFIG_OLD_SUNXI_KERNEL_COMPAT
-#define BOOTCMD_SUNXI_COMPAT \
-	"bootcmd_sunxi_compat=" \
-		"setenv root /dev/mmcblk0p3 rootwait; " \
-		"if ext2load mmc 0 0x44000000 uEnv.txt; then " \
-			"echo Loaded environment from uEnv.txt; " \
-			"env import -t 0x44000000 ${filesize}; " \
-		"fi; " \
-		"setenv bootargs console=${console} root=${root} ${extraargs}; " \
-		"ext2load mmc 0 0x43000000 script.bin && " \
-		"ext2load mmc 0 0x48000000 uImage && " \
-		"bootm 0x48000000\0"
-#else
-#define BOOTCMD_SUNXI_COMPAT
-#endif
-
-#ifdef CONFIG_USB_KEYBOARD
-#define CONSOLE_STDIN_SETTINGS \
-	"stdin=serial,usbkbd\0"
-#else
-#define CONSOLE_STDIN_SETTINGS \
-	"stdin=serial\0"
-#endif
-
-#ifdef CONFIG_VIDEO
-#define CONSOLE_STDOUT_SETTINGS \
-	"stdout=serial,vidconsole\0" \
-	"stderr=serial,vidconsole\0"
-#else
-#define CONSOLE_STDOUT_SETTINGS \
-	"stdout=serial\0" \
-	"stderr=serial\0"
-#endif
-
-#define PARTS_DEFAULT \
-	"name=loader1,start=8k,size=32k,uuid=${uuid_gpt_loader1};" \
-	"name=loader2,size=984k,uuid=${uuid_gpt_loader2};" \
-	"name=esp,size=128M,bootable,uuid=${uuid_gpt_esp};" \
-	"name=system,size=-,uuid=${uuid_gpt_system};"
-
-#define UUID_GPT_ESP "c12a7328-f81f-11d2-ba4b-00a0c93ec93b"
-
-#ifdef CONFIG_ARM64
-#define UUID_GPT_SYSTEM "b921b045-1df0-41c3-af44-4c6f280d3fae"
-#else
-#define UUID_GPT_SYSTEM "69dad710-2ce4-4e3c-b16c-21a1d49abed3"
-#endif
-
-#define CONSOLE_ENV_SETTINGS \
-	CONSOLE_STDIN_SETTINGS \
-	CONSOLE_STDOUT_SETTINGS
-
-#ifdef CONFIG_ARM64
-#define FDTFILE "allwinner/" CONFIG_DEFAULT_DEVICE_TREE ".dtb"
-#else
-#define FDTFILE CONFIG_DEFAULT_DEVICE_TREE ".dtb"
-#endif
-
-#define CFG_EXTRA_ENV_SETTINGS \
-	CONSOLE_ENV_SETTINGS \
-	MEM_LAYOUT_ENV_SETTINGS \
-	MEM_LAYOUT_ENV_EXTRA_SETTINGS \
-	DFU_ALT_INFO_RAM \
-	"fdtfile=" FDTFILE "\0" \
-	"console=ttyS0,115200\0" \
-	"uuid_gpt_esp=" UUID_GPT_ESP "\0" \
-	"uuid_gpt_system=" UUID_GPT_SYSTEM "\0" \
-	"partitions=" PARTS_DEFAULT "\0" \
-	BOOTCMD_SUNXI_COMPAT
-
 #endif /* _SUNXI_COMMON_CONFIG_H */
-- 
2.34.1

Re: [PATCH v5 1/8] bootstd: Avoid depending on BLK

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 4038 bytes --]

On Wed, Nov 13, 2024 at 08:09:31AM -0700, Simon Glass wrote:

> In principle bootstd can work without block devices, even if it does
> require driver model to be enabled in that case.
> 
> The use of a 'depends on BLK' for BOOTSTD conflicts with the way 'BLK'
> is now defined, producing recursive errors through multiple different
> paths, one of which is this (with Linksprite_pcDuino3 and
> BOOTSTD_DEFAULTS enabled):
> 
>   arch/arm/Kconfig:7:error: recursive dependency detected!
>   arch/arm/Kconfig:7: symbol ARM64 is selected by ARCH_UNIPHIER_V8_MULTI
>   arch/arm/mach-uniphier/Kconfig:17: symbol ARCH_UNIPHIER_V8_MULTI is
>      part of choice <choice>
>   arch/arm/mach-uniphier/Kconfig:6: choice <choice> contains symbol
>      ARCH_UNIPHIER_V8_MULTI
>   arch/arm/mach-uniphier/Kconfig:17: symbol ARCH_UNIPHIER_V8_MULTI is
>      part of choice SPL
>   arch/arm/mach-stm32mp/Kconfig:3: symbol SPL depends on SUPPORT_SPL
>   common/spl/Kconfig:1: symbol SUPPORT_SPL is selected by ASPEED_AST2600
>   arch/arm/mach-aspeed/Kconfig:26: symbol ASPEED_AST2600 is part of
>      choice <choice>
>   arch/arm/mach-aspeed/Kconfig:12: choice <choice> contains symbol
>      ASPEED_AST2500
>   arch/arm/mach-aspeed/Kconfig:17: symbol ASPEED_AST2500 is part of
>      choice DM_RESET
>   arch/arm/mach-renesas/Kconfig.rcar3:197: symbol DM_RESET is selected
>      by CLK_RCAR_GEN3
>   drivers/clk/renesas/Kconfig:53: symbol CLK_RCAR_GEN3 depends on
>      CLK_RENESAS
>   drivers/clk/renesas/Kconfig:1: symbol CLK_RENESAS depends on CLK
>   drivers/clk/Kconfig:3: symbol CLK is selected by IMX8M_POWER_DOMAIN
>   drivers/power/domain/Kconfig:35: symbol IMX8M_POWER_DOMAIN depends on
>      POWER_DOMAIN
>   drivers/power/domain/Kconfig:3: symbol POWER_DOMAIN is selected by
>      BCM6318_USBH_PHY
>   drivers/phy/Kconfig:83: symbol BCM6318_USBH_PHY depends on PHY
>   drivers/phy/Kconfig:4: symbol PHY is selected by USB_EHCI_MX7
>   drivers/usb/host/Kconfig:211: symbol USB_EHCI_MX7 depends on USB
>   drivers/usb/Kconfig:1: symbol USB is selected by BOOTSTD_DEFAULTS
>   boot/Kconfig:455: symbol BOOTSTD_DEFAULTS depends on BOOTSTD
>   boot/Kconfig:398: symbol BOOTSTD depends on BLK
>   drivers/block/Kconfig:1: symbol BLK is selected by PVBLOCK
>   drivers/xen/Kconfig:1: symbol PVBLOCK depends on XEN
>   Kconfig:176: symbol XEN depends on ARM64
> 
> We don't want to revert the change to BLK, which has been in place for
> a year now. We don't want to select BLK in BOOTSTD since it should
> support booting without block devices. The only realistic option is to
> remove BOOTSTD's dependency on BLK.
> 
> Disable standard boot on the one board which fails.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> (no changes since v3)
> 
> Changes in v3:
> - Drop wip (work-in-progress) comment in commit
> 
> Changes in v2:
> - Add new patch to resolve BOOTSTD->BLK recursion with Kconfig
> 
>  boot/Kconfig                                   | 2 +-
>  configs/gardena-smart-gateway-mt7688_defconfig | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/boot/Kconfig b/boot/Kconfig
> index 7dd30a030e3..b5433e88f10 100644
> --- a/boot/Kconfig
> +++ b/boot/Kconfig
> @@ -393,7 +393,7 @@ config BOOT_DEFAULTS
>  menuconfig BOOTSTD
>  	bool "Standard boot"
>  	default y
> -	depends on DM && OF_CONTROL && BLK
> +	depends on DM && OF_CONTROL
>  	help
>  	  U-Boot supports a standard way of locating something to boot,
>  	  typically an Operating System such as Linux, provided by a distro such

This ends up being a massive size bloat on all of the boards which did
not use BOOTSTD before, and still can't (because there's no appropriate
methods). You need to not just disable it on the one board that fails
but on everything not currently enabling it, which now does enable it.

Or you need to better explain what's going on here, exactly and why
depending on BLK here is wrong, for what you're doing.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v5 3/8] efi_loader: bootstd: Drop bootmgr for sunxi

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 647 bytes --]

On Wed, Nov 13, 2024 at 08:09:33AM -0700, Simon Glass wrote:

> This causes problems with the boot order, so drop it until we can figure
> out a better way to know when bootmgr should be used.
> 
> Link: https://lore.kernel.org/u-boot/20241112171205.4e80548d@donnerap.manchester.arm.com/
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

I would really prefer a short summary of the contents of the link in the
commit message. I'm not NAK'ing the patch as-is, but it would be helpful
when revisiting this if there was a short explanation of the problem to
solve here and not just "problems with the boot order". Thanks.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v5 4/8] RFC: Revert "bootstd: Make efi_mgr bootmeth work for non-sandbox setups"

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 895 bytes --]

On Wed, Nov 13, 2024 at 08:09:34AM -0700, Simon Glass wrote:

> This is another option to fix sunxi booting with bootstd, which may be
> better since it will work for all boards. We can then figure out how to
> automatically and deterministicaly decide when bootmgr should be used.
> 
> This reverts commit f2bfa0cb17948aa4a0fa20fdf9014296b9c4d9c7.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> If this patch is applied, we don't need to drop bootmgr for sunxi
> 
> (no changes since v1)
> 
>  boot/bootmeth_efi_mgr.c | 18 +-----------------
>  1 file changed, 1 insertion(+), 17 deletions(-)

This seems like the wrong direction to go since one of the common cases
should be "Do we just want to fire off the EFI boot manager? Yes" since
lots of "off the shelf" operating systems want to treat ARM just like
x86 and not have to special-case things.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v5 7/8] env: Provide a work-around for unquoting fdtfile

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 480 bytes --]

On Wed, Nov 13, 2024 at 08:09:37AM -0700, Simon Glass wrote:

> Some boards use a CONFIG option to specify the value of this variable.
> This is normally handled by efi_get_distro_fdt_name() but in the case
> of sunxi this does not work, since 'soc' is sunxi, but the files are
> in the allwinner directory.
> 
> Provide a work-around for this particular case.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v5 1/8] bootstd: Avoid depending on BLK

[Simon Glass]

Hi Tom,

On Wed, 13 Nov 2024 at 10:47, Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Nov 13, 2024 at 08:09:31AM -0700, Simon Glass wrote:
>
> > In principle bootstd can work without block devices, even if it does
> > require driver model to be enabled in that case.
> >
> > The use of a 'depends on BLK' for BOOTSTD conflicts with the way 'BLK'
> > is now defined, producing recursive errors through multiple different
> > paths, one of which is this (with Linksprite_pcDuino3 and
> > BOOTSTD_DEFAULTS enabled):
> >
> >   arch/arm/Kconfig:7:error: recursive dependency detected!
> >   arch/arm/Kconfig:7: symbol ARM64 is selected by ARCH_UNIPHIER_V8_MULTI
> >   arch/arm/mach-uniphier/Kconfig:17: symbol ARCH_UNIPHIER_V8_MULTI is
> >      part of choice <choice>
> >   arch/arm/mach-uniphier/Kconfig:6: choice <choice> contains symbol
> >      ARCH_UNIPHIER_V8_MULTI
> >   arch/arm/mach-uniphier/Kconfig:17: symbol ARCH_UNIPHIER_V8_MULTI is
> >      part of choice SPL
> >   arch/arm/mach-stm32mp/Kconfig:3: symbol SPL depends on SUPPORT_SPL
> >   common/spl/Kconfig:1: symbol SUPPORT_SPL is selected by ASPEED_AST2600
> >   arch/arm/mach-aspeed/Kconfig:26: symbol ASPEED_AST2600 is part of
> >      choice <choice>
> >   arch/arm/mach-aspeed/Kconfig:12: choice <choice> contains symbol
> >      ASPEED_AST2500
> >   arch/arm/mach-aspeed/Kconfig:17: symbol ASPEED_AST2500 is part of
> >      choice DM_RESET
> >   arch/arm/mach-renesas/Kconfig.rcar3:197: symbol DM_RESET is selected
> >      by CLK_RCAR_GEN3
> >   drivers/clk/renesas/Kconfig:53: symbol CLK_RCAR_GEN3 depends on
> >      CLK_RENESAS
> >   drivers/clk/renesas/Kconfig:1: symbol CLK_RENESAS depends on CLK
> >   drivers/clk/Kconfig:3: symbol CLK is selected by IMX8M_POWER_DOMAIN
> >   drivers/power/domain/Kconfig:35: symbol IMX8M_POWER_DOMAIN depends on
> >      POWER_DOMAIN
> >   drivers/power/domain/Kconfig:3: symbol POWER_DOMAIN is selected by
> >      BCM6318_USBH_PHY
> >   drivers/phy/Kconfig:83: symbol BCM6318_USBH_PHY depends on PHY
> >   drivers/phy/Kconfig:4: symbol PHY is selected by USB_EHCI_MX7
> >   drivers/usb/host/Kconfig:211: symbol USB_EHCI_MX7 depends on USB
> >   drivers/usb/Kconfig:1: symbol USB is selected by BOOTSTD_DEFAULTS
> >   boot/Kconfig:455: symbol BOOTSTD_DEFAULTS depends on BOOTSTD
> >   boot/Kconfig:398: symbol BOOTSTD depends on BLK
> >   drivers/block/Kconfig:1: symbol BLK is selected by PVBLOCK
> >   drivers/xen/Kconfig:1: symbol PVBLOCK depends on XEN
> >   Kconfig:176: symbol XEN depends on ARM64
> >
> > We don't want to revert the change to BLK, which has been in place for
> > a year now. We don't want to select BLK in BOOTSTD since it should
> > support booting without block devices. The only realistic option is to
> > remove BOOTSTD's dependency on BLK.
> >
> > Disable standard boot on the one board which fails.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > (no changes since v3)
> >
> > Changes in v3:
> > - Drop wip (work-in-progress) comment in commit
> >
> > Changes in v2:
> > - Add new patch to resolve BOOTSTD->BLK recursion with Kconfig
> >
> >  boot/Kconfig                                   | 2 +-
> >  configs/gardena-smart-gateway-mt7688_defconfig | 1 +
> >  2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/boot/Kconfig b/boot/Kconfig
> > index 7dd30a030e3..b5433e88f10 100644
> > --- a/boot/Kconfig
> > +++ b/boot/Kconfig
> > @@ -393,7 +393,7 @@ config BOOT_DEFAULTS
> >  menuconfig BOOTSTD
> >       bool "Standard boot"
> >       default y
> > -     depends on DM && OF_CONTROL && BLK
> > +     depends on DM && OF_CONTROL
> >       help
> >         U-Boot supports a standard way of locating something to boot,
> >         typically an Operating System such as Linux, provided by a distro such
>
> This ends up being a massive size bloat on all of the boards which did
> not use BOOTSTD before, and still can't (because there's no appropriate
> methods). You need to not just disable it on the one board that fails
> but on everything not currently enabling it, which now does enable it.

Looking through the list it is hard to know which boards can't use
bootstd, nor what the missing methods are. See [1]. I could perhaps
disable bootstd for all of the boards?

>
> Or you need to better explain what's going on here, exactly and why
> depending on BLK here is wrong, for what you're doing.

I tried that already. We had quite a long thread about it.

Regards,
Simon

[1]
10m50
3c120
CMPC885
CMPCPRO
M5208EVBE
M5235EVB_Flash32
M5235EVB
M5249EVB
M5272C3
M5275EVB
M5282EVB
M53017EVB
M5329AFEE
M5329BFEE
M5373EVB
MCR3000
MPC8548CDS_36BIT
MPC8548CDS
MPC8548CDS_legacy
SBx81LIFKW
SBx81LIFXCAT
amcore
amd_versal2_mini
amd_versal2_mini_ospi
amd_versal2_mini_qspi
ap121
ap143
ap152
astro_mcf5373l
axm
bcm968380gerg_ram
bcmns
boston32r2
boston32r2el
boston32r6
boston32r6el
boston64r2
boston64r2el
boston64r6
boston64r6el
cobra5272
cortina_presidio-asic-base
cortina_presidio-asic-pnand
crs305-1g-4s-bit
crs305-1g-4s
crs326-24g-2s-bit
crs326-24g-2s
crs328-4c-20s-4s-bit
crs328-4c-20s-4s
eb_cpu5282
eb_cpu5282_internal
efi-x86_app32
efi-x86_app64
evb-ast2500
gardena-smart-gateway-at91sam
gardena-smart-gateway-mt7688
gxp
ibex-ast2700
imgtec_xilfpga
integratorap_cm720t
integratorap_cm920t
integratorap_cm926ejs
integratorap_cm946es
integratorcp_cm1136
integratorcp_cm920t
integratorcp_cm926ejs
integratorcp_cm946es
inteno_xg6846_ram
kmcoge5ne
kmeter1
kmopti2
kmsupx5
kmtepr2
lschlv2
lsxhl
maxbcm
meesc_dataflash
meesc
microblaze-generic
mscc_jr2
mscc_luton
mscc_ocelot
mscc_serval
mscc_servalt
mt7620_mt7530_rfb
mt7628_rfb
mt7981_rfb
mt7986_rfb
mx6memcal
netgear_cg3100d_ram
nsim_700
nsim_700be
nsim_hs38be
pg_wcom_expu1
pg_wcom_expu1_update
pg_wcom_seli8
pg_wcom_seli8_update
r8a77970_eagle
rcar3_salvator-x
sagem_f@st1704_ram
sama5d29_curiosity_mmc1
sama5d29_curiosity_mmc
sama5d29_curiosity_qspiflash
sama7g54_curiosity_mmc
sama7g54_curiosity_nandflash
sama7g54_curiosity_qspiflash
smdkc100
smegw01
socfpga_is1
stm32f429-discovery
stm32mp25
stmark2
tb100
tbs2910
th1520_lpi4a
thunderx_88xx
tuge1
tuxx1
usb_a9263_dataflash
work_92105
xilinx_versal_mini
xilinx_versal_mini_ospi
xilinx_versal_mini_qspi
xilinx_versal_net_mini
xilinx_versal_net_mini_ospi
xilinx_versal_net_mini_qspi
xilinx_zynqmp_mini
xilinx_zynqmp_mini_nand
xilinx_zynqmp_mini_nand_single
xilinx_zynqmp_mini_qspi
xtfpga
zynq_cse_nand
zynq_cse_nor
zynq_cse_qspi

Re: [PATCH v5 1/8] bootstd: Avoid depending on BLK

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 5250 bytes --]

On Wed, Nov 13, 2024 at 08:53:31PM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Wed, 13 Nov 2024 at 10:47, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Wed, Nov 13, 2024 at 08:09:31AM -0700, Simon Glass wrote:
> >
> > > In principle bootstd can work without block devices, even if it does
> > > require driver model to be enabled in that case.
> > >
> > > The use of a 'depends on BLK' for BOOTSTD conflicts with the way 'BLK'
> > > is now defined, producing recursive errors through multiple different
> > > paths, one of which is this (with Linksprite_pcDuino3 and
> > > BOOTSTD_DEFAULTS enabled):
> > >
> > >   arch/arm/Kconfig:7:error: recursive dependency detected!
> > >   arch/arm/Kconfig:7: symbol ARM64 is selected by ARCH_UNIPHIER_V8_MULTI
> > >   arch/arm/mach-uniphier/Kconfig:17: symbol ARCH_UNIPHIER_V8_MULTI is
> > >      part of choice <choice>
> > >   arch/arm/mach-uniphier/Kconfig:6: choice <choice> contains symbol
> > >      ARCH_UNIPHIER_V8_MULTI
> > >   arch/arm/mach-uniphier/Kconfig:17: symbol ARCH_UNIPHIER_V8_MULTI is
> > >      part of choice SPL
> > >   arch/arm/mach-stm32mp/Kconfig:3: symbol SPL depends on SUPPORT_SPL
> > >   common/spl/Kconfig:1: symbol SUPPORT_SPL is selected by ASPEED_AST2600
> > >   arch/arm/mach-aspeed/Kconfig:26: symbol ASPEED_AST2600 is part of
> > >      choice <choice>
> > >   arch/arm/mach-aspeed/Kconfig:12: choice <choice> contains symbol
> > >      ASPEED_AST2500
> > >   arch/arm/mach-aspeed/Kconfig:17: symbol ASPEED_AST2500 is part of
> > >      choice DM_RESET
> > >   arch/arm/mach-renesas/Kconfig.rcar3:197: symbol DM_RESET is selected
> > >      by CLK_RCAR_GEN3
> > >   drivers/clk/renesas/Kconfig:53: symbol CLK_RCAR_GEN3 depends on
> > >      CLK_RENESAS
> > >   drivers/clk/renesas/Kconfig:1: symbol CLK_RENESAS depends on CLK
> > >   drivers/clk/Kconfig:3: symbol CLK is selected by IMX8M_POWER_DOMAIN
> > >   drivers/power/domain/Kconfig:35: symbol IMX8M_POWER_DOMAIN depends on
> > >      POWER_DOMAIN
> > >   drivers/power/domain/Kconfig:3: symbol POWER_DOMAIN is selected by
> > >      BCM6318_USBH_PHY
> > >   drivers/phy/Kconfig:83: symbol BCM6318_USBH_PHY depends on PHY
> > >   drivers/phy/Kconfig:4: symbol PHY is selected by USB_EHCI_MX7
> > >   drivers/usb/host/Kconfig:211: symbol USB_EHCI_MX7 depends on USB
> > >   drivers/usb/Kconfig:1: symbol USB is selected by BOOTSTD_DEFAULTS
> > >   boot/Kconfig:455: symbol BOOTSTD_DEFAULTS depends on BOOTSTD
> > >   boot/Kconfig:398: symbol BOOTSTD depends on BLK
> > >   drivers/block/Kconfig:1: symbol BLK is selected by PVBLOCK
> > >   drivers/xen/Kconfig:1: symbol PVBLOCK depends on XEN
> > >   Kconfig:176: symbol XEN depends on ARM64
> > >
> > > We don't want to revert the change to BLK, which has been in place for
> > > a year now. We don't want to select BLK in BOOTSTD since it should
> > > support booting without block devices. The only realistic option is to
> > > remove BOOTSTD's dependency on BLK.
> > >
> > > Disable standard boot on the one board which fails.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > ---
> > >
> > > (no changes since v3)
> > >
> > > Changes in v3:
> > > - Drop wip (work-in-progress) comment in commit
> > >
> > > Changes in v2:
> > > - Add new patch to resolve BOOTSTD->BLK recursion with Kconfig
> > >
> > >  boot/Kconfig                                   | 2 +-
> > >  configs/gardena-smart-gateway-mt7688_defconfig | 1 +
> > >  2 files changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/boot/Kconfig b/boot/Kconfig
> > > index 7dd30a030e3..b5433e88f10 100644
> > > --- a/boot/Kconfig
> > > +++ b/boot/Kconfig
> > > @@ -393,7 +393,7 @@ config BOOT_DEFAULTS
> > >  menuconfig BOOTSTD
> > >       bool "Standard boot"
> > >       default y
> > > -     depends on DM && OF_CONTROL && BLK
> > > +     depends on DM && OF_CONTROL
> > >       help
> > >         U-Boot supports a standard way of locating something to boot,
> > >         typically an Operating System such as Linux, provided by a distro such
> >
> > This ends up being a massive size bloat on all of the boards which did
> > not use BOOTSTD before, and still can't (because there's no appropriate
> > methods). You need to not just disable it on the one board that fails
> > but on everything not currently enabling it, which now does enable it.
> 
> Looking through the list it is hard to know which boards can't use
> bootstd, nor what the missing methods are. See [1]. I could perhaps
> disable bootstd for all of the boards?

Well, since you cannot have a block device without BLK at this point,
none of them can use bootstd since there's no methods for whatever flash
they use?

> > Or you need to better explain what's going on here, exactly and why
> > depending on BLK here is wrong, for what you're doing.
> 
> I tried that already. We had quite a long thread about it.

Yes, can you remind me please? I still don't see why this is required
for sunxi support. And I think this is another good example of where
your commit message explains your solution, but not the underlying
problem you're solving. None of the platforms in [1] are ARCH_SUNXI.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v5 3/8] efi_loader: bootstd: Drop bootmgr for sunxi

[Simon Glass]

Hi Tom,

On Wed, 13 Nov 2024 at 10:50, Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Nov 13, 2024 at 08:09:33AM -0700, Simon Glass wrote:
>
> > This causes problems with the boot order, so drop it until we can figure
> > out a better way to know when bootmgr should be used.
> >
> > Link: https://lore.kernel.org/u-boot/20241112171205.4e80548d@donnerap.manchester.arm.com/
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
>
> I would really prefer a short summary of the contents of the link in the
> commit message. I'm not NAK'ing the patch as-is, but it would be helpful
> when revisiting this if there was a short explanation of the problem to
> solve here and not just "problems with the boot order". Thanks.

The summary is that sunxi supports FEL (USB boot) but bootmgr is not
aware of it, so boots the OS instead.

Regards,
Simon

Re: [PATCH v5 1/8] bootstd: Avoid depending on BLK

[Simon Glass]

Hi Tom,

On Thu, 14 Nov 2024 at 07:22, Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Nov 13, 2024 at 08:53:31PM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Wed, 13 Nov 2024 at 10:47, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Wed, Nov 13, 2024 at 08:09:31AM -0700, Simon Glass wrote:
> > >
> > > > In principle bootstd can work without block devices, even if it does
> > > > require driver model to be enabled in that case.
> > > >
> > > > The use of a 'depends on BLK' for BOOTSTD conflicts with the way 'BLK'
> > > > is now defined, producing recursive errors through multiple different
> > > > paths, one of which is this (with Linksprite_pcDuino3 and
> > > > BOOTSTD_DEFAULTS enabled):
> > > >
> > > >   arch/arm/Kconfig:7:error: recursive dependency detected!
> > > >   arch/arm/Kconfig:7: symbol ARM64 is selected by ARCH_UNIPHIER_V8_MULTI
> > > >   arch/arm/mach-uniphier/Kconfig:17: symbol ARCH_UNIPHIER_V8_MULTI is
> > > >      part of choice <choice>
> > > >   arch/arm/mach-uniphier/Kconfig:6: choice <choice> contains symbol
> > > >      ARCH_UNIPHIER_V8_MULTI
> > > >   arch/arm/mach-uniphier/Kconfig:17: symbol ARCH_UNIPHIER_V8_MULTI is
> > > >      part of choice SPL
> > > >   arch/arm/mach-stm32mp/Kconfig:3: symbol SPL depends on SUPPORT_SPL
> > > >   common/spl/Kconfig:1: symbol SUPPORT_SPL is selected by ASPEED_AST2600
> > > >   arch/arm/mach-aspeed/Kconfig:26: symbol ASPEED_AST2600 is part of
> > > >      choice <choice>
> > > >   arch/arm/mach-aspeed/Kconfig:12: choice <choice> contains symbol
> > > >      ASPEED_AST2500
> > > >   arch/arm/mach-aspeed/Kconfig:17: symbol ASPEED_AST2500 is part of
> > > >      choice DM_RESET
> > > >   arch/arm/mach-renesas/Kconfig.rcar3:197: symbol DM_RESET is selected
> > > >      by CLK_RCAR_GEN3
> > > >   drivers/clk/renesas/Kconfig:53: symbol CLK_RCAR_GEN3 depends on
> > > >      CLK_RENESAS
> > > >   drivers/clk/renesas/Kconfig:1: symbol CLK_RENESAS depends on CLK
> > > >   drivers/clk/Kconfig:3: symbol CLK is selected by IMX8M_POWER_DOMAIN
> > > >   drivers/power/domain/Kconfig:35: symbol IMX8M_POWER_DOMAIN depends on
> > > >      POWER_DOMAIN
> > > >   drivers/power/domain/Kconfig:3: symbol POWER_DOMAIN is selected by
> > > >      BCM6318_USBH_PHY
> > > >   drivers/phy/Kconfig:83: symbol BCM6318_USBH_PHY depends on PHY
> > > >   drivers/phy/Kconfig:4: symbol PHY is selected by USB_EHCI_MX7
> > > >   drivers/usb/host/Kconfig:211: symbol USB_EHCI_MX7 depends on USB
> > > >   drivers/usb/Kconfig:1: symbol USB is selected by BOOTSTD_DEFAULTS
> > > >   boot/Kconfig:455: symbol BOOTSTD_DEFAULTS depends on BOOTSTD
> > > >   boot/Kconfig:398: symbol BOOTSTD depends on BLK
> > > >   drivers/block/Kconfig:1: symbol BLK is selected by PVBLOCK
> > > >   drivers/xen/Kconfig:1: symbol PVBLOCK depends on XEN
> > > >   Kconfig:176: symbol XEN depends on ARM64
> > > >
> > > > We don't want to revert the change to BLK, which has been in place for
> > > > a year now. We don't want to select BLK in BOOTSTD since it should
> > > > support booting without block devices. The only realistic option is to
> > > > remove BOOTSTD's dependency on BLK.
> > > >
> > > > Disable standard boot on the one board which fails.
> > > >
> > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > ---
> > > >
> > > > (no changes since v3)
> > > >
> > > > Changes in v3:
> > > > - Drop wip (work-in-progress) comment in commit
> > > >
> > > > Changes in v2:
> > > > - Add new patch to resolve BOOTSTD->BLK recursion with Kconfig
> > > >
> > > >  boot/Kconfig                                   | 2 +-
> > > >  configs/gardena-smart-gateway-mt7688_defconfig | 1 +
> > > >  2 files changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/boot/Kconfig b/boot/Kconfig
> > > > index 7dd30a030e3..b5433e88f10 100644
> > > > --- a/boot/Kconfig
> > > > +++ b/boot/Kconfig
> > > > @@ -393,7 +393,7 @@ config BOOT_DEFAULTS
> > > >  menuconfig BOOTSTD
> > > >       bool "Standard boot"
> > > >       default y
> > > > -     depends on DM && OF_CONTROL && BLK
> > > > +     depends on DM && OF_CONTROL
> > > >       help
> > > >         U-Boot supports a standard way of locating something to boot,
> > > >         typically an Operating System such as Linux, provided by a distro such
> > >
> > > This ends up being a massive size bloat on all of the boards which did
> > > not use BOOTSTD before, and still can't (because there's no appropriate
> > > methods). You need to not just disable it on the one board that fails
> > > but on everything not currently enabling it, which now does enable it.
> >
> > Looking through the list it is hard to know which boards can't use
> > bootstd, nor what the missing methods are. See [1]. I could perhaps
> > disable bootstd for all of the boards?
>
> Well, since you cannot have a block device without BLK at this point,
> none of them can use bootstd since there's no methods for whatever flash
> they use?

We have SPI flash and FEL, for example. Also, sandbox's hostfs doesn't
use BLK. Plus the network bootmeths are available without it.

>
> > > Or you need to better explain what's going on here, exactly and why
> > > depending on BLK here is wrong, for what you're doing.
> >
> > I tried that already. We had quite a long thread about it.
>
> Yes, can you remind me please? I still don't see why this is required
> for sunxi support. And I think this is another good example of where
> your commit message explains your solution, but not the underlying
> problem you're solving. None of the platforms in [1] are ARCH_SUNXI.

Yes it is at [2]. If you look at BLK it says:

config BLK
   bool # "Support block devices"
   depends on DM
   def_bool y if MMC || USB || SCSI || NVME || IDE || AHCI || SATA
   def_bool y if EFI_MEDIA || VIRTIO_BLK || PVBLOCK

We also have, in the commit message:

symbol BLK is selected by EFI_LOADER

Having a 'select X' and 'depends on X' is known to cause problems. We
really shouldn't do it. So I am removing 'depends on BLK'.

Regards,
Simon

[2] https://patchwork.ozlabs.org/project/uboot/patch/20240901222734.462334-4-sjg@chromium.org/

Re: [PATCH v5 1/8] bootstd: Avoid depending on BLK

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 6658 bytes --]

On Fri, Nov 15, 2024 at 07:27:19AM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Thu, 14 Nov 2024 at 07:22, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Wed, Nov 13, 2024 at 08:53:31PM -0700, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Wed, 13 Nov 2024 at 10:47, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Wed, Nov 13, 2024 at 08:09:31AM -0700, Simon Glass wrote:
> > > >
> > > > > In principle bootstd can work without block devices, even if it does
> > > > > require driver model to be enabled in that case.
> > > > >
> > > > > The use of a 'depends on BLK' for BOOTSTD conflicts with the way 'BLK'
> > > > > is now defined, producing recursive errors through multiple different
> > > > > paths, one of which is this (with Linksprite_pcDuino3 and
> > > > > BOOTSTD_DEFAULTS enabled):
> > > > >
> > > > >   arch/arm/Kconfig:7:error: recursive dependency detected!
> > > > >   arch/arm/Kconfig:7: symbol ARM64 is selected by ARCH_UNIPHIER_V8_MULTI
> > > > >   arch/arm/mach-uniphier/Kconfig:17: symbol ARCH_UNIPHIER_V8_MULTI is
> > > > >      part of choice <choice>
> > > > >   arch/arm/mach-uniphier/Kconfig:6: choice <choice> contains symbol
> > > > >      ARCH_UNIPHIER_V8_MULTI
> > > > >   arch/arm/mach-uniphier/Kconfig:17: symbol ARCH_UNIPHIER_V8_MULTI is
> > > > >      part of choice SPL
> > > > >   arch/arm/mach-stm32mp/Kconfig:3: symbol SPL depends on SUPPORT_SPL
> > > > >   common/spl/Kconfig:1: symbol SUPPORT_SPL is selected by ASPEED_AST2600
> > > > >   arch/arm/mach-aspeed/Kconfig:26: symbol ASPEED_AST2600 is part of
> > > > >      choice <choice>
> > > > >   arch/arm/mach-aspeed/Kconfig:12: choice <choice> contains symbol
> > > > >      ASPEED_AST2500
> > > > >   arch/arm/mach-aspeed/Kconfig:17: symbol ASPEED_AST2500 is part of
> > > > >      choice DM_RESET
> > > > >   arch/arm/mach-renesas/Kconfig.rcar3:197: symbol DM_RESET is selected
> > > > >      by CLK_RCAR_GEN3
> > > > >   drivers/clk/renesas/Kconfig:53: symbol CLK_RCAR_GEN3 depends on
> > > > >      CLK_RENESAS
> > > > >   drivers/clk/renesas/Kconfig:1: symbol CLK_RENESAS depends on CLK
> > > > >   drivers/clk/Kconfig:3: symbol CLK is selected by IMX8M_POWER_DOMAIN
> > > > >   drivers/power/domain/Kconfig:35: symbol IMX8M_POWER_DOMAIN depends on
> > > > >      POWER_DOMAIN
> > > > >   drivers/power/domain/Kconfig:3: symbol POWER_DOMAIN is selected by
> > > > >      BCM6318_USBH_PHY
> > > > >   drivers/phy/Kconfig:83: symbol BCM6318_USBH_PHY depends on PHY
> > > > >   drivers/phy/Kconfig:4: symbol PHY is selected by USB_EHCI_MX7
> > > > >   drivers/usb/host/Kconfig:211: symbol USB_EHCI_MX7 depends on USB
> > > > >   drivers/usb/Kconfig:1: symbol USB is selected by BOOTSTD_DEFAULTS
> > > > >   boot/Kconfig:455: symbol BOOTSTD_DEFAULTS depends on BOOTSTD
> > > > >   boot/Kconfig:398: symbol BOOTSTD depends on BLK
> > > > >   drivers/block/Kconfig:1: symbol BLK is selected by PVBLOCK
> > > > >   drivers/xen/Kconfig:1: symbol PVBLOCK depends on XEN
> > > > >   Kconfig:176: symbol XEN depends on ARM64
> > > > >
> > > > > We don't want to revert the change to BLK, which has been in place for
> > > > > a year now. We don't want to select BLK in BOOTSTD since it should
> > > > > support booting without block devices. The only realistic option is to
> > > > > remove BOOTSTD's dependency on BLK.
> > > > >
> > > > > Disable standard boot on the one board which fails.
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > ---
> > > > >
> > > > > (no changes since v3)
> > > > >
> > > > > Changes in v3:
> > > > > - Drop wip (work-in-progress) comment in commit
> > > > >
> > > > > Changes in v2:
> > > > > - Add new patch to resolve BOOTSTD->BLK recursion with Kconfig
> > > > >
> > > > >  boot/Kconfig                                   | 2 +-
> > > > >  configs/gardena-smart-gateway-mt7688_defconfig | 1 +
> > > > >  2 files changed, 2 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/boot/Kconfig b/boot/Kconfig
> > > > > index 7dd30a030e3..b5433e88f10 100644
> > > > > --- a/boot/Kconfig
> > > > > +++ b/boot/Kconfig
> > > > > @@ -393,7 +393,7 @@ config BOOT_DEFAULTS
> > > > >  menuconfig BOOTSTD
> > > > >       bool "Standard boot"
> > > > >       default y
> > > > > -     depends on DM && OF_CONTROL && BLK
> > > > > +     depends on DM && OF_CONTROL
> > > > >       help
> > > > >         U-Boot supports a standard way of locating something to boot,
> > > > >         typically an Operating System such as Linux, provided by a distro such
> > > >
> > > > This ends up being a massive size bloat on all of the boards which did
> > > > not use BOOTSTD before, and still can't (because there's no appropriate
> > > > methods). You need to not just disable it on the one board that fails
> > > > but on everything not currently enabling it, which now does enable it.
> > >
> > > Looking through the list it is hard to know which boards can't use
> > > bootstd, nor what the missing methods are. See [1]. I could perhaps
> > > disable bootstd for all of the boards?
> >
> > Well, since you cannot have a block device without BLK at this point,
> > none of them can use bootstd since there's no methods for whatever flash
> > they use?
> 
> We have SPI flash and FEL, for example. Also, sandbox's hostfs doesn't
> use BLK. Plus the network bootmeths are available without it.
> 
> >
> > > > Or you need to better explain what's going on here, exactly and why
> > > > depending on BLK here is wrong, for what you're doing.
> > >
> > > I tried that already. We had quite a long thread about it.
> >
> > Yes, can you remind me please? I still don't see why this is required
> > for sunxi support. And I think this is another good example of where
> > your commit message explains your solution, but not the underlying
> > problem you're solving. None of the platforms in [1] are ARCH_SUNXI.
> 
> Yes it is at [2]. If you look at BLK it says:
> 
> config BLK
>    bool # "Support block devices"
>    depends on DM
>    def_bool y if MMC || USB || SCSI || NVME || IDE || AHCI || SATA
>    def_bool y if EFI_MEDIA || VIRTIO_BLK || PVBLOCK
> 
> We also have, in the commit message:
> 
> symbol BLK is selected by EFI_LOADER
> 
> Having a 'select X' and 'depends on X' is known to cause problems. We
> really shouldn't do it. So I am removing 'depends on BLK'.

The challenge here is that when working with "default y" symbols, you
need to exercise a lot more care. I'm sending out later today a series
that also addresses the problems this patch exposed.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v5 3/8] efi_loader: bootstd: Drop bootmgr for sunxi

[Heinrich Schuchardt]

Am 13. November 2024 16:09:33 MEZ schrieb Simon Glass <sjg@chromium.org>:
>This causes problems with the boot order, so drop it until we can figure
>out a better way to know when bootmgr should be used.

It dhould be used by default.

Please, describe your observations in sufficient detail.

I don't think that we should break EFI booting on sunxi boards. Please, mark the series as RFC, if it is not working properly.

Best regards

Heinrich

>
>Link: https://lore.kernel.org/u-boot/20241112171205.4e80548d@donnerap.manchester.arm.com/
>
>Signed-off-by: Simon Glass <sjg@chromium.org>
>---
>
>Changes in v5:
>- Add new patch to drop bootmgr for sunxi
>
> boot/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/boot/Kconfig b/boot/Kconfig
>index b612360ca55..42c5ed288c5 100644
>--- a/boot/Kconfig
>+++ b/boot/Kconfig
>@@ -583,7 +583,7 @@ config BOOTMETH_EFI_BOOTMGR
> 	bool "Bootdev support for EFI boot manager"
> 	depends on EFI_BOOTMGR
> 	select BOOTMETH_GLOBAL
>-	default y
>+	default y if !ARCH_SUNXI  # Temporary work-around for sunxi
> 	help
> 	  Enable booting via the UEFI boot manager. Based on the EFI variables
> 	  the EFI binary to be launched is determined. To set the EFI variables

Re: [PATCH v5 3/8] efi_loader: bootstd: Drop bootmgr for sunxi

[Andre Przywara]

On Fri, 15 Nov 2024 07:21:53 -0700
Simon Glass <sjg@chromium.org> wrote:

Hi Simon, Tom, Heinrich,

sorry for the delay, I was away.

> On Wed, 13 Nov 2024 at 10:50, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Wed, Nov 13, 2024 at 08:09:33AM -0700, Simon Glass wrote:
> >  
> > > This causes problems with the boot order, so drop it until we can figure
> > > out a better way to know when bootmgr should be used.
> > >
> > > Link: https://lore.kernel.org/u-boot/20241112171205.4e80548d@donnerap.manchester.arm.com/
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>  
> >
> > I would really prefer a short summary of the contents of the link in the
> > commit message. I'm not NAK'ing the patch as-is, but it would be helpful
> > when revisiting this if there was a short explanation of the problem to
> > solve here and not just "problems with the boot order". Thanks.  
> 
> The summary is that sunxi supports FEL (USB boot) but bootmgr is not
> aware of it, so boots the OS instead.

So that's not the problem: FEL is working fine, since it's marked as
global, same as efi_bootmgr, but with a lower order number. The problem
is that the higher priority non-global methods (scripts) are "ignored".

So this is the list without this patch:
Order  Seq  Name                Description
-----  ---  ------------------  ------------------
    0    0  extlinux            extlinux
 glob    1  fel                 FEL
    2    2  script              script
 glob    3  efi_mgr             EFI bootmgr flow
    4    4  efi                 EFI
    5    5  pxe                 PXE
 glob    6  vbe_simple          vbe-simple
-----  ---  ------------------  ------------------

The sequence looks roughly alright, only that it's not the actual
order, since "glob" takes precedence. So the actual order is:
fel -> efi_mgr -> vbe_simple -> extlinux -> script -> efi -> pxe
And that puts efi_mgr before extlinux and script, which is not what we
want, because efi_mgr finds bootaa64.efi *somewhere* (eMMC or SD card or
USB stick) and uses that, even when there is a tailored boot.scr or
extlinux.conf on the *boot media* which should be higher priority.
"fel" being first is fine, since it's *only* triggered if someone
deliberately uploaded a boot script via USB: in this case they surely
want to execute exactly that.

So I think the underlying problem is the "glob" tag for efi_mgr PLUS
the fact that it scans all media for bootaa64.efi files. I think it
would be fine if efi_mgr would just consider EFI boot order variables,
and would ignore any "removable media" default files (bootaa64.efi).
The "efi" bootmeth does the scanning as well, and it's fine there,
since it's lower priority, so we can still boot EFI systems.

I guess it would make sense if an installer on an USB stick should boot
even if there is something on the eMMC, for instance. But we want to
control that, and we can't, because the order is ignored due to the
"glob" flag.

So while I agree that disabling efi_bootmgr sounds like a hack, it
solves the problem quite elegantly: We keep the (current) boot order,
which prioritises scripts over EFI, but still retain EFI. If people
want EFI first, they can change the order in the environment (I think?).

Without that patch there is quite a regression for users, because
suddenly any bootaa64.efi files *somewhere* would be booted first, and
there is no way to turn that off.

This has bitten me actually: for development I put a custom boot.scr on
an SD card which loads a kernel+DTB via TFTP from my box. And I want
that to be first, despite there being a proper EFI setup on the eMMC or
USB drive.

So I hope that clears that up. I am not a big fan of that patch, but I
would like sunxi to be converted, and Simon seemed to be eager to do
this as well, so in the interest of not blocking this, I am fine with
that.

Cheers,
Andre

Re: [PATCH v5 3/8] efi_loader: bootstd: Drop bootmgr for sunxi

[Heinrich Schuchardt]

Am 23. November 2024 03:08:02 MEZ schrieb Andre Przywara <andre.przywara@arm.com>:
>On Fri, 15 Nov 2024 07:21:53 -0700
>Simon Glass <sjg@chromium.org> wrote:
>
>Hi Simon, Tom, Heinrich,
>
>sorry for the delay, I was away.
>
>> On Wed, 13 Nov 2024 at 10:50, Tom Rini <trini@konsulko.com> wrote:
>> >
>> > On Wed, Nov 13, 2024 at 08:09:33AM -0700, Simon Glass wrote:
>> >  
>> > > This causes problems with the boot order, so drop it until we can figure
>> > > out a better way to know when bootmgr should be used.
>> > >
>> > > Link: https://lore.kernel.org/u-boot/20241112171205.4e80548d@donnerap.manchester.arm.com/
>> > >
>> > > Signed-off-by: Simon Glass <sjg@chromium.org>  
>> >
>> > I would really prefer a short summary of the contents of the link in the
>> > commit message. I'm not NAK'ing the patch as-is, but it would be helpful
>> > when revisiting this if there was a short explanation of the problem to
>> > solve here and not just "problems with the boot order". Thanks.  
>> 
>> The summary is that sunxi supports FEL (USB boot) but bootmgr is not
>> aware of it, so boots the OS instead.
>
>So that's not the problem: FEL is working fine, since it's marked as
>global, same as efi_bootmgr, but with a lower order number. The problem
>is that the higher priority non-global methods (scripts) are "ignored".
>
>So this is the list without this patch:
>Order  Seq  Name                Description
>-----  ---  ------------------  ------------------
>    0    0  extlinux            extlinux
> glob    1  fel                 FEL
>    2    2  script              script
> glob    3  efi_mgr             EFI bootmgr flow
>    4    4  efi                 EFI
>    5    5  pxe                 PXE
> glob    6  vbe_simple          vbe-simple
>-----  ---  ------------------  ------------------
>
>The sequence looks roughly alright, only that it's not the actual
>order, since "glob" takes precedence. So the actual order is:
>fel -> efi_mgr -> vbe_simple -> extlinux -> script -> efi -> pxe
>And that puts efi_mgr before extlinux and script, which is not what we
>want, because efi_mgr finds bootaa64.efi *somewhere* (eMMC or SD card or
>USB stick) and uses that, even when there is a tailored boot.scr or
>extlinux.conf on the *boot media* which should be higher priority.
>"fel" being first is fine, since it's *only* triggered if someone
>deliberately uploaded a boot script via USB: in this case they surely
>want to execute exactly that.
>
>So I think the underlying problem is the "glob" tag for efi_mgr PLUS
>the fact that it scans all media for bootaa64.efi files. I think it
>would be fine if efi_mgr would just consider EFI boot order variables,
>and would ignore any "removable media" default files (bootaa64.efi).
>The "efi" bootmeth does the scanning as well, and it's fine there,
>since it's lower priority, so we can still boot EFI systems.
>
>I guess it would make sense if an installer on an USB stick should boot
>even if there is something on the eMMC, for instance. But we want to
>control that, and we can't, because the order is ignored due to the
>"glob" flag.
>
>So while I agree that disabling efi_bootmgr sounds like a hack, it
>solves the problem quite elegantly: We keep the (current) boot order,
>which prioritises scripts over EFI, but still retain EFI. If people
>want EFI first, they can change the order in the environment (I think?).
>
>Without that patch there is quite a regression for users, because
>suddenly any bootaa64.efi files *somewhere* would be booted first, and
>there is no way to turn that off.
>
>This has bitten me actually: for development I put a custom boot.scr on
>an SD card which loads a kernel+DTB via TFTP from my box. And I want
>that to be first, despite there being a proper EFI setup on the eMMC or
>USB drive.
>
>So I hope that clears that up. I am not a big fan of that patch, but I
>would like sunxi to be converted, and Simon seemed to be eager to do
>this as well, so in the interest of not blocking this, I am fine with
>that.

Disabling EFI will break booting for users that rely on it (like me).

>
>Cheers,
>Andre

Yes, the introduction of boot standard changed the boot order and specifically deprioritizing scripts is unexpected.

The priority sequence used to be

scripts
extlinux
efi
pxe

bootmeth_efimgr always running before scripts be fixed. bootmeth_efi should be removed from the code base as redundant.

Best regards

Heinrich

Re: [PATCH v5 3/8] efi_loader: bootstd: Drop bootmgr for sunxi

[Andre Przywara]

On Sat, 23 Nov 2024 05:06:49 +0100
Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

Hi,

> Am 23. November 2024 03:08:02 MEZ schrieb Andre Przywara <andre.przywara@arm.com>:
> >On Fri, 15 Nov 2024 07:21:53 -0700
> >Simon Glass <sjg@chromium.org> wrote:
> >
> >Hi Simon, Tom, Heinrich,
> >
> >sorry for the delay, I was away.
> >  
> >> On Wed, 13 Nov 2024 at 10:50, Tom Rini <trini@konsulko.com> wrote:  
> >> >
> >> > On Wed, Nov 13, 2024 at 08:09:33AM -0700, Simon Glass wrote:
> >> >    
> >> > > This causes problems with the boot order, so drop it until we can figure
> >> > > out a better way to know when bootmgr should be used.
> >> > >
> >> > > Link: https://lore.kernel.org/u-boot/20241112171205.4e80548d@donnerap.manchester.arm.com/
> >> > >
> >> > > Signed-off-by: Simon Glass <sjg@chromium.org>    
> >> >
> >> > I would really prefer a short summary of the contents of the link in the
> >> > commit message. I'm not NAK'ing the patch as-is, but it would be helpful
> >> > when revisiting this if there was a short explanation of the problem to
> >> > solve here and not just "problems with the boot order". Thanks.    
> >> 
> >> The summary is that sunxi supports FEL (USB boot) but bootmgr is not
> >> aware of it, so boots the OS instead.  
> >
> >So that's not the problem: FEL is working fine, since it's marked as
> >global, same as efi_bootmgr, but with a lower order number. The problem
> >is that the higher priority non-global methods (scripts) are "ignored".
> >
> >So this is the list without this patch:
> >Order  Seq  Name                Description
> >-----  ---  ------------------  ------------------
> >    0    0  extlinux            extlinux
> > glob    1  fel                 FEL
> >    2    2  script              script
> > glob    3  efi_mgr             EFI bootmgr flow
> >    4    4  efi                 EFI
> >    5    5  pxe                 PXE
> > glob    6  vbe_simple          vbe-simple
> >-----  ---  ------------------  ------------------
> >
> >The sequence looks roughly alright, only that it's not the actual
> >order, since "glob" takes precedence. So the actual order is:
> >fel -> efi_mgr -> vbe_simple -> extlinux -> script -> efi -> pxe
> >And that puts efi_mgr before extlinux and script, which is not what we
> >want, because efi_mgr finds bootaa64.efi *somewhere* (eMMC or SD card or
> >USB stick) and uses that, even when there is a tailored boot.scr or
> >extlinux.conf on the *boot media* which should be higher priority.
> >"fel" being first is fine, since it's *only* triggered if someone
> >deliberately uploaded a boot script via USB: in this case they surely
> >want to execute exactly that.
> >
> >So I think the underlying problem is the "glob" tag for efi_mgr PLUS
> >the fact that it scans all media for bootaa64.efi files. I think it
> >would be fine if efi_mgr would just consider EFI boot order variables,
> >and would ignore any "removable media" default files (bootaa64.efi).
> >The "efi" bootmeth does the scanning as well, and it's fine there,
> >since it's lower priority, so we can still boot EFI systems.
> >
> >I guess it would make sense if an installer on an USB stick should boot
> >even if there is something on the eMMC, for instance. But we want to
> >control that, and we can't, because the order is ignored due to the
> >"glob" flag.
> >
> >So while I agree that disabling efi_bootmgr sounds like a hack, it
> >solves the problem quite elegantly: We keep the (current) boot order,
> >which prioritises scripts over EFI, but still retain EFI. If people
> >want EFI first, they can change the order in the environment (I think?).
> >
> >Without that patch there is quite a regression for users, because
> >suddenly any bootaa64.efi files *somewhere* would be booted first, and
> >there is no way to turn that off.
> >
> >This has bitten me actually: for development I put a custom boot.scr on
> >an SD card which loads a kernel+DTB via TFTP from my box. And I want
> >that to be first, despite there being a proper EFI setup on the eMMC or
> >USB drive.
> >
> >So I hope that clears that up. I am not a big fan of that patch, but I
> >would like sunxi to be converted, and Simon seemed to be eager to do
> >this as well, so in the interest of not blocking this, I am fine with
> >that.  
> 
> Disabling EFI will break booting for users that rely on it (like me).

So are you using the UEFI BootOrder variables? How do you set them?
Manually on the U-Boot shell, or via some Linux tool manipulating the
U-Boot environment file? IIUC those are the only two methods that work
for sunxi, since the UEFI variable runtime services are not implemented.

In reality I am using UEFI booting as well, but entirely rely on
bootaa64.efi, hence bootmeth_efi is totally sufficient for me.

> Yes, the introduction of boot standard changed the boot order and specifically deprioritizing scripts is unexpected.
> 
> The priority sequence used to be
> 
> scripts
> extlinux
> efi
> pxe
>
> bootmeth_efimgr always running before scripts be fixed.

I agree, but with bootmeth_efimgr marked as global this is not gonna work
easily, at least that's my understanding.

> bootmeth_efi should be removed from the code base as redundant.

I wonder if a solution is to split this up: bootmeth_efimgr is only
looking at variables - then "glob" would be correct. Then people
can place bootmeth_efi at the position they want, either before the
scripts (allowing USB boot) or after. I think this would also solve the
case where bootaa64.efi is both on an eMMC and an SD card, as it would
pick the boot media first.

A lot of sunxi use cases are embedded, and people mostly don't
want to boot from USB on for instance their TV box, just because the
stick with the movies and some bootaa64.efi was still in the slot when
they turned the box on. Even more so for truly embedded devices. And
that's also the default setting for PCs, I think, where USB booting
would need to be explicitly enabled in the BIOS.

Cheers,
Andre

Re: [PATCH v5 3/8] efi_loader: bootstd: Drop bootmgr for sunxi

[Ilias Apalodimas]

Hi Andre,

[...]

> > >So while I agree that disabling efi_bootmgr sounds like a hack, it
> > >solves the problem quite elegantly: We keep the (current) boot order,
> > >which prioritises scripts over EFI, but still retain EFI. If people
> > >want EFI first, they can change the order in the environment (I think?).
> > >
> > >Without that patch there is quite a regression for users, because
> > >suddenly any bootaa64.efi files *somewhere* would be booted first, and
> > >there is no way to turn that off.
> > >
> > >This has bitten me actually: for development I put a custom boot.scr on
> > >an SD card which loads a kernel+DTB via TFTP from my box. And I want
> > >that to be first, despite there being a proper EFI setup on the eMMC or
> > >USB drive.
> > >
> > >So I hope that clears that up. I am not a big fan of that patch, but I
> > >would like sunxi to be converted, and Simon seemed to be eager to do
> > >this as well, so in the interest of not blocking this, I am fine with
> > >that.
> >
> > Disabling EFI will break booting for users that rely on it (like me).
>
> So are you using the UEFI BootOrder variables? How do you set them?
> Manually on the U-Boot shell, or via some Linux tool manipulating the
> U-Boot environment file? IIUC those are the only two methods that work
> for sunxi, since the UEFI variable runtime services are not implemented.

I haven't followed the rest of the thread but EFI runtime does work
since commit 00da8d65a3ba.
Yes for now you have to manually dd the file on updates, but this is
supposed to be solved by [0]

>
> In reality I am using UEFI booting as well, but entirely rely on
> bootaa64.efi, hence bootmeth_efi is totally sufficient for me.
>
> > Yes, the introduction of boot standard changed the boot order and specifically deprioritizing scripts is unexpected.
> >
> > The priority sequence used to be
> >
> > scripts
> > extlinux
> > efi
> > pxe
> >
> > bootmeth_efimgr always running before scripts be fixed.
>
> I agree, but with bootmeth_efimgr marked as global this is not gonna work
> easily, at least that's my understanding.
>
> > bootmeth_efi should be removed from the code base as redundant.
>
> I wonder if a solution is to split this up: bootmeth_efimgr is only
> looking at variables - then "glob" would be correct. Then people
> can place bootmeth_efi at the position they want, either before the
> scripts (allowing USB boot) or after. I think this would also solve the
> case where bootaa64.efi is both on an eMMC and an SD card, as it would
> pick the boot media first.
>
> A lot of sunxi use cases are embedded, and people mostly don't
> want to boot from USB on for instance their TV box, just because the
> stick with the movies and some bootaa64.efi was still in the slot when
> they turned the box on. Even more so for truly embedded devices. And
> that's also the default setting for PCs, I think, where USB booting
> would need to be explicitly enabled in the BIOS.
>
> Cheers,
> Andre

[0] https://github.com/rhboot/efivar/pull/267

Cheers
/Ilias

Re: [PATCH v5 4/8] RFC: Revert "bootstd: Make efi_mgr bootmeth work for non-sandbox setups"

[Heinrich Schuchardt]

On 11/13/24 16:09, Simon Glass wrote:
> This is another option to fix sunxi booting with bootstd, which may be
> better since it will work for all boards. We can then figure out how to
> automatically and deterministicaly decide when bootmgr should be used.
>
> This reverts commit f2bfa0cb17948aa4a0fa20fdf9014296b9c4d9c7.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> If this patch is applied, we don't need to drop bootmgr for sunxi
>
> (no changes since v1)
>
>   boot/bootmeth_efi_mgr.c | 18 +-----------------
>   1 file changed, 1 insertion(+), 17 deletions(-)
>
> diff --git a/boot/bootmeth_efi_mgr.c b/boot/bootmeth_efi_mgr.c
> index 23ae1e610ac..095fa74fc60 100644
> --- a/boot/bootmeth_efi_mgr.c
> +++ b/boot/bootmeth_efi_mgr.c
> @@ -14,8 +14,6 @@
>   #include <command.h>
>   #include <dm.h>
>   #include <efi_loader.h>
> -#include <efi_variable.h>
> -#include <malloc.h>
>
>   /**
>    * struct efi_mgr_priv - private info for the efi-mgr driver
> @@ -48,27 +46,13 @@ static int efi_mgr_check(struct udevice *dev, struct bootflow_iter *iter)
>   static int efi_mgr_read_bootflow(struct udevice *dev, struct bootflow *bflow)
>   {
>   	struct efi_mgr_priv *priv = dev_get_priv(dev);
> -	efi_status_t ret;
> -	efi_uintn_t size;
> -	u16 *bootorder;
>
>   	if (priv->fake_dev) {
>   		bflow->state = BOOTFLOWST_READY;
>   		return 0;
>   	}
>
> -	ret = efi_init_obj_list();
> -	if (ret)
> -		return log_msg_ret("init", ret);
> -
> -	/* Enable this method if the "BootOrder" UEFI exists. */
> -	bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid,
> -				&size);
> -	if (bootorder) {
> -		free(bootorder);
> -		bflow->state = BOOTFLOWST_READY;
> -		return 0;
> -	}
> +	/* To be implemented */

The EFI boot manager can boot based on:

* variable BootOrder
* variable BootNext
* an existing file EFI/BOOT/BOOT<arch>.EFI

It obsoletes bootsmeth_efi.

>
>   	return -EINVAL;

We must always run the EFI boot manager if it is enabled. So -EINVAL is
wrong here.

Best regards

Heinrich

Re: [PATCH v5 4/8] RFC: Revert "bootstd: Make efi_mgr bootmeth work for non-sandbox setups"

[Simon Glass]

Hi Heinrich,

On Sat, 4 Jan 2025 at 19:50, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 11/13/24 16:09, Simon Glass wrote:
> > This is another option to fix sunxi booting with bootstd, which may be
> > better since it will work for all boards. We can then figure out how to
> > automatically and deterministicaly decide when bootmgr should be used.
> >
> > This reverts commit f2bfa0cb17948aa4a0fa20fdf9014296b9c4d9c7.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> > If this patch is applied, we don't need to drop bootmgr for sunxi
> >
> > (no changes since v1)
> >
> >   boot/bootmeth_efi_mgr.c | 18 +-----------------
> >   1 file changed, 1 insertion(+), 17 deletions(-)avilable
> >
> > diff --git a/boot/bootmeth_efi_mgr.c b/boot/bootmeth_efi_mgr.c
> > index 23ae1e610ac..095fa74fc60 100644
> > --- a/boot/bootmeth_efi_mgr.c
> > +++ b/boot/bootmeth_efi_mgr.c
> > @@ -14,8 +14,6 @@
> >   #include <command.h>
> >   #include <dm.h>
> >   #include <efi_loader.h>
> > -#include <efi_variable.h>
> > -#include <malloc.h>
> >
> >   /**
> >    * struct efi_mgr_priv - private info for the efi-mgr driver
> > @@ -48,27 +46,13 @@ static int efi_mgr_check(struct udevice *dev, struct bootflow_iter *iter)
> >   static int efi_mgr_read_bootflow(struct udevice *dev, struct bootflow *bflow)
> >   {
> >       struct efi_mgr_priv *priv = dev_get_priv(dev);
> > -     efi_status_t ret;
> > -     efi_uintn_t size;
> > -     u16 *bootorder;
> >
> >       if (priv->fake_dev) {
> >               bflow->state = BOOTFLOWST_READY;
> >               return 0;
> >       }
> >
> > -     ret = efi_init_obj_list();
> > -     if (ret)
> > -             return log_msg_ret("init", ret);
> > -
> > -     /* Enable this method if the "BootOrder" UEFI exists. */
> > -     bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid,
> > -                             &size);
> > -     if (bootorder) {
> > -             free(bootorder);
> > -             bflow->state = BOOTFLOWST_READY;
> > -             return 0;
> > -     }
> > +     /* To be implemented */
>
> The EFI boot manager can boot based on:
>
> * variable BootOrder
> * variable BootNext
> * an existing file EFI/BOOT/BOOT<arch>.EFI
>
> It obsoletes bootsmeth_efi.
>
> >
> >       return -EINVAL;
>
> We must always run the EFI boot manager if it is enabled. So -EINVAL is
> wrong here.

Well, I don't believe you have a solution, then.

You did suggest putting bootmgr later, as we discussed on irc.

For now, I think we should apply this patch (and series), while we
sort out how to make bootmgr more incremental. It should not be
scanning every available device before it starts, since that can be
very slow.

Regards,
Simon

Re: [PATCH v5 4/8] RFC: Revert "bootstd: Make efi_mgr bootmeth work for non-sandbox setups"

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3488 bytes --]

On Thu, Jan 09, 2025 at 08:01:13AM -0700, Simon Glass wrote:
> Hi Heinrich,
> 
> On Sat, 4 Jan 2025 at 19:50, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> > On 11/13/24 16:09, Simon Glass wrote:
> > > This is another option to fix sunxi booting with bootstd, which may be
> > > better since it will work for all boards. We can then figure out how to
> > > automatically and deterministicaly decide when bootmgr should be used.
> > >
> > > This reverts commit f2bfa0cb17948aa4a0fa20fdf9014296b9c4d9c7.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > ---
> > > If this patch is applied, we don't need to drop bootmgr for sunxi
> > >
> > > (no changes since v1)
> > >
> > >   boot/bootmeth_efi_mgr.c | 18 +-----------------
> > >   1 file changed, 1 insertion(+), 17 deletions(-)avilable
> > >
> > > diff --git a/boot/bootmeth_efi_mgr.c b/boot/bootmeth_efi_mgr.c
> > > index 23ae1e610ac..095fa74fc60 100644
> > > --- a/boot/bootmeth_efi_mgr.c
> > > +++ b/boot/bootmeth_efi_mgr.c
> > > @@ -14,8 +14,6 @@
> > >   #include <command.h>
> > >   #include <dm.h>
> > >   #include <efi_loader.h>
> > > -#include <efi_variable.h>
> > > -#include <malloc.h>
> > >
> > >   /**
> > >    * struct efi_mgr_priv - private info for the efi-mgr driver
> > > @@ -48,27 +46,13 @@ static int efi_mgr_check(struct udevice *dev, struct bootflow_iter *iter)
> > >   static int efi_mgr_read_bootflow(struct udevice *dev, struct bootflow *bflow)
> > >   {
> > >       struct efi_mgr_priv *priv = dev_get_priv(dev);
> > > -     efi_status_t ret;
> > > -     efi_uintn_t size;
> > > -     u16 *bootorder;
> > >
> > >       if (priv->fake_dev) {
> > >               bflow->state = BOOTFLOWST_READY;
> > >               return 0;
> > >       }
> > >
> > > -     ret = efi_init_obj_list();
> > > -     if (ret)
> > > -             return log_msg_ret("init", ret);
> > > -
> > > -     /* Enable this method if the "BootOrder" UEFI exists. */
> > > -     bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid,
> > > -                             &size);
> > > -     if (bootorder) {
> > > -             free(bootorder);
> > > -             bflow->state = BOOTFLOWST_READY;
> > > -             return 0;
> > > -     }
> > > +     /* To be implemented */
> >
> > The EFI boot manager can boot based on:
> >
> > * variable BootOrder
> > * variable BootNext
> > * an existing file EFI/BOOT/BOOT<arch>.EFI
> >
> > It obsoletes bootsmeth_efi.
> >
> > >
> > >       return -EINVAL;
> >
> > We must always run the EFI boot manager if it is enabled. So -EINVAL is
> > wrong here.
> 
> Well, I don't believe you have a solution, then.
> 
> You did suggest putting bootmgr later, as we discussed on irc.
> 
> For now, I think we should apply this patch (and series), while we
> sort out how to make bootmgr more incremental. It should not be
> scanning every available device before it starts, since that can be
> very slow.

Heinrich and I talked the other day, and we think the right path is the
"later" path, where we don't try and use efi bootmanager until
everything has been probed, and also drop the single "efi" option. This
should mean that by the time we would be trying efi bootmanager most if
not everything that needs to be probed has been probed. It doesn't make
any sense to have "efi bootmanger" be more incremental as conceptually
it's point is to show the user all the options.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v5 4/8] RFC: Revert "bootstd: Make efi_mgr bootmeth work for non-sandbox setups"

[Simon Glass]

Hi Tom,

On Thu, 9 Jan 2025 at 08:05, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Jan 09, 2025 at 08:01:13AM -0700, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Sat, 4 Jan 2025 at 19:50, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > >
> > > On 11/13/24 16:09, Simon Glass wrote:
> > > > This is another option to fix sunxi booting with bootstd, which may be
> > > > better since it will work for all boards. We can then figure out how to
> > > > automatically and deterministicaly decide when bootmgr should be used.
> > > >
> > > > This reverts commit f2bfa0cb17948aa4a0fa20fdf9014296b9c4d9c7.
> > > >
> > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > ---
> > > > If this patch is applied, we don't need to drop bootmgr for sunxi
> > > >
> > > > (no changes since v1)
> > > >
> > > >   boot/bootmeth_efi_mgr.c | 18 +-----------------
> > > >   1 file changed, 1 insertion(+), 17 deletions(-)avilable
> > > >
> > > > diff --git a/boot/bootmeth_efi_mgr.c b/boot/bootmeth_efi_mgr.c
> > > > index 23ae1e610ac..095fa74fc60 100644
> > > > --- a/boot/bootmeth_efi_mgr.c
> > > > +++ b/boot/bootmeth_efi_mgr.c
> > > > @@ -14,8 +14,6 @@
> > > >   #include <command.h>
> > > >   #include <dm.h>
> > > >   #include <efi_loader.h>
> > > > -#include <efi_variable.h>
> > > > -#include <malloc.h>
> > > >
> > > >   /**
> > > >    * struct efi_mgr_priv - private info for the efi-mgr driver
> > > > @@ -48,27 +46,13 @@ static int efi_mgr_check(struct udevice *dev, struct bootflow_iter *iter)
> > > >   static int efi_mgr_read_bootflow(struct udevice *dev, struct bootflow *bflow)
> > > >   {
> > > >       struct efi_mgr_priv *priv = dev_get_priv(dev);
> > > > -     efi_status_t ret;
> > > > -     efi_uintn_t size;
> > > > -     u16 *bootorder;
> > > >
> > > >       if (priv->fake_dev) {
> > > >               bflow->state = BOOTFLOWST_READY;
> > > >               return 0;
> > > >       }
> > > >
> > > > -     ret = efi_init_obj_list();
> > > > -     if (ret)
> > > > -             return log_msg_ret("init", ret);
> > > > -
> > > > -     /* Enable this method if the "BootOrder" UEFI exists. */
> > > > -     bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid,
> > > > -                             &size);
> > > > -     if (bootorder) {
> > > > -             free(bootorder);
> > > > -             bflow->state = BOOTFLOWST_READY;
> > > > -             return 0;
> > > > -     }
> > > > +     /* To be implemented */
> > >
> > > The EFI boot manager can boot based on:
> > >
> > > * variable BootOrder
> > > * variable BootNext
> > > * an existing file EFI/BOOT/BOOT<arch>.EFI
> > >
> > > It obsoletes bootsmeth_efi.
> > >
> > > >
> > > >       return -EINVAL;
> > >
> > > We must always run the EFI boot manager if it is enabled. So -EINVAL is
> > > wrong here.
> >
> > Well, I don't believe you have a solution, then.
> >
> > You did suggest putting bootmgr later, as we discussed on irc.
> >
> > For now, I think we should apply this patch (and series), while we
> > sort out how to make bootmgr more incremental. It should not be
> > scanning every available device before it starts, since that can be
> > very slow.
>
> Heinrich and I talked the other day, and we think the right path is the
> "later" path, where we don't try and use efi bootmanager until
> everything has been probed, and also drop the single "efi" option. This
> should mean that by the time we would be trying efi bootmanager most if
> not everything that needs to be probed has been probed. It doesn't make
> any sense to have "efi bootmanger" be more incremental as conceptually
> it's point is to show the user all the options.

What is the single 'efi' option? I hope you don't mean the EFI bootmeth.

Putting bootmanager at the very end is OK with me, if we really cannot
figure out a way to know whether the system is using it or now. It is
'putting it in the middle' that I don't like.

How to handle the case where bootorder only specifies a subset of
options? Ideally we would just probe devices related to that subset.
That is what I was getting at.

Regards,
Simon

Re: [PATCH v5 4/8] RFC: Revert "bootstd: Make efi_mgr bootmeth work for non-sandbox setups"

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 5336 bytes --]

On Thu, Jan 09, 2025 at 08:11:58AM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Thu, 9 Jan 2025 at 08:05, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Jan 09, 2025 at 08:01:13AM -0700, Simon Glass wrote:
> > > Hi Heinrich,
> > >
> > > On Sat, 4 Jan 2025 at 19:50, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > > >
> > > > On 11/13/24 16:09, Simon Glass wrote:
> > > > > This is another option to fix sunxi booting with bootstd, which may be
> > > > > better since it will work for all boards. We can then figure out how to
> > > > > automatically and deterministicaly decide when bootmgr should be used.
> > > > >
> > > > > This reverts commit f2bfa0cb17948aa4a0fa20fdf9014296b9c4d9c7.
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > ---
> > > > > If this patch is applied, we don't need to drop bootmgr for sunxi
> > > > >
> > > > > (no changes since v1)
> > > > >
> > > > >   boot/bootmeth_efi_mgr.c | 18 +-----------------
> > > > >   1 file changed, 1 insertion(+), 17 deletions(-)avilable
> > > > >
> > > > > diff --git a/boot/bootmeth_efi_mgr.c b/boot/bootmeth_efi_mgr.c
> > > > > index 23ae1e610ac..095fa74fc60 100644
> > > > > --- a/boot/bootmeth_efi_mgr.c
> > > > > +++ b/boot/bootmeth_efi_mgr.c
> > > > > @@ -14,8 +14,6 @@
> > > > >   #include <command.h>
> > > > >   #include <dm.h>
> > > > >   #include <efi_loader.h>
> > > > > -#include <efi_variable.h>
> > > > > -#include <malloc.h>
> > > > >
> > > > >   /**
> > > > >    * struct efi_mgr_priv - private info for the efi-mgr driver
> > > > > @@ -48,27 +46,13 @@ static int efi_mgr_check(struct udevice *dev, struct bootflow_iter *iter)
> > > > >   static int efi_mgr_read_bootflow(struct udevice *dev, struct bootflow *bflow)
> > > > >   {
> > > > >       struct efi_mgr_priv *priv = dev_get_priv(dev);
> > > > > -     efi_status_t ret;
> > > > > -     efi_uintn_t size;
> > > > > -     u16 *bootorder;
> > > > >
> > > > >       if (priv->fake_dev) {
> > > > >               bflow->state = BOOTFLOWST_READY;
> > > > >               return 0;
> > > > >       }
> > > > >
> > > > > -     ret = efi_init_obj_list();
> > > > > -     if (ret)
> > > > > -             return log_msg_ret("init", ret);
> > > > > -
> > > > > -     /* Enable this method if the "BootOrder" UEFI exists. */
> > > > > -     bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid,
> > > > > -                             &size);
> > > > > -     if (bootorder) {
> > > > > -             free(bootorder);
> > > > > -             bflow->state = BOOTFLOWST_READY;
> > > > > -             return 0;
> > > > > -     }
> > > > > +     /* To be implemented */
> > > >
> > > > The EFI boot manager can boot based on:
> > > >
> > > > * variable BootOrder
> > > > * variable BootNext
> > > > * an existing file EFI/BOOT/BOOT<arch>.EFI
> > > >
> > > > It obsoletes bootsmeth_efi.
> > > >
> > > > >
> > > > >       return -EINVAL;
> > > >
> > > > We must always run the EFI boot manager if it is enabled. So -EINVAL is
> > > > wrong here.
> > >
> > > Well, I don't believe you have a solution, then.
> > >
> > > You did suggest putting bootmgr later, as we discussed on irc.
> > >
> > > For now, I think we should apply this patch (and series), while we
> > > sort out how to make bootmgr more incremental. It should not be
> > > scanning every available device before it starts, since that can be
> > > very slow.
> >
> > Heinrich and I talked the other day, and we think the right path is the
> > "later" path, where we don't try and use efi bootmanager until
> > everything has been probed, and also drop the single "efi" option. This
> > should mean that by the time we would be trying efi bootmanager most if
> > not everything that needs to be probed has been probed. It doesn't make
> > any sense to have "efi bootmanger" be more incremental as conceptually
> > it's point is to show the user all the options.
> 
> What is the single 'efi' option? I hope you don't mean the EFI bootmeth.

Yes, the single EFI bootmeth. Because part of the issue with that is to
be compliant a bunch of things need to be probed. And rather than have
an option to be only semi-compliant, the preference is to be compliant
and just tried later in the boot sequence.

> Putting bootmanager at the very end is OK with me, if we really cannot
> figure out a way to know whether the system is using it or now. It is
> 'putting it in the middle' that I don't like.

In the case of no specified boot order, after block (and after very
special things like FEL) and before network is what makes the most
sense. We don't need to try and DHCP/etc for it, just need to know if
the interface exists (and so the user can say / have configured, to try
and boot it).

> How to handle the case where bootorder only specifies a subset of
> options? Ideally we would just probe devices related to that subset.
> That is what I was getting at.

Yes, that should work just fine, especially if we're in the middle?

The big challenge here is that conceptually, "bootstd" and "efi
bootmanager" are duplicate functionality. They are both "figure out what
on the system we should boot". The compromise is to try "bootstd" on
block devices first (the common case).

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v5 4/8] RFC: Revert "bootstd: Make efi_mgr bootmeth work for non-sandbox setups"

[Simon Glass]

Hi Tom,

On Thu, 9 Jan 2025 at 08:22, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Jan 09, 2025 at 08:11:58AM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Thu, 9 Jan 2025 at 08:05, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Thu, Jan 09, 2025 at 08:01:13AM -0700, Simon Glass wrote:
> > > > Hi Heinrich,
> > > >
> > > > On Sat, 4 Jan 2025 at 19:50, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > > > >
> > > > > On 11/13/24 16:09, Simon Glass wrote:
> > > > > > This is another option to fix sunxi booting with bootstd, which may be
> > > > > > better since it will work for all boards. We can then figure out how to
> > > > > > automatically and deterministicaly decide when bootmgr should be used.
> > > > > >
> > > > > > This reverts commit f2bfa0cb17948aa4a0fa20fdf9014296b9c4d9c7.
> > > > > >
> > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > ---
> > > > > > If this patch is applied, we don't need to drop bootmgr for sunxi
> > > > > >
> > > > > > (no changes since v1)
> > > > > >
> > > > > >   boot/bootmeth_efi_mgr.c | 18 +-----------------
> > > > > >   1 file changed, 1 insertion(+), 17 deletions(-)avilable
> > > > > >
> > > > > > diff --git a/boot/bootmeth_efi_mgr.c b/boot/bootmeth_efi_mgr.c
> > > > > > index 23ae1e610ac..095fa74fc60 100644
> > > > > > --- a/boot/bootmeth_efi_mgr.c
> > > > > > +++ b/boot/bootmeth_efi_mgr.c
> > > > > > @@ -14,8 +14,6 @@
> > > > > >   #include <command.h>
> > > > > >   #include <dm.h>
> > > > > >   #include <efi_loader.h>
> > > > > > -#include <efi_variable.h>
> > > > > > -#include <malloc.h>
> > > > > >
> > > > > >   /**
> > > > > >    * struct efi_mgr_priv - private info for the efi-mgr driver
> > > > > > @@ -48,27 +46,13 @@ static int efi_mgr_check(struct udevice *dev, struct bootflow_iter *iter)
> > > > > >   static int efi_mgr_read_bootflow(struct udevice *dev, struct bootflow *bflow)
> > > > > >   {
> > > > > >       struct efi_mgr_priv *priv = dev_get_priv(dev);
> > > > > > -     efi_status_t ret;
> > > > > > -     efi_uintn_t size;
> > > > > > -     u16 *bootorder;
> > > > > >
> > > > > >       if (priv->fake_dev) {
> > > > > >               bflow->state = BOOTFLOWST_READY;
> > > > > >               return 0;
> > > > > >       }
> > > > > >
> > > > > > -     ret = efi_init_obj_list();
> > > > > > -     if (ret)
> > > > > > -             return log_msg_ret("init", ret);
> > > > > > -
> > > > > > -     /* Enable this method if the "BootOrder" UEFI exists. */
> > > > > > -     bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid,
> > > > > > -                             &size);
> > > > > > -     if (bootorder) {
> > > > > > -             free(bootorder);
> > > > > > -             bflow->state = BOOTFLOWST_READY;
> > > > > > -             return 0;
> > > > > > -     }
> > > > > > +     /* To be implemented */
> > > > >
> > > > > The EFI boot manager can boot based on:
> > > > >
> > > > > * variable BootOrder
> > > > > * variable BootNext
> > > > > * an existing file EFI/BOOT/BOOT<arch>.EFI
> > > > >
> > > > > It obsoletes bootsmeth_efi.
> > > > >
> > > > > >
> > > > > >       return -EINVAL;
> > > > >
> > > > > We must always run the EFI boot manager if it is enabled. So -EINVAL is
> > > > > wrong here.
> > > >
> > > > Well, I don't believe you have a solution, then.
> > > >
> > > > You did suggest putting bootmgr later, as we discussed on irc.
> > > >
> > > > For now, I think we should apply this patch (and series), while we
> > > > sort out how to make bootmgr more incremental. It should not be
> > > > scanning every available device before it starts, since that can be
> > > > very slow.
> > >
> > > Heinrich and I talked the other day, and we think the right path is the
> > > "later" path, where we don't try and use efi bootmanager until
> > > everything has been probed, and also drop the single "efi" option. This
> > > should mean that by the time we would be trying efi bootmanager most if
> > > not everything that needs to be probed has been probed. It doesn't make
> > > any sense to have "efi bootmanger" be more incremental as conceptually
> > > it's point is to show the user all the options.
> >
> > What is the single 'efi' option? I hope you don't mean the EFI bootmeth.
>
> Yes, the single EFI bootmeth. Because part of the issue with that is to
> be compliant a bunch of things need to be probed. And rather than have
> an option to be only semi-compliant, the preference is to be compliant
> and just tried later in the boot sequence.

Well then you need a way to disable the single EFI bootmeth?

>
> > Putting bootmanager at the very end is OK with me, if we really cannot
> > figure out a way to know whether the system is using it or now. It is
> > 'putting it in the middle' that I don't like.
>
> In the case of no specified boot order, after block (and after very
> special things like FEL) and before network is what makes the most
> sense. We don't need to try and DHCP/etc for it, just need to know if
> the interface exists (and so the user can say / have configured, to try
> and boot it).

Apart from FEL, we also have ChromeOS, Android, QFW.

>
> > How to handle the case where bootorder only specifies a subset of
> > options? Ideally we would just probe devices related to that subset.
> > That is what I was getting at.
>
> Yes, that should work just fine, especially if we're in the middle?
>
> The big challenge here is that conceptually, "bootstd" and "efi
> bootmanager" are duplicate functionality. They are both "figure out what
> on the system we should boot". The compromise is to try "bootstd" on
> block devices first (the common case).

Are we trying to make boot slower?

A better way to design this would be to integrate the EFI stuff with
bootstd, rather than duplicating things. For example, a way to convert
a BOOTxxxx device into a bootdev would permit faster booting, since we
can just work through the boot order and request each device in turn.

Once we give up on that and ask the user, we need to probe all devices.

The monolithic init of EFI_LOADER is creating quite a few problems. Am
I the only one that sees it?

Regards,
Simon

Re: [PATCH v5 4/8] RFC: Revert "bootstd: Make efi_mgr bootmeth work for non-sandbox setups"

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 7867 bytes --]

On Fri, Jan 10, 2025 at 06:41:00AM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Thu, 9 Jan 2025 at 08:22, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Jan 09, 2025 at 08:11:58AM -0700, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Thu, 9 Jan 2025 at 08:05, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Thu, Jan 09, 2025 at 08:01:13AM -0700, Simon Glass wrote:
> > > > > Hi Heinrich,
> > > > >
> > > > > On Sat, 4 Jan 2025 at 19:50, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > > > > >
> > > > > > On 11/13/24 16:09, Simon Glass wrote:
> > > > > > > This is another option to fix sunxi booting with bootstd, which may be
> > > > > > > better since it will work for all boards. We can then figure out how to
> > > > > > > automatically and deterministicaly decide when bootmgr should be used.
> > > > > > >
> > > > > > > This reverts commit f2bfa0cb17948aa4a0fa20fdf9014296b9c4d9c7.
> > > > > > >
> > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > ---
> > > > > > > If this patch is applied, we don't need to drop bootmgr for sunxi
> > > > > > >
> > > > > > > (no changes since v1)
> > > > > > >
> > > > > > >   boot/bootmeth_efi_mgr.c | 18 +-----------------
> > > > > > >   1 file changed, 1 insertion(+), 17 deletions(-)avilable
> > > > > > >
> > > > > > > diff --git a/boot/bootmeth_efi_mgr.c b/boot/bootmeth_efi_mgr.c
> > > > > > > index 23ae1e610ac..095fa74fc60 100644
> > > > > > > --- a/boot/bootmeth_efi_mgr.c
> > > > > > > +++ b/boot/bootmeth_efi_mgr.c
> > > > > > > @@ -14,8 +14,6 @@
> > > > > > >   #include <command.h>
> > > > > > >   #include <dm.h>
> > > > > > >   #include <efi_loader.h>
> > > > > > > -#include <efi_variable.h>
> > > > > > > -#include <malloc.h>
> > > > > > >
> > > > > > >   /**
> > > > > > >    * struct efi_mgr_priv - private info for the efi-mgr driver
> > > > > > > @@ -48,27 +46,13 @@ static int efi_mgr_check(struct udevice *dev, struct bootflow_iter *iter)
> > > > > > >   static int efi_mgr_read_bootflow(struct udevice *dev, struct bootflow *bflow)
> > > > > > >   {
> > > > > > >       struct efi_mgr_priv *priv = dev_get_priv(dev);
> > > > > > > -     efi_status_t ret;
> > > > > > > -     efi_uintn_t size;
> > > > > > > -     u16 *bootorder;
> > > > > > >
> > > > > > >       if (priv->fake_dev) {
> > > > > > >               bflow->state = BOOTFLOWST_READY;
> > > > > > >               return 0;
> > > > > > >       }
> > > > > > >
> > > > > > > -     ret = efi_init_obj_list();
> > > > > > > -     if (ret)
> > > > > > > -             return log_msg_ret("init", ret);
> > > > > > > -
> > > > > > > -     /* Enable this method if the "BootOrder" UEFI exists. */
> > > > > > > -     bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid,
> > > > > > > -                             &size);
> > > > > > > -     if (bootorder) {
> > > > > > > -             free(bootorder);
> > > > > > > -             bflow->state = BOOTFLOWST_READY;
> > > > > > > -             return 0;
> > > > > > > -     }
> > > > > > > +     /* To be implemented */
> > > > > >
> > > > > > The EFI boot manager can boot based on:
> > > > > >
> > > > > > * variable BootOrder
> > > > > > * variable BootNext
> > > > > > * an existing file EFI/BOOT/BOOT<arch>.EFI
> > > > > >
> > > > > > It obsoletes bootsmeth_efi.
> > > > > >
> > > > > > >
> > > > > > >       return -EINVAL;
> > > > > >
> > > > > > We must always run the EFI boot manager if it is enabled. So -EINVAL is
> > > > > > wrong here.
> > > > >
> > > > > Well, I don't believe you have a solution, then.
> > > > >
> > > > > You did suggest putting bootmgr later, as we discussed on irc.
> > > > >
> > > > > For now, I think we should apply this patch (and series), while we
> > > > > sort out how to make bootmgr more incremental. It should not be
> > > > > scanning every available device before it starts, since that can be
> > > > > very slow.
> > > >
> > > > Heinrich and I talked the other day, and we think the right path is the
> > > > "later" path, where we don't try and use efi bootmanager until
> > > > everything has been probed, and also drop the single "efi" option. This
> > > > should mean that by the time we would be trying efi bootmanager most if
> > > > not everything that needs to be probed has been probed. It doesn't make
> > > > any sense to have "efi bootmanger" be more incremental as conceptually
> > > > it's point is to show the user all the options.
> > >
> > > What is the single 'efi' option? I hope you don't mean the EFI bootmeth.
> >
> > Yes, the single EFI bootmeth. Because part of the issue with that is to
> > be compliant a bunch of things need to be probed. And rather than have
> > an option to be only semi-compliant, the preference is to be compliant
> > and just tried later in the boot sequence.
> 
> Well then you need a way to disable the single EFI bootmeth?

We have BOOTMETH_EFILOADER already.

> > > Putting bootmanager at the very end is OK with me, if we really cannot
> > > figure out a way to know whether the system is using it or now. It is
> > > 'putting it in the middle' that I don't like.
> >
> > In the case of no specified boot order, after block (and after very
> > special things like FEL) and before network is what makes the most
> > sense. We don't need to try and DHCP/etc for it, just need to know if
> > the interface exists (and so the user can say / have configured, to try
> > and boot it).
> 
> Apart from FEL, we also have ChromeOS, Android, QFW.

OK? Yes, bootstd needs to handle "we're in a special update things
state". I'm not sure if QFW counts as a faux-block device or not. And
ChromeOS I would have thought is "try and boot like $this from $device".
I don't know if by "Android" you mean booting the OS, or the AVB state
machine. The former would be like ChromeOS and the latter the special
state machine case.

> > > How to handle the case where bootorder only specifies a subset of
> > > options? Ideally we would just probe devices related to that subset.
> > > That is what I was getting at.
> >
> > Yes, that should work just fine, especially if we're in the middle?
> >
> > The big challenge here is that conceptually, "bootstd" and "efi
> > bootmanager" are duplicate functionality. They are both "figure out what
> > on the system we should boot". The compromise is to try "bootstd" on
> > block devices first (the common case).
> 
> Are we trying to make boot slower?

We're already in the slow path, because we're making guesses. But no, we
aren't trying to make anything slower. This should potentially faster in
the non-EFI case since we won't be trying to EFI boot every block device
until we have exhausted the non-EFI methods.

> A better way to design this would be to integrate the EFI stuff with
> bootstd, rather than duplicating things. For example, a way to convert
> a BOOTxxxx device into a bootdev would permit faster booting, since we
> can just work through the boot order and request each device in turn.
> 
> Once we give up on that and ask the user, we need to probe all devices.

No, we're trying to untangle "EFI", "EFI bootmanager" and "bootstd".

> The monolithic init of EFI_LOADER is creating quite a few problems. Am
> I the only one that sees it?

Yes, because your "monolithic init is a problem" is the subsystem
maintainers "monolithic init makes us spec compliant".

And when we're in some sort of "probe the device and see what can be
booted" the subsystem maintainers would rather be spec compliant.

And for the role I played in this confusion by suggesting how to
integrate things privately and has now gone off the rails, I am sorry
for the confusion and wasted efforts. I don't know if I was unclear or
misspoke.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v5 4/8] RFC: Revert "bootstd: Make efi_mgr bootmeth work for non-sandbox setups"

[Simon Glass]

Hi Tom,

On Fri, 10 Jan 2025 at 10:05, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Jan 10, 2025 at 06:41:00AM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Thu, 9 Jan 2025 at 08:22, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Thu, Jan 09, 2025 at 08:11:58AM -0700, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Thu, 9 Jan 2025 at 08:05, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Thu, Jan 09, 2025 at 08:01:13AM -0700, Simon Glass wrote:
> > > > > > Hi Heinrich,
> > > > > >
> > > > > > On Sat, 4 Jan 2025 at 19:50, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > > > > > >
> > > > > > > On 11/13/24 16:09, Simon Glass wrote:
> > > > > > > > This is another option to fix sunxi booting with bootstd, which may be
> > > > > > > > better since it will work for all boards. We can then figure out how to
> > > > > > > > automatically and deterministicaly decide when bootmgr should be used.
> > > > > > > >
> > > > > > > > This reverts commit f2bfa0cb17948aa4a0fa20fdf9014296b9c4d9c7.
> > > > > > > >
> > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > > ---
> > > > > > > > If this patch is applied, we don't need to drop bootmgr for sunxi
> > > > > > > >
> > > > > > > > (no changes since v1)
> > > > > > > >
> > > > > > > >   boot/bootmeth_efi_mgr.c | 18 +-----------------
> > > > > > > >   1 file changed, 1 insertion(+), 17 deletions(-)avilable
> > > > > > > >
> > > > > > > > diff --git a/boot/bootmeth_efi_mgr.c b/boot/bootmeth_efi_mgr.c
> > > > > > > > index 23ae1e610ac..095fa74fc60 100644
> > > > > > > > --- a/boot/bootmeth_efi_mgr.c
> > > > > > > > +++ b/boot/bootmeth_efi_mgr.c
> > > > > > > > @@ -14,8 +14,6 @@
> > > > > > > >   #include <command.h>
> > > > > > > >   #include <dm.h>
> > > > > > > >   #include <efi_loader.h>
> > > > > > > > -#include <efi_variable.h>
> > > > > > > > -#include <malloc.h>
> > > > > > > >
> > > > > > > >   /**
> > > > > > > >    * struct efi_mgr_priv - private info for the efi-mgr driver
> > > > > > > > @@ -48,27 +46,13 @@ static int efi_mgr_check(struct udevice *dev, struct bootflow_iter *iter)
> > > > > > > >   static int efi_mgr_read_bootflow(struct udevice *dev, struct bootflow *bflow)
> > > > > > > >   {
> > > > > > > >       struct efi_mgr_priv *priv = dev_get_priv(dev);
> > > > > > > > -     efi_status_t ret;
> > > > > > > > -     efi_uintn_t size;
> > > > > > > > -     u16 *bootorder;
> > > > > > > >
> > > > > > > >       if (priv->fake_dev) {
> > > > > > > >               bflow->state = BOOTFLOWST_READY;
> > > > > > > >               return 0;
> > > > > > > >       }
> > > > > > > >
> > > > > > > > -     ret = efi_init_obj_list();
> > > > > > > > -     if (ret)
> > > > > > > > -             return log_msg_ret("init", ret);
> > > > > > > > -
> > > > > > > > -     /* Enable this method if the "BootOrder" UEFI exists. */
> > > > > > > > -     bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid,
> > > > > > > > -                             &size);
> > > > > > > > -     if (bootorder) {
> > > > > > > > -             free(bootorder);
> > > > > > > > -             bflow->state = BOOTFLOWST_READY;
> > > > > > > > -             return 0;
> > > > > > > > -     }
> > > > > > > > +     /* To be implemented */
> > > > > > >
> > > > > > > The EFI boot manager can boot based on:
> > > > > > >
> > > > > > > * variable BootOrder
> > > > > > > * variable BootNext
> > > > > > > * an existing file EFI/BOOT/BOOT<arch>.EFI
> > > > > > >
> > > > > > > It obsoletes bootsmeth_efi.
> > > > > > >
> > > > > > > >
> > > > > > > >       return -EINVAL;
> > > > > > >
> > > > > > > We must always run the EFI boot manager if it is enabled. So -EINVAL is
> > > > > > > wrong here.
> > > > > >
> > > > > > Well, I don't believe you have a solution, then.
> > > > > >
> > > > > > You did suggest putting bootmgr later, as we discussed on irc.
> > > > > >
> > > > > > For now, I think we should apply this patch (and series), while we
> > > > > > sort out how to make bootmgr more incremental. It should not be
> > > > > > scanning every available device before it starts, since that can be
> > > > > > very slow.
> > > > >
> > > > > Heinrich and I talked the other day, and we think the right path is the
> > > > > "later" path, where we don't try and use efi bootmanager until
> > > > > everything has been probed, and also drop the single "efi" option. This
> > > > > should mean that by the time we would be trying efi bootmanager most if
> > > > > not everything that needs to be probed has been probed. It doesn't make
> > > > > any sense to have "efi bootmanger" be more incremental as conceptually
> > > > > it's point is to show the user all the options.
> > > >
> > > > What is the single 'efi' option? I hope you don't mean the EFI bootmeth.
> > >
> > > Yes, the single EFI bootmeth. Because part of the issue with that is to
> > > be compliant a bunch of things need to be probed. And rather than have
> > > an option to be only semi-compliant, the preference is to be compliant
> > > and just tried later in the boot sequence.
> >
> > Well then you need a way to disable the single EFI bootmeth?
>
> We have BOOTMETH_EFILOADER already.
>
> > > > Putting bootmanager at the very end is OK with me, if we really cannot
> > > > figure out a way to know whether the system is using it or now. It is
> > > > 'putting it in the middle' that I don't like.
> > >
> > > In the case of no specified boot order, after block (and after very
> > > special things like FEL) and before network is what makes the most
> > > sense. We don't need to try and DHCP/etc for it, just need to know if
> > > the interface exists (and so the user can say / have configured, to try
> > > and boot it).
> >
> > Apart from FEL, we also have ChromeOS, Android, QFW.
>
> OK? Yes, bootstd needs to handle "we're in a special update things
> state". I'm not sure if QFW counts as a faux-block device or not. And
> ChromeOS I would have thought is "try and boot like $this from $device".
> I don't know if by "Android" you mean booting the OS, or the AVB state
> machine. The former would be like ChromeOS and the latter the special
> state machine case.

Standard boot is designed as a generalisation of the boot process and
is able to cope with most use cases. Everything uses the same
algorithm, at a high level.

>
> > > > How to handle the case where bootorder only specifies a subset of
> > > > options? Ideally we would just probe devices related to that subset.
> > > > That is what I was getting at.
> > >
> > > Yes, that should work just fine, especially if we're in the middle?
> > >
> > > The big challenge here is that conceptually, "bootstd" and "efi
> > > bootmanager" are duplicate functionality. They are both "figure out what
> > > on the system we should boot". The compromise is to try "bootstd" on
> > > block devices first (the common case).
> >
> > Are we trying to make boot slower?
>
> We're already in the slow path, because we're making guesses. But no, we
> aren't trying to make anything slower. This should potentially faster in
> the non-EFI case since we won't be trying to EFI boot every block device
> until we have exhausted the non-EFI methods.
>
> > A better way to design this would be to integrate the EFI stuff with
> > bootstd, rather than duplicating things. For example, a way to convert
> > a BOOTxxxx device into a bootdev would permit faster booting, since we
> > can just work through the boot order and request each device in turn.
> >
> > Once we give up on that and ask the user, we need to probe all devices.
>
> No, we're trying to untangle "EFI", "EFI bootmanager" and "bootstd".

By neutering bootstd? It would be better to update EFI to make use of
bootdev, for example. Heinrich did a patch to use the 'hunt' feature
of bootdev.

>
> > The monolithic init of EFI_LOADER is creating quite a few problems. Am
> > I the only one that sees it?
>
> Yes, because your "monolithic init is a problem" is the subsystem
> maintainers "monolithic init makes us spec compliant".
>
> And when we're in some sort of "probe the device and see what can be
> booted" the subsystem maintainers would rather be spec compliant.

We don't need to be noncompliant with spec. But some thoughtful design
would help us a lot.

>
> And for the role I played in this confusion by suggesting how to
> integrate things privately and has now gone off the rails, I am sorry
> for the confusion and wasted efforts. I don't know if I was unclear or
> misspoke.

I'm not sure what that relates to?

Regards,
Simon

Re: [PATCH v5 4/8] RFC: Revert "bootstd: Make efi_mgr bootmeth work for non-sandbox setups"

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 10181 bytes --]

On Wed, Jan 15, 2025 at 06:22:40AM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Fri, 10 Jan 2025 at 10:05, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Fri, Jan 10, 2025 at 06:41:00AM -0700, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Thu, 9 Jan 2025 at 08:22, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Thu, Jan 09, 2025 at 08:11:58AM -0700, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Thu, 9 Jan 2025 at 08:05, Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Thu, Jan 09, 2025 at 08:01:13AM -0700, Simon Glass wrote:
> > > > > > > Hi Heinrich,
> > > > > > >
> > > > > > > On Sat, 4 Jan 2025 at 19:50, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > > > > > > >
> > > > > > > > On 11/13/24 16:09, Simon Glass wrote:
> > > > > > > > > This is another option to fix sunxi booting with bootstd, which may be
> > > > > > > > > better since it will work for all boards. We can then figure out how to
> > > > > > > > > automatically and deterministicaly decide when bootmgr should be used.
> > > > > > > > >
> > > > > > > > > This reverts commit f2bfa0cb17948aa4a0fa20fdf9014296b9c4d9c7.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > > > ---
> > > > > > > > > If this patch is applied, we don't need to drop bootmgr for sunxi
> > > > > > > > >
> > > > > > > > > (no changes since v1)
> > > > > > > > >
> > > > > > > > >   boot/bootmeth_efi_mgr.c | 18 +-----------------
> > > > > > > > >   1 file changed, 1 insertion(+), 17 deletions(-)avilable
> > > > > > > > >
> > > > > > > > > diff --git a/boot/bootmeth_efi_mgr.c b/boot/bootmeth_efi_mgr.c
> > > > > > > > > index 23ae1e610ac..095fa74fc60 100644
> > > > > > > > > --- a/boot/bootmeth_efi_mgr.c
> > > > > > > > > +++ b/boot/bootmeth_efi_mgr.c
> > > > > > > > > @@ -14,8 +14,6 @@
> > > > > > > > >   #include <command.h>
> > > > > > > > >   #include <dm.h>
> > > > > > > > >   #include <efi_loader.h>
> > > > > > > > > -#include <efi_variable.h>
> > > > > > > > > -#include <malloc.h>
> > > > > > > > >
> > > > > > > > >   /**
> > > > > > > > >    * struct efi_mgr_priv - private info for the efi-mgr driver
> > > > > > > > > @@ -48,27 +46,13 @@ static int efi_mgr_check(struct udevice *dev, struct bootflow_iter *iter)
> > > > > > > > >   static int efi_mgr_read_bootflow(struct udevice *dev, struct bootflow *bflow)
> > > > > > > > >   {
> > > > > > > > >       struct efi_mgr_priv *priv = dev_get_priv(dev);
> > > > > > > > > -     efi_status_t ret;
> > > > > > > > > -     efi_uintn_t size;
> > > > > > > > > -     u16 *bootorder;
> > > > > > > > >
> > > > > > > > >       if (priv->fake_dev) {
> > > > > > > > >               bflow->state = BOOTFLOWST_READY;
> > > > > > > > >               return 0;
> > > > > > > > >       }
> > > > > > > > >
> > > > > > > > > -     ret = efi_init_obj_list();
> > > > > > > > > -     if (ret)
> > > > > > > > > -             return log_msg_ret("init", ret);
> > > > > > > > > -
> > > > > > > > > -     /* Enable this method if the "BootOrder" UEFI exists. */
> > > > > > > > > -     bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid,
> > > > > > > > > -                             &size);
> > > > > > > > > -     if (bootorder) {
> > > > > > > > > -             free(bootorder);
> > > > > > > > > -             bflow->state = BOOTFLOWST_READY;
> > > > > > > > > -             return 0;
> > > > > > > > > -     }
> > > > > > > > > +     /* To be implemented */
> > > > > > > >
> > > > > > > > The EFI boot manager can boot based on:
> > > > > > > >
> > > > > > > > * variable BootOrder
> > > > > > > > * variable BootNext
> > > > > > > > * an existing file EFI/BOOT/BOOT<arch>.EFI
> > > > > > > >
> > > > > > > > It obsoletes bootsmeth_efi.
> > > > > > > >
> > > > > > > > >
> > > > > > > > >       return -EINVAL;
> > > > > > > >
> > > > > > > > We must always run the EFI boot manager if it is enabled. So -EINVAL is
> > > > > > > > wrong here.
> > > > > > >
> > > > > > > Well, I don't believe you have a solution, then.
> > > > > > >
> > > > > > > You did suggest putting bootmgr later, as we discussed on irc.
> > > > > > >
> > > > > > > For now, I think we should apply this patch (and series), while we
> > > > > > > sort out how to make bootmgr more incremental. It should not be
> > > > > > > scanning every available device before it starts, since that can be
> > > > > > > very slow.
> > > > > >
> > > > > > Heinrich and I talked the other day, and we think the right path is the
> > > > > > "later" path, where we don't try and use efi bootmanager until
> > > > > > everything has been probed, and also drop the single "efi" option. This
> > > > > > should mean that by the time we would be trying efi bootmanager most if
> > > > > > not everything that needs to be probed has been probed. It doesn't make
> > > > > > any sense to have "efi bootmanger" be more incremental as conceptually
> > > > > > it's point is to show the user all the options.
> > > > >
> > > > > What is the single 'efi' option? I hope you don't mean the EFI bootmeth.
> > > >
> > > > Yes, the single EFI bootmeth. Because part of the issue with that is to
> > > > be compliant a bunch of things need to be probed. And rather than have
> > > > an option to be only semi-compliant, the preference is to be compliant
> > > > and just tried later in the boot sequence.
> > >
> > > Well then you need a way to disable the single EFI bootmeth?
> >
> > We have BOOTMETH_EFILOADER already.
> >
> > > > > Putting bootmanager at the very end is OK with me, if we really cannot
> > > > > figure out a way to know whether the system is using it or now. It is
> > > > > 'putting it in the middle' that I don't like.
> > > >
> > > > In the case of no specified boot order, after block (and after very
> > > > special things like FEL) and before network is what makes the most
> > > > sense. We don't need to try and DHCP/etc for it, just need to know if
> > > > the interface exists (and so the user can say / have configured, to try
> > > > and boot it).
> > >
> > > Apart from FEL, we also have ChromeOS, Android, QFW.
> >
> > OK? Yes, bootstd needs to handle "we're in a special update things
> > state". I'm not sure if QFW counts as a faux-block device or not. And
> > ChromeOS I would have thought is "try and boot like $this from $device".
> > I don't know if by "Android" you mean booting the OS, or the AVB state
> > machine. The former would be like ChromeOS and the latter the special
> > state machine case.
> 
> Standard boot is designed as a generalisation of the boot process and
> is able to cope with most use cases. Everything uses the same
> algorithm, at a high level.

That will be tricky as some of those use cases have their own required
order of operations.

> > > > > How to handle the case where bootorder only specifies a subset of
> > > > > options? Ideally we would just probe devices related to that subset.
> > > > > That is what I was getting at.
> > > >
> > > > Yes, that should work just fine, especially if we're in the middle?
> > > >
> > > > The big challenge here is that conceptually, "bootstd" and "efi
> > > > bootmanager" are duplicate functionality. They are both "figure out what
> > > > on the system we should boot". The compromise is to try "bootstd" on
> > > > block devices first (the common case).
> > >
> > > Are we trying to make boot slower?
> >
> > We're already in the slow path, because we're making guesses. But no, we
> > aren't trying to make anything slower. This should potentially faster in
> > the non-EFI case since we won't be trying to EFI boot every block device
> > until we have exhausted the non-EFI methods.
> >
> > > A better way to design this would be to integrate the EFI stuff with
> > > bootstd, rather than duplicating things. For example, a way to convert
> > > a BOOTxxxx device into a bootdev would permit faster booting, since we
> > > can just work through the boot order and request each device in turn.
> > >
> > > Once we give up on that and ask the user, we need to probe all devices.
> >
> > No, we're trying to untangle "EFI", "EFI bootmanager" and "bootstd".
> 
> By neutering bootstd?

No, by taking one generalization of the boot process algorithm out of
another generalization of the boot process algorithm.

> It would be better to update EFI to make use of
> bootdev, for example. Heinrich did a patch to use the 'hunt' feature
> of bootdev.

There's "EFI" and "EFI bootmanager" and I'm not sure which one you're
referring to here. But yes, Heinrich has had suggestions on how to work
with bootstd, including suggesting what I'm also seeing as the best path
forward I believe.

> > > The monolithic init of EFI_LOADER is creating quite a few problems. Am
> > > I the only one that sees it?
> >
> > Yes, because your "monolithic init is a problem" is the subsystem
> > maintainers "monolithic init makes us spec compliant".
> >
> > And when we're in some sort of "probe the device and see what can be
> > booted" the subsystem maintainers would rather be spec compliant.
> 
> We don't need to be noncompliant with spec. But some thoughtful design
> would help us a lot.

Maybe. But that's also not the part your problems with the EFI_LOADER
subsystem that you've been talking about in this thread. This thread is
about "can we probe less?" to which the answer seems to be "only if we
stop being spec compliant". Or if strictly speaking still spec compliant
but breaking the rule of least surprise and so forth.

> > And for the role I played in this confusion by suggesting how to
> > integrate things privately and has now gone off the rails, I am sorry
> > for the confusion and wasted efforts. I don't know if I was unclear or
> > misspoke.
> 
> I'm not sure what that relates to?

I know at some points I had given you some advice on how to integrate
EFI in to bootstd. And since that seems to have gone in the wrong
direction I'm acknowledging that and trying to reset things.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v5 7/8] env: Provide a work-around for unquoting fdtfile

[Tom Rini]

On Wed, 13 Nov 2024 08:09:37 -0700, Simon Glass wrote:

> Some boards use a CONFIG option to specify the value of this variable.
> This is normally handled by efi_get_distro_fdt_name() but in the case
> of sunxi this does not work, since 'soc' is sunxi, but the files are
> in the allwinner directory.
> 
> Provide a work-around for this particular case.
> 
> [...]

Applied to u-boot/next, thanks!

[7/8] env: Provide a work-around for unquoting fdtfile
      commit: d085e692c98d0d7b57cc577ed9befda159cd4a40
-- 
Tom