[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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |