[PATCH 0/7] pxe_utils: small fixups, implement BLS type 1 boot on top of them

[PATCH 0/7] pxe_utils: small fixups, implement BLS type 1 boot on top of them

[Alexey Charkov]

Add support for the Boot Loader Specification (BLS) type 1 boot entries,
as generated by default by systemd's kernel-install when loader=bls.

Given that the format of BLS entries is pretty much the same as PXElinux,
reuse the existing library for the parsing logic.

BLS type 2 entries are out of scope, as they are effectively just EFI
applications and should be booted as such.

This implementation is also only allowing a single top-sorting entry to
boot, as the standard boot infrastruture currently doesn't support
multiple entries per partition-bootmeth tuple. I have a proposed extension
to enable that and will post it separately as RFC - that enables the use
of `bootflow menu` to select the kernel to boot without jumping through
several menus with different behavior as is currently required with
extlinux.conf.

Signed-off-by: Alexey Charkov <alchark@flipper.net>
---
Alexey Charkov (7):
      pxe_utils: fix W=1 kernel-doc warnings
      pxe_utils: accept "options" as synonym for "append"
      pxe_utils: extract per-entry key parsing into parse_label_keys()
      pxe_utils: export per-entry label helpers
      pxe_utils: optionally ignore unknown keys in parse_label_keys()
      pxe_utils: accept "title" inside a label as a synonym for "menu label"
      boot: add a minimal bootmeth for the Boot Loader Specification

 boot/Kconfig        |  17 +++
 boot/Makefile       |   1 +
 boot/bootmeth_bls.c | 333 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 boot/pxe_utils.c    | 134 +++++++++++++--------
 include/pxe_utils.h |  62 ++++++++++
 5 files changed, 499 insertions(+), 48 deletions(-)
---
base-commit: 31af00bdc6a3ab5d4ada83e08407f2ce289f0ec6
change-id: 20260604-bls-db6a30b913ae

Best regards,
--  
Alexey Charkov <alchark@flipper.net>

[PATCH 1/7] pxe_utils: fix W=1 kernel-doc warnings

[Alexey Charkov]

Add the missing parameter and enum value descriptions reported by
scripts/kernel-doc when building with W=1:

  - get_relfile():           document @type
  - get_pxelinux_path():     document @pxefile_addr_r
  - enum lex_state:          document L_NORMAL, L_KEYWORD, L_SLITERAL
  - get_token():             document @t and @state

No functional change.

Signed-off-by: Alexey Charkov <alchark@flipper.net>
---
 boot/pxe_utils.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
index 419ab1f1b0ef..774d5f43c036 100644
--- a/boot/pxe_utils.c
+++ b/boot/pxe_utils.c
@@ -95,6 +95,7 @@ int format_mac_pxe(char *outbuf, size_t outbuf_len)
  * @ctx: PXE context
  * @file_path: File path to read (relative to the PXE file)
  * @file_addr: Address to load file to
+ * @type: Image type used to record the loaded file in the bootflow
  * @filesizep: If not NULL, returns the file size in bytes
  * Returns 1 for success, or < 0 on error
  */
