[PATCH v3 01/18] x86/PoD: map one page at a time in p2m_pod_zero_check()

[George Dunlap <gwd@xxxxxxxxxxxxxx>]

p2m_pod_zero_check() maps every page of its batch that passes its
checks, up to POD_SWEEP_STRIDE (16) of them, and keeps them mapped
across the p2m lookups and updates which follow, each of which maps
page-table pages of its own.  Its callers hold mappings too: the p2m
lookup which found the entry to populate still has its page-table page
mapped, and a PV mmu_update() the L1 page it is writing.

map_domain_page() cannot always provide that many at once.  It uses the
mapcache on behalf of PV vCPUs only: in debug builds for every page, in
release builds for pages beyond the reach of the directmap in PV guests'
page-tables (5TiB, 3.5TiB with CONFIG_BIGMEM).  The mapcache has
MAPCACHE_VCPU_ENTRIES (16) slots per vCPU of the domain, shared by its
vCPUs, and a mapping which finds no slot free or reclaimable hits the
BUG_ON() in map_domain_page(): the host crashes.

The sweep runs in the context of whichever vCPU populates a PoD entry
while the domain's PoD cache is empty.  The guest's own vCPUs, being
HVM, map through the directmap; a PV vCPU gets there when its domain
maps or copies the guest's memory: a device model in dom0 or in a stub
domain, a backend doing grant copies.  A PV domain with a single vCPU,
as a device-model stub domain or a dom0 booted with dom0_max_vcpus=1
is, has 16 slots, and a sweep over 16 candidate pages needs more.

