[PATCH v3] bootstd: Replicate the dtb-filename quirks of distroboot

[PATCH v3] bootstd: Replicate the dtb-filename quirks of distroboot

[Simon Glass]

For EFI, the distro boot scripts search in three different directories
for the .dtb file. The SOC-based filename fallback is supported only for
32-bit ARM.

Adjust the code to mirror this behaviour.

Also some boards can use a prior-stage FDT if one is not found in the
normal way. Support this and show a warning in that case.

Signed-off-by: Simon Glass <sjg@chromium.org>
Suggested-by: Mark Kettenis <kettenis@openbsd.org>
---

Changes in v3:
- Change the message from a warning to a statement

Changes in v2:
- Allow use of the prior-stage FDT if nothing else is found
- Warn about using a prior-stage FDT

 boot/bootflow.c     |  3 ++
 boot/bootmeth_efi.c | 70 +++++++++++++++++++++++++++++++++++++++------
 include/bootflow.h  | 14 +++++++++
 3 files changed, 78 insertions(+), 9 deletions(-)

diff --git a/boot/bootflow.c b/boot/bootflow.c
index 4999018e36e..0d809728716 100644
--- a/boot/bootflow.c
+++ b/boot/bootflow.c
@@ -463,6 +463,9 @@ int bootflow_run_boot(struct bootflow_iter *iter, struct bootflow *bflow)
 
 	printf("** Booting bootflow '%s' with %s\n", bflow->name,
 	       bflow->method->name);
+	if (IS_ENABLED(CONFIG_OF_HAS_PRIOR_STAGE) &&
+	    (bflow->flags & BOOTFLOWF_USE_PRIOR_FDT))
+		printf("Using prior-stage device tree\n");
 	ret = bootflow_boot(bflow);
 	if (!IS_ENABLED(CONFIG_BOOTSTD_FULL)) {
 		printf("Boot failed (err=%d)\n", ret);
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
index 67c972e3fe4..6a97ac02ff5 100644
--- a/boot/bootmeth_efi.c
+++ b/boot/bootmeth_efi.c
@@ -147,25 +147,60 @@ static int distro_efi_check(struct udevice *dev, struct bootflow_iter *iter)
 	return 0;
 }
 
-static void distro_efi_get_fdt_name(char *fname, int size)
+/**
+ * distro_efi_get_fdt_name() - Get the filename for reading the .dtb file
+ *
+ * @fname: Place to put filename
+ * @size: Max size of filename
+ * @seq: Sequence number, to cycle through options (0=first)
+ * Returns: 0 on success, -ENOENT if the "fdtfile" env var does not exist,
+ * -EINVAL if there are no more options, -EALREADY if the control FDT should be
+ * used
+ */
+static int distro_efi_get_fdt_name(char *fname, int size, int seq)
 {
 	const char *fdt_fname;
+	const char *prefix;
+
+	/* select the prefix */
+	switch (seq) {
+	case 0:
+		/* this is the default */
+		prefix = "/dtb";
+		break;
+	case 1:
+		prefix = "";
+		break;
+	case 2:
+		prefix = "/dtb/current";
+		break;
+	default:
+		return log_msg_ret("pref", -EINVAL);
+	}
 
 	fdt_fname = env_get("fdtfile");
 	if (fdt_fname) {
-		snprintf(fname, size, "dtb/%s", fdt_fname);
+		snprintf(fname, size, "%s/%s", prefix, fdt_fname);
 		log_debug("Using device tree: %s\n", fname);
-	} else {
+	} else if (IS_ENABLED(CONFIG_OF_HAS_PRIOR_STAGE)) {
+		strcpy(fname, "<prior>");
+		return log_msg_ret("pref", -EALREADY);
+	/* Use this fallback only for 32-bit ARM */
+	} else if (IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64)) {
 		const char *soc = env_get("soc");
 		const char *board = env_get("board");
 		const char *boardver = env_get("boardver");
 
 		/* cf the code in label_boot() which seems very complex */
-		snprintf(fname, size, "dtb/%s%s%s%s.dtb",
+		snprintf(fname, size, "%s/%s%s%s%s.dtb", prefix,
 			 soc ? soc : "", soc ? "-" : "", board ? board : "",
 			 boardver ? boardver : "");
 		log_debug("Using default device tree: %s\n", fname);
+	} else {
+		return log_msg_ret("env", -ENOENT);
 	}
+
+	return 0;
 }
 
 static int distro_efi_read_bootflow_file(struct udevice *dev,
@@ -174,7 +209,7 @@ static int distro_efi_read_bootflow_file(struct udevice *dev,
 	struct blk_desc *desc = NULL;
 	ulong fdt_addr, size;
 	char fname[256];
-	int ret;
+	int ret, seq;
 
 	/* We require a partition table */
 	if (!bflow->part)
@@ -196,13 +231,26 @@ static int distro_efi_read_bootflow_file(struct udevice *dev,
 	if (ret)
 		return log_msg_ret("read", -EINVAL);
 
-	distro_efi_get_fdt_name(fname, sizeof(fname));
+	fdt_addr = env_get_hex("fdt_addr_r", 0);
+
+	/* try the various available names */
+	ret = -ENOENT;
+	for (seq = 0; ret; seq++) {
+		ret = distro_efi_get_fdt_name(fname, sizeof(fname), seq);
+		if (ret == -EALREADY) {
+			bflow->flags = BOOTFLOWF_USE_PRIOR_FDT;
+			break;
+		}
+		if (ret)
+			return log_msg_ret("nam", ret);
+		ret = bootmeth_common_read_file(dev, bflow, fname, fdt_addr,
+						&size);
+	}
+
 	bflow->fdt_fname = strdup(fname);
 	if (!bflow->fdt_fname)
 		return log_msg_ret("fil", -ENOMEM);
 
-	fdt_addr = env_get_hex("fdt_addr_r", 0);
-	ret = bootmeth_common_read_file(dev, bflow, fname, fdt_addr, &size);
 	if (!ret) {
 		bflow->fdt_size = size;
 		bflow->fdt_addr = fdt_addr;
@@ -277,7 +325,11 @@ static int distro_efi_read_bootflow_net(struct bootflow *bflow)
 	fdt_addr = hextoul(fdt_addr_str, NULL);
 	sprintf(file_addr, "%lx", fdt_addr);
 
-	distro_efi_get_fdt_name(fname, sizeof(fname));
+	/* We only allow the first prefix with PXE */
+	ret = distro_efi_get_fdt_name(fname, sizeof(fname), 0);
+	if (ret)
+		return log_msg_ret("nam", ret);
+
 	bflow->fdt_fname = strdup(fname);
 	if (!bflow->fdt_fname)
 		return log_msg_ret("fil", -ENOMEM);
diff --git a/include/bootflow.h b/include/bootflow.h
index e5fdf5f29d1..f20f575030f 100644
--- a/include/bootflow.h
+++ b/include/bootflow.h
@@ -36,6 +36,18 @@ enum bootflow_state_t {
 	BOOTFLOWST_COUNT
 };
 
+/**
+ * enum bootflow_flags_t - flags for bootflows
+ *
+ * @BOOTFLOWF_USE_PRIOR_FDT: Indicates that an FDT was not found by the bootmeth
+ *	and it is using the prior-stage FDT, which is the U-Boot control FDT.
+ *	This is only possible with the EFI bootmeth (distro-efi) and only when
+ *	CONFIG_OF_HAS_PRIOR_STAGE is enabled
+ */
+enum bootflow_flags_t {
+	BOOTFLOWF_USE_PRIOR_FDT	= 1 << 0,
+};
+
 /**
  * struct bootflow - information about a bootflow
  *
@@ -68,6 +80,7 @@ enum bootflow_state_t {
  * @fdt_fname: Filename of FDT file
  * @fdt_size: Size of FDT file
  * @fdt_addr: Address of loaded fdt
+ * @flags: Flags for the bootflow (see enum bootflow_flags_t)
  */
 struct bootflow {
 	struct list_head bm_node;
@@ -90,6 +103,7 @@ struct bootflow {
 	char *fdt_fname;
 	int fdt_size;
 	ulong fdt_addr;
+	int flags;
 };
 
 /**
-- 
2.39.1.519.gcb327c4b5f-goog

Re: [PATCH v3] bootstd: Replicate the dtb-filename quirks of distroboot

[Tom Rini]

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

On Sun, Feb 05, 2023 at 01:24:38PM -0700, Simon Glass wrote:

> For EFI, the distro boot scripts search in three different directories
> for the .dtb file. The SOC-based filename fallback is supported only for
> 32-bit ARM.
> 
> Adjust the code to mirror this behaviour.
> 
> Also some boards can use a prior-stage FDT if one is not found in the
> normal way. Support this and show a warning in that case.

The code now does _not_ warn, thanks, but the commit message is wrong.
Will fixup when applying if there's no further feedback.

-- 
Tom

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

Re: [PATCH v3] bootstd: Replicate the dtb-filename quirks of distroboot

[Tom Rini]

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

On Sun, Feb 05, 2023 at 01:24:38PM -0700, Simon Glass wrote:

> For EFI, the distro boot scripts search in three different directories
> for the .dtb file. The SOC-based filename fallback is supported only for
> 32-bit ARM.
> 
> Adjust the code to mirror this behaviour.
> 
> Also some boards can use a prior-stage FDT if one is not found in the
> normal way. Support this and show a warning in that case.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Suggested-by: Mark Kettenis <kettenis@openbsd.org>

As-is, sandbox fails to build with this, on master.

-- 
Tom

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

Re: [PATCH v3] bootstd: Replicate the dtb-filename quirks of distroboot

[Simon Glass]

Hi Tom,

On Fri, 17 Feb 2023 at 08:51, Tom Rini <trini@konsulko.com> wrote:
>
> On Sun, Feb 05, 2023 at 01:24:38PM -0700, Simon Glass wrote:
>
> > For EFI, the distro boot scripts search in three different directories
> > for the .dtb file. The SOC-based filename fallback is supported only for
> > 32-bit ARM.
> >
> > Adjust the code to mirror this behaviour.
> >
> > Also some boards can use a prior-stage FDT if one is not found in the
> > normal way. Support this and show a warning in that case.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > Suggested-by: Mark Kettenis <kettenis@openbsd.org>
>
> As-is, sandbox fails to build with this, on master.

I can't repeat this, but I updated the series with the commit-message
change and will resend. The CI result is here:

https://source.denx.de/u-boot/custodians/u-boot-dm/-/pipelines/15309

Regards,
Simon

>
> --
> Tom