Re: [PATCH v3 16/18] x86/domain_page: keep the maphash's slots, not their mappings

[George Dunlap <gwd@xxxxxxxxxxxxxx>]

On Thu, Oct 8, 2026 at 8:09 PM Roger Pau Monné <roger@xxxxxxxxxxxxxx> wrote:
>
> On Wed, Oct 07, 2026 at 11:40:49AM +0100, George Dunlap wrote:
> > unmap_domain_page() keeps a recently unmapped slot in the vCPU's
> > maphash (MAPHASH_ENTRIES, 8 per vCPU) for a quick remap of the same
> > MFN, and leaves its PTE in place until the entry is displaced by a later
> > unmap or reclaimed by a full cache.  Until then the page stays mapped,
> > although nothing refers to the mapping and the mapcache holds no
> > reference to the page.  A page freed meanwhile and handed to another
> > domain stays mapped in the previous user's mapcache: in the vCPU's own
> > area for a domain with per-vCPU page-tables, in the domain-wide area of
> > a PV domain, where all of its vCPUs can reach it.
> >
> > While the full directmap is loaded, that reaches nothing it does not
> > map anyway.  On the sparse view of the directmap, Xen reaches
> > domain-heap memory only through mappings made for the purpose, and a
> > mapping kept in the maphash would leave another domain's page
> > reachable from a context which has nothing to do with it.
> >
> > Keep the slot, not the mapping: unmapping the last reference zaps the
> > PTE and keeps the hash entry, and a later hit maps the page again in the
> > same slot.  The slot has mapped nothing else in between, so that needs
> > no flush.  Reusing the slot for another page still goes through the
> > garbage map, or through the replacement of a hash entry when the cache
> > is full, both of which flush first.  A translation of the zapped PTE
> > left in a TLB is covered, for owned pages, by the flush the page
> > allocator arranges before a freed page is reused wherever a sparse view
> > is configured ("x86/mm: flush owned pages before reuse with the sparse
> > directmap view").
>
> As commented on the previous patch, I'm not convinced that's enough.
> I do think we need to do a flush here, as it's not so much as the page
> being used by another domain (or by Xen itself), internally the guest
> might also have different contexts and we should prevent leaks between
> them.

Same answer as 15; this is meant to close one particular hole, not be
a comprehensive closure of all holes.

> > The cost is one PTE write per unmap into the maphash and per remap from
> > it.  A second unmap of an entry in the maphash now finds its PTE not
> > present and is caught by the check unmap_domain_page() has for entries
> > the first unmap zapped, rather than by the reference count's ASSERT()
> > in debug builds only.
> >
> > The change does not depend on asi= or on a sparse view of the directmap
> > being configured.  PV vCPUs use the domain-wide mapcache on the default
> > command line too: in debug builds for every page, in release builds for
> > pages beyond the reach of the full directmap in their page-tables (5TiB,
> > 3.5TiB with CONFIG_BIGMEM).
>
> There's likely no need to mention again the exact conditions that make
> the mapcache active for PV domains, that's IMO too verbose and doesn't
> add anything of value to the reasoning here.

I'll reword it, thanks.

 -George