Re: [PATCH v3 05/18] x86/mm: prepare create_perdomain_mapping() for per-vCPU perdomain areas

[Roger Pau Monné <roger@xxxxxxxxxxxxxx>]

On Wed, Oct 07, 2026 at 11:40:38AM +0100, George Dunlap wrote:
> From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> We want to change per-domain mappings to be per-vCPU mappings.  In
> preparation for that, we want to arrange that
> create_perdomain_mapping() work either with a single perdomain area,
> or with a per-vCPU perdomain area.
> 
> Most of the remaining callers are already in a vCPU context.  This is
> no accident: the perdomain area has always been laid out in per-vCPU
> slices -- each vCPU has its own GDT/LDT window, its own
> COMPAT_ARG_XLAT pages, its own window of mapcache entries -- and each
> vCPU's slice is set up as that vCPU is created.  For these callers, we
> just need to change the parameter from a domain pointer to a vCPU
> pointer.  Once the perdomain area itself becomes per-vCPU, the same
> calls will populate the owning vCPU's own area rather than slices of a
> shared one.
> 
> One exception is the call in hvm_domain_initialise().  An HVM vCPU's
> monitor table is created during vCPU initialisation, and
> init_xen_l4_slots() stamps the perdomain slot into it at that point --
> far earlier than for PV, where the Xen slots are written only once
> guest page tables are built.  hvm_domain_initialise() therefore had an
> explicit create_perdomain_mapping() call just to make the perdomain
> root exist ahead of that.  Move it to arch_vcpu_create(), covering HVM
> and PV alike.  With a single shared area, the call allocates at most
> once per domain; but once each vCPU has its own perdomain area, this
> is the call that will allocate every vCPU's root before any page
> tables referencing it are built.  vCPU creation is
> where the call must end up; move it there directly.

I think it's clear from the paragraph above the call must be moved
there, there's likely no need to re-iterate that fact.

> For PV guests
> nothing observable changes: the root was already being created during
> vCPU creation as a side effect (by mapcache_vcpu_init(), or failing
> that pv_create_gdt_ldt_l1tab()); it now merely becomes explicit.
> 
> Note that we cannot yet do a parallel movement of
> free_perdomain_mappings(): the per-domain page-table hierarchy is
> still a single domain-wide structure shared by all vCPUs, so tearing
> it down from a per-vCPU path would pull the mappings out from under
> sibling vCPUs (e.g.  on a partially failed, retryable
> XEN_DOMCTL_max_vcpus), and vCPU-create error paths can rely on domain
> destruction to free a partially set up hierarchy.  Teardown will move
> to vCPU scope only once the structure itself becomes per-vCPU.

Even then - we will still need to support non-ASI (PV domains) not
using a per-vCPU slot, and hence the call can't be moved as-is just
when per-vCPU support is available?

> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> Assisted-by: Claude Code:claude-fable-5, Claude Code:claude-opus-4-8, Claude 
> Code:claude-opus-5-5
> Signed-off-by: George Dunlap <gwd@xxxxxxxxxxxxxx>
> ---
> Changes in v3:
> - Rebased onto staging without the GDT/LDT conversions (for the
>   HVM-only series): the capture parameters stay, as the PV GDT/LDT
>   stash still uses them.
> 
> Changes in v2:
> - Added to the series
> 
> Changes since the previously posted version:
> - Keep free_perdomain_mappings() (and hence perdomain teardown)
>   domain-scoped.
> - Keep the idle domain without a perdomain area.
> - Retitle (was: "x86/mm: switch {create,destroy}_perdomain_mapping()
>   domain parameter to vCPU"); destroy_perdomain_mapping() was switched
>   in a separate patch.
> - Split the removal of mapcache_domain_init()'s redundant
>   create_perdomain_mapping() call into its own (preceding) patch.
> ---
>  xen/arch/x86/domain.c         | 10 ++++++++++
>  xen/arch/x86/domain_page.c    |  6 +++---
>  xen/arch/x86/hvm/hvm.c        |  5 -----
>  xen/arch/x86/include/asm/mm.h |  2 +-
>  xen/arch/x86/mm.c             |  3 ++-
>  xen/arch/x86/pv/domain.c      |  2 +-
>  xen/arch/x86/x86_64/mm.c      |  2 +-
>  7 files changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 85cedc3d03..b670cea52b 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -517,6 +517,16 @@ int arch_vcpu_create(struct vcpu *v)
>  
>      if ( !is_idle_domain(d) )
>      {
> +        /*
> +         * Make sure the per-domain L3 exists ahead of any consumer (e.g.
> +         * init_xen_l4_slots() for the HVM monitor tables): with
> +         * create_perdomain_mapping() taking a vCPU this can no longer be
> +         * done when creating the domain.

I think the comment here is not very useful - it's not helpful that
the comment here references the previous position of the call.  It
only makes sense in the context of this change, but not for someone
looking at the code after this commit has landed.  I would terminate
the commit at the ":".

The code itself looks good to me.

Thanks, Roger.