[PATCH 00/15] x86: PVH Dom0 loading rework

[PATCH 00/15] x86: PVH Dom0 loading rework

[Jan Beulich]

The primary goal is to accommodate large initrd-s without requiring people
to restrict Dom0's memory just for that. Other issues are being addressed
as well, though, and some cleanup is done, in part even outside of the main
target area of this series.

01: EFI: guard against oversized modules
02: PV: make better use of assign_pages()
03: setup: break out E820_RAM accumulation
04: setup: special treatment of initrd for PVH Dom0
05: PVH: split pvh_load_kernel()
06: PVH: keep a copy of Dom0's initial E820 map
07: PVH: lift E820 alignment requirement
08: PVH: split pvh_setup_acpi()
09: PVH: split pvh_setup_acpi()
10: PVH: convert find_memory() to using a rangeset
11: PVH: try to map initrd directly into Dom0
12: PVH: heed the kernel in pvh_steal_ram()
13: PVH: prefer smaller E820 regions for stealing from
14: PVH: log Dom0's E820 map earlier
15: PVH: simplify writing of Dom0's RSDP

While repeatedly looking at the logs, the memory allocation stats logged
further suggested that we may be able to do better in populating the P2M.
I was hoping that by going backwards, we might be able to allow for one
more 1G mapping. Yet testing that (now omitted) change on two sufficiently
different systems resulted in "makes no difference". I conclude (without
having proven it) that perhaps earlier during boot we shatter an order-18
page, when perhaps we could avoid doing so.

Jan

[PATCH 01/15] x86/EFI: guard against oversized modules

[Jan Beulich]

x86/EFI: guard against oversized modules

module_t's mod_end field (which we repurpose to hold a module's size)
cannot hold values 4Gb and up. Don't let truncation go silently.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
MB and MB2 also constrain module to below 4Gb. The constraint here may be
possible to drop once hyperlaunch work has progressed enough to no longer
require the intermediate mb_modules[].

--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -736,6 +736,13 @@ static void __init efi_arch_handle_modul
     place_string(&mb_modules[mbi.mods_count].string, local_name.s);
     mb_modules[mbi.mods_count].mod_start = file->addr >> PAGE_SHIFT;
     mb_modules[mbi.mods_count].mod_end = file->size;
+
+    if ( mb_modules[mbi.mods_count].mod_end != file->size )
+    {
+        PrintStr(name);
+        blexit(L" is too large");
+    }
+
     ++mbi.mods_count;
     efi_bs->FreePool(ptr);
 }

Re: [PATCH 01/15] x86/EFI: guard against oversized modules

[George Dunlap]

[Re-sending from @xenproject.org, since my other email bounced]

On Mon, Oct 12, 2026 at 9:19 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> x86/EFI: guard against oversized modules

Nit: This repeats the subject line.  `git am` of this has two identical lines.

> module_t's mod_end field (which we repurpose to hold a module's size)
> cannot hold values 4Gb and up. Don't let truncation go silently.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

With the repeated subject line fixed:

Reviewed-by: George Dunlap <gwd@xenproject.org>

[PATCH 02/15] x86/PV: make better use of assign_pages()

[Jan Beulich]

As of the referenced commit there's no reason to open-code the loop
anymore. With that replaced by a single call, there's then also no point
in using the local "mfn" in this code.

