All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xen/efi: Handle efi_memmap with Xen relocating itself
@ 2026-11-06 23:18 Jason Andryuk
  2026-11-09 15:11 ` Marek Marczykowski-Górecki
  2026-11-10  8:42 ` Jan Beulich
  0 siblings, 2 replies; 6+ messages in thread
From: Jason Andryuk @ 2026-11-06 23:18 UTC (permalink / raw)
  To: xen-devel
  Cc: Jason Andryuk, Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Michal Orzel, Volodymyr Babchuk, Daniel P. Smith,
	Marek Marczykowski-Górecki, Jan Beulich, Andrew Cooper,
	Roger Pau Monné, Teddy Astie

With Xen booting via Multiboot2/GRUB on an EFI platform, and modified to
use MULTIBOOT2_LOAD_PREFERENCE_LOW, Xen relocates itself with
move_xen().

However a PVH dom0 shows:
kernel: Failed to lookup header 0x77ffd000 in Xen memory map: error -3
kernel: ACPI=0x77ffd000

... for all the Xen efi memory map lookups.  Dumping the memory map at
runtime showed it was scrubbed.  Because Xen relocated itself, the
original physical address referenced through a directmap address goes
stale.

Add efi_arch_mmap_buffer_virt() to convert from the initial value to an
appropriate runtime value.  ARM uses Boot Services AllocatePool(), so it
must continue to use DIRECTMAP_VIRT_START.

For x86, we use an offset into the loaded Xen binary at the time of the
call and add the to XEN_VIRT_START.  ebmalloc() allocates from the Xen
image memory, so it moves during relocation.  xen_phys_start is set for
xen.efi in efi_exit_boot(), but not for xen.gz, so it cannot be used
Use `lea __image_base__(%rip)` to get the current load address, and use
that to calculate the offset into the Xen image.  That offset is also
valid from XEN_VIRT_START.

Fixes: c5b9805bc1f7 ("efi: create new early memory allocator")
Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
I think it could be seen without MULTIBOOT2_LOAD_PREFERENCE_LOW if xen
chose to relocate itself.  But since grub usually loads xen high, it
doesn't relocate itself.
---
 xen/arch/arm/efi/efi-boot.h |  5 +++++
 xen/arch/x86/efi/efi-boot.h | 10 ++++++++++
 xen/common/efi/boot.c       |  2 +-
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
index 069cc68b0a..717bd8d607 100644
--- a/xen/arch/arm/efi/efi-boot.h
+++ b/xen/arch/arm/efi/efi-boot.h
@@ -449,6 +449,11 @@ static void *__init efi_arch_allocate_mmap_buffer(UINTN map_size)
     return ptr;
 }
 
+static void *__init efi_arch_mmap_buffer_virt(void *ptr)
+{
+    return ptr + DIRECTMAP_VIRT_START;
+}
+
 static void __init efi_arch_edd(void)
 {
 }
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index d738b839ee..ffc277ccfd 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -224,6 +224,16 @@ static void *__init efi_arch_allocate_mmap_buffer(UINTN map_size)
     return ebmalloc(map_size);
 }
 
+static void *__init efi_arch_mmap_buffer_virt(void *ptr)
+{
+    unsigned long load_base;
+
+    /* RIP-relative address is the current Xen load address. */
+    asm("lea __image_base__(%%rip), %0" : "=r" (load_base));
+
+    return ptr - load_base + XEN_VIRT_START;
+}
+
 static void __init efi_arch_pre_exit_boot(void)
 {
     if ( !trampoline_phys )
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 89b5831691..5396bad7a5 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1425,8 +1425,8 @@ static void __init efi_exit_boot(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *Syste
     /* Adjust pointers into EFI. */
     efi_ct = (const void *)efi_ct + DIRECTMAP_VIRT_START;
     efi_rs = (const void *)efi_rs + DIRECTMAP_VIRT_START;
-    efi_memmap = (void *)efi_memmap + DIRECTMAP_VIRT_START;
     efi_fw_vendor = (const void *)efi_fw_vendor + DIRECTMAP_VIRT_START;
+    efi_memmap = efi_arch_mmap_buffer_virt(efi_memmap);
 }
 
 /* SAF-1-safe */
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] xen/efi: Handle efi_memmap with Xen relocating itself
  2026-11-06 23:18 [PATCH] xen/efi: Handle efi_memmap with Xen relocating itself Jason Andryuk
@ 2026-11-09 15:11 ` Marek Marczykowski-Górecki
  2026-11-10  8:42 ` Jan Beulich
  1 sibling, 0 replies; 6+ messages in thread
