Re: [PATCH v3 14/18] x86/domain_page: key the directmap fast paths on the loaded view

[George Dunlap <gwd@xxxxxxxxxxxxxx>]

On Thu, Oct 8, 2026 at 7:51 PM Roger Pau Monné <roger@xxxxxxxxxxxxxx> wrote:
>
> On Wed, Oct 07, 2026 at 11:40:47AM +0100, George Dunlap wrote:
> > map_domain_page() returns directmap addresses whenever it can: for any
> > MFN the directmap covers in release builds, and for vCPUs without a
> > mapcache.  Once guest contexts may run on the sparse view of the
> > directmap, modify this to take the fast path returning the directmap
> > address only when the full directmap is loaded.
> >
> > The release-build shortcut ahead of the vCPU lookup looks at the vCPU
> > whose page-tables are loaded, without completing a lazy context
> > switch: if that vCPU is not on the sparse view, its page-tables map
> > the page through the directmap, and completing the switch would only
> > move the CPU to the idle vCPU's page-tables, which map it too.
> >
> > mapcache_current_vcpu() may complete the lazy context switch
> > (sync_local_execstate()) and switch us onto the idle vCPU (which has a
> > full view), so check again if we can use the directmap afterwards.
> >
> > A vCPU on the sparse view always has a mapcache: HVM ones through
> > per-vCPU page-tables, PV ones through the domain-wide mapcache, which
> > mapcache_domain_init() now sets up for a domain using the sparse view in
> > release builds too, rather than only when the directmap does not cover
> > all memory.
> >
> > map_domain_page_global() mappings must be valid in every context: use
> > vmap() for all of them once a sparse view is configured, as
> > arch_mfns_in_directmap() then says.
> >
> > Add d->arch.sparse_dmap and sparse_dmap_active for this; nothing sets
> > them yet.
> >
> > No functional change.
> >
> > Assisted-by: Claude Code:claude-opus-5-5
> > Signed-off-by: George Dunlap <gwd@xxxxxxxxxxxxxx>
> > ---
> > Changes in v3:
> > - New in this version.
> > ---
> >  xen/arch/x86/domain_page.c                  | 44 +++++++++++++++++++--
> >  xen/arch/x86/include/asm/domain.h           |  6 +++
> >  xen/arch/x86/include/asm/sparse-directmap.h |  8 ++++
> >  xen/arch/x86/sparse-directmap.c             |  1 +
> >  4 files changed, 56 insertions(+), 3 deletions(-)
> >
> > diff --git a/xen/arch/x86/domain_page.c b/xen/arch/x86/domain_page.c
> > index 0500bb9dba..48f85b670d 100644
> > --- a/xen/arch/x86/domain_page.c
> > +++ b/xen/arch/x86/domain_page.c
> > @@ -61,6 +61,16 @@ static inline struct vcpu *mapcache_current_vcpu(void)
> >      return ACCESS_ONCE(this_cpu(pgtable_vcpu));
> >  }
> >
> > +/*
> > + * Whether the directmap in @v's page-tables is the sparse view, which maps
> > + * none of the heap's memory beyond the xenheap: the mapcache must serve
> > + * every mapping then.
> > + */
> > +static bool on_sparse_dmap(const struct vcpu *v)
> > +{
> > +    return sparse_dmap_active && v->domain->arch.sparse_dmap;
> > +}
> > +
> >  /*
> >   * The accounting for v's mapcache: its own for a vCPU-PT domain, the 
> > domain's
> >   * otherwise, in which case *dcache is set so the caller takes the 
> > domain-wide
> > @@ -142,17 +152,39 @@ void *map_domain_page(mfn_t mfn)
> >
> >  #ifdef NDEBUG
> >      if ( mfn_x(mfn) <= PFN_DOWN(__pa(HYPERVISOR_VIRT_END - 1)) )
> > -        return mfn_to_virt(mfn_x(mfn));
> > +    {
> > +        const struct vcpu *pt_vcpu = this_cpu(pgtable_vcpu);
> > +
> > +        /*
> > +         * The page-tables loaded map @mfn through the directmap, unless 
> > they
> > +         * carry the sparse view.  Don't complete a lazy context switch to
> > +         * find out: it would only move this CPU to the idle vCPU's
> > +         * page-tables, which map @mfn too.
> > +         */
> > +        if ( !pt_vcpu || !on_sparse_dmap(pt_vcpu) )
> > +            return mfn_to_virt(mfn_x(mfn));
> > +    }
> >  #endif
> >
> > +    /* No vCPU (early boot), or no mapcache: the full directmap is loaded. 
> > */
> >      v = mapcache_current_vcpu();
> >      if ( !v )
> >          return mfn_to_virt(mfn_x(mfn));
> >
> > +#ifdef NDEBUG
> > +    /* The lookup may have completed a lazy switch off the sparse view. */
> > +    if ( !on_sparse_dmap(v) &&
> > +         mfn_x(mfn) <= PFN_DOWN(__pa(HYPERVISOR_VIRT_END - 1)) )
> > +        return mfn_to_virt(mfn_x(mfn));
> > +#endif
> > +
> >      cache = vcpu_mapcache(v, &dcache);
> >      vcache = &v->arch.mapcache;
> >      if ( !cache->inuse )
> > +    {
> > +        ASSERT(!on_sparse_dmap(v));
> >          return mfn_to_virt(mfn_x(mfn));
> > +    }
> >
> >      perfc_incr(map_domain_page_count);
> >
> > @@ -307,7 +339,9 @@ void mapcache_domain_init(struct domain *d)
> >      ASSERT(is_pv_domain(d));
> >
> >  #ifdef NDEBUG
> > -    if ( !mem_hotplug && max_page <= PFN_DOWN(__pa(HYPERVISOR_VIRT_END - 
> > 1)) )
> > +    /* Not needed when the directmap loaded for the domain covers 
> > everything. */
> > +    if ( !d->arch.sparse_dmap && !mem_hotplug &&
> > +         max_page <= PFN_DOWN(__pa(HYPERVISOR_VIRT_END - 1)) )
> >          return;
> >  #endif
> >
> > @@ -392,8 +426,12 @@ void *map_domain_page_global(mfn_t mfn)
> >               system_state < SYS_STATE_active) ||
> >              local_irq_is_enabled()));
> >
> > +    /*
> > +     * Global mappings must hold in every context: a sparse view of the
> > +     * directmap rules the directmap out.
> > +     */
> >  #ifdef NDEBUG
> > -    if ( mfn_x(mfn) <= PFN_DOWN(__pa(HYPERVISOR_VIRT_END - 1)) )
> > +    if ( arch_mfns_in_directmap(mfn_x(mfn), 1) )
> >          return mfn_to_virt(mfn_x(mfn));
> >  #endif
> >
> > diff --git a/xen/arch/x86/include/asm/domain.h 
> > b/xen/arch/x86/include/asm/domain.h
> > index 80415ab2c2..76a22b4040 100644
> > --- a/xen/arch/x86/include/asm/domain.h
> > +++ b/xen/arch/x86/include/asm/domain.h
> > @@ -499,6 +499,12 @@ struct arch_domain
> >       */
> >      bool vcpu_pt;
> >
> > +    /*
> > +     * Root page-tables carry the sparse view of the directmap, once it is
> > +     * active (see asm/sparse-directmap.h).
> > +     */
> > +    bool sparse_dmap;
>
> What do you mean with "once it is active"?  Either a domain is using
> the sparse direct map or not, but it shouldn't have an external
> dependency on anything.  Or maybe I'm reading the comment the wrong
> way.

I've reworded it.

> > +/*
> > + * Set once guest contexts run on the sparse view: from then on, the root
> > + * page-tables of domains with d->arch.sparse_dmap carry it.
> > + */
> > +extern bool sparse_dmap_active;
>
> Shouldn't sparse_dmap_active be opt_sparse_dmap_pv ||
> opt_sparse_dmap_hvm?  I'm afraid I don't understand the point of
> tracking whether there are domain using the sparse direct map or not,
> that's already tracked by d->arch.sparse_dmap?

Yeah, sparse_dmap_active was originally meant to be used to shortcut
certain things when no domains were using the sparse directmap; but it
hasn't turned out to be needed.  I'll remove it.

 -George