@@ -162,6 +163,7 @@ int get_pxe_file(struct pxe_context *ctx, const char *file_path,
  *
  * @ctx: PXE context
  * @file: Filename to process (relative to pxelinux.cfg/)
+ * @pxefile_addr_r: Address to load the file to
  * Returns 1 for success, -ENAMETOOLONG if the resulting path is too long.
  *	or other value < 0 on other error
  */
@@ -914,6 +916,13 @@ static const struct token keywords[] = {
  * Since pxe(linux) files don't have a token to identify the start of a
  * literal, we have to keep track of when we're in a state where a literal is
  * expected vs when we're in a state a keyword is expected.
+ *
+ * @L_NORMAL: Outside any specific lexical context; whitespace is skipped
+ *	and the next non-blank token determines what is read
+ * @L_KEYWORD: A keyword is expected; the next word is matched against the
+ *	keyword table and tagged with the matching token type
+ * @L_SLITERAL: A string literal is expected; characters are read up to the
+ *	end of the line
  */
 enum lex_state {
 	L_NORMAL = 0,
@@ -1018,6 +1027,9 @@ static void get_keyword(struct token *t)
  *
  * @p: Points to a pointer to the current position in the input being processed.
  *	Updated to point at the first character after the current token
+ * @t: Token to populate with the type and value of what was read
+ * @state: Current lexer state, controlling whether a keyword or a string
+ *	literal is expected next
  */
 static void get_token(char **p, struct token *t, enum lex_state state)
 {

-- 
2.53.0

[PATCH 2/7] pxe_utils: accept "options" as synonym for "append"

[Alexey Charkov]

The Boot Loader Specification [1] type #2 entry files use the keyword
"options" for the kernel command line, which corresponds to extlinux's
"append". Recognising it here lets the existing pxelinux parser ingest
BLS entries unchanged, paving the way for a BLS bootmeth that reuses
the parser instead of duplicating it.

No effect on existing extlinux/pxelinux files: "options" is not a valid
keyword in those formats, so it cannot collide with prior usage.

[1] https://uapi-group.org/specifications/specs/boot_loader_specification/

Signed-off-by: Alexey Charkov <alchark@flipper.net>
---
 boot/pxe_utils.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
index 774d5f43c036..c45c9d0cd012 100644
--- a/boot/pxe_utils.c
+++ b/boot/pxe_utils.c
@@ -894,6 +894,7 @@ static const struct token keywords[] = {
 	{"linux", T_LINUX},
 	{"localboot", T_LOCALBOOT},
 	{"append", T_APPEND},
+	{"options", T_APPEND},
 	{"initrd", T_INITRD},
 	{"include", T_INCLUDE},
 	{"devicetree", T_FDT},

-- 
2.53.0

[PATCH 3/7] pxe_utils: extract per-entry key parsing into parse_label_keys()

[Alexey Charkov]

Split the body of parse_label() into a standalone parse_label_keys()
helper that walks key/value lines and populates a pre-existing
struct pxe_label. parse_label() becomes a thin wrapper that creates
the label, reads its name, attaches it to the menu, and delegates.

This is a pure refactor: the new helper contains the original loop
verbatim, with the local variable declarations moved to its scope.
No call sites or behaviour change.

A subsequent change will export this helper so callers parsing
formats that lack a 'label' header (notably Boot Loader Specification
type #2 entries) can populate a label directly from a file body
without duplicating the parser.

Signed-off-by: Alexey Charkov <alchark@flipper.net>
---
 boot/pxe_utils.c | 58 ++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 37 insertions(+), 21 deletions(-)

diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
index c45c9d0cd012..420cee307baf 100644
--- a/boot/pxe_utils.c
+++ b/boot/pxe_utils.c
@@ -1288,34 +1288,20 @@ static int parse_label_kernel(char **c, struct pxe_label *label)
 }
 
 /*
- * Parses a label and adds it to the list of labels for a menu.
- *
- * A label ends when we either get to the end of a file, or
- * get some input we otherwise don't have a handler defined
- * for.
+ * Parse the body of a label: the sequence of key/value lines that follow
+ * the 'label NAME' header. Stops at end-of-file or at a token that doesn't
+ * belong inside a label (which is pushed back so the caller can handle it).
  *
+ * Returns 1 on success, < 0 on error.
  */
-static int parse_label(char **c, struct pxe_menu *cfg)
+static int parse_label_keys(char **c, struct pxe_menu *cfg,
+			    struct pxe_label *label)
 {
 	struct token t;
+	char *s;
 	int len;
-	char *s = *c;
-	struct pxe_label *label;
 	int err;
 
-	label = label_create();
-	if (!label)
-		return -ENOMEM;
-
-	err = parse_sliteral(c, &label->name);
-	if (err < 0) {
-		printf("Expected label name: %.*s\n", (int)(*c - s), s);
-		label_destroy(label);
-		return -EINVAL;
-	}
-
-	list_add_tail(&label->list, &cfg->labels);
-
 	while (1) {
 		s = *c;
 		get_token(c, &t, L_KEYWORD);
@@ -1397,6 +1383,36 @@ static int parse_label(char **c, struct pxe_menu *cfg)
 	}
 }
 
+/*
+ * Parses a label and adds it to the list of labels for a menu.
+ *
+ * A label ends when we either get to the end of a file, or
+ * get some input we otherwise don't have a handler defined
+ * for.
+ *
+ */
+static int parse_label(char **c, struct pxe_menu *cfg)
+{
+	char *s = *c;
+	struct pxe_label *label;
+	int err;
+
+	label = label_create();
+	if (!label)
+		return -ENOMEM;
+
+	err = parse_sliteral(c, &label->name);
+	if (err < 0) {
+		printf("Expected label name: %.*s\n", (int)(*c - s), s);
+		label_destroy(label);
+		return -EINVAL;
+	}
+
+	list_add_tail(&label->list, &cfg->labels);
+
+	return parse_label_keys(c, cfg, label);
+}
+
 /*
  * This 16 comes from the limit pxelinux imposes on nested includes.
  *

-- 
2.53.0

[PATCH 4/7] pxe_utils: export per-entry label helpers

[Alexey Charkov]

Drop the static qualifier from label_create(), label_destroy(),
parse_label_keys() and label_boot(), and declare them in
<pxe_utils.h> to make them reusable.

The intended consumer is a Boot Loader Specification bootmeth, where
each on-disk file under loader/entries/ is a single entry with no
'label' header and no menu to traverse: the bootmeth derives the
label name from the filename, hands the file body to
parse_label_keys(), and invokes label_boot() directly.

No behaviour change: the implementations are unchanged and existing
in-tree callers in pxe_utils.c continue to use them as before.

Signed-off-by: Alexey Charkov <alchark@flipper.net>
---
 boot/pxe_utils.c    | 37 ++++-------------------------------
 include/pxe_utils.h | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 33 deletions(-)

diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
index 420cee307baf..6dad3045f9d4 100644
--- a/boot/pxe_utils.c
+++ b/boot/pxe_utils.c
@@ -218,15 +218,7 @@ static int get_relfile_envaddr(struct pxe_context *ctx, const char *file_path,
 	return get_relfile(ctx, file_path, file_addr, type, filesizep);
 }
 
-/**
- * label_create() - crate a new PXE label
- *
- * Allocates memory for and initializes a pxe_label. This uses malloc, so the
- * result must be free()'d to reclaim the memory.
- *
- * Returns a pointer to the label, or NULL if out of memory
- */
-static struct pxe_label *label_create(void)
+struct pxe_label *label_create(void)
 {
 	struct pxe_label *label;
 
@@ -239,20 +231,7 @@ static struct pxe_label *label_create(void)
 	return label;
 }
 
-/**
- * label_destroy() - free the memory used by a pxe_label
- *
- * This frees @label itself as well as memory used by its name,
- * kernel, config, append, initrd, fdt, fdtdir and fdtoverlay members, if
- * they're non-NULL.
- *
- * So - be sure to only use dynamically allocated memory for the members of
- * the pxe_label struct, unless you want to clean it up first. These are
- * currently only created by the pxe file parsing code.
- *
- * @label: Label to free
- */
-static void label_destroy(struct pxe_label *label)
+void label_destroy(struct pxe_label *label)
 {
 	free(label->name);
 	free(label->kernel_label);
@@ -542,7 +521,7 @@ cleanup:
  * 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 label_boot(struct pxe_context *ctx, struct pxe_label *label)
 {
 	char *bootm_argv[] = { "bootm", NULL, NULL, NULL, NULL };
 	char *zboot_argv[] = { "zboot", NULL, "0", NULL, NULL };
@@ -1287,15 +1266,7 @@ static int parse_label_kernel(char **c, struct pxe_label *label)
 	return 1;
 }
 
-/*
- * Parse the body of a label: the sequence of key/value lines that follow
- * the 'label NAME' header. Stops at end-of-file or at a token that doesn't
- * belong inside a label (which is pushed back so the caller can handle it).
- *
- * Returns 1 on success, < 0 on error.
- */
-static int parse_label_keys(char **c, struct pxe_menu *cfg,
-			    struct pxe_label *label)
+int parse_label_keys(char **c, struct pxe_menu *cfg, struct pxe_label *label)
 {
 	struct token t;
 	char *s;
diff --git a/include/pxe_utils.h b/include/pxe_utils.h
index 0378f2889f7b..e639e59e5dc8 100644
--- a/include/pxe_utils.h
+++ b/include/pxe_utils.h
@@ -202,6 +202,62 @@ void handle_pxe_menu(struct pxe_context *ctx, struct pxe_menu *cfg);
  */
 struct pxe_menu *parse_pxefile(struct pxe_context *ctx, ulong menucfg);
 
+/**
+ * label_create() - Allocate and zero-initialise a struct pxe_label
+ *
+ * The result must be freed with label_destroy().
+ *
+ * Return: pointer to the new label, or NULL if out of memory
+ */
+struct pxe_label *label_create(void);
+
+/**
+ * label_destroy() - Free a struct pxe_label and its string members
+ *
+ * Frees @label as well as memory used by its name, kernel, config, append,
+ * initrd, fdt, fdtdir and fdtoverlays members, if they are non-NULL. All
+ * such members must therefore reference malloc()'d memory.
+ *
+ * @label: Label to free
+ */
+void label_destroy(struct pxe_label *label);
+
+/**
+ * parse_label_keys() - Parse the body of a label
+ *
+ * Walks the sequence of key/value lines that follow a 'label NAME' header,
+ * populating @label. Stops at end-of-file or at a token that does not
+ * belong inside a label (which is pushed back so the caller can handle it).
+ *
+ * This is exported so that callers handling formats which lack a 'label'
+ * header (e.g. Boot Loader Specification type #2 entry files) can populate
+ * a pre-created label directly from a file body.
+ *
+ * @c: Pointer to the cursor into the file being parsed; updated on return
+ * @cfg: Menu the label belongs to (used for 'menu default' bookkeeping)
+ * @label: Label to populate; must already be allocated and (when called for
+ *	a file that has a 'label' header) attached to @cfg->labels
+ * Return: 1 on success, < 0 on error
+ */
+int parse_label_keys(char **c, struct pxe_menu *cfg, struct pxe_label *label);
+
+/**
+ * label_boot() - Boot according to the contents of a single pxe_label
+ *
+ * On success this function does not return; on failure it returns to
+ * let the caller try a different label or surface an error.
+ *
+ * The kernel is staged at $kernel_addr_r, an optional initrd at
+ * $ramdisk_addr_r, and an optional FDT at $fdt_addr_r. If the label
+ * has an 'append' string it overwrites $bootargs.
+ *
+ * @ctx: PXE context
+ * @label: Label to boot
+ * Return: does not return on success; 0 if a 'localboot' label was
+ *	processed; 1 on error
+ */
+int label_boot(struct pxe_context *ctx, struct pxe_label *label);
+
 /**
  * format_mac_pxe() - Convert a MAC address to PXE format
  *

-- 
2.53.0

[PATCH 5/7] pxe_utils: optionally ignore unknown keys in parse_label_keys()

[Alexey Charkov]

The parser currently treats any keyword it does not recognise inside a
label body as the end of that label, pushing the token back so the
caller can dispatch it as menu-level input. That is correct for
extlinux/pxelinux, where the only thing legitimately following a label
body is another 'label' (or a top-level 'menu ...') line.

Other formats that share enough syntax to reuse this parser have
different rules. The Boot Loader Specification, in particular, lists a
number of entry-level keys (title, version, sort-key, machine-id,
architecture, ...) that this parser knows nothing about, and the spec
explicitly requires implementations to silently ignore unrecognised
keys rather than treat them as a structural boundary.

Add an ignore_unknown flag to parse_label_keys(): when set, the default
switch case consumes the rest of the offending line via eol_or_eof()
and continues parsing instead of returning. The existing extlinux
caller passes false and so is unaffected.

Signed-off-by: Alexey Charkov <alchark@flipper.net>
---
 boot/pxe_utils.c    | 29 +++++++++++++++++++++++++++--
 include/pxe_utils.h |  8 +++++++-
 2 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
index 6dad3045f9d4..7ecee86a9ada 100644
--- a/boot/pxe_utils.c
+++ b/boot/pxe_utils.c
@@ -1266,7 +1266,8 @@ static int parse_label_kernel(char **c, struct pxe_label *label)
 	return 1;
 }
 
-int parse_label_keys(char **c, struct pxe_menu *cfg, struct pxe_label *label)
+int parse_label_keys(char **c, struct pxe_menu *cfg, struct pxe_label *label,
+		     bool ignore_unknown)
 {
 	struct token t;
 	char *s;
@@ -1339,7 +1340,31 @@ int parse_label_keys(char **c, struct pxe_menu *cfg, struct pxe_label *label)
 		case T_EOL:
 			break;
 
+		case T_EOF:
+			if (ignore_unknown) {
+				/*
+				 * BLS-style callers parse a standalone label
+				 * body, so there is no outer context to push
+				 * T_EOF back into — stop cleanly here.
+				 */
+				return 1;
+			}
+			/*
+			 * For pxelinux/extlinux, fall through so the default
+			 * case pushes T_EOF back for the top-level parser.
+			 */
+			fallthrough;
 		default:
+			if (ignore_unknown) {
+				/*
+				 * Skip the rest of the line and keep going.
+				 * Used for formats like the Boot Loader
+				 * Specification, where the spec mandates that
+				 * unknown keys must be silently ignored.
+				 */
+				eol_or_eof(c);
+				break;
+			}
 			/*
 			 * put the token back! we don't want it - it's the end
 			 * of a label and whatever token this is, it's
@@ -1381,7 +1406,7 @@ static int parse_label(char **c, struct pxe_menu *cfg)
 
 	list_add_tail(&label->list, &cfg->labels);
 
-	return parse_label_keys(c, cfg, label);
+	return parse_label_keys(c, cfg, label, false);
 }
 
 /*
diff --git a/include/pxe_utils.h b/include/pxe_utils.h
index e639e59e5dc8..653e1a7d866e 100644
--- a/include/pxe_utils.h
+++ b/include/pxe_utils.h
@@ -237,9 +237,15 @@ void label_destroy(struct pxe_label *label);
  * @cfg: Menu the label belongs to (used for 'menu default' bookkeeping)
  * @label: Label to populate; must already be allocated and (when called for
  *	a file that has a 'label' header) attached to @cfg->labels
+ * @ignore_unknown: If true, silently skip unknown keys (and consume the
+ *	rest of their lines) instead of stopping. This matches the Boot
+ *	Loader Specification's requirement that unknown keys be ignored.
+ *	If false (extlinux/pxelinux behaviour), an unknown token is pushed
+ *	back so the caller can treat it as the start of the next label.
  * Return: 1 on success, < 0 on error
  */
-int parse_label_keys(char **c, struct pxe_menu *cfg, struct pxe_label *label);
+int parse_label_keys(char **c, struct pxe_menu *cfg, struct pxe_label *label,
+		     bool ignore_unknown);
 
 /**
  * label_boot() - Boot according to the contents of a single pxe_label

-- 
2.53.0

[PATCH 6/7] pxe_utils: accept "title" inside a label as a synonym for "menu label"

[Alexey Charkov]

The parser already understands the "title" keyword at the menu level
(via "menu title"). Boot Loader Specification [1] type #2 entry files
use a bare "title" line at the entry level to give the human-readable
name of the entry, the closest extlinux equivalent of which is
"menu label" inside a label body.

Make parse_label_keys() honour "title" the same way it honours
"menu label", populating label->menu. This lets BLS entries surface
their pretty name through the existing label->menu plumbing without
the BLS bootmeth (or any other caller) having to special-case it.

No effect on existing extlinux/pxelinux files: those use "menu label"
inside a label, not bare "title", so this change is purely additive.

[1] https://uapi-group.org/specifications/specs/boot_loader_specification/

Signed-off-by: Alexey Charkov <alchark@flipper.net>
---
 boot/pxe_utils.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
index 7ecee86a9ada..1f3f6150d9b1 100644
--- a/boot/pxe_utils.c
+++ b/boot/pxe_utils.c
@@ -1284,6 +1284,19 @@ int parse_label_keys(char **c, struct pxe_menu *cfg, struct pxe_label *label,
 			err = parse_label_menu(c, cfg, label);
 			break;
 
+		case T_TITLE:
+			/*
+			 * Equivalent to 'menu label' inside a label body.
+			 * Boot Loader Specification entries use a bare
+			 * 'title' line for the human-readable name; honour
+			 * it here so the existing parser handles BLS files
+			 * natively. extlinux/pxelinux files conventionally
+			 * use 'menu label' instead, so this is additive.
+			 */
+			if (!label->menu)
+				err = parse_sliteral(c, &label->menu);
+			break;
+
 		case T_KERNEL:
 		case T_LINUX:
 			err = parse_label_kernel(c, label);

-- 
2.53.0

[PATCH 7/7] boot: add a minimal bootmeth for the Boot Loader Specification

[Alexey Charkov]

Add a bootmeth that finds and boots Boot Loader Specification (BLS)
type #1 entry files [1]. On each block-device partition it scans, the
bootmeth looks for files matching '<prefix>loader/entries/*.conf'
(where <prefix> comes from bootstd_get_prefixes(), typically '/' and
'/boot/'), picks the highest-sorting filename, parses it, and exposes
it as a bootflow.

Implementation reuses the existing pxelinux infrastructure.

For now the entry chosen on a partition is purely the lexicographic
maximum of *.conf filenames; sort-key / version field handling
(spec-mandated tiebreakers) and boot-counting (the '+TRIES_LEFT'
filename suffix) are left as TODOs. Likewise, only the top-sorted entry
is surfaced because the bootstd framework currently allows one bootflow
per (bootmeth, partition); exposing every discovered entry will require
a framework extension.

Type #2 BLS (drop-in directory of EFI binaries) is out of scope here;
existing EFI bootmeths cover that use case.

[1] https://uapi-group.org/specifications/specs/boot_loader_specification/

Signed-off-by: Alexey Charkov <alchark@flipper.net>
---
 boot/Kconfig        |  17 +++
 boot/Makefile       |   1 +
 boot/bootmeth_bls.c | 333 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 351 insertions(+)

diff --git a/boot/Kconfig b/boot/Kconfig
index e1114aea843e..fa871da46640 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -636,6 +636,23 @@ config BOOTMETH_EXTLINUX_PXE
 
 	  This provides a way to try out standard boot on an existing boot flow.
 
+config BOOTMETH_BLS
+	bool "Bootdev support for Boot Loader Specification entries"
+	select PXE_UTILS
+	default y
+	help
+	  Enables support for booting via Boot Loader Specification type #1
+	  entry files. The bootmeth scans each filesystem it finds for files
+	  matching 'loader/entries/*.conf' (or '/boot/loader/entries/*.conf')
+	  and boots the highest-sorting entry.
+
+	  The specification is here:
+
+	    https://uapi-group.org/specifications/specs/boot_loader_specification/
+
+	  Type #2 BLS (drop-in directory of EFI binaries) is not handled here;
+	  use BOOTMETH_EFI_BOOTMGR for that.
+
 config BOOTMETH_EFILOADER
 	bool "Bootdev support for EFI boot"
 	depends on EFI_BINARY_EXEC
diff --git a/boot/Makefile b/boot/Makefile
index 7fb56e7ef379..0ce6fd1cd050 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_$(PHASE_)BOOTSTD_PROG) += prog_boot.o
 
 obj-$(CONFIG_$(PHASE_)BOOTMETH_EXTLINUX) += bootmeth_extlinux.o
 obj-$(CONFIG_$(PHASE_)BOOTMETH_EXTLINUX_PXE) += bootmeth_pxe.o
+obj-$(CONFIG_$(PHASE_)BOOTMETH_BLS) += bootmeth_bls.o
 obj-$(CONFIG_$(PHASE_)BOOTMETH_EFILOADER) += bootmeth_efi.o
 obj-$(CONFIG_$(PHASE_)BOOTMETH_CROS) += bootm.o bootm_os.o bootmeth_cros.o
 obj-$(CONFIG_$(PHASE_)BOOTMETH_QFW) += bootmeth_qfw.o
diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
new file mode 100644
index 000000000000..3df4f20a11dd
--- /dev/null
+++ b/boot/bootmeth_bls.c
@@ -0,0 +1,333 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Bootmethod for the Boot Loader Specification (type #1 entry files)
+ *
+ * Reuses the pxelinux parser/boot path: each on-disk entry is read into a
+ * struct pxe_label via parse_label_keys() and booted via label_boot().
+ *
+ * Spec: https://uapi-group.org/specifications/specs/boot_loader_specification/
+ *
+ * TODO: a partition typically holds several BLS entries, but the bootstd
+ * framework currently allows only one bootflow per (bootmeth, partition)
+ * pair, so this bootmeth surfaces only the highest-sorting entry. Once the
+ * framework grows a way for a bootmeth to emit multiple bootflows from a
+ * single partition, this should expose every discovered entry so the user
+ * can pick from the standard 'bootflow menu' UI rather than be limited to
+ * the default pick.
+ */
+
+#define LOG_CATEGORY UCLASS_BOOTSTD
+
+#include <bootdev.h>
+#include <bootflow.h>
+#include <bootmeth.h>
+#include <bootstd.h>
+#include <command.h>
+#include <dm.h>
+#include <extlinux.h>
+#include <fs.h>
+#include <malloc.h>
+#include <mapmem.h>
+#include <pxe_utils.h>
+#include <linux/sizes.h>
+
+#define BLS_DIR		"loader/entries"
+#define BLS_SUFFIX	".conf"
+
+static int bls_check(struct udevice *dev, struct bootflow_iter *iter)
+{
+	int ret;
+
+	/* This only works on block devices */
+	ret = bootflow_iter_check_blk(iter);
+	if (ret)
+		return log_msg_ret("blk", ret);
+
+	return 0;
+}
+
+static int bls_getfile(struct pxe_context *ctx, const char *file_path,
+		       char *file_addr, enum bootflow_img_t type, ulong *sizep)
+{
+	struct extlinux_info *info = ctx->userdata;
+	ulong addr;
+	int ret;
+
+	addr = simple_strtoul(file_addr, NULL, 16);
+
+	/* Allow up to 1GB */
+	*sizep = 1 << 30;
+	ret = bootmeth_read_file(info->dev, info->bflow, file_path, addr,
+				 type, sizep);
+	if (ret)
+		return log_msg_ret("read", ret);
+
+	return 0;
+}
+
+/**
+ * bls_pick_entry() - Find the highest-sorting *.conf across bootstd prefixes
+ *
+ * Walks ``<prefix>/loader/entries/`` for each prefix in @prefixes and
+ * returns the lexicographically maximum full path seen.
+ *
+ * The spec leaves ordering between prefixes unspecified; comparing full
+ * paths is a deterministic-and-cheap stand-in.
+ *
+ * The Boot Loader Specification says entries should be sorted by sort-key
+ * (descending), then version (descending), then filename (descending). For
+ * the time being only the filename criterion is implemented, which is
+ * sufficient for most distros that encode kernel version into the filename.
+ *
+ * TODO: implement proper spec-compliant ordering. That requires reading
+ * each candidate entry, parsing its 'sort-key' and 'version' fields (the
+ * latter compared with strverscmp()-style logic), and only falling back to
+ * filename order when those tie.
+ *
+ * @prefixes:	NULL-terminated array of bootstd prefixes to search
+ * @desc:	Block descriptor (used to re-mount per prefix)
+ * @bflow:	Bootflow being populated (used to re-mount per prefix)
+ * @fullp:	Returns the chosen full path (allocated), or NULL if none
+ * Return: 0 on success, -ENOENT if no entry was found, < 0 on other error
+ */
+static int bls_pick_entry(const char *const *prefixes, struct blk_desc *desc,
+			  struct bootflow *bflow, char **fullp)
+{
+	char dirpath[256];
+	char *best = NULL;
+	int ret;
+	int i;
+
+	for (i = 0; prefixes && prefixes[i]; i++) {
+		struct fs_dir_stream *dirs;
+		struct fs_dirent *dent;
+
+		/* fs_closedir() below resets the global fs_type. */
+		ret = bootmeth_setup_fs(bflow, desc);
+		if (ret) {
+			free(best);
+			return log_msg_ret("fs", ret);
+		}
+
+		snprintf(dirpath, sizeof(dirpath), "%s%s",
+			 prefixes[i], BLS_DIR);
+		dirs = fs_opendir(dirpath);
+		if (!dirs)
+			continue;
+
+		while ((dent = fs_readdir(dirs))) {
+			size_t len = strlen(dent->name);
+			char *full;
+
+			if (dent->type != FS_DT_REG)
+				continue;
+			if (len <= strlen(BLS_SUFFIX))
+				continue;
+			if (strcmp(dent->name + len - strlen(BLS_SUFFIX),
+				   BLS_SUFFIX))
+				continue;
+
+			full = malloc(strlen(dirpath) + 1 + len + 1);
+			if (!full) {
+				free(best);
+				fs_closedir(dirs);
+				return -ENOMEM;
+			}
+			sprintf(full, "%s/%s", dirpath, dent->name);
+
+			if (!best || strcmp(full, best) > 0) {
+				free(best);
+				best = full;
+			} else {
+				free(full);
+			}
+		}
+		fs_closedir(dirs);
+	}
+
+	if (!best)
+		return -ENOENT;
+
+	*fullp = best;
+
+	return 0;
+}
+
+/*
+ * TODO: BLS entry filenames may carry a boot-counter suffix of the form
+ * '+TRIES_LEFT[-TRIES_DONE]' immediately before the .conf extension (see
+ * the spec section on "Boot counting"). When that is implemented, this
+ * bootmeth should:
+ *   - parse and strip the suffix from the displayed entry name,
+ *   - skip entries whose TRIES_LEFT has reached zero,
+ *   - decrement TRIES_LEFT (renaming the file) on each boot attempt.
+ * For now the suffix is left intact in the entry name and ignored.
+ */
+static int bls_read_bootflow(struct udevice *dev, struct bootflow *bflow)
+{
+	struct blk_desc *desc;
+	const char *const *prefixes;
+	struct udevice *bootstd;
+	struct pxe_label *label = NULL;
+	struct pxe_menu scratch = {};
+	char *fpath = NULL;
+	const char *base;
+	char *body;
+	int ret;
+
+	ret = uclass_first_device_err(UCLASS_BOOTSTD, &bootstd);
+	if (ret)
+		return log_msg_ret("std", ret);
+
+	/* We require a partitioned block device */
+	if (!bflow->blk || !bflow->part)
+		return -ENOENT;
+
+	desc = dev_get_uclass_plat(bflow->blk);
+	prefixes = bootstd_get_prefixes(bootstd);
+
+	ret = bls_pick_entry(prefixes, desc, bflow, &fpath);
+	if (ret)
+		return log_msg_ret("scan", ret);
+
+	base = strrchr(fpath, '/');
+	base = base ? base + 1 : fpath;
+
+	/*
+	 * bls_pick_entry() finished with fs_closedir(), which resets the
+	 * global fs_type. Re-mount the partition so bootmeth_try_file()'s
+	 * internal fs_size() call can find the right filesystem driver.
+	 */
+	ret = bootmeth_setup_fs(bflow, desc);
+	if (ret) {
+		free(fpath);
+		return log_msg_ret("fs", ret);
+	}
+
+	ret = bootmeth_try_file(bflow, desc, NULL, fpath);
+	if (ret) {
+		free(fpath);
+		return log_msg_ret("try", ret);
+	}
+
+	ret = bootmeth_alloc_file(bflow, SZ_64K, 1, BFI_EXTLINUX_CFG);
+	if (ret) {
+		free(fpath);
+		return log_msg_ret("read", ret);
+	}
+
+	label = label_create();
+	if (!label) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	/*
+	 * BLS files have no 'label NAME' header — derive the label name from
+	 * the basename (without the .conf suffix) so messages are useful.
+	 */
+	label->name = strndup(base, strlen(base) - strlen(BLS_SUFFIX));
+	if (!label->name) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	body = bflow->buf;
+	ret = parse_label_keys(&body, &scratch, label, true);
+	if (ret < 0)
+		goto err;
+
+	/*
+	 * scratch is only used to give parse_label_keys() somewhere safe to
+	 * stash menu-level state (e.g. a stray 'menu default' line). BLS
+	 * entry files don't contain such lines but defensively free anything
+	 * that did get allocated.
+	 */
+	free(scratch.default_label);
+
+	/*
+	 * label->menu was populated either from a BLS 'title' line (the
+	 * spec-mandated human-readable name) or from a stray 'menu label'
+	 * the parser may have picked up. Fall back to the filename-derived
+	 * label name when neither is present.
+	 */
+	bflow->os_name = strdup(label->menu ? label->menu : label->name);
+	if (!bflow->os_name) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	bflow->bootmeth_priv = label;
+	free(fpath);
+
+	return 0;
+
+err:
+	if (label)
+		label_destroy(label);
+	free(fpath);
+	return log_msg_ret("bls", ret);
+}
+
+static int bls_boot(struct udevice *dev, struct bootflow *bflow)
+{
+	struct cmd_tbl cmdtp = {};	/* dummy */
+	struct pxe_context ctx;
+	struct extlinux_info info;
+	struct pxe_label *label = bflow->bootmeth_priv;
+	int ret;
+
+	if (!label)
+		return log_msg_ret("lbl", -ENOENT);
+
+	info.dev = dev;
+	info.bflow = bflow;
+
+	/*
+	 * BLS paths are absolute relative to the filesystem root of the
+	 * partition the entry lives on. allow_abs_path=true honours that;
+	 * passing NULL as the bootfile keeps the prefix empty so absolute
+	 * paths are not rebased.
+	 */
+	ret = pxe_setup_ctx(&ctx, &cmdtp, bls_getfile, &info, true,
+			    NULL, false, false);
+	if (ret)
+		return log_msg_ret("ctx", -EINVAL);
+
+	ret = label_boot(&ctx, label);
+	pxe_destroy_ctx(&ctx);
+	if (ret)
+		return log_msg_ret("boot", -EINVAL);
+
+	return 0;
+}
+
+static int bls_bootmeth_bind(struct udevice *dev)
+{
+	struct bootmeth_uc_plat *plat = dev_get_uclass_plat(dev);
+
+	plat->desc = IS_ENABLED(CONFIG_BOOTSTD_FULL) ?
+		"Boot Loader Specification" : "bls";
+
+	return 0;
+}
+
+static struct bootmeth_ops bls_bootmeth_ops = {
+	.check		= bls_check,
+	.read_bootflow	= bls_read_bootflow,
+	.read_file	= bootmeth_common_read_file,
+	.boot		= bls_boot,
+};
+
+static const struct udevice_id bls_bootmeth_ids[] = {
+	{ .compatible = "u-boot,bls" },
+	{ }
+};
+
+U_BOOT_DRIVER(bootmeth_2bls) = {
+	.name		= "bootmeth_bls",
+	.id		= UCLASS_BOOTMETH,
+	.of_match	= bls_bootmeth_ids,
+	.ops		= &bls_bootmeth_ops,
+	.bind		= bls_bootmeth_bind,
+};

-- 
2.53.0

Re: [PATCH 0/7] pxe_utils: small fixups, implement BLS type 1 boot on top of them

[Tom Rini]

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

On Thu, Jun 04, 2026 at 07:31:05PM +0400, Alexey Charkov wrote:

> Add support for the Boot Loader Specification (BLS) type 1 boot entries,
> as generated by default by systemd's kernel-install when loader=bls.
> 
> Given that the format of BLS entries is pretty much the same as PXElinux,
> reuse the existing library for the parsing logic.
> 
> BLS type 2 entries are out of scope, as they are effectively just EFI
> applications and should be booted as such.
> 
> This implementation is also only allowing a single top-sorting entry to
> boot, as the standard boot infrastruture currently doesn't support
> multiple entries per partition-bootmeth tuple. I have a proposed extension
> to enable that and will post it separately as RFC - that enables the use
> of `bootflow menu` to select the kernel to boot without jumping through
> several menus with different behavior as is currently required with
> extlinux.conf.
> 
> Signed-off-by: Alexey Charkov <alchark@flipper.net>

This is good to see, thanks for posting it. One concern I have is that
out of 1529 platforms, this would be enabled on 1392 of them. And the
size growth is around 1.5 kilobytes. Is there some smaller percentage of
platforms this should be "default y" for ? Or is the expectation that
something like Debian will be configuring for this moving forward, so
yes even something like PowerPC platforms be using this? Thanks!

-- 
Tom

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

Re: [PATCH 1/7] pxe_utils: fix W=1 kernel-doc warnings

[Tom Rini]

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

On Thu, Jun 04, 2026 at 07:31:06PM +0400, Alexey Charkov wrote:

> Add the missing parameter and enum value descriptions reported by
> scripts/kernel-doc when building with W=1:
> 
>   - get_relfile():           document @type
>   - get_pxelinux_path():     document @pxefile_addr_r
>   - enum lex_state:          document L_NORMAL, L_KEYWORD, L_SLITERAL
>   - get_token():             document @t and @state
> 
> No functional change.
> 
> Signed-off-by: Alexey Charkov <alchark@flipper.net>

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

-- 
Tom

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

Re: [PATCH 2/7] pxe_utils: accept "options" as synonym for "append"

[Tom Rini]

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

On Thu, Jun 04, 2026 at 07:31:07PM +0400, Alexey Charkov wrote:

> The Boot Loader Specification [1] type #2 entry files use the keyword
> "options" for the kernel command line, which corresponds to extlinux's
> "append". Recognising it here lets the existing pxelinux parser ingest
> BLS entries unchanged, paving the way for a BLS bootmeth that reuses
> the parser instead of duplicating it.
> 
> No effect on existing extlinux/pxelinux files: "options" is not a valid
> keyword in those formats, so it cannot collide with prior usage.
> 
> [1] https://uapi-group.org/specifications/specs/boot_loader_specification/
> 
> Signed-off-by: Alexey Charkov <alchark@flipper.net>

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

-- 
Tom

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

Re: [PATCH 3/7] pxe_utils: extract per-entry key parsing into parse_label_keys()

[Tom Rini]

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

On Thu, Jun 04, 2026 at 07:31:08PM +0400, Alexey Charkov wrote:

> Split the body of parse_label() into a standalone parse_label_keys()
> helper that walks key/value lines and populates a pre-existing
> struct pxe_label. parse_label() becomes a thin wrapper that creates
> the label, reads its name, attaches it to the menu, and delegates.
> 
> This is a pure refactor: the new helper contains the original loop
> verbatim, with the local variable declarations moved to its scope.
> No call sites or behaviour change.
> 
> A subsequent change will export this helper so callers parsing
> formats that lack a 'label' header (notably Boot Loader Specification
> type #2 entries) can populate a label directly from a file body
> without duplicating the parser.
> 
> Signed-off-by: Alexey Charkov <alchark@flipper.net>
> ---
>  boot/pxe_utils.c | 58 ++++++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 37 insertions(+), 21 deletions(-)

There's no size impact here at, so this is fine.

-- 
Tom

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

Re: [PATCH 0/7] pxe_utils: small fixups, implement BLS type 1 boot on top of them

[Alexey Charkov]

On Thu, Jun 4, 2026 at 8:05 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Jun 04, 2026 at 07:31:05PM +0400, Alexey Charkov wrote:
>
> > Add support for the Boot Loader Specification (BLS) type 1 boot entries,
> > as generated by default by systemd's kernel-install when loader=bls.
> >
> > Given that the format of BLS entries is pretty much the same as PXElinux,
> > reuse the existing library for the parsing logic.
> >
> > BLS type 2 entries are out of scope, as they are effectively just EFI
> > applications and should be booted as such.
> >
> > This implementation is also only allowing a single top-sorting entry to
> > boot, as the standard boot infrastruture currently doesn't support
> > multiple entries per partition-bootmeth tuple. I have a proposed extension
> > to enable that and will post it separately as RFC - that enables the use
> > of `bootflow menu` to select the kernel to boot without jumping through
> > several menus with different behavior as is currently required with
> > extlinux.conf.
> >
> > Signed-off-by: Alexey Charkov <alchark@flipper.net>
>
> This is good to see, thanks for posting it. One concern I have is that
> out of 1529 platforms, this would be enabled on 1392 of them. And the
> size growth is around 1.5 kilobytes. Is there some smaller percentage of
> platforms this should be "default y" for ? Or is the expectation that
> something like Debian will be configuring for this moving forward, so
> yes even something like PowerPC platforms be using this? Thanks!

Well this is the config format that systemd's kernel-install produces
out of the box, and it doesn't require EFI support unlike UKI (a.k.a.
BLS type 2). So one could argue that any system shipping with systemd
benefits directly. In my view, that makes it more accessible for end
users than e.g. bootmeth_extlinux, which is also "default y" but I
could only find a suitable config generator for it with device tree
support in Debian (not in Fedora, nor in Gentoo out of my extremely
representative sample of N=3 distros).

Happy to make it conditional on whatever platforms find this helpful
though, or drop "default y" altogether. I personally only care about
ARM, and Rockchip in particular :)

Best regards,
Alexey

Re: [PATCH 0/7] pxe_utils: small fixups, implement BLS type 1 boot on top of them

[Tom Rini]

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

On Thu, Jun 04, 2026 at 09:16:58PM +0400, Alexey Charkov wrote:
> On Thu, Jun 4, 2026 at 8:05 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Jun 04, 2026 at 07:31:05PM +0400, Alexey Charkov wrote:
> >
> > > Add support for the Boot Loader Specification (BLS) type 1 boot entries,
> > > as generated by default by systemd's kernel-install when loader=bls.
> > >
> > > Given that the format of BLS entries is pretty much the same as PXElinux,
> > > reuse the existing library for the parsing logic.
> > >
> > > BLS type 2 entries are out of scope, as they are effectively just EFI
> > > applications and should be booted as such.
> > >
> > > This implementation is also only allowing a single top-sorting entry to
> > > boot, as the standard boot infrastruture currently doesn't support
> > > multiple entries per partition-bootmeth tuple. I have a proposed extension
> > > to enable that and will post it separately as RFC - that enables the use
> > > of `bootflow menu` to select the kernel to boot without jumping through
> > > several menus with different behavior as is currently required with
> > > extlinux.conf.
> > >
> > > Signed-off-by: Alexey Charkov <alchark@flipper.net>
> >
> > This is good to see, thanks for posting it. One concern I have is that
> > out of 1529 platforms, this would be enabled on 1392 of them. And the
> > size growth is around 1.5 kilobytes. Is there some smaller percentage of
> > platforms this should be "default y" for ? Or is the expectation that
> > something like Debian will be configuring for this moving forward, so
> > yes even something like PowerPC platforms be using this? Thanks!
> 
> Well this is the config format that systemd's kernel-install produces
> out of the box, and it doesn't require EFI support unlike UKI (a.k.a.
> BLS type 2). So one could argue that any system shipping with systemd
> benefits directly. In my view, that makes it more accessible for end
> users than e.g. bootmeth_extlinux, which is also "default y" but I
> could only find a suitable config generator for it with device tree
> support in Debian (not in Fedora, nor in Gentoo out of my extremely
> representative sample of N=3 distros).
> 
> Happy to make it conditional on whatever platforms find this helpful
> though, or drop "default y" altogether. I personally only care about
> ARM, and Rockchip in particular :)

Right. What I'm asking is, do you know how many distros are planning to
adopt BLS type 1 support by default? And since you're on Rockchip, why
not just use BLS type 2?

-- 
Tom

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

Re: [PATCH 0/7] pxe_utils: small fixups, implement BLS type 1 boot on top of them

[Alexey Charkov]

On Thu, Jun 4, 2026 at 9:21 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Jun 04, 2026 at 09:16:58PM +0400, Alexey Charkov wrote:
> > On Thu, Jun 4, 2026 at 8:05 PM Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Thu, Jun 04, 2026 at 07:31:05PM +0400, Alexey Charkov wrote:
> > >
> > > > Add support for the Boot Loader Specification (BLS) type 1 boot entries,
> > > > as generated by default by systemd's kernel-install when loader=bls.
> > > >
> > > > Given that the format of BLS entries is pretty much the same as PXElinux,
> > > > reuse the existing library for the parsing logic.
> > > >
> > > > BLS type 2 entries are out of scope, as they are effectively just EFI
> > > > applications and should be booted as such.
> > > >
> > > > This implementation is also only allowing a single top-sorting entry to
> > > > boot, as the standard boot infrastruture currently doesn't support
> > > > multiple entries per partition-bootmeth tuple. I have a proposed extension
> > > > to enable that and will post it separately as RFC - that enables the use
> > > > of `bootflow menu` to select the kernel to boot without jumping through
> > > > several menus with different behavior as is currently required with
> > > > extlinux.conf.
> > > >
> > > > Signed-off-by: Alexey Charkov <alchark@flipper.net>
> > >
> > > This is good to see, thanks for posting it. One concern I have is that
> > > out of 1529 platforms, this would be enabled on 1392 of them. And the
> > > size growth is around 1.5 kilobytes. Is there some smaller percentage of
> > > platforms this should be "default y" for ? Or is the expectation that
> > > something like Debian will be configuring for this moving forward, so
> > > yes even something like PowerPC platforms be using this? Thanks!
> >
> > Well this is the config format that systemd's kernel-install produces
> > out of the box, and it doesn't require EFI support unlike UKI (a.k.a.
> > BLS type 2). So one could argue that any system shipping with systemd
> > benefits directly. In my view, that makes it more accessible for end
> > users than e.g. bootmeth_extlinux, which is also "default y" but I
> > could only find a suitable config generator for it with device tree
> > support in Debian (not in Fedora, nor in Gentoo out of my extremely
> > representative sample of N=3 distros).
> >
> > Happy to make it conditional on whatever platforms find this helpful
> > though, or drop "default y" altogether. I personally only care about
> > ARM, and Rockchip in particular :)
>
> Right. What I'm asking is, do you know how many distros are planning to
> adopt BLS type 1 support by default? And since you're on Rockchip, why
> not just use BLS type 2?

I've only seen Fedora use it by default, but I'm not that much of a
distro connoisseur. There might be more, or there might not.

BLS type 2 didn't work for me last time I tried it because it
seemingly wanted to bake the DTB into the UKI and couldn't handle DT
overlays, which made booting different boards with the same kernel
binary a pain. BLS type 1 didn't have those issues, and a concise
plaintext config has its (subjective) appeal over
yet-another-packed-binary-format.

Best regards,
Alexey

Re: [PATCH 0/7] pxe_utils: small fixups, implement BLS type 1 boot on top of them

[Simon Glass]

Hi,

On Thu, 4 Jun 2026 at 11:35, Alexey Charkov <alchark@flipper.net> wrote:
>
> On Thu, Jun 4, 2026 at 9:21 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Jun 04, 2026 at 09:16:58PM +0400, Alexey Charkov wrote:
> > > On Thu, Jun 4, 2026 at 8:05 PM Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Thu, Jun 04, 2026 at 07:31:05PM +0400, Alexey Charkov wrote:
> > > >
> > > > > Add support for the Boot Loader Specification (BLS) type 1 boot entries,
> > > > > as generated by default by systemd's kernel-install when loader=bls.
> > > > >
> > > > > Given that the format of BLS entries is pretty much the same as PXElinux,
> > > > > reuse the existing library for the parsing logic.
> > > > >
> > > > > BLS type 2 entries are out of scope, as they are effectively just EFI
> > > > > applications and should be booted as such.
> > > > >
> > > > > This implementation is also only allowing a single top-sorting entry to
> > > > > boot, as the standard boot infrastruture currently doesn't support
> > > > > multiple entries per partition-bootmeth tuple. I have a proposed extension
> > > > > to enable that and will post it separately as RFC - that enables the use
> > > > > of `bootflow menu` to select the kernel to boot without jumping through
> > > > > several menus with different behavior as is currently required with
> > > > > extlinux.conf.
> > > > >
> > > > > Signed-off-by: Alexey Charkov <alchark@flipper.net>
> > > >
> > > > This is good to see, thanks for posting it. One concern I have is that
> > > > out of 1529 platforms, this would be enabled on 1392 of them. And the
> > > > size growth is around 1.5 kilobytes. Is there some smaller percentage of
> > > > platforms this should be "default y" for ? Or is the expectation that
> > > > something like Debian will be configuring for this moving forward, so
> > > > yes even something like PowerPC platforms be using this? Thanks!
> > >
> > > Well this is the config format that systemd's kernel-install produces
> > > out of the box, and it doesn't require EFI support unlike UKI (a.k.a.
> > > BLS type 2). So one could argue that any system shipping with systemd
> > > benefits directly. In my view, that makes it more accessible for end
> > > users than e.g. bootmeth_extlinux, which is also "default y" but I
> > > could only find a suitable config generator for it with device tree
> > > support in Debian (not in Fedora, nor in Gentoo out of my extremely
> > > representative sample of N=3 distros).
> > >
> > > Happy to make it conditional on whatever platforms find this helpful
> > > though, or drop "default y" altogether. I personally only care about
> > > ARM, and Rockchip in particular :)
> >
> > Right. What I'm asking is, do you know how many distros are planning to
> > adopt BLS type 1 support by default? And since you're on Rockchip, why
> > not just use BLS type 2?
>
> I've only seen Fedora use it by default, but I'm not that much of a
> distro connoisseur. There might be more, or there might not.
>
> BLS type 2 didn't work for me last time I tried it because it
> seemingly wanted to bake the DTB into the UKI and couldn't handle DT
> overlays, which made booting different boards with the same kernel
> binary a pain. BLS type 1 didn't have those issues, and a concise
> plaintext config has its (subjective) appeal over
> yet-another-packed-binary-format.