Amends: 5260e8fb93f0 ("xen: re-define assign_pages and introduce a new function assign_page")
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -618,12 +618,12 @@ static int __init dom0_construct(const s
                      (vinitrd_start - v_start) >> PAGE_SHIFT :
                      domain_tot_pages(d);
         initrd_mfn = paddr_to_pfn(initrd->start);
-        mfn = initrd_mfn;
         count = PFN_UP(initrd_len);
 
 #ifdef CONFIG_PV32
         if ( d->arch.pv.physaddr_bitsize &&
-             ((mfn + count - 1) >> (d->arch.pv.physaddr_bitsize - PAGE_SHIFT)) )
+             ((initrd_mfn + count - 1) >>
+              (d->arch.pv.physaddr_bitsize - PAGE_SHIFT)) )
         {
             order = get_order_from_pages(count);
             page = alloc_domheap_pages(d, order, MEMF_no_scrub);
@@ -648,9 +648,8 @@ static int __init dom0_construct(const s
         else
 #endif
         {
-            while ( count-- )
-                if ( assign_pages(mfn_to_page(_mfn(mfn++)), 1, d, 0) )
-                    BUG();
+            if ( assign_pages(mfn_to_page(_mfn(initrd_mfn)), count, d, 0) )
+                BUG();
             /*
              * We have mapped the initrd directly into dom0, and assigned the
              * pages. Tell the boot_module handling that we've freed it, so the

Re: [PATCH 02/15] x86/PV: make better use of assign_pages()

[George Dunlap]

On Mon, Oct 12, 2026 at 9:19 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> As of the referenced commit there's no reason to open-code the loop
> anymore. With that replaced by a single call, there's then also no point
> in using the local "mfn" in this code.
>
> Amends: 5260e8fb93f0 ("xen: re-define assign_pages and introduce a new function assign_page")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: George Dunlap <gwd@xenproject.org>

[PATCH 03/15] x86/setup: break out E820_RAM accumulation

[Jan Beulich]

We already have two instances of this, and we're about to gain a third.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1007,6 +1007,17 @@ static unsigned int __init copy_bios_e82
     return n;
 }
 
+static unsigned long __init nr_ram_pages(void)
+{
+    unsigned long nr_pages = 0;
+
+    for ( unsigned int i = 0; i < e820.nr_map; i++ )
+        if ( e820.map[i].type == E820_RAM )
+            nr_pages += e820.map[i].size >> PAGE_SHIFT;
+
+    return nr_pages;
+}
+
 /*
  * Calculate the maximum possible size of the dom0 cmdline.  Pieces of the
  * dom0 cmdline optionally come from the bootloader directly, from Xen's
@@ -1139,7 +1150,7 @@ void asmlinkage __init noreturn __start_
     struct cpu_info *info = get_cpu_info(), *bsp_info;
     unsigned int initrdidx, num_parked = 0;
     struct boot_info *bi;
-    unsigned long nr_pages, raw_max_page;
+    unsigned long raw_max_page;
     int i, j, bytes = 0;
     unsigned long eb_start, eb_end;
     bool acpi_boot_table_init_done = false, relocated = false;
@@ -1491,11 +1502,7 @@ void asmlinkage __init noreturn __start_
     memcpy(&boot_e820, &e820, sizeof(e820));
 
     /* Early kexec reservation (explicit static start address). */
-    nr_pages = 0;
-    for ( i = 0; i < e820.nr_map; i++ )
-        if ( e820.map[i].type == E820_RAM )
-            nr_pages += e820.map[i].size >> PAGE_SHIFT;
-    set_kexec_crash_area_size((u64)nr_pages << PAGE_SHIFT);
+    set_kexec_crash_area_size(nr_ram_pages() << PAGE_SHIFT);
     kexec_reserve_area();
 
     for ( i = 0; i < bi->nr_modules; i++ )
@@ -1862,14 +1869,10 @@ void asmlinkage __init noreturn __start_
                         (unsigned long)__va(__pa(__2M_rodata_end)),
                         PAGE_HYPERVISOR_RO);
 
-    nr_pages = 0;
-    for ( i = 0; i < e820.nr_map; i++ )
-        if ( e820.map[i].type == E820_RAM )
-            nr_pages += e820.map[i].size >> PAGE_SHIFT;
+    total_pages = nr_ram_pages();
     printk("System RAM: %luMB (%lukB)\n",
-           nr_pages >> (20 - PAGE_SHIFT),
-           nr_pages << (PAGE_SHIFT - 10));
-    total_pages = nr_pages;
+           total_pages >> (20 - PAGE_SHIFT),
+           total_pages << (PAGE_SHIFT - 10));
 
     /* Sanity check for unwanted bloat of certain hypercall structures. */
     BUILD_BUG_ON(sizeof_field(struct xen_platform_op, u) !=

Re: [PATCH 03/15] x86/setup: break out E820_RAM accumulation

[George Dunlap]

On Mon, Oct 12, 2026 at 9:20 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> We already have two instances of this, and we're about to gain a third.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: George Dunlap <gwd@laleolanguage.com>

Re: [PATCH 03/15] x86/setup: break out E820_RAM accumulation

[Jan Beulich]

On 12.10.2026 12:20, George Dunlap wrote:
> On Mon, Oct 12, 2026 at 9:20 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> We already have two instances of this, and we're about to gain a third.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: George Dunlap <gwd@laleolanguage.com>

Thanks, yet to double check - did you really mean to use this more private
email of yours?

Jan

Re: [PATCH 03/15] x86/setup: break out E820_RAM accumulation

[George Dunlap]

On Mon, Oct 12, 2026 at 11:23 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 12.10.2026 12:20, George Dunlap wrote:
> > On Mon, Oct 12, 2026 at 9:20 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> We already have two instances of this, and we're about to gain a third.
> >>
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >
> > Reviewed-by: George Dunlap <gwd@laleolanguage.com>
>
> Thanks, yet to double check - did you really mean to use this more private
> email of yours?

*Sigh*  No, just autocomplete.  Please use this one:

Reviewed-by: George Dunlap <gwd@xenproject.org>

Re: [PATCH 03/15] x86/setup: break out E820_RAM accumulation

[Roger Pau Monné]

On Mon, Oct 12, 2026 at 10:20:16AM +0200, Jan Beulich wrote:
> We already have two instances of this, and we're about to gain a third.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Roger Pau Monné <roger@xenproject.org>

> 
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1007,6 +1007,17 @@ static unsigned int __init copy_bios_e82
>      return n;
>  }
>  
> +static unsigned long __init nr_ram_pages(void)
> +{
> +    unsigned long nr_pages = 0;
> +
> +    for ( unsigned int i = 0; i < e820.nr_map; i++ )
> +        if ( e820.map[i].type == E820_RAM )
> +            nr_pages += e820.map[i].size >> PAGE_SHIFT;

Do we want/need to ensure size is page-aligned here?  AFAICT this is
the sanitized memory map, so it should have all regions aligned to
page boundaries.

Thanks, Roger.

Re: [PATCH 03/15] x86/setup: break out E820_RAM accumulation

[Jan Beulich]

On 13.10.2026 13:05, Roger Pau Monné wrote:
> On Mon, Oct 12, 2026 at 10:20:16AM +0200, Jan Beulich wrote:
>> We already have two instances of this, and we're about to gain a third.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: Roger Pau Monné <roger@xenproject.org>

Thanks.

>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -1007,6 +1007,17 @@ static unsigned int __init copy_bios_e82
>>      return n;
>>  }
>>  
>> +static unsigned long __init nr_ram_pages(void)
>> +{
>> +    unsigned long nr_pages = 0;
>> +
>> +    for ( unsigned int i = 0; i < e820.nr_map; i++ )
>> +        if ( e820.map[i].type == E820_RAM )
>> +            nr_pages += e820.map[i].size >> PAGE_SHIFT;
> 
> Do we want/need to ensure size is page-aligned here?  AFAICT this is
> the sanitized memory map, so it should have all regions aligned to
> page boundaries.

Neither user cares about an entirely exact value, so keeping things
simple looks sufficient.

Jan

[PATCH 04/15] x86/setup: special treatment of initrd for PVH Dom0

[Jan Beulich]

Dealing with large initrd-s is problematic in two ways: We needlessly
copy around a large chunk of data, and our calculation of how much
memory to give Dom0 assumes the space taken by the initrd is actually
available at the time of populating Dom0's P2M.

As a first step, make effort during early boot to try to place the initrd
in a chunk of memory that's a multiple of 1Gb (or, if not available / not
possibly going to fit, in a chunk a multiple of 2Mb in size). This may
(but generally won't) result in more copying, which we simply need to
accept. Record (and use) the extra space allocated in (as) headroom, which
then needs accouting for in pvh_load_kernel().

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Not being able to check bm->kind is of course not very nice. The field is
only set quite a bit later, and that setting can - afaict - not be moved
ahead of the call to xsm_multiboot_init().

The bumping of mask to L3's in __start_xen() transiently breaks booting of
Dom0 with overly much memory assigned to it. That'll be resolved when the
headroom space is properly dealt with (consumed into Dom0, or freed).
Therefore this patch and everything up to the one adding the consuming
code want committing together, or addition of that conditional wants
delaying until that later change.

Simply using nr_ram_pages()' return value is of course a pretty raw
estimate. We could e.g. also subtract Xen's image size and, perhaps more
relevant, an estimate of frame_table[].

--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -445,6 +445,20 @@ unsigned long __init dom0_compute_nr_pag
     return nr_pages;
 }
 
+/*
+ * A simplistic sibling of dom0_compute_nr_pages(), for obtaining a very early
+ * estimate.
+ */
+unsigned long __init get_dom0_size(unsigned long avail)
+{
+    if ( CONFIG_DOM0_MEM[0] && !dom0_mem_set )
+        parse_dom0_mem(CONFIG_DOM0_MEM);
+
+    return get_memsize(memsize_gt_zero(&dom0_size) ? &dom0_size
+                                                   : &dom0_max_size,
+                       avail);
+}
+
 static void __init process_dom0_ioports_disable(struct domain *dom0)
 {
     unsigned long io_from, io_to;
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -653,7 +653,8 @@ static int __init pvh_load_kernel(
     void *image_base = bootstrap_map_bm(image);
     void *image_start = image_base + image->arch.headroom;
     unsigned long image_len = image->size;
-    unsigned long initrd_len = initrd ? initrd->size : 0;
+    unsigned long initrd_len =
+        initrd ? initrd->size - initrd->arch.headroom : 0;
     size_t cmdline_len = bd->cmdline ? strlen(bd->cmdline) + 1 : 0;
     const char *initrd_cmdline = NULL;
     struct elf_binary elf;
@@ -751,7 +752,8 @@ static int __init pvh_load_kernel(
 
     if ( initrd != NULL )
     {
-        rc = hvm_copy_to_guest_phys(last_addr, __va(initrd->start),
+        rc = hvm_copy_to_guest_phys(last_addr,
+                                    __va(initrd->start + initrd->arch.headroom),
                                     initrd_len, v);
         if ( rc )
         {
--- a/xen/arch/x86/include/asm/setup.h
+++ b/xen/arch/x86/include/asm/setup.h
@@ -68,4 +68,6 @@ extern bool opt_dom0_verbose;
 extern bool opt_dom0_cpuid_faulting;
 extern bool opt_dom0_msr_relaxed;
 
+unsigned long get_dom0_size(unsigned long avail);
+
 #endif
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1608,6 +1608,40 @@ void asmlinkage __init noreturn __start_
             struct boot_module *bm = &bi->mods[j];
             unsigned long size = PAGE_ALIGN(bm->arch.headroom + bm->size);
 
+            /*
+             * For PVH Dom0 try to place the initrd in a full multi-Gb (or
+             * multi-2Mb) chunk, such that assigning the pages directly can
+             * be done while still using superpage mappings.
+             */
+            if ( opt_dom0_pvh && j == 1 /* bm->kind == BOOTMOD_RAMDISK */ )
+            {
+                if ( get_dom0_size(nr_ram_pages()) >= PFN_DOWN(GB(2)) )
+                    mask = (1UL << L3_PAGETABLE_SHIFT) - 1;
+
+                for ( ; mask > PAGE_SIZE; mask >>= PAGETABLE_ORDER )
+                {
+                    if ( !(bm->start & mask) && bm->arch.relocated )
+                        break;
+
+                    size = PAGE_ALIGN(bm->size - bm->arch.headroom);
+                    end = e;
+                    do {
+                        end = consider_modules((s + mask) & ~mask, end & ~mask,
+                                               (size + mask) & ~mask, bi->mods,
+                                               bi->nr_modules + relocated, j);
+                    } while ( (end & mask) && end > s );
+
+                    if ( highmem_start && end > highmem_start )
+                        break;
+
+                    size = (size + mask) & ~mask;
+                    bm->start += bm->arch.headroom;
+                    bm->size  -= bm->arch.headroom;
+                    bm->arch.headroom = size - PAGE_ALIGN(bm->size);
+                    goto move_module;
+                }
+            }
+
             if ( bm->arch.relocated )
                 continue;
 
@@ -1620,6 +1654,7 @@ void asmlinkage __init noreturn __start_
 
             if ( s < end && (bm->arch.headroom || (end - size) > bm->start) )
             {
+            move_module:
                 move_memory(end - size + bm->arch.headroom, bm->start, bm->size);
                 bm->start = (end - size);
                 bm->size += bm->arch.headroom;

Re: [PATCH 04/15] x86/setup: special treatment of initrd for PVH Dom0

[George Dunlap]

On Mon, Oct 12, 2026 at 9:20 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> Dealing with large initrd-s is problematic in two ways: We needlessly
> copy around a large chunk of data, and our calculation of how much
> memory to give Dom0 assumes the space taken by the initrd is actually
> available at the time of populating Dom0's P2M.
>
> As a first step, make effort during early boot to try to place the initrd
> in a chunk of memory that's a multiple of 1Gb (or, if not available / not
> possibly going to fit, in a chunk a multiple of 2Mb in size). This may
> (but generally won't) result in more copying, which we simply need to
> accept. Record (and use) the extra space allocated in (as) headroom, which
> then needs accouting for in pvh_load_kernel().
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Not being able to check bm->kind is of course not very nice. The field is
> only set quite a bit later, and that setting can - afaict - not be moved
> ahead of the call to xsm_multiboot_init().
>
> The bumping of mask to L3's in __start_xen() transiently breaks booting of
> Dom0 with overly much memory assigned to it. That'll be resolved when the
> headroom space is properly dealt with (consumed into Dom0, or freed).
> Therefore this patch and everything up to the one adding the consuming
> code want committing together, or addition of that conditional wants
> delaying until that later change.

I'm generally opposed to breaking bisection.  I'd personally prefer an
extra conditional until then, but I'd defer to the other maintainers
on that (don't consider that a nack).

> --- a/xen/arch/x86/include/asm/setup.h
> +++ b/xen/arch/x86/include/asm/setup.h
> @@ -68,4 +68,6 @@ extern bool opt_dom0_verbose;
>  extern bool opt_dom0_cpuid_faulting;
>  extern bool opt_dom0_msr_relaxed;
>
> +unsigned long get_dom0_size(unsigned long avail);
> +
>  #endif
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1608,6 +1608,40 @@ void asmlinkage __init noreturn __start_
>              struct boot_module *bm = &bi->mods[j];
>              unsigned long size = PAGE_ALIGN(bm->arch.headroom + bm->size);
>
> +            /*
> +             * For PVH Dom0 try to place the initrd in a full multi-Gb (or
> +             * multi-2Mb) chunk, such that assigning the pages directly can
> +             * be done while still using superpage mappings.
> +             */
> +            if ( opt_dom0_pvh && j == 1 /* bm->kind == BOOTMOD_RAMDISK */ )

What if there's a microcode blob or an XSM policy here instead?

> +            {
> +                if ( get_dom0_size(nr_ram_pages()) >= PFN_DOWN(GB(2)) )
> +                    mask = (1UL << L3_PAGETABLE_SHIFT) - 1;
> +
> +                for ( ; mask > PAGE_SIZE; mask >>= PAGETABLE_ORDER )
> +                {
> +                    if ( !(bm->start & mask) && bm->arch.relocated )
> +                        break;
> +
> +                    size = PAGE_ALIGN(bm->size - bm->arch.headroom);
> +                    end = e;
> +                    do {
> +                        end = consider_modules((s + mask) & ~mask, end & ~mask,
> +                                               (size + mask) & ~mask, bi->mods,
> +                                               bi->nr_modules + relocated, j);
> +                    } while ( (end & mask) && end > s );
> +
> +                    if ( highmem_start && end > highmem_start )
> +                        break;
> +
> +                    size = (size + mask) & ~mask;
> +                    bm->start += bm->arch.headroom;
> +                    bm->size  -= bm->arch.headroom;
> +                    bm->arch.headroom = size - PAGE_ALIGN(bm->size);
> +                    goto move_module;
> +                }

Every path through this block either does a 'break' or a 'goto'; so
this isn't actually a for loop at all -- the 2MiB fallback can never
execute.

Did you forget an `if (end) { }` somewhere?

As it stands, if the 1GiB pass doesn't work, it looks like we'll jump
to move_module with 'end' at 0, and likely fault the copy.

Also, looks like consider_modules() takes a uint32_t for size, and we
round up to the nearest GiB; so if the dom0 initrd is between 3GiB and
4GiB, won't we pass 0 as a size argument, which will always fit?

 -George

Re: [PATCH 04/15] x86/setup: special treatment of initrd for PVH Dom0

[Jan Beulich]

On 12.10.2026 13:23, George Dunlap wrote:
> On Mon, Oct 12, 2026 at 9:20 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> Dealing with large initrd-s is problematic in two ways: We needlessly
>> copy around a large chunk of data, and our calculation of how much
>> memory to give Dom0 assumes the space taken by the initrd is actually
>> available at the time of populating Dom0's P2M.
>>
>> As a first step, make effort during early boot to try to place the initrd
>> in a chunk of memory that's a multiple of 1Gb (or, if not available / not
>> possibly going to fit, in a chunk a multiple of 2Mb in size). This may
>> (but generally won't) result in more copying, which we simply need to
>> accept. Record (and use) the extra space allocated in (as) headroom, which
>> then needs accouting for in pvh_load_kernel().
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> Not being able to check bm->kind is of course not very nice. The field is
>> only set quite a bit later, and that setting can - afaict - not be moved
>> ahead of the call to xsm_multiboot_init().
>>
>> The bumping of mask to L3's in __start_xen() transiently breaks booting of
>> Dom0 with overly much memory assigned to it. That'll be resolved when the
>> headroom space is properly dealt with (consumed into Dom0, or freed).
>> Therefore this patch and everything up to the one adding the consuming
>> code want committing together, or addition of that conditional wants
>> delaying until that later change.
> 
> I'm generally opposed to breaking bisection.  I'd personally prefer an
> extra conditional until then, but I'd defer to the other maintainers
> on that (don't consider that a nack).

Noted. If no-one else voices a different opinion, I'll wrap that piece of
code in "#if 0".

>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -1608,6 +1608,40 @@ void asmlinkage __init noreturn __start_
>>              struct boot_module *bm = &bi->mods[j];
>>              unsigned long size = PAGE_ALIGN(bm->arch.headroom + bm->size);
>>
>> +            /*
>> +             * For PVH Dom0 try to place the initrd in a full multi-Gb (or
>> +             * multi-2Mb) chunk, such that assigning the pages directly can
>> +             * be done while still using superpage mappings.
>> +             */
>> +            if ( opt_dom0_pvh && j == 1 /* bm->kind == BOOTMOD_RAMDISK */ )
> 
> What if there's a microcode blob or an XSM policy here instead?

Especially for the XSM policy - very good question (hence also the
respective post-commit-message remark). The ucode blob we can tell,
if necessary, but aiui the XSM policy we can't. I have absolutely no
idea how to deal with this, and it's only because of this that I
left things in this shape.

>> +            {
>> +                if ( get_dom0_size(nr_ram_pages()) >= PFN_DOWN(GB(2)) )
>> +                    mask = (1UL << L3_PAGETABLE_SHIFT) - 1;
>> +
>> +                for ( ; mask > PAGE_SIZE; mask >>= PAGETABLE_ORDER )
>> +                {
>> +                    if ( !(bm->start & mask) && bm->arch.relocated )
>> +                        break;
>> +
>> +                    size = PAGE_ALIGN(bm->size - bm->arch.headroom);
>> +                    end = e;
>> +                    do {
>> +                        end = consider_modules((s + mask) & ~mask, end & ~mask,
>> +                                               (size + mask) & ~mask, bi->mods,
>> +                                               bi->nr_modules + relocated, j);
>> +                    } while ( (end & mask) && end > s );
>> +
>> +                    if ( highmem_start && end > highmem_start )
>> +                        break;
>> +
>> +                    size = (size + mask) & ~mask;
>> +                    bm->start += bm->arch.headroom;
>> +                    bm->size  -= bm->arch.headroom;
>> +                    bm->arch.headroom = size - PAGE_ALIGN(bm->size);
>> +                    goto move_module;
>> +                }
> 
> Every path through this block either does a 'break' or a 'goto'; so
> this isn't actually a for loop at all -- the 2MiB fallback can never
> execute.
> 
> Did you forget an `if (end) { }` somewhere?

"Forget" as in "wrongly purge when purging other debugging code", as I
had temporary logic there to actually also exercise the falling back to
2M and 4k. So yes,

                    if ( end <= s )
                        continue;

is supposed to live right after the inner loop.

> Also, looks like consider_modules() takes a uint32_t for size, and we
> round up to the nearest GiB; so if the dom0 initrd is between 3GiB and
> 4GiB, won't we pass 0 as a size argument, which will always fit?

Hmm, ugly. Since imo at some point we will want to lift the 4G
restriction on initrd and kernel anyway, I think I'll slip in a prereq
patch widening the function's parameter.

Thank you very much for spotting both of these flaws.

Jan

Re: [PATCH 04/15] x86/setup: special treatment of initrd for PVH Dom0

[George Dunlap]

On Mon, Oct 12, 2026 at 12:51 PM Jan Beulich <jbeulich@suse.com> wrote:
> Thank you very much for spotting both of these flaws.

Both were first spotted by Opus 5.5; I mostly filtered and verified
the issues then wrote a response in my own words.  I figured it would
be a useful way to contribute back that wouldn't delay the ASI series
too much.  (And I'm a bit torn between adding "Claude noticed.." all
over the place, which sounds kind of pretentious or like shilling, or
not mentioning it, and making it sound like I did all the work.)

 -George

[PATCH 05/15] x86/PVH: split pvh_load_kernel()

[Jan Beulich]

All E820 manipulations want to happen before the initrd is placed. As a
first step, split placing the kernel from actually loading it. Then
placement of other data (ACPI, VMX) can be inserted between kernel
placement and P2M population (followed by the loading of the kernel).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
If need be, moving the "elf" local variable to dom0_construct_pvh() could
of course further be split off. Just that this by itself doesn't make much
sense.

--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -644,26 +644,17 @@ static bool __init check_and_adjust_load
     return true;
 }
 
-static int __init pvh_load_kernel(
-    const struct boot_domain *bd, paddr_t *entry, paddr_t *start_info_addr)
+static int __init pvh_place_kernel(
+    const struct boot_domain *bd,
+    struct elf_binary *elf,
+    paddr_t *entry)
 {
     struct domain *d = bd->d;
     struct boot_module *image = bd->kernel;
-    struct boot_module *initrd = bd->initrd;
     void *image_base = bootstrap_map_bm(image);
     void *image_start = image_base + image->arch.headroom;
     unsigned long image_len = image->size;
-    unsigned long initrd_len =
-        initrd ? initrd->size - initrd->arch.headroom : 0;
-    size_t cmdline_len = bd->cmdline ? strlen(bd->cmdline) + 1 : 0;
-    const char *initrd_cmdline = NULL;
-    struct elf_binary elf;
     struct elf_dom_parms parms;
-    size_t extra_space;
-    paddr_t last_addr;
-    struct hvm_start_info start_info = { 0 };
-    struct hvm_modlist_entry mod = { 0 };
-    struct vcpu *v = d->vcpu[0];
     int rc;
 
     if ( (rc = bzimage_parse(image_base, &image_start, &image_len)) != 0 )
@@ -672,19 +663,19 @@ static int __init pvh_load_kernel(
         return rc;
     }
 
-    if ( (rc = elf_init(&elf, image_start, image_len)) != 0 )
+    if ( (rc = elf_init(elf, image_start, image_len)) != 0 )
     {
         printk("Unable to init ELF\n");
         return rc;
     }
     if ( opt_dom0_verbose )
-        elf_set_verbose(&elf);
-    elf_parse_binary(&elf);
-    if ( (rc = elf_xen_parse(&elf, &parms, true)) != 0 )
+        elf_set_verbose(elf);
+    elf_parse_binary(elf);
+    if ( (rc = elf_xen_parse(elf, &parms, true)) != 0 )
     {
         printk("Unable to parse kernel for ELFNOTES\n");
-        if ( elf_check_broken(&elf) )
-            printk("%pd kernel: broken ELF: %s\n", d, elf_check_broken(&elf));
+        if ( elf_check_broken(elf) )
+            printk("%pd kernel: broken ELF: %s\n", d, elf_check_broken(elf));
         return rc;
     }
 
@@ -697,20 +688,43 @@ static int __init pvh_load_kernel(
     if ( (rc = initdom_check_parms(d, &parms)) != 0 )
         return rc;
 
-    /* Copy the OS image and free temporary buffer. */
-    elf.dest_base = (void *)(parms.virt_kstart - parms.virt_base);
-    elf.dest_size = parms.virt_kend - parms.virt_kstart;
+    elf->dest_base = (void *)(parms.virt_kstart - parms.virt_base);
+    elf->dest_size = parms.virt_kend - parms.virt_kstart;
 
-    if ( !check_and_adjust_load_address(d, &elf, &parms) )
+    if ( !check_and_adjust_load_address(d, elf, &parms) )
         return -ENOSPC;
 
-    elf_set_vcpu(&elf, v);
-    rc = elf_load_binary(&elf);
+    *entry = parms.phys_entry;
+
+    return 0;
+}
+
+static int __init pvh_load_kernel(
+    const struct boot_domain *bd,
+    struct elf_binary *elf,
+    paddr_t *start_info_addr)
+{
+    struct domain *d = bd->d;
+    struct boot_module *initrd = bd->initrd;
+    unsigned long initrd_len =
+        initrd ? initrd->size - initrd->arch.headroom : 0;
+    size_t cmdline_len = bd->cmdline ? strlen(bd->cmdline) + 1 : 0;
+    const char *initrd_cmdline = NULL;
+    size_t extra_space;
+    paddr_t last_addr;
+    struct hvm_start_info start_info = { 0 };
+    struct hvm_modlist_entry mod = { 0 };
+    struct vcpu *v = d->vcpu[0];
+    int rc;
+
+    /* Copy the OS image and free temporary buffer. */
+    elf_set_vcpu(elf, v);
+    rc = elf_load_binary(elf);
     if ( rc < 0 )
     {
         printk("Failed to load kernel: %d\n", rc);
-        if ( elf_check_broken(&elf) )
-            printk("%pd kernel: broken ELF: %s\n", d, elf_check_broken(&elf));
+        if ( elf_check_broken(elf) )
+            printk("%pd kernel: broken ELF: %s\n", d, elf_check_broken(elf));
         return rc;
     }
 
@@ -724,7 +738,7 @@ static int __init pvh_load_kernel(
 
     if ( initrd )
     {
-        size_t initrd_space = elf_round_up(&elf, initrd_len);
+        size_t initrd_space = elf_round_up(elf, initrd_len);
 
         if ( initrd->arch.cmdline_pa )
         {
@@ -741,9 +755,9 @@ static int __init pvh_load_kernel(
             initrd = NULL;
     }
 
-    extra_space += elf_round_up(&elf, cmdline_len);
+    extra_space += elf_round_up(elf, cmdline_len);
 
-    last_addr = find_memory(d, &elf, extra_space);
+    last_addr = find_memory(d, elf, extra_space);
     if ( last_addr == INVALID_PADDR )
     {
         printk("Unable to find a memory region to load initrd and metadata\n");
@@ -763,7 +777,7 @@ static int __init pvh_load_kernel(
 
         mod.paddr = last_addr;
         mod.size = initrd_len;
-        last_addr += elf_round_up(&elf, initrd_len);
+        last_addr += elf_round_up(elf, initrd_len);
         if ( initrd_cmdline )
         {
             size_t len = strlen(initrd_cmdline) + 1;
@@ -796,7 +810,7 @@ static int __init pvh_load_kernel(
      * Round up to 32/64 bits (depending on the guest kernel bitness) so
      * the modlist/start_info is aligned.
      */
-    last_addr += elf_round_up(&elf, cmdline_len);
+    last_addr += elf_round_up(elf, cmdline_len);
 
     if ( initrd != NULL )
     {
@@ -820,7 +834,6 @@ static int __init pvh_load_kernel(
         return rc;
     }
 
-    *entry = parms.phys_entry;
     *start_info_addr = last_addr;
 
     return 0;
@@ -1338,6 +1351,7 @@ int __init dom0_construct_pvh(const stru
 {
     paddr_t entry, start_info;
     struct domain *d = bd->d;
+    struct elf_binary elf;
     int rc;
 
     printk(XENLOG_INFO "*** Building a PVH Dom%d ***\n", d->domain_id);
@@ -1375,6 +1389,13 @@ int __init dom0_construct_pvh(const stru
 
     iommu_hwdom_init(d);
 
+    rc = pvh_place_kernel(bd, &elf, &entry);
+    if ( rc )
+    {
+        printk("Failed to place Dom0 kernel\n");
+        return rc;
+    }
+
     rc = pvh_populate_p2m(d);
     if ( rc )
     {
@@ -1382,7 +1403,7 @@ int __init dom0_construct_pvh(const stru
         return rc;
     }
 
-    rc = pvh_load_kernel(bd, &entry, &start_info);
+    rc = pvh_load_kernel(bd, &elf, &start_info);
     if ( rc )
     {
         printk("Failed to load Dom0 kernel\n");

Re: [PATCH 05/15] x86/PVH: split pvh_load_kernel()

[Teddy Astie]

[-- Attachment #1.1: Type: text/plain, Size: 619 bytes --]

Le 12/10/2026 à 10:23, Jan Beulich a écrit :
> All E820 manipulations want to happen before the initrd is placed. As a
> first step, split placing the kernel from actually loading it. Then
> placement of other data (ACPI, VMX) can be inserted between kernel
> placement and P2M population (followed by the loading of the kernel).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> If need be, moving the "elf" local variable to dom0_construct_pvh() could
> of course further be split off. Just that this by itself doesn't make much
> sense.
> 
Reviewed-by: Teddy Astie <teddy.astie@vates.tech>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]

[PATCH 06/15] x86/PVH: keep a copy of Dom0's initial E820 map

[Jan Beulich]

pvh_populate_p2m() needs to populate RAM according to the initial map
established, prior to any stealing by pvh_steal_ram(). Subsequently the
invocations of those functions will (need to) move ahead of
pvh_populate_p2m(), so what to act on is no longer Dom0's memory map at
the time of invocation.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -13,6 +13,7 @@
 #include <xen/multiboot.h>
 #include <xen/pci.h>
 #include <xen/softirq.h>
+#include <xen/xvmalloc.h>
 
 #include <acpi/actables.h>
 
@@ -413,7 +414,10 @@ static void __init pvh_init_p2m(struct d
     } while ( preempted );
 }
 
-static int __init pvh_populate_p2m(struct domain *d)
+static int __init pvh_populate_p2m(
+    struct domain *d,
+    const struct e820entry *e820,
+    unsigned int nr_e820)
 {
     struct vcpu *v = d->vcpu[0];
     unsigned int i;
@@ -421,15 +425,15 @@ static int __init pvh_populate_p2m(struc
 #define MB1_PAGES PFN_DOWN(MB(1))
 
     /* Populate memory map. */
-    for ( i = 0; i < d->arch.nr_e820; i++ )
+    for ( i = 0; i < nr_e820; i++ )
     {
         unsigned long addr, size;
 
-        if ( d->arch.e820[i].type != E820_RAM )
+        if ( e820[i].type != E820_RAM )
             continue;
 
-        addr = PFN_DOWN(d->arch.e820[i].addr);
-        size = PFN_DOWN(d->arch.e820[i].size);
+        addr = PFN_DOWN(e820[i].addr);
+        size = PFN_DOWN(e820[i].size);
 
         rc = pvh_populate_memory_range(d, addr, size);
         if ( rc )
@@ -438,11 +442,11 @@ static int __init pvh_populate_p2m(struc
         if ( addr < MB1_PAGES )
         {
             uint64_t end = min_t(uint64_t, MB(1),
-                                 d->arch.e820[i].addr + d->arch.e820[i].size);
+                                 e820[i].addr + e820[i].size);
             enum hvm_translation_result res =
                  hvm_copy_to_guest_phys(mfn_to_maddr(_mfn(addr)),
                                         mfn_to_virt(addr),
-                                        end - d->arch.e820[i].addr,
+                                        end - e820[i].addr,
                                         v);
 
             if ( res != HVMTRANS_okay )
@@ -1352,6 +1356,8 @@ int __init dom0_construct_pvh(const stru
     paddr_t entry, start_info;
     struct domain *d = bd->d;
     struct elf_binary elf;
+    struct e820entry *orig_e820;
+    unsigned int nr_orig_e820;
     int rc;
 
     printk(XENLOG_INFO "*** Building a PVH Dom%d ***\n", d->domain_id);
@@ -1389,6 +1395,15 @@ int __init dom0_construct_pvh(const stru
 
     iommu_hwdom_init(d);
 
+    nr_orig_e820 = d->arch.nr_e820;
+    orig_e820 = xvmalloc_array(struct e820entry, nr_orig_e820);
+    if ( !orig_e820 )
+    {
+        printk("No memory for copy of E820 map (%u entries)\n", nr_orig_e820);
+        return -ENOMEM;
+    }
+    memcpy(orig_e820, d->arch.e820, nr_orig_e820 * sizeof(*orig_e820));
+
     rc = pvh_place_kernel(bd, &elf, &entry);
     if ( rc )
     {
@@ -1396,13 +1411,15 @@ int __init dom0_construct_pvh(const stru
         return rc;
     }
 
-    rc = pvh_populate_p2m(d);
+    rc = pvh_populate_p2m(d, orig_e820, nr_orig_e820);
     if ( rc )
     {
         printk("Failed to setup Dom0 physical memory map\n");
         return rc;
     }
 
+    xvfree(orig_e820);
+
     rc = pvh_load_kernel(bd, &elf, &start_info);
     if ( rc )
     {

Re: [PATCH 06/15] x86/PVH: keep a copy of Dom0's initial E820 map

[Teddy Astie]

[-- Attachment #1.1: Type: text/plain, Size: 3958 bytes --]

Le 12/10/2026 à 10:23, Jan Beulich a écrit :
> pvh_populate_p2m() needs to populate RAM according to the initial map
> established, prior to any stealing by pvh_steal_ram(). Subsequently the
> invocations of those functions will (need to) move ahead of
> pvh_populate_p2m(), so what to act on is no longer Dom0's memory map at
> the time of invocation.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/arch/x86/hvm/dom0_build.c
> +++ b/xen/arch/x86/hvm/dom0_build.c
> @@ -13,6 +13,7 @@
>   #include <xen/multiboot.h>
>   #include <xen/pci.h>
>   #include <xen/softirq.h>
> +#include <xen/xvmalloc.h>
>   
>   #include <acpi/actables.h>
>   
> @@ -413,7 +414,10 @@ static void __init pvh_init_p2m(struct d
>       } while ( preempted );
>   }
>   
> -static int __init pvh_populate_p2m(struct domain *d)
> +static int __init pvh_populate_p2m(
> +    struct domain *d,
> +    const struct e820entry *e820,
> +    unsigned int nr_e820)
>   {
>       struct vcpu *v = d->vcpu[0];
>       unsigned int i;
> @@ -421,15 +425,15 @@ static int __init pvh_populate_p2m(struc
>   #define MB1_PAGES PFN_DOWN(MB(1))
>   
>       /* Populate memory map. */
> -    for ( i = 0; i < d->arch.nr_e820; i++ )
> +    for ( i = 0; i < nr_e820; i++ )
>       {
>           unsigned long addr, size;
>   
> -        if ( d->arch.e820[i].type != E820_RAM )
> +        if ( e820[i].type != E820_RAM )
>               continue;
>   
> -        addr = PFN_DOWN(d->arch.e820[i].addr);
> -        size = PFN_DOWN(d->arch.e820[i].size);
> +        addr = PFN_DOWN(e820[i].addr);
> +        size = PFN_DOWN(e820[i].size);
>   
>           rc = pvh_populate_memory_range(d, addr, size);
>           if ( rc )
> @@ -438,11 +442,11 @@ static int __init pvh_populate_p2m(struc
>           if ( addr < MB1_PAGES )
>           {
>               uint64_t end = min_t(uint64_t, MB(1),
> -                                 d->arch.e820[i].addr + d->arch.e820[i].size);
> +                                 e820[i].addr + e820[i].size);
>               enum hvm_translation_result res =
>                    hvm_copy_to_guest_phys(mfn_to_maddr(_mfn(addr)),
>                                           mfn_to_virt(addr),
> -                                        end - d->arch.e820[i].addr,
> +                                        end - e820[i].addr,
>                                           v);
>   
>               if ( res != HVMTRANS_okay )
> @@ -1352,6 +1356,8 @@ int __init dom0_construct_pvh(const stru
>       paddr_t entry, start_info;
>       struct domain *d = bd->d;
>       struct elf_binary elf;
> +    struct e820entry *orig_e820;
> +    unsigned int nr_orig_e820;
>       int rc;
>   
>       printk(XENLOG_INFO "*** Building a PVH Dom%d ***\n", d->domain_id);
> @@ -1389,6 +1395,15 @@ int __init dom0_construct_pvh(const stru
>   
>       iommu_hwdom_init(d);
>   
> +    nr_orig_e820 = d->arch.nr_e820;
> +    orig_e820 = xvmalloc_array(struct e820entry, nr_orig_e820);
> +    if ( !orig_e820 )
> +    {
> +        printk("No memory for copy of E820 map (%u entries)\n", nr_orig_e820);
> +        return -ENOMEM;
> +    }
> +    memcpy(orig_e820, d->arch.e820, nr_orig_e820 * sizeof(*orig_e820));
> +

I don't know if that matters much, but if anything after 
(pvh_place_kernel, pvh_place_acpi, ...) fails, we leak orig_e820.

>       rc = pvh_place_kernel(bd, &elf, &entry);
>       if ( rc )
>       {
> @@ -1396,13 +1411,15 @@ int __init dom0_construct_pvh(const stru
>           return rc;
>       }
>   
> -    rc = pvh_populate_p2m(d);
> +    rc = pvh_populate_p2m(d, orig_e820, nr_orig_e820);
>       if ( rc )
>       {
>           printk("Failed to setup Dom0 physical memory map\n");
>           return rc;
>       }
>   
> +    xvfree(orig_e820);
> +
>       rc = pvh_load_kernel(bd, &elf, &start_info);
>       if ( rc )
>       {


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]

Re: [PATCH 06/15] x86/PVH: keep a copy of Dom0's initial E820 map

[Jan Beulich]

On 13.10.2026 08:57, Teddy Astie wrote:
> Le 12/10/2026 à 10:23, Jan Beulich a écrit :
>> @@ -1389,6 +1395,15 @@ int __init dom0_construct_pvh(const stru
>>   
>>       iommu_hwdom_init(d);
>>   
>> +    nr_orig_e820 = d->arch.nr_e820;
>> +    orig_e820 = xvmalloc_array(struct e820entry, nr_orig_e820);
>> +    if ( !orig_e820 )
>> +    {
>> +        printk("No memory for copy of E820 map (%u entries)\n", nr_orig_e820);
>> +        return -ENOMEM;
>> +    }
>> +    memcpy(orig_e820, d->arch.e820, nr_orig_e820 * sizeof(*orig_e820));
>> +
> 
> I don't know if that matters much, but if anything after 
> (pvh_place_kernel, pvh_place_acpi, ...) fails, we leak orig_e820.

Yes - we're about to panic() anyway.

Jan

Re: [PATCH 06/15] x86/PVH: keep a copy of Dom0's initial E820 map

[Roger Pau Monné]

On Mon, Oct 12, 2026 at 10:21:55AM +0200, Jan Beulich wrote:
> pvh_populate_p2m() needs to populate RAM according to the initial map
> established, prior to any stealing by pvh_steal_ram(). Subsequently the
> invocations of those functions will (need to) move ahead of
> pvh_populate_p2m(), so what to act on is no longer Dom0's memory map at
> the time of invocation.

But (most of?) the uses of pvh_steal_ram() expect to access the stolen
RAM, and hence using pvh_steal_ram() ahead of pvh_populate_p2m() is
not a present use-case?

I assume you have a further use-case for pvh_steal_ram() that wants to
steal a range before it's populated, which might be worth mentioning
here, to add a bit more context?

> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/arch/x86/hvm/dom0_build.c
> +++ b/xen/arch/x86/hvm/dom0_build.c
> @@ -13,6 +13,7 @@
>  #include <xen/multiboot.h>
>  #include <xen/pci.h>
>  #include <xen/softirq.h>
> +#include <xen/xvmalloc.h>
>  
>  #include <acpi/actables.h>
>  
> @@ -413,7 +414,10 @@ static void __init pvh_init_p2m(struct d
>      } while ( preempted );
>  }
>  
> -static int __init pvh_populate_p2m(struct domain *d)
> +static int __init pvh_populate_p2m(
> +    struct domain *d,
> +    const struct e820entry *e820,
> +    unsigned int nr_e820)
>  {
>      struct vcpu *v = d->vcpu[0];
>      unsigned int i;
> @@ -421,15 +425,15 @@ static int __init pvh_populate_p2m(struc
>  #define MB1_PAGES PFN_DOWN(MB(1))
>  
>      /* Populate memory map. */
> -    for ( i = 0; i < d->arch.nr_e820; i++ )
> +    for ( i = 0; i < nr_e820; i++ )
>      {
>          unsigned long addr, size;
>  
> -        if ( d->arch.e820[i].type != E820_RAM )
> +        if ( e820[i].type != E820_RAM )
>              continue;
>  
> -        addr = PFN_DOWN(d->arch.e820[i].addr);
> -        size = PFN_DOWN(d->arch.e820[i].size);
> +        addr = PFN_DOWN(e820[i].addr);
> +        size = PFN_DOWN(e820[i].size);
>  
>          rc = pvh_populate_memory_range(d, addr, size);
>          if ( rc )
> @@ -438,11 +442,11 @@ static int __init pvh_populate_p2m(struc
>          if ( addr < MB1_PAGES )
>          {
>              uint64_t end = min_t(uint64_t, MB(1),
> -                                 d->arch.e820[i].addr + d->arch.e820[i].size);
> +                                 e820[i].addr + e820[i].size);
>              enum hvm_translation_result res =
>                   hvm_copy_to_guest_phys(mfn_to_maddr(_mfn(addr)),
>                                          mfn_to_virt(addr),
> -                                        end - d->arch.e820[i].addr,
> +                                        end - e820[i].addr,
>                                          v);
>  
>              if ( res != HVMTRANS_okay )
> @@ -1352,6 +1356,8 @@ int __init dom0_construct_pvh(const stru
>      paddr_t entry, start_info;
>      struct domain *d = bd->d;
>      struct elf_binary elf;
> +    struct e820entry *orig_e820;
> +    unsigned int nr_orig_e820;
>      int rc;
>  
>      printk(XENLOG_INFO "*** Building a PVH Dom%d ***\n", d->domain_id);
> @@ -1389,6 +1395,15 @@ int __init dom0_construct_pvh(const stru
>  
>      iommu_hwdom_init(d);
>  
> +    nr_orig_e820 = d->arch.nr_e820;
> +    orig_e820 = xvmalloc_array(struct e820entry, nr_orig_e820);
> +    if ( !orig_e820 )
> +    {
> +        printk("No memory for copy of E820 map (%u entries)\n", nr_orig_e820);

For some of the printks during dom0 PVH construction we already print
the domain (%pd), and I think it would be good to print it here also.
Some not-yet-upstream uses (hyperlaunch) create several domains using
this logic, it would be good to easily spot which domain triggered the
errors.

Thanks, Roger.

Re: [PATCH 06/15] x86/PVH: keep a copy of Dom0's initial E820 map

[Jan Beulich]

On 13.10.2026 15:48, Roger Pau Monné wrote:
> On Mon, Oct 12, 2026 at 10:21:55AM +0200, Jan Beulich wrote:
>> pvh_populate_p2m() needs to populate RAM according to the initial map
>> established, prior to any stealing by pvh_steal_ram(). Subsequently the
>> invocations of those functions will (need to) move ahead of
>> pvh_populate_p2m(), so what to act on is no longer Dom0's memory map at
>> the time of invocation.
> 
> But (most of?) the uses of pvh_steal_ram() expect to access the stolen
> RAM, and hence using pvh_steal_ram() ahead of pvh_populate_p2m() is
> not a present use-case?
> 
> I assume you have a further use-case for pvh_steal_ram() that wants to
> steal a range before it's populated, which might be worth mentioning
> here, to add a bit more context?

I thought that I'm saying just that already with "Subsequently ...". All
that logic needs splitting: Obtain a range ahead of populating, access
the range only afterwards.

>> @@ -1389,6 +1395,15 @@ int __init dom0_construct_pvh(const stru
>>  
>>      iommu_hwdom_init(d);
>>  
>> +    nr_orig_e820 = d->arch.nr_e820;
>> +    orig_e820 = xvmalloc_array(struct e820entry, nr_orig_e820);
>> +    if ( !orig_e820 )
>> +    {
>> +        printk("No memory for copy of E820 map (%u entries)\n", nr_orig_e820);
> 
> For some of the printks during dom0 PVH construction we already print
> the domain (%pd), and I think it would be good to print it here also.
> Some not-yet-upstream uses (hyperlaunch) create several domains using
> this logic, it would be good to easily spot which domain triggered the
> errors.

Can do, sure. That'll be a little inconsistent then for a while; maybe
I should add a patch adjusting all printk()s I didn't otherwise touch.

Jan

Re: [PATCH 06/15] x86/PVH: keep a copy of Dom0's initial E820 map

[Roger Pau Monné]

On Tue, Oct 13, 2026 at 04:53:11PM +0200, Jan Beulich wrote:
> On 13.10.2026 15:48, Roger Pau Monné wrote:
> > On Mon, Oct 12, 2026 at 10:21:55AM +0200, Jan Beulich wrote:
> >> pvh_populate_p2m() needs to populate RAM according to the initial map
> >> established, prior to any stealing by pvh_steal_ram(). Subsequently the
> >> invocations of those functions will (need to) move ahead of
> >> pvh_populate_p2m(), so what to act on is no longer Dom0's memory map at
> >> the time of invocation.
> > 
> > But (most of?) the uses of pvh_steal_ram() expect to access the stolen
> > RAM, and hence using pvh_steal_ram() ahead of pvh_populate_p2m() is
> > not a present use-case?
> > 
> > I assume you have a further use-case for pvh_steal_ram() that wants to
> > steal a range before it's populated, which might be worth mentioning
> > here, to add a bit more context?
> 
> I thought that I'm saying just that already with "Subsequently ...". All
> that logic needs splitting: Obtain a range ahead of populating, access
> the range only afterwards.

OK, it wasn't obviously clear from the message.  I would possibly
explicitly mention that it's just the allocation that's moved ahead,
the access must be kept after pvh_populate_p2m().

> >> @@ -1389,6 +1395,15 @@ int __init dom0_construct_pvh(const stru
> >>  
> >>      iommu_hwdom_init(d);
> >>  
> >> +    nr_orig_e820 = d->arch.nr_e820;
> >> +    orig_e820 = xvmalloc_array(struct e820entry, nr_orig_e820);
> >> +    if ( !orig_e820 )
> >> +    {
> >> +        printk("No memory for copy of E820 map (%u entries)\n", nr_orig_e820);
> > 
> > For some of the printks during dom0 PVH construction we already print
> > the domain (%pd), and I think it would be good to print it here also.
> > Some not-yet-upstream uses (hyperlaunch) create several domains using
> > this logic, it would be good to easily spot which domain triggered the
> > errors.
> 
> Can do, sure. That'll be a little inconsistent then for a while; maybe
> I should add a patch adjusting all printk()s I didn't otherwise touch.

It's kind of inconsistent already, see for example the messages at the
top of dom0_construct_pvh().  I won't request that you fix the
existing ones, but it would be better if new additions print the
domain as part of the message.

Thanks, Roger.

[PATCH 07/15] x86/PVH: lift E820 alignment requirement

[Jan Beulich]

Before reordering things (to move in particular pvh_steal_ram()
invocations ahead of the pvh_populate_p2m() one) drop an undue (and as it
turns out unnecessary) assumption: find_memory() doesn't really depend on
E820 regions to end on page boundaries.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Really the assumption towards the start of regions doesn't look to be
relevant either, yet I thought I'd limit the change to the minimum needed.
(In fact the statement immediately ahead of the assertion arranges for
page alignment, so the check is dubious altogether.)

We also could consider whether kernel_end really needs rounding up to a
page boundary: Technically there's no need, but the question is whether it
would be a good idea to have the tail of the kernel region share a page
with metadata (or, as things still are at this point of the series, the
initrd).

--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -365,6 +365,9 @@ static __init void pvh_setup_e820(struct
          * that's the minimum granularity of the 2nd stage translation. Since
          * the p2m code uses PAGE_ORDER_4K internally, also use it here in
          * order to prevent this code from getting out of sync.
+         *
+         * Note that this is only for pvh_populate_p2m().  Subsequent uses of
+         * pvh_steal_ram() will break the length part.
          */
         start = ROUNDUP(entry->addr, PAGE_SIZE << PAGE_ORDER_4K);
         end = (entry->addr + entry->size) &
@@ -506,8 +509,8 @@ static paddr_t __init find_memory(
     unsigned int i;
 
     /*
-     * The memory map is sorted and all RAM regions starts and sizes are
-     * aligned to page boundaries.
+     * The memory map is sorted and all RAM region starts are aligned to
+     * page boundaries.
      */
     for ( i = 0; i < d->arch.nr_e820; i++ )
     {
@@ -519,7 +522,7 @@ static paddr_t __init find_memory(
 
         start = MAX(ROUNDUP(d->arch.e820[i].addr, PAGE_SIZE), MB(1));
 
-        ASSERT(IS_ALIGNED(start, PAGE_SIZE) && IS_ALIGNED(end, PAGE_SIZE));
+        ASSERT(IS_ALIGNED(start, PAGE_SIZE));
 
         /*
          * NB: Even better would be to use rangesets to determine a suitable

Re: [PATCH 07/15] x86/PVH: lift E820 alignment requirement

[Roger Pau Monné]

On Mon, Oct 12, 2026 at 10:22:28AM +0200, Jan Beulich wrote:
> Before reordering things (to move in particular pvh_steal_ram()
> invocations ahead of the pvh_populate_p2m() one) drop an undue (and as it
> turns out unnecessary) assumption: find_memory() doesn't really depend on
> E820 regions to end on page boundaries.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Really the assumption towards the start of regions doesn't look to be
> relevant either, yet I thought I'd limit the change to the minimum needed.
> (In fact the statement immediately ahead of the assertion arranges for
> page alignment, so the check is dubious altogether.)
> 
> We also could consider whether kernel_end really needs rounding up to a
> page boundary: Technically there's no need, but the question is whether it
> would be a good idea to have the tail of the kernel region share a page
> with metadata (or, as things still are at this point of the series, the
> initrd).

I think it would a bad idea to have this kind of page-sharing.  The
guest might use NX for metadata page-tables (ie: initrd), so it
sharing a page with the kernel could be problematic.

> 
> --- a/xen/arch/x86/hvm/dom0_build.c
> +++ b/xen/arch/x86/hvm/dom0_build.c
> @@ -365,6 +365,9 @@ static __init void pvh_setup_e820(struct
>           * that's the minimum granularity of the 2nd stage translation. Since
>           * the p2m code uses PAGE_ORDER_4K internally, also use it here in
>           * order to prevent this code from getting out of sync.
> +         *
> +         * Note that this is only for pvh_populate_p2m().  Subsequent uses of
> +         * pvh_steal_ram() will break the length part.

Maybe because I'm too familiar with this code, but the previous
comment mentioning the 4K alignment being required for the 2nd stage
translation kind of implies this is the reason for it?  I guess you
want to make it clear there is no further logic depending on that
alignment.

>           */
>          start = ROUNDUP(entry->addr, PAGE_SIZE << PAGE_ORDER_4K);
>          end = (entry->addr + entry->size) &
> @@ -506,8 +509,8 @@ static paddr_t __init find_memory(
>      unsigned int i;
>  
>      /*
> -     * The memory map is sorted and all RAM regions starts and sizes are
> -     * aligned to page boundaries.
> +     * The memory map is sorted and all RAM region starts are aligned to
> +     * page boundaries.
>       */
>      for ( i = 0; i < d->arch.nr_e820; i++ )
>      {
> @@ -519,7 +522,7 @@ static paddr_t __init find_memory(
>  
>          start = MAX(ROUNDUP(d->arch.e820[i].addr, PAGE_SIZE), MB(1));
>  
> -        ASSERT(IS_ALIGNED(start, PAGE_SIZE) && IS_ALIGNED(end, PAGE_SIZE));
> +        ASSERT(IS_ALIGNED(start, PAGE_SIZE));

I would remove this altogether - as you noticed start is clearly
aligned by the code in the line just above.

Thanks, Roger.

Re: [PATCH 07/15] x86/PVH: lift E820 alignment requirement

[Jan Beulich]

On 13.10.2026 16:59, Roger Pau Monné wrote:
> On Mon, Oct 12, 2026 at 10:22:28AM +0200, Jan Beulich wrote:
>> Before reordering things (to move in particular pvh_steal_ram()
>> invocations ahead of the pvh_populate_p2m() one) drop an undue (and as it
>> turns out unnecessary) assumption: find_memory() doesn't really depend on
>> E820 regions to end on page boundaries.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> Really the assumption towards the start of regions doesn't look to be
>> relevant either, yet I thought I'd limit the change to the minimum needed.
>> (In fact the statement immediately ahead of the assertion arranges for
>> page alignment, so the check is dubious altogether.)
>>
>> We also could consider whether kernel_end really needs rounding up to a
>> page boundary: Technically there's no need, but the question is whether it
>> would be a good idea to have the tail of the kernel region share a page
>> with metadata (or, as things still are at this point of the series, the
>> initrd).
> 
> I think it would a bad idea to have this kind of page-sharing.  The
> guest might use NX for metadata page-tables (ie: initrd), so it
> sharing a page with the kernel could be problematic.

Okay, will not pursue this then.

>> --- a/xen/arch/x86/hvm/dom0_build.c
>> +++ b/xen/arch/x86/hvm/dom0_build.c
>> @@ -365,6 +365,9 @@ static __init void pvh_setup_e820(struct
>>           * that's the minimum granularity of the 2nd stage translation. Since
>>           * the p2m code uses PAGE_ORDER_4K internally, also use it here in
>>           * order to prevent this code from getting out of sync.
>> +         *
>> +         * Note that this is only for pvh_populate_p2m().  Subsequent uses of
>> +         * pvh_steal_ram() will break the length part.
> 
> Maybe because I'm too familiar with this code, but the previous
> comment mentioning the 4K alignment being required for the 2nd stage
> translation kind of implies this is the reason for it?  I guess you
> want to make it clear there is no further logic depending on that
> alignment.

To me it seemed as if the other comments / assumptions were (at least in
part) building on what's said here. I.e. yes, I meant to verbally break
any such connections people may have memorized. If you think I'm going
too far with this comment addition - getting rid of it again is easy.

>> @@ -506,8 +509,8 @@ static paddr_t __init find_memory(
>>      unsigned int i;
>>  
>>      /*
>> -     * The memory map is sorted and all RAM regions starts and sizes are
>> -     * aligned to page boundaries.
>> +     * The memory map is sorted and all RAM region starts are aligned to
>> +     * page boundaries.
>>       */
>>      for ( i = 0; i < d->arch.nr_e820; i++ )
>>      {
>> @@ -519,7 +522,7 @@ static paddr_t __init find_memory(
>>  
>>          start = MAX(ROUNDUP(d->arch.e820[i].addr, PAGE_SIZE), MB(1));
>>  
>> -        ASSERT(IS_ALIGNED(start, PAGE_SIZE) && IS_ALIGNED(end, PAGE_SIZE));
>> +        ASSERT(IS_ALIGNED(start, PAGE_SIZE));
> 
> I would remove this altogether - as you noticed start is clearly
> aligned by the code in the line just above.

Will do. I kept this part because it wasn't really the first time round
that I had noticed the check being pointless. I.e. there still was that
chance that I might have been missing something.

Alternatively I could drop the ROUNDUP() from the assignment of start.
Else I think the comment ahead of the loop may want altering further.

Jan

[PATCH 08/15] x86/PVH: split pvh_setup_acpi()

[Jan Beulich]

All E820 manipulations want to happen before the initrd is placed, or else
pvh_steal_ram() may steal from the tail part of it. As a second step,
split placing ACPI data from actually populating it. To reuse size
calculations and limit churn, convert pvh_setup_acpi_{madt,xsdt}() to
two-pass functions: The 1st pass merely reserves space, while the 2nd pass
actually populates information.

While altering the copying of the RSDP pointer into Dom0's start-info, add
a BUILD_BUG_ON() making sure field sizes actually match up.

Fixes: 1d74282c455f ("x86: setup PVHv2 Dom0 ACPI tables")
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -920,8 +920,9 @@ static int __init pvh_setup_acpi_madt(st
     int rc;
 
     /* Count number of interrupt overrides in the MADT. */
-    acpi_table_parse_madt(ACPI_MADT_TYPE_INTERRUPT_OVERRIDE,
-                          acpi_count_intr_ovr, UINT_MAX);
+    if ( !*addr )
+        acpi_table_parse_madt(ACPI_MADT_TYPE_INTERRUPT_OVERRIDE,
+                              acpi_count_intr_ovr, UINT_MAX);
 
     /* Calculate the size of the crafted MADT. */
     size = sizeof(*madt);
@@ -929,6 +930,22 @@ static int __init pvh_setup_acpi_madt(st
     size += sizeof(*intsrcovr) * acpi_intr_overrides;
     size += sizeof(*x2apic) * d->max_vcpus;
 
+    if ( !*addr )
+    {
+        /* Reserve space to place the new MADT in guest memory space. */
+        if ( pvh_steal_ram(d, size, 0, GB(4), addr) )
+        {
+            printk("Unable to steal guest RAM for MADT\n");
+            return -ENOMEM;
+        }
+
+        /* Mark this region as E820_ACPI. */
+        if ( pvh_add_mem_range(d, *addr, *addr + size, E820_ACPI) )
+            printk("Unable to add MADT region to memory map\n");
+
+        return 0;
+    }
+
     madt = xzalloc_bytes(size);
     if ( !madt )
     {
@@ -991,17 +1008,6 @@ static int __init pvh_setup_acpi_madt(st
     madt->header.checksum -= acpi_tb_checksum(ACPI_CAST_PTR(u8, madt), size);
 
     /* Place the new MADT in guest memory space. */
-    if ( pvh_steal_ram(d, size, 0, GB(4), addr) )
-    {
-        printk("Unable to steal guest RAM for MADT\n");
-        rc = -ENOMEM;
-        goto out;
-    }
-
-    /* Mark this region as E820_ACPI. */
-    if ( pvh_add_mem_range(d, *addr, *addr + size, E820_ACPI) )
-        printk("Unable to add MADT region to memory map\n");
-
     rc = hvm_copy_to_guest_phys(*addr, madt, size, d->vcpu[0]);
     if ( rc )
     {
@@ -1116,6 +1122,22 @@ static int __init pvh_setup_acpi_xsdt(st
      */
     size += num_tables * sizeof(xsdt->table_offset_entry[0]);
 
+    if ( !*addr )
+    {
+        /* Reserve space to place the new XSDT in guest memory space. */
+        if ( pvh_steal_ram(d, size, 0, GB(4), addr) )
+        {
+            printk("Unable to find guest RAM for XSDT\n");
+            return -ENOMEM;
+        }
+
+        /* Mark this region as E820_ACPI. */
+        if ( pvh_add_mem_range(d, *addr, *addr + size, E820_ACPI) )
+            printk("Unable to add XSDT region to memory map\n");
+
+        return 0;
+    }
+
     xsdt = xzalloc_bytes(size);
     if ( !xsdt )
     {
@@ -1179,17 +1201,6 @@ static int __init pvh_setup_acpi_xsdt(st
     xsdt->header.checksum -= acpi_tb_checksum(ACPI_CAST_PTR(u8, xsdt), size);
 
     /* Place the new XSDT in guest memory space. */
-    if ( pvh_steal_ram(d, size, 0, GB(4), addr) )
-    {
-        printk("Unable to find guest RAM for XSDT\n");
-        rc = -ENOMEM;
-        goto out;
-    }
-
-    /* Mark this region as E820_ACPI. */
-    if ( pvh_add_mem_range(d, *addr, *addr + size, E820_ACPI) )
-        printk("Unable to add XSDT region to memory map\n");
-
     rc = hvm_copy_to_guest_phys(*addr, xsdt, size, d->vcpu[0]);
     if ( rc )
     {
@@ -1205,18 +1216,18 @@ static int __init pvh_setup_acpi_xsdt(st
     return rc;
 }
 
-static int __init pvh_setup_acpi(struct domain *d, paddr_t start_info)
+struct acpi_info {
+    paddr_t madt;
+    paddr_t xsdt;
+    paddr_t rsdp;
+};
+
+static int __init pvh_place_acpi(
+    struct domain *d,
+    struct acpi_info *info)
 {
-    unsigned long pfn, nr_pages;
-    paddr_t madt_paddr, xsdt_paddr, rsdp_paddr;
     unsigned int i;
     int rc;
-    struct acpi_table_rsdp *native_rsdp, rsdp = {
-        .signature = ACPI_SIG_RSDP,
-        .revision = 2,
-        .length = sizeof(rsdp),
-    };
-
 
     /* Scan top-level tables and add their regions to the guest memory map. */
     for( i = 0; i < acpi_gbl_root_table_list.count; i++ )
@@ -1239,6 +1250,8 @@ static int __init pvh_setup_acpi(struct
     /* Identity map ACPI e820 regions. */
     for ( i = 0; i < d->arch.nr_e820; i++ )
     {
+        unsigned long pfn, nr_pages;
+
         if ( d->arch.e820[i].type != E820_ACPI &&
              d->arch.e820[i].type != E820_NVS )
             continue;
@@ -1267,32 +1280,49 @@ static int __init pvh_setup_acpi(struct
         }
     }
 
-    rc = pvh_setup_acpi_madt(d, &madt_paddr);
+    rc = pvh_setup_acpi_madt(d, &info->madt);
     if ( rc )
         return rc;
 
-    rc = pvh_setup_acpi_xsdt(d, madt_paddr, &xsdt_paddr);
+    rc = pvh_setup_acpi_xsdt(d, info->madt, &info->xsdt);
     if ( rc )
         return rc;
 
-    /* Craft a custom RSDP. */
-    native_rsdp = acpi_os_map_memory(acpi_os_get_root_pointer(), sizeof(rsdp));
-    if ( !native_rsdp )
+    if ( pvh_steal_ram(d, sizeof(struct acpi_table_rsdp), 0, GB(4),
+                       &info->rsdp) )
     {
-        printk("Failed to map native RSDP\n");
+        printk("Unable to allocate guest RAM for RSDP\n");
         return -ENOMEM;
     }
-    memcpy(rsdp.oem_id, native_rsdp->oem_id, sizeof(rsdp.oem_id));
-    acpi_os_unmap_memory(native_rsdp, sizeof(rsdp));
-    rsdp.xsdt_physical_address = xsdt_paddr;
-    /*
-     * Calling acpi_tb_checksum here is a layering violation, but
-     * introducing a wrapper for such simple usage seems overkill.
-     */
-    rsdp.checksum -= acpi_tb_checksum(ACPI_CAST_PTR(u8, &rsdp),
-                                      ACPI_RSDP_REV0_SIZE);
-    rsdp.extended_checksum -= acpi_tb_checksum(ACPI_CAST_PTR(u8, &rsdp),
-                                               sizeof(rsdp));
+
+    /* Mark this region as E820_ACPI. */
+    if ( pvh_add_mem_range(d, info->rsdp,
+                           info->rsdp + sizeof(struct acpi_table_rsdp),
+                           E820_ACPI) )
+        printk("Unable to add RSDP region to memory map\n");
+
+    return 0;
+}
+
+static int __init pvh_setup_acpi(
+    struct domain *d,
+    struct acpi_info *info,
+    paddr_t start_info)
+{
+    int rc;
+    struct acpi_table_rsdp *native_rsdp, rsdp = {
+        .signature = ACPI_SIG_RSDP,
+        .revision = 2,
+        .length = sizeof(rsdp),
+    };
+
+    rc = pvh_setup_acpi_madt(d, &info->madt);
+    if ( rc )
+        return rc;
+
+    rc = pvh_setup_acpi_xsdt(d, info->madt, &info->xsdt);
+    if ( rc )
+        return rc;
 
     /*
      * Place the new RSDP in guest memory space.
@@ -1302,19 +1332,27 @@ static int __init pvh_setup_acpi(struct
      * the native RSDT, and should not be used for the Dom0 kernel's boot
      * purposes (we keep it visible for post boot access).
      */
-    if ( pvh_steal_ram(d, sizeof(rsdp), 0, GB(4), &rsdp_paddr) )
+    native_rsdp = acpi_os_map_memory(acpi_os_get_root_pointer(), sizeof(rsdp));
+    if ( !native_rsdp )
     {
-        printk("Unable to allocate guest RAM for RSDP\n");
+        printk("Failed to map native RSDP\n");
         return -ENOMEM;
     }
+    memcpy(rsdp.oem_id, native_rsdp->oem_id, sizeof(rsdp.oem_id));
+    acpi_os_unmap_memory(native_rsdp, sizeof(rsdp));
+    rsdp.xsdt_physical_address = info->xsdt;
 
-    /* Mark this region as E820_ACPI. */
-    if ( pvh_add_mem_range(d, rsdp_paddr, rsdp_paddr + sizeof(rsdp),
-                           E820_ACPI) )
-        printk("Unable to add RSDP region to memory map\n");
+    /*
+     * Calling acpi_tb_checksum here is a layering violation, but
+     * introducing a wrapper for such simple usage seems overkill.
+     */
+    rsdp.checksum -= acpi_tb_checksum(ACPI_CAST_PTR(u8, &rsdp),
+                                      ACPI_RSDP_REV0_SIZE);
+    rsdp.extended_checksum -= acpi_tb_checksum(ACPI_CAST_PTR(u8, &rsdp),
+                                               sizeof(rsdp));
 
     /* Copy RSDP into guest memory. */
-    rc = hvm_copy_to_guest_phys(rsdp_paddr, &rsdp, sizeof(rsdp), d->vcpu[0]);
+    rc = hvm_copy_to_guest_phys(info->rsdp, &rsdp, sizeof(rsdp), d->vcpu[0]);
     if ( rc )
     {
         printk("Unable to copy RSDP into guest memory\n");
@@ -1322,9 +1360,11 @@ static int __init pvh_setup_acpi(struct
     }
 
     /* Copy RSDP address to start_info. */
+    BUILD_BUG_ON(sizeof(info->rsdp) !=
+                 sizeof_field(struct hvm_start_info, rsdp_paddr));
     rc = hvm_copy_to_guest_phys(start_info +
                                 offsetof(struct hvm_start_info, rsdp_paddr),
-                                &rsdp_paddr,
+                                &info->rsdp,
                                 sizeof_field(struct hvm_start_info, rsdp_paddr),
                                 d->vcpu[0]);
     if ( rc )
@@ -1361,6 +1401,7 @@ int __init dom0_construct_pvh(const stru
     struct elf_binary elf;
     struct e820entry *orig_e820;
     unsigned int nr_orig_e820;
+    struct acpi_info acpi = {};
     int rc;
 
     printk(XENLOG_INFO "*** Building a PVH Dom%d ***\n", d->domain_id);
@@ -1414,6 +1455,13 @@ int __init dom0_construct_pvh(const stru
         return rc;
     }
 
+    rc = pvh_place_acpi(d, &acpi);
+    if ( rc )
+    {
+        printk("Failed to place Dom0 ACPI tables: %d\n", rc);
+        return rc;
+    }
+
     rc = pvh_populate_p2m(d, orig_e820, nr_orig_e820);
     if ( rc )
     {
@@ -1437,7 +1485,7 @@ int __init dom0_construct_pvh(const stru
         return rc;
     }
 
-    rc = pvh_setup_acpi(d, start_info);
+    rc = pvh_setup_acpi(d, &acpi, start_info);
     if ( rc )
     {
         printk("Failed to setup Dom0 ACPI tables: %d\n", rc);

Re: [PATCH 08/15] x86/PVH: split pvh_setup_acpi()

[Roger Pau Monné]

On Mon, Oct 12, 2026 at 10:23:16AM +0200, Jan Beulich wrote:
> All E820 manipulations want to happen before the initrd is placed, or else
> pvh_steal_ram() may steal from the tail part of it. As a second step,
> split placing ACPI data from actually populating it.

Would it be less intrusive to duplicate the memory map (like it's done
in a change ahead of this one), and subtract the kernel and initrd
region from that memory map, then use it as the basis for
pvh_steal_ram()?

We could adjust/replace pvh_add_mem_range() so it allows changing
regions from RAM to other types.  That would seem to be less churn
overall.  Maybe you did already consider this and found further
problem with this approach that I'm failing to see?

Thanks, Roger.

Re: [PATCH 08/15] x86/PVH: split pvh_setup_acpi()

[Jan Beulich]

On 13.10.2026 17:52, Roger Pau Monné wrote:
> On Mon, Oct 12, 2026 at 10:23:16AM +0200, Jan Beulich wrote:
>> All E820 manipulations want to happen before the initrd is placed, or else
>> pvh_steal_ram() may steal from the tail part of it. As a second step,
>> split placing ACPI data from actually populating it.
> 
> Would it be less intrusive to duplicate the memory map (like it's done
> in a change ahead of this one), and subtract the kernel and initrd
> region from that memory map, then use it as the basis for
> pvh_steal_ram()?
> 
> We could adjust/replace pvh_add_mem_range() so it allows changing
> regions from RAM to other types.  That would seem to be less churn
> overall.  Maybe you did already consider this and found further
> problem with this approach that I'm failing to see?

No, I didn't try that route. In fact despite the order now this patch
(and maybe also the VMX one) pre-dates the E820-copying one. The one
aspect I consider problematic with the suggested alternative is that
then two E820 maps need updating: The one with kernel (not sure about
initrd; see below) subtracted out, and the one we actually mean to hand
to Dom0.

As to overall churn: I'm not sure which one would cause less. Certainly
the patch here would remain largely unaltered, as the splitting is
going to be necessary no matter what.

As to initrd: That one wants to go last, as it has no constraints on
placement. If there's not enough space below 4G, we just put it above.
The same isn't true for ACPI or VMX data (albeit for the latter I
haven't checked in how far placing below 4G is strictly necessary).

Jan

[PATCH 09/15] x86/PVH: split pvh_setup_vmx_realmode_helpers()

[Jan Beulich]

All E820 manipulations want to happen before the initrd is placed, or else
pvh_steal_ram() may steal from the tail part of it. While this is
currently suitably ordered, mapping the initrd directly into Dom0 will
want to happen ahead of pvh_populate_p2m(). Hence as a third step, split
placing VMX data (if needed at all) from actually populating it.

Also move the invocation of pvh_setup_vmx_realmode_helpers() out of
pvh_populate_p2m(). Make it unconditional as well, instead bailing from
the function when pvh_place_vmx_realmode_helpers() wasn't called.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -268,26 +268,30 @@ static int __init pvh_add_mem_range(stru
     return 0;
 }
 
-static int __init pvh_setup_vmx_realmode_helpers(struct domain *d)
+struct vmx_info {
+    paddr_t tss;
+    gfn_t identpt;
+};
+
+static int __init pvh_place_vmx_realmode_helpers(
+    struct domain *d,
+    struct vmx_info *info)
 {
-    uint32_t rc, *ident_pt;
-    mfn_t mfn;
     paddr_t gaddr;
-    struct vcpu *v = d->vcpu[0];
 
     /*
-     * Steal some space from the last RAM region below 4GB and use it to
-     * store the real-mode TSS. It needs to be aligned to 128 so that the
-     * TSS structure (which accounts for the first 104b) doesn't cross
-     * a page boundary.
+     * Steal some space from the last RAM region below 4GB to store the
+     * real-mode TSS.  It needs to be aligned to 128 so that the TSS
+     * structure (which accounts for the first 104b) doesn't cross a page
+     * boundary.
      */
     if ( !pvh_steal_ram(d, HVM_VM86_TSS_SIZE, 128, GB(4), &gaddr) )
     {
-        if ( hvm_copy_to_guest_phys(gaddr, NULL, HVM_VM86_TSS_SIZE, v) !=
-             HVMTRANS_okay )
-            printk("Unable to zero VM86 TSS area\n");
         d->arch.hvm.params[HVM_PARAM_VM86_TSS_SIZED] =
             VM86_TSS_UPDATED | ((uint64_t)HVM_VM86_TSS_SIZE << 32) | gaddr;
+
+        info->tss = gaddr;
+
         if ( pvh_add_mem_range(d, gaddr, gaddr + HVM_VM86_TSS_SIZE,
                                E820_RESERVED) )
             printk("Unable to set VM86 TSS as reserved in the memory map\n");
@@ -302,12 +306,41 @@ static int __init pvh_setup_vmx_realmode
         return -ENOMEM;
     }
 
+    if ( pvh_add_mem_range(d, gaddr, gaddr + PAGE_SIZE, E820_RESERVED) )
+        printk("Unable to set identity page tables as reserved in the memory map\n");
+
+    d->arch.hvm.params[HVM_PARAM_IDENT_PT] = gaddr;
+    info->identpt = _gfn(PFN_DOWN(gaddr));
+
+    return 0;
+}
+
+static int __init pvh_setup_vmx_realmode_helpers(
+    const struct domain *d,
+    const struct vmx_info *info)
+{
+    uint32_t rc, *ident_pt;
+    mfn_t mfn;
+    struct vcpu *v = d->vcpu[0];
+
+    /* Bail if no space needed reserving in the first place. */
+    if ( gfn_eq(info->identpt, _gfn(0)) )
+        return 0;
+
+    /* Store the real-mode TSS, if space for it was successfully stolen. */
+    if ( info->tss )
+    {
+        if ( hvm_copy_to_guest_phys(info->tss, NULL, HVM_VM86_TSS_SIZE, v) !=
+             HVMTRANS_okay )
+            printk("Unable to zero VM86 TSS area\n");
+    }
+
     /*
      * Identity-map page table is required for running with CR0.PG=0
      * when using Intel EPT. Create a 32-bit non-PAE page directory of
      * superpages.
      */
-    ident_pt = map_domain_gfn(p2m_get_hostp2m(d), _gfn(PFN_DOWN(gaddr)),
+    ident_pt = map_domain_gfn(p2m_get_hostp2m(d), info->identpt,
                               &mfn, 0, &rc);
     if ( ident_pt == NULL )
     {
@@ -317,9 +350,6 @@ static int __init pvh_setup_vmx_realmode
     write_32bit_pse_identmap(ident_pt);
     unmap_domain_page(ident_pt);
     put_page(mfn_to_page(mfn));
-    d->arch.hvm.params[HVM_PARAM_IDENT_PT] = gaddr;
-    if ( pvh_add_mem_range(d, gaddr, gaddr + PAGE_SIZE, E820_RESERVED) )
-            printk("Unable to set identity page tables as reserved in the memory map\n");
 
     return 0;
 }
@@ -481,18 +511,6 @@ static int __init pvh_populate_p2m(
         }
     }
 
-    if ( using_vmx() && !paging_mode_shadow(d) && !vmx_unrestricted_guest(v) )
-    {
-        /*
-         * Since Dom0 cannot be migrated, we will only setup the
-         * unrestricted guest helpers if they are needed by the current
-         * hardware we are running on.
-         */
-        rc = pvh_setup_vmx_realmode_helpers(d);
-        if ( rc )
-            return rc;
-    }
-
     if ( opt_dom0_verbose )
         print_order_stats(d);
 
@@ -1402,6 +1420,7 @@ int __init dom0_construct_pvh(const stru
     struct e820entry *orig_e820;
     unsigned int nr_orig_e820;
     struct acpi_info acpi = {};
+    struct vmx_info vmx = {};
     int rc;
 
     printk(XENLOG_INFO "*** Building a PVH Dom%d ***\n", d->domain_id);
@@ -1462,6 +1481,22 @@ int __init dom0_construct_pvh(const stru
         return rc;
     }
 
+    /*
+     * Since Dom0 cannot be migrated, only set up the unrestricted guest
+     * helpers if they are needed by the hardware we are running on.
+     */
+    if ( using_vmx() && !paging_mode_shadow(d) &&
+         /*
+          * TODO: Unrestricted Guest should be a domain property, not a vCPU
+          * one.
+          */
+         !vmx_unrestricted_guest(d->vcpu[0]) )
+    {
+        rc = pvh_place_vmx_realmode_helpers(d, &vmx);
+        if ( rc )
+            return rc;
+    }
+
     rc = pvh_populate_p2m(d, orig_e820, nr_orig_e820);
     if ( rc )
     {
@@ -1492,6 +1527,10 @@ int __init dom0_construct_pvh(const stru
         return rc;
     }
 
+    rc = pvh_setup_vmx_realmode_helpers(d, &vmx);
+    if ( rc )
+        return rc;
+
     if ( opt_dom0_verbose )
     {
         printk("Dom%u memory map:\n", d->domain_id);

[PATCH 10/15] x86/PVH: convert find_memory() to using a rangeset

[Jan Beulich]

Subsequently we will need to exclude a 2nd range here. To avoid
complicating the logic when it needs to cover two ranges, follow the
suggestion left in a code comment and convert to using a rangeset.

Suggested-by: Xenia Ragiadakou <xenia.ragiadakou@amd.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
The Suggested-by: is based on the respective commit's S-o-b. I didn't go
hunt for whether this may have been suggested in review of that patch.

--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -518,14 +518,38 @@ static int __init pvh_populate_p2m(
 #undef MB1_PAGES
 }
 
+struct mem_info {
+    size_t size;
+    paddr_t addr;
+};
+
+static int cf_check __init _find_memory(
+    unsigned long s, unsigned long e, void *data)
+{
+    struct mem_info *info = data;
+
+    if ( e + 1 - s >= info->size )
+    {
+        info->addr = s;
+        return 1;
+    }
+
+    return 0;
+}
+
 static paddr_t __init find_memory(
     const struct domain *d, const struct elf_binary *elf, size_t size)
 {
-    paddr_t kernel_start = (paddr_t)elf->dest_base & PAGE_MASK;
-    paddr_t kernel_end = ROUNDUP((paddr_t)elf->dest_base + elf->dest_size,
-                                 PAGE_SIZE);
+    struct rangeset *ram = rangeset_new(NULL, NULL, 0);
+    struct mem_info info = {
+        .size = size,
+        .addr = INVALID_PADDR,
+    };
     unsigned int i;
 
+    if ( !ram )
+        return INVALID_PADDR;
+
     /*
      * The memory map is sorted and all RAM region starts are aligned to
      * page boundaries.
@@ -542,26 +566,22 @@ static paddr_t __init find_memory(
 
         ASSERT(IS_ALIGNED(start, PAGE_SIZE));
 
-        /*
-         * NB: Even better would be to use rangesets to determine a suitable
-         * range, in particular in case a kernel requests multiple heavily
-         * discontiguous regions (which right now we fold all into one big
-         * region).
-         */
-        if ( end <= kernel_start || start >= kernel_end )
-        {
-            /* No overlap, just check whether the region is large enough. */
-            if ( end - start >= size )
-                return start;
-        }
-        /* Deal with the kernel already being loaded in the region. */
-        else if ( kernel_start > start && kernel_start - start >= size )
-            return start;
-        else if ( kernel_end < end && end - kernel_end >= size )
-            return kernel_end;
+        if ( start < end &&
+             rangeset_add_range(ram, start, end - 1) != 0 )
+            return INVALID_PADDR;
     }
 
-    return INVALID_PADDR;
+    /* Remove the kernel range, rounded to page boundaries. */
+    if ( rangeset_remove_range(ram, (paddr_t)elf->dest_base & PAGE_MASK,
+                               ROUNDUP((paddr_t)elf->dest_base + elf->dest_size,
+                                       PAGE_SIZE) - 1) != 0 )
+        return INVALID_PADDR;
+
+    rangeset_report_ranges(ram, MB(1), -1, _find_memory, &info);
+
+    rangeset_destroy(ram);
+
+    return info.addr;
 }
 
 static bool __init check_load_address(

[PATCH 11/15] x86/PVH: try to map initrd directly into Dom0

[Jan Beulich]

Our present handling of initrd is problematic in particular when the
initrd is huge, and when at the same time "dom0_mem=" isn't in use to
significantly shrink Dom0's memory (from the default). In that case,
insufficient memory may exist for pvh_populate_p2m() to succeed (because
initrd memory can be freed only after copying).

Make an attempt to map the blob (including headroom) into Dom0 instead,
thus also saving the copying operation. With __start_xen() having arranged
for a larger than necessary blob, that mapping can typically be done with
superpage mappings.

Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Is there a need to prune (scrub) the headroom space?

Why does find_memory() go upwards, while find_kernel_memory() goes
downwards? And why do we use the lowest part of the region found by
find_memory() rather than the highest part?

--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -450,7 +450,9 @@ static void __init pvh_init_p2m(struct d
 static int __init pvh_populate_p2m(
     struct domain *d,
     const struct e820entry *e820,
-    unsigned int nr_e820)
+    unsigned int nr_e820,
+    unsigned long initrd_start,
+    unsigned long initrd_end)
 {
     struct vcpu *v = d->vcpu[0];
     unsigned int i;
@@ -468,7 +470,12 @@ static int __init pvh_populate_p2m(
         addr = PFN_DOWN(e820[i].addr);
         size = PFN_DOWN(e820[i].size);
 
-        rc = pvh_populate_memory_range(d, addr, size);
+        if ( addr + size <= initrd_start || addr >= initrd_end )
+            rc = pvh_populate_memory_range(d, addr, size);
+        else
+            rc = pvh_populate_memory_range(d, addr, initrd_start - addr)
+                 ?: pvh_populate_memory_range(d, initrd_end,
+                                              addr + size - initrd_end);
         if ( rc )
             return rc;
 
@@ -518,6 +525,43 @@ static int __init pvh_populate_p2m(
 #undef MB1_PAGES
 }
 
+static int __init pvh_map_initrd(
+    struct domain *d, const struct boot_module *initrd,
+    paddr_t initrd_start, unsigned int order)
+{
+    for ( paddr_t ga = initrd_start, ma = initrd->start;
+          ga < initrd_start + initrd->size;
+          ga += PAGE_SIZE << order, ma += PAGE_SIZE << order )
+    {
+        int rc = p2m_add_page(d, gaddr_to_gfn(ga), maddr_to_mfn(ma),
+                              order, p2m_ram_rw);
+
+        if ( rc )
+        {
+            printk("Failed to map initrd: [%#lx,%#lx): %d\n",
+                   PFN_DOWN(ga), PFN_DOWN(ga) + (1UL << order), rc);
+
+            /*
+             * Best effort rollback: The range will be replaced anyway by the
+             * subsequent pvh_populate_p2m().
+             */
+            while ( ga > initrd_start )
+            {
+                ga -= PAGE_SIZE << order;
+                ma -= PAGE_SIZE << order;
+                p2m_remove_page(d, gaddr_to_gfn(ga), maddr_to_mfn(ma), order);
+                order_stats[order]--;
+            }
+
+            return rc;
+        }
+
+        order_stats[order]++;
+    }
+
+    return 0;
+}
+
 struct mem_info {
     size_t size;
     paddr_t addr;
@@ -538,7 +582,10 @@ static int cf_check __init _find_memory(
 }
 
 static paddr_t __init find_memory(
-    const struct domain *d, const struct elf_binary *elf, size_t size)
+    const struct domain *d,
+    const struct elf_binary *elf,
+    const struct hvm_modlist_entry *initrd,
+    size_t size)
 {
     struct rangeset *ram = rangeset_new(NULL, NULL, 0);
     struct mem_info info = {
@@ -577,6 +624,13 @@ static paddr_t __init find_memory(
                                        PAGE_SIZE) - 1) != 0 )
         return INVALID_PADDR;
 
+    /* Remove the mapped initrd range (if any), rounded to page boundaries. */
+    if ( initrd->paddr &&
+         rangeset_remove_range(ram, initrd->paddr,
+                               ROUNDUP(initrd->paddr + initrd->size,
+                                       PAGE_SIZE) - 1) != 0 )
+        return INVALID_PADDR;
+
     rangeset_report_ranges(ram, MB(1), -1, _find_memory, &info);
 
     rangeset_destroy(ram);
@@ -689,6 +743,70 @@ static bool __init check_and_adjust_load
     return true;
 }
 
+static paddr_t __init find_initrd_memory(
+    const struct domain *d,
+    const struct elf_binary *elf,
+    const struct boot_module *initrd,
+    size_t align,
+    paddr_t limit)
+{
+    paddr_t kernel_start = (paddr_t)elf->dest_base & PAGE_MASK;
+    paddr_t kernel_end = ROUNDUP((paddr_t)elf->dest_base + elf->dest_size,
+                                 PAGE_SIZE);
+    unsigned long initrd_size = ROUNDUP(initrd->size, PAGE_SIZE);
+    unsigned long initrd_len = initrd_size - initrd->arch.headroom;
+
+    if ( !IS_ALIGNED(initrd->start | initrd_size, align) )
+        return INVALID_PADDR;
+
+    /*
+     * The memory map is sorted.  Search backwards to find the highest
+     * suitable address.
+     */
+    for ( unsigned int i = d->arch.nr_e820; i--; )
+    {
+        paddr_t start = d->arch.e820[i].addr;
+        paddr_t end = start + d->arch.e820[i].size;
+
+        if ( d->arch.e820[i].type != E820_RAM )
+            continue;
+
+        if ( end > limit )
+            continue;
+
+        start = MAX(ROUNDUP(start, align), MB(1));
+        end = ROUNDDOWN(end, align);
+
+        /* Don't use memory below 1MB, as it could overwrite BDA/EBDA/IBFT. */
+        if ( end <= MB(1) )
+            break;
+
+        if ( end <= initrd_size )
+            break;
+
+        if ( end - initrd_size <= start )
+            continue;
+
+        /*
+         * NB: Even better would be to use rangesets to determine a suitable
+         * range, in particular in case a kernel requests multiple heavily
+         * discontiguous regions (which right now we fold all into one big
+         * region).
+         */
+        if ( end <= kernel_start || end - initrd_len >= kernel_end )
+            return end - initrd_size; /* No overlap. */
+
+        /* Deal with the kernel already being loaded in the region. */
+        if ( kernel_end < end && end - kernel_end >= initrd_len )
+            return end - initrd_size;
+        if ( kernel_start > start &&
+             ROUNDDOWN(kernel_start, align) - start >= initrd_size )
+            return ROUNDDOWN(kernel_start, align) - initrd_size;
+    }
+
+    return INVALID_PADDR;
+}
+
 static int __init pvh_place_kernel(
     const struct boot_domain *bd,
     struct elf_binary *elf,
@@ -744,9 +862,93 @@ static int __init pvh_place_kernel(
     return 0;
 }
 
+static void __init pvh_place_initrd(
+    const struct boot_domain *bd,
+    const struct elf_binary *elf,
+    struct hvm_modlist_entry *mod,
+    paddr_t *blob_start,
+    paddr_t *blob_end)
+{
+    struct domain *d = bd->d;
+    struct boot_module *initrd = bd->initrd;
+    struct boot_module try = *initrd;
+    unsigned long initrd_len =
+        initrd ? initrd->size - initrd->arch.headroom : 0;
+    size_t align = 1UL << L3_PAGETABLE_SHIFT;
+    int rc;
+
+    *blob_start = INVALID_PADDR;
+    *blob_end   = 0;
+
+    if ( !initrd_len )
+        return;
+
+    mod->size = initrd_len;
+
+    /*
+     * As we promise to place below 4G whatever fits there, make a first pass
+     * limiting permitted addresses.  If we can't fit it there, fall back to
+     * placing above 4G.
+     */
+    for ( paddr_t limit = GB(4); ; )
+    {
+        paddr_t start;
+
+        *blob_start = find_initrd_memory(d, elf, &try, align, limit);
+        if ( *blob_start != INVALID_PADDR )
+            break;
+
+        if ( align == PAGE_SIZE )
+        {
+            if ( !(limit + 1) )
+                return;
+
+            limit = -1;
+            align = 1UL << L3_PAGETABLE_SHIFT;
+            try = *initrd;
+            continue;
+        }
+
+        align >>= PAGETABLE_ORDER;
+
+        start = ROUNDDOWN(try.start + try.arch.headroom, align);
+        if ( start > try.start )
+        {
+            /* Free (part of) and reduce headroom. */
+            try.arch.headroom -= start - try.start;
+            try.size          -= start - try.start;
+            try.start          = start;
+        }
+    }
+
+    /* Free unused headroom, if any. */
+    if ( try.start > initrd->start )
+    {
+        init_domheap_pages(initrd->start, try.start);
+        *initrd = try;
+    }
+
+    rc = pvh_map_initrd(d, initrd, *blob_start, get_order_from_bytes(align));
+    if ( rc )
+    {
+        printk("Failed to map initrd @ %#lx: %d\n", *blob_start, rc);
+        return;
+    }
+
+    rc = assign_pages(maddr_to_page(initrd->start),
+                      PFN_UP(initrd->size), d, dom0_memflags);
+    if ( rc )
+        panic("%pd: cannot assign initrd RAM: %d\n", d, rc);
+
+    mod->paddr = *blob_start + initrd->arch.headroom;
+    *blob_end  = *blob_start + initrd->size;
+    initrd->arch.released = true;
+}
+
 static int __init pvh_load_kernel(
     const struct boot_domain *bd,
     struct elf_binary *elf,
+    struct hvm_modlist_entry *mod,
     paddr_t *start_info_addr)
 {
     struct domain *d = bd->d;
@@ -756,9 +958,8 @@ static int __init pvh_load_kernel(
     size_t cmdline_len = bd->cmdline ? strlen(bd->cmdline) + 1 : 0;
     const char *initrd_cmdline = NULL;
     size_t extra_space;
-    paddr_t last_addr;
+    paddr_t last_addr, initrd_start = 0;
     struct hvm_start_info start_info = { 0 };
-    struct hvm_modlist_entry mod = { 0 };
     struct vcpu *v = d->vcpu[0];
     int rc;
 
@@ -775,15 +976,18 @@ static int __init pvh_load_kernel(
 
     /*
      * Find a RAM region big enough (and that doesn't overlap with the loaded
-     * kernel) in order to load the initrd and the metadata. Note it could be
-     * split into smaller allocations, done as a single region in order to
-     * simplify it.
+     * kernel) in order to load the initrd (if it wasn't mapped directly above)
+     * and the metadata.  Note it could be split into smaller allocations, done
+     * as a single region in order to simplify it.
      */
     extra_space = sizeof(start_info);
 
     if ( initrd )
     {
-        size_t initrd_space = elf_round_up(elf, initrd_len);
+        size_t initrd_space = 0;
+
+        if ( !mod->paddr )
+            initrd_space = elf_round_up(elf, initrd_len);
 
         if ( initrd->arch.cmdline_pa )
         {
@@ -795,21 +999,24 @@ static int __init pvh_load_kernel(
             initrd_space += strlen(initrd_cmdline) + 1;
 
         if ( initrd_space )
-            extra_space += ROUNDUP(initrd_space, PAGE_SIZE) + sizeof(mod);
+            extra_space += ROUNDUP(initrd_space, PAGE_SIZE) + sizeof(*mod);
+        else if ( mod->paddr )
+            extra_space += sizeof(*mod);
         else
             initrd = NULL;
     }
 
     extra_space += elf_round_up(elf, cmdline_len);
 
-    last_addr = find_memory(d, elf, extra_space);
+    last_addr = find_memory(d, elf, mod, extra_space);
     if ( last_addr == INVALID_PADDR )
     {
-        printk("Unable to find a memory region to load initrd and metadata\n");
+        printk("Unable to find a memory region to %s metadata\n",
+               mod->paddr ? "put" : "load initrd and");
         return -ENOMEM;
     }
 
-    if ( initrd != NULL )
+    if ( initrd && !mod->paddr )
     {
         rc = hvm_copy_to_guest_phys(last_addr,
                                     __va(initrd->start + initrd->arch.headroom),
@@ -820,28 +1027,29 @@ static int __init pvh_load_kernel(
             return rc;
         }
 
-        mod.paddr = last_addr;
-        mod.size = initrd_len;
+        initrd_start = last_addr;
         last_addr += elf_round_up(elf, initrd_len);
-        if ( initrd_cmdline )
-        {
-            size_t len = strlen(initrd_cmdline) + 1;
-
-            rc = hvm_copy_to_guest_phys(last_addr, initrd_cmdline, len, v);
-            if ( rc )
-            {
-                printk("Unable to copy module command line\n");
-                return rc;
-            }
-            mod.cmdline_paddr = last_addr;
-            last_addr += len;
-        }
-        last_addr = ROUNDUP(last_addr, PAGE_SIZE);
     }
 
     /* Free temporary buffers. */
     free_boot_modules();
 
+    if ( initrd_cmdline )
+    {
+        size_t len = strlen(initrd_cmdline) + 1;
+
+        rc = hvm_copy_to_guest_phys(last_addr, initrd_cmdline, len, v);
+        if ( rc )
+        {
+            printk("Unable to copy module command line\n");
+            return rc;
+        }
+        mod->cmdline_paddr = last_addr;
+        last_addr = ROUNDUP(last_addr + len, PAGE_SIZE);
+    }
+    else if ( initrd && !mod->paddr )
+        last_addr = ROUNDUP(last_addr, PAGE_SIZE);
+
     rc = hvm_copy_to_guest_phys(last_addr, bd->cmdline, cmdline_len, v);
     if ( rc )
     {
@@ -859,7 +1067,10 @@ static int __init pvh_load_kernel(
 
     if ( initrd != NULL )
     {
-        rc = hvm_copy_to_guest_phys(last_addr, &mod, sizeof(mod), v);
+        if ( !mod->paddr )
+            mod->paddr = initrd_start;
+
+        rc = hvm_copy_to_guest_phys(last_addr, mod, sizeof(*mod), v);
         if ( rc )
         {
             printk("Unable to copy guest modules\n");
@@ -867,7 +1078,7 @@ static int __init pvh_load_kernel(
         }
         start_info.modlist_paddr = last_addr;
         start_info.nr_modules = 1;
-        last_addr += sizeof(mod);
+        last_addr += sizeof(*mod);
     }
 
     start_info.magic = XEN_HVM_START_MAGIC_VALUE;
@@ -1441,6 +1652,8 @@ int __init dom0_construct_pvh(const stru
     unsigned int nr_orig_e820;
     struct acpi_info acpi = {};
     struct vmx_info vmx = {};
+    struct hvm_modlist_entry initrd = {};
+    paddr_t blob_start, blob_end;
     int rc;
 
     printk(XENLOG_INFO "*** Building a PVH Dom%d ***\n", d->domain_id);
@@ -1517,7 +1730,10 @@ int __init dom0_construct_pvh(const stru
             return rc;
     }
 
-    rc = pvh_populate_p2m(d, orig_e820, nr_orig_e820);
+    pvh_place_initrd(bd, &elf, &initrd, &blob_start, &blob_end);
+
+    rc = pvh_populate_p2m(d, orig_e820, nr_orig_e820,
+                          PFN_DOWN(blob_start), PFN_UP(blob_end));
     if ( rc )
     {
         printk("Failed to setup Dom0 physical memory map\n");
@@ -1526,7 +1742,7 @@ int __init dom0_construct_pvh(const stru
 
     xvfree(orig_e820);
 
-    rc = pvh_load_kernel(bd, &elf, &start_info);
+    rc = pvh_load_kernel(bd, &elf, &initrd, &start_info);
     if ( rc )
     {
         printk("Failed to load Dom0 kernel\n");

[PATCH 12/15] x86/PVH: heed the kernel in pvh_steal_ram()

[Jan Beulich]

While the function working downwards will typically result in no conflict
with the kernel range, there's no guarantee of that.

Fixes: 1d74282c455f ("x86: setup PVHv2 Dom0 ACPI tables")
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -186,8 +186,12 @@ static int __init pvh_populate_memory_ra
 /* Steal RAM from the end of a memory region. */
 static int __init pvh_steal_ram(struct domain *d, unsigned long size,
                                 unsigned long align, paddr_t limit,
+                                const struct elf_binary *elf,
                                 paddr_t *addr)
 {
+    paddr_t kernel_start = (paddr_t)elf->dest_base & PAGE_MASK;
+    paddr_t kernel_end = ROUNDUP((paddr_t)elf->dest_base + elf->dest_size,
+                                 PAGE_SIZE);
     unsigned int i = d->arch.nr_e820;
 
     /*
@@ -203,6 +207,11 @@ static int __init pvh_steal_ram(struct d
             continue;
 
         *addr = (entry->addr + entry->size - size) & ~(align - 1);
+
+        /* Avoid kernel range, if this is the region it lives in. */
+        if ( *addr < kernel_end && *addr + size > kernel_start )
+            *addr = (kernel_start - size) & ~(align - 1);
+
         if ( *addr < entry->addr ||
              /* Don't steal from the low 1MB due to the copying done there. */
              *addr < MB(1) )
@@ -275,6 +284,7 @@ struct vmx_info {
 
 static int __init pvh_place_vmx_realmode_helpers(
     struct domain *d,
+    const struct elf_binary *elf,
     struct vmx_info *info)
 {
     paddr_t gaddr;
@@ -285,7 +295,7 @@ static int __init pvh_place_vmx_realmode
      * structure (which accounts for the first 104b) doesn't cross a page
      * boundary.
      */
-    if ( !pvh_steal_ram(d, HVM_VM86_TSS_SIZE, 128, GB(4), &gaddr) )
+    if ( !pvh_steal_ram(d, HVM_VM86_TSS_SIZE, 128, GB(4), elf, &gaddr) )
     {
         d->arch.hvm.params[HVM_PARAM_VM86_TSS_SIZED] =
             VM86_TSS_UPDATED | ((uint64_t)HVM_VM86_TSS_SIZE << 32) | gaddr;
@@ -300,7 +310,7 @@ static int __init pvh_place_vmx_realmode
         printk("Unable to allocate VM86 TSS area\n");
 
     /* Steal some more RAM for the identity page tables. */
-    if ( pvh_steal_ram(d, PAGE_SIZE, PAGE_SIZE, GB(4), &gaddr) )
+    if ( pvh_steal_ram(d, PAGE_SIZE, PAGE_SIZE, GB(4), elf, &gaddr) )
     {
         printk("Unable to find memory to stash the identity page tables\n");
         return -ENOMEM;
@@ -1157,7 +1167,10 @@ static int __init cf_check acpi_set_intr
     return 0;
 }
 
-static int __init pvh_setup_acpi_madt(struct domain *d, paddr_t *addr)
+static int __init pvh_setup_acpi_madt(
+    struct domain *d,
+    const struct elf_binary *elf,
+    paddr_t *addr)
 {
     struct acpi_table_madt *madt;
     struct acpi_table_header *table;
@@ -1182,7 +1195,7 @@ static int __init pvh_setup_acpi_madt(st
     if ( !*addr )
     {
         /* Reserve space to place the new MADT in guest memory space. */
-        if ( pvh_steal_ram(d, size, 0, GB(4), addr) )
+        if ( pvh_steal_ram(d, size, 0, GB(4), elf, addr) )
         {
             printk("Unable to steal guest RAM for MADT\n");
             return -ENOMEM;
@@ -1337,8 +1350,11 @@ static bool __init pvh_acpi_xsdt_table_a
             strncmp(sig, ACPI_SIG_FACS, ACPI_NAME_SIZE));
 }
 
-static int __init pvh_setup_acpi_xsdt(struct domain *d, paddr_t madt_addr,
-                                      paddr_t *addr)
+static int __init pvh_setup_acpi_xsdt(
+    struct domain *d,
+    paddr_t madt_addr,
+    const struct elf_binary *elf,
+    paddr_t *addr)
 {
     struct acpi_table_xsdt *xsdt;
     struct acpi_table_header *table;
@@ -1374,7 +1390,7 @@ static int __init pvh_setup_acpi_xsdt(st
     if ( !*addr )
     {
         /* Reserve space to place the new XSDT in guest memory space. */
-        if ( pvh_steal_ram(d, size, 0, GB(4), addr) )
+        if ( pvh_steal_ram(d, size, 0, GB(4), elf, addr) )
         {
             printk("Unable to find guest RAM for XSDT\n");
             return -ENOMEM;
@@ -1473,6 +1489,7 @@ struct acpi_info {
 
 static int __init pvh_place_acpi(
     struct domain *d,
+    const struct elf_binary *elf,
     struct acpi_info *info)
 {
     unsigned int i;
@@ -1529,15 +1546,15 @@ static int __init pvh_place_acpi(
         }
     }
 
-    rc = pvh_setup_acpi_madt(d, &info->madt);
+    rc = pvh_setup_acpi_madt(d, elf, &info->madt);
     if ( rc )
         return rc;
 
-    rc = pvh_setup_acpi_xsdt(d, info->madt, &info->xsdt);
+    rc = pvh_setup_acpi_xsdt(d, info->madt, elf, &info->xsdt);
     if ( rc )
         return rc;
 
-    if ( pvh_steal_ram(d, sizeof(struct acpi_table_rsdp), 0, GB(4),
+    if ( pvh_steal_ram(d, sizeof(struct acpi_table_rsdp), 0, GB(4), elf,
                        &info->rsdp) )
     {
         printk("Unable to allocate guest RAM for RSDP\n");
@@ -1565,11 +1582,11 @@ static int __init pvh_setup_acpi(
         .length = sizeof(rsdp),
     };
 
-    rc = pvh_setup_acpi_madt(d, &info->madt);
+    rc = pvh_setup_acpi_madt(d, NULL, &info->madt);
     if ( rc )
         return rc;
 
-    rc = pvh_setup_acpi_xsdt(d, info->madt, &info->xsdt);
+    rc = pvh_setup_acpi_xsdt(d, info->madt, NULL, &info->xsdt);
     if ( rc )
         return rc;
 
@@ -1707,7 +1724,7 @@ int __init dom0_construct_pvh(const stru
         return rc;
     }
 
-    rc = pvh_place_acpi(d, &acpi);
+    rc = pvh_place_acpi(d, &elf, &acpi);
     if ( rc )
     {
         printk("Failed to place Dom0 ACPI tables: %d\n", rc);
@@ -1725,7 +1742,7 @@ int __init dom0_construct_pvh(const stru
           */
          !vmx_unrestricted_guest(d->vcpu[0]) )
     {
-        rc = pvh_place_vmx_realmode_helpers(d, &vmx);
+        rc = pvh_place_vmx_realmode_helpers(d, &elf, &vmx);
         if ( rc )
             return rc;
     }

[PATCH 13/15] x86/PVH: prefer smaller E820 regions for stealing from

[Jan Beulich]

Nibbling from the end of large regions tends to split full 2Mb (or even
full 1Gb) ranges, which Dom0 may otherwise be able to use superpage
mappings for (when, like Linux, it e.g. has a direct-map).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Instead of comparing original sizes, we may want to compare remaining
sizes. That may (slightly) further increase the risk of successive steals
not ending up adjacent, though.

There is the (pre-existing and purely theoretical at this point) risk of
(entry->addr + entry->size - size) underflowing. Do we think we need to
cover that remote case?

--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -192,6 +192,7 @@ static int __init pvh_steal_ram(struct d
     paddr_t kernel_start = (paddr_t)elf->dest_base & PAGE_MASK;
     paddr_t kernel_end = ROUNDUP((paddr_t)elf->dest_base + elf->dest_size,
                                  PAGE_SIZE);
+    struct e820entry *good = NULL;
     unsigned int i = d->arch.nr_e820;
 
     /*
@@ -202,26 +203,35 @@ static int __init pvh_steal_ram(struct d
     while ( i-- )
     {
         struct e820entry *entry = &d->arch.e820[i];
+        paddr_t res;
 
         if ( entry->type != E820_RAM || entry->addr + entry->size > limit )
             continue;
 
-        *addr = (entry->addr + entry->size - size) & ~(align - 1);
+        res = (entry->addr + entry->size - size) & ~(align - 1);
 
         /* Avoid kernel range, if this is the region it lives in. */
-        if ( *addr < kernel_end && *addr + size > kernel_start )
-            *addr = (kernel_start - size) & ~(align - 1);
+        if ( res < kernel_end && res + size > kernel_start )
+            res = (kernel_start - size) & ~(align - 1);
 
-        if ( *addr < entry->addr ||
+        if ( res < entry->addr ||
              /* Don't steal from the low 1MB due to the copying done there. */
-             *addr < MB(1) )
+             res < MB(1) )
             continue;
 
-        entry->size = *addr - entry->addr;
-        return 0;
+        if ( !good || good->size > entry->size )
+        {
+            good = entry;
+            *addr = res;
+        }
     }
 
-    return -ENOMEM;
+    if ( !good )
+        return -ENOMEM;
+
+    good->size = *addr - good->addr;
+
+    return 0;
 }
 
 /* NB: memory map must be sorted at all times for this to work correctly. */

[PATCH 14/15] x86/PVH: log Dom0's E820 map earlier

[Jan Beulich]

Do this as soon as it's final. For debug builds also mark it final, with
assertions put in place to make sure it won't further be altered. (For a
release build such further altering isn't the end of the world normally,
so the situation is left alone there.)

Switch to using %pd while moving the code up.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -46,6 +46,9 @@
  */
 #define HVM_VM86_TSS_SIZE 265
 
+/* Marker set once Dom0's E820 isn't supposed to change anymore */
+static bool __initdata e820_finalized;
+
 static unsigned int __initdata acpi_intr_overrides;
 static struct acpi_madt_interrupt_override __initdata *intsrcovr;
 
@@ -195,6 +198,8 @@ static int __init pvh_steal_ram(struct d
     struct e820entry *good = NULL;
     unsigned int i = d->arch.nr_e820;
 
+    ASSERT(!e820_finalized);
+
     /*
      * Alignment 0 should be set to 1, so it doesn't wrap around in the
      * calculations below.
@@ -241,6 +246,8 @@ static int __init pvh_add_mem_range(stru
     struct e820entry *map;
     unsigned int i;
 
+    ASSERT(!e820_finalized);
+
     for ( i = 0; i < d->arch.nr_e820; i++ )
     {
         uint64_t rs = d->arch.e820[i].addr;
@@ -1759,6 +1766,13 @@ int __init dom0_construct_pvh(const stru
 
     pvh_place_initrd(bd, &elf, &initrd, &blob_start, &blob_end);
 
+    if ( opt_dom0_verbose )
+    {
+        printk("%pd memory map:\n", d);
+        print_e820_memory_map(d->arch.e820, d->arch.nr_e820);
+        e820_finalized = true;
+    }
+
     rc = pvh_populate_p2m(d, orig_e820, nr_orig_e820,
                           PFN_DOWN(blob_start), PFN_UP(blob_end));
     if ( rc )
@@ -1794,12 +1808,6 @@ int __init dom0_construct_pvh(const stru
     if ( rc )
         return rc;
 
-    if ( opt_dom0_verbose )
-    {
-        printk("Dom%u memory map:\n", d->domain_id);
-        print_e820_memory_map(d->arch.e820, d->arch.nr_e820);
-    }
-
     return 0;
 }
 

[PATCH 15/15] x86/PVH: simplify writing of Dom0's RSDP

[Jan Beulich]

Now that we establish where the RSDP is to live before actually loading
the kernel (and writing start-info there) we can simply hand the pointer
there, eliminating the need for the (fragile) separate write to the field
out of pvh_setup_acpi().

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Alternatively we could move start_info to the top-level function, have
pvh_load_kernel() and pvh_setup_acpi() fill what need filling, and do
the write from the top-level function. Yet it looks to me as if this was
more churn for little extra gain.

--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -976,7 +976,8 @@ static int __init pvh_load_kernel(
     const struct boot_domain *bd,
     struct elf_binary *elf,
     struct hvm_modlist_entry *mod,
-    paddr_t *start_info_addr)
+    paddr_t *start_info_addr,
+    paddr_t rsdp)
 {
     struct domain *d = bd->d;
     struct boot_module *initrd = bd->initrd;
@@ -1110,6 +1111,7 @@ static int __init pvh_load_kernel(
 
     start_info.magic = XEN_HVM_START_MAGIC_VALUE;
     start_info.flags = SIF_PRIVILEGED | SIF_INITDOMAIN;
+    start_info.rsdp_paddr = rsdp;
     rc = hvm_copy_to_guest_phys(last_addr, &start_info, sizeof(start_info), v);
     if ( rc )
     {
@@ -1587,10 +1589,7 @@ static int __init pvh_place_acpi(
     return 0;
 }
 
-static int __init pvh_setup_acpi(
-    struct domain *d,
-    struct acpi_info *info,
-    paddr_t start_info)
+static int __init pvh_setup_acpi(struct domain *d, struct acpi_info *info)
 {
     int rc;
     struct acpi_table_rsdp *native_rsdp, rsdp = {
@@ -1642,20 +1641,6 @@ static int __init pvh_setup_acpi(
         return rc;
     }
 
-    /* Copy RSDP address to start_info. */
-    BUILD_BUG_ON(sizeof(info->rsdp) !=
-                 sizeof_field(struct hvm_start_info, rsdp_paddr));
-    rc = hvm_copy_to_guest_phys(start_info +
-                                offsetof(struct hvm_start_info, rsdp_paddr),
-                                &info->rsdp,
-                                sizeof_field(struct hvm_start_info, rsdp_paddr),
-                                d->vcpu[0]);
-    if ( rc )
-    {
-        printk("Unable to copy RSDP address to start info\n");
-        return rc;
-    }
-
     return 0;
 }
 
@@ -1783,7 +1768,7 @@ int __init dom0_construct_pvh(const stru
 
     xvfree(orig_e820);
 
-    rc = pvh_load_kernel(bd, &elf, &initrd, &start_info);
+    rc = pvh_load_kernel(bd, &elf, &initrd, &start_info, acpi.rsdp);
     if ( rc )
     {
         printk("Failed to load Dom0 kernel\n");
@@ -1797,7 +1782,7 @@ int __init dom0_construct_pvh(const stru
         return rc;
     }
 
-    rc = pvh_setup_acpi(d, &acpi, start_info);
+    rc = pvh_setup_acpi(d, &acpi);
     if ( rc )
     {
         printk("Failed to setup Dom0 ACPI tables: %d\n", rc);

Re: [PATCH 00/15] x86: PVH Dom0 loading rework

[Jan Beulich]

On 12.10.2026 10:17, Jan Beulich wrote:
> The primary goal is to accommodate large initrd-s without requiring people
> to restrict Dom0's memory just for that. Other issues are being addressed
> as well, though, and some cleanup is done, in part even outside of the main
> target area of this series.
> 
> 01: EFI: guard against oversized modules
> 02: PV: make better use of assign_pages()
> 03: setup: break out E820_RAM accumulation
> 04: setup: special treatment of initrd for PVH Dom0
> 05: PVH: split pvh_load_kernel()
> 06: PVH: keep a copy of Dom0's initial E820 map
> 07: PVH: lift E820 alignment requirement
> 08: PVH: split pvh_setup_acpi()
> 09: PVH: split pvh_setup_acpi()

As can be seen from the actual patch title, this was a copy-and-paste mistake
of mine. Really:

09: PVH: split pvh_setup_vmx_realmode_helpers()

Jan

> 10: PVH: convert find_memory() to using a rangeset
> 11: PVH: try to map initrd directly into Dom0
> 12: PVH: heed the kernel in pvh_steal_ram()
> 13: PVH: prefer smaller E820 regions for stealing from
> 14: PVH: log Dom0's E820 map earlier
> 15: PVH: simplify writing of Dom0's RSDP