The sweep has mapped its whole batch at once since 9ad9a1609fbe ("PoD
memory 5/9: emergency scan"), when x86-64 reached all memory through the
directmap.  The per-vCPU budget came with the mapcache's return to
x86-64 in 4b28bf6ae90b ("x86: re-introduce map_domain_page() et al").

Map one page at a time instead, as p2m_pod_zero_check_superpage()
already does: map a page for the quick check and unmap it again, and
map it anew for the full check after the p2m update and the TLB flush.
The checks and their order are unchanged.  Each page is mapped twice,
which costs little next to scanning it, and the error paths no longer
have mappings to undo.

Fixes: 4b28bf6ae90b ("x86: re-introduce map_domain_page() et al")
Assisted-by: Claude Code:claude-opus-5-5
Signed-off-by: George Dunlap <gwd@xxxxxxxxxxxxxx>
---
NB this issue was raised in review and verified only by code
inspection; it hasn't been demonstrated.
---
 xen/arch/x86/mm/p2m-pod.c | 72 +++++++++++++++++++--------------------
 1 file changed, 35 insertions(+), 37 deletions(-)

diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index 4602c32cff..0b0beca15a 100644
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -901,7 +901,8 @@ p2m_pod_zero_check(struct p2m_domain *p2m, const gfn_t 
*gfns, unsigned int count
 {
     mfn_t mfns[POD_SWEEP_STRIDE];
     p2m_type_t types[POD_SWEEP_STRIDE];
-    unsigned long *map[POD_SWEEP_STRIDE];
+    bool check[POD_SWEEP_STRIDE];
+    const unsigned long *map;
     struct domain *d = p2m->domain;
     unsigned int i, j, max_ref = 1;
 
@@ -911,7 +912,12 @@ p2m_pod_zero_check(struct p2m_domain *p2m, const gfn_t 
*gfns, unsigned int count
     if ( paging_mode_shadow(d) )
         max_ref++;
 
-    /* First, get the gfn list, translate to mfns, and map the pages. */
+    /*
+     * First, get the gfn list, translate to mfns, and pick the pages to
+     * check.  They are mapped one at a time below, as in
+     * p2m_pod_zero_check_superpage(): a vCPU can hold only a few transient
+     * mappings at once, and the p2m lookups and updates here need some too.
+     */
     for ( i = 0; i < count; i++ )
     {
         p2m_access_t a;
@@ -921,18 +927,17 @@ p2m_pod_zero_check(struct p2m_domain *p2m, const gfn_t 
*gfns, unsigned int count
 
         /*
          * If this is ram, and not a pagetable or a special page, and
-         * probably not mapped elsewhere, map it; otherwise, skip.
+         * probably not mapped elsewhere, check it; otherwise, skip.
          */
-        map[i] = NULL;
+        check[i] = false;
         if ( p2m_is_ram(types[i]) )
         {
             const struct page_info *pg = mfn_to_page(mfns[i]);
 
-            if ( !is_special_page(pg) &&
-                 (pg->count_info & PGC_allocated) &&
-                 !(pg->count_info & PGC_shadowed_pt) &&
-                 ((pg->count_info & PGC_count_mask) <= max_ref) )
-                map[i] = map_domain_page(mfns[i]);
+            check[i] = !is_special_page(pg) &&
+                       (pg->count_info & PGC_allocated) &&
+                       !(pg->count_info & PGC_shadowed_pt) &&
+                       ((pg->count_info & PGC_count_mask) <= max_ref);
         }
     }
 
@@ -942,18 +947,24 @@ p2m_pod_zero_check(struct p2m_domain *p2m, const gfn_t 
*gfns, unsigned int count
      */
     for ( i = 0; i < count; i++ )
     {
-        if ( !map[i] )
+        if ( !check[i] )
             continue;
 
         /* Quick zero-check */
+        map = map_domain_page(mfns[i]);
         for ( j = 0; j < 16; j++ )
-            if ( *(map[i] + j) != 0 )
-                goto skip;
+            if ( map[j] != 0 )
+                break;
+        unmap_domain_page(map);
 
         /* Try to remove the page, restoring old mapping if it fails. */
-        if ( p2m_set_entry(p2m, gfns[i], INVALID_MFN, PAGE_ORDER_4K,
+        if ( j < 16 ||
+             p2m_set_entry(p2m, gfns[i], INVALID_MFN, PAGE_ORDER_4K,
                            p2m_populate_on_demand, p2m->default_access) )
-            goto skip;
+        {
+            check[i] = false;
+            continue;
+        }
 
         /*
          * See if the page was successfully unmapped.  (Allow one refcount
@@ -961,6 +972,8 @@ p2m_pod_zero_check(struct p2m_domain *p2m, const gfn_t 
*gfns, unsigned int count
          */
         if ( (mfn_to_page(mfns[i])->count_info & PGC_count_mask) > 1 )
         {
+            check[i] = false;
+
             /*
              * If the previous p2m_set_entry call succeeded, this one shouldn't
              * be able to fail.  If it does, crashing the domain should be 
safe.
@@ -970,14 +983,8 @@ p2m_pod_zero_check(struct p2m_domain *p2m, const gfn_t 
*gfns, unsigned int count
             {
                 ASSERT_UNREACHABLE();
                 domain_crash(d);
-                goto out_unmap;
+                return;
             }
-
-        skip:
-            unmap_domain_page(map[i]);
-            map[i] = NULL;
-
-            continue;
         }
     }
 
@@ -986,22 +993,20 @@ p2m_pod_zero_check(struct p2m_domain *p2m, const gfn_t 
*gfns, unsigned int count
     /* Now check each page for real */
     for ( i = 0; i < count; i++ )
     {
-        if ( !map[i] )
+        if ( !check[i] )
             continue;
 
-        for ( j = 0; j < (PAGE_SIZE / sizeof(*map[i])); j++ )
-            if ( *(map[i] + j) != 0 )
+        map = map_domain_page(mfns[i]);
+        for ( j = 0; j < (PAGE_SIZE / sizeof(*map)); j++ )
+            if ( map[j] != 0 )
                 break;
-
-        unmap_domain_page(map[i]);
-
-        map[i] = NULL;
+        unmap_domain_page(map);
 
         /*
          * See comment in p2m_pod_zero_check_superpage() re gnttab
          * check timing.
          */
-        if ( j < (PAGE_SIZE / sizeof(*map[i])) )
+        if ( j < (PAGE_SIZE / sizeof(*map)) )
         {
             /*
              * If the previous p2m_set_entry call succeeded, this one shouldn't
@@ -1012,14 +1017,7 @@ p2m_pod_zero_check(struct p2m_domain *p2m, const gfn_t 
*gfns, unsigned int count
             {
                 ASSERT_UNREACHABLE();
                 domain_crash(d);
- out_unmap:
-                /*
-                 * Something went wrong, probably crashing the domain.  Unmap
-                 * everything and return.
-                 */
-                for ( i = 0; i < count; i++ )
-                    if ( map[i] )
-                        unmap_domain_page(map[i]);
+                return;
             }
         }
         else
-- 
2.55.0