From: Marek Marczykowski-Górecki @ 2026-11-09 15:11 UTC (permalink / raw)
  To: Jason Andryuk
  Cc: xen-devel, Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Michal Orzel, Volodymyr Babchuk, Daniel P. Smith, Jan Beulich,
	Andrew Cooper, Roger Pau Monné, Teddy Astie

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

On Fri, Nov 06, 2026 at 06:18:18PM -0500, Jason Andryuk wrote:
> With Xen booting via Multiboot2/GRUB on an EFI platform, and modified to
> use MULTIBOOT2_LOAD_PREFERENCE_LOW, Xen relocates itself with
> move_xen().
> 
> However a PVH dom0 shows:
> kernel: Failed to lookup header 0x77ffd000 in Xen memory map: error -3
> kernel: ACPI=0x77ffd000
> 
> ... for all the Xen efi memory map lookups.  Dumping the memory map at
> runtime showed it was scrubbed.  Because Xen relocated itself, the
> original physical address referenced through a directmap address goes
> stale.
> 
> Add efi_arch_mmap_buffer_virt() to convert from the initial value to an
> appropriate runtime value.  ARM uses Boot Services AllocatePool(), so it
> must continue to use DIRECTMAP_VIRT_START.
> 
> For x86, we use an offset into the loaded Xen binary at the time of the
> call and add the to XEN_VIRT_START.  ebmalloc() allocates from the Xen
> image memory, so it moves during relocation.  xen_phys_start is set for
> xen.efi in efi_exit_boot(), but not for xen.gz, so it cannot be used
> Use `lea __image_base__(%rip)` to get the current load address, and use
> that to calculate the offset into the Xen image.  That offset is also
> valid from XEN_VIRT_START.
> 
> Fixes: c5b9805bc1f7 ("efi: create new early memory allocator")
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>

I did not tested this, but looks correct:
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

