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