From: Leif Lindholm <leif.lindholm@oss.qualcomm.com>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: linux-efi@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Huacai Chen <chenhuacai@kernel.org>,
WANG Xuerui <kernel@xen0n.name>,
loongarch@lists.linux.dev
Subject: Re: [PATCH 2/3] efi: Pass EFI boot memmap struct address to core kernel
Date: Fri, 14 Aug 2026 16:16:28 +0100 [thread overview]
Message-ID: <an8xTBiCy_PEuwZi@leviathan> (raw)
In-Reply-To: <20260813074506.643472-7-ardb@kernel.org>
On Thu, Aug 13, 2026 at 09:45:09 +0200, Ard Biesheuvel wrote:
> The EFI stub already passes a struct efi_boot_memmap populated with the
> information of the EFI memory map as a configuration table, and so
> passing the physical address, size, descriptor size and descriptor
> version via 4 different DT properties is kind of redundant.
>
> Instead, pass the physical address of this struct in memory so that the
> kernel can just retrieve the values directly.
>
> Unfortunately, the scheme with four separate properties is boot ABI for
> Xen, and so this needs to remain supported.
To clarify, you mean boot ABI when running under Xen?
> But for the EFI stub itself,
> this is just an internal ABI that can be modified.
Hmm...
So this leaves only three properties that cannot be derived from the
System Table:
- The System Table address itself
- kaslr-seed
- bootargs
The latter two could also be given the config table treatment.
Is it time to update the boot ABI to say x0 will hold the physical
address of "device tree blob (dtb) or EFI System Table in system RAM"?
They can be distinguished by 0xd00dfeed / "IBI SYST".
The use of a generated DT when none was provided by firmware has led
to both confusion and shenanigans, and might be nice to get rid of?
Either way, it would be nice to get rid of all four of the
linux,uefi-mmap nodes if they're now completely redundant.
Minor bikeshedding below.
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> Documentation/arch/arm/uefi.rst | 10 +----
> drivers/firmware/efi/fdtparams.c | 39 ++++++++++++++++---
> drivers/firmware/efi/libstub/fdt.c | 40 ++------------------
> 3 files changed, 37 insertions(+), 52 deletions(-)
>
> diff --git a/Documentation/arch/arm/uefi.rst b/Documentation/arch/arm/uefi.rst
> index 2b7ad9bd7cd2..c19cf3720f27 100644
> --- a/Documentation/arch/arm/uefi.rst
> +++ b/Documentation/arch/arm/uefi.rst
> @@ -54,17 +54,9 @@ Name Type Description
> ========================== ====== ===========================================
> linux,uefi-system-table 64-bit Physical address of the UEFI System Table.
>
> -linux,uefi-mmap-start 64-bit Physical address of the UEFI memory map,
> +linux,uefi-boot-memmap 64-bit Physical address of a struct efi_boot_memmap
> populated by the UEFI GetMemoryMap() call.
>
> -linux,uefi-mmap-size 32-bit Size in bytes of the UEFI memory map
> - pointed to in previous entry.
> -
> -linux,uefi-mmap-desc-size 32-bit Size in bytes of each entry in the UEFI
> - memory map.
> -
> -linux,uefi-mmap-desc-ver 32-bit Version of the mmap descriptor format.
> -
> kaslr-seed 64-bit Entropy used to randomize the kernel image
> base address location.
>
> diff --git a/drivers/firmware/efi/fdtparams.c b/drivers/firmware/efi/fdtparams.c
> index b815d2a754ee..a54a76a6aaeb 100644
> --- a/drivers/firmware/efi/fdtparams.c
> +++ b/drivers/firmware/efi/fdtparams.c
> @@ -10,22 +10,30 @@
>
> #include <linux/unaligned.h>
>
> +#include <asm/early_ioremap.h>
> +
> enum {
> SYSTAB,
> + MEMMAP,
> +#ifdef CONFIG_XEN
> MMBASE,
> MMSIZE,
> DCSIZE,
> DCVERS,
> +#endif
>
> PARAMCOUNT
> };
>
> static __initconst const char name[][22] = {
> [SYSTAB] = "System Table ",
> + [MEMMAP] = "Boot Memory Map ",
> +#ifdef CONFIG_XEN
> [MMBASE] = "MemMap Address ",
> [MMSIZE] = "MemMap Size ",
> [DCSIZE] = "MemMap Desc. Size ",
> [DCVERS] = "MemMap Desc. Version ",
> +#endif
> };
>
> static __initconst const struct {
> @@ -49,10 +57,7 @@ static __initconst const struct {
> .path = "/chosen",
> .params = { // <-----------26----------->
> [SYSTAB] = "linux,uefi-system-table",
> - [MMBASE] = "linux,uefi-mmap-start",
> - [MMSIZE] = "linux,uefi-mmap-size",
> - [DCSIZE] = "linux,uefi-mmap-desc-size",
> - [DCVERS] = "linux,uefi-mmap-desc-ver",
> + [MEMMAP] = "linux,uefi-boot-memmap",
> }
> }
> };
> @@ -84,17 +89,20 @@ static int __init efi_get_fdt_prop(const void *fdt, int node, const char *pname,
> u64 __init efi_get_fdt_params(struct efi_memory_map_data *mm)
> {
> const void *fdt = initial_boot_params;
> - unsigned long systab;
> + unsigned long systab, memmap;
> int i, j, node;
> struct {
> void *var;
> int size;
> } target[] = {
> [SYSTAB] = { &systab, sizeof(systab) },
> + [MEMMAP] = { &memmap, sizeof(memmap) },
> +#ifdef CONFIG_XEN
> [MMBASE] = { &mm->phys_map, sizeof(mm->phys_map) },
> [MMSIZE] = { &mm->size, sizeof(mm->size) },
> [DCSIZE] = { &mm->desc_size, sizeof(mm->desc_size) },
> [DCVERS] = { &mm->desc_version, sizeof(mm->desc_version) },
> +#endif
> };
>
> BUILD_BUG_ON(ARRAY_SIZE(target) != ARRAY_SIZE(name));
> @@ -115,6 +123,9 @@ u64 __init efi_get_fdt_params(struct efi_memory_map_data *mm)
> for (j = 0; j < ARRAY_SIZE(target); j++) {
> const char *pname = dt_params[i].params[j];
>
> + if (pname[0] == '\0')
> + continue;
> +
> if (!efi_get_fdt_prop(fdt, node, pname, name[j],
> target[j].var, target[j].size))
> continue;
> @@ -123,8 +134,24 @@ u64 __init efi_get_fdt_params(struct efi_memory_map_data *mm)
> pr_err("Can't find property '%s' in DT!\n", pname);
> return 0;
> }
> - if (dt_params[i].paravirt)
> + if (IS_ENABLED(CONFIG_XEN) && dt_params[i].paravirt) {
> set_bit(EFI_PARAVIRT, &efi.flags);
> + } else {
This condition branch doesn't in fact have anything to do with the fdt.
Should it still live in fdtparams.c?
/
Leif
> + struct efi_boot_memmap *bm;
> +
> + bm = early_memremap_ro(memmap, sizeof(*bm));
> + if (!bm) {
> + pr_err("Cannot remap EFI boot memory map\n");
> + return 0;
> + }
> +
> + mm->phys_map = memmap + sizeof(*bm);
> + mm->size = bm->map_size;
> + mm->desc_size = bm->desc_size;
> + mm->desc_version = bm->desc_ver;
> +
> + early_memunmap(bm, sizeof(*bm));
> + }
> return systab;
> }
> notfound:
> diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
> index 23b3543d3041..417b1344bdd8 100644
> --- a/drivers/firmware/efi/libstub/fdt.c
> +++ b/drivers/firmware/efi/libstub/fdt.c
> @@ -32,7 +32,6 @@ static efi_status_t update_fdt(void *orig_fdt, unsigned long orig_fdt_size,
> {
> int node, num_rsv;
> int status;
> - fdt32_t fdt_val32;
> fdt64_t fdt_val64;
>
> /* Do some checks on provided FDT, if it exists: */
> @@ -102,21 +101,7 @@ static efi_status_t update_fdt(void *orig_fdt, unsigned long orig_fdt_size,
>
> fdt_val64 = cpu_to_fdt64(U64_MAX); /* placeholder */
>
> - status = fdt_setprop_var(fdt, node, "linux,uefi-mmap-start", fdt_val64);
> - if (status)
> - goto fdt_set_fail;
> -
> - fdt_val32 = cpu_to_fdt32(U32_MAX); /* placeholder */
> -
> - status = fdt_setprop_var(fdt, node, "linux,uefi-mmap-size", fdt_val32);
> - if (status)
> - goto fdt_set_fail;
> -
> - status = fdt_setprop_var(fdt, node, "linux,uefi-mmap-desc-size", fdt_val32);
> - if (status)
> - goto fdt_set_fail;
> -
> - status = fdt_setprop_var(fdt, node, "linux,uefi-mmap-desc-ver", fdt_val32);
> + status = fdt_setprop_var(fdt, node, "linux,uefi-boot-memmap", fdt_val64);
> if (status)
> goto fdt_set_fail;
>
> @@ -148,33 +133,14 @@ static efi_status_t update_fdt_memmap(void *fdt, struct efi_boot_memmap *map)
> {
> int node = fdt_path_offset(fdt, "/chosen");
> fdt64_t fdt_val64;
> - fdt32_t fdt_val32;
> int err;
>
> if (node < 0)
> return EFI_LOAD_ERROR;
>
> - fdt_val64 = cpu_to_fdt64((unsigned long)map->map);
> -
> - err = fdt_setprop_inplace_var(fdt, node, "linux,uefi-mmap-start", fdt_val64);
> - if (err)
> - return EFI_LOAD_ERROR;
> -
> - fdt_val32 = cpu_to_fdt32(map->map_size);
> -
> - err = fdt_setprop_inplace_var(fdt, node, "linux,uefi-mmap-size", fdt_val32);
> - if (err)
> - return EFI_LOAD_ERROR;
> -
> - fdt_val32 = cpu_to_fdt32(map->desc_size);
> -
> - err = fdt_setprop_inplace_var(fdt, node, "linux,uefi-mmap-desc-size", fdt_val32);
> - if (err)
> - return EFI_LOAD_ERROR;
> -
> - fdt_val32 = cpu_to_fdt32(map->desc_ver);
> + fdt_val64 = cpu_to_fdt64((unsigned long)map);
>
> - err = fdt_setprop_inplace_var(fdt, node, "linux,uefi-mmap-desc-ver", fdt_val32);
> + err = fdt_setprop_inplace_var(fdt, node, "linux,uefi-boot-memmap", fdt_val64);
> if (err)
> return EFI_LOAD_ERROR;
>
> --
> 2.47.3
>
next prev parent reply other threads:[~2026-08-14 15:16 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-13 7:45 [PATCH 0/3] efi: Simply DT handoff from stub to kernel Ard Biesheuvel
2026-08-13 7:45 ` [PATCH 1/3] efi: Turn boot memmap handling into shared code Ard Biesheuvel
2026-08-13 8:24 ` Richard Lyu
2026-08-13 7:45 ` [PATCH 2/3] efi: Pass EFI boot memmap struct address to core kernel Ard Biesheuvel
2026-08-13 8:25 ` Richard Lyu
2026-08-14 15:16 ` Leif Lindholm [this message]
2026-08-13 7:45 ` [PATCH 3/3] efi: Make the 'linux,uefi-boot-memmap' DT property optional Ard Biesheuvel
2026-08-13 8:25 ` Richard Lyu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=an8xTBiCy_PEuwZi@leviathan \
--to=leif.lindholm@oss.qualcomm.com \
--cc=ardb@kernel.org \
--cc=chenhuacai@kernel.org \
--cc=kernel@xen0n.name \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-efi@vger.kernel.org \
--cc=loongarch@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox