Hi Alexey,

On 2026-06-04T15:31:05, Alexey Charkov <[email protected]> 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

Reply via email to