On 06/22/11 18:10, Tim Deegan wrote:
# HG changeset patch
# User Tim Deegan<Tim.Deegan@xxxxxxxxxx>
# Date 1308758840 -3600
# Node ID a168af9b6d2f604c841465e5ed911b7a12b5ba15
# Parent b265371addbbc8a58c95a269fe3cd0fdc866aaa3
Nested p2m: rework locking around nested-p2m flushes and updates.
The nestedp2m_lock now only covers the mapping from nested-cr3 to
nested-p2m; the tables themselves may be updated or flushed using only
the relevant p2m lock.
This means that the nested-p2m lock is only taken on one path, and
always before any p2m locks.
Signed-off-by: Tim Deegan<Tim.Deegan@xxxxxxxxxx>
diff -r b265371addbb -r a168af9b6d2f xen/arch/x86/mm/hap/nested_hap.c
--- a/xen/arch/x86/mm/hap/nested_hap.c Wed Jun 22 17:04:08 2011 +0100
+++ b/xen/arch/x86/mm/hap/nested_hap.c Wed Jun 22 17:07:20 2011 +0100
@@ -96,17 +96,23 @@ nestedp2m_write_p2m_entry(struct p2m_dom
/* NESTED VIRT FUNCTIONS */
/********************************************/
static void
-nestedhap_fix_p2m(struct p2m_domain *p2m, paddr_t L2_gpa, paddr_t L0_gpa,
- p2m_type_t p2mt, p2m_access_t p2ma)
+nestedhap_fix_p2m(struct vcpu *v, struct p2m_domain *p2m,
+ paddr_t L2_gpa, paddr_t L0_gpa,
+ p2m_type_t p2mt, p2m_access_t p2ma)
{
- int rv;
+ int rv = 0;
ASSERT(p2m);
ASSERT(p2m->set_entry);
p2m_lock(p2m);
- rv = set_p2m_entry(p2m, L2_gpa>> PAGE_SHIFT,
- page_to_mfn(maddr_to_page(L0_gpa)),
- 0 /*4K*/, p2mt, p2ma);
+
+ /* If this p2m table has been flushed or recycled under our feet,
+ * leave it alone. We'll pick up the right one as we try to
+ * vmenter the guest. */
+ if ( p2m->cr3 == nhvm_vcpu_hostcr3(v) )
+ rv = set_p2m_entry(p2m, L2_gpa>> PAGE_SHIFT,
+ page_to_mfn(maddr_to_page(L0_gpa)),
+ 0 /*4K*/, p2mt, p2ma);
The introduction of this check leads to this crash when the L2 guest
initializes its third vcpu:
(XEN) nested_hap.c:121:d1 failed to set entry for 0xebcff0 -> 0x2654bcff0
(XEN) Xen BUG at nested_hap.c:122
(XEN) ----[ Xen-4.2-unstable x86_64 debug=y Tainted: C ]----
(XEN) CPU: 1
(XEN) RIP: e008:[<ffff82c48020aa70>]
nestedhvm_hap_nested_page_fault+0x27f/0x29d
(XEN) RFLAGS: 0000000000010292 CONTEXT: hypervisor
(XEN) rax: 0000000000000000 rbx: 0000000000000000 rcx: 0000000000000001
(XEN) rdx: ffff82c48025b2e8 rsi: 000000000000000a rdi: ffff82c48025b2e8
(XEN) rbp: ffff830425217cf8 rsp: ffff830425217ca8 r8: 0000000000000001
(XEN) r9: 0000000000000004 r10: 0000000000000004 r11: 0000000000000004
(XEN) r12: ffff8300cf2a1000 r13: ffff830421db1d90 r14: ffff830421db1d90
(XEN) r15: ffff830421db1d90 cr0: 000000008005003b cr4: 00000000000406f0
(XEN) cr3: 0000000403b02000 cr2: 0000000000000000
(XEN) ds: 0000 es: 0000 fs: 0010 gs: 0010 ss: 0000 cs: e008
(XEN) Xen stack trace from rsp=ffff830425217ca8:
(XEN) ffff8300cf2a1000 0000000000ebcff0 00000002654bcff0 00000002654bcff0
(XEN) 00000000d98bcff0 0000000000000000 ffff8300cf2a1000 0000000000ebcff0
(XEN) ffff8300cf2a1000 0000000000ebcff0 ffff830425217d58 ffff82c4801aa78d
(XEN) 00000000cf2a1000 ffffffffffffffff 00ff830425217d28 ffff82c4801a66ad
(XEN) ffff830425217d58 0000000000000000 0000000000000ebc 0000000000ebcff0
(XEN) ffff8300cf2a1000 ffff83023ec07000 ffff830425217f08 ffff82c4801c0faa
(XEN) ffff830400000000 ffff82c48017638e ffff830425217d90 ffff830421db1d90
(XEN) ffff830425217f18<G><0>nested_hap.c:121:d1 failed to set entry
for 0x7ff8 ->
0x2644d1ff8
(XEN) 0100000000000293Xen BUG at nested_hap.c:122
Christoph
p2m_unlock(p2m);
if (rv == 0) {
@@ -211,12 +217,10 @@ nestedhvm_hap_nested_page_fault(struct v
break;
}
- nestedp2m_lock(d);
/* fix p2m_get_pagetable(nested_p2m) */
- nestedhap_fix_p2m(nested_p2m, L2_gpa, L0_gpa,
+ nestedhap_fix_p2m(v, nested_p2m, L2_gpa, L0_gpa,
p2m_ram_rw,
p2m_access_rwx /* FIXME: Should use same permission as l1 guest */);
- nestedp2m_unlock(d);
return NESTEDHVM_PAGEFAULT_DONE;
}
diff -r b265371addbb -r a168af9b6d2f xen/arch/x86/mm/mm-locks.h
--- a/xen/arch/x86/mm/mm-locks.h Wed Jun 22 17:04:08 2011 +0100
+++ b/xen/arch/x86/mm/mm-locks.h Wed Jun 22 17:07:20 2011 +0100
@@ -96,8 +96,11 @@ declare_mm_lock(shr)
/* Nested P2M lock (per-domain)
*
- * A per-domain lock that protects some of the nested p2m datastructures.
- * TODO: find out exactly what needs to be covered by this lock */
+ * A per-domain lock that protects the mapping from nested-CR3 to
+ * nested-p2m. In particular it covers:
+ * - the array of nested-p2m tables, and all LRU activity therein; and
+ * - setting the "cr3" field of any p2m table to a non-CR3_EADDR value.
+ * (i.e. assigning a p2m table to be the shadow of that cr3 */
declare_mm_lock(nestedp2m)
#define nestedp2m_lock(d) mm_lock(nestedp2m,&(d)->arch.nested_p2m_lock)
diff -r b265371addbb -r a168af9b6d2f xen/arch/x86/mm/p2m.c
--- a/xen/arch/x86/mm/p2m.c Wed Jun 22 17:04:08 2011 +0100
+++ b/xen/arch/x86/mm/p2m.c Wed Jun 22 17:07:20 2011 +0100
@@ -1052,7 +1052,7 @@ p2m_getlru_nestedp2m(struct domain *d, s
/* Reset this p2m table to be empty */
static void
-p2m_flush_locked(struct p2m_domain *p2m)
+p2m_flush_table(struct p2m_domain *p2m)
{
struct page_info *top, *pg;
struct domain *d = p2m->domain;
@@ -1094,21 +1094,16 @@ p2m_flush(struct vcpu *v, struct p2m_dom
ASSERT(v->domain == d);
vcpu_nestedhvm(v).nv_p2m = NULL;
- nestedp2m_lock(d);
- p2m_flush_locked(p2m);
+ p2m_flush_table(p2m);
hvm_asid_flush_vcpu(v);
- nestedp2m_unlock(d);
}
void
p2m_flush_nestedp2m(struct domain *d)
{
int i;
-
- nestedp2m_lock(d);
for ( i = 0; i< MAX_NESTEDP2M; i++ )
- p2m_flush_locked(d->arch.nested_p2m[i]);
- nestedp2m_unlock(d);
+ p2m_flush_table(d->arch.nested_p2m[i]);
}
struct p2m_domain *
@@ -1132,17 +1127,23 @@ p2m_get_nestedp2m(struct vcpu *v, uint64
d = v->domain;
nestedp2m_lock(d);
p2m = nv->nv_p2m;
- if ( p2m&& (p2m->cr3 == cr3 || p2m->cr3 == CR3_EADDR) )
+ if ( p2m )
{
- nv->nv_flushp2m = 0;
- p2m_getlru_nestedp2m(d, p2m);
- nv->nv_p2m = p2m;
- if (p2m->cr3 == CR3_EADDR)
- hvm_asid_flush_vcpu(v);
- p2m->cr3 = cr3;
- cpu_set(v->processor, p2m->p2m_dirty_cpumask);
- nestedp2m_unlock(d);
- return p2m;
+ p2m_lock(p2m);
+ if ( p2m->cr3 == cr3 || p2m->cr3 == CR3_EADDR )
+ {
+ nv->nv_flushp2m = 0;
+ p2m_getlru_nestedp2m(d, p2m);
+ nv->nv_p2m = p2m;
+ if (p2m->cr3 == CR3_EADDR)
+ hvm_asid_flush_vcpu(v);
+ p2m->cr3 = cr3;
+ cpu_set(v->processor, p2m->p2m_dirty_cpumask);
+ p2m_unlock(p2m);
+ nestedp2m_unlock(d);
+ return p2m;
+ }
+ p2m_unlock(p2m)
}
/* All p2m's are or were in use. Take the least recent used one,
@@ -1150,14 +1151,16 @@ p2m_get_nestedp2m(struct vcpu *v, uint64
*/
for (i = 0; i< MAX_NESTEDP2M; i++) {
p2m = p2m_getlru_nestedp2m(d, NULL);
- p2m_flush_locked(p2m);
+ p2m_flush_table(p2m);
}
+ p2m_lock(p2m);
nv->nv_p2m = p2m;
p2m->cr3 = cr3;
nv->nv_flushp2m = 0;
hvm_asid_flush_vcpu(v);
nestedhvm_vmcx_flushtlb(nv->nv_p2m);
cpu_set(v->processor, p2m->p2m_dirty_cpumask);
+ p2m_unlock(p2m);
nestedp2m_unlock(d);
return p2m;
diff -r b265371addbb -r a168af9b6d2f xen/include/asm-x86/p2m.h
--- a/xen/include/asm-x86/p2m.h Wed Jun 22 17:04:08 2011 +0100
+++ b/xen/include/asm-x86/p2m.h Wed Jun 22 17:07:20 2011 +0100
@@ -201,8 +201,13 @@ struct p2m_domain {
cpumask_t p2m_dirty_cpumask;
struct domain *domain; /* back pointer to domain */
+
+ /* Nested p2ms only: nested-CR3 value that this p2m shadows.
+ * This can be cleared to CR3_EADDR under the per-p2m lock but
+ * needs both the per-p2m lock and the per-domain nestedp2m lock
+ * to set it to any other value. */
#define CR3_EADDR (~0ULL)
- uint64_t cr3; /* to identify this p2m for re-use */
+ uint64_t cr3;
/* Pages used to construct the p2m */
struct page_list_head pages;
--
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home