BLS type 2 is an EFI app, so a huge amount of extra code and
complexity to do the same thing.

I already have BLS in the Concept tree and would love to see it in
mainline. Relevant to this I have:

- refactoring so that pxe_utils can support parsing without booting
- support in bootstd for multiple bootflows per partition
- a script and test for booting Ubuntu this way (i.e. without EFI)

I'm not entirely sure of how to upstream this, given all the
refactoring that is needed, but I'll take another look.

Regards,
Simon

Re: [PATCH 7/7] boot: add a minimal bootmeth for the Boot Loader Specification

[Simon Glass]

Hi Alexey,

On 2026-06-04T15:31:05, Alexey Charkov <alchark@flipper.net> wrote:
> boot: add a minimal bootmeth for the Boot Loader Specification
>
> Add a bootmeth that finds and boots Boot Loader Specification (BLS)
> type #1 entry files [1]. On each block-device partition it scans, the
> bootmeth looks for files matching '<prefix>loader/entries/*.conf'
> (where <prefix> comes from bootstd_get_prefixes(), typically '/' and
> '/boot/'), picks the highest-sorting filename, parses it, and exposes
> it as a bootflow.
>
> Implementation reuses the existing pxelinux infrastructure.
>
> For now the entry chosen on a partition is purely the lexicographic
> maximum of *.conf filenames; sort-key / version field handling
> (spec-mandated tiebreakers) and boot-counting (the '+TRIES_LEFT'
> filename suffix) are left as TODOs. Likewise, only the top-sorted entry
> is surfaced because the bootstd framework currently allows one bootflow
> per (bootmeth, partition); exposing every discovered entry will require
> a framework extension.
>
> Type #2 BLS (drop-in directory of EFI binaries) is out of scope here;
> [...]
>
> boot/Kconfig        |  17 +++
>  boot/Makefile       |   1 +
>  boot/bootmeth_bls.c | 333 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 351 insertions(+)

> diff --git a/boot/Kconfig b/boot/Kconfig
> @@ -649,6 +649,23 @@ config BOOTMETH_EXTLINUX_PXE
> +config BOOTMETH_BLS
> +     bool "Bootdev support for Boot Loader Specification entries"
> +     select PXE_UTILS
> +     default y

Wherever the 'default y' discussion lands, this will be enabled on
sandbox, so the 'bootmeth list' tests in test/boot/bootmeth.c will
fail since they check the exact list and count. Please can you run the
sandbox tests and update them as needed?

Since you are adding a new bootmeth you also need a sandbox test that
exercises it (see the extlinux tests in test/boot/bootflow.c, with a
fixture disk image) and a documentation page, e.g.
doc/develop/bootstd/bls.rst with an entry in the index there.

> diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> @@ -0,0 +1,333 @@
> +static int bls_getfile(struct pxe_context *ctx, const char *file_path,
> +                    char *file_addr, enum bootflow_img_t type, ulong *sizep)

This is a verbatim copy of extlinux_getfile() - please can you export
that, or move it into a shared helper?

> diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> @@ -0,0 +1,333 @@
> +     ret = bootmeth_alloc_file(bflow, SZ_64K, 1, BFI_EXTLINUX_CFG);

The extlinux bootmeth passes ARCH_DMA_MINALIGN here, since the buffer
is filled by a block-device read which may use DMA on some platforms.
An alignment of 1 risks cache-line corruption there, so please use
ARCH_DMA_MINALIGN.

> diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> @@ -0,0 +1,333 @@
> +     bflow->bootmeth_priv = label;
> +     free(fpath);

This leaks memory: bootflow_free() releases bootmeth_priv with a plain
free(), so the label's string members (name, menu, kernel, append,
initrd, etc.) are never freed - for every bootflow discarded after a
scan, not just on error paths.

Since get_string() copies tokens out of the buffer rather than
modifying it in place, you could follow the extlinux approach: keep
only bflow->buf across the scan (already populated by
bootmeth_alloc_file() and freed by the framework), use a temporary
label in bls_read_bootflow() just to extract the title, destroy it
with label_destroy(), and re-parse the buffer in bls_boot(). Then
bootmeth_priv is not needed at all. What do you think?

> diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> @@ -0,0 +1,333 @@
> +     ret = pxe_setup_ctx(&ctx, &cmdtp, bls_getfile, &info, true,
> +                         NULL, false, false);

Just to check, how does this behave when the entry is found under a
non-empty prefix? bls_pick_entry() searches every bootstd prefix, but
the prefix the winning entry came from is not recorded. The spec says
paths in an entry are relative to the root of $BOOT, so for an entry
under '/boot/loader/entries/' the kernel must be fetched from
'/boot/...', yet with allow_abs_path set and a NULL bootfile,
get_relfile() uses the absolute path from the partition root and the
lookup will miss. I suspect you need to remember the discovered prefix
(bflow->subdir is a natural place, and is freed automatically) and
prepend it when fetching files.

> diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> @@ -0,0 +1,333 @@
> +U_BOOT_DRIVER(bootmeth_2bls) = {

The number prefix sets the default bootmeth ordering, so this places
BLS after extlinux but ahead of script, efi_mgr and efi on every board
- and '2' is already used by bootmeth_2script, making the relative
order of those two less obvious. Please can you add a comment like the
one in bootmeth_extlinux.c explaining the choice, and say in the
commit message why this position is the right one?

Regards,
Simon

Re: [PATCH 3/7] pxe_utils: extract per-entry key parsing into parse_label_keys()

[Simon Glass]

Hi Alexey,

On 2026-06-04T15:31:05, Alexey Charkov <alchark@flipper.net> wrote:
> pxe_utils: extract per-entry key parsing into parse_label_keys()
>
> Split the body of parse_label() into a standalone parse_label_keys()
> helper that walks key/value lines and populates a pre-existing
> struct pxe_label. parse_label() becomes a thin wrapper that creates
> the label, reads its name, attaches it to the menu, and delegates.
>
> This is a pure refactor: the new helper contains the original loop
> verbatim, with the local variable declarations moved to its scope.
> No call sites or behaviour change.
>
> A subsequent change will export this helper so callers parsing
> formats that lack a 'label' header (notably Boot Loader Specification
> type #2 entries) can populate a label directly from a file body
> without duplicating the parser.
>
> Signed-off-by: Alexey Charkov <alchark@flipper.net>
>
> boot/pxe_utils.c | 58 ++++++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 37 insertions(+), 21 deletions(-)

> diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
> A subsequent change will export this helper so callers parsing
> formats that lack a 'label' header (notably Boot Loader Specification
> type #2 entries) can populate a label directly from a file body
> without duplicating the parser.

This should be type #1, not type #2, right? The cover letter and patch
7 both make clear that BLS type #2 are EFI binaries (out of scope),
and the new bootmeth parses BLS type #1 .conf entry files.

> diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
> @@ -1288,34 +1288,20 @@ static int parse_label_kernel(char **c, struct pxe_label *label)
> +static int parse_label_keys(char **c, struct pxe_menu *cfg,
> +                         struct pxe_label *label)

Please can you add a kernel-doc style comment, since patch 4 promotes
this to a public symbol. The file uses /** ... */ for the exported
helpers added in patch 2.

Regards,
Simon

Re: [PATCH 1/7] pxe_utils: fix W=1 kernel-doc warnings

[Simon Glass]

On 2026-06-04T15:31:05, Alexey Charkov <alchark@flipper.net> wrote:
> pxe_utils: fix W=1 kernel-doc warnings
>
> Add the missing parameter and enum value descriptions reported by
> scripts/kernel-doc when building with W=1:
>
>   - get_relfile():           document @type
>   - get_pxelinux_path():     document @pxefile_addr_r
>   - enum lex_state:          document L_NORMAL, L_KEYWORD, L_SLITERAL
>   - get_token():             document @t and @state
>
> No functional change.
>
> Signed-off-by: Alexey Charkov <alchark@flipper.net>
> Reviewed-by: Tom Rini <trini@konsulko.com>
>
> boot/pxe_utils.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

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

Re: [PATCH 7/7] boot: add a minimal bootmeth for the Boot Loader Specification

[Simon Glass]

Hi Alexey,

On 2026-06-04T15:31:05, Alexey Charkov <alchark@flipper.net> wrote:
> boot: add a minimal bootmeth for the Boot Loader Specification
>
> Add a bootmeth that finds and boots Boot Loader Specification (BLS)
> type #1 entry files [1]. On each block-device partition it scans, the
> bootmeth looks for files matching '<prefix>loader/entries/*.conf'
> (where <prefix> comes from bootstd_get_prefixes(), typically '/' and
> '/boot/'), picks the highest-sorting filename, parses it, and exposes
> it as a bootflow.
>
> Implementation reuses the existing pxelinux infrastructure.
>
> For now the entry chosen on a partition is purely the lexicographic
> maximum of *.conf filenames; sort-key / version field handling
> (spec-mandated tiebreakers) and boot-counting (the '+TRIES_LEFT'
> filename suffix) are left as TODOs. Likewise, only the top-sorted entry
> is surfaced because the bootstd framework currently allows one bootflow
> per (bootmeth, partition); exposing every discovered entry will require
> a framework extension.
>
> Type #2 BLS (drop-in directory of EFI binaries) is out of scope here;
> [...]
>
> boot/Kconfig        |  17 +++
>  boot/Makefile       |   1 +
>  boot/bootmeth_bls.c | 333 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 351 insertions(+)

> diff --git a/boot/Kconfig b/boot/Kconfig
> @@ -649,6 +649,23 @@ config BOOTMETH_EXTLINUX_PXE
> +config BOOTMETH_BLS
> +     bool "Bootdev support for Boot Loader Specification entries"
> +     select PXE_UTILS
> +     default y

Wherever the 'default y' discussion lands, this will be enabled on
sandbox, so the 'bootmeth list' tests in test/boot/bootmeth.c will
fail since they check the exact list and count. Please can you run the
sandbox tests and update them as needed?

Since you are adding a new bootmeth you also need a sandbox test that
exercises it (see the extlinux tests in test/boot/bootflow.c, with a
fixture disk image) and a documentation page, e.g.
doc/develop/bootstd/bls.rst with an entry in the index there.

> diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> @@ -0,0 +1,333 @@
> +static int bls_getfile(struct pxe_context *ctx, const char *file_path,
> +                    char *file_addr, enum bootflow_img_t type, ulong *sizep)

This is a verbatim copy of extlinux_getfile() - please can you export
that, or move it into a shared helper?

> diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> @@ -0,0 +1,333 @@
> +     ret = bootmeth_alloc_file(bflow, SZ_64K, 1, BFI_EXTLINUX_CFG);

The extlinux bootmeth passes ARCH_DMA_MINALIGN here, since the buffer
is filled by a block-device read which may use DMA on some platforms.
An alignment of 1 risks cache-line corruption there, so please use
ARCH_DMA_MINALIGN.

> diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> @@ -0,0 +1,333 @@
> +     bflow->bootmeth_priv = label;
> +     free(fpath);

This leaks memory: bootflow_free() releases bootmeth_priv with a plain
free(), so the label's string members (name, menu, kernel, append,
initrd, etc.) are never freed - for every bootflow discarded after a
scan, not just on error paths.

Since get_string() copies tokens out of the buffer rather than
modifying it in place, you could follow the extlinux approach: keep
only bflow->buf across the scan (already populated by
bootmeth_alloc_file() and freed by the framework), use a temporary
label in bls_read_bootflow() just to extract the title, destroy it
with label_destroy(), and re-parse the buffer in bls_boot(). Then
bootmeth_priv is not needed at all. What do you think?

> diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> @@ -0,0 +1,333 @@
> +     ret = pxe_setup_ctx(&ctx, &cmdtp, bls_getfile, &info, true,
> +                         NULL, false, false);

Just to check, how does this behave when the entry is found under a
non-empty prefix? bls_pick_entry() searches every bootstd prefix, but
the prefix the winning entry came from is not recorded. The spec says
paths in an entry are relative to the root of $BOOT, so for an entry
under '/boot/loader/entries/' the kernel must be fetched from
'/boot/...', yet with allow_abs_path set and a NULL bootfile,
get_relfile() uses the absolute path from the partition root and the
lookup will miss. I suspect you need to remember the discovered prefix
(bflow->subdir is a natural place, and is freed automatically) and
prepend it when fetching files.

> diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> @@ -0,0 +1,333 @@
> +U_BOOT_DRIVER(bootmeth_2bls) = {

The number prefix sets the default bootmeth ordering, so this places
BLS after extlinux but ahead of script, efi_mgr and efi on every board
- and '2' is already used by bootmeth_2script, making the relative
order of those two non-obvious. Please can you add a comment like the
one in bootmeth_extlinux.c explaining the choice, and say in the
commit message why this position is the right one?

Regards,
Simon

Re: [PATCH 1/7] pxe_utils: fix W=1 kernel-doc warnings

[Simon Glass]

On 2026-06-04T15:31:05, Alexey Charkov <alchark@flipper.net> wrote:
> pxe_utils: fix W=1 kernel-doc warnings
>
> Add the missing parameter and enum value descriptions reported by
> scripts/kernel-doc when building with W=1:
>
>   - get_relfile():           document @type
>   - get_pxelinux_path():     document @pxefile_addr_r
>   - enum lex_state:          document L_NORMAL, L_KEYWORD, L_SLITERAL
>   - get_token():             document @t and @state
>
> No functional change.
>
> Signed-off-by: Alexey Charkov <alchark@flipper.net>
> Reviewed-by: Tom Rini <trini@konsulko.com>
>
> boot/pxe_utils.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

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

Re: [PATCH 3/7] pxe_utils: extract per-entry key parsing into parse_label_keys()

[Simon Glass]

Hi Alexey,

On 2026-06-04T15:31:05, Alexey Charkov <alchark@flipper.net> wrote:
> pxe_utils: extract per-entry key parsing into parse_label_keys()
>
> Split the body of parse_label() into a standalone parse_label_keys()
> helper that walks key/value lines and populates a pre-existing
> struct pxe_label. parse_label() becomes a thin wrapper that creates
> the label, reads its name, attaches it to the menu, and delegates.
>
> This is a pure refactor: the new helper contains the original loop
> verbatim, with the local variable declarations moved to its scope.
> No call sites or behaviour change.
>
> A subsequent change will export this helper so callers parsing
> formats that lack a 'label' header (notably Boot Loader Specification
> type #2 entries) can populate a label directly from a file body
> without duplicating the parser.
>
> Signed-off-by: Alexey Charkov <alchark@flipper.net>
>
> boot/pxe_utils.c | 58 ++++++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 37 insertions(+), 21 deletions(-)

> diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
> A subsequent change will export this helper so callers parsing
> formats that lack a 'label' header (notably Boot Loader Specification
> type #2 entries) can populate a label directly from a file body
> without duplicating the parser.

This should be type #1, not type #2. The cover letter and patch 7 both
make clear that BLS type #2 are EFI binaries (out of scope), and the
new bootmeth parses BLS type #1 .conf entry files.

> diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
> @@ -1288,34 +1288,20 @@ static int parse_label_kernel(char **c, struct pxe_label *label)
> +static int parse_label_keys(char **c, struct pxe_menu *cfg,
> +                         struct pxe_label *label)

Please can you add a kernel-doc style comment, since patch 4 promotes
this to a public symbol. The file uses /** ... */ for the exported
helpers added in patch 2.

Regards,
Simon

Re: [PATCH 7/7] boot: add a minimal bootmeth for the Boot Loader Specification

[Alexey Charkov]

Hi Simon,

On Thu, Jun 25, 2026 at 7:28 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Alexey,
>
> On 2026-06-04T15:31:05, Alexey Charkov <alchark@flipper.net> wrote:
> > boot: add a minimal bootmeth for the Boot Loader Specification
> >
> > Add a bootmeth that finds and boots Boot Loader Specification (BLS)
> > type #1 entry files [1]. On each block-device partition it scans, the
> > bootmeth looks for files matching '<prefix>loader/entries/*.conf'
> > (where <prefix> comes from bootstd_get_prefixes(), typically '/' and
> > '/boot/'), picks the highest-sorting filename, parses it, and exposes
> > it as a bootflow.
> >
> > Implementation reuses the existing pxelinux infrastructure.
> >
> > For now the entry chosen on a partition is purely the lexicographic
> > maximum of *.conf filenames; sort-key / version field handling
> > (spec-mandated tiebreakers) and boot-counting (the '+TRIES_LEFT'
> > filename suffix) are left as TODOs. Likewise, only the top-sorted entry
> > is surfaced because the bootstd framework currently allows one bootflow
> > per (bootmeth, partition); exposing every discovered entry will require
> > a framework extension.
> >
> > Type #2 BLS (drop-in directory of EFI binaries) is out of scope here;
> > [...]
> >
> > boot/Kconfig        |  17 +++
> >  boot/Makefile       |   1 +
> >  boot/bootmeth_bls.c | 333 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 351 insertions(+)
>
> > diff --git a/boot/Kconfig b/boot/Kconfig
> > @@ -649,6 +649,23 @@ config BOOTMETH_EXTLINUX_PXE
> > +config BOOTMETH_BLS
> > +     bool "Bootdev support for Boot Loader Specification entries"
> > +     select PXE_UTILS
> > +     default y
>
> Wherever the 'default y' discussion lands, this will be enabled on
> sandbox, so the 'bootmeth list' tests in test/boot/bootmeth.c will
> fail since they check the exact list and count. Please can you run the
> sandbox tests and update them as needed?
>
> Since you are adding a new bootmeth you also need a sandbox test that
> exercises it (see the extlinux tests in test/boot/bootflow.c, with a
> fixture disk image) and a documentation page, e.g.
> doc/develop/bootstd/bls.rst with an entry in the index there.

Will do, thanks!

> > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > @@ -0,0 +1,333 @@
> > +static int bls_getfile(struct pxe_context *ctx, const char *file_path,
> > +                    char *file_addr, enum bootflow_img_t type, ulong *sizep)
>
> This is a verbatim copy of extlinux_getfile() - please can you export
> that, or move it into a shared helper?

Ack

> > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > @@ -0,0 +1,333 @@
> > +     ret = bootmeth_alloc_file(bflow, SZ_64K, 1, BFI_EXTLINUX_CFG);
>
> The extlinux bootmeth passes ARCH_DMA_MINALIGN here, since the buffer
> is filled by a block-device read which may use DMA on some platforms.
> An alignment of 1 risks cache-line corruption there, so please use
> ARCH_DMA_MINALIGN.

Ack

> > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > @@ -0,0 +1,333 @@
> > +     bflow->bootmeth_priv = label;
> > +     free(fpath);
>
> This leaks memory: bootflow_free() releases bootmeth_priv with a plain
> free(), so the label's string members (name, menu, kernel, append,
> initrd, etc.) are never freed - for every bootflow discarded after a
> scan, not just on error paths.
>
> Since get_string() copies tokens out of the buffer rather than
> modifying it in place, you could follow the extlinux approach: keep
> only bflow->buf across the scan (already populated by
> bootmeth_alloc_file() and freed by the framework), use a temporary
> label in bls_read_bootflow() just to extract the title, destroy it
> with label_destroy(), and re-parse the buffer in bls_boot(). Then
> bootmeth_priv is not needed at all. What do you think?

Will address the leak, thanks for pointing it out!

The bls_priv structure is needed for my follow-up extension which
allows multiple boot entries to be returned by each (bootdev,
bootmeth, partition) tuple, and I wanted to minimize churn between
those two. I haven't sent that follow-up for review yet, as I wanted
to confirm this simpler version is acceptable first.

> > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > @@ -0,0 +1,333 @@
> > +     ret = pxe_setup_ctx(&ctx, &cmdtp, bls_getfile, &info, true,
> > +                         NULL, false, false);
>
> Just to check, how does this behave when the entry is found under a
> non-empty prefix? bls_pick_entry() searches every bootstd prefix, but
> the prefix the winning entry came from is not recorded. The spec says
> paths in an entry are relative to the root of $BOOT, so for an entry
> under '/boot/loader/entries/' the kernel must be fetched from
> '/boot/...', yet with allow_abs_path set and a NULL bootfile,
> get_relfile() uses the absolute path from the partition root and the
> lookup will miss. I suspect you need to remember the discovered prefix
> (bflow->subdir is a natural place, and is freed automatically) and
> prepend it when fetching files.

Hmm, I am booting it with a non-empty prefix and it works as-is (I use
a single ext4 partition for the root fs, and my entries are under
/boot, i.e. there is this "boot/" prefix from the root of the
partition to get to "loader/entries").

My loader entries use full absolute paths including the "/boot"
component, which is the way systemd's 90-loaderentry.install creates
them in this setup. Or did you mean that this behavior is not
spec-compliant? Not sure if prefixes are spec-compliant anyway, but
definitely useful as an extension.

> > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > @@ -0,0 +1,333 @@
> > +U_BOOT_DRIVER(bootmeth_2bls) = {
>
> The number prefix sets the default bootmeth ordering, so this places
> BLS after extlinux but ahead of script, efi_mgr and efi on every board
> - and '2' is already used by bootmeth_2script, making the relative
> order of those two non-obvious. Please can you add a comment like the
> one in bootmeth_extlinux.c explaining the choice, and say in the
> commit message why this position is the right one?

It should perhaps just lose the number, as there is no legacy to
consider here the way there was one for extlinux. Will adjust in v2.

Thank you for your review!

Best regards,
Alexey

Re: [PATCH 3/7] pxe_utils: extract per-entry key parsing into parse_label_keys()

[Alexey Charkov]

On Thu, Jun 25, 2026 at 7:28 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Alexey,
>
> On 2026-06-04T15:31:05, Alexey Charkov <alchark@flipper.net> wrote:
> > pxe_utils: extract per-entry key parsing into parse_label_keys()
> >
> > Split the body of parse_label() into a standalone parse_label_keys()
> > helper that walks key/value lines and populates a pre-existing
> > struct pxe_label. parse_label() becomes a thin wrapper that creates
> > the label, reads its name, attaches it to the menu, and delegates.
> >
> > This is a pure refactor: the new helper contains the original loop
> > verbatim, with the local variable declarations moved to its scope.
> > No call sites or behaviour change.
> >
> > A subsequent change will export this helper so callers parsing
> > formats that lack a 'label' header (notably Boot Loader Specification
> > type #2 entries) can populate a label directly from a file body
> > without duplicating the parser.
> >
> > Signed-off-by: Alexey Charkov <alchark@flipper.net>
> >
> > boot/pxe_utils.c | 58 ++++++++++++++++++++++++++++++++++++--------------------
> >  1 file changed, 37 insertions(+), 21 deletions(-)
>
> > diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
> > A subsequent change will export this helper so callers parsing
> > formats that lack a 'label' header (notably Boot Loader Specification
> > type #2 entries) can populate a label directly from a file body
> > without duplicating the parser.
>
> This should be type #1, not type #2. The cover letter and patch 7 both
> make clear that BLS type #2 are EFI binaries (out of scope), and the
> new bootmeth parses BLS type #1 .conf entry files.

Yes, it's a typo. There's another one like this in another patch -
will fix in v2.

> > diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
> > @@ -1288,34 +1288,20 @@ static int parse_label_kernel(char **c, struct pxe_label *label)
> > +static int parse_label_keys(char **c, struct pxe_menu *cfg,
> > +                         struct pxe_label *label)
>
> Please can you add a kernel-doc style comment, since patch 4 promotes
> this to a public symbol. The file uses /** ... */ for the exported
> helpers added in patch 2.

Sure. The kernel-doc comment was added in the following commit
(straight to the header), but adding it here already won't hurt (apart
from a slightly larger diff).

Thanks for your review!

Best regards,
Alexey

Re: [PATCH 7/7] boot: add a minimal bootmeth for the Boot Loader Specification

[Simon Glass]

Hi Alexey,

On Thu, 25 Jun 2026 at 16:52, Alexey Charkov <alchark@flipper.net> wrote:
>
> Hi Simon,
>
> On Thu, Jun 25, 2026 at 7:28 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Alexey,
> >
> > On 2026-06-04T15:31:05, Alexey Charkov <alchark@flipper.net> wrote:
> > > boot: add a minimal bootmeth for the Boot Loader Specification
> > >
> > > Add a bootmeth that finds and boots Boot Loader Specification (BLS)
> > > type #1 entry files [1]. On each block-device partition it scans, the
> > > bootmeth looks for files matching '<prefix>loader/entries/*.conf'
> > > (where <prefix> comes from bootstd_get_prefixes(), typically '/' and
> > > '/boot/'), picks the highest-sorting filename, parses it, and exposes
> > > it as a bootflow.
> > >
> > > Implementation reuses the existing pxelinux infrastructure.
> > >
> > > For now the entry chosen on a partition is purely the lexicographic
> > > maximum of *.conf filenames; sort-key / version field handling
> > > (spec-mandated tiebreakers) and boot-counting (the '+TRIES_LEFT'
> > > filename suffix) are left as TODOs. Likewise, only the top-sorted entry
> > > is surfaced because the bootstd framework currently allows one bootflow
> > > per (bootmeth, partition); exposing every discovered entry will require
> > > a framework extension.
> > >
> > > Type #2 BLS (drop-in directory of EFI binaries) is out of scope here;
> > > [...]
> > >
> > > boot/Kconfig        |  17 +++
> > >  boot/Makefile       |   1 +
> > >  boot/bootmeth_bls.c | 333 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 351 insertions(+)
> >
> > > diff --git a/boot/Kconfig b/boot/Kconfig
> > > @@ -649,6 +649,23 @@ config BOOTMETH_EXTLINUX_PXE
> > > +config BOOTMETH_BLS
> > > +     bool "Bootdev support for Boot Loader Specification entries"
> > > +     select PXE_UTILS
> > > +     default y
> >
> > Wherever the 'default y' discussion lands, this will be enabled on
> > sandbox, so the 'bootmeth list' tests in test/boot/bootmeth.c will
> > fail since they check the exact list and count. Please can you run the
> > sandbox tests and update them as needed?
> >
> > Since you are adding a new bootmeth you also need a sandbox test that
> > exercises it (see the extlinux tests in test/boot/bootflow.c, with a
> > fixture disk image) and a documentation page, e.g.
> > doc/develop/bootstd/bls.rst with an entry in the index there.
>
> Will do, thanks!
>
> > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > @@ -0,0 +1,333 @@
> > > +static int bls_getfile(struct pxe_context *ctx, const char *file_path,
> > > +                    char *file_addr, enum bootflow_img_t type, ulong *sizep)
> >
> > This is a verbatim copy of extlinux_getfile() - please can you export
> > that, or move it into a shared helper?
>
> Ack
>
> > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > @@ -0,0 +1,333 @@
> > > +     ret = bootmeth_alloc_file(bflow, SZ_64K, 1, BFI_EXTLINUX_CFG);
> >
> > The extlinux bootmeth passes ARCH_DMA_MINALIGN here, since the buffer
> > is filled by a block-device read which may use DMA on some platforms.
> > An alignment of 1 risks cache-line corruption there, so please use
> > ARCH_DMA_MINALIGN.
>
> Ack
>
> > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > @@ -0,0 +1,333 @@
> > > +     bflow->bootmeth_priv = label;
> > > +     free(fpath);
> >
> > This leaks memory: bootflow_free() releases bootmeth_priv with a plain
> > free(), so the label's string members (name, menu, kernel, append,
> > initrd, etc.) are never freed - for every bootflow discarded after a
> > scan, not just on error paths.
> >
> > Since get_string() copies tokens out of the buffer rather than
> > modifying it in place, you could follow the extlinux approach: keep
> > only bflow->buf across the scan (already populated by
> > bootmeth_alloc_file() and freed by the framework), use a temporary
> > label in bls_read_bootflow() just to extract the title, destroy it
> > with label_destroy(), and re-parse the buffer in bls_boot(). Then
> > bootmeth_priv is not needed at all. What do you think?
>
> Will address the leak, thanks for pointing it out!
>
> The bls_priv structure is needed for my follow-up extension which
> allows multiple boot entries to be returned by each (bootdev,
> bootmeth, partition) tuple, and I wanted to minimize churn between
> those two. I haven't sent that follow-up for review yet, as I wanted
> to confirm this simpler version is acceptable first.

We should implement this using an generic index rather than something
bls-specific...please see some commits at:

https://concept.deinde.dev/u-boot/u-boot/-/merge_requests/782

>
> > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > @@ -0,0 +1,333 @@
> > > +     ret = pxe_setup_ctx(&ctx, &cmdtp, bls_getfile, &info, true,
> > > +                         NULL, false, false);
> >
> > Just to check, how does this behave when the entry is found under a
> > non-empty prefix? bls_pick_entry() searches every bootstd prefix, but
> > the prefix the winning entry came from is not recorded. The spec says
> > paths in an entry are relative to the root of $BOOT, so for an entry
> > under '/boot/loader/entries/' the kernel must be fetched from
> > '/boot/...', yet with allow_abs_path set and a NULL bootfile,
> > get_relfile() uses the absolute path from the partition root and the
> > lookup will miss. I suspect you need to remember the discovered prefix
> > (bflow->subdir is a natural place, and is freed automatically) and
> > prepend it when fetching files.
>
> Hmm, I am booting it with a non-empty prefix and it works as-is (I use
> a single ext4 partition for the root fs, and my entries are under
> /boot, i.e. there is this "boot/" prefix from the root of the
> partition to get to "loader/entries").
>
> My loader entries use full absolute paths including the "/boot"
> component, which is the way systemd's 90-loaderentry.install creates
> them in this setup. Or did you mean that this behavior is not
> spec-compliant? Not sure if prefixes are spec-compliant anyway, but
> definitely useful as an extension.

Ah OK. I was thinking of extlinux - but what happens if you don't use
full absolute paths? There is the case where the boot partition is a
separate filesystem and the OS files are there, so the running system
sees the file as e.g. /boot/vmlinux (since the boot partition is
mounted at /boot) but U-Boot sees it as /vmlinux (since it does not
have a VFS).

>
> > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > @@ -0,0 +1,333 @@
> > > +U_BOOT_DRIVER(bootmeth_2bls) = {
> >
> > The number prefix sets the default bootmeth ordering, so this places
> > BLS after extlinux but ahead of script, efi_mgr and efi on every board
> > - and '2' is already used by bootmeth_2script, making the relative
> > order of those two non-obvious. Please can you add a comment like the
> > one in bootmeth_extlinux.c explaining the choice, and say in the
> > commit message why this position is the right one?
>
> It should perhaps just lose the number, as there is no legacy to
> consider here the way there was one for extlinux. Will adjust in v2.
>
> Thank you for your review!

You're welcome, thanks for your efforts on this!

Regards,
Simon

Re: [PATCH 7/7] boot: add a minimal bootmeth for the Boot Loader Specification

[Alexey Charkov]

On Thu, Jun 25, 2026 at 8:24 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Alexey,
>
> On Thu, 25 Jun 2026 at 16:52, Alexey Charkov <alchark@flipper.net> wrote:
> >
> > Hi Simon,
> >
> > On Thu, Jun 25, 2026 at 7:28 PM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Alexey,
> > >
> > > On 2026-06-04T15:31:05, Alexey Charkov <alchark@flipper.net> wrote:
> > > > boot: add a minimal bootmeth for the Boot Loader Specification
> > > >
> > > > Add a bootmeth that finds and boots Boot Loader Specification (BLS)
> > > > type #1 entry files [1]. On each block-device partition it scans, the
> > > > bootmeth looks for files matching '<prefix>loader/entries/*.conf'
> > > > (where <prefix> comes from bootstd_get_prefixes(), typically '/' and
> > > > '/boot/'), picks the highest-sorting filename, parses it, and exposes
> > > > it as a bootflow.
> > > >
> > > > Implementation reuses the existing pxelinux infrastructure.
> > > >
> > > > For now the entry chosen on a partition is purely the lexicographic
> > > > maximum of *.conf filenames; sort-key / version field handling
> > > > (spec-mandated tiebreakers) and boot-counting (the '+TRIES_LEFT'
> > > > filename suffix) are left as TODOs. Likewise, only the top-sorted entry
> > > > is surfaced because the bootstd framework currently allows one bootflow
> > > > per (bootmeth, partition); exposing every discovered entry will require
> > > > a framework extension.
> > > >
> > > > Type #2 BLS (drop-in directory of EFI binaries) is out of scope here;
> > > > [...]
> > > >
> > > > boot/Kconfig        |  17 +++
> > > >  boot/Makefile       |   1 +
> > > >  boot/bootmeth_bls.c | 333 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  3 files changed, 351 insertions(+)
> > >
> > > > diff --git a/boot/Kconfig b/boot/Kconfig
> > > > @@ -649,6 +649,23 @@ config BOOTMETH_EXTLINUX_PXE
> > > > +config BOOTMETH_BLS
> > > > +     bool "Bootdev support for Boot Loader Specification entries"
> > > > +     select PXE_UTILS
> > > > +     default y
> > >
> > > Wherever the 'default y' discussion lands, this will be enabled on
> > > sandbox, so the 'bootmeth list' tests in test/boot/bootmeth.c will
> > > fail since they check the exact list and count. Please can you run the
> > > sandbox tests and update them as needed?
> > >
> > > Since you are adding a new bootmeth you also need a sandbox test that
> > > exercises it (see the extlinux tests in test/boot/bootflow.c, with a
> > > fixture disk image) and a documentation page, e.g.
> > > doc/develop/bootstd/bls.rst with an entry in the index there.
> >
> > Will do, thanks!
> >
> > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > @@ -0,0 +1,333 @@
> > > > +static int bls_getfile(struct pxe_context *ctx, const char *file_path,
> > > > +                    char *file_addr, enum bootflow_img_t type, ulong *sizep)
> > >
> > > This is a verbatim copy of extlinux_getfile() - please can you export
> > > that, or move it into a shared helper?
> >
> > Ack
> >
> > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > @@ -0,0 +1,333 @@
> > > > +     ret = bootmeth_alloc_file(bflow, SZ_64K, 1, BFI_EXTLINUX_CFG);
> > >
> > > The extlinux bootmeth passes ARCH_DMA_MINALIGN here, since the buffer
> > > is filled by a block-device read which may use DMA on some platforms.
> > > An alignment of 1 risks cache-line corruption there, so please use
> > > ARCH_DMA_MINALIGN.
> >
> > Ack
> >
> > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > @@ -0,0 +1,333 @@
> > > > +     bflow->bootmeth_priv = label;
> > > > +     free(fpath);
> > >
> > > This leaks memory: bootflow_free() releases bootmeth_priv with a plain
> > > free(), so the label's string members (name, menu, kernel, append,
> > > initrd, etc.) are never freed - for every bootflow discarded after a
> > > scan, not just on error paths.
> > >
> > > Since get_string() copies tokens out of the buffer rather than
> > > modifying it in place, you could follow the extlinux approach: keep
> > > only bflow->buf across the scan (already populated by
> > > bootmeth_alloc_file() and freed by the framework), use a temporary
> > > label in bls_read_bootflow() just to extract the title, destroy it
> > > with label_destroy(), and re-parse the buffer in bls_boot(). Then
> > > bootmeth_priv is not needed at all. What do you think?
> >
> > Will address the leak, thanks for pointing it out!
> >
> > The bls_priv structure is needed for my follow-up extension which
> > allows multiple boot entries to be returned by each (bootdev,
> > bootmeth, partition) tuple, and I wanted to minimize churn between
> > those two. I haven't sent that follow-up for review yet, as I wanted
> > to confirm this simpler version is acceptable first.
>
> We should implement this using an generic index rather than something
> bls-specific...please see some commits at:
>
> https://concept.deinde.dev/u-boot/u-boot/-/merge_requests/782

Oh, there's already BLS in your tree :-D

Would you like to merge that instead? Your version looks much more
full-fledged. If you happen to have one rebased on master or next I'd
be happy to test!

In my attempt at multi-bootflows I just pass an extra 'seq' parameter
to each bootmeth. If all bootflows on a partition are exhausted, or if
the bootmeth in question doesn't support multiple bootflows per
partition (and seq > 0 is passed) they return -ENOENT, so that the
caller can proceed to the next (bootdev, bootmeth, partition) tuple.
Otherwise the caller repeats with the same tuple until it gets
-ENOENT. So no extra flag to indicate the multi-bootflow capability as
in your version.

For BLS, I store an alist caching all discovered entries per partition
inside the `priv` structure to avoid needlessly repeating directory
operations.

> > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > @@ -0,0 +1,333 @@
> > > > +     ret = pxe_setup_ctx(&ctx, &cmdtp, bls_getfile, &info, true,
> > > > +                         NULL, false, false);
> > >
> > > Just to check, how does this behave when the entry is found under a
> > > non-empty prefix? bls_pick_entry() searches every bootstd prefix, but
> > > the prefix the winning entry came from is not recorded. The spec says
> > > paths in an entry are relative to the root of $BOOT, so for an entry
> > > under '/boot/loader/entries/' the kernel must be fetched from
> > > '/boot/...', yet with allow_abs_path set and a NULL bootfile,
> > > get_relfile() uses the absolute path from the partition root and the
> > > lookup will miss. I suspect you need to remember the discovered prefix
> > > (bflow->subdir is a natural place, and is freed automatically) and
> > > prepend it when fetching files.
> >
> > Hmm, I am booting it with a non-empty prefix and it works as-is (I use
> > a single ext4 partition for the root fs, and my entries are under
> > /boot, i.e. there is this "boot/" prefix from the root of the
> > partition to get to "loader/entries").
> >
> > My loader entries use full absolute paths including the "/boot"
> > component, which is the way systemd's 90-loaderentry.install creates
> > them in this setup. Or did you mean that this behavior is not
> > spec-compliant? Not sure if prefixes are spec-compliant anyway, but
> > definitely useful as an extension.
>
> Ah OK. I was thinking of extlinux - but what happens if you don't use
> full absolute paths? There is the case where the boot partition is a
> separate filesystem and the OS files are there, so the running system
> sees the file as e.g. /boot/vmlinux (since the boot partition is
> mounted at /boot) but U-Boot sees it as /vmlinux (since it does not
> have a VFS).

I will recheck with a separate partition and paths stemming from its
root (i.e. /vmlinuz or /[entry-key]/[entry-id]/linux), thank you!

Best regards,
Alexey

Re: [PATCH 7/7] boot: add a minimal bootmeth for the Boot Loader Specification

[Tom Rini]

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

On Thu, Jun 25, 2026 at 08:57:13PM +0400, Alexey Charkov wrote:
> On Thu, Jun 25, 2026 at 8:24 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Alexey,
> >
> > On Thu, 25 Jun 2026 at 16:52, Alexey Charkov <alchark@flipper.net> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Thu, Jun 25, 2026 at 7:28 PM Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Alexey,
> > > >
> > > > On 2026-06-04T15:31:05, Alexey Charkov <alchark@flipper.net> wrote:
> > > > > boot: add a minimal bootmeth for the Boot Loader Specification
> > > > >
> > > > > Add a bootmeth that finds and boots Boot Loader Specification (BLS)
> > > > > type #1 entry files [1]. On each block-device partition it scans, the
> > > > > bootmeth looks for files matching '<prefix>loader/entries/*.conf'
> > > > > (where <prefix> comes from bootstd_get_prefixes(), typically '/' and
> > > > > '/boot/'), picks the highest-sorting filename, parses it, and exposes
> > > > > it as a bootflow.
> > > > >
> > > > > Implementation reuses the existing pxelinux infrastructure.
> > > > >
> > > > > For now the entry chosen on a partition is purely the lexicographic
> > > > > maximum of *.conf filenames; sort-key / version field handling
> > > > > (spec-mandated tiebreakers) and boot-counting (the '+TRIES_LEFT'
> > > > > filename suffix) are left as TODOs. Likewise, only the top-sorted entry
> > > > > is surfaced because the bootstd framework currently allows one bootflow
> > > > > per (bootmeth, partition); exposing every discovered entry will require
> > > > > a framework extension.
> > > > >
> > > > > Type #2 BLS (drop-in directory of EFI binaries) is out of scope here;
> > > > > [...]
> > > > >
> > > > > boot/Kconfig        |  17 +++
> > > > >  boot/Makefile       |   1 +
> > > > >  boot/bootmeth_bls.c | 333 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > >  3 files changed, 351 insertions(+)
> > > >
> > > > > diff --git a/boot/Kconfig b/boot/Kconfig
> > > > > @@ -649,6 +649,23 @@ config BOOTMETH_EXTLINUX_PXE
> > > > > +config BOOTMETH_BLS
> > > > > +     bool "Bootdev support for Boot Loader Specification entries"
> > > > > +     select PXE_UTILS
> > > > > +     default y
> > > >
> > > > Wherever the 'default y' discussion lands, this will be enabled on
> > > > sandbox, so the 'bootmeth list' tests in test/boot/bootmeth.c will
> > > > fail since they check the exact list and count. Please can you run the
> > > > sandbox tests and update them as needed?
> > > >
> > > > Since you are adding a new bootmeth you also need a sandbox test that
> > > > exercises it (see the extlinux tests in test/boot/bootflow.c, with a
> > > > fixture disk image) and a documentation page, e.g.
> > > > doc/develop/bootstd/bls.rst with an entry in the index there.
> > >
> > > Will do, thanks!
> > >
> > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > @@ -0,0 +1,333 @@
> > > > > +static int bls_getfile(struct pxe_context *ctx, const char *file_path,
> > > > > +                    char *file_addr, enum bootflow_img_t type, ulong *sizep)
> > > >
> > > > This is a verbatim copy of extlinux_getfile() - please can you export
> > > > that, or move it into a shared helper?
> > >
> > > Ack
> > >
> > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > @@ -0,0 +1,333 @@
> > > > > +     ret = bootmeth_alloc_file(bflow, SZ_64K, 1, BFI_EXTLINUX_CFG);
> > > >
> > > > The extlinux bootmeth passes ARCH_DMA_MINALIGN here, since the buffer
> > > > is filled by a block-device read which may use DMA on some platforms.
> > > > An alignment of 1 risks cache-line corruption there, so please use
> > > > ARCH_DMA_MINALIGN.
> > >
> > > Ack
> > >
> > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > @@ -0,0 +1,333 @@
> > > > > +     bflow->bootmeth_priv = label;
> > > > > +     free(fpath);
> > > >
> > > > This leaks memory: bootflow_free() releases bootmeth_priv with a plain
> > > > free(), so the label's string members (name, menu, kernel, append,
> > > > initrd, etc.) are never freed - for every bootflow discarded after a
> > > > scan, not just on error paths.
> > > >
> > > > Since get_string() copies tokens out of the buffer rather than
> > > > modifying it in place, you could follow the extlinux approach: keep
> > > > only bflow->buf across the scan (already populated by
> > > > bootmeth_alloc_file() and freed by the framework), use a temporary
> > > > label in bls_read_bootflow() just to extract the title, destroy it
> > > > with label_destroy(), and re-parse the buffer in bls_boot(). Then
> > > > bootmeth_priv is not needed at all. What do you think?
> > >
> > > Will address the leak, thanks for pointing it out!
> > >
> > > The bls_priv structure is needed for my follow-up extension which
> > > allows multiple boot entries to be returned by each (bootdev,
> > > bootmeth, partition) tuple, and I wanted to minimize churn between
> > > those two. I haven't sent that follow-up for review yet, as I wanted
> > > to confirm this simpler version is acceptable first.
> >
> > We should implement this using an generic index rather than something
> > bls-specific...please see some commits at:
> >
> > https://concept.deinde.dev/u-boot/u-boot/-/merge_requests/782
> 
> Oh, there's already BLS in your tree :-D
> 
> Would you like to merge that instead? Your version looks much more
> full-fledged. If you happen to have one rebased on master or next I'd
> be happy to test!

Unfortunately Simon's work here is AI-generated and so not particularly
welcome, among other problems right now.

-- 
Tom

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

Re: [PATCH 7/7] boot: add a minimal bootmeth for the Boot Loader Specification

[Simon Glass]

Hi Alexey,

On Thu, 25 Jun 2026 at 17:57, Alexey Charkov <alchark@flipper.net> wrote:
>
> On Thu, Jun 25, 2026 at 8:24 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Alexey,
> >
> > On Thu, 25 Jun 2026 at 16:52, Alexey Charkov <alchark@flipper.net> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Thu, Jun 25, 2026 at 7:28 PM Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Alexey,
> > > >
> > > > On 2026-06-04T15:31:05, Alexey Charkov <alchark@flipper.net> wrote:
> > > > > boot: add a minimal bootmeth for the Boot Loader Specification
> > > > >
> > > > > Add a bootmeth that finds and boots Boot Loader Specification (BLS)
> > > > > type #1 entry files [1]. On each block-device partition it scans, the
> > > > > bootmeth looks for files matching '<prefix>loader/entries/*.conf'
> > > > > (where <prefix> comes from bootstd_get_prefixes(), typically '/' and
> > > > > '/boot/'), picks the highest-sorting filename, parses it, and exposes
> > > > > it as a bootflow.
> > > > >
> > > > > Implementation reuses the existing pxelinux infrastructure.
> > > > >
> > > > > For now the entry chosen on a partition is purely the lexicographic
> > > > > maximum of *.conf filenames; sort-key / version field handling
> > > > > (spec-mandated tiebreakers) and boot-counting (the '+TRIES_LEFT'
> > > > > filename suffix) are left as TODOs. Likewise, only the top-sorted entry
> > > > > is surfaced because the bootstd framework currently allows one bootflow
> > > > > per (bootmeth, partition); exposing every discovered entry will require
> > > > > a framework extension.
> > > > >
> > > > > Type #2 BLS (drop-in directory of EFI binaries) is out of scope here;
> > > > > [...]
> > > > >
> > > > > boot/Kconfig        |  17 +++
> > > > >  boot/Makefile       |   1 +
> > > > >  boot/bootmeth_bls.c | 333 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > >  3 files changed, 351 insertions(+)
> > > >
> > > > > diff --git a/boot/Kconfig b/boot/Kconfig
> > > > > @@ -649,6 +649,23 @@ config BOOTMETH_EXTLINUX_PXE
> > > > > +config BOOTMETH_BLS
> > > > > +     bool "Bootdev support for Boot Loader Specification entries"
> > > > > +     select PXE_UTILS
> > > > > +     default y
> > > >
> > > > Wherever the 'default y' discussion lands, this will be enabled on
> > > > sandbox, so the 'bootmeth list' tests in test/boot/bootmeth.c will
> > > > fail since they check the exact list and count. Please can you run the
> > > > sandbox tests and update them as needed?
> > > >
> > > > Since you are adding a new bootmeth you also need a sandbox test that
> > > > exercises it (see the extlinux tests in test/boot/bootflow.c, with a
> > > > fixture disk image) and a documentation page, e.g.
> > > > doc/develop/bootstd/bls.rst with an entry in the index there.
> > >
> > > Will do, thanks!
> > >
> > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > @@ -0,0 +1,333 @@
> > > > > +static int bls_getfile(struct pxe_context *ctx, const char *file_path,
> > > > > +                    char *file_addr, enum bootflow_img_t type, ulong *sizep)
> > > >
> > > > This is a verbatim copy of extlinux_getfile() - please can you export
> > > > that, or move it into a shared helper?
> > >
> > > Ack
> > >
> > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > @@ -0,0 +1,333 @@
> > > > > +     ret = bootmeth_alloc_file(bflow, SZ_64K, 1, BFI_EXTLINUX_CFG);
> > > >
> > > > The extlinux bootmeth passes ARCH_DMA_MINALIGN here, since the buffer
> > > > is filled by a block-device read which may use DMA on some platforms.
> > > > An alignment of 1 risks cache-line corruption there, so please use
> > > > ARCH_DMA_MINALIGN.
> > >
> > > Ack
> > >
> > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > @@ -0,0 +1,333 @@
> > > > > +     bflow->bootmeth_priv = label;
> > > > > +     free(fpath);
> > > >
> > > > This leaks memory: bootflow_free() releases bootmeth_priv with a plain
> > > > free(), so the label's string members (name, menu, kernel, append,
> > > > initrd, etc.) are never freed - for every bootflow discarded after a
> > > > scan, not just on error paths.
> > > >
> > > > Since get_string() copies tokens out of the buffer rather than
> > > > modifying it in place, you could follow the extlinux approach: keep
> > > > only bflow->buf across the scan (already populated by
> > > > bootmeth_alloc_file() and freed by the framework), use a temporary
> > > > label in bls_read_bootflow() just to extract the title, destroy it
> > > > with label_destroy(), and re-parse the buffer in bls_boot(). Then
> > > > bootmeth_priv is not needed at all. What do you think?
> > >
> > > Will address the leak, thanks for pointing it out!
> > >
> > > The bls_priv structure is needed for my follow-up extension which
> > > allows multiple boot entries to be returned by each (bootdev,
> > > bootmeth, partition) tuple, and I wanted to minimize churn between
> > > those two. I haven't sent that follow-up for review yet, as I wanted
> > > to confirm this simpler version is acceptable first.
> >
> > We should implement this using an generic index rather than something
> > bls-specific...please see some commits at:
> >
> > https://concept.deinde.dev/u-boot/u-boot/-/merge_requests/782
>
> Oh, there's already BLS in your tree :-D
>
> Would you like to merge that instead? Your version looks much more
> full-fledged. If you happen to have one rebased on master or next I'd
> be happy to test!

Unfortunately there is a lot of pxe refactoring, because I wanted to
make it possible to fully parse an extlinux file (plus includes)
without actually booting anything. I actually did look at it a few
weeks back.

Your approach is easier. I can easily add my patches on top of yours
once they land.

>
> In my attempt at multi-bootflows I just pass an extra 'seq' parameter
> to each bootmeth. If all bootflows on a partition are exhausted, or if
> the bootmeth in question doesn't support multiple bootflows per
> partition (and seq > 0 is passed) they return -ENOENT, so that the
> caller can proceed to the next (bootdev, bootmeth, partition) tuple.
> Otherwise the caller repeats with the same tuple until it gets
> -ENOENT. So no extra flag to indicate the multi-bootflow capability as
> in your version.

I'm not wedded to the flag - we can update other bootmeths to return
-ENOENT if the seq is non-zero.

>
> For BLS, I store an alist caching all discovered entries per partition
> inside the `priv` structure to avoid needlessly repeating directory
> operations.

So long as you handle clearing that out, that seems fine.

>
> > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > @@ -0,0 +1,333 @@
> > > > > +     ret = pxe_setup_ctx(&ctx, &cmdtp, bls_getfile, &info, true,
> > > > > +                         NULL, false, false);
> > > >
> > > > Just to check, how does this behave when the entry is found under a
> > > > non-empty prefix? bls_pick_entry() searches every bootstd prefix, but
> > > > the prefix the winning entry came from is not recorded. The spec says
> > > > paths in an entry are relative to the root of $BOOT, so for an entry
> > > > under '/boot/loader/entries/' the kernel must be fetched from
> > > > '/boot/...', yet with allow_abs_path set and a NULL bootfile,
> > > > get_relfile() uses the absolute path from the partition root and the
> > > > lookup will miss. I suspect you need to remember the discovered prefix
> > > > (bflow->subdir is a natural place, and is freed automatically) and
> > > > prepend it when fetching files.
> > >
> > > Hmm, I am booting it with a non-empty prefix and it works as-is (I use
> > > a single ext4 partition for the root fs, and my entries are under
> > > /boot, i.e. there is this "boot/" prefix from the root of the
> > > partition to get to "loader/entries").
> > >
> > > My loader entries use full absolute paths including the "/boot"
> > > component, which is the way systemd's 90-loaderentry.install creates
> > > them in this setup. Or did you mean that this behavior is not
> > > spec-compliant? Not sure if prefixes are spec-compliant anyway, but
> > > definitely useful as an extension.
> >
> > Ah OK. I was thinking of extlinux - but what happens if you don't use
> > full absolute paths? There is the case where the boot partition is a
> > separate filesystem and the OS files are there, so the running system
> > sees the file as e.g. /boot/vmlinux (since the boot partition is
> > mounted at /boot) but U-Boot sees it as /vmlinux (since it does not
> > have a VFS).
>
> I will recheck with a separate partition and paths stemming from its
> root (i.e. /vmlinuz or /[entry-key]/[entry-id]/linux), thank you!

Good to have a test :-)

Regards,
Simon

Re: [PATCH 7/7] boot: add a minimal bootmeth for the Boot Loader Specification

[Simon Glass]

Hi Tom,

On Thu, 25 Jun 2026 at 18:24, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Jun 25, 2026 at 08:57:13PM +0400, Alexey Charkov wrote:
> > On Thu, Jun 25, 2026 at 8:24 PM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Alexey,
> > >
> > > On Thu, 25 Jun 2026 at 16:52, Alexey Charkov <alchark@flipper.net> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Thu, Jun 25, 2026 at 7:28 PM Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > > > > Hi Alexey,
> > > > >
> > > > > On 2026-06-04T15:31:05, Alexey Charkov <alchark@flipper.net> wrote:
> > > > > > boot: add a minimal bootmeth for the Boot Loader Specification
> > > > > >
> > > > > > Add a bootmeth that finds and boots Boot Loader Specification (BLS)
> > > > > > type #1 entry files [1]. On each block-device partition it scans, the
> > > > > > bootmeth looks for files matching '<prefix>loader/entries/*.conf'
> > > > > > (where <prefix> comes from bootstd_get_prefixes(), typically '/' and
> > > > > > '/boot/'), picks the highest-sorting filename, parses it, and exposes
> > > > > > it as a bootflow.
> > > > > >
> > > > > > Implementation reuses the existing pxelinux infrastructure.
> > > > > >
> > > > > > For now the entry chosen on a partition is purely the lexicographic
> > > > > > maximum of *.conf filenames; sort-key / version field handling
> > > > > > (spec-mandated tiebreakers) and boot-counting (the '+TRIES_LEFT'
> > > > > > filename suffix) are left as TODOs. Likewise, only the top-sorted entry
> > > > > > is surfaced because the bootstd framework currently allows one bootflow
> > > > > > per (bootmeth, partition); exposing every discovered entry will require
> > > > > > a framework extension.
> > > > > >
> > > > > > Type #2 BLS (drop-in directory of EFI binaries) is out of scope here;
> > > > > > [...]
> > > > > >
> > > > > > boot/Kconfig        |  17 +++
> > > > > >  boot/Makefile       |   1 +
> > > > > >  boot/bootmeth_bls.c | 333 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > > >  3 files changed, 351 insertions(+)
> > > > >
> > > > > > diff --git a/boot/Kconfig b/boot/Kconfig
> > > > > > @@ -649,6 +649,23 @@ config BOOTMETH_EXTLINUX_PXE
> > > > > > +config BOOTMETH_BLS
> > > > > > +     bool "Bootdev support for Boot Loader Specification entries"
> > > > > > +     select PXE_UTILS
> > > > > > +     default y
> > > > >
> > > > > Wherever the 'default y' discussion lands, this will be enabled on
> > > > > sandbox, so the 'bootmeth list' tests in test/boot/bootmeth.c will
> > > > > fail since they check the exact list and count. Please can you run the
> > > > > sandbox tests and update them as needed?
> > > > >
> > > > > Since you are adding a new bootmeth you also need a sandbox test that
> > > > > exercises it (see the extlinux tests in test/boot/bootflow.c, with a
> > > > > fixture disk image) and a documentation page, e.g.
> > > > > doc/develop/bootstd/bls.rst with an entry in the index there.
> > > >
> > > > Will do, thanks!
> > > >
> > > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > > @@ -0,0 +1,333 @@
> > > > > > +static int bls_getfile(struct pxe_context *ctx, const char *file_path,
> > > > > > +                    char *file_addr, enum bootflow_img_t type, ulong *sizep)
> > > > >
> > > > > This is a verbatim copy of extlinux_getfile() - please can you export
> > > > > that, or move it into a shared helper?
> > > >
> > > > Ack
> > > >
> > > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > > @@ -0,0 +1,333 @@
> > > > > > +     ret = bootmeth_alloc_file(bflow, SZ_64K, 1, BFI_EXTLINUX_CFG);
> > > > >
> > > > > The extlinux bootmeth passes ARCH_DMA_MINALIGN here, since the buffer
> > > > > is filled by a block-device read which may use DMA on some platforms.
> > > > > An alignment of 1 risks cache-line corruption there, so please use
> > > > > ARCH_DMA_MINALIGN.
> > > >
> > > > Ack
> > > >
> > > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > > @@ -0,0 +1,333 @@
> > > > > > +     bflow->bootmeth_priv = label;
> > > > > > +     free(fpath);
> > > > >
> > > > > This leaks memory: bootflow_free() releases bootmeth_priv with a plain
> > > > > free(), so the label's string members (name, menu, kernel, append,
> > > > > initrd, etc.) are never freed - for every bootflow discarded after a
> > > > > scan, not just on error paths.
> > > > >
> > > > > Since get_string() copies tokens out of the buffer rather than
> > > > > modifying it in place, you could follow the extlinux approach: keep
> > > > > only bflow->buf across the scan (already populated by
> > > > > bootmeth_alloc_file() and freed by the framework), use a temporary
> > > > > label in bls_read_bootflow() just to extract the title, destroy it
> > > > > with label_destroy(), and re-parse the buffer in bls_boot(). Then
> > > > > bootmeth_priv is not needed at all. What do you think?
> > > >
> > > > Will address the leak, thanks for pointing it out!
> > > >
> > > > The bls_priv structure is needed for my follow-up extension which
> > > > allows multiple boot entries to be returned by each (bootdev,
> > > > bootmeth, partition) tuple, and I wanted to minimize churn between
> > > > those two. I haven't sent that follow-up for review yet, as I wanted
> > > > to confirm this simpler version is acceptable first.
> > >
> > > We should implement this using an generic index rather than something
> > > bls-specific...please see some commits at:
> > >
> > > https://concept.deinde.dev/u-boot/u-boot/-/merge_requests/782
> >
> > Oh, there's already BLS in your tree :-D
> >
> > Would you like to merge that instead? Your version looks much more
> > full-fledged. If you happen to have one rebased on master or next I'd
> > be happy to test!
>
> Unfortunately Simon's work here is AI-generated and so not particularly
> welcome, among other problems right now.

No, not AI-generated, but certainly AI-assisted (perhaps that is what
you meant?)

Regards,
SImon

Re: [PATCH 7/7] boot: add a minimal bootmeth for the Boot Loader Specification

[Tom Rini]

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

On Thu, Jun 25, 2026 at 06:29:18PM +0100, Simon Glass wrote:
> Hi Tom,
> 
> On Thu, 25 Jun 2026 at 18:24, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Jun 25, 2026 at 08:57:13PM +0400, Alexey Charkov wrote:
> > > On Thu, Jun 25, 2026 at 8:24 PM Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Alexey,
> > > >
> > > > On Thu, 25 Jun 2026 at 16:52, Alexey Charkov <alchark@flipper.net> wrote:
> > > > >
> > > > > Hi Simon,
> > > > >
> > > > > On Thu, Jun 25, 2026 at 7:28 PM Simon Glass <sjg@chromium.org> wrote:
> > > > > >
> > > > > > Hi Alexey,
> > > > > >
> > > > > > On 2026-06-04T15:31:05, Alexey Charkov <alchark@flipper.net> wrote:
> > > > > > > boot: add a minimal bootmeth for the Boot Loader Specification
> > > > > > >
> > > > > > > Add a bootmeth that finds and boots Boot Loader Specification (BLS)
> > > > > > > type #1 entry files [1]. On each block-device partition it scans, the
> > > > > > > bootmeth looks for files matching '<prefix>loader/entries/*.conf'
> > > > > > > (where <prefix> comes from bootstd_get_prefixes(), typically '/' and
> > > > > > > '/boot/'), picks the highest-sorting filename, parses it, and exposes
> > > > > > > it as a bootflow.
> > > > > > >
> > > > > > > Implementation reuses the existing pxelinux infrastructure.
> > > > > > >
> > > > > > > For now the entry chosen on a partition is purely the lexicographic
> > > > > > > maximum of *.conf filenames; sort-key / version field handling
> > > > > > > (spec-mandated tiebreakers) and boot-counting (the '+TRIES_LEFT'
> > > > > > > filename suffix) are left as TODOs. Likewise, only the top-sorted entry
> > > > > > > is surfaced because the bootstd framework currently allows one bootflow
> > > > > > > per (bootmeth, partition); exposing every discovered entry will require
> > > > > > > a framework extension.
> > > > > > >
> > > > > > > Type #2 BLS (drop-in directory of EFI binaries) is out of scope here;
> > > > > > > [...]
> > > > > > >
> > > > > > > boot/Kconfig        |  17 +++
> > > > > > >  boot/Makefile       |   1 +
> > > > > > >  boot/bootmeth_bls.c | 333 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > > > >  3 files changed, 351 insertions(+)
> > > > > >
> > > > > > > diff --git a/boot/Kconfig b/boot/Kconfig
> > > > > > > @@ -649,6 +649,23 @@ config BOOTMETH_EXTLINUX_PXE
> > > > > > > +config BOOTMETH_BLS
> > > > > > > +     bool "Bootdev support for Boot Loader Specification entries"
> > > > > > > +     select PXE_UTILS
> > > > > > > +     default y
> > > > > >
> > > > > > Wherever the 'default y' discussion lands, this will be enabled on
> > > > > > sandbox, so the 'bootmeth list' tests in test/boot/bootmeth.c will
> > > > > > fail since they check the exact list and count. Please can you run the
> > > > > > sandbox tests and update them as needed?
> > > > > >
> > > > > > Since you are adding a new bootmeth you also need a sandbox test that
> > > > > > exercises it (see the extlinux tests in test/boot/bootflow.c, with a
> > > > > > fixture disk image) and a documentation page, e.g.
> > > > > > doc/develop/bootstd/bls.rst with an entry in the index there.
> > > > >
> > > > > Will do, thanks!
> > > > >
> > > > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > > > @@ -0,0 +1,333 @@
> > > > > > > +static int bls_getfile(struct pxe_context *ctx, const char *file_path,
> > > > > > > +                    char *file_addr, enum bootflow_img_t type, ulong *sizep)
> > > > > >
> > > > > > This is a verbatim copy of extlinux_getfile() - please can you export
> > > > > > that, or move it into a shared helper?
> > > > >
> > > > > Ack
> > > > >
> > > > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > > > @@ -0,0 +1,333 @@
> > > > > > > +     ret = bootmeth_alloc_file(bflow, SZ_64K, 1, BFI_EXTLINUX_CFG);
> > > > > >
> > > > > > The extlinux bootmeth passes ARCH_DMA_MINALIGN here, since the buffer
> > > > > > is filled by a block-device read which may use DMA on some platforms.
> > > > > > An alignment of 1 risks cache-line corruption there, so please use
> > > > > > ARCH_DMA_MINALIGN.
> > > > >
> > > > > Ack
> > > > >
> > > > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > > > @@ -0,0 +1,333 @@
> > > > > > > +     bflow->bootmeth_priv = label;
> > > > > > > +     free(fpath);
> > > > > >
> > > > > > This leaks memory: bootflow_free() releases bootmeth_priv with a plain
> > > > > > free(), so the label's string members (name, menu, kernel, append,
> > > > > > initrd, etc.) are never freed - for every bootflow discarded after a
> > > > > > scan, not just on error paths.
> > > > > >
> > > > > > Since get_string() copies tokens out of the buffer rather than
> > > > > > modifying it in place, you could follow the extlinux approach: keep
> > > > > > only bflow->buf across the scan (already populated by
> > > > > > bootmeth_alloc_file() and freed by the framework), use a temporary
> > > > > > label in bls_read_bootflow() just to extract the title, destroy it
> > > > > > with label_destroy(), and re-parse the buffer in bls_boot(). Then
> > > > > > bootmeth_priv is not needed at all. What do you think?
> > > > >
> > > > > Will address the leak, thanks for pointing it out!
> > > > >
> > > > > The bls_priv structure is needed for my follow-up extension which
> > > > > allows multiple boot entries to be returned by each (bootdev,
> > > > > bootmeth, partition) tuple, and I wanted to minimize churn between
> > > > > those two. I haven't sent that follow-up for review yet, as I wanted
> > > > > to confirm this simpler version is acceptable first.
> > > >
> > > > We should implement this using an generic index rather than something
> > > > bls-specific...please see some commits at:
> > > >
> > > > https://concept.deinde.dev/u-boot/u-boot/-/merge_requests/782
> > >
> > > Oh, there's already BLS in your tree :-D
> > >
> > > Would you like to merge that instead? Your version looks much more
> > > full-fledged. If you happen to have one rebased on master or next I'd
> > > be happy to test!
> >
> > Unfortunately Simon's work here is AI-generated and so not particularly
> > welcome, among other problems right now.
> 
> No, not AI-generated, but certainly AI-assisted (perhaps that is what
> you meant?)