> ---
> I think it could be seen without MULTIBOOT2_LOAD_PREFERENCE_LOW if xen
> chose to relocate itself.  But since grub usually loads xen high, it
> doesn't relocate itself.
> ---
>  xen/arch/arm/efi/efi-boot.h |  5 +++++
>  xen/arch/x86/efi/efi-boot.h | 10 ++++++++++
>  xen/common/efi/boot.c       |  2 +-
>  3 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
> index 069cc68b0a..717bd8d607 100644
> --- a/xen/arch/arm/efi/efi-boot.h
> +++ b/xen/arch/arm/efi/efi-boot.h
> @@ -449,6 +449,11 @@ static void *__init efi_arch_allocate_mmap_buffer(UINTN map_size)
>      return ptr;
>  }
>  
> +static void *__init efi_arch_mmap_buffer_virt(void *ptr)
> +{
> +    return ptr + DIRECTMAP_VIRT_START;
> +}
> +
>  static void __init efi_arch_edd(void)
>  {
>  }
> diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
> index d738b839ee..ffc277ccfd 100644
> --- a/xen/arch/x86/efi/efi-boot.h
> +++ b/xen/arch/x86/efi/efi-boot.h
> @@ -224,6 +224,16 @@ static void *__init efi_arch_allocate_mmap_buffer(UINTN map_size)
>      return ebmalloc(map_size);
>  }
>  
> +static void *__init efi_arch_mmap_buffer_virt(void *ptr)
> +{
> +    unsigned long load_base;
> +
> +    /* RIP-relative address is the current Xen load address. */
> +    asm("lea __image_base__(%%rip), %0" : "=r" (load_base));
> +
> +    return ptr - load_base + XEN_VIRT_START;
> +}
> +
>  static void __init efi_arch_pre_exit_boot(void)
>  {
>      if ( !trampoline_phys )
> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
> index 89b5831691..5396bad7a5 100644
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -1425,8 +1425,8 @@ static void __init efi_exit_boot(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *Syste
>      /* Adjust pointers into EFI. */
>      efi_ct = (const void *)efi_ct + DIRECTMAP_VIRT_START;
>      efi_rs = (const void *)efi_rs + DIRECTMAP_VIRT_START;
> -    efi_memmap = (void *)efi_memmap + DIRECTMAP_VIRT_START;
>      efi_fw_vendor = (const void *)efi_fw_vendor + DIRECTMAP_VIRT_START;
> +    efi_memmap = efi_arch_mmap_buffer_virt(efi_memmap);
>  }
>  
>  /* SAF-1-safe */
> -- 
> 2.34.1
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xen/efi: Handle efi_memmap with Xen relocating itself
  2026-11-06 23:18 [PATCH] xen/efi: Handle efi_memmap with Xen relocating itself Jason Andryuk
  2026-11-09 15:11 ` Marek Marczykowski-Górecki
@ 2026-11-10  8:42 ` Jan Beulich
  2026-11-10 14:42   ` Jason Andryuk
  1 sibling, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2026-11-10  8:42 UTC (permalink / raw)
  To: Jason Andryuk, Marek Marczykowski-Górecki
  Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
	Volodymyr Babchuk, Daniel P. Smith, Andrew Cooper,
	Roger Pau Monné, Teddy Astie, xen-devel

On 07.11.2026 00:18, Jason Andryuk wrote:
> --- a/xen/arch/x86/efi/efi-boot.h
> +++ b/xen/arch/x86/efi/efi-boot.h
> @@ -224,6 +224,16 @@ static void *__init efi_arch_allocate_mmap_buffer(UINTN map_size)
>      return ebmalloc(map_size);
>  }
>  
> +static void *__init efi_arch_mmap_buffer_virt(void *ptr)
> +{
> +    unsigned long load_base;
> +
> +    /* RIP-relative address is the current Xen load address. */
> +    asm("lea __image_base__(%%rip), %0" : "=r" (load_base));
> +
> +    return ptr - load_base + XEN_VIRT_START;
> +}

There's nothing specific to the mmap in this function, hence it's name is
a little odd.

Also the asm() is lacking a number of blanks. (This aspect could be dealt
with while committing, but a possible rename I'd prefer to be done in a v2.)

Jan


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xen/efi: Handle efi_memmap with Xen relocating itself
  2026-11-10  8:42 ` Jan Beulich
@ 2026-11-10 14:42   ` Jason Andryuk
  2026-11-10 15:14     ` Jan Beulich
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Andryuk @ 2026-11-10 14:42 UTC (permalink / raw)
  To: Jan Beulich, Marek Marczykowski-Górecki
  Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
	Volodymyr Babchuk, Daniel P. Smith, Andrew Cooper,
	Roger Pau Monné, Teddy Astie, xen-devel

On 2026-11-10 03:42, Jan Beulich wrote:
> On 07.11.2026 00:18, Jason Andryuk wrote:
>> --- a/xen/arch/x86/efi/efi-boot.h
>> +++ b/xen/arch/x86/efi/efi-boot.h
>> @@ -224,6 +224,16 @@ static void *__init efi_arch_allocate_mmap_buffer(UINTN map_size)
>>       return ebmalloc(map_size);
>>   }
>>   
>> +static void *__init efi_arch_mmap_buffer_virt(void *ptr)
>> +{
>> +    unsigned long load_base;
>> +
>> +    /* RIP-relative address is the current Xen load address. */
>> +    asm("lea __image_base__(%%rip), %0" : "=r" (load_base));
>> +
>> +    return ptr - load_base + XEN_VIRT_START;
>> +}
> 
> There's nothing specific to the mmap in this function, hence it's name is
> a little odd.

It is paired with efi_arch_allocate_mmap_buffer() above, so I wanted the 
name to indicate that it is converting mmap_buffer to a virt address. 
`ptr` needs to be within the image, so it has limited applicability. 
It's handling a special case, so naming it for that special case helps 
limit its use.  A generic name could be misused.

Having said that, I am open to renaming if that is desired. 
Alternatives could be:
efi_arch_xen_phys_to_virt()
or
efi_arch_image_phys_to_virt()
or
efi_arch_xen_image_to_virt()

