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

[Roger Pau Monné <roger@xxxxxxxxxxxxxx>]

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.

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

Thanks, Roger.