Re: [PATCH v3 05/17] x86/efi: Simplify real mode trampoline allocation quirk

On Thu, Apr 23, 2026 at 05:20:30PM +0200, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <[email protected]>
> 
> To work around a common bug in EFI firmware for x86 systems, Linux
> reserves all EFI boot services code and data regions until after it has
> invoked the SetVirtualAddressMap() EFI runtime service. This is needed
> because those regions may still be accessed by the firmware during that
> call, even though the EFI spec says that they shouldn't.
> 
> This includes any boot services data regions below 1M, which might mean
> that by the time the real mode trampoline is being allocated, all memory
> below 1M is already exhausted.
> 
> Commit
> 
>   5bc653b73182 ("x86/efi: Allocate a trampoline if needed in efi_free_boot_services()")
> 
> added a quirk to detect this condition, and to make another attempt at
> allocating the real mode trampoline when freeing those boot services
> regions again. This is a rather crude hack, which gets in the way of
> cleanup work on the EFI/x86 memory map handling code.
> 
> Given that
> 
> - the real mode trampoline is normally allocated soon after all EFI boot
>   services regions are reserved temporarily,
> - this allocation logic marks all memory below 1M as reserved,
> - the trampoline memory is not actually populated until an early
>   initcall,
> 
> there is actually no need to reserve any boot services regions below 1M,
> even if they are mapped into the EFI page tables during the call to
> SetVirtualAddressMap(). So cap the lower bound of the reserved regions
> to 1M, and fix up the size accordingly when making the reservation. This
> allows the additional quirk to be dropped entirely.
> 
> Signed-off-by: Ard Biesheuvel <[email protected]>
> ---
>  arch/x86/platform/efi/quirks.c | 34 ++++----------------
>  1 file changed, 6 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
> index e2e57e9201a9..e79fb94c1bf6 100644
> --- a/arch/x86/platform/efi/quirks.c
> +++ b/arch/x86/platform/efi/quirks.c
> @@ -324,10 +324,14 @@ void __init efi_reserve_boot_services(void)
>  		return;
>  
>  	for_each_efi_memory_desc(md) {
> -		u64 start = md->phys_addr;
> -		u64 size = md->num_pages << EFI_PAGE_SHIFT;
> +		u64 start = max(md->phys_addr, SZ_1M);

A comment that says that we don't reserve regions below 1M because they are
reserved elsewhere would be nice here. Other that that

Acked-by: Mike Rapoport (Microsoft) <[email protected]>

> +		u64 end = md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT);
> +		u64 size = end - start;
>  		bool already_reserved;
>  
> +		if (end <= start)
> +			continue;
> +
>  		if (md->type != EFI_BOOT_SERVICES_CODE &&
>  		    md->type != EFI_BOOT_SERVICES_DATA)
>  			continue;

-- 
Sincerely yours,
Mike.