I didn't dig back super far to see what all AI-generated (such as your
dlmalloc upgrade) vs "AI-assisted" (commit from just a few months ago)
and try and guess based on the quality if it's "generated" or
"assisted". But it doesn't really matter in the AI context. And it
certainly doesn't matter until you've donated u-boot.org to the project.

-- 
Tom

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

Re: [PATCH 7/7] boot: add a minimal bootmeth for the Boot Loader Specification

[Simon Glass]

Hi Tom,

On Thu, 25 Jun 2026 at 18:33, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Jun 25, 2026 at 06:29:18PM +0100, Simon Glass wrote:
> > Hi Tom,
> >
> > On Thu, 25 Jun 2026 at 18:24, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Thu, Jun 25, 2026 at 08:57:13PM +0400, Alexey Charkov wrote:
> > > > On Thu, Jun 25, 2026 at 8:24 PM Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > > > > Hi Alexey,
> > > > >
> > > > > On Thu, 25 Jun 2026 at 16:52, Alexey Charkov <alchark@flipper.net> wrote:
> > > > > >
> > > > > > Hi Simon,
> > > > > >
> > > > > > On Thu, Jun 25, 2026 at 7:28 PM Simon Glass <sjg@chromium.org> wrote:
> > > > > > >
> > > > > > > Hi Alexey,
> > > > > > >
> > > > > > > On 2026-06-04T15:31:05, Alexey Charkov <alchark@flipper.net> wrote:
> > > > > > > > boot: add a minimal bootmeth for the Boot Loader Specification
> > > > > > > >
> > > > > > > > Add a bootmeth that finds and boots Boot Loader Specification (BLS)
> > > > > > > > type #1 entry files [1]. On each block-device partition it scans, the
> > > > > > > > bootmeth looks for files matching '<prefix>loader/entries/*.conf'
> > > > > > > > (where <prefix> comes from bootstd_get_prefixes(), typically '/' and
> > > > > > > > '/boot/'), picks the highest-sorting filename, parses it, and exposes
> > > > > > > > it as a bootflow.
> > > > > > > >
> > > > > > > > Implementation reuses the existing pxelinux infrastructure.
> > > > > > > >
> > > > > > > > For now the entry chosen on a partition is purely the lexicographic
> > > > > > > > maximum of *.conf filenames; sort-key / version field handling
> > > > > > > > (spec-mandated tiebreakers) and boot-counting (the '+TRIES_LEFT'
> > > > > > > > filename suffix) are left as TODOs. Likewise, only the top-sorted entry
> > > > > > > > is surfaced because the bootstd framework currently allows one bootflow
> > > > > > > > per (bootmeth, partition); exposing every discovered entry will require
> > > > > > > > a framework extension.
> > > > > > > >
> > > > > > > > Type #2 BLS (drop-in directory of EFI binaries) is out of scope here;
> > > > > > > > [...]
> > > > > > > >
> > > > > > > > boot/Kconfig        |  17 +++
> > > > > > > >  boot/Makefile       |   1 +
> > > > > > > >  boot/bootmeth_bls.c | 333 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > > > > >  3 files changed, 351 insertions(+)
> > > > > > >
> > > > > > > > diff --git a/boot/Kconfig b/boot/Kconfig
> > > > > > > > @@ -649,6 +649,23 @@ config BOOTMETH_EXTLINUX_PXE
> > > > > > > > +config BOOTMETH_BLS
> > > > > > > > +     bool "Bootdev support for Boot Loader Specification entries"
> > > > > > > > +     select PXE_UTILS
> > > > > > > > +     default y
> > > > > > >
> > > > > > > Wherever the 'default y' discussion lands, this will be enabled on
> > > > > > > sandbox, so the 'bootmeth list' tests in test/boot/bootmeth.c will
> > > > > > > fail since they check the exact list and count. Please can you run the
> > > > > > > sandbox tests and update them as needed?
> > > > > > >
> > > > > > > Since you are adding a new bootmeth you also need a sandbox test that
> > > > > > > exercises it (see the extlinux tests in test/boot/bootflow.c, with a
> > > > > > > fixture disk image) and a documentation page, e.g.
> > > > > > > doc/develop/bootstd/bls.rst with an entry in the index there.
> > > > > >
> > > > > > Will do, thanks!
> > > > > >
> > > > > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > > > > @@ -0,0 +1,333 @@
> > > > > > > > +static int bls_getfile(struct pxe_context *ctx, const char *file_path,
> > > > > > > > +                    char *file_addr, enum bootflow_img_t type, ulong *sizep)
> > > > > > >
> > > > > > > This is a verbatim copy of extlinux_getfile() - please can you export
> > > > > > > that, or move it into a shared helper?
> > > > > >
> > > > > > Ack
> > > > > >
> > > > > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > > > > @@ -0,0 +1,333 @@
> > > > > > > > +     ret = bootmeth_alloc_file(bflow, SZ_64K, 1, BFI_EXTLINUX_CFG);
> > > > > > >
> > > > > > > The extlinux bootmeth passes ARCH_DMA_MINALIGN here, since the buffer
> > > > > > > is filled by a block-device read which may use DMA on some platforms.
> > > > > > > An alignment of 1 risks cache-line corruption there, so please use
> > > > > > > ARCH_DMA_MINALIGN.
> > > > > >
> > > > > > Ack
> > > > > >
> > > > > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > > > > @@ -0,0 +1,333 @@
> > > > > > > > +     bflow->bootmeth_priv = label;
> > > > > > > > +     free(fpath);
> > > > > > >
> > > > > > > This leaks memory: bootflow_free() releases bootmeth_priv with a plain
> > > > > > > free(), so the label's string members (name, menu, kernel, append,
> > > > > > > initrd, etc.) are never freed - for every bootflow discarded after a
> > > > > > > scan, not just on error paths.
> > > > > > >
> > > > > > > Since get_string() copies tokens out of the buffer rather than
> > > > > > > modifying it in place, you could follow the extlinux approach: keep
> > > > > > > only bflow->buf across the scan (already populated by
> > > > > > > bootmeth_alloc_file() and freed by the framework), use a temporary
> > > > > > > label in bls_read_bootflow() just to extract the title, destroy it
> > > > > > > with label_destroy(), and re-parse the buffer in bls_boot(). Then
> > > > > > > bootmeth_priv is not needed at all. What do you think?
> > > > > >
> > > > > > Will address the leak, thanks for pointing it out!
> > > > > >
> > > > > > The bls_priv structure is needed for my follow-up extension which
> > > > > > allows multiple boot entries to be returned by each (bootdev,
> > > > > > bootmeth, partition) tuple, and I wanted to minimize churn between
> > > > > > those two. I haven't sent that follow-up for review yet, as I wanted
> > > > > > to confirm this simpler version is acceptable first.
> > > > >
> > > > > We should implement this using an generic index rather than something
> > > > > bls-specific...please see some commits at:
> > > > >
> > > > > https://concept.deinde.dev/u-boot/u-boot/-/merge_requests/782
> > > >
> > > > Oh, there's already BLS in your tree :-D
> > > >
> > > > Would you like to merge that instead? Your version looks much more
> > > > full-fledged. If you happen to have one rebased on master or next I'd
> > > > be happy to test!
> > >
> > > Unfortunately Simon's work here is AI-generated and so not particularly
> > > welcome, among other problems right now.
> >
> > No, not AI-generated, but certainly AI-assisted (perhaps that is what
> > you meant?)
>
> I didn't dig back super far to see what all AI-generated (such as your
> dlmalloc upgrade) vs "AI-assisted" (commit from just a few months ago)
> and try and guess based on the quality if it's "generated" or
> "assisted". But it doesn't really matter in the AI context. And it
> certainly doesn't matter until you've donated u-boot.org to the project.

