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

[Xen-devel] [PATCH] x86: fix paging_log_dirty_op to work with paging guests



When the caller of paging_log_dirty_op is a paging mode guest Xen
would choke when trying to copy the dirty bitmap to the guest provided
buffer because the paging lock of the target is already locked, and
trying to lock the paging lock of the caller will cause the mm lock
order checks to trigger:

(XEN) mm locking order violation: 64 > 16
(XEN) Xen BUG at ./mm-locks.h:143
(XEN) ----[ Xen-4.12-unstable  x86_64  debug=y   Tainted:  C   ]----
(XEN) CPU:    4
(XEN) RIP:    e008:[<ffff82d080328581>] p2m.c#_mm_read_lock+0x41/0x50
(XEN) RFLAGS: 0000000000010286   CONTEXT: hypervisor (d0v3)
[...]
(XEN) Xen call trace:
(XEN)    [<ffff82d080328581>] p2m.c#_mm_read_lock+0x41/0x50
(XEN)    [<ffff82d080322ef6>] vmac.c#p2m_get_page_from_gfn+0x46/0x200
(XEN)    [<ffff82d08035249a>] vmac.c#hap_p2m_ga_to_gfn_4_levels+0x4a/0x270
(XEN)    [<ffff82d0802eb103>] vmac.c#hvm_translate_get_page+0x33/0x1c0
(XEN)    [<ffff82d0802eb35d>] hvm.c#__hvm_copy+0x8d/0x230
(XEN)    [<ffff82d0802eada6>] vmac.c#hvm_copy_to_guest_linear+0x46/0x60
(XEN)    [<ffff82d0802eb5c9>] vmac.c#copy_to_user_hvm+0x79/0x90
(XEN)    [<ffff82d0803314f9>] paging.c#paging_log_dirty_op+0x2c9/0x6d0
(XEN)    [<ffff82d08027384e>] vmac.c#arch_do_domctl+0x63e/0x1c00
(XEN)    [<ffff82d080205720>] vmac.c#do_domctl+0x450/0x1020
(XEN)    [<ffff82d0802ef929>] vmac.c#hvm_hypercall+0x1c9/0x4b0
(XEN)    [<ffff82d080313161>] vmac.c#vmx_vmexit_handler+0x6c1/0xea0
(XEN)    [<ffff82d08031a84a>] vmac.c#vmx_asm_vmexit_handler+0xfa/0x270
(XEN)
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 4:
(XEN) Xen BUG at ./mm-locks.h:143
(XEN) ****************************************

Fix this by releasing the target paging lock before attempting to
perform the copy of the dirty bitmap, and then forcing a restart of
the whole process in case there have been changes to the dirty bitmap
tables.

Note that the path for non-paging guests remains the same, and the
paging lock is not dropped in that case.

Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
Cc: Jan Beulich <jbeulich@xxxxxxxx>
Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
---
 xen/arch/x86/mm/paging.c     | 37 +++++++++++++++++++++++++++++++++---
 xen/include/asm-x86/domain.h |  1 +
 2 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/mm/paging.c b/xen/arch/x86/mm/paging.c
index d5836eb688..752f827f38 100644
--- a/xen/arch/x86/mm/paging.c
+++ b/xen/arch/x86/mm/paging.c
@@ -408,6 +408,7 @@ static int paging_log_dirty_op(struct domain *d,
     unsigned long *l1 = NULL;
     int i4, i3, i2;
 
+ again:
     if ( !resuming )
     {
         /*
@@ -468,17 +469,18 @@ static int paging_log_dirty_op(struct domain *d,
     l4 = paging_map_log_dirty_bitmap(d);
     i4 = d->arch.paging.preempt.log_dirty.i4;
     i3 = d->arch.paging.preempt.log_dirty.i3;
+    i2 = d->arch.paging.preempt.log_dirty.i2;
     pages = d->arch.paging.preempt.log_dirty.done;
 
     for ( ; (pages < sc->pages) && (i4 < LOGDIRTY_NODE_ENTRIES); i4++, i3 = 0 )
     {
         l3 = (l4 && mfn_valid(l4[i4])) ? map_domain_page(l4[i4]) : NULL;
-        for ( ; (pages < sc->pages) && (i3 < LOGDIRTY_NODE_ENTRIES); i3++ )
+        for ( ; (pages < sc->pages) && (i3 < LOGDIRTY_NODE_ENTRIES);
+              i3++, i2 = 0 )
         {
             l2 = ((l3 && mfn_valid(l3[i3])) ?
                   map_domain_page(l3[i3]) : NULL);
-            for ( i2 = 0;
-                  (pages < sc->pages) && (i2 < LOGDIRTY_NODE_ENTRIES);
+            for ( ; (pages < sc->pages) && (i2 < LOGDIRTY_NODE_ENTRIES);
                   i2++ )
             {
                 unsigned int bytes = PAGE_SIZE;
@@ -488,6 +490,16 @@ static int paging_log_dirty_op(struct domain *d,
                     bytes = (unsigned int)((sc->pages - pages + 7) >> 3);
                 if ( likely(peek) )
                 {
+                    if ( paging_mode_enabled(current->domain) )
+                        /*
+                         * Drop the target p2m lock, or else Xen will panic
+                         * when trying to acquire the p2m lock of the caller
+                         * due to invalid lock order. Note that there are no
+                         * lock ordering issues here, and the panic is due to
+                         * the fact that the lock level tracking doesn't record
+                         * the domain the lock belongs to.
+                         */
+                        paging_unlock(d);
                     if ( (l1 ? copy_to_guest_offset(sc->dirty_bitmap,
                                                     pages >> 3, (uint8_t *)l1,
                                                     bytes)
@@ -505,6 +517,23 @@ static int paging_log_dirty_op(struct domain *d,
                         clear_page(l1);
                     unmap_domain_page(l1);
                 }
+                if ( paging_mode_enabled(current->domain) && peek )
+                {
+                    d->arch.paging.preempt.log_dirty.i4 = i4;
+                    d->arch.paging.preempt.log_dirty.i3 = i3;
+                    d->arch.paging.preempt.log_dirty.i2 = i2 + 1;
+                    d->arch.paging.preempt.log_dirty.done = pages;
+                    d->arch.paging.preempt.dom = current->domain;
+                    d->arch.paging.preempt.op = sc->op;
+                    resuming = 1;
+                    if ( l2 )
+                        unmap_domain_page(l2);
+                    if ( l3 )
+                        unmap_domain_page(l3);
+                    if ( l4 )
+                        unmap_domain_page(l4);
+                    goto again;
+                }
             }
             if ( l2 )
                 unmap_domain_page(l2);
@@ -513,6 +542,7 @@ static int paging_log_dirty_op(struct domain *d,
             {
                 d->arch.paging.preempt.log_dirty.i4 = i4;
                 d->arch.paging.preempt.log_dirty.i3 = i3 + 1;
+                d->arch.paging.preempt.log_dirty.i2 = 0;
                 rv = -ERESTART;
                 break;
             }
@@ -525,6 +555,7 @@ static int paging_log_dirty_op(struct domain *d,
         {
             d->arch.paging.preempt.log_dirty.i4 = i4 + 1;
             d->arch.paging.preempt.log_dirty.i3 = 0;
+            d->arch.paging.preempt.log_dirty.i2 = 0;
             rv = -ERESTART;
         }
         if ( rv )
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 277f99f633..b3e527cd51 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -212,6 +212,7 @@ struct paging_domain {
                 unsigned long done:PADDR_BITS - PAGE_SHIFT;
                 unsigned long i4:PAGETABLE_ORDER;
                 unsigned long i3:PAGETABLE_ORDER;
+                unsigned long i2:PAGETABLE_ORDER;
             } log_dirty;
         };
     } preempt;
-- 
2.19.2


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

 


Rackspace

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