[PATCH 0/5] pxe: Support an automatic localboot

[PATCH 0/5] pxe: Support an automatic localboot

[Simon Glass]

It seems that extlinux expects that boards know how to boot a local OS
from attached media. U-Boot currently assumes that boards define a
"localcmd" environment variable to support this. None does.

Provide an automatic means to find a kernel and ramdisk, using common
filenames. This addresses booting on MAAS, at least.


Simon Glass (5):
  test/py: Refactor extlinux-image creation into a function
  test: bootflow: Avoid a confusing error condition
  pxe_utils: Allow the FDT to be missing
  pxe_utils: Support a backup for localboot
  pxe_utils: Support the SAY command

 arch/sandbox/dts/test.dts |  8 ++++
 boot/Kconfig              |  9 +++++
 boot/pxe_utils.c          | 50 ++++++++++++++++++++---
 test/boot/bootflow.c      | 56 +++++++++++++++++++++++++-
 test/py/tests/test_ut.py  | 85 ++++++++++++++++++++++++++-------------
 5 files changed, 173 insertions(+), 35 deletions(-)

-- 
2.34.1

[PATCH 1/5] test/py: Refactor extlinux-image creation into a function

[Simon Glass]

We would like to make another image which is very similar, so create a
function to produce an extlinux image.

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

 test/py/tests/test_ut.py | 67 +++++++++++++++++++++++-----------------
 1 file changed, 38 insertions(+), 29 deletions(-)

diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py
index cacf11f7c0a..e3b988efe23 100644
--- a/test/py/tests/test_ut.py
+++ b/test/py/tests/test_ut.py
@@ -57,6 +57,43 @@ def setup_image(cons, devnum, part_type, img_size=20, second_part=False,
                              stdin=spec.encode('utf-8'))
     return fname, mnt
 
+def setup_extlinux_image(cons, mmc_dev, vmlinux, initrd, dtbdir, script):
+    """Create a 20MB disk image with a single FAT partition"""
+    fname, mnt = setup_image(cons, mmc_dev, 0xc, second_part=True)
+
+    ext = os.path.join(mnt, 'extlinux')
+    mkdir_cond(ext)
+
+    conf = os.path.join(ext, 'extlinux.conf')
+    with open(conf, 'w', encoding='ascii') as fd:
+        print(script, file=fd)
+
+    inf = os.path.join(cons.config.persistent_data_dir, 'inf')
+    with open(inf, 'wb') as fd:
+        fd.write(gzip.compress(b'vmlinux'))
+    mkimage = cons.config.build_dir + '/tools/mkimage'
+    u_boot_utils.run_and_log(
+        cons, f'{mkimage} -f auto -d {inf} {os.path.join(mnt, vmlinux)}')
+
+    with open(os.path.join(mnt, initrd), 'w', encoding='ascii') as fd:
+        print('initrd', file=fd)
+
+    if dtbdir:
+        mkdir_cond(os.path.join(mnt, dtbdir))
+
+        dtb_file = os.path.join(mnt, f'{dtbdir}/sandbox.dtb')
+        u_boot_utils.run_and_log(
+            cons, f'dtc -o {dtb_file}', stdin=b'/dts-v1/; / {};')
+
+    fsfile = 'vfat18M.img'
+    u_boot_utils.run_and_log(cons, f'fallocate -l 18M {fsfile}')
+    u_boot_utils.run_and_log(cons, f'mkfs.vfat {fsfile}')
+    u_boot_utils.run_and_log(cons, ['sh', '-c', f'mcopy -i {fsfile} {mnt}/* ::/'])
+    u_boot_utils.run_and_log(cons, f'dd if={fsfile} of={fname} bs=1M seek=1')
+    u_boot_utils.run_and_log(cons, f'rm -rf {mnt}')
+    u_boot_utils.run_and_log(cons, f'rm -f {fsfile}')
+
+
 def setup_bootmenu_image(cons):
     """Create a 20MB disk image with a single ext4 partition
 
@@ -197,36 +234,8 @@ label Fedora-Workstation-armhfp-31-1.9 (5.3.7-301.fc31.armv7hl)
         append ro root=UUID=9732b35b-4cd5-458b-9b91-80f7047e0b8a rhgb quiet LANG=en_US.UTF-8 cma=192MB cma=256MB
         fdtdir /%s/
         initrd /%s''' % (vmlinux, dtbdir, initrd)
-    ext = os.path.join(mnt, 'extlinux')
-    mkdir_cond(ext)
 
-    conf = os.path.join(ext, 'extlinux.conf')
-    with open(conf, 'w', encoding='ascii') as fd:
-        print(script, file=fd)
-
-    inf = os.path.join(cons.config.persistent_data_dir, 'inf')
-    with open(inf, 'wb') as fd:
-        fd.write(gzip.compress(b'vmlinux'))
-    mkimage = cons.config.build_dir + '/tools/mkimage'
-    u_boot_utils.run_and_log(
-        cons, f'{mkimage} -f auto -d {inf} {os.path.join(mnt, vmlinux)}')
-
-    with open(os.path.join(mnt, initrd), 'w', encoding='ascii') as fd:
-        print('initrd', file=fd)
-
-    mkdir_cond(os.path.join(mnt, dtbdir))
-
-    dtb_file = os.path.join(mnt, f'{dtbdir}/sandbox.dtb')
-    u_boot_utils.run_and_log(
-        cons, f'dtc -o {dtb_file}', stdin=b'/dts-v1/; / {};')
-
-    fsfile = 'vfat18M.img'
-    u_boot_utils.run_and_log(cons, f'fallocate -l 18M {fsfile}')
-    u_boot_utils.run_and_log(cons, f'mkfs.vfat {fsfile}')
-    u_boot_utils.run_and_log(cons, ['sh', '-c', f'mcopy -i {fsfile} {mnt}/* ::/'])
-    u_boot_utils.run_and_log(cons, f'dd if={fsfile} of={fname} bs=1M seek=1')
-    u_boot_utils.run_and_log(cons, f'rm -rf {mnt}')
-    u_boot_utils.run_and_log(cons, f'rm -f {fsfile}')
+    setup_extlinux_image(cons, mmc_dev, vmlinux, initrd, dtbdir, script)
 
 def setup_cros_image(cons):
     """Create a 20MB disk image with ChromiumOS partitions"""
-- 
2.34.1

[PATCH 2/5] test: bootflow: Avoid a confusing error condition

[Simon Glass]

The prep_mmc_bootdev() function replaces bootstd's bootdev_order with
its own static version, then returns it.

From then on std->bootdev_order cannot be freed, since it was not
allocated.

So long as the test passes, all is well. But if a test fails, the test
system will try to free std->bootdev_order and this will fail.

Adjust prep_mmc_bootdev() to allocate the boot_dev order, instead.

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

 test/boot/bootflow.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index 35580cee90c..4d7d795cbe1 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -540,12 +540,15 @@ BOOTSTD_TEST(bootflow_cmd_boot, UTF_DM | UTF_SCAN_FDT | UTF_CONSOLE);
 static int prep_mmc_bootdev(struct unit_test_state *uts, const char *mmc_dev,
 			    bool bind_cros_android, const char ***old_orderp)
 {
-	static const char *order[] = {"mmc2", "mmc1", NULL, NULL};
+	static const char **order;
 	struct udevice *dev, *bootstd;
 	struct bootstd_priv *std;
 	const char **old_order;
 	ofnode root, node;
 
+	order = calloc(sizeof(void *), 4);
+	order[0] = "mmc2";
+	order[1] = "mmc1";
 	order[2] = mmc_dev;
 
 	/* Enable the requested mmc node since we need a second bootflow */
@@ -605,6 +608,7 @@ static int scan_mmc_bootdev(struct unit_test_state *uts, const char *mmc_dev,
 	/* Restore the order used by the device tree */
 	ut_assertok(uclass_first_device_err(UCLASS_BOOTSTD, &bootstd));
 	std = dev_get_priv(bootstd);
+	free(std->bootdev_order);
 	std->bootdev_order = old_order;
 
 	return 0;
@@ -635,6 +639,7 @@ static int scan_mmc_android_bootdev(struct unit_test_state *uts, const char *mmc
 	/* Restore the order used by the device tree */
 	ut_assertok(uclass_first_device_err(UCLASS_BOOTSTD, &bootstd));
 	std = dev_get_priv(bootstd);
+	free(std->bootdev_order);
 	std->bootdev_order = old_order;
 
 	return 0;
@@ -726,6 +731,7 @@ static int bootflow_scan_menu(struct unit_test_state *uts)
 
 	std->bootdev_order = new_order; /* Blue Monday */
 	ut_assertok(run_command("bootflow scan -lm", 0));
+	free(std->bootdev_order);
 	std->bootdev_order = old_order;
 
 	ut_assertnull(std->cur_bootflow);
-- 
2.34.1

[PATCH 3/5] pxe_utils: Allow the FDT to be missing

[Simon Glass]

The devicetree file may not be provided, so avoid a failure in that
case.

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

 boot/pxe_utils.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
index 3a4caa9329a..8bebf4ec925 100644
--- a/boot/pxe_utils.c
+++ b/boot/pxe_utils.c
@@ -799,7 +799,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label)
 		}
 		unmap_sysmem(buf);
 	}
-	if (ctx->bflow)
+	if (ctx->bflow && conf_fdt)
 		ctx->bflow->fdt_addr = hextoul(conf_fdt, NULL);
 
 	if (IS_ENABLED(CONFIG_BOOTSTD_FULL) && ctx->no_boot) {
@@ -819,7 +819,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label)
 				  ctx->initrd_addr_str, ctx->initrd_filesize,
 				  ctx->initrd_str);
 		}
-		if (!ctx->kernel_addr || !ctx->conf_fdt ||
+		if (!ctx->kernel_addr || (conf_fdt && !ctx->conf_fdt) ||
 		    (initrd_addr_str && (!ctx->initrd_addr_str ||
 		     !ctx->initrd_filesize || !ctx->initrd_str))) {
 			printf("malloc fail (saving label)\n");
-- 
2.34.1

[PATCH 4/5] pxe_utils: Support a backup for localboot

[Simon Glass]

The current localboot implementation assumes that a 'localcmd'
environment variable is provided, with the instructions to follow. This
may not be included, so provide a fallback in that case.

Add a test image and test as well.

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

 arch/sandbox/dts/test.dts |  8 +++++++
 boot/Kconfig              |  9 ++++++++
 boot/pxe_utils.c          | 33 ++++++++++++++++++++++++---
 test/boot/bootflow.c      | 47 +++++++++++++++++++++++++++++++++++++++
 test/py/tests/test_ut.py  | 17 ++++++++++++++
 5 files changed, 111 insertions(+), 3 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 36cfbf213e4..47e17070886 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -45,6 +45,7 @@
 		mmc6 = "/mmc6";
 		mmc7 = "/mmc7";
 		mmc8 = "/mmc8";
+		mmc9 = "/mmc9";
 		pci0 = &pci0;
 		pci1 = &pci1;
 		pci2 = &pci2;
@@ -1153,6 +1154,13 @@
 		filename = "mmc8.img";
 	};
 
+	/* This is used for extlinux localboot */
+	mmc9 {
+		status = "disabled";
+		compatible = "sandbox,mmc";
+		filename = "mmc9.img";
+	};
+
 	pch {
 		compatible = "sandbox,pch";
 	};
diff --git a/boot/Kconfig b/boot/Kconfig
index 705947cfa95..1356699021a 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -556,6 +556,15 @@ config BOOTMETH_EXTLINUX_PXE
 
 	  This provides a way to try out standard boot on an existing boot flow.
 
+config BOOTMETH_EXTLINUX_LOCALBOOT
+	bool "Boot method for extlinux localboot"
+	depends on BOOTMETH_EXTLINUX
+	default y
+	help
+	  Enables standard boot support for the extlinux 'localboot' feature.
+	  This attempts to find a kernel and initrd on the disk and boot it,
+	  in the case where there is no "localcmd" in the environment.
+
 config BOOTMETH_EFILOADER
 	bool "Bootdev support for EFI boot"
 	depends on EFI_BINARY_EXEC
diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
index 8bebf4ec925..5d52a5965d6 100644
--- a/boot/pxe_utils.c
+++ b/boot/pxe_utils.c
@@ -640,6 +640,25 @@ static int label_run_boot(struct pxe_context *ctx, struct pxe_label *label,
 	return 0;
 }
 
+/**
+ * generate_localboot() - Try to come up with a localboot definition
+ *
+ * Adds a default kernel and initrd filename for use with localboot
+ *
+ * @label: Label to process
+ * Return 0 if OK, -ENOMEM if out of memory
+ */
+static int generate_localboot(struct pxe_label *label)
+{
+	label->kernel = strdup("/vmlinuz");
+	label->kernel_label = strdup(label->kernel);
+	label->initrd = strdup("/initrd.img");
+	if (!label->kernel || !label->kernel_label || !label->initrd)
+		return -ENOMEM;
+
+	return 0;
+}
+
 /**
  * label_boot() - Boot according to the contents of a pxe_label
  *
@@ -677,9 +696,17 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label)
 	label->attempted = 1;
 
 	if (label->localboot) {
-		if (label->localboot_val >= 0)
-			label_localboot(label);
-		return 0;
+		if (label->localboot_val >= 0) {
+			ret = label_localboot(label);
+
+			if (IS_ENABLED(CONFIG_BOOTMETH_EXTLINUX_LOCALBOOT) &&
+			    ret == -ENOENT)
+				ret = generate_localboot(label);
+			if (ret)
+				return ret;
+		} else {
+			return 0;
+		}
 	}
 
 	if (!label->kernel) {
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index 4d7d795cbe1..db1af0e5729 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -1510,3 +1510,50 @@ static int bootflow_scan_extlinux(struct unit_test_state *uts)
 	return 0;
 }
 BOOTSTD_TEST(bootflow_scan_extlinux, UTF_DM | UTF_SCAN_FDT | UTF_CONSOLE);
+
+/* Check automatically generating a extlinux 'localboot' */
+static int bootflow_extlinux_localboot(struct unit_test_state *uts)
+{
+	const struct bootflow_img *img;
+	struct bootstd_priv *std;
+	const char **old_order;
+	struct bootflow *bflow;
+
+	ut_assertok(prep_mmc_bootdev(uts, "mmc9", false, &old_order));
+
+	ut_assertok(run_command("bootflow scan", 0));
+	ut_assert_console_end();
+
+	/* Restore the order used by the device tree */
+	ut_assertok(bootstd_get_priv(&std));
+	free(std->bootdev_order);
+	std->bootdev_order = old_order;
+
+	/* boot the second bootflow */
+	ut_asserteq(2, std->bootflows.count);
+	bflow = alist_getw(&std->bootflows, 1, struct bootflow);
+	std->cur_bootflow = bflow;
+
+	/* read all the images, but don't actually boot */
+	ut_assertok(bootflow_read_all(bflow));
+	ut_assert_nextline("1:\tlocal");
+	ut_assert_nextline("missing environment variable: localcmd");
+	ut_assert_nextline("Retrieving file: /vmlinuz");
+	ut_assert_nextline("Retrieving file: /initrd.img");
+
+	ut_assert_console_end();
+
+	ut_asserteq(3, bflow->images.count);
+
+	/* check the two localboot images */
+	img = alist_get(&bflow->images, 1, struct bootflow_img);
+	ut_asserteq(IH_TYPE_KERNEL, img->type);
+	ut_asserteq(0x1000000, img->addr);	/* kernel_addr_r */
+
+	img = alist_get(&bflow->images, 2, struct bootflow_img);
+	ut_asserteq(IH_TYPE_RAMDISK, img->type);
+	ut_asserteq(0x2000000, img->addr);	/* ramdisk_addr_r */
+
+	return 0;
+}
+BOOTSTD_TEST(bootflow_extlinux_localboot, UTF_DM | UTF_SCAN_FDT | UTF_CONSOLE);
diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py
index e3b988efe23..b6b4717c834 100644
--- a/test/py/tests/test_ut.py
+++ b/test/py/tests/test_ut.py
@@ -575,6 +575,22 @@ def setup_efi_image(cons):
     u_boot_utils.run_and_log(cons, f'rm -rf {mnt}')
     u_boot_utils.run_and_log(cons, f'rm -f {fsfile}')
 
+
+def setup_localboot_image(cons):
+    """Create a 20MB disk image with a single FAT partition"""
+    mmc_dev = 9
+    fname, mnt = setup_image(cons, mmc_dev, 0xc, second_part=True)
+
+    script = '''DEFAULT local
+
+LABEL local
+  LOCALBOOT 0
+'''
+    vmlinux = 'vmlinuz'
+    initrd = 'initrd.img'
+    setup_extlinux_image(cons, mmc_dev, vmlinux, initrd, None, script)
+
+
 @pytest.mark.buildconfigspec('cmd_bootflow')
 @pytest.mark.buildconfigspec('sandbox')
 def test_ut_dm_init_bootstd(u_boot_console):
@@ -586,6 +602,7 @@ def test_ut_dm_init_bootstd(u_boot_console):
     setup_cros_image(u_boot_console)
     setup_android_image(u_boot_console)
     setup_efi_image(u_boot_console)
+    setup_localboot_image(u_boot_console)
 
     # Restart so that the new mmc1.img is picked up
     u_boot_console.restart_uboot()
-- 
2.34.1

[PATCH 5/5] pxe_utils: Support the SAY command

[Simon Glass]

This shows a message for the user. Implement it to keep the user
informed.

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

 boot/pxe_utils.c         | 13 +++++++++++++
 test/boot/bootflow.c     |  1 +
 test/py/tests/test_ut.py |  1 +
 3 files changed, 15 insertions(+)

diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
index 5d52a5965d6..5a8fbbe57ad 100644
--- a/boot/pxe_utils.c
+++ b/boot/pxe_utils.c
@@ -890,6 +890,7 @@ enum token_type {
 	T_BACKGROUND,
 	T_KASLRSEED,
 	T_FALLBACK,
+	T_SAY,
 	T_INVALID
 };
 
@@ -924,6 +925,7 @@ static const struct token keywords[] = {
 	{"background", T_BACKGROUND,},
 	{"kaslrseed", T_KASLRSEED,},
 	{"fallback", T_FALLBACK,},
+	{"say", T_SAY,},
 	{NULL, T_INVALID}
 };
 
@@ -1388,6 +1390,17 @@ static int parse_label(char **c, struct pxe_menu *cfg)
 		case T_EOL:
 			break;
 
+		case T_SAY: {
+			char *p = strchr(s, '\n');
+
+			if (p) {
+				printf("%.*s\n", (int)(p - *c) - 1, *c + 1);
+
+				*c = p;
+			}
+			break;
+		}
+
 		default:
 			/*
 			 * put the token back! we don't want it - it's the end
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index db1af0e5729..7675d632884 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -1536,6 +1536,7 @@ static int bootflow_extlinux_localboot(struct unit_test_state *uts)
 
 	/* read all the images, but don't actually boot */
 	ut_assertok(bootflow_read_all(bflow));
+	ut_assert_nextline("Doing local boot...");
 	ut_assert_nextline("1:\tlocal");
 	ut_assert_nextline("missing environment variable: localcmd");
 	ut_assert_nextline("Retrieving file: /vmlinuz");
diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py
index b6b4717c834..89c765c0e6e 100644
--- a/test/py/tests/test_ut.py
+++ b/test/py/tests/test_ut.py
@@ -584,6 +584,7 @@ def setup_localboot_image(cons):
     script = '''DEFAULT local
 
 LABEL local
+  SAY Doing local boot...
   LOCALBOOT 0
 '''
     vmlinux = 'vmlinuz'
-- 
2.34.1

Re: [PATCH 5/5] pxe_utils: Support the SAY command

[Tom Rini]

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

On Thu, Dec 19, 2024 at 09:01:20PM -0700, Simon Glass wrote:

> This shows a message for the user. Implement it to keep the user
> informed.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Where does the SAY command come from? This is in neither
https://systemd.io/BOOT_LOADER_SPECIFICATION/ (which redirects and is
limited) nor https://wiki.syslinux.org/wiki/index.php?title=PXELINUX
(which is where http://syslinux.zytor.com/wiki/index.php/Doc/pxelinux
redirects to now).

-- 
Tom

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

Re: [PATCH 3/5] pxe_utils: Allow the FDT to be missing

[Tom Rini]

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

On Thu, Dec 19, 2024 at 09:01:18PM -0700, Simon Glass wrote:

> The devicetree file may not be provided, so avoid a failure in that
> 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 4/5] pxe_utils: Support a backup for localboot

[Tom Rini]

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

On Thu, Dec 19, 2024 at 09:01:19PM -0700, Simon Glass wrote:

> The current localboot implementation assumes that a 'localcmd'
> environment variable is provided, with the instructions to follow. This
> may not be included, so provide a fallback in that case.
> 
> Add a test image and test as well.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

This is a pretty niche feature, I had to dig around a bit to see how
it's specified elsewhere (not really) and how it's used. And I think
that based on how it's used, making up a bootcmd when localcmd is
undefined is the wrong approach. It's the hook for "run what I defined
in the environment", so if not set erroring back out seems appropriate.

-- 
Tom

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

Re: [PATCH 4/5] pxe_utils: Support a backup for localboot

[Simon Glass]

Hi Tom,

On Fri, 20 Dec 2024 at 07:56, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Dec 19, 2024 at 09:01:19PM -0700, Simon Glass wrote:
>
> > The current localboot implementation assumes that a 'localcmd'
> > environment variable is provided, with the instructions to follow. This
> > may not be included, so provide a fallback in that case.
> >
> > Add a test image and test as well.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
>
> This is a pretty niche feature, I had to dig around a bit to see how
> it's specified elsewhere (not really) and how it's used. And I think
> that based on how it's used, making up a bootcmd when localcmd is
> undefined is the wrong approach. It's the hook for "run what I defined
> in the environment", so if not set erroring back out seems appropriate.

Yes, but unfortunately it seems to be used and we should support it.
The problem with scripts is that we don't know the boot device, etc,
so it needs to be integrated into PXE. I did consider putting
something in bootstd, but we only find out that it is requesting a
localboot when actually running the extlinux bootmeth, so this is what
I came up with.

It will be interesting to see if any other cases come up.

Regards,
Simon

Re: [PATCH 4/5] pxe_utils: Support a backup for localboot

[Tom Rini]

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

On Fri, Dec 20, 2024 at 10:18:17AM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Fri, 20 Dec 2024 at 07:56, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Dec 19, 2024 at 09:01:19PM -0700, Simon Glass wrote:
> >
> > > The current localboot implementation assumes that a 'localcmd'
> > > environment variable is provided, with the instructions to follow. This
> > > may not be included, so provide a fallback in that case.
> > >
> > > Add a test image and test as well.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> >
> > This is a pretty niche feature, I had to dig around a bit to see how
> > it's specified elsewhere (not really) and how it's used. And I think
> > that based on how it's used, making up a bootcmd when localcmd is
> > undefined is the wrong approach. It's the hook for "run what I defined
> > in the environment", so if not set erroring back out seems appropriate.
> 
> Yes, but unfortunately it seems to be used and we should support it.
> The problem with scripts is that we don't know the boot device, etc,
> so it needs to be integrated into PXE. I did consider putting
> something in bootstd, but we only find out that it is requesting a
> localboot when actually running the extlinux bootmeth, so this is what
> I came up with.
> 
> It will be interesting to see if any other cases come up.

It would be helpful at this point I think if you can point to how the
code for handling this case (the LOCALBOOT keyword followed by an
integer) in other projects so that we can be compliant with what's
expected, even if it's poorly documented.

-- 
Tom

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

Re: [PATCH 5/5] pxe_utils: Support the SAY command

[Simon Glass]

Hi Tom,

On Fri, 20 Dec 2024 at 07:44, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Dec 19, 2024 at 09:01:20PM -0700, Simon Glass wrote:
>
> > This shows a message for the user. Implement it to keep the user
> > informed.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
>
> Where does the SAY command come from? This is in neither
> https://systemd.io/BOOT_LOADER_SPECIFICATION/ (which redirects and is
> limited) nor https://wiki.syslinux.org/wiki/index.php?title=PXELINUX
> (which is where http://syslinux.zytor.com/wiki/index.php/Doc/pxelinux
> redirects to now).

https://wiki.syslinux.org/wiki/index.php?title=SYSLINUX

Regards,
Simon

Re: [PATCH 4/5] pxe_utils: Support a backup for localboot

[Simon Glass]

Hi Tom,

On Fri, 20 Dec 2024 at 10:23, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Dec 20, 2024 at 10:18:17AM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Fri, 20 Dec 2024 at 07:56, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Thu, Dec 19, 2024 at 09:01:19PM -0700, Simon Glass wrote:
> > >
> > > > The current localboot implementation assumes that a 'localcmd'
> > > > environment variable is provided, with the instructions to follow. This
> > > > may not be included, so provide a fallback in that case.
> > > >
> > > > Add a test image and test as well.
> > > >
> > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > >
> > > This is a pretty niche feature, I had to dig around a bit to see how
> > > it's specified elsewhere (not really) and how it's used. And I think
> > > that based on how it's used, making up a bootcmd when localcmd is
> > > undefined is the wrong approach. It's the hook for "run what I defined
> > > in the environment", so if not set erroring back out seems appropriate.
> >
> > Yes, but unfortunately it seems to be used and we should support it.
> > The problem with scripts is that we don't know the boot device, etc,
> > so it needs to be integrated into PXE. I did consider putting
> > something in bootstd, but we only find out that it is requesting a
> > localboot when actually running the extlinux bootmeth, so this is what
> > I came up with.
> >
> > It will be interesting to see if any other cases come up.
>
> It would be helpful at this point I think if you can point to how the
> code for handling this case (the LOCALBOOT keyword followed by an
> integer) in other projects so that we can be compliant with what's
> expected, even if it's poorly documented.

Yes, it seems very hard to find anything at all.

The implementation in syslinux uses the BIOS to jump to a boot sector:

https://repo.or.cz/syslinux.git/blob/HEAD:/core/bios/localboot.c

I'm really not sure how it is supposed to work with a filesystem.

[1] is a usage of it for rpi

But other than that, even Gemini doesn't seems to have much idea.

Regards,
Simon

[1] https://github.com/ppisa/rpi-utils/blob/master/u-boot-setup/boot/extlinux/extlinux.conf

Re: [PATCH 5/5] pxe_utils: Support the SAY command

[Tom Rini]

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

On Fri, Dec 20, 2024 at 10:36:56AM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Fri, 20 Dec 2024 at 07:44, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Dec 19, 2024 at 09:01:20PM -0700, Simon Glass wrote:
> >
> > > This shows a message for the user. Implement it to keep the user
> > > informed.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> >
> > Where does the SAY command come from? This is in neither
> > https://systemd.io/BOOT_LOADER_SPECIFICATION/ (which redirects and is
> > limited) nor https://wiki.syslinux.org/wiki/index.php?title=PXELINUX
> > (which is where http://syslinux.zytor.com/wiki/index.php/Doc/pxelinux
> > redirects to now).
> 
> https://wiki.syslinux.org/wiki/index.php?title=SYSLINUX

Ah, thanks. Should link to that too in the docs then, I'll v2 my patch
there.

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

-- 
Tom

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

Re: [PATCH 5/5] pxe_utils: Support the SAY command

[Simon Glass]

Hi Tom,

On Fri, 20 Dec 2024 at 11:31, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Dec 20, 2024 at 10:36:56AM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Fri, 20 Dec 2024 at 07:44, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Thu, Dec 19, 2024 at 09:01:20PM -0700, Simon Glass wrote:
> > >
> > > > This shows a message for the user. Implement it to keep the user
> > > > informed.
> > > >
> > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > >
> > > Where does the SAY command come from? This is in neither
> > > https://systemd.io/BOOT_LOADER_SPECIFICATION/ (which redirects and is
> > > limited) nor https://wiki.syslinux.org/wiki/index.php?title=PXELINUX
> > > (which is where http://syslinux.zytor.com/wiki/index.php/Doc/pxelinux
> > > redirects to now).
> >
> > https://wiki.syslinux.org/wiki/index.php?title=SYSLINUX
>
> Ah, thanks. Should link to that too in the docs then, I'll v2 my patch
> there.
>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Thanks. The confusing thing is that there are a few slightly different
formats at the same URL...

Regards,
Simon

Re: [PATCH 4/5] pxe_utils: Support a backup for localboot

[Tom Rini]

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

On Fri, Dec 20, 2024 at 10:37:34AM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Fri, 20 Dec 2024 at 10:23, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Fri, Dec 20, 2024 at 10:18:17AM -0700, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Fri, 20 Dec 2024 at 07:56, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Thu, Dec 19, 2024 at 09:01:19PM -0700, Simon Glass wrote:
> > > >
> > > > > The current localboot implementation assumes that a 'localcmd'
> > > > > environment variable is provided, with the instructions to follow. This
> > > > > may not be included, so provide a fallback in that case.
> > > > >
> > > > > Add a test image and test as well.
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > >
> > > > This is a pretty niche feature, I had to dig around a bit to see how
> > > > it's specified elsewhere (not really) and how it's used. And I think
> > > > that based on how it's used, making up a bootcmd when localcmd is
> > > > undefined is the wrong approach. It's the hook for "run what I defined
> > > > in the environment", so if not set erroring back out seems appropriate.
> > >
> > > Yes, but unfortunately it seems to be used and we should support it.
> > > The problem with scripts is that we don't know the boot device, etc,
> > > so it needs to be integrated into PXE. I did consider putting
> > > something in bootstd, but we only find out that it is requesting a
> > > localboot when actually running the extlinux bootmeth, so this is what
> > > I came up with.
> > >
> > > It will be interesting to see if any other cases come up.
> >
> > It would be helpful at this point I think if you can point to how the
> > code for handling this case (the LOCALBOOT keyword followed by an
> > integer) in other projects so that we can be compliant with what's
> > expected, even if it's poorly documented.
> 
> Yes, it seems very hard to find anything at all.
> 
> The implementation in syslinux uses the BIOS to jump to a boot sector:
> 
> https://repo.or.cz/syslinux.git/blob/HEAD:/core/bios/localboot.c
> 
> I'm really not sure how it is supposed to work with a filesystem.
> 
> [1] is a usage of it for rpi

Yes, I did some searching on extlinux.conf and localboot and got a few
hits on people using localcmd for when they need to run their own
special thing, for various reasons, for example:
https://stackoverflow.com/questions/54258960/applying-fdt-overlay-with-u-boot-using-extlinux-conf

> But other than that, even Gemini doesn't seems to have much idea.

I'm only half surprised it didn't make something up for it.

-- 
Tom

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

Re: [PATCH 4/5] pxe_utils: Support a backup for localboot

[Simon Glass]

Hi Tom,

On Fri, 20 Dec 2024 at 13:48, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Dec 20, 2024 at 10:37:34AM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Fri, 20 Dec 2024 at 10:23, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Fri, Dec 20, 2024 at 10:18:17AM -0700, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Fri, 20 Dec 2024 at 07:56, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Thu, Dec 19, 2024 at 09:01:19PM -0700, Simon Glass wrote:
> > > > >
> > > > > > The current localboot implementation assumes that a 'localcmd'
> > > > > > environment variable is provided, with the instructions to follow. This
> > > > > > may not be included, so provide a fallback in that case.
> > > > > >
> > > > > > Add a test image and test as well.
> > > > > >
> > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > >
> > > > > This is a pretty niche feature, I had to dig around a bit to see how
> > > > > it's specified elsewhere (not really) and how it's used. And I think
> > > > > that based on how it's used, making up a bootcmd when localcmd is
> > > > > undefined is the wrong approach. It's the hook for "run what I defined
> > > > > in the environment", so if not set erroring back out seems appropriate.
> > > >
> > > > Yes, but unfortunately it seems to be used and we should support it.
> > > > The problem with scripts is that we don't know the boot device, etc,
> > > > so it needs to be integrated into PXE. I did consider putting
> > > > something in bootstd, but we only find out that it is requesting a
> > > > localboot when actually running the extlinux bootmeth, so this is what
> > > > I came up with.
> > > >
> > > > It will be interesting to see if any other cases come up.
> > >
> > > It would be helpful at this point I think if you can point to how the
> > > code for handling this case (the LOCALBOOT keyword followed by an
> > > integer) in other projects so that we can be compliant with what's
> > > expected, even if it's poorly documented.
> >
> > Yes, it seems very hard to find anything at all.
> >
> > The implementation in syslinux uses the BIOS to jump to a boot sector:
> >
> > https://repo.or.cz/syslinux.git/blob/HEAD:/core/bios/localboot.c
> >
> > I'm really not sure how it is supposed to work with a filesystem.
> >
> > [1] is a usage of it for rpi
>
> Yes, I did some searching on extlinux.conf and localboot and got a few
> hits on people using localcmd for when they need to run their own
> special thing, for various reasons, for example:
> https://stackoverflow.com/questions/54258960/applying-fdt-overlay-with-u-boot-using-extlinux-conf
>
> > But other than that, even Gemini doesn't seems to have much idea.
>
> I'm only half surprised it didn't make something up for it.

I've been quite amazed at its capabilities recently. I can mostly get
good answers to quite complex questions and it is very fast now. I'm
just waiting for when I can start using it for development.

I wonder who maintains the extlinux stuff. Do you have any idea? It
would be nice to move it into a git tree and generate the spec from
there.

Regards,
SImon

Re: [PATCH 4/5] pxe_utils: Support a backup for localboot

[Tom Rini]

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

On Wed, Jan 08, 2025 at 10:04:07AM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Fri, 20 Dec 2024 at 13:48, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Fri, Dec 20, 2024 at 10:37:34AM -0700, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Fri, 20 Dec 2024 at 10:23, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Fri, Dec 20, 2024 at 10:18:17AM -0700, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Fri, 20 Dec 2024 at 07:56, Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Thu, Dec 19, 2024 at 09:01:19PM -0700, Simon Glass wrote:
> > > > > >
> > > > > > > The current localboot implementation assumes that a 'localcmd'
> > > > > > > environment variable is provided, with the instructions to follow. This
> > > > > > > may not be included, so provide a fallback in that case.
> > > > > > >
> > > > > > > Add a test image and test as well.
> > > > > > >
> > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > >
> > > > > > This is a pretty niche feature, I had to dig around a bit to see how
> > > > > > it's specified elsewhere (not really) and how it's used. And I think
> > > > > > that based on how it's used, making up a bootcmd when localcmd is
> > > > > > undefined is the wrong approach. It's the hook for "run what I defined
> > > > > > in the environment", so if not set erroring back out seems appropriate.
> > > > >
> > > > > Yes, but unfortunately it seems to be used and we should support it.
> > > > > The problem with scripts is that we don't know the boot device, etc,
> > > > > so it needs to be integrated into PXE. I did consider putting
> > > > > something in bootstd, but we only find out that it is requesting a
> > > > > localboot when actually running the extlinux bootmeth, so this is what
> > > > > I came up with.
> > > > >
> > > > > It will be interesting to see if any other cases come up.
> > > >
> > > > It would be helpful at this point I think if you can point to how the
> > > > code for handling this case (the LOCALBOOT keyword followed by an
> > > > integer) in other projects so that we can be compliant with what's
> > > > expected, even if it's poorly documented.
> > >
> > > Yes, it seems very hard to find anything at all.
> > >
> > > The implementation in syslinux uses the BIOS to jump to a boot sector:
> > >
> > > https://repo.or.cz/syslinux.git/blob/HEAD:/core/bios/localboot.c
> > >
> > > I'm really not sure how it is supposed to work with a filesystem.
> > >
> > > [1] is a usage of it for rpi
> >
> > Yes, I did some searching on extlinux.conf and localboot and got a few
> > hits on people using localcmd for when they need to run their own
> > special thing, for various reasons, for example:
> > https://stackoverflow.com/questions/54258960/applying-fdt-overlay-with-u-boot-using-extlinux-conf
> >
> > > But other than that, even Gemini doesn't seems to have much idea.
> >
> > I'm only half surprised it didn't make something up for it.
> 
> I've been quite amazed at its capabilities recently. I can mostly get
> good answers to quite complex questions and it is very fast now. I'm
> just waiting for when I can start using it for development.

Oh no, please no. We don't have a policy yet, but no "AI" created code,
please.

> I wonder who maintains the extlinux stuff. Do you have any idea? It
> would be nice to move it into a git tree and generate the spec from
> there.

I would say the syslinux project should be the authority unless they say
otherwise.

-- 
Tom

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

Re: [PATCH 4/5] pxe_utils: Support a backup for localboot

[Simon Glass]

On Wed, 8 Jan 2025 at 10:31, Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Jan 08, 2025 at 10:04:07AM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Fri, 20 Dec 2024 at 13:48, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Fri, Dec 20, 2024 at 10:37:34AM -0700, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Fri, 20 Dec 2024 at 10:23, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Fri, Dec 20, 2024 at 10:18:17AM -0700, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Fri, 20 Dec 2024 at 07:56, Tom Rini <trini@konsulko.com> wrote:
> > > > > > >
> > > > > > > On Thu, Dec 19, 2024 at 09:01:19PM -0700, Simon Glass wrote:
> > > > > > >
> > > > > > > > The current localboot implementation assumes that a 'localcmd'
> > > > > > > > environment variable is provided, with the instructions to follow. This
> > > > > > > > may not be included, so provide a fallback in that case.
> > > > > > > >
> > > > > > > > Add a test image and test as well.
> > > > > > > >
> > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > >
> > > > > > > This is a pretty niche feature, I had to dig around a bit to see how
> > > > > > > it's specified elsewhere (not really) and how it's used. And I think
> > > > > > > that based on how it's used, making up a bootcmd when localcmd is
> > > > > > > undefined is the wrong approach. It's the hook for "run what I defined
> > > > > > > in the environment", so if not set erroring back out seems appropriate.
> > > > > >
> > > > > > Yes, but unfortunately it seems to be used and we should support it.
> > > > > > The problem with scripts is that we don't know the boot device, etc,
> > > > > > so it needs to be integrated into PXE. I did consider putting
> > > > > > something in bootstd, but we only find out that it is requesting a
> > > > > > localboot when actually running the extlinux bootmeth, so this is what
> > > > > > I came up with.
> > > > > >
> > > > > > It will be interesting to see if any other cases come up.
> > > > >
> > > > > It would be helpful at this point I think if you can point to how the
> > > > > code for handling this case (the LOCALBOOT keyword followed by an
> > > > > integer) in other projects so that we can be compliant with what's
> > > > > expected, even if it's poorly documented.
> > > >
> > > > Yes, it seems very hard to find anything at all.
> > > >
> > > > The implementation in syslinux uses the BIOS to jump to a boot sector:
> > > >
> > > > https://repo.or.cz/syslinux.git/blob/HEAD:/core/bios/localboot.c
> > > >
> > > > I'm really not sure how it is supposed to work with a filesystem.
> > > >
> > > > [1] is a usage of it for rpi
> > >
> > > Yes, I did some searching on extlinux.conf and localboot and got a few
> > > hits on people using localcmd for when they need to run their own
> > > special thing, for various reasons, for example:
> > > https://stackoverflow.com/questions/54258960/applying-fdt-overlay-with-u-boot-using-extlinux-conf
> > >
> > > > But other than that, even Gemini doesn't seems to have much idea.
> > >
> > > I'm only half surprised it didn't make something up for it.
> >
> > I've been quite amazed at its capabilities recently. I can mostly get
> > good answers to quite complex questions and it is very fast now. I'm
> > just waiting for when I can start using it for development.
>
> Oh no, please no. We don't have a policy yet, but no "AI" created code,
> please.

lol me too :-)

>
> > I wonder who maintains the extlinux stuff. Do you have any idea? It
> > would be nice to move it into a git tree and generate the spec from
> > there.
>
> I would say the syslinux project should be the authority unless they say
> otherwise.

Yes I've tried irc and will see if I get any ideas. Will send an email too.

Regards,
Simon

Re: [PATCH 3/5] pxe_utils: Allow the FDT to be missing

[Quentin Schulz]

Hi Simon,

On 12/20/24 5:01 AM, Simon Glass wrote:
> The devicetree file may not be provided, so avoid a failure in that
> case.
> 

Can you provide a bit more information on when/why this shouldn't be a 
failure? I assume likely x86? or Aarch64 with ACPI instead of OF?

Cheers,
Quentin

Re: [PATCH 3/5] pxe_utils: Allow the FDT to be missing

[Simon Glass]

Hi Quentin,

On Tue, 14 Jan 2025 at 08:13, Quentin Schulz <quentin.schulz@cherry.de> wrote:
>
> Hi Simon,
>
> On 12/20/24 5:01 AM, Simon Glass wrote:
> > The devicetree file may not be provided, so avoid a failure in that
> > case.
> >
>
> Can you provide a bit more information on when/why this shouldn't be a
> failure? I assume likely x86? or Aarch64 with ACPI instead of OF?

Yes, either of those, but also, even on platforms which use a
devicetree, some don't have it in the extlinux file. For example rpi
passes it through to U-Boot from its own closed-source firmware.

Somewhat related: my opinion with extlinux (and EFI for that matter)
is that we should be using FIT.

Regards,
Simon

Re: [PATCH 3/5] pxe_utils: Allow the FDT to be missing

[Quentin Schulz]

Hi Simon,

On 1/15/25 2:16 AM, Simon Glass wrote:
> Hi Quentin,
> 
> On Tue, 14 Jan 2025 at 08:13, Quentin Schulz <quentin.schulz@cherry.de> wrote:
>>
>> Hi Simon,
>>
>> On 12/20/24 5:01 AM, Simon Glass wrote:
>>> The devicetree file may not be provided, so avoid a failure in that
>>> case.
>>>
>>
>> Can you provide a bit more information on when/why this shouldn't be a
>> failure? I assume likely x86? or Aarch64 with ACPI instead of OF?
> 
> Yes, either of those, but also, even on platforms which use a
> devicetree, some don't have it in the extlinux file. For example rpi
> passes it through to U-Boot from its own closed-source firmware.
> 

Ah yes, I had forgotten about platforms using one DTB and passing it 
through stages, I believe Qualcomm does this as well?

Could you please add what you just said to the commit log so that we 
have an idea which scenario this intends to support (for future 
reference if this introduces a regression or we want to do refactoring 
in the future).

I'm wondering if this isn't going to allow booting FDT systems without 
an FDT at all, which really isn't great? Is there something we 
could/should do to prevent that from happening (or at least notify the 
user of such a scenario so it's easier to debug)?

> Somewhat related: my opinion with extlinux (and EFI for that matter)
> is that we should be using FIT.
> 

FIT images are not really developer-friendly though :/ I've been asked 
to ditch FIT images to allow easy replacement of customer Device 
Trees/kernel images. Much easier to say "copy your file there, modify 
extlinux.conf to point at the new filepath for the FDT" rather than "so 
this is the ITS, change the path here, and there, don't forget to have 
those binaries in the current path, use mkimage, then copy this itb file 
there".

In any case, I don't believe we could remove non-FIT support, so here we 
are :)

Cheers,
Quentin

Re: [PATCH 3/5] pxe_utils: Allow the FDT to be missing

[Simon Glass]

Hi Quentin,

On Wed, 15 Jan 2025 at 04:59, Quentin Schulz <quentin.schulz@cherry.de> wrote:
>
> Hi Simon,
>
> On 1/15/25 2:16 AM, Simon Glass wrote:
> > Hi Quentin,
> >
> > On Tue, 14 Jan 2025 at 08:13, Quentin Schulz <quentin.schulz@cherry.de> wrote:
> >>
> >> Hi Simon,
> >>
> >> On 12/20/24 5:01 AM, Simon Glass wrote:
> >>> The devicetree file may not be provided, so avoid a failure in that
> >>> case.
> >>>
> >>
> >> Can you provide a bit more information on when/why this shouldn't be a
> >> failure? I assume likely x86? or Aarch64 with ACPI instead of OF?
> >
> > Yes, either of those, but also, even on platforms which use a
> > devicetree, some don't have it in the extlinux file. For example rpi
> > passes it through to U-Boot from its own closed-source firmware.
> >
>
> Ah yes, I had forgotten about platforms using one DTB and passing it
> through stages, I believe Qualcomm does this as well?
>
> Could you please add what you just said to the commit log so that we
> have an idea which scenario this intends to support (for future
> reference if this introduces a regression or we want to do refactoring
> in the future).

OK

>
> I'm wondering if this isn't going to allow booting FDT systems without
> an FDT at all, which really isn't great? Is there something we
> could/should do to prevent that from happening (or at least notify the
> user of such a scenario so it's easier to debug)?

It shouldn't make any difference to the current code. Passing a
bootflow just allows the information to be recorded. The logic of
whether to use the FDT is not changed by this patch.

>
> > Somewhat related: my opinion with extlinux (and EFI for that matter)
> > is that we should be using FIT.
> >
>
> FIT images are not really developer-friendly though :/ I've been asked
> to ditch FIT images to allow easy replacement of customer Device
> Trees/kernel images. Much easier to say "copy your file there, modify
> extlinux.conf to point at the new filepath for the FDT" rather than "so
> this is the ITS, change the path here, and there, don't forget to have
> those binaries in the current path, use mkimage, then copy this itb file
> there".

Yes we need to keep that flow working, of course.

Re FIT, well, we could write a tool. Would that help? If someone did
that, what would it look like?

I've been happy with FIT for a long time, but particularly recently
due to all the workarounds and gymnastics people are doing to make
single files work, e.g. figuring out which devicetree to use.

There is 'u-boot-menu' which works on the running system.

>
> In any case, I don't believe we could remove non-FIT support, so here we
> are :)

Regards,
SImon