Yes the dlmalloc upgrade was a lot of work and brings a lot of
benefits to U-Boot. There is also a backtrace feature which is really
handlyfor tracking down memory leaks and hangs. I found a ton of
leaks, some of them very large (e.g. SCMI). There was certainly a lot
of manual work involved, along with AI assistance. I believe AI is
particularly effective when used by someone who knows the project and
codebase well.

Regards,
Simon

Re: [PATCH 7/7] boot: add a minimal bootmeth for the Boot Loader Specification

[Tom Rini]

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

On Fri, Jun 26, 2026 at 11:45:22AM +0100, Simon Glass wrote:
> Hi Tom,
> 
> On Thu, 25 Jun 2026 at 18:33, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Jun 25, 2026 at 06:29:18PM +0100, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Thu, 25 Jun 2026 at 18:24, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Thu, Jun 25, 2026 at 08:57:13PM +0400, Alexey Charkov wrote:
> > > > > On Thu, Jun 25, 2026 at 8:24 PM Simon Glass <sjg@chromium.org> wrote:
> > > > > >
> > > > > > Hi Alexey,
> > > > > >
> > > > > > On Thu, 25 Jun 2026 at 16:52, Alexey Charkov <alchark@flipper.net> wrote:
> > > > > > >
> > > > > > > Hi Simon,
> > > > > > >
> > > > > > > On Thu, Jun 25, 2026 at 7:28 PM Simon Glass <sjg@chromium.org> wrote:
> > > > > > > >
> > > > > > > > Hi Alexey,
> > > > > > > >
> > > > > > > > On 2026-06-04T15:31:05, Alexey Charkov <alchark@flipper.net> wrote:
> > > > > > > > > boot: add a minimal bootmeth for the Boot Loader Specification
> > > > > > > > >
> > > > > > > > > Add a bootmeth that finds and boots Boot Loader Specification (BLS)
> > > > > > > > > type #1 entry files [1]. On each block-device partition it scans, the
> > > > > > > > > bootmeth looks for files matching '<prefix>loader/entries/*.conf'
> > > > > > > > > (where <prefix> comes from bootstd_get_prefixes(), typically '/' and
> > > > > > > > > '/boot/'), picks the highest-sorting filename, parses it, and exposes
> > > > > > > > > it as a bootflow.
> > > > > > > > >
> > > > > > > > > Implementation reuses the existing pxelinux infrastructure.
> > > > > > > > >
> > > > > > > > > For now the entry chosen on a partition is purely the lexicographic
> > > > > > > > > maximum of *.conf filenames; sort-key / version field handling
> > > > > > > > > (spec-mandated tiebreakers) and boot-counting (the '+TRIES_LEFT'
> > > > > > > > > filename suffix) are left as TODOs. Likewise, only the top-sorted entry
> > > > > > > > > is surfaced because the bootstd framework currently allows one bootflow
> > > > > > > > > per (bootmeth, partition); exposing every discovered entry will require
> > > > > > > > > a framework extension.
> > > > > > > > >
> > > > > > > > > Type #2 BLS (drop-in directory of EFI binaries) is out of scope here;
> > > > > > > > > [...]
> > > > > > > > >
> > > > > > > > > boot/Kconfig        |  17 +++
> > > > > > > > >  boot/Makefile       |   1 +
> > > > > > > > >  boot/bootmeth_bls.c | 333 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > > > > > >  3 files changed, 351 insertions(+)
> > > > > > > >
> > > > > > > > > diff --git a/boot/Kconfig b/boot/Kconfig
> > > > > > > > > @@ -649,6 +649,23 @@ config BOOTMETH_EXTLINUX_PXE
> > > > > > > > > +config BOOTMETH_BLS
> > > > > > > > > +     bool "Bootdev support for Boot Loader Specification entries"
> > > > > > > > > +     select PXE_UTILS
> > > > > > > > > +     default y
> > > > > > > >
> > > > > > > > Wherever the 'default y' discussion lands, this will be enabled on
> > > > > > > > sandbox, so the 'bootmeth list' tests in test/boot/bootmeth.c will
> > > > > > > > fail since they check the exact list and count. Please can you run the
> > > > > > > > sandbox tests and update them as needed?
> > > > > > > >
> > > > > > > > Since you are adding a new bootmeth you also need a sandbox test that
> > > > > > > > exercises it (see the extlinux tests in test/boot/bootflow.c, with a
> > > > > > > > fixture disk image) and a documentation page, e.g.
> > > > > > > > doc/develop/bootstd/bls.rst with an entry in the index there.
> > > > > > >
> > > > > > > Will do, thanks!
> > > > > > >
> > > > > > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > > > > > @@ -0,0 +1,333 @@
> > > > > > > > > +static int bls_getfile(struct pxe_context *ctx, const char *file_path,
> > > > > > > > > +                    char *file_addr, enum bootflow_img_t type, ulong *sizep)
> > > > > > > >
> > > > > > > > This is a verbatim copy of extlinux_getfile() - please can you export
> > > > > > > > that, or move it into a shared helper?
> > > > > > >
> > > > > > > Ack
> > > > > > >
> > > > > > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > > > > > @@ -0,0 +1,333 @@
> > > > > > > > > +     ret = bootmeth_alloc_file(bflow, SZ_64K, 1, BFI_EXTLINUX_CFG);
> > > > > > > >
> > > > > > > > The extlinux bootmeth passes ARCH_DMA_MINALIGN here, since the buffer
> > > > > > > > is filled by a block-device read which may use DMA on some platforms.
> > > > > > > > An alignment of 1 risks cache-line corruption there, so please use
> > > > > > > > ARCH_DMA_MINALIGN.
> > > > > > >
> > > > > > > Ack
> > > > > > >
> > > > > > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > > > > > @@ -0,0 +1,333 @@
> > > > > > > > > +     bflow->bootmeth_priv = label;
> > > > > > > > > +     free(fpath);
> > > > > > > >
> > > > > > > > This leaks memory: bootflow_free() releases bootmeth_priv with a plain
> > > > > > > > free(), so the label's string members (name, menu, kernel, append,
> > > > > > > > initrd, etc.) are never freed - for every bootflow discarded after a
> > > > > > > > scan, not just on error paths.
> > > > > > > >
> > > > > > > > Since get_string() copies tokens out of the buffer rather than
> > > > > > > > modifying it in place, you could follow the extlinux approach: keep
> > > > > > > > only bflow->buf across the scan (already populated by
> > > > > > > > bootmeth_alloc_file() and freed by the framework), use a temporary
> > > > > > > > label in bls_read_bootflow() just to extract the title, destroy it
> > > > > > > > with label_destroy(), and re-parse the buffer in bls_boot(). Then
> > > > > > > > bootmeth_priv is not needed at all. What do you think?
> > > > > > >
> > > > > > > Will address the leak, thanks for pointing it out!
> > > > > > >
> > > > > > > The bls_priv structure is needed for my follow-up extension which
> > > > > > > allows multiple boot entries to be returned by each (bootdev,
> > > > > > > bootmeth, partition) tuple, and I wanted to minimize churn between
> > > > > > > those two. I haven't sent that follow-up for review yet, as I wanted
> > > > > > > to confirm this simpler version is acceptable first.
> > > > > >
> > > > > > We should implement this using an generic index rather than something
> > > > > > bls-specific...please see some commits at:
> > > > > >
> > > > > > https://concept.deinde.dev/u-boot/u-boot/-/merge_requests/782
> > > > >
> > > > > Oh, there's already BLS in your tree :-D
> > > > >
> > > > > Would you like to merge that instead? Your version looks much more
> > > > > full-fledged. If you happen to have one rebased on master or next I'd
> > > > > be happy to test!
> > > >
> > > > Unfortunately Simon's work here is AI-generated and so not particularly
> > > > welcome, among other problems right now.
> > >
> > > No, not AI-generated, but certainly AI-assisted (perhaps that is what
> > > you meant?)
> >
> > I didn't dig back super far to see what all AI-generated (such as your
> > dlmalloc upgrade) vs "AI-assisted" (commit from just a few months ago)
> > and try and guess based on the quality if it's "generated" or
> > "assisted". But it doesn't really matter in the AI context. And it
> > certainly doesn't matter until you've donated u-boot.org to the project.
> 
> Yes the dlmalloc upgrade was a lot of work and brings a lot of
> benefits to U-Boot. There is also a backtrace feature which is really
> handlyfor tracking down memory leaks and hangs. I found a ton of
> leaks, some of them very large (e.g. SCMI). There was certainly a lot
> of manual work involved, along with AI assistance. I believe AI is
> particularly effective when used by someone who knows the project and
> codebase well.

OK, but did you review that dlmalloc upgrade? Did you think Claude did a
good and appropriate job there?

-- 
Tom

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

Re: [PATCH 7/7] boot: add a minimal bootmeth for the Boot Loader Specification

[Simon Glass]

Hi Tom,

On Fri, 26 Jun 2026 at 14:47, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Jun 26, 2026 at 11:45:22AM +0100, Simon Glass wrote:
> > Hi Tom,
> >
> > On Thu, 25 Jun 2026 at 18:33, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Thu, Jun 25, 2026 at 06:29:18PM +0100, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Thu, 25 Jun 2026 at 18:24, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Thu, Jun 25, 2026 at 08:57:13PM +0400, Alexey Charkov wrote:
> > > > > > On Thu, Jun 25, 2026 at 8:24 PM Simon Glass <sjg@chromium.org> wrote:
> > > > > > >
> > > > > > > Hi Alexey,
> > > > > > >
> > > > > > > On Thu, 25 Jun 2026 at 16:52, Alexey Charkov <alchark@flipper.net> wrote:
> > > > > > > >
> > > > > > > > Hi Simon,
> > > > > > > >
> > > > > > > > On Thu, Jun 25, 2026 at 7:28 PM Simon Glass <sjg@chromium.org> wrote:
> > > > > > > > >
> > > > > > > > > Hi Alexey,
> > > > > > > > >
> > > > > > > > > On 2026-06-04T15:31:05, Alexey Charkov <alchark@flipper.net> wrote:
> > > > > > > > > > boot: add a minimal bootmeth for the Boot Loader Specification
> > > > > > > > > >
> > > > > > > > > > Add a bootmeth that finds and boots Boot Loader Specification (BLS)
> > > > > > > > > > type #1 entry files [1]. On each block-device partition it scans, the
> > > > > > > > > > bootmeth looks for files matching '<prefix>loader/entries/*.conf'
> > > > > > > > > > (where <prefix> comes from bootstd_get_prefixes(), typically '/' and
> > > > > > > > > > '/boot/'), picks the highest-sorting filename, parses it, and exposes
> > > > > > > > > > it as a bootflow.
> > > > > > > > > >
> > > > > > > > > > Implementation reuses the existing pxelinux infrastructure.
> > > > > > > > > >
> > > > > > > > > > For now the entry chosen on a partition is purely the lexicographic
> > > > > > > > > > maximum of *.conf filenames; sort-key / version field handling
> > > > > > > > > > (spec-mandated tiebreakers) and boot-counting (the '+TRIES_LEFT'
> > > > > > > > > > filename suffix) are left as TODOs. Likewise, only the top-sorted entry
> > > > > > > > > > is surfaced because the bootstd framework currently allows one bootflow
> > > > > > > > > > per (bootmeth, partition); exposing every discovered entry will require
> > > > > > > > > > a framework extension.
> > > > > > > > > >
> > > > > > > > > > Type #2 BLS (drop-in directory of EFI binaries) is out of scope here;
> > > > > > > > > > [...]
> > > > > > > > > >
> > > > > > > > > > boot/Kconfig        |  17 +++
> > > > > > > > > >  boot/Makefile       |   1 +
> > > > > > > > > >  boot/bootmeth_bls.c | 333 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > > > > > > >  3 files changed, 351 insertions(+)
> > > > > > > > >
> > > > > > > > > > diff --git a/boot/Kconfig b/boot/Kconfig
> > > > > > > > > > @@ -649,6 +649,23 @@ config BOOTMETH_EXTLINUX_PXE
> > > > > > > > > > +config BOOTMETH_BLS
> > > > > > > > > > +     bool "Bootdev support for Boot Loader Specification entries"
> > > > > > > > > > +     select PXE_UTILS
> > > > > > > > > > +     default y
> > > > > > > > >
> > > > > > > > > Wherever the 'default y' discussion lands, this will be enabled on
> > > > > > > > > sandbox, so the 'bootmeth list' tests in test/boot/bootmeth.c will
> > > > > > > > > fail since they check the exact list and count. Please can you run the
> > > > > > > > > sandbox tests and update them as needed?
> > > > > > > > >
> > > > > > > > > Since you are adding a new bootmeth you also need a sandbox test that
> > > > > > > > > exercises it (see the extlinux tests in test/boot/bootflow.c, with a
> > > > > > > > > fixture disk image) and a documentation page, e.g.
> > > > > > > > > doc/develop/bootstd/bls.rst with an entry in the index there.
> > > > > > > >
> > > > > > > > Will do, thanks!
> > > > > > > >
> > > > > > > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > > > > > > @@ -0,0 +1,333 @@
> > > > > > > > > > +static int bls_getfile(struct pxe_context *ctx, const char *file_path,
> > > > > > > > > > +                    char *file_addr, enum bootflow_img_t type, ulong *sizep)
> > > > > > > > >
> > > > > > > > > This is a verbatim copy of extlinux_getfile() - please can you export
> > > > > > > > > that, or move it into a shared helper?
> > > > > > > >
> > > > > > > > Ack
> > > > > > > >
> > > > > > > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > > > > > > @@ -0,0 +1,333 @@
> > > > > > > > > > +     ret = bootmeth_alloc_file(bflow, SZ_64K, 1, BFI_EXTLINUX_CFG);
> > > > > > > > >
> > > > > > > > > The extlinux bootmeth passes ARCH_DMA_MINALIGN here, since the buffer
> > > > > > > > > is filled by a block-device read which may use DMA on some platforms.
> > > > > > > > > An alignment of 1 risks cache-line corruption there, so please use
> > > > > > > > > ARCH_DMA_MINALIGN.
> > > > > > > >
> > > > > > > > Ack
> > > > > > > >
> > > > > > > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > > > > > > @@ -0,0 +1,333 @@
> > > > > > > > > > +     bflow->bootmeth_priv = label;
> > > > > > > > > > +     free(fpath);
> > > > > > > > >
> > > > > > > > > This leaks memory: bootflow_free() releases bootmeth_priv with a plain
> > > > > > > > > free(), so the label's string members (name, menu, kernel, append,
> > > > > > > > > initrd, etc.) are never freed - for every bootflow discarded after a
> > > > > > > > > scan, not just on error paths.
> > > > > > > > >
> > > > > > > > > Since get_string() copies tokens out of the buffer rather than
> > > > > > > > > modifying it in place, you could follow the extlinux approach: keep
> > > > > > > > > only bflow->buf across the scan (already populated by
> > > > > > > > > bootmeth_alloc_file() and freed by the framework), use a temporary
> > > > > > > > > label in bls_read_bootflow() just to extract the title, destroy it
> > > > > > > > > with label_destroy(), and re-parse the buffer in bls_boot(). Then
> > > > > > > > > bootmeth_priv is not needed at all. What do you think?
> > > > > > > >
> > > > > > > > Will address the leak, thanks for pointing it out!
> > > > > > > >
> > > > > > > > The bls_priv structure is needed for my follow-up extension which
> > > > > > > > allows multiple boot entries to be returned by each (bootdev,
> > > > > > > > bootmeth, partition) tuple, and I wanted to minimize churn between
> > > > > > > > those two. I haven't sent that follow-up for review yet, as I wanted
> > > > > > > > to confirm this simpler version is acceptable first.
> > > > > > >
> > > > > > > We should implement this using an generic index rather than something
> > > > > > > bls-specific...please see some commits at:
> > > > > > >
> > > > > > > https://concept.deinde.dev/u-boot/u-boot/-/merge_requests/782
> > > > > >
> > > > > > Oh, there's already BLS in your tree :-D
> > > > > >
> > > > > > Would you like to merge that instead? Your version looks much more
> > > > > > full-fledged. If you happen to have one rebased on master or next I'd
> > > > > > be happy to test!
> > > > >
> > > > > Unfortunately Simon's work here is AI-generated and so not particularly
> > > > > welcome, among other problems right now.
> > > >
> > > > No, not AI-generated, but certainly AI-assisted (perhaps that is what
> > > > you meant?)
> > >
> > > I didn't dig back super far to see what all AI-generated (such as your
> > > dlmalloc upgrade) vs "AI-assisted" (commit from just a few months ago)
> > > and try and guess based on the quality if it's "generated" or
> > > "assisted". But it doesn't really matter in the AI context. And it
> > > certainly doesn't matter until you've donated u-boot.org to the project.
> >
> > Yes the dlmalloc upgrade was a lot of work and brings a lot of
> > benefits to U-Boot. There is also a backtrace feature which is really
> > handlyfor tracking down memory leaks and hangs. I found a ton of
> > leaks, some of them very large (e.g. SCMI). There was certainly a lot
> > of manual work involved, along with AI assistance. I believe AI is
> > particularly effective when used by someone who knows the project and
> > codebase well.
>
> OK, but did you review that dlmalloc upgrade? Did you think Claude did a
> good and appropriate job there?

