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

[Roger Pau Monné <roger@xxxxxxxxxxxxxx>]

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@xxxxxxxx>
> 
> --- 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.