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

[George Dunlap <gwd@xxxxxxxxxxxxxx>]

On Wed, Oct 7, 2026 at 5:32 PM Roger Pau Monné <roger@xxxxxxxxxxxxxx> wrote:
>
> 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.

Done, thanks.

 -George