It's a while ago, but I remember being about 3 bugs deep at the time
(crashes in CI which turned out to be memory leaks) so I am sure it
could be better. If you are interested in it for mainline I could take
another look.

Regards,
Simon

Re: [PATCH 7/7] boot: add a minimal bootmeth for the Boot Loader Specification

[Tom Rini]

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

On Mon, Jun 29, 2026 at 06:45:07AM +0100, Simon Glass wrote:
> Hi Tom,
> 
> On Fri, 26 Jun 2026 at 14:47, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Fri, Jun 26, 2026 at 11:45:22AM +0100, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Thu, 25 Jun 2026 at 18:33, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Thu, Jun 25, 2026 at 06:29:18PM +0100, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Thu, 25 Jun 2026 at 18:24, Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Thu, Jun 25, 2026 at 08:57:13PM +0400, Alexey Charkov wrote:
> > > > > > > On Thu, Jun 25, 2026 at 8:24 PM Simon Glass <sjg@chromium.org> wrote:
> > > > > > > >
> > > > > > > > Hi Alexey,
> > > > > > > >
> > > > > > > > On Thu, 25 Jun 2026 at 16:52, Alexey Charkov <alchark@flipper.net> wrote:
> > > > > > > > >
> > > > > > > > > Hi Simon,
> > > > > > > > >
> > > > > > > > > On Thu, Jun 25, 2026 at 7:28 PM Simon Glass <sjg@chromium.org> wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Alexey,
> > > > > > > > > >
> > > > > > > > > > On 2026-06-04T15:31:05, Alexey Charkov <alchark@flipper.net> wrote:
> > > > > > > > > > > boot: add a minimal bootmeth for the Boot Loader Specification
> > > > > > > > > > >
> > > > > > > > > > > Add a bootmeth that finds and boots Boot Loader Specification (BLS)
> > > > > > > > > > > type #1 entry files [1]. On each block-device partition it scans, the
> > > > > > > > > > > bootmeth looks for files matching '<prefix>loader/entries/*.conf'
> > > > > > > > > > > (where <prefix> comes from bootstd_get_prefixes(), typically '/' and
> > > > > > > > > > > '/boot/'), picks the highest-sorting filename, parses it, and exposes
> > > > > > > > > > > it as a bootflow.
> > > > > > > > > > >
> > > > > > > > > > > Implementation reuses the existing pxelinux infrastructure.
> > > > > > > > > > >
> > > > > > > > > > > For now the entry chosen on a partition is purely the lexicographic
> > > > > > > > > > > maximum of *.conf filenames; sort-key / version field handling
> > > > > > > > > > > (spec-mandated tiebreakers) and boot-counting (the '+TRIES_LEFT'
> > > > > > > > > > > filename suffix) are left as TODOs. Likewise, only the top-sorted entry
> > > > > > > > > > > is surfaced because the bootstd framework currently allows one bootflow
> > > > > > > > > > > per (bootmeth, partition); exposing every discovered entry will require
> > > > > > > > > > > a framework extension.
> > > > > > > > > > >
> > > > > > > > > > > Type #2 BLS (drop-in directory of EFI binaries) is out of scope here;
> > > > > > > > > > > [...]
> > > > > > > > > > >
> > > > > > > > > > > boot/Kconfig        |  17 +++
> > > > > > > > > > >  boot/Makefile       |   1 +
> > > > > > > > > > >  boot/bootmeth_bls.c | 333 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > > > > > > > >  3 files changed, 351 insertions(+)
> > > > > > > > > >
> > > > > > > > > > > diff --git a/boot/Kconfig b/boot/Kconfig
> > > > > > > > > > > @@ -649,6 +649,23 @@ config BOOTMETH_EXTLINUX_PXE
> > > > > > > > > > > +config BOOTMETH_BLS
> > > > > > > > > > > +     bool "Bootdev support for Boot Loader Specification entries"
> > > > > > > > > > > +     select PXE_UTILS
> > > > > > > > > > > +     default y
> > > > > > > > > >
> > > > > > > > > > Wherever the 'default y' discussion lands, this will be enabled on
> > > > > > > > > > sandbox, so the 'bootmeth list' tests in test/boot/bootmeth.c will
> > > > > > > > > > fail since they check the exact list and count. Please can you run the
> > > > > > > > > > sandbox tests and update them as needed?
> > > > > > > > > >
> > > > > > > > > > Since you are adding a new bootmeth you also need a sandbox test that
> > > > > > > > > > exercises it (see the extlinux tests in test/boot/bootflow.c, with a
> > > > > > > > > > fixture disk image) and a documentation page, e.g.
> > > > > > > > > > doc/develop/bootstd/bls.rst with an entry in the index there.
> > > > > > > > >
> > > > > > > > > Will do, thanks!
> > > > > > > > >
> > > > > > > > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > > > > > > > @@ -0,0 +1,333 @@
> > > > > > > > > > > +static int bls_getfile(struct pxe_context *ctx, const char *file_path,
> > > > > > > > > > > +                    char *file_addr, enum bootflow_img_t type, ulong *sizep)
> > > > > > > > > >
> > > > > > > > > > This is a verbatim copy of extlinux_getfile() - please can you export
> > > > > > > > > > that, or move it into a shared helper?
> > > > > > > > >
> > > > > > > > > Ack
> > > > > > > > >
> > > > > > > > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > > > > > > > @@ -0,0 +1,333 @@
> > > > > > > > > > > +     ret = bootmeth_alloc_file(bflow, SZ_64K, 1, BFI_EXTLINUX_CFG);
> > > > > > > > > >
> > > > > > > > > > The extlinux bootmeth passes ARCH_DMA_MINALIGN here, since the buffer
> > > > > > > > > > is filled by a block-device read which may use DMA on some platforms.
> > > > > > > > > > An alignment of 1 risks cache-line corruption there, so please use
> > > > > > > > > > ARCH_DMA_MINALIGN.
> > > > > > > > >
> > > > > > > > > Ack
> > > > > > > > >
> > > > > > > > > > > diff --git a/boot/bootmeth_bls.c b/boot/bootmeth_bls.c
> > > > > > > > > > > @@ -0,0 +1,333 @@
> > > > > > > > > > > +     bflow->bootmeth_priv = label;
> > > > > > > > > > > +     free(fpath);
> > > > > > > > > >
> > > > > > > > > > This leaks memory: bootflow_free() releases bootmeth_priv with a plain
> > > > > > > > > > free(), so the label's string members (name, menu, kernel, append,
> > > > > > > > > > initrd, etc.) are never freed - for every bootflow discarded after a
> > > > > > > > > > scan, not just on error paths.
> > > > > > > > > >
> > > > > > > > > > Since get_string() copies tokens out of the buffer rather than
> > > > > > > > > > modifying it in place, you could follow the extlinux approach: keep
> > > > > > > > > > only bflow->buf across the scan (already populated by
> > > > > > > > > > bootmeth_alloc_file() and freed by the framework), use a temporary
> > > > > > > > > > label in bls_read_bootflow() just to extract the title, destroy it
> > > > > > > > > > with label_destroy(), and re-parse the buffer in bls_boot(). Then
> > > > > > > > > > bootmeth_priv is not needed at all. What do you think?
> > > > > > > > >
> > > > > > > > > Will address the leak, thanks for pointing it out!
> > > > > > > > >
> > > > > > > > > The bls_priv structure is needed for my follow-up extension which
> > > > > > > > > allows multiple boot entries to be returned by each (bootdev,
> > > > > > > > > bootmeth, partition) tuple, and I wanted to minimize churn between
> > > > > > > > > those two. I haven't sent that follow-up for review yet, as I wanted
> > > > > > > > > to confirm this simpler version is acceptable first.
> > > > > > > >
> > > > > > > > We should implement this using an generic index rather than something
> > > > > > > > bls-specific...please see some commits at:
> > > > > > > >
> > > > > > > > https://concept.deinde.dev/u-boot/u-boot/-/merge_requests/782
> > > > > > >
> > > > > > > Oh, there's already BLS in your tree :-D
> > > > > > >
> > > > > > > Would you like to merge that instead? Your version looks much more
> > > > > > > full-fledged. If you happen to have one rebased on master or next I'd
> > > > > > > be happy to test!
> > > > > >
> > > > > > Unfortunately Simon's work here is AI-generated and so not particularly
> > > > > > welcome, among other problems right now.
> > > > >
> > > > > No, not AI-generated, but certainly AI-assisted (perhaps that is what
> > > > > you meant?)
> > > >
> > > > I didn't dig back super far to see what all AI-generated (such as your
> > > > dlmalloc upgrade) vs "AI-assisted" (commit from just a few months ago)
> > > > and try and guess based on the quality if it's "generated" or
> > > > "assisted". But it doesn't really matter in the AI context. And it
> > > > certainly doesn't matter until you've donated u-boot.org to the project.
> > >
> > > Yes the dlmalloc upgrade was a lot of work and brings a lot of
> > > benefits to U-Boot. There is also a backtrace feature which is really
> > > handlyfor tracking down memory leaks and hangs. I found a ton of
> > > leaks, some of them very large (e.g. SCMI). There was certainly a lot
> > > of manual work involved, along with AI assistance. I believe AI is
> > > particularly effective when used by someone who knows the project and
> > > codebase well.
> >
> > OK, but did you review that dlmalloc upgrade? Did you think Claude did a
> > good and appropriate job there?
> 
> It's a while ago, but I remember being about 3 bugs deep at the time
> (crashes in CI which turned out to be memory leaks) so I am sure it
> could be better. If you are interested in it for mainline I could take
> another look.

Really, I think that just adds to the list of reasons why these "tools"
are so terrible. The series itself is disrespectful to the community, as
it kept the commit messages *and* review tags of the actual humans that
originally did work while the changes being done almost never[1] had any
clear relationship to what was originally done. And as a "cherry on
top", tracking down what one of the original commits even is was not
easy as Claude rewrote the last third or so of the git hash.

So no, it would not be "take another look", it would be "throw it out
and do it correctly".

[1]: Of what I was cc'd one, which was a reasonable number, in only one
case was the original change, and the new change doing the same thing as
before in a clear manner that the commit message was still certainly
correct, and the remaining change was the same as before, so if a human
did it, it would have been considered reasonable to do with an
explanation below the "---".

-- 
Tom

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