[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH] Re: [PATCH 6 of 9] x86/mm: Rework stale p2m auditing



At 08:28 +0000 on 02 Dec (1322814505), Jan Beulich wrote:
>  >>> On 29.11.11 at 21:21, Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx> 
> wrote:
> > @@ -559,13 +560,15 @@ int set_p2m_entry(struct p2m_domain *p2m
> >  extern void p2m_pt_init(struct p2m_domain *p2m);
> >  
> >  /* Debugging and auditing of the P2M code? */
> > -#define P2M_AUDIT     0
> > +#define P2M_AUDIT     1
> >  #define P2M_DEBUGGING 0
> >  
> >  #if P2M_AUDIT
> 
> Was this change of the default really intended?

I think so - now that the ausdit code is not on any critical paths there's
no reason not to have it available by default.

> It uncovered a problem
> in 22356:cbb6b4b17024 (the code meanwhile moved elsewhere): In a
> 32-bit build,
> 
>         if ( gfn == 0x55555555 || gfn == 0x5555555555555555 )
> 
> causes (at least with some compiler versions, didn't check tonight's
> stage tester results yet) a compiler warning 

I don't think these debug values are serving any purpose.  How about we
just get rid of them? 

x86/mm: remove 0x55 debug pattern from M2P table

It's not really any more useful than explicitly setting new M2P entries
to the invalid value.

Signed-off-by: Tim Deegan <tim@xxxxxxx>

diff -r 60d4e257d04b tools/libxc/xc_domain.c
--- a/tools/libxc/xc_domain.c   Fri Dec 02 09:05:26 2011 +0100
+++ b/tools/libxc/xc_domain.c   Fri Dec 02 10:19:18 2011 +0000
@@ -1474,8 +1474,7 @@ int xc_domain_debug_control(xc_interface
 
 int xc_domain_p2m_audit(xc_interface *xch, 
                         uint32_t domid,
-                        uint64_t *orphans_debug,
-                        uint64_t *orphans_invalid,
+                        uint64_t *orphans,
                         uint64_t *m2p_bad,   
                         uint64_t *p2m_bad)
 {
@@ -1486,10 +1485,9 @@ int xc_domain_p2m_audit(xc_interface *xc
     domctl.domain = domid;
     rc = do_domctl(xch, &domctl);
 
-    *orphans_debug      = domctl.u.audit_p2m.orphans_debug;
-    *orphans_invalid    = domctl.u.audit_p2m.orphans_invalid;
-    *m2p_bad            = domctl.u.audit_p2m.m2p_bad;
-    *p2m_bad            = domctl.u.audit_p2m.p2m_bad;
+    *orphans = domctl.u.audit_p2m.orphans;
+    *m2p_bad = domctl.u.audit_p2m.m2p_bad;
+    *p2m_bad = domctl.u.audit_p2m.p2m_bad;
 
     return rc;
 }
diff -r 60d4e257d04b tools/libxc/xenctrl.h
--- a/tools/libxc/xenctrl.h     Fri Dec 02 09:05:26 2011 +0100
+++ b/tools/libxc/xenctrl.h     Fri Dec 02 10:19:18 2011 +0000
@@ -718,9 +718,7 @@ int xc_domain_setdebugging(xc_interface 
  * @parm xch a handle to an open hypervisor interface
  * @parm domid the domain id whose top level p2m we 
  *       want to audit
- * @parm orphans_debug count of m2p entries for valid
- *       domain pages containing a debug value
- * @parm orphans_invalid count of m2p entries for valid
+ * @parm orphans count of m2p entries for valid
  *       domain pages containing an invalid value
  * @parm m2p_bad count of m2p entries mismatching the
  *       associated p2m entry for this domain
@@ -732,9 +730,8 @@ int xc_domain_setdebugging(xc_interface 
  *          -EFAULT: could not copy results back to guest
  */
 int xc_domain_p2m_audit(xc_interface *xch,
-                                       uint32_t domid,
-                        uint64_t *orphans_debug,
-                        uint64_t *orphans_invalid,
+                        uint32_t domid,
+                        uint64_t *orphans,
                         uint64_t *m2p_bad,   
                         uint64_t *p2m_bad);
 
diff -r 60d4e257d04b xen/arch/x86/domctl.c
--- a/xen/arch/x86/domctl.c     Fri Dec 02 09:05:26 2011 +0100
+++ b/xen/arch/x86/domctl.c     Fri Dec 02 10:19:18 2011 +0000
@@ -1459,8 +1459,7 @@ long arch_do_domctl(
             break;
 
         audit_p2m(d,
-                  &domctl->u.audit_p2m.orphans_debug,
-                  &domctl->u.audit_p2m.orphans_invalid,
+                  &domctl->u.audit_p2m.orphans,
                   &domctl->u.audit_p2m.m2p_bad,
                   &domctl->u.audit_p2m.p2m_bad);
         rcu_unlock_domain(d);
diff -r 60d4e257d04b xen/arch/x86/mm/p2m.c
--- a/xen/arch/x86/mm/p2m.c     Fri Dec 02 09:05:26 2011 +0100
+++ b/xen/arch/x86/mm/p2m.c     Fri Dec 02 10:19:18 2011 +0000
@@ -307,13 +307,7 @@ int p2m_alloc_table(struct p2m_domain *p
             /* Pages should not be shared that early */
             ASSERT(gfn != SHARED_M2P_ENTRY);
             page_count++;
-            if (
-#ifdef __x86_64__
-                (gfn != 0x5555555555555555L)
-#else
-                (gfn != 0x55555555L)
-#endif
-                && gfn != INVALID_M2P_ENTRY
+            if ( gfn != INVALID_M2P_ENTRY
                 && !set_p2m_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2m_ram_rw, 
p2m->default_access) )
                 goto error_unlock;
         }
@@ -513,14 +507,7 @@ guest_physmap_add_entry(struct domain *d
         if ( page_get_owner(mfn_to_page(_mfn(mfn + i))) != d )
             continue;
         ogfn = mfn_to_gfn(d, _mfn(mfn+i));
-        if (
-#ifdef __x86_64__
-            (ogfn != 0x5555555555555555L)
-#else
-            (ogfn != 0x55555555L)
-#endif
-            && (ogfn != INVALID_M2P_ENTRY)
-            && (ogfn != gfn + i) )
+        if ( (ogfn != INVALID_M2P_ENTRY) && (ogfn != gfn + i) )
         {
             /* This machine frame is already mapped at another physical
              * address */
@@ -1447,8 +1434,7 @@ unsigned long paging_gva_to_gfn(struct v
 
 #if P2M_AUDIT
 void audit_p2m(struct domain *d,
-                uint64_t *orphans_debug,
-                uint64_t *orphans_invalid,
+               uint64_t *orphans,
                 uint64_t *m2p_bad,
                 uint64_t *p2m_bad)
 {
@@ -1456,7 +1442,7 @@ void audit_p2m(struct domain *d,
     struct domain *od;
     unsigned long mfn, gfn;
     mfn_t p2mfn;
-    unsigned long orphans_d = 0, orphans_i = 0, mpbad = 0, pmbad = 0;
+    unsigned long orphans_count = 0, mpbad = 0, pmbad = 0;
     p2m_access_t p2ma;
     p2m_type_t type;
     struct p2m_domain *p2m = p2m_get_hostp2m(d);
@@ -1492,20 +1478,12 @@ void audit_p2m(struct domain *d,
         gfn = get_gpfn_from_mfn(mfn);
         if ( gfn == INVALID_M2P_ENTRY )
         {
-            orphans_i++;
+            orphans_count++;
             P2M_PRINTK("orphaned guest page: mfn=%#lx has invalid gfn\n",
                            mfn);
             continue;
         }
 
-        if ( gfn == 0x55555555 || gfn == 0x5555555555555555 )
-        {
-            orphans_d++;
-            P2M_PRINTK("orphaned guest page: mfn=%#lx has debug gfn\n",
-                           mfn);
-            continue;
-        }
-
         if ( gfn == SHARED_M2P_ENTRY )
         {
             P2M_PRINTK("shared mfn (%lx) on domain page list!\n",
@@ -1538,9 +1516,8 @@ void audit_p2m(struct domain *d,
     p2m_unlock(p2m);
  
     P2M_PRINTK("p2m audit complete\n");
-    if ( orphans_i | orphans_d | mpbad | pmbad )
-        P2M_PRINTK("p2m audit found %lu orphans (%lu inval %lu debug)\n",
-                       orphans_i + orphans_d, orphans_i, orphans_d);
+    if ( orphans_count | mpbad | pmbad )
+        P2M_PRINTK("p2m audit found %lu orphans\n", orphans);
     if ( mpbad | pmbad )
     {
         P2M_PRINTK("p2m audit found %lu odd p2m, %lu bad m2p entries\n",
@@ -1549,10 +1526,9 @@ void audit_p2m(struct domain *d,
     }
 
 out_p2m_audit:
-    *orphans_debug      = (uint64_t) orphans_d;
-    *orphans_invalid    = (uint64_t) orphans_i;
-    *m2p_bad            = (uint64_t) mpbad;
-    *p2m_bad            = (uint64_t) pmbad;
+    *orphans = (uint64_t) orphans_count;
+    *m2p_bad = (uint64_t) mpbad;
+    *p2m_bad = (uint64_t) pmbad;
 }
 #endif /* P2M_AUDIT */
 
diff -r 60d4e257d04b xen/arch/x86/x86_32/mm.c
--- a/xen/arch/x86/x86_32/mm.c  Fri Dec 02 09:05:26 2011 +0100
+++ b/xen/arch/x86/x86_32/mm.c  Fri Dec 02 10:19:18 2011 +0000
@@ -114,8 +114,8 @@ void __init paging_init(void)
         l2e_write(&idle_pg_table_l2[l2_linear_offset(RO_MPT_VIRT_START) + i],
                   l2e_from_page(
                       pg, (__PAGE_HYPERVISOR | _PAGE_PSE) & ~_PAGE_RW));
-        /* Fill with an obvious debug pattern. */
-        memset((void *)(RDWR_MPT_VIRT_START + (i << L2_PAGETABLE_SHIFT)), 0x55,
+        /* Fill with INVALID_M2P_ENTRY. */
+        memset((void *)(RDWR_MPT_VIRT_START + (i << L2_PAGETABLE_SHIFT)), 0xFF,
                1UL << L2_PAGETABLE_SHIFT);
     }
 #undef CNT
diff -r 60d4e257d04b xen/arch/x86/x86_64/mm.c
--- a/xen/arch/x86/x86_64/mm.c  Fri Dec 02 09:05:26 2011 +0100
+++ b/xen/arch/x86/x86_64/mm.c  Fri Dec 02 10:19:18 2011 +0000
@@ -495,7 +495,8 @@ static int setup_compat_m2p_table(struct
                                PAGE_HYPERVISOR);
         if ( err )
             break;
-        memset((void *)rwva, 0x55, 1UL << L2_PAGETABLE_SHIFT);
+        /* Fill with INVALID_M2P_ENTRY. */
+        memset((void *)rwva, 0xFF, 1UL << L2_PAGETABLE_SHIFT);
         /* NB. Cannot be GLOBAL as the ptes get copied into per-VM space. */
         l2e_write(&l2_ro_mpt[l2_table_offset(va)], l2e_from_page(l1_pg, 
_PAGE_PSE|_PAGE_PRESENT));
     }
@@ -569,8 +570,9 @@ static int setup_m2p_table(struct mem_ho
                         PAGE_HYPERVISOR);
             if ( ret )
                 goto error;
+            /* Fill with INVALID_M2P_ENTRY. */
             memset((void *)(RDWR_MPT_VIRT_START + i * sizeof(unsigned long)),
-                   0x55, 1UL << L2_PAGETABLE_SHIFT);
+                   0xFF, 1UL << L2_PAGETABLE_SHIFT);
 
             ASSERT(!(l3e_get_flags(l3_ro_mpt[l3_table_offset(va)]) &
                   _PAGE_PSE));
@@ -727,8 +729,9 @@ void __init paging_init(void)
                 page_to_mfn(l1_pg),
                 1UL << PAGETABLE_ORDER,
                 PAGE_HYPERVISOR);
+            /* Fill with INVALID_M2P_ENTRY. */
             memset((void *)(RDWR_MPT_VIRT_START + (i << L2_PAGETABLE_SHIFT)),
-                   0x55, 1UL << L2_PAGETABLE_SHIFT);
+                   0xFF, 1UL << L2_PAGETABLE_SHIFT);
         }
         if ( !((unsigned long)l2_ro_mpt & ~PAGE_MASK) )
         {
diff -r 60d4e257d04b xen/include/asm-x86/p2m.h
--- a/xen/include/asm-x86/p2m.h Fri Dec 02 09:05:26 2011 +0100
+++ b/xen/include/asm-x86/p2m.h Fri Dec 02 10:19:18 2011 +0000
@@ -566,10 +566,9 @@ extern void p2m_pt_init(struct p2m_domai
 
 #if P2M_AUDIT
 extern void audit_p2m(struct domain *d,
-                        uint64_t *orphans_debug,
-                        uint64_t *orphans_invalid,
-                        uint64_t *m2p_bad,
-                        uint64_t *p2m_bad);
+                      uint64_t *orphans,
+                      uint64_t *m2p_bad,
+                      uint64_t *p2m_bad);
 #endif /* P2M_AUDIT */
 
 /* Printouts */
diff -r 60d4e257d04b xen/include/public/domctl.h
--- a/xen/include/public/domctl.h       Fri Dec 02 09:05:26 2011 +0100
+++ b/xen/include/public/domctl.h       Fri Dec 02 10:19:18 2011 +0000
@@ -806,8 +806,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_mem_s
 
 struct xen_domctl_audit_p2m {
     /* OUT error counts */
-    uint64_t orphans_debug;
-    uint64_t orphans_invalid;
+    uint64_t orphans;
     uint64_t m2p_bad;
     uint64_t p2m_bad;
 };

Attachment: x
Description: Text document

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.