> Also the asm() is lacking a number of blanks. (This aspect could be dealt
> with while committing, but a possible rename I'd prefer to be done in a v2.)
Oh, yes, sorry about that.

Thanks,
Jason


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xen/efi: Handle efi_memmap with Xen relocating itself
  2026-11-10 14:42   ` Jason Andryuk
@ 2026-11-10 15:14     ` Jan Beulich
  2026-11-10 15:16       ` Jan Beulich
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2026-11-10 15:14 UTC (permalink / raw)
  To: Jason Andryuk, Marek Marczykowski-Górecki
  Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
	Volodymyr Babchuk, Daniel P. Smith, Andrew Cooper,
	Roger Pau Monné, Teddy Astie, xen-devel

On 10.11.2026 15:42, Jason Andryuk wrote:
> On 2026-11-10 03:42, Jan Beulich wrote:
>> On 07.11.2026 00:18, Jason Andryuk wrote:
>>> --- a/xen/arch/x86/efi/efi-boot.h
>>> +++ b/xen/arch/x86/efi/efi-boot.h
>>> @@ -224,6 +224,16 @@ static void *__init efi_arch_allocate_mmap_buffer(UINTN map_size)
>>>       return ebmalloc(map_size);
>>>   }
>>>   
>>> +static void *__init efi_arch_mmap_buffer_virt(void *ptr)
>>> +{
>>> +    unsigned long load_base;
>>> +
>>> +    /* RIP-relative address is the current Xen load address. */
>>> +    asm("lea __image_base__(%%rip), %0" : "=r" (load_base));
>>> +
>>> +    return ptr - load_base + XEN_VIRT_START;
>>> +}
>>
>> There's nothing specific to the mmap in this function, hence it's name is
>> a little odd.
> 
> It is paired with efi_arch_allocate_mmap_buffer() above, so I wanted the 
> name to indicate that it is converting mmap_buffer to a virt address. 

Hmm, right. And this is entirely different between x86 and Arm. So common
code wants to call a function of this name, but on x86 this may then want
to be a thin wrapper around ...

> Having said that, I am open to renaming if that is desired. 
> Alternatives could be:
> efi_arch_xen_phys_to_virt()
> or
> efi_arch_image_phys_to_virt()
> or
> efi_arch_xen_image_to_virt()

... a function of any of these names (which in turn would be possible to
use for other purposes). Marek - thoughts?

With this constraint on the x86 side, wouldn't you then better add an
ASSERT()-like construct to the actual translation function?

Jan


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xen/efi: Handle efi_memmap with Xen relocating itself
  2026-11-10 15:14     ` Jan Beulich
@ 2026-11-10 15:16       ` Jan Beulich
  0 siblings, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2026-11-10 15:16 UTC (permalink / raw)
  To: Jason Andryuk, Marek Marczykowski-Górecki
  Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
	Volodymyr Babchuk, Daniel P. Smith, Andrew Cooper,
	Roger Pau Monné, Teddy Astie, xen-devel

On 10.11.2026 16:14, Jan Beulich wrote:
> On 10.11.2026 15:42, Jason Andryuk wrote:
>> On 2026-11-10 03:42, Jan Beulich wrote:
>>> On 07.11.2026 00:18, Jason Andryuk wrote:
>>>> --- a/xen/arch/x86/efi/efi-boot.h
>>>> +++ b/xen/arch/x86/efi/efi-boot.h
>>>> @@ -224,6 +224,16 @@ static void *__init efi_arch_allocate_mmap_buffer(UINTN map_size)
>>>>       return ebmalloc(map_size);
>>>>   }
>>>>   
>>>> +static void *__init efi_arch_mmap_buffer_virt(void *ptr)
>>>> +{
>>>> +    unsigned long load_base;
>>>> +
>>>> +    /* RIP-relative address is the current Xen load address. */
>>>> +    asm("lea __image_base__(%%rip), %0" : "=r" (load_base));
>>>> +
>>>> +    return ptr - load_base + XEN_VIRT_START;
>>>> +}
>>>
>>> There's nothing specific to the mmap in this function, hence it's name is
>>> a little odd.
>>
>> It is paired with efi_arch_allocate_mmap_buffer() above, so I wanted the 
>> name to indicate that it is converting mmap_buffer to a virt address. 
> 
> Hmm, right. And this is entirely different between x86 and Arm. So common
> code wants to call a function of this name, but on x86 this may then want
> to be a thin wrapper around ...
> 
>> Having said that, I am open to renaming if that is desired. 
>> Alternatives could be:
>> efi_arch_xen_phys_to_virt()
>> or
>> efi_arch_image_phys_to_virt()
>> or
>> efi_arch_xen_image_to_virt()
> 
> ... a function of any of these names (which in turn would be possible to
> use for other purposes). Marek - thoughts?
> 
> With this constraint on the x86 side, wouldn't you then better add an
> ASSERT()-like construct to the actual translation function?

Or use the existing (and Arm) logic when the pointer isn't within the Xen
image. (There can be ambiguous cases, though, I fear.)

Jan


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-11-10 15:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-06 23:18 [PATCH] xen/efi: Handle efi_memmap with Xen relocating itself Jason Andryuk
2026-11-09 15:11 ` Marek Marczykowski-Górecki
2026-11-10  8:42 ` Jan Beulich
2026-11-10 14:42   ` Jason Andryuk
2026-11-10 15:14     ` Jan Beulich
2026-11-10 15:16       ` Jan Beulich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.