[Concept,05/32] boot: pxe: Add separate APIs for label selection and file loading

Message ID 20260109231151.4056804-6-sjg@u-boot.org
State New
Headers
Series boot: pxe: Refactor into separate load/setup phases |

Commit Message

Simon Glass Jan. 9, 2026, 11:11 p.m. UTC
  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(-)
  

Patch

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();