[Xen-devel] [PATCH 2/4] x86/mm: rename and tidy create_pae_xen_mappings()

[Jan Beulich <jbeulich@xxxxxxxx>]

After dad74b0f9e ("i386: fix handling of Xen entries in final L2 page
table") and the removal of 32-bit support the function doesn't modify
state anymore, and hence its name has been misleading. Change its name,
constify parameters and a local variable, and make it return bool.

Also drop the call to it from mod_l3_entry(): The function explicitly
disallows 32-bit domains to modify slot 3. Furthermore, re-checking
of slot 3 when other slots change has needlessly disallowed doing this
together with making some L2 table recursively link back to an L2 used
in some L3's 3rd slot (because of the checking of the type ref count to
be 1). (Note that allowing dynamic changes of L3 entries in the way we
do is bogus anyway, as that's not how L3s behave in the native case:
They get re-evaluated only upon CR3 reloads.)

As a result of this we no longer need to play games to get at the start
of the L3 table. Leave an ASSERT() though, just in case.

Further move a BUG_ON() such that in the common case its condition
wouldn't need evaluating.

Finally, since we're at it, move init_xen_pae_l2_slots() next to the
renamed function, as they really belong together (in fact
init_xen_pae_l2_slots() was [indirectly] broken out of this function).

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
We could go further here and delete the function altogether: There are
no linear mappings in a PGT_pae_xen_l2 table anymore (this was on 32-bit
only). The corresponding conditional in mod_l3_entry() could then go
away as well (or, more precisely, would need to be replaced by correct
handling of 3rd slot updates). This would mean that a 32-bit guest
functioning on new Xen may fail to work on older (possibly 32-bit) Xen.

--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -1414,22 +1414,23 @@ static int promote_l1_table(struct page_
     return ret;
 }
 
-static int create_pae_xen_mappings(struct domain *d, l3_pgentry_t *pl3e)
+static bool pae_xen_mappings_check(const struct domain *d,
+                                   const l3_pgentry_t *pl3e)
 {
-    struct page_info *page;
-    l3_pgentry_t     l3e3;
+    const struct page_info *page;
+    l3_pgentry_t l3e3;
 
     if ( !is_pv_32bit_domain(d) )
-        return 1;
+        return true;
 
-    pl3e = (l3_pgentry_t *)((unsigned long)pl3e & PAGE_MASK);
+    ASSERT(!((unsigned long)pl3e & ~PAGE_MASK));
 
     /* 3rd L3 slot contains L2 with Xen-private mappings. It *must* exist. */
     l3e3 = pl3e[3];
     if ( !(l3e_get_flags(l3e3) & _PAGE_PRESENT) )
     {
         gdprintk(XENLOG_WARNING, "PAE L3 3rd slot is empty\n");
-        return 0;
+        return false;
     }
 
     /*
@@ -1443,15 +1444,23 @@ static int create_pae_xen_mappings(struc
      */
     page = l3e_get_page(l3e3);
     BUG_ON(page->u.inuse.type_info & PGT_pinned);
-    BUG_ON((page->u.inuse.type_info & PGT_count_mask) == 0);
     BUG_ON(!(page->u.inuse.type_info & PGT_pae_xen_l2));
     if ( (page->u.inuse.type_info & PGT_count_mask) != 1 )
     {
+        BUG_ON(!(page->u.inuse.type_info & PGT_count_mask));
         gdprintk(XENLOG_WARNING, "PAE L3 3rd slot is shared\n");
-        return 0;
+        return false;
     }
 
-    return 1;
+    return true;
+}
+
+void init_xen_pae_l2_slots(l2_pgentry_t *l2t, const struct domain *d)
+{
+    memcpy(&l2t[COMPAT_L2_PAGETABLE_FIRST_XEN_SLOT(d)],
+           &compat_idle_pg_table_l2[
+               l2_table_offset(HIRO_COMPAT_MPT_VIRT_START)],
+           COMPAT_L2_PAGETABLE_XEN_SLOTS(d) * sizeof(*l2t));
 }
 
 static int promote_l2_table(struct page_info *page, unsigned long type)
@@ -1621,7 +1630,7 @@ static int promote_l3_table(struct page_
         pl3e[i] = adjust_guest_l3e(l3e, d);
     }
 
-    if ( !rc && !create_pae_xen_mappings(d, pl3e) )
+    if ( !rc && !pae_xen_mappings_check(d, pl3e) )
         rc = -EINVAL;
     if ( rc < 0 && rc != -ERESTART && rc != -EINTR )
     {
@@ -1663,14 +1672,6 @@ static int promote_l3_table(struct page_
     unmap_domain_page(pl3e);
     return rc;
 }
-
-void init_xen_pae_l2_slots(l2_pgentry_t *l2t, const struct domain *d)
-{
-    memcpy(&l2t[COMPAT_L2_PAGETABLE_FIRST_XEN_SLOT(d)],
-           &compat_idle_pg_table_l2[
-               l2_table_offset(HIRO_COMPAT_MPT_VIRT_START)],
-           COMPAT_L2_PAGETABLE_XEN_SLOTS(d) * sizeof(*l2t));
-}
 #endif /* CONFIG_PV */
 
 /*
@@ -2349,10 +2350,6 @@ static int mod_l3_entry(l3_pgentry_t *pl
         return -EFAULT;
     }
 
-    if ( likely(rc == 0) )
-        if ( !create_pae_xen_mappings(d, pl3e) )
-            BUG();
-
     put_page_from_l3e(ol3e, mfn, PTF_defer);
     return rc;
 }


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel