[PATCH 0/5] pxe: Support an automatic localboot
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(-)
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"""
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);
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");
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
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
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
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
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
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()
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.
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
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.
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/ex...
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-boo...
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.
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-boo...
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
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-boo...
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.
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-boo...
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
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'
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).
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
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).
Ah, thanks. Should link to that too in the docs then, I'll v2 my patch there.
Reviewed-by: Tom Rini trini@konsulko.com
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).
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
participants (3)
-
Quentin Schulz -
Simon Glass -
Tom Rini