Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Ard Biesheuvel" <ardb@kernel.org>
To: "Leif Lindholm" <leif.lindholm@oss.qualcomm.com>
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: Mon, 17 Aug 2026 19:59:02 +0300	[thread overview]
Message-ID: <c3cd6952-87fd-442b-8d54-13c814b8d5f2@app.fastmail.com> (raw)
In-Reply-To: <aoMV3TX0hgTsj3_m@leviathan>



On Mon, 17 Aug 2026, at 17:08, Leif Lindholm wrote:
> On Mon, Aug 17, 2026 at 14:00:16 +0300, Ard Biesheuvel wrote:
>> >> > 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".
>> >> 
>> >> Let's avoid 'boot ABI' here, given that we are talking about an internal
>> >> interface between the EFI stub and the kernel proper.
>> >
>> > It's an internal business in the topic under discussion, but if we
>> > were to change it, that would mean updating booting.rst, which I
>> > consider an ABI.
>> >
>> 
>> But not an external ABI. It documents specifically how the EFI stub
>> interfaces with the kernel proper. This might change at any point,
>> without any obligation whatsoever to remain compatible with the
>> previous method.
>
> So you're saying if the kernel proper decides to use x1 for something
> else, we'll move to x2?
>
> I agree that in the context of how the stub calls the kernel proper,
> this is not ABI, but x1 is one of the registers currently marked as
> "reserved for future use" in booting.rst. So it feels weird for me to
> allocate one of those registers for this internal use then not mention
> it.
>

Whether we mention it or not is irrelevant: the only thing that matters
is that this allocation is not something that OS loaders other than the
EFI stub are able to rely upon, given that EFI boot on arm64 requires
booting via the stub (except for Xen dom0).

> In that case, couldn't we instead pick a register outside of the
> reserved ones?
>

It has to be a reserved one, given that the state of other registers is
unspecified at boot. Otherwise, we'd have to reason about whether a non-zero
value is accidental, or represents a physical address that we can map and
dereference, which is needed even to check the system table's magin

>> ...
>> >> > Minor bikeshedding below.
>> >> >
>> >> >> @@ -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?
>> >> >
>> >> 
>> >> I don't follow. The EFI_PARAVIRT flag is set based on whether we are 
>> >> using the generic or the Xen-specific set of DT properties. What would
>> >> be a better place to decide this?
>> >
>> > Apologies, I may have commented confusingly - my comment was about the
>> > else branch:
>> >
>> > +
>> > +                       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));
>> >
>> > So to restate - this function is called from efi_init():
>> > ---
>> >         /* Grab UEFI information placed in FDT by stub */
>> >         efi_system_table = efi_get_fdt_params(&data);
>> >         if (!efi_system_table)
>> >                 return;
>> > ---
>> >
>> > Before this set, this function called get_fdt_params() indeed gets
>> > "params" from a device tree. After this set, this function gets params
>> > from a device tree in some instances, and not in others.
>> > Which feels suboptimal.
>> >
>> > We could rename the function, but then there's still DT-unrelated
>> > code held in fdtparams.c.
>> >
>> > If we go down the route of passing the system table in x1 on boot,
>> > then I guess the effect of assigning efi_system_table will already be
>> > broken out. But should we then split the mm struct initialisation into
>> > separate DT and config table helper functions?
>> >
>> 
>> Not disagreeing but I think it is fine to leave it as I suggested at this
>> point.
>> 
>> Some additional work is needed to get rid of linux,uefi-boot-memmap
>> entirely, and until that happens, passing the EFI system table via
>> X1 and linux,uefi-boot-memmap via DT is not a huge improvement.
>> 
>> linux,uefi-boot-memmap is needed when SetVirtualAddressMap() is called
>> [with a non-1:1 mapping], as the EFI system table contains a remapped
>> address of the config table array in that case.
>
> Of course, the devil on my shoulder suggests the stub could always
> add an address fixup handler for the config table too...
>

What is an address fixup handler? We cannot change this value back,
as that might break the runtime services.


  reply	other threads:[~2026-08-17 16:59 UTC|newest]

Thread overview: 13+ 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
2026-08-17  8:41     ` Ard Biesheuvel
2026-08-17  9:18       ` Leif Lindholm
2026-08-17 11:00         ` Ard Biesheuvel
2026-08-17 14:08           ` Leif Lindholm
2026-08-17 16:59             ` Ard Biesheuvel [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=c3cd6952-87fd-442b-8d54-13c814b8d5f2@app.fastmail.com \
    --to=ardb@kernel.org \
    --cc=chenhuacai@kernel.org \
    --cc=kernel@xen0n.name \
    --cc=leif.lindholm@oss.qualcomm.com \
    --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