From: Simon Glass <simon.glass@canonical.com> Skip cleanup of filesystem images in the pxe_fdtdir_image and pxe_error_image fixtures when the persist option is set. This allows inspection of the test files after a test run for debugging purposes. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- test/py/tests/test_pxe_parser.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/py/tests/test_pxe_parser.py b/test/py/tests/test_pxe_parser.py index de01f219244..d52bf3e6b50 100644 --- a/test/py/tests/test_pxe_parser.py +++ b/test/py/tests/test_pxe_parser.py @@ -325,7 +325,8 @@ def pxe_fdtdir_image(u_boot_config): yield fsh.fs_img, cfg_path - fsh.cleanup() + if not u_boot_config.persist: + fsh.cleanup() @pytest.fixture @@ -385,7 +386,8 @@ def pxe_error_image(u_boot_config): yield fsh.fs_img, cfg_path - fsh.cleanup() + if not u_boot_config.persist: + fsh.cleanup() @pytest.mark.boardspec('sandbox') -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Add pxe_check_menu() helper function that verifies all console output lines from loading and displaying the PXE menu. This includes config file retrieval, include files, background image attempt, menu title and all menu items. Update pxe_test_sysboot_norun() to use the new helper instead of ut_assert_skip_to_line(), providing more thorough output verification. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- test/boot/pxe.c | 175 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 142 insertions(+), 33 deletions(-) diff --git a/test/boot/pxe.c b/test/boot/pxe.c index 0da28be8576..f12add77db6 100644 --- a/test/boot/pxe.c +++ b/test/boot/pxe.c @@ -76,6 +76,87 @@ static int pxe_test_getfile(struct pxe_context *ctx, const char *file_path, return 0; } +/** + * pxe_check_menu() - Check the standard menu output lines + * + * This helper checks all the console output lines from loading and displaying + * the PXE menu, including config file retrieval, include files, background + * image attempt, and the menu itself. + * + * @uts: Unit test state + * @say_msg: Expected 'say' message (shown before menu), or NULL if none + * @error_msg: Expected error message after background image, or NULL if none + * Return: 0 if OK, -ve on error + */ +static int pxe_check_menu(struct unit_test_state *uts, const char *say_msg, + const char *error_msg) +{ + int i; + + /* Config file retrieval */ + ut_assert_nextline("Retrieving file: /extlinux/extlinux.conf"); + + /* Say message appears before includes are processed */ + if (say_msg) + ut_assert_nextline(say_msg); + + /* Include file retrievals */ + ut_assert_nextline("Retrieving file: /extlinux/extra.conf"); + for (i = 3; i <= 16; i++) + ut_assert_nextline("Retrieving file: /extlinux/nest%d.conf", i); + + /* Background image attempt */ + ut_assert_nextline("Retrieving file: /boot/background.bmp"); + ut_assert_nextline("There is no valid bmp file at the given address"); + + /* Optional error message before menu */ + if (error_msg) + ut_assert_nextline(error_msg); + + /* Menu title and items */ + ut_assert_nextline("Test Boot Menu"); + ut_assert_nextline("1:\tBoot Linux"); + ut_assert_nextline("2:\tRescue Mode"); + ut_assert_nextline("3:\tLocal Boot"); + ut_assert_nextline("4:\tFIT Boot"); + ut_assert_nextline("5:\tIncluded Label"); + for (i = 6; i <= 19; i++) + ut_assert_nextline("%d:\tLevel %d Label", i, i - 3); + + return 0; +} + +/** + * pxe_check_fdtdir() - Check fdtdir test menu and boot output + * + * Helper for pxe_test_fdtdir_norun() that checks the menu output and boot + * file retrieval messages for the fdtdir test configuration. + * + * @uts: Unit test state + * @dtb_name: Expected DTB filename (e.g., "test-board.dtb") + * Return: 0 if OK, -ve on error + */ +static int pxe_check_fdtdir(struct unit_test_state *uts, const char *dtb_name) +{ + /* Menu output */ + ut_assert_nextline("Retrieving file: /extlinux/extlinux.conf"); + ut_assert_nextline("Test Boot Menu"); + ut_assert_nextline("1:\tTest fdtfile env var"); + ut_assert_nextline("2:\tTest soc/board construction"); + ut_assert_nextline("Enter choice: 1:\tTest fdtfile env var"); + + /* Boot file retrieval */ + ut_assert_nextline("Retrieving file: /vmlinuz"); + ut_assert_nextline("append: console=ttyS0"); + ut_assert_nextline("Retrieving file: /dtb/%s", dtb_name); + ut_assert_nextline("Retrieving file: /dtb/overlay1.dtbo"); + + /* Boot fails on sandbox */ + ut_assert_nextline("Unrecognized zImage"); + + return 0; +} + /** * Test parsing an extlinux.conf file * @@ -124,6 +205,7 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_assert_nextline("Retrieving file: /extlinux/extra.conf"); for (i = 3; i <= 16; i++) ut_assert_nextline("Retrieving file: /extlinux/nest%d.conf", i); + ut_assert_console_end(); /* Verify menu properties */ ut_asserteq_str("Test Boot Menu", cfg->title); @@ -286,8 +368,9 @@ static int pxe_test_sysboot_norun(struct unit_test_state *uts) ut_assertok(run_commandf("sysboot host 0:0 any %x %s", PXE_LOAD_ADDR, cfg_path)); - /* Skip menu output and find the first label boot attempt */ - ut_assert_skip_to_line("Enter choice: 1:\tBoot Linux"); + /* Check menu output */ + ut_assertok(pxe_check_menu(uts, "Booting default Linux kernel", NULL)); + ut_assert_nextline("Enter choice: 1:\tBoot Linux"); /* Verify files were loaded in order */ ut_assert_nextline("Retrieving file: /vmlinuz"); @@ -299,6 +382,7 @@ static int pxe_test_sysboot_norun(struct unit_test_state *uts) /* Boot fails on sandbox */ ut_assert_nextline("Unrecognized zImage"); + ut_assert_console_end(); /* Verify files were loaded at the correct addresses */ kernel = map_sysmem(PXE_KERNEL_ADDR, 0); @@ -363,18 +447,7 @@ static int pxe_test_fdtdir_norun(struct unit_test_state *uts) ut_assertok(run_commandf("sysboot host 0:0 any %x %s", PXE_LOAD_ADDR, cfg_path)); - - /* Skip to the boot attempt - first label is fdtfile-test */ - ut_assert_skip_to_line("Enter choice: 1:\tTest fdtfile env var"); - - /* Verify fdtdir used fdtfile env var to construct path */ - ut_assert_nextline("Retrieving file: /vmlinuz"); - ut_assert_nextline("append: console=ttyS0"); - ut_assert_nextline("Retrieving file: /dtb/test-board.dtb"); - ut_assert_nextline("Retrieving file: /dtb/overlay1.dtbo"); - - /* Boot fails but we verified the path construction */ - ut_assert_nextline("Unrecognized zImage"); + ut_assertok(pxe_check_fdtdir(uts, "test-board.dtb")); /* Verify FDT was loaded correctly */ fdt = map_sysmem(PXE_FDT_ADDR, 0); @@ -391,18 +464,8 @@ static int pxe_test_fdtdir_norun(struct unit_test_state *uts) ut_assertok(run_commandf("sysboot host 0:0 any %x %s", PXE_LOAD_ADDR, cfg_path)); - - /* Still boots default label, but now uses soc-board path construction */ - ut_assert_skip_to_line("Enter choice: 1:\tTest fdtfile env var"); - - /* Verify fdtdir constructed path from soc-board */ - ut_assert_nextline("Retrieving file: /vmlinuz"); - ut_assert_nextline("append: console=ttyS0"); - ut_assert_nextline("Retrieving file: /dtb/tegra-jetson.dtb"); - ut_assert_nextline("Retrieving file: /dtb/overlay1.dtbo"); - - /* Boot fails but we verified the path construction */ - ut_assert_nextline("Unrecognized zImage"); + ut_assertok(pxe_check_fdtdir(uts, "tegra-jetson.dtb")); + ut_assert_console_end(); /* Verify FDT was loaded */ fdt = map_sysmem(PXE_FDT_ADDR, 0); @@ -455,12 +518,19 @@ static int pxe_test_errors_norun(struct unit_test_state *uts) ut_assertok(run_commandf("sysboot host 0:0 any %x %s", PXE_LOAD_ADDR, cfg_path)); + /* Check menu output */ + ut_assert_nextline("Retrieving file: /extlinux/extlinux.conf"); + ut_assert_nextline("Test Boot Menu"); + ut_assert_nextline("1:\tMissing explicit FDT"); + ut_assert_nextline("2:\tMissing fdtdir FDT"); + ut_assert_nextline("3:\tMissing overlay"); + ut_assert_nextline("Enter choice: 1:\tMissing explicit FDT"); + /* * Test 1: Explicit FDT file not found * First label (missing-fdt) has fdt=/dtb/nonexistent.dtb * Should fail and move to next label */ - ut_assert_skip_to_line("Enter choice: 1:\tMissing explicit FDT"); ut_assert_nextline("Retrieving file: /vmlinuz"); ut_assert_nextline("Retrieving file: /dtb/nonexistent.dtb"); ut_assert_nextline("Skipping missing-fdt for failure retrieving FDT"); @@ -480,6 +550,7 @@ static int pxe_test_errors_norun(struct unit_test_state *uts) * that label loading continued despite missing fdtdir FDT */ ut_assert_nextline("Unrecognized zImage"); + ut_assert_console_end(); /* Clean up env vars */ env_set("fdtfile", NULL); @@ -632,8 +703,12 @@ static int pxe_test_ipappend_norun(struct unit_test_state *uts) ut_assertok(run_commandf("sysboot host 0:0 any %x %s", PXE_LOAD_ADDR, cfg_path)); - /* Skip to the rescue label boot */ - ut_assert_skip_to_line("Retrieving file: /vmlinuz-rescue"); + /* Check menu output */ + ut_assertok(pxe_check_menu(uts, "Booting default Linux kernel", NULL)); + ut_assert_nextline("Enter choice: 2:\tRescue Mode"); + + /* Rescue label boot attempt */ + ut_assert_nextline("Retrieving file: /vmlinuz-rescue"); /* * Verify ipappend output - should have: @@ -644,6 +719,15 @@ static int pxe_test_ipappend_norun(struct unit_test_state *uts) ut_assert_nextlinen("append: single ip=192.168.1.10:192.168.1.1:" "192.168.1.254:255.255.255.0 BOOTIF=01-"); + /* + * Rescue label has fdtdir=/dtb/ but no fdtfile is set, so it tries + * to load /dtb/.dtb which fails. Boot still proceeds without FDT. + */ + ut_assert_nextline("Retrieving file: /dtb/.dtb"); + ut_assert_nextline("Skipping fdtdir /dtb/ for failure retrieving dts"); + ut_assert_nextline("Unrecognized zImage"); + ut_assert_console_end(); + /* Clean up */ env_set("ipaddr", NULL); env_set("serverip", NULL); @@ -729,6 +813,7 @@ static int pxe_test_label_override_norun(struct unit_test_state *uts) ut_assertok(env_set_hex("kernel_addr_r", PXE_KERNEL_ADDR)); ut_assertok(env_set_hex("ramdisk_addr_r", PXE_INITRD_ADDR)); ut_assertok(env_set_hex("fdt_addr_r", PXE_FDT_ADDR)); + ut_assertok(env_set_hex("fdtoverlay_addr_r", PXE_OVERLAY_ADDR)); ut_assertok(env_set("bootfile", cfg_path)); ut_assertok(env_set("pxe_timeout", "1")); @@ -737,16 +822,40 @@ static int pxe_test_label_override_norun(struct unit_test_state *uts) ut_assertok(run_commandf("sysboot host 0:0 any %x %s", PXE_LOAD_ADDR, cfg_path)); + /* Check menu output - say message is from default label */ + ut_assertok(pxe_check_menu(uts, "Booting default Linux kernel", NULL)); + /* Should boot 'local' label instead of default 'linux' */ - ut_assert_skip_to_line("3:\tLocal Boot"); - ut_assert_skip_to_line("missing environment variable: localcmd"); + ut_assert_nextline("Enter choice: 3:\tLocal Boot"); + ut_assert_nextline("missing environment variable: localcmd"); + + /* + * Localboot fails, so try default 'linux' label instead. + * Boot is minimal - just kernel/initrd, no FDT. + */ + ut_assert_nextline("Retrieving file: /vmlinuz"); + ut_assert_nextline("Retrieving file: /initrd.img"); + ut_assert_nextline("Unrecognized zImage"); - /* Test 2: Invalid override - should print error */ + /* Test 2: Invalid override - should print error before menu */ ut_assertok(env_set("pxe_label_override", "nonexistent")); ut_assertok(run_commandf("sysboot host 0:0 any %x %s", PXE_LOAD_ADDR, cfg_path)); - ut_assert_skip_to_line("Missing override pxe label: nonexistent"); + /* Check menu with error message before it */ + ut_assertok(pxe_check_menu(uts, "Booting default Linux kernel", + "Missing override pxe label: nonexistent")); + ut_assert_nextline("Enter choice: 1:\tBoot Linux"); + + /* Default label boot attempt */ + ut_assert_nextline("Retrieving file: /vmlinuz"); + ut_assert_nextline("Retrieving file: /initrd.img"); + ut_assert_nextline("append: root=/dev/sda1 quiet"); + ut_assert_nextline("Retrieving file: /dtb/board.dtb"); + ut_assert_nextline("Retrieving file: /dtb/overlay1.dtbo"); + ut_assert_nextline("Retrieving file: /dtb/overlay2.dtbo"); + ut_assert_nextline("Unrecognized zImage"); + ut_assert_console_end(); /* Clean up */ ut_assertok(env_set("pxe_label_override", NULL)); -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> The extlinux.conf parser is currently tightly coupled with the boot logic. While a no_boot flag exists in pxe_context to defer booting, files are still loaded before the flag is checked. This makes it difficult to simply parse a config and inspect the available labels without performing file operations. Add two new public APIs to enable finer-grained control: - pxe_select_label(): Select a label from a parsed menu using the menu system, without loading any files. Returns the selected pxe_label pointer. - pxe_load_label(): Load kernel/initrd/FDT for a specific label into memory, saving addresses and sizes in the pxe_context. These allow callers to: 1. Parse a config with parse_pxefile() 2. Inspect labels without loading (iterate cfg->labels) 3. Optionally select a label with pxe_select_label() 4. Optionally load files with pxe_load_label() 5. Optionally boot with pxe_do_boot() The existing high-level APIs (pxe_process(), pxe_probe() and pxe_do_boot()) continue to work unchanged for backward compatibility. The only functional change is that the kernel command-line is processed after all files are loaded, so update the tests to handle this. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_utils.c | 252 ++++++++++++++++++++++++++------------------ include/pxe_utils.h | 29 +++++ test/boot/pxe.c | 23 ++-- 3 files changed, 193 insertions(+), 111 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 9034c3d86e7..384293d9f8a 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -678,63 +678,34 @@ static int generate_localboot(struct pxe_label *label) return 0; } -/** - * label_boot() - Boot according to the contents of a pxe_label - * - * If we can't boot for any reason, we return. A successful boot never - * returns. - * - * The kernel will be stored in the location given by the 'kernel_addr_r' - * environment variable. - * - * If the label specifies an initrd file, it will be stored in the location - * given by the 'ramdisk_addr_r' environment variable. - * - * If the label specifies an 'append' line, its contents will overwrite that - * of the 'bootargs' environment variable. - * - * @ctx: PXE context - * @label: Label to process - * Returns does not return on success, otherwise returns 0 if a localboot - * label was processed, or 1 on error - */ -static int label_boot(struct pxe_context *ctx, struct pxe_label *label) +int pxe_load_label(struct pxe_context *ctx, struct pxe_label *label) { - char *kern_addr_str; + char fit_addr[200]; + const char *conf_fdt_str; ulong kern_addr = 0; ulong initrd_addr = 0; ulong initrd_size = 0; - char initrd_str[28] = ""; - char mac_str[29] = ""; - char ip_str[68] = ""; - char fit_addr[200]; - const char *conf_fdt_str; - ulong conf_fdt = 0; ulong kern_size; + ulong conf_fdt = 0; + char initrd_str[28] = ""; int ret; - label_print(label); - - label->attempted = 1; - if (label->localboot) { if (label->localboot_val >= 0) { - ret = label_localboot(label); - - if (IS_ENABLED(CONFIG_BOOTMETH_EXTLINUX_LOCALBOOT) && - ret == -ENOENT) + if (IS_ENABLED(CONFIG_BOOTMETH_EXTLINUX_LOCALBOOT)) { ret = generate_localboot(label); - if (ret) - return ret; - } else { - return 0; + if (ret) + return ret; + } } + /* negative localboot_val means skip loading */ + if (!label->kernel) + return 0; } if (!label->kernel) { - printf("No kernel given, skipping %s\n", - label->name); - return 1; + printf("No kernel given, skipping %s\n", label->name); + return -ENOENT; } if (get_relfile_envaddr(ctx, label->kernel, "kernel_addr_r", SZ_2M, @@ -742,20 +713,18 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) &kern_addr, &kern_size) < 0) { printf("Skipping %s for failure retrieving kernel\n", label->name); - return 1; + return -EIO; } /* for FIT, append the configuration identifier */ snprintf(fit_addr, sizeof(fit_addr), "%lx%s", kern_addr, label->config ? label->config : ""); - kern_addr_str = fit_addr; /* For FIT, the label can be identical to kernel one */ if (label->initrd && !strcmp(label->kernel_label, label->initrd)) { initrd_addr = kern_addr; } else if (label->initrd) { ulong size; - int ret; ret = get_relfile_envaddr(ctx, label->initrd, "ramdisk_addr_r", SZ_2M, @@ -764,12 +733,106 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) if (ret < 0) { printf("Skipping %s for failure retrieving initrd\n", label->name); - return 1; + return -EIO; } initrd_size = size; size = snprintf(initrd_str, sizeof(initrd_str), "%lx:%lx", initrd_addr, size); if (size >= sizeof(initrd_str)) + return -ENOSPC; + } + + conf_fdt_str = env_get("fdt_addr_r"); + ret = label_process_fdt(ctx, label, fit_addr, &conf_fdt_str); + if (ret) + return ret; + + if (!conf_fdt_str) + conf_fdt_str = pxe_get_fdt_fallback(label, kern_addr); + if (conf_fdt_str) + conf_fdt = hextoul(conf_fdt_str, NULL); + log_debug("conf_fdt %lx\n", conf_fdt); + + if (ctx->bflow && conf_fdt_str) + ctx->bflow->fdt_addr = conf_fdt; + + /* Save the loaded info to context */ + ctx->label = label; + ctx->kern_addr_str = strdup(fit_addr); + ctx->kern_addr = kern_addr; + ctx->kern_size = kern_size; + if (initrd_addr) { + ctx->initrd_addr = initrd_addr; + ctx->initrd_size = initrd_size; + ctx->initrd_str = strdup(initrd_str); + } + ctx->conf_fdt_str = strdup(conf_fdt_str); + ctx->conf_fdt = conf_fdt; + + log_debug("Loaded label '%s':\n", label->name); + log_debug("- kern_addr_str '%s' conf_fdt_str '%s' conf_fdt %lx\n", + ctx->kern_addr_str, ctx->conf_fdt_str, conf_fdt); + if (initrd_addr) { + log_debug("- initrd addr %lx filesize %lx str '%s'\n", + ctx->initrd_addr, ctx->initrd_size, ctx->initrd_str); + } + if (!ctx->kern_addr_str || (conf_fdt_str && !ctx->conf_fdt_str) || + (initrd_addr && !ctx->initrd_str)) { + printf("malloc fail (saving label)\n"); + return -ENOMEM; + } + + return 0; +} + +/** + * label_boot() - Boot according to the contents of a pxe_label + * + * If we can't boot for any reason, we return. A successful boot never + * returns. + * + * The kernel will be stored in the location given by the 'kernel_addr_r' + * environment variable. + * + * If the label specifies an initrd file, it will be stored in the location + * given by the 'ramdisk_addr_r' environment variable. + * + * If the label specifies an 'append' line, its contents will overwrite that + * of the 'bootargs' environment variable. + * + * @ctx: PXE context + * @label: Label to process + * Returns does not return on success, otherwise returns 0 if a localboot + * label was processed, or 1 on error + */ +static int label_boot(struct pxe_context *ctx, struct pxe_label *label) +{ + char mac_str[29] = ""; + char ip_str[68] = ""; + int ret; + + label_print(label); + + label->attempted = 1; + + if (label->localboot) { + 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; + } + } + + /* Load files if not already loaded */ + if (!ctx->label) { + ret = pxe_load_label(ctx, label); + if (ret) return 1; } @@ -815,51 +878,13 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) printf("append: %s\n", finalbootargs); } - conf_fdt_str = env_get("fdt_addr_r"); - ret = label_process_fdt(ctx, label, kern_addr_str, &conf_fdt_str); - if (ret) - return ret; - - if (!conf_fdt_str) - conf_fdt_str = pxe_get_fdt_fallback(label, kern_addr); - if (conf_fdt_str) - conf_fdt = hextoul(conf_fdt_str, NULL); - log_debug("conf_fdt %lx\n", conf_fdt); - - if (ctx->bflow && conf_fdt_str) - ctx->bflow->fdt_addr = conf_fdt; - - if (IS_ENABLED(CONFIG_BOOTSTD_FULL) && ctx->no_boot) { - ctx->label = label; - ctx->kern_addr_str = strdup(kern_addr_str); - ctx->kern_addr = kern_addr; - ctx->kern_size = kern_size; - if (initrd_addr) { - ctx->initrd_addr = initrd_addr; - ctx->initrd_size = initrd_size; - ctx->initrd_str = strdup(initrd_str); - } - ctx->conf_fdt_str = strdup(conf_fdt_str); - ctx->conf_fdt = conf_fdt; - log_debug("Saving label '%s':\n", label->name); - log_debug("- kern_addr_str '%s' conf_fdt_str '%s' conf_fdt %lx\n", - ctx->kern_addr_str, ctx->conf_fdt_str, conf_fdt); - if (initrd_addr) { - log_debug("- initrd addr %lx filesize %lx str '%s'\n", - ctx->initrd_addr, ctx->initrd_size, - ctx->initrd_str); - } - if (!ctx->kern_addr_str || (conf_fdt_str && !ctx->conf_fdt_str) || - (initrd_addr && !ctx->initrd_str)) { - printf("malloc fail (saving label)\n"); - return 1; - } + if (IS_ENABLED(CONFIG_BOOTSTD_FULL) && ctx->no_boot) return 0; - } - label_run_boot(ctx, label, kern_addr_str, kern_addr, kern_size, - initrd_addr, initrd_size, initrd_str, conf_fdt_str, - conf_fdt); + label_run_boot(ctx, label, ctx->kern_addr_str, ctx->kern_addr, + ctx->kern_size, ctx->initrd_addr, ctx->initrd_size, + ctx->initrd_str, ctx->conf_fdt_str, ctx->conf_fdt); + /* ignore the error value since we are going to fail anyway */ /* * Sandbox cannot boot a real kernel, so stop after the first attempt. @@ -1017,12 +1042,44 @@ static void boot_unattempted_labels(struct pxe_context *ctx, } } -void handle_pxe_menu(struct pxe_context *ctx, struct pxe_menu *cfg) +int pxe_select_label(struct pxe_menu *cfg, bool prompt, + struct pxe_label **labelp) { void *choice; struct menu *m; int err; + if (prompt) + cfg->prompt = 1; + + m = pxe_menu_to_menu(cfg); + if (!m) + return -ENOMEM; + + err = menu_get_choice(m, &choice); + menu_destroy(m); + + /* + * err == 1 means we got a choice back from menu_get_choice. + * + * err == -ENOENT if the menu was setup to select the default but no + * default was set. + * + * otherwise, the user interrupted or there was some other error. + */ + if (err == 1) { + *labelp = choice; + return 0; + } + + return err == -ENOENT ? -ENOENT : -ECANCELED; +} + +void handle_pxe_menu(struct pxe_context *ctx, struct pxe_menu *cfg) +{ + struct pxe_label *label; + int err; + if (IS_ENABLED(CONFIG_CMD_BMP)) { /* display BMP if available */ if (cfg->bmp) { @@ -1044,15 +1101,10 @@ void handle_pxe_menu(struct pxe_context *ctx, struct pxe_menu *cfg) } } - m = pxe_menu_to_menu(cfg); - if (!m) - return; - - err = menu_get_choice(m, &choice); - menu_destroy(m); + err = pxe_select_label(cfg, false, &label); /* - * err == 1 means we got a choice back from menu_get_choice. + * err == 0 means we got a choice back. * * err == -ENOENT if the menu was setup to select the default but no * default was set. in that case, we should continue trying to boot @@ -1062,8 +1114,8 @@ void handle_pxe_menu(struct pxe_context *ctx, struct pxe_menu *cfg) * we give up. */ - if (err == 1) { - err = label_boot(ctx, choice); + if (!err) { + err = label_boot(ctx, label); log_debug("label_boot() returns %d\n", err); if (!err) return; diff --git a/include/pxe_utils.h b/include/pxe_utils.h index 9629f051a91..ef664f075a0 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -358,6 +358,35 @@ int pxe_probe(struct pxe_context *ctx, ulong pxefile_addr_r, bool prompt); */ int pxe_do_boot(struct pxe_context *ctx); +/** + * pxe_select_label() - Select a label from a parsed menu + * + * Uses the menu system to get the user's choice or the default. + * Does NOT load any files or attempt to boot. + * + * @cfg: Parsed PXE menu + * @prompt: Force user prompt regardless of timeout + * @labelp: Returns selected label (not a copy, points into cfg) + * Return: 0 on success, -ENOMEM if out of memory, -ENOENT if no default set, + * -ECANCELED if user cancelled + */ +int pxe_select_label(struct pxe_menu *cfg, bool prompt, + struct pxe_label **labelp); + +/** + * pxe_load_label() - Load kernel/initrd/FDT for a label + * + * Loads the files specified in the label into memory and saves the + * addresses and sizes in @ctx. Call this only when ready to boot or + * inspect loaded files. + * + * @ctx: PXE context with getfile callback + * @label: Label whose files to load + * Return: 0 on success, -ENOENT if no kernel specified, -EIO if file + * retrieval failed, -ENOMEM if out of memory + */ +int pxe_load_label(struct pxe_context *ctx, struct pxe_label *label); + /* * Entry point for parsing a menu file. nest_level indicates how many times * we've nested in includes. It will be 1 for the top level menu file. diff --git a/test/boot/pxe.c b/test/boot/pxe.c index f12add77db6..db1cf48d7a5 100644 --- a/test/boot/pxe.c +++ b/test/boot/pxe.c @@ -145,11 +145,11 @@ static int pxe_check_fdtdir(struct unit_test_state *uts, const char *dtb_name) ut_assert_nextline("2:\tTest soc/board construction"); ut_assert_nextline("Enter choice: 1:\tTest fdtfile env var"); - /* Boot file retrieval */ + /* Boot file retrieval - FDT/overlays loaded before append is printed */ ut_assert_nextline("Retrieving file: /vmlinuz"); - ut_assert_nextline("append: console=ttyS0"); ut_assert_nextline("Retrieving file: /dtb/%s", dtb_name); ut_assert_nextline("Retrieving file: /dtb/overlay1.dtbo"); + ut_assert_nextline("append: console=ttyS0"); /* Boot fails on sandbox */ ut_assert_nextline("Unrecognized zImage"); @@ -375,10 +375,10 @@ static int pxe_test_sysboot_norun(struct unit_test_state *uts) /* Verify files were loaded in order */ ut_assert_nextline("Retrieving file: /vmlinuz"); ut_assert_nextline("Retrieving file: /initrd.img"); - ut_assert_nextline("append: root=/dev/sda1 quiet"); ut_assert_nextline("Retrieving file: /dtb/board.dtb"); ut_assert_nextline("Retrieving file: /dtb/overlay1.dtbo"); ut_assert_nextline("Retrieving file: /dtb/overlay2.dtbo"); + ut_assert_nextline("append: root=/dev/sda1 quiet"); /* Boot fails on sandbox */ ut_assert_nextline("Unrecognized zImage"); @@ -710,6 +710,13 @@ static int pxe_test_ipappend_norun(struct unit_test_state *uts) /* Rescue label boot attempt */ ut_assert_nextline("Retrieving file: /vmlinuz-rescue"); + /* + * Rescue label has fdtdir=/dtb/ but no fdtfile is set, so it tries + * to load /dtb/.dtb which fails. FDT is loaded before append. + */ + ut_assert_nextline("Retrieving file: /dtb/.dtb"); + ut_assert_nextline("Skipping fdtdir /dtb/ for failure retrieving dts"); + /* * Verify ipappend output - should have: * - original append: "single" @@ -719,12 +726,6 @@ static int pxe_test_ipappend_norun(struct unit_test_state *uts) ut_assert_nextlinen("append: single ip=192.168.1.10:192.168.1.1:" "192.168.1.254:255.255.255.0 BOOTIF=01-"); - /* - * Rescue label has fdtdir=/dtb/ but no fdtfile is set, so it tries - * to load /dtb/.dtb which fails. Boot still proceeds without FDT. - */ - ut_assert_nextline("Retrieving file: /dtb/.dtb"); - ut_assert_nextline("Skipping fdtdir /dtb/ for failure retrieving dts"); ut_assert_nextline("Unrecognized zImage"); ut_assert_console_end(); @@ -847,13 +848,13 @@ static int pxe_test_label_override_norun(struct unit_test_state *uts) "Missing override pxe label: nonexistent")); ut_assert_nextline("Enter choice: 1:\tBoot Linux"); - /* Default label boot attempt */ + /* Default label boot attempt - FDT/overlays loaded before append */ ut_assert_nextline("Retrieving file: /vmlinuz"); ut_assert_nextline("Retrieving file: /initrd.img"); - ut_assert_nextline("append: root=/dev/sda1 quiet"); ut_assert_nextline("Retrieving file: /dtb/board.dtb"); ut_assert_nextline("Retrieving file: /dtb/overlay1.dtbo"); ut_assert_nextline("Retrieving file: /dtb/overlay2.dtbo"); + ut_assert_nextline("append: root=/dev/sda1 quiet"); ut_assert_nextline("Unrecognized zImage"); ut_assert_console_end(); -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Add a test that verifies the behaviour when a label has fdtoverlays defined in the extlinux.conf but fdtoverlay_addr_r is not set in the environment. The expected behaviour is that the overlay loading is skipped with a warning message ("Invalid fdtoverlay_addr_r for loading overlays") but the FDT is still loaded successfully, allowing the boot to proceed without overlays. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- test/boot/pxe.c | 87 ++++++++++++++++++++++++++++++++ test/py/tests/test_pxe_parser.py | 7 +++ 2 files changed, 94 insertions(+) diff --git a/test/boot/pxe.c b/test/boot/pxe.c index 2ee2ad24a7d..46b26496fbd 100644 --- a/test/boot/pxe.c +++ b/test/boot/pxe.c @@ -609,6 +609,93 @@ PXE_TEST_ARGS(pxe_test_errors_norun, UTF_CONSOLE | UTF_MANUAL, { "fs_image", UT_ARG_STR }, { "cfg_path", UT_ARG_STR }); +/** + * Test overlay loading when fdtoverlay_addr_r is not set + * + * This tests that when a label has fdtoverlays but fdtoverlay_addr_r is not + * set, the overlay loading is skipped with an appropriate warning message, + * but the FDT is still loaded successfully. + */ +static int pxe_test_overlay_no_addr_norun(struct unit_test_state *uts) +{ + const char *fs_image = ut_str(PXE_ARG_FS_IMAGE); + const char *cfg_path = ut_str(PXE_ARG_CFG_PATH); + struct pxe_test_info info; + struct pxe_context ctx; + struct pxe_label *label; + struct pxe_menu *cfg; + ulong addr = PXE_LOAD_ADDR; + void *fdt; + uint i; + + ut_assertnonnull(fs_image); + ut_assertnonnull(cfg_path); + + info.uts = uts; + + /* Bind the filesystem image */ + ut_assertok(run_commandf("host bind 0 %s", fs_image)); + + /* Set up the PXE context */ + ut_assertok(pxe_setup_ctx(&ctx, pxe_test_getfile, &info, true, cfg_path, + false, false, NULL)); + + /* Read and parse the config file */ + ut_asserteq(1, get_pxe_file(&ctx, cfg_path, addr)); + + cfg = parse_pxefile(&ctx, addr); + ut_assertnonnull(cfg); + + /* Consume parsing output */ + ut_assert_nextline("Retrieving file: %s", cfg_path); + ut_assert_nextline("Booting default Linux kernel"); + ut_assert_nextline("Retrieving file: /extlinux/extra.conf"); + for (i = 3; i <= 16; i++) + ut_assert_nextline("Retrieving file: /extlinux/nest%d.conf", i); + ut_assert_console_end(); + + /* + * Set up environment for loading, but do NOT set fdtoverlay_addr_r. + * This should cause overlay loading to be skipped with a warning. + */ + ut_assertok(env_set_hex("kernel_addr_r", PXE_KERNEL_ADDR)); + ut_assertok(env_set_hex("ramdisk_addr_r", PXE_INITRD_ADDR)); + ut_assertok(env_set_hex("fdt_addr_r", PXE_FDT_ADDR)); + ut_assertok(env_set("fdtoverlay_addr_r", NULL)); /* Clear it */ + + /* Get the first label (linux) which has fdtoverlays */ + label = list_first_entry(&cfg->labels, struct pxe_label, list); + ut_asserteq_str("linux", label->name); + ut_assertnonnull(label->fdtoverlays); + + /* Load the label - should succeed but skip overlays */ + ut_assertok(pxe_load_label(&ctx, label)); + + /* FDT should be loaded */ + ut_asserteq(PXE_FDT_ADDR, ctx.conf_fdt); + fdt = map_sysmem(PXE_FDT_ADDR, 0); + ut_assertok(fdt_check_header(fdt)); + + /* + * Check console output - FDT loaded, but overlays skipped with + * warning about missing fdtoverlay_addr_r + */ + ut_assert_nextline("Retrieving file: /vmlinuz"); + ut_assert_nextline("Retrieving file: /initrd.img"); + ut_assert_nextline("Retrieving file: /dtb/board.dtb"); + ut_assert_nextline("Invalid fdtoverlay_addr_r for loading overlays"); + ut_assert_console_end(); + + /* Clean up */ + destroy_pxe_menu(cfg); + pxe_destroy_ctx(&ctx); + + return 0; +} +PXE_TEST_ARGS(pxe_test_overlay_no_addr_norun, UTF_CONSOLE | UTF_MANUAL, + { "fs_image", UT_ARG_STR }, + { "cfg_path", UT_ARG_STR }); + /** * Test pxe_get_file_size() function * diff --git a/test/py/tests/test_pxe_parser.py b/test/py/tests/test_pxe_parser.py index 61ff6d11e3e..b728fb86946 100644 --- a/test/py/tests/test_pxe_parser.py +++ b/test/py/tests/test_pxe_parser.py @@ -460,3 +460,10 @@ class TestPxeParser: with ubman.log.section('Test PXE alloc'): ubman.run_ut('pxe', 'pxe_test_alloc', fs_image=fs_img, cfg_path=cfg_path) + + def test_pxe_overlay_no_addr(self, ubman, pxe_image): + """Test overlay loading when fdtoverlay_addr_r is not set""" + fs_img, cfg_path = pxe_image + with ubman.log.section('Test PXE overlay no addr'): + ubman.run_ut('pxe', 'pxe_test_overlay_no_addr', + fs_image=fs_img, cfg_path=cfg_path) -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Add a 'quiet' field to struct pxe_context to allow callers to suppress the "Retrieving file" message when loading files. This is useful when using the parser API to inspect labels without booting, where the message would be noise. Normal boot flows leave quiet as false (the default) so the message is still printed. Parse-only callers can set ctx.quiet = true to suppress it. Update the PXE parser test to use quiet mode. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_utils.c | 3 ++- include/pxe_utils.h | 2 ++ test/boot/pxe.c | 12 ++++++------ 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 384293d9f8a..3c48b0ac51b 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -124,7 +124,8 @@ static int get_relfile(struct pxe_context *ctx, const char *file_path, strcat(relfile, file_path); - printf("Retrieving file: %s\n", relfile); + if (!ctx->quiet) + printf("Retrieving file: %s\n", relfile); ret = ctx->getfile(ctx, relfile, addrp, align, type, &size); if (ret < 0) diff --git a/include/pxe_utils.h b/include/pxe_utils.h index ef664f075a0..f910ea9d284 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -120,6 +120,7 @@ typedef int (*pxe_getfile_func)(struct pxe_context *ctx, const char *file_path, * @use_fallback: TRUE : use "fallback" option as default, FALSE : use * "default" option as default * @no_boot: Stop show of actually booting and just return + * @quiet: Suppress "Retrieving file" messages when loading files * @bflow: Bootflow being booted, or NULL if none (must be valid if @no_boot) * @cfg: PXE menu (NULL if not yet probed) * @@ -157,6 +158,7 @@ struct pxe_context { bool use_ipv6; bool use_fallback; bool no_boot; + bool quiet; struct bootflow *bflow; struct pxe_menu *cfg; diff --git a/test/boot/pxe.c b/test/boot/pxe.c index 4c87aafce55..af45c1fbffd 100644 --- a/test/boot/pxe.c +++ b/test/boot/pxe.c @@ -160,7 +160,8 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_assertok(pxe_setup_ctx(&ctx, pxe_test_getfile, &info, true, cfg_path, false, false, NULL)); - /* Read the config file into memory */ + /* Read the config file into memory (quiet since we're just parsing) */ + ctx.quiet = true; ret = get_pxe_file(&ctx, cfg_path, addr); ut_asserteq(1, ret); /* get_pxe_file returns 1 on success */ @@ -168,12 +169,11 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) cfg = parse_pxefile(&ctx, addr); ut_assertnonnull(cfg); - /* Verify 'say' keyword printed its message during parsing */ - ut_assert_nextline("Retrieving file: %s", cfg_path); + /* + * Verify 'say' keyword printed its message during parsing (quiet + * suppresses file messages) + */ ut_assert_nextline("Booting default Linux kernel"); - ut_assert_nextline("Retrieving file: /extlinux/extra.conf"); - for (i = 3; i <= 16; i++) - ut_assert_nextline("Retrieving file: /extlinux/nest%d.conf", i); ut_assert_console_end(); /* Verify menu properties */ -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Use a consistent naming convention with the pxe_ prefix. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_utils.c | 6 +++--- include/pxe_utils.h | 6 +++--- test/boot/pxe.c | 12 ++++++------ 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 1516065a467..a824b1b5623 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -900,7 +900,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) return 1; } -void destroy_pxe_menu(struct pxe_menu *cfg) +void pxe_menu_uninit(struct pxe_menu *cfg) { struct list_head *pos, *n; struct pxe_label *label; @@ -946,7 +946,7 @@ struct pxe_menu *parse_pxefile(struct pxe_context *ctx, unsigned long menucfg) unmap_sysmem(buf); if (r < 0) { - destroy_pxe_menu(cfg); + pxe_menu_uninit(cfg); return NULL; } @@ -1192,7 +1192,7 @@ int pxe_process(struct pxe_context *ctx, ulong pxefile_addr_r, bool prompt) handle_pxe_menu(ctx, cfg); - destroy_pxe_menu(cfg); + pxe_menu_uninit(cfg); return 0; } diff --git a/include/pxe_utils.h b/include/pxe_utils.h index c0d2167cc25..0de2f49aab5 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -179,13 +179,13 @@ struct pxe_context { }; /** - * destroy_pxe_menu() - Destroy an allocated pxe structure + * pxe_menu_uninit() - Destroy an allocated pxe structure * * Free the memory used by a pxe_menu and its labels * * @cfg: Config to destroy, previous returned from parse_pxefile() */ -void destroy_pxe_menu(struct pxe_menu *cfg); +void pxe_menu_uninit(struct pxe_menu *cfg); /** * get_pxe_file() - Read a file @@ -243,7 +243,7 @@ void handle_pxe_menu(struct pxe_context *ctx, struct pxe_menu *cfg); * Returns NULL if there is an error, otherwise, returns a pointer to a * pxe_menu struct populated with the results of parsing the pxe file (and any * files it includes). The resulting pxe_menu struct can be free()'d by using - * the destroy_pxe_menu() function. + * the pxe_menu_uninit() function. */ struct pxe_menu *parse_pxefile(struct pxe_context *ctx, ulong menucfg); diff --git a/test/boot/pxe.c b/test/boot/pxe.c index 4ac0d6c6fa6..8d7c6f7f2ae 100644 --- a/test/boot/pxe.c +++ b/test/boot/pxe.c @@ -286,7 +286,7 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_assert_console_end(); /* Clean up */ - destroy_pxe_menu(cfg); + pxe_menu_uninit(cfg); pxe_destroy_ctx(&ctx); return 0; @@ -476,7 +476,7 @@ static int pxe_test_fdtdir_norun(struct unit_test_state *uts) env_set("soc", NULL); env_set("board", NULL); - destroy_pxe_menu(cfg); + pxe_menu_uninit(cfg); pxe_destroy_ctx(&ctx); return 0; @@ -595,7 +595,7 @@ static int pxe_test_errors_norun(struct unit_test_state *uts) /* Clean up */ env_set("fdtfile", NULL); - destroy_pxe_menu(cfg); + pxe_menu_uninit(cfg); pxe_destroy_ctx(&ctx); return 0; @@ -681,7 +681,7 @@ static int pxe_test_overlay_no_addr_norun(struct unit_test_state *uts) ut_assert_console_end(); /* Clean up */ - destroy_pxe_menu(cfg); + pxe_menu_uninit(cfg); pxe_destroy_ctx(&ctx); return 0; @@ -1151,7 +1151,7 @@ static int pxe_test_alloc_norun(struct unit_test_state *uts) ut_asserteq(false, ctx.fake_go); /* Clean up */ - destroy_pxe_menu(ctx.cfg); + pxe_menu_uninit(ctx.cfg); pxe_destroy_ctx(&ctx); ut_assertok(env_set("pxe_timeout", NULL)); ut_assertok(env_set("fdt_addr", orig_fdt_addr)); @@ -1245,7 +1245,7 @@ static int pxe_test_fit_embedded_fdt_norun(struct unit_test_state *uts) ut_asserteq(0, ctx.conf_fdt); /* Clean up */ - destroy_pxe_menu(cfg); + pxe_menu_uninit(cfg); pxe_destroy_ctx(&ctx); return 0; -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Instead of loading and parsing included files inline during parsing, store them in cfg->includes and process them after the initial parse completes. This separates the parsing logic from file loading. Add struct pxe_include to store the path, target menu pointer and nesting level. The nesting level ensures the MAX_NEST_LEVEL (16) limit is properly enforced for deeply nested includes. Add pxe_parse_include() for callers who want manual control over include processing. The parse_pxefile() function processes includes automatically to maintain backwards compatibility with existing callers. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_parse.c | 38 ++++++++++----------------- boot/pxe_utils.c | 62 +++++++++++++++++++++++++++++++++++++++++---- include/pxe_utils.h | 58 +++++++++++++++++++++++++++++++++++++----- test/boot/pxe.c | 19 +++++++++----- 4 files changed, 135 insertions(+), 42 deletions(-) diff --git a/boot/pxe_parse.c b/boot/pxe_parse.c index e3412166a63..72a672354b5 100644 --- a/boot/pxe_parse.c +++ b/boot/pxe_parse.c @@ -326,39 +326,31 @@ static int parse_integer(char **c, int *dst) } /* - * Parse an include statement, and retrieve and parse the file it mentions. + * Parse an include statement and store the path for later loading. * - * base should point to a location where it's safe to store the file, and - * nest_level should indicate how many nested includes have occurred. For this - * include, nest_level has already been incremented and doesn't need to be - * incremented here. + * The include is added to cfg->includes. The caller is responsible for + * loading these files and calling pxe_parse_include() to parse them. */ -static int handle_include(struct pxe_context *ctx, char **c, unsigned long base, - struct pxe_menu *cfg, int nest_level) +static int handle_include(char **c, struct pxe_menu *cfg, int nest_level) { - char *include_path; + struct pxe_include inc; char *s = *c; int err; - char *buf; - int ret; - err = parse_sliteral(c, &include_path); + err = parse_sliteral(c, &inc.path); if (err < 0) { printf("Expected include path: %.*s\n", (int)(*c - s), s); return err; } + inc.cfg = cfg; + inc.nest_level = nest_level + 1; - err = get_pxe_file(ctx, include_path, base); - if (err < 0) { - printf("Couldn't retrieve %s\n", include_path); - return err; + if (!alist_add(&cfg->includes, inc)) { + free(inc.path); + return -ENOMEM; } - buf = map_sysmem(base, 0); - ret = parse_pxefile_top(ctx, buf, base, cfg, nest_level); - unmap_sysmem(buf); - - return ret; + return 1; } /* @@ -385,7 +377,7 @@ static int parse_menu(struct pxe_context *ctx, char **c, struct pxe_menu *cfg, err = parse_sliteral(c, &cfg->title); break; case T_INCLUDE: - err = handle_include(ctx, c, base, cfg, nest_level + 1); + err = handle_include(c, cfg, nest_level); break; case T_BACKGROUND: err = parse_sliteral(c, &cfg->bmp); @@ -635,9 +627,7 @@ int parse_pxefile_top(struct pxe_context *ctx, char *p, ulong base, } break; case T_INCLUDE: - err = handle_include(ctx, &p, - base + ALIGN(strlen(b), 4), cfg, - nest_level + 1); + err = handle_include(&p, cfg, nest_level); break; case T_PROMPT: err = parse_integer(&p, &cfg->prompt); diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index e0b7faddbd0..410f2d1eafe 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -910,12 +910,14 @@ struct pxe_menu *pxe_menu_init(void) memset(cfg, '\0', sizeof(struct pxe_menu)); INIT_LIST_HEAD(&cfg->labels); + alist_init(&cfg->includes, sizeof(struct pxe_include), 0); return cfg; } void pxe_menu_uninit(struct pxe_menu *cfg) { + struct pxe_include *inc; struct list_head *pos, *n; struct pxe_label *label; @@ -929,6 +931,10 @@ void pxe_menu_uninit(struct pxe_menu *cfg) label_destroy(label); } + alist_for_each(inc, &cfg->includes) + free(inc->path); + alist_uninit(&cfg->includes); + free(cfg); } @@ -944,6 +950,12 @@ struct pxe_menu *parse_pxefile(struct pxe_context *ctx, unsigned long menucfg) buf = map_sysmem(menucfg, 0); r = parse_pxefile_top(ctx, buf, menucfg, cfg, 1); + unmap_sysmem(buf); + + if (r < 0) { + pxe_menu_uninit(cfg); + return NULL; + } if (ctx->use_fallback) { if (cfg->fallback_label) { @@ -954,13 +966,46 @@ struct pxe_menu *parse_pxefile(struct pxe_context *ctx, unsigned long menucfg) } } - unmap_sysmem(buf); - if (r < 0) { - pxe_menu_uninit(cfg); - return NULL; + return cfg; +} + +int pxe_process_includes(struct pxe_context *ctx, struct pxe_menu *cfg, + ulong base) +{ + struct pxe_include *inc; + char *buf; + uint i; + int r; + + /* + * Process includes - load each file and parse it. Get the include + * fresh each iteration since parsing may add more includes and cause + * alist reallocation. + */ + for (i = 0; i < cfg->includes.count; i++) { + inc = alist_getw(&cfg->includes, i, struct pxe_include); + + r = get_pxe_file(ctx, inc->path, base); + if (r < 0) { + printf("Couldn't retrieve %s\n", inc->path); + return r; + } + + buf = map_sysmem(base, 0); + r = pxe_parse_include(ctx, inc, buf, base); + unmap_sysmem(buf); + + if (r < 0) + return r; } - return cfg; + return 0; +} + +int pxe_parse_include(struct pxe_context *ctx, struct pxe_include *inc, + char *buf, ulong base) +{ + return parse_pxefile_top(ctx, buf, base, inc->cfg, inc->nest_level); } /* @@ -1179,6 +1224,7 @@ struct pxe_menu *pxe_prepare(struct pxe_context *ctx, ulong pxefile_addr_r, bool prompt) { struct pxe_menu *cfg; + int ret; cfg = parse_pxefile(ctx, pxefile_addr_r); if (!cfg) { @@ -1186,6 +1232,12 @@ struct pxe_menu *pxe_prepare(struct pxe_context *ctx, ulong pxefile_addr_r, return NULL; } + ret = pxe_process_includes(ctx, cfg, pxefile_addr_r); + if (ret) { + pxe_menu_uninit(cfg); + return NULL; + } + if (prompt) cfg->prompt = prompt; diff --git a/include/pxe_utils.h b/include/pxe_utils.h index 0ea250300d6..7f5b8c040d6 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -3,6 +3,7 @@ #ifndef __PXE_UTILS_H #define __PXE_UTILS_H +#include <alist.h> #include <bootflow.h> #include <linux/list.h> @@ -68,6 +69,19 @@ struct pxe_label { struct list_head list; }; +/** + * struct pxe_include - an include file that needs to be loaded + * + * @path: Path to the include file + * @cfg: Menu to parse the include into + * @nest_level: Nesting level to use when parsing this include + */ +struct pxe_include { + char *path; + struct pxe_menu *cfg; + int nest_level; +}; + /* * Describes a pxe menu as given via pxe files. * @@ -81,6 +95,7 @@ struct pxe_label { * interrupted. If 1, always prompt for a choice regardless of * timeout. * labels - a list of labels defined for the menu. + * includes - list of struct pxe_include for files that need loading/parsing */ struct pxe_menu { char *title; @@ -90,6 +105,7 @@ struct pxe_menu { int timeout; int prompt; struct list_head labels; + struct alist includes; }; struct pxe_context; @@ -242,18 +258,48 @@ int get_pxelinux_path(struct pxe_context *ctx, const char *file, void handle_pxe_menu(struct pxe_context *ctx, struct pxe_menu *cfg); /** - * parse_pxefile() - Parsing a pxe file + * parse_pxefile() - Parse a pxe file * - * This is only used for the top-level file. + * Parse the top-level file. Any includes are stored in cfg->includes and + * should be processed by calling pxe_process_includes(). * * @ctx: PXE context (provided by the caller) - * Returns NULL if there is an error, otherwise, returns a pointer to a - * pxe_menu struct populated with the results of parsing the pxe file (and any - * files it includes). The resulting pxe_menu struct can be free()'d by using - * the pxe_menu_uninit() function. + * @menucfg: Address of the PXE file in memory + * Return: NULL on error, otherwise a pointer to a pxe_menu struct. Use + * pxe_menu_uninit() to free it. */ struct pxe_menu *parse_pxefile(struct pxe_context *ctx, ulong menucfg); +/** + * pxe_process_includes() - Process include files in a parsed menu + * + * Load and parse all include files referenced in cfg->includes. This may + * add more includes if nested includes are found. + * + * @ctx: PXE context with getfile callback + * @cfg: Parsed PXE menu with includes to process + * @base: Memory address for loading include files + * Return: 0 on success, -ve on error + */ +int pxe_process_includes(struct pxe_context *ctx, struct pxe_menu *cfg, + ulong base); + +/** + * pxe_parse_include() - Parse an included file into its target menu + * + * After loading an include file referenced in cfg->includes, call this + * to parse it and merge any labels into the target menu. This may add + * more entries to cfg->includes if the included file has its own includes. + * + * @ctx: PXE context + * @inc: Include info with path and target menu + * @buf: Buffer containing the included file content + * @base: Memory address where buf is located + * Return: 1 on success, -ve on error + */ +int pxe_parse_include(struct pxe_context *ctx, struct pxe_include *inc, + char *buf, ulong base); + /** * format_mac_pxe() - Convert a MAC address to PXE format * diff --git a/test/boot/pxe.c b/test/boot/pxe.c index 8d7c6f7f2ae..365aff9f37a 100644 --- a/test/boot/pxe.c +++ b/test/boot/pxe.c @@ -164,6 +164,9 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) cfg = parse_pxefile(&ctx, addr); ut_assertnonnull(cfg); + /* Process any include files */ + ut_assertok(pxe_process_includes(&ctx, cfg, addr)); + /* Verify no console output during parsing (say is printed on boot) */ ut_assert_console_end(); @@ -621,7 +624,6 @@ static int pxe_test_overlay_no_addr_norun(struct unit_test_state *uts) struct pxe_menu *cfg; ulong addr = PXE_LOAD_ADDR; void *fdt; - uint i; ut_assertnonnull(fs_image); ut_assertnonnull(cfg_path); @@ -635,17 +637,17 @@ static int pxe_test_overlay_no_addr_norun(struct unit_test_state *uts) ut_assertok(pxe_setup_ctx(&ctx, pxe_test_getfile, &info, true, cfg_path, false, false, NULL)); - /* Read and parse the config file */ + /* Read and parse the config file (quiet since we're just parsing) */ + ctx.quiet = true; ut_asserteq(1, get_pxe_file(&ctx, cfg_path, addr)); cfg = parse_pxefile(&ctx, addr); ut_assertnonnull(cfg); - /* Consume parsing output (say message is printed on boot, not parsing) */ - ut_assert_nextline("Retrieving file: %s", cfg_path); - ut_assert_nextline("Retrieving file: /extlinux/extra.conf"); - for (i = 3; i <= 16; i++) - ut_assert_nextline("Retrieving file: /extlinux/nest%d.conf", i); + /* Process any include files */ + ut_assertok(pxe_process_includes(&ctx, cfg, addr)); + + /* Verify no console output during parsing (say is printed on boot) */ ut_assert_console_end(); /* @@ -662,6 +664,9 @@ static int pxe_test_overlay_no_addr_norun(struct unit_test_state *uts) ut_asserteq_str("linux", label->name); ut_assertnonnull(label->fdtoverlays); + /* Enable output for loading phase */ + ctx.quiet = false; + /* Load the label - should succeed but skip overlays */ ut_assertok(pxe_load_label(&ctx, label)); -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Separate the logic that determines the FDT filename into a new label_get_fdt_path() function. This handles both the label->fdt case and the label->fdtdir case where the filename is constructed from environment variables. The new function returns an allocated string that the caller must free. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_utils.c | 148 +++++++++++++++++++++++++++-------------------- 1 file changed, 85 insertions(+), 63 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 55b738e5c45..0994e7e5196 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -428,6 +428,84 @@ const char *pxe_get_fdt_fallback(struct pxe_label *label, ulong kern_addr) return conf_fdt_str; } +/** + * label_get_fdt_path() - Get the FDT path for a label + * + * Determine the FDT filename from label->fdt or by constructing it from + * label->fdtdir and environment variables. + * + * @label: Label to get FDT path for + * @fdtfilep: Returns allocated FDT path, or NULL if none. Caller must free. + * Return: 0 on success, -ENOMEM on allocation failure + */ +static int label_get_fdt_path(struct pxe_label *label, char **fdtfilep) +{ + char *fdtfile = NULL; + + if (label->fdt) { + if (IS_ENABLED(CONFIG_SUPPORT_PASSING_ATAGS)) { + if (strcmp("-", label->fdt)) + fdtfile = strdup(label->fdt); + } else { + fdtfile = strdup(label->fdt); + } + } else if (label->fdtdir) { + char *f1, *f2, *f3, *f4, *slash; + int len; + + f1 = env_get("fdtfile"); + if (f1) { + f2 = ""; + f3 = ""; + f4 = ""; + } else { + /* + * For complex cases where this code doesn't + * generate the correct filename, the board + * code should set $fdtfile during early boot, + * or the boot scripts should set $fdtfile + * before invoking "pxe" or "sysboot". + */ + f1 = env_get("soc"); + f2 = "-"; + f3 = env_get("board"); + f4 = ".dtb"; + if (!f1) { + f1 = ""; + f2 = ""; + } + if (!f3) { + f2 = ""; + f3 = ""; + } + } + + len = strlen(label->fdtdir); + if (!len) + slash = "./"; + else if (label->fdtdir[len - 1] != '/') + slash = "/"; + else + slash = ""; + + len = strlen(label->fdtdir) + strlen(slash) + + strlen(f1) + strlen(f2) + strlen(f3) + + strlen(f4) + 1; + fdtfile = malloc(len); + if (!fdtfile) { + printf("malloc fail (FDT filename)\n"); + return -ENOMEM; + } + + snprintf(fdtfile, len, "%s%s%s%s%s%s", + label->fdtdir, slash, f1, f2, f3, f4); + } + + *fdtfilep = fdtfile; + + return 0; +} + /* * label_process_fdt() - Process FDT for the label * @@ -459,6 +537,9 @@ const char *pxe_get_fdt_fallback(struct pxe_label *label, ulong kern_addr) static int label_process_fdt(struct pxe_context *ctx, struct pxe_label *label, char *kernel_addr, const char **fdt_argp) { + char *fdtfile; + int ret; + log_debug("label '%s' kernel_addr '%s' label->fdt '%s' fdtdir '%s' " "kernel_label '%s' fdt_argp '%s'\n", label->name, kernel_addr, label->fdt, label->fdtdir, @@ -469,68 +550,9 @@ static int label_process_fdt(struct pxe_context *ctx, struct pxe_label *label, *fdt_argp = kernel_addr; /* if fdt label is defined then get fdt from server */ } else if (*fdt_argp) { - char *fdtfile = NULL; - char *fdtfilefree = NULL; - - if (label->fdt) { - if (IS_ENABLED(CONFIG_SUPPORT_PASSING_ATAGS)) { - if (strcmp("-", label->fdt)) - fdtfile = label->fdt; - } else { - fdtfile = label->fdt; - } - } else if (label->fdtdir) { - char *f1, *f2, *f3, *f4, *slash; - int len; - - f1 = env_get("fdtfile"); - if (f1) { - f2 = ""; - f3 = ""; - f4 = ""; - } else { - /* - * For complex cases where this code doesn't - * generate the correct filename, the board - * code should set $fdtfile during early boot, - * or the boot scripts should set $fdtfile - * before invoking "pxe" or "sysboot". - */ - f1 = env_get("soc"); - f2 = "-"; - f3 = env_get("board"); - f4 = ".dtb"; - if (!f1) { - f1 = ""; - f2 = ""; - } - if (!f3) { - f2 = ""; - f3 = ""; - } - } - - len = strlen(label->fdtdir); - if (!len) - slash = "./"; - else if (label->fdtdir[len - 1] != '/') - slash = "/"; - else - slash = ""; - - len = strlen(label->fdtdir) + strlen(slash) + - strlen(f1) + strlen(f2) + strlen(f3) + - strlen(f4) + 1; - fdtfilefree = malloc(len); - if (!fdtfilefree) { - printf("malloc fail (FDT filename)\n"); - return -ENOMEM; - } - - snprintf(fdtfilefree, len, "%s%s%s%s%s%s", - label->fdtdir, slash, f1, f2, f3, f4); - fdtfile = fdtfilefree; - } + ret = label_get_fdt_path(label, &fdtfile); + if (ret) + return ret; if (fdtfile) { ulong addr; @@ -541,7 +563,7 @@ static int label_process_fdt(struct pxe_context *ctx, struct pxe_label *label, (enum bootflow_img_t)IH_TYPE_FLATDT, &addr, NULL); - free(fdtfilefree); + free(fdtfile); if (err < 0) { *fdt_argp = NULL; -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> The label_process_fdt() function handles both FIT images (where the FDT is embedded in the FIT) and separate FDT files. Move the FIT detection to pxe_load_label() to simplify label_process_fdt() In the FIT case, the label's fdt field matches kernel_label, indicating the FDT comes from the FIT image rather than a separate file. When this is detected, use the fit_addr string (which includes the FIT configuration) directly as conf_fdt_str This allows label_process_fdt() to focus solely on loading separate FDT files. Rename it to label_load_fdt() to reflect this. Also move the log_debug() and scenario documentation to the caller where more context is available. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_utils.c | 83 ++++++++++++++++++++++++++---------------------- 1 file changed, 45 insertions(+), 38 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 93e00b6f97e..0e335cb2fdd 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -499,50 +499,31 @@ static int label_get_fdt_path(struct pxe_label *label, char **fdtfilep) return 0; } -/* - * label_process_fdt() - Process FDT for the label - * - * @ctx: PXE context - * @label: Label to process - * @kernel_addr: String containing kernel address - * @fdt_argp: bootm argument to fill in, for FDT - * Return: 0 if OK, -ENOMEM if out of memory, -ENOENT if FDT file could not be - * loaded - * - * fdt usage is optional: - * It handles the following scenarios. - * - * Scenario 1: If fdt_addr_r specified and "fdt" or "fdtdir" label is - * defined in pxe file, retrieve fdt blob from server. Pass fdt_addr_r to - * bootm, and adjust argc appropriately. - * - * If retrieve fails and no exact fdt blob is specified in pxe file with - * "fdt" label, try Scenario 2. +/** + * label_load_fdt() - Load FDT file for the label * - * Scenario 2: If there is an fdt_addr specified, pass it along to - * bootm, and adjust argc appropriately. + * If "fdt" or "fdtdir" is defined in the pxe file, retrieve the FDT blob + * from the server. This is only called when fdt_addr_r is set in the + * environment. * - * Scenario 3: If there is an fdtcontroladdr specified, pass it along to - * bootm, and adjust argc appropriately, unless the image type is fitImage. + * If retrieval fails and an exact FDT file is specified with "fdt", an + * error is returned. If "fdtdir" is used and retrieval fails, this returns + * success and allows the caller to use fallback options. * - * Scenario 4: fdt blob is not available. + * @ctx: PXE context + * @label: Label to process + * @fdt_argp: Updated to NULL if FDT retrieval fails + * Return: 0 if OK, -ENOMEM if out of memory, -ENOENT if FDT file specified + * by "fdt" label could not be loaded */ -static int label_process_fdt(struct pxe_context *ctx, struct pxe_label *label, - char *kernel_addr, const char **fdt_argp) +static int label_load_fdt(struct pxe_context *ctx, struct pxe_label *label, + const char **fdt_argp) { char *fdtfile; int ret; - log_debug("label '%s' kernel_addr '%s' label->fdt '%s' fdtdir '%s' " - "kernel_label '%s' fdt_argp '%s'\n", - label->name, kernel_addr, label->fdt, label->fdtdir, - label->kernel_label, *fdt_argp); - - /* For FIT, the label can be identical to kernel one */ - if (label->fdt && !strcmp(label->kernel_label, label->fdt)) { - *fdt_argp = kernel_addr; /* if fdt label is defined then get fdt from server */ - } else if (*fdt_argp) { + if (*fdt_argp) { ret = label_get_fdt_path(label, &fdtfile); if (ret) return ret; @@ -769,11 +750,37 @@ int pxe_load_label(struct pxe_context *ctx, struct pxe_label *label) return -ENOSPC; } + /* + * FDT handling has several scenarios: + * + * 1. FIT image with embedded FDT: label->fdt matches kernel_label, + * use the FIT address so bootm extracts the FDT from the FIT + * + * 2. Separate FDT file: if fdt_addr_r is set and "fdt" or "fdtdir" + * is specified, load the FDT from the server + * + * 3. Fallback to fdt_addr env var if set + * + * 4. Fallback to fdtcontroladdr for non-FIT images + * + * 5. No FDT available + */ conf_fdt_str = env_get("fdt_addr_r"); - ret = label_process_fdt(ctx, label, fit_addr, &conf_fdt_str); - if (ret) - return ret; + log_debug("label '%s' kernel_addr '%s' label->fdt '%s' fdtdir '%s' kernel_label '%s' fdt_argp '%s'\n", + label->name, fit_addr, label->fdt, label->fdtdir, + label->kernel_label, conf_fdt_str); + + /* Scenario 1: FIT with embedded FDT */ + if (label->fdt && !strcmp(label->kernel_label, label->fdt)) { + conf_fdt_str = fit_addr; + } else { + /* Scenario 2: load FDT file if fdt_addr_r is set */ + ret = label_load_fdt(ctx, label, &conf_fdt_str); + if (ret) + return ret; + } + /* Scenarios 3 and 4: fallback options */ if (!conf_fdt_str) conf_fdt_str = pxe_get_fdt_fallback(label, ctx->kern_addr); if (conf_fdt_str) -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Return early if no FDT file is specified, rather than using an else branch. This flattens the code and makes the control flow clearer. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_utils.c | 54 ++++++++++++++++++++++++------------------------ 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index f6ad3f905b9..c0ef524f3f1 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -526,39 +526,39 @@ static int label_load_fdt(struct pxe_context *ctx, struct pxe_label *label, if (ret) return ret; - if (fdtfile) { - ret = get_relfile_envaddr(ctx, fdtfile, "fdt_addr_r", SZ_4K, - (enum bootflow_img_t)IH_TYPE_FLATDT, - &addr, NULL); - - free(fdtfile); - if (ret < 0) { - *fdt_argp = NULL; - - if (label->fdt) { - printf("Skipping %s for failure retrieving FDT\n", - label->name); - return -ENOENT; - } + if (!fdtfile) { + *fdt_argp = NULL; + return 0; + } - if (label->fdtdir) { - printf("Skipping fdtdir %s for failure retrieving dts\n", - label->fdtdir); - } - } else { - ctx->fdt = map_sysmem(addr, 0); + ret = get_relfile_envaddr(ctx, fdtfile, "fdt_addr_r", SZ_4K, + (enum bootflow_img_t)IH_TYPE_FLATDT, + &addr, NULL); + free(fdtfile); + if (ret < 0) { + *fdt_argp = NULL; - if (label->kaslrseed) - label_boot_kaslrseed(ctx); + if (label->fdt) { + printf("Skipping %s for failure retrieving FDT\n", + label->name); + return -ENOENT; + } - if (IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY) && - label->fdtoverlays) - label_boot_fdtoverlay(ctx, label); + if (label->fdtdir) { + printf("Skipping fdtdir %s for failure retrieving dts\n", + label->fdtdir); } - } else { - *fdt_argp = NULL; + return 0; } + ctx->fdt = map_sysmem(addr, 0); + + if (label->kaslrseed) + label_boot_kaslrseed(ctx); + + if (IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY) && label->fdtoverlays) + label_boot_fdtoverlay(ctx, label); + return 0; } /** -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Move the FDT path lookup from label_process_fdt() to pxe_load_label(), calling it early just after determining if this is a FIT boot. This groups related path-resolution logic together in the caller. Update label_process_fdt() to take the fdtfile as a parameter instead of calling label_get_fdt_path() internally. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_utils.c | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 05102914717..4cb14783c77 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -506,33 +506,24 @@ static int label_get_fdt_path(struct pxe_label *label, char **fdtfilep) * * @ctx: PXE context * @label: Label to process - * @fdt_argp: Updated to NULL if FDT retrieval fails or no FDT is specified + * @fdtfile: FDT file path to load (freed by this function), or NULL * Return: 0 if OK, -ENOMEM if out of memory, -ENOENT if FDT file specified * by "fdt" label could not be loaded */ static int label_load_fdt(struct pxe_context *ctx, struct pxe_label *label, - const char **fdt_argp) + char *fdtfile) { - char *fdtfile; ulong addr; int ret; - ret = label_get_fdt_path(label, &fdtfile); - if (ret) - return ret; - - if (!fdtfile) { - *fdt_argp = NULL; + if (!fdtfile) return 0; - } ret = get_relfile_envaddr(ctx, fdtfile, "fdt_addr_r", SZ_4K, (enum bootflow_img_t)IH_TYPE_FLATDT, &addr, NULL); free(fdtfile); if (ret < 0) { - *fdt_argp = NULL; - if (label->fdt) { printf("Skipping %s for failure retrieving FDT\n", label->name); @@ -704,6 +695,8 @@ int pxe_load_label(struct pxe_context *ctx, struct pxe_label *label) const char *conf_fdt_str; ulong conf_fdt = 0; char initrd_str[28] = ""; + char *fdtfile = NULL; + bool is_fit; int ret; if (label->localboot) { @@ -719,9 +712,21 @@ int pxe_load_label(struct pxe_context *ctx, struct pxe_label *label) return 0; } + /* Check for FIT case: FDT comes from FIT image, not a separate file */ + is_fit = label->fdt && label->kernel_label && + !strcmp(label->kernel_label, label->fdt); + + if (!is_fit) { + ret = label_get_fdt_path(label, &fdtfile); + if (ret) + return ret; + } + ret = pxe_load_files(ctx, label); - if (ret) + if (ret) { + free(fdtfile); return ret; + } /* for FIT, append the configuration identifier */ snprintf(fit_addr, sizeof(fit_addr), "%lx%s", ctx->kern_addr, @@ -757,13 +762,17 @@ int pxe_load_label(struct pxe_context *ctx, struct pxe_label *label) label->kernel_label, conf_fdt_str); /* Scenario 1: FIT with embedded FDT */ - if (label->fdt && !strcmp(label->kernel_label, label->fdt)) { + if (is_fit) { conf_fdt_str = fit_addr; - } else if (conf_fdt_str) { + } else if (fdtfile) { /* Scenario 2: load FDT file if fdt_addr_r is set */ - ret = label_load_fdt(ctx, label, &conf_fdt_str); + ret = label_load_fdt(ctx, label, fdtfile); if (ret) return ret; + conf_fdt_str = env_get("fdt_addr_r"); + } else { + /* No FDT specified, use fallback */ + conf_fdt_str = NULL; } /* Scenarios 3 and 4: fallback options */ -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Replace #ifdef CONFIG_OF_LIBFDT_OVERLAY with IS_ENABLED() to allow the compiler to check the code even when the option is disabled. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_utils.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 2b1fe45c86d..2084bb3e9bf 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -303,7 +303,6 @@ static void label_boot_kaslrseed(struct pxe_context *ctx) * @ctx: PXE context * @label: Label to process */ -#ifdef CONFIG_OF_LIBFDT_OVERLAY static void label_boot_fdtoverlay(struct pxe_context *ctx, struct pxe_label *label) { @@ -383,7 +382,6 @@ skip_overlay: free(overlayfile); } while ((fdtoverlay = strstr(fdtoverlay, " "))); } -#endif const char *pxe_get_fdt_fallback(struct pxe_label *label, ulong kern_addr) { -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Change fdtoverlays from a space-separated string to an alist of individual paths. This moves the string parsing from boot time to config-file parsing time, simplifying label_boot_fdtoverlay(). The alist is initialised in label_create() and each overlay path is added during parsing by the new parse_fdtoverlays() function. The paths are freed in label_destroy(). Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_parse.c | 53 ++++++++++++++++++++++++++++++++++++++++++--- boot/pxe_utils.c | 50 ++++++++++-------------------------------- include/pxe_utils.h | 4 ++-- test/boot/pxe.c | 18 +++++++++------ 4 files changed, 75 insertions(+), 50 deletions(-) diff --git a/boot/pxe_parse.c b/boot/pxe_parse.c index 72a672354b5..b115fb413a2 100644 --- a/boot/pxe_parse.c +++ b/boot/pxe_parse.c @@ -106,12 +106,15 @@ static struct pxe_label *label_create(void) if (!label) return NULL; memset(label, 0, sizeof(struct pxe_label)); + alist_init_struct(&label->fdtoverlays, char *); return label; } void label_destroy(struct pxe_label *label) { + char **overlayp; + free(label->name); free(label->kernel_label); free(label->kernel); @@ -120,7 +123,9 @@ void label_destroy(struct pxe_label *label) free(label->initrd); free(label->fdt); free(label->fdtdir); - free(label->fdtoverlays); + alist_for_each(overlayp, &label->fdtoverlays) + free(*overlayp); + alist_uninit(&label->fdtoverlays); free(label->say); free(label); } @@ -305,6 +310,48 @@ static int parse_sliteral(char **c, char **dst) return 1; } +/* + * Parse a space-separated list of overlay paths into an alist. + */ +static int parse_fdtoverlays(char **c, struct alist *overlays) +{ + char *val; + int err; + + err = parse_sliteral(c, &val); + if (err < 0) + return err; + + while (*val) { + char *path; + char *end; + + /* Skip leading spaces */ + while (*val == ' ') + val++; + + if (!*val) + break; + + /* Find end of this path */ + end = strchr(val, ' '); + if (end) { + path = strndup(val, end - val); + val = end; + } else { + path = strdup(val); + val += strlen(val); + } + + if (!path || !alist_add(overlays, path)) { + free(path); + return -ENOMEM; + } + } + + return 1; +} + /* * Parse a base 10 (unsigned) integer and store it at *dst. */ @@ -527,8 +574,8 @@ static int parse_label(char **c, struct pxe_menu *cfg) err = parse_sliteral(c, &label->fdtdir); break; case T_FDTOVERLAYS: - if (!label->fdtoverlays) - err = parse_sliteral(c, &label->fdtoverlays); + if (!label->fdtoverlays.count) + err = parse_fdtoverlays(c, &label->fdtoverlays); break; case T_LOCALBOOT: label->localboot = 1; diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index b269ef13f5a..e368b15e500 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -306,37 +306,18 @@ static void label_boot_kaslrseed(struct pxe_context *ctx) static void label_boot_fdtoverlay(struct pxe_context *ctx, struct pxe_label *label) { - char *fdtoverlay = label->fdtoverlays; + struct fdt_header *blob; + char **overlayp; + ulong addr; int err; err = fdt_check_header(ctx->fdt); if (err) return; - /* Cycle over the overlay files and apply them in order */ - do { - struct fdt_header *blob; - char *overlayfile; - ulong addr; - char *end; - int len; - - /* Drop leading spaces */ - while (*fdtoverlay == ' ') - ++fdtoverlay; - - /* Copy a single filename if multiple provided */ - end = strstr(fdtoverlay, " "); - if (end) { - len = (int)(end - fdtoverlay); - overlayfile = malloc(len + 1); - strncpy(overlayfile, fdtoverlay, len); - overlayfile[len] = '\0'; - } else - overlayfile = fdtoverlay; - - if (!strlen(overlayfile)) - goto skip_overlay; + /* Apply each overlay file in order */ + alist_for_each(overlayp, &label->fdtoverlays) { + const char *overlayfile = *overlayp; /* Load overlay file */ err = get_relfile_envaddr(ctx, overlayfile, "fdtoverlay_addr_r", @@ -345,7 +326,7 @@ static void label_boot_fdtoverlay(struct pxe_context *ctx, &addr, NULL); if (err < 0) { printf("Failed loading overlay %s\n", overlayfile); - goto skip_overlay; + continue; } /* Resize main fdt */ @@ -354,22 +335,15 @@ static void label_boot_fdtoverlay(struct pxe_context *ctx, blob = map_sysmem(addr, 0); err = fdt_check_header(blob); if (err) { - printf("Invalid overlay %s, skipping\n", - overlayfile); - goto skip_overlay; + printf("Invalid overlay %s, skipping\n", overlayfile); + continue; } err = fdt_overlay_apply_verbose(ctx->fdt, blob); - if (err) { + if (err) printf("Failed to apply overlay %s, skipping\n", overlayfile); - goto skip_overlay; - } - -skip_overlay: - if (end) - free(overlayfile); - } while ((fdtoverlay = strstr(fdtoverlay, " "))); + } } const char *pxe_get_fdt_fallback(struct pxe_label *label, ulong kern_addr) @@ -501,7 +475,7 @@ static int label_process_fdt(struct pxe_context *ctx, struct pxe_label *label) if (label->kaslrseed) label_boot_kaslrseed(ctx); - if (IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY) && label->fdtoverlays) + if (IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY) && label->fdtoverlays.count) label_boot_fdtoverlay(ctx, label); return 0; diff --git a/include/pxe_utils.h b/include/pxe_utils.h index ec7f12cd7bf..52a520e4823 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -39,7 +39,7 @@ * @initrd: path to the initrd to use for this label. * @fdt: path to FDT to use * @fdtdir: path to FDT directory to use - * @fdtoverlays: space-separated list of paths of FDT overlays to apply + * @fdtoverlays: list of paths of FDT overlays to apply (alist of char *) * @say: message to print when this label is selected for booting * @ipappend: flags for appending IP address (0x1) and MAC address (0x3) * @attempted: 0 if we haven't tried to boot this label, 1 if we have @@ -59,7 +59,7 @@ struct pxe_label { char *initrd; char *fdt; char *fdtdir; - char *fdtoverlays; + struct alist fdtoverlays; char *say; int ipappend; int attempted; diff --git a/test/boot/pxe.c b/test/boot/pxe.c index 7132d318c56..47cfa4042f5 100644 --- a/test/boot/pxe.c +++ b/test/boot/pxe.c @@ -190,8 +190,11 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_asserteq_str("/initrd.img", label->initrd); ut_asserteq_str("/dtb/board.dtb", label->fdt); ut_assertnull(label->fdtdir); - ut_asserteq_str("/dtb/overlay1.dtbo /dtb/overlay2.dtbo", - label->fdtoverlays); + ut_asserteq(2, label->fdtoverlays.count); + ut_asserteq_str("/dtb/overlay1.dtbo", + *alist_get(&label->fdtoverlays, 0, char *)); + ut_asserteq_str("/dtb/overlay2.dtbo", + *alist_get(&label->fdtoverlays, 1, char *)); ut_asserteq_str("Booting default Linux kernel", label->say); ut_asserteq(0, label->ipappend); ut_asserteq(0, label->attempted); @@ -211,7 +214,7 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_assertnull(label->initrd); ut_assertnull(label->fdt); ut_asserteq_str("/dtb/", label->fdtdir); - ut_assertnull(label->fdtoverlays); + ut_asserteq(0, label->fdtoverlays.count); ut_assertnull(label->say); ut_asserteq(3, label->ipappend); ut_asserteq(0, label->attempted); @@ -231,7 +234,7 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_assertnull(label->initrd); ut_assertnull(label->fdt); ut_assertnull(label->fdtdir); - ut_assertnull(label->fdtoverlays); + ut_asserteq(0, label->fdtoverlays.count); ut_assertnull(label->say); ut_asserteq(0, label->ipappend); ut_asserteq(0, label->attempted); @@ -251,7 +254,7 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_assertnull(label->initrd); ut_assertnull(label->fdt); ut_assertnull(label->fdtdir); - ut_assertnull(label->fdtoverlays); + ut_asserteq(0, label->fdtoverlays.count); ut_assertnull(label->say); ut_asserteq(0, label->ipappend); ut_asserteq(0, label->attempted); @@ -271,7 +274,8 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_assertnull(label->initrd); ut_assertnull(label->fdt); ut_assertnull(label->fdtdir); - ut_assertnull(label->fdtoverlays); + ut_asserteq(0, label->fdtoverlays.count); + ut_assertnull(label->say); ut_asserteq(0, label->ipappend); ut_asserteq(0, label->attempted); ut_asserteq(0, label->localboot); @@ -662,7 +666,7 @@ static int pxe_test_overlay_no_addr_norun(struct unit_test_state *uts) /* Get the first label (linux) which has fdtoverlays */ label = list_first_entry(&cfg->labels, struct pxe_label, list); ut_asserteq_str("linux", label->name); - ut_assertnonnull(label->fdtoverlays); + ut_assert(label->fdtoverlays.count > 0); /* Enable output for loading phase */ ctx.quiet = false; -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Introduce struct pxe_fdtoverlay to hold both the path and loaded address for each overlay. Update label_boot_fdtoverlay() to: 1. First pass: load all overlay files and store their addresses 2. Resize the main FDT once to make room for all overlays 3. Second pass: apply all loaded overlays This ensures all overlays are loaded before any are applied, which may be useful when overlays depend on each other or when the FDT resize could affect memory layout. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_parse.c | 19 ++++++++++--------- boot/pxe_utils.c | 37 +++++++++++++++++++++---------------- include/pxe_utils.h | 13 ++++++++++++- test/boot/pxe.c | 4 ++-- 4 files changed, 45 insertions(+), 28 deletions(-) diff --git a/boot/pxe_parse.c b/boot/pxe_parse.c index b115fb413a2..6d148756b0d 100644 --- a/boot/pxe_parse.c +++ b/boot/pxe_parse.c @@ -106,14 +106,14 @@ static struct pxe_label *label_create(void) if (!label) return NULL; memset(label, 0, sizeof(struct pxe_label)); - alist_init_struct(&label->fdtoverlays, char *); + alist_init_struct(&label->fdtoverlays, struct pxe_fdtoverlay); return label; } void label_destroy(struct pxe_label *label) { - char **overlayp; + struct pxe_fdtoverlay *overlay; free(label->name); free(label->kernel_label); @@ -123,8 +123,8 @@ void label_destroy(struct pxe_label *label) free(label->initrd); free(label->fdt); free(label->fdtdir); - alist_for_each(overlayp, &label->fdtoverlays) - free(*overlayp); + alist_for_each(overlay, &label->fdtoverlays) + free(overlay->path); alist_uninit(&label->fdtoverlays); free(label->say); free(label); @@ -323,7 +323,7 @@ static int parse_fdtoverlays(char **c, struct alist *overlays) return err; while (*val) { - char *path; + struct pxe_fdtoverlay item; char *end; /* Skip leading spaces */ @@ -336,15 +336,16 @@ static int parse_fdtoverlays(char **c, struct alist *overlays) /* Find end of this path */ end = strchr(val, ' '); if (end) { - path = strndup(val, end - val); + item.path = strndup(val, end - val); val = end; } else { - path = strdup(val); + item.path = strdup(val); val += strlen(val); } + item.addr = 0; - if (!path || !alist_add(overlays, path)) { - free(path); + if (!item.path || !alist_add(overlays, item)) { + free(item.path); return -ENOMEM; } } diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index a50fa4bc333..8d565d971fc 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -306,9 +306,9 @@ static void label_boot_kaslrseed(struct pxe_context *ctx) static void label_boot_fdtoverlay(struct pxe_context *ctx, struct pxe_label *label) { + struct pxe_fdtoverlay *overlay; struct fdt_header *blob; ulong fdtoverlay_addr; - char **overlayp; bool use_lmb; char *envaddr; int err; @@ -331,38 +331,43 @@ static void label_boot_fdtoverlay(struct pxe_context *ctx, use_lmb = true; } - /* Apply each overlay file in order */ - alist_for_each(overlayp, &label->fdtoverlays) { - const char *overlayfile = *overlayp; + /* First pass: load all overlay files */ + alist_for_each(overlay, &label->fdtoverlays) { ulong addr = fdtoverlay_addr; ulong size; - /* Load overlay file */ - err = get_relfile(ctx, overlayfile, &addr, SZ_4K, + err = get_relfile(ctx, overlay->path, &addr, SZ_4K, (enum bootflow_img_t)IH_TYPE_FLATDT, &size); if (err < 0) { - printf("Failed loading overlay %s\n", overlayfile); + printf("Failed loading overlay %s\n", overlay->path); continue; } + overlay->addr = addr; - /* Resize main fdt */ - fdt_shrink_to_minimum(ctx->fdt, 8192); + /* Move to next address if using fixed addresses */ + if (!use_lmb) + fdtoverlay_addr = addr + size; + } + + /* Resize main fdt to make room for overlays */ + fdt_shrink_to_minimum(ctx->fdt, 8192); - blob = map_sysmem(addr, 0); + /* Second pass: apply all loaded overlays */ + alist_for_each(overlay, &label->fdtoverlays) { + if (!overlay->addr) + continue; + + blob = map_sysmem(overlay->addr, 0); err = fdt_check_header(blob); if (err) { - printf("Invalid overlay %s, skipping\n", overlayfile); + printf("Invalid overlay %s, skipping\n", overlay->path); continue; } err = fdt_overlay_apply_verbose(ctx->fdt, blob); if (err) printf("Failed to apply overlay %s, skipping\n", - overlayfile); - - /* Move to next address if using fixed addresses */ - if (!use_lmb) - fdtoverlay_addr = addr + size; + overlay->path); } } diff --git a/include/pxe_utils.h b/include/pxe_utils.h index 52a520e4823..d2062c03997 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -24,6 +24,17 @@ * take a 'include file getter' function. */ +/** + * struct pxe_fdtoverlay - info about an FDT overlay + * + * @path: Path to the overlay file + * @addr: Address where the overlay was loaded (0 if not yet loaded) + */ +struct pxe_fdtoverlay { + char *path; + ulong addr; +}; + /** * struct pxe_label - describes a single label in a pxe file * @@ -39,7 +50,7 @@ * @initrd: path to the initrd to use for this label. * @fdt: path to FDT to use * @fdtdir: path to FDT directory to use - * @fdtoverlays: list of paths of FDT overlays to apply (alist of char *) + * @fdtoverlays: list of FDT overlays to apply (alist of struct pxe_fdtoverlay) * @say: message to print when this label is selected for booting * @ipappend: flags for appending IP address (0x1) and MAC address (0x3) * @attempted: 0 if we haven't tried to boot this label, 1 if we have diff --git a/test/boot/pxe.c b/test/boot/pxe.c index 47cfa4042f5..df5c106caab 100644 --- a/test/boot/pxe.c +++ b/test/boot/pxe.c @@ -192,9 +192,9 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_assertnull(label->fdtdir); ut_asserteq(2, label->fdtoverlays.count); ut_asserteq_str("/dtb/overlay1.dtbo", - *alist_get(&label->fdtoverlays, 0, char *)); + alist_get(&label->fdtoverlays, 0, struct pxe_fdtoverlay)->path); ut_asserteq_str("/dtb/overlay2.dtbo", - *alist_get(&label->fdtoverlays, 1, char *)); + alist_get(&label->fdtoverlays, 1, struct pxe_fdtoverlay)->path); ut_asserteq_str("Booting default Linux kernel", label->say); ut_asserteq(0, label->ipappend); ut_asserteq(0, label->attempted); -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Move the overlay loading from label_process_fdt() to pxe_load_files(), grouping all file loading (kernel, initrd, FDT, overlays) in one place. The overlay applying remains in label_process_fdt() since it operates on the working FDT after it has been set up. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_utils.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 4bae830f353..e336b125716 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -515,10 +515,8 @@ static int label_process_fdt(struct pxe_context *ctx, struct pxe_label *label) if (label->kaslrseed) label_boot_kaslrseed(ctx); - if (IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY) && label->fdtoverlays.count) { - label_load_fdtoverlays(ctx, label); + if (IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY) && label->fdtoverlays.count) label_apply_fdtoverlays(ctx, label); - } return 0; } @@ -681,6 +679,9 @@ static int pxe_load_files(struct pxe_context *ctx, struct pxe_label *label, } } + if (IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY) && label->fdtoverlays.count) + label_load_fdtoverlays(ctx, label); + return 0; } -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Split pxe_load_label() into two functions to allow more flexible use: - pxe_load_label(): Handles localboot setup, FIT detection, and calls pxe_load_files() to load all files (kernel, initrd, FDT, overlays) - pxe_setup_label(): Processes the FDT (applying overlays), determines the FDT address, and saves boot parameters to the context This separation allows callers to load files without immediately setting up boot parameters, which is useful for inspection or validation before booting. Update label_boot() to call both functions in sequence. Copy fdt_addr to conf_fdt at the end of pxe_load_label() to maintain backward compatibility for callers that only call pxe_load_label() without pxe_setup_label(). Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_utils.c | 28 +++++++++++++++++++++++----- include/pxe_utils.h | 19 ++++++++++++++++--- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 8609d832aa6..49934d626ee 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -687,10 +687,6 @@ int pxe_load_files(struct pxe_context *ctx, struct pxe_label *label, int pxe_load_label(struct pxe_context *ctx, struct pxe_label *label) { - char fit_addr[200]; - const char *conf_fdt_str; - ulong conf_fdt = 0; - char initrd_str[28] = ""; char *fdtfile = NULL; bool is_fit; int ret; @@ -722,6 +718,25 @@ int pxe_load_label(struct pxe_context *ctx, struct pxe_label *label) if (ret) return ret; + /* Copy fdt_addr to conf_fdt for callers that don't use pxe_setup_label */ + ctx->conf_fdt = ctx->fdt_addr; + + return 0; +} + +int pxe_setup_label(struct pxe_context *ctx, struct pxe_label *label) +{ + char fit_addr[200]; + const char *conf_fdt_str; + ulong conf_fdt = 0; + char initrd_str[28] = ""; + bool is_fit; + int ret; + + /* Check for FIT case: FDT comes from FIT image, not a separate file */ + is_fit = label->fdt && label->kernel_label && + !strcmp(label->kernel_label, label->fdt); + /* for FIT, append the configuration identifier */ snprintf(fit_addr, sizeof(fit_addr), "%lx%s", ctx->kern_addr, label->config ? label->config : ""); @@ -850,11 +865,14 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) } } - /* Load files if not already loaded */ + /* Load files and set up boot params if not already done */ if (!ctx->label) { ret = pxe_load_label(ctx, label); if (ret) return 1; + ret = pxe_setup_label(ctx, label); + if (ret) + return 1; } if (label->ipappend & 0x1) { diff --git a/include/pxe_utils.h b/include/pxe_utils.h index aa73f5ff7f0..a03e00e2a4a 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -464,9 +464,9 @@ int pxe_load_files(struct pxe_context *ctx, struct pxe_label *label, /** * pxe_load_label() - Load kernel/initrd/FDT for a label * - * Loads the files specified in the label into memory and saves the - * addresses and sizes in @ctx. Call this only when ready to boot or - * inspect loaded files. + * Loads the files specified in the label into memory. Call + * pxe_setup_label() after this to process the FDT and set up + * boot parameters. * * @ctx: PXE context with getfile callback * @label: Label whose files to load @@ -475,6 +475,19 @@ int pxe_load_files(struct pxe_context *ctx, struct pxe_label *label, */ int pxe_load_label(struct pxe_context *ctx, struct pxe_label *label); +/** + * pxe_setup_label() - Set up boot parameters for a loaded label + * + * Processes the FDT (applying overlays if needed) and saves the boot + * parameters in @ctx. Call this after pxe_load_label(). + * + * @ctx: PXE context with loaded files + * @label: Label to set up + * Return: 0 on success, -ENOSPC if initrd string too long, -ENOMEM if + * out of memory + */ +int pxe_setup_label(struct pxe_context *ctx, struct pxe_label *label); + /* * Entry point for parsing a menu file. nest_level indicates how many times * we've nested in includes. It will be 1 for the top level menu file. -- 2.43.0