* [PATCH] nestedsvm: fix l2 guest display refresh issue
@ 2012-07-06 13:36 Christoph Egger
2012-07-12 11:26 ` Tim Deegan
0 siblings, 1 reply; 2+ messages in thread
From: Christoph Egger @ 2012-07-06 13:36 UTC (permalink / raw)
To: xen-devel@lists.xen.org, Tim Deegan
[-- Attachment #1: Type: text/plain, Size: 266 bytes --]
--
---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
[-- Attachment #2: xen_nh_p2m.diff --]
[-- Type: text/plain, Size: 24185 bytes --]
# HG changeset patch
# User Christoph Egger
# Date 1341572213 -7200
Fix l2 guest refresh problem.
When l2 guest does a write access the l1 hypervisor
mapped read only then inject VMEXIT(#NPF) into l1
hypervisor.
When l2 guest does a write access the host mapped
as read only then let the host handle the NPF.
Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
diff -r 5fdb3f48b970 -r d71ce8cd7e8a xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1278,7 +1278,8 @@ int hvm_hap_nested_page_fault(unsigned l
* into l1 guest if not fixable. The algorithm is
* the same as for shadow paging.
*/
- rv = nestedhvm_hap_nested_page_fault(v, gpa);
+ rv = nestedhvm_hap_nested_page_fault(v, gpa,
+ access_r, access_w, access_x);
switch (rv) {
case NESTEDHVM_PAGEFAULT_DONE:
return 1;
@@ -1290,6 +1291,8 @@ int hvm_hap_nested_page_fault(unsigned l
if ( !handle_mmio() )
hvm_inject_hw_exception(TRAP_gp_fault, 0);
return 1;
+ case NESTEDHVM_PAGEFAULT_READONLY:
+ break;
}
}
diff -r 5fdb3f48b970 -r d71ce8cd7e8a xen/arch/x86/mm/hap/guest_walk.c
--- a/xen/arch/x86/mm/hap/guest_walk.c
+++ b/xen/arch/x86/mm/hap/guest_walk.c
@@ -43,12 +43,13 @@ unsigned long hap_gva_to_gfn(GUEST_PAGIN
struct vcpu *v, struct p2m_domain *p2m, unsigned long gva, uint32_t *pfec)
{
unsigned long cr3 = v->arch.hvm_vcpu.guest_cr[3];
- return hap_p2m_ga_to_gfn(GUEST_PAGING_LEVELS)(v, p2m, cr3, gva, pfec, NULL);
+ return hap_p2m_ga_to_gfn(GUEST_PAGING_LEVELS)(v, p2m, cr3,
+ gva, NULL, pfec, NULL);
}
unsigned long hap_p2m_ga_to_gfn(GUEST_PAGING_LEVELS)(
struct vcpu *v, struct p2m_domain *p2m, unsigned long cr3,
- paddr_t ga, uint32_t *pfec, unsigned int *page_order)
+ paddr_t ga, unsigned long *flags, uint32_t *pfec, unsigned int *page_order)
{
uint32_t missing;
mfn_t top_mfn;
@@ -58,6 +59,9 @@ unsigned long hap_p2m_ga_to_gfn(GUEST_PA
unsigned long top_gfn;
struct page_info *top_page;
+ if ( flags )
+ *flags = 0;
+
/* Get the top-level table's MFN */
top_gfn = cr3 >> PAGE_SHIFT;
top_page = get_page_from_gfn_p2m(p2m->domain, p2m, top_gfn,
@@ -120,6 +124,9 @@ unsigned long hap_p2m_ga_to_gfn(GUEST_PA
if ( page_order )
*page_order = guest_walk_to_page_order(&gw);
+ if ( flags )
+ *flags = guest_walk_to_flags(&gw);
+
return gfn_x(gfn);
}
diff -r 5fdb3f48b970 -r d71ce8cd7e8a xen/arch/x86/mm/hap/hap.c
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -908,10 +908,12 @@ static unsigned long hap_gva_to_gfn_real
static unsigned long hap_p2m_ga_to_gfn_real_mode(
struct vcpu *v, struct p2m_domain *p2m, unsigned long cr3,
- paddr_t ga, uint32_t *pfec, unsigned int *page_order)
+ paddr_t ga, unsigned long *flags, uint32_t *pfec, unsigned int *page_order)
{
if ( page_order )
*page_order = PAGE_ORDER_4K;
+ if ( flags )
+ *flags = (_PAGE_PRESENT|_PAGE_RW|_PAGE_USER);
return (ga >> PAGE_SHIFT);
}
diff -r 5fdb3f48b970 -r d71ce8cd7e8a xen/arch/x86/mm/hap/nested_hap.c
--- a/xen/arch/x86/mm/hap/nested_hap.c
+++ b/xen/arch/x86/mm/hap/nested_hap.c
@@ -74,6 +74,7 @@
#define mfn_valid(_mfn) __mfn_valid(mfn_x(_mfn))
#undef page_to_mfn
#define page_to_mfn(_pg) _mfn(__page_to_mfn(_pg))
+#define _PAGE_NX (1ULL<<63)
void
nestedp2m_write_p2m_entry(struct p2m_domain *p2m, unsigned long gfn,
@@ -141,23 +142,27 @@ nestedhap_fix_p2m(struct vcpu *v, struct
*/
static int
nestedhap_walk_L0_p2m(struct p2m_domain *p2m, paddr_t L1_gpa, paddr_t *L0_gpa,
- unsigned int *page_order)
+ p2m_type_t *p2mt, p2m_access_t *p2ma,
+ unsigned int *page_order,
+ bool_t access_r, bool_t access_w, bool_t access_x)
{
mfn_t mfn;
- p2m_type_t p2mt;
- p2m_access_t p2ma;
int rc;
/* walk L0 P2M table */
- mfn = get_gfn_type_access(p2m, L1_gpa >> PAGE_SHIFT, &p2mt, &p2ma,
+ mfn = get_gfn_type_access(p2m, L1_gpa >> PAGE_SHIFT, p2mt, p2ma,
0, page_order);
rc = NESTEDHVM_PAGEFAULT_MMIO;
- if ( p2m_is_mmio(p2mt) )
+ if ( p2m_is_mmio(*p2mt) )
+ goto out;
+
+ rc = NESTEDHVM_PAGEFAULT_READONLY;
+ if (access_w && p2m_is_readonly(*p2mt))
goto out;
rc = NESTEDHVM_PAGEFAULT_ERROR;
- if ( p2m_is_paging(p2mt) || p2m_is_shared(p2mt) || !p2m_is_ram(p2mt) )
+ if ( p2m_is_paging(*p2mt) || p2m_is_shared(*p2mt) || !p2m_is_ram(*p2mt) )
goto out;
rc = NESTEDHVM_PAGEFAULT_ERROR;
@@ -177,19 +182,25 @@ out:
*/
static int
nestedhap_walk_L1_p2m(struct vcpu *v, paddr_t L2_gpa, paddr_t *L1_gpa,
- unsigned int *page_order)
+ unsigned long *flags, unsigned int *page_order,
+ uint32_t *pfec,
+ bool_t access_r, bool_t access_w, bool_t access_x)
{
- uint32_t pfec;
unsigned long nested_cr3, gfn;
nested_cr3 = nhvm_vcpu_hostcr3(v);
/* Walk the guest-supplied NPT table, just as if it were a pagetable */
- gfn = paging_ga_to_gfn_cr3(v, nested_cr3, L2_gpa, &pfec, page_order);
+ gfn = paging_ga_to_gfn_cr3(v, nested_cr3, L2_gpa, flags, pfec, page_order);
if ( gfn == INVALID_GFN )
return NESTEDHVM_PAGEFAULT_INJECT;
+ if (access_x && (*flags & _PAGE_NX))
+ return NESTEDHVM_PAGEFAULT_INJECT;
+ if (access_w && !(*flags & _PAGE_RW))
+ return NESTEDHVM_PAGEFAULT_INJECT;
+
*L1_gpa = (gfn << PAGE_SHIFT) + (L2_gpa & ~PAGE_MASK);
return NESTEDHVM_PAGEFAULT_DONE;
}
@@ -200,19 +211,25 @@ nestedhap_walk_L1_p2m(struct vcpu *v, pa
* Returns:
*/
int
-nestedhvm_hap_nested_page_fault(struct vcpu *v, paddr_t L2_gpa)
+nestedhvm_hap_nested_page_fault(struct vcpu *v, paddr_t L2_gpa,
+ bool_t access_r, bool_t access_w, bool_t access_x)
{
int rv;
paddr_t L1_gpa, L0_gpa;
struct domain *d = v->domain;
struct p2m_domain *p2m, *nested_p2m;
unsigned int page_order_21, page_order_10, page_order_20;
+ uint32_t pfec;
+ unsigned long flags_21;
+ p2m_type_t p2mt_10, p2mt_20;
+ p2m_access_t p2ma_10, p2ma_20;
p2m = p2m_get_hostp2m(d); /* L0 p2m */
nested_p2m = p2m_get_nestedp2m(v, nhvm_vcpu_hostcr3(v));
/* walk the L1 P2M table */
- rv = nestedhap_walk_L1_p2m(v, L2_gpa, &L1_gpa, &page_order_21);
+ rv = nestedhap_walk_L1_p2m(v, L2_gpa, &L1_gpa, &flags_21,
+ &page_order_21, &pfec, access_r, access_w, access_x);
/* let caller to handle these two cases */
switch (rv) {
@@ -222,22 +239,6 @@ nestedhvm_hap_nested_page_fault(struct v
return rv;
case NESTEDHVM_PAGEFAULT_DONE:
break;
- default:
- BUG();
- break;
- }
-
- /* ==> we have to walk L0 P2M */
- rv = nestedhap_walk_L0_p2m(p2m, L1_gpa, &L0_gpa, &page_order_10);
-
- /* let upper level caller to handle these two cases */
- switch (rv) {
- case NESTEDHVM_PAGEFAULT_INJECT:
- return rv;
- case NESTEDHVM_PAGEFAULT_ERROR:
- return rv;
- case NESTEDHVM_PAGEFAULT_DONE:
- break;
case NESTEDHVM_PAGEFAULT_MMIO:
return rv;
default:
@@ -245,12 +246,42 @@ nestedhvm_hap_nested_page_fault(struct v
break;
}
+ /* ==> we have to walk L0 P2M */
+ rv = nestedhap_walk_L0_p2m(p2m, L1_gpa, &L0_gpa,
+ &p2mt_10, &p2ma_10, &page_order_10,
+ access_r, access_w, access_x);
+
+ /* let upper level caller to handle these two cases */
+ switch (rv) {
+ case NESTEDHVM_PAGEFAULT_INJECT:
+ return rv;
+ case NESTEDHVM_PAGEFAULT_ERROR:
+ return rv;
+ case NESTEDHVM_PAGEFAULT_DONE:
+ break;
+ case NESTEDHVM_PAGEFAULT_MMIO:
+ return rv;
+ case NESTEDHVM_PAGEFAULT_READONLY:
+ return rv;
+ default:
+ BUG();
+ break;
+ }
+
page_order_20 = min(page_order_21, page_order_10);
+ p2mt_20 = p2mt_10;
+ p2ma_20 = p2m_access_rx;
+ if (flags_21 & _PAGE_NX)
+ p2ma_20 = p2m_access_r;
+ if (flags_21 & _PAGE_RW)
+ p2ma_20 = p2m_access_rwx;
+ if ((flags_21 & (_PAGE_RW|_PAGE_NX)) == (_PAGE_RW|_PAGE_NX))
+ p2ma_20 = p2m_access_rw;
+
/* fix p2m_get_pagetable(nested_p2m) */
nestedhap_fix_p2m(v, nested_p2m, L2_gpa, L0_gpa, page_order_20,
- p2m_ram_rw,
- p2m_access_rwx /* FIXME: Should use same permission as l1 guest */);
+ p2mt_20, p2ma_20);
return NESTEDHVM_PAGEFAULT_DONE;
}
diff -r 5fdb3f48b970 -r d71ce8cd7e8a xen/arch/x86/mm/hap/private.h
--- a/xen/arch/x86/mm/hap/private.h
+++ b/xen/arch/x86/mm/hap/private.h
@@ -40,12 +40,15 @@ unsigned long hap_gva_to_gfn_4_levels(st
unsigned long hap_p2m_ga_to_gfn_2_levels(struct vcpu *v,
struct p2m_domain *p2m, unsigned long cr3,
- paddr_t ga, uint32_t *pfec, unsigned int *page_order);
+ paddr_t ga, unsigned long *flags, uint32_t *pfec,
+ unsigned int *page_order);
unsigned long hap_p2m_ga_to_gfn_3_levels(struct vcpu *v,
struct p2m_domain *p2m, unsigned long cr3,
- paddr_t ga, uint32_t *pfec, unsigned int *page_order);
+ paddr_t ga, unsigned long *flags, uint32_t *pfec,
+ unsigned int *page_order);
unsigned long hap_p2m_ga_to_gfn_4_levels(struct vcpu *v,
struct p2m_domain *p2m, unsigned long cr3,
- paddr_t ga, uint32_t *pfec, unsigned int *page_order);
+ paddr_t ga, unsigned long *flags, uint32_t *pfec,
+ unsigned int *page_order);
#endif /* __HAP_PRIVATE_H__ */
diff -r 5fdb3f48b970 -r d71ce8cd7e8a xen/arch/x86/mm/p2m-pt.c
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -60,7 +60,7 @@
#define P2M_BASE_FLAGS \
(_PAGE_PRESENT | _PAGE_USER | _PAGE_DIRTY | _PAGE_ACCESSED)
-static unsigned long p2m_type_to_flags(p2m_type_t t, mfn_t mfn)
+unsigned long p2m_type_to_flags(p2m_type_t t, mfn_t mfn, p2m_access_t access)
{
unsigned long flags;
#ifdef __x86_64__
@@ -79,8 +79,8 @@ static unsigned long p2m_type_to_flags(p
BUG_ON(t > p2m_populate_on_demand);
#endif
- switch(t)
- {
+ /* First apply type permissions */
+ switch(t) {
case p2m_invalid:
case p2m_mmio_dm:
case p2m_populate_on_demand:
@@ -88,22 +88,91 @@ static unsigned long p2m_type_to_flags(p
case p2m_ram_paged:
case p2m_ram_paging_in:
default:
- return flags;
+ flags |= _PAGE_NX_BIT;
+ break;
+ case p2m_grant_map_ro:
+ flags |= (P2M_BASE_FLAGS | _PAGE_NX_BIT);
+ break;
case p2m_ram_ro:
- case p2m_grant_map_ro:
case p2m_ram_logdirty:
case p2m_ram_shared:
- return flags | P2M_BASE_FLAGS;
+ flags |= P2M_BASE_FLAGS;
+ break;
case p2m_ram_rw:
+ flags |= (P2M_BASE_FLAGS | _PAGE_RW);
+ break;
case p2m_grant_map_rw:
- return flags | P2M_BASE_FLAGS | _PAGE_RW;
+ flags |= (P2M_BASE_FLAGS | _PAGE_RW | _PAGE_NX_BIT);
+ break;
case p2m_mmio_direct:
if ( !rangeset_contains_singleton(mmio_ro_ranges, mfn_x(mfn)) )
flags |= _PAGE_RW;
- return flags | P2M_BASE_FLAGS | _PAGE_PCD;
+ flags |= (P2M_BASE_FLAGS | _PAGE_PCD);
+ break;
}
+
+ /* Then restrict with access permissions */
+ switch (access) {
+ case p2m_access_n:
+ case p2m_access_n2rwx:
+ flags &= ~_PAGE_RW;
+ flags |= _PAGE_NX_BIT;
+ break;
+ case p2m_access_r:
+ flags &= ~_PAGE_RW;
+ flags |= _PAGE_NX_BIT;
+ break;
+ case p2m_access_w:
+ flags |= _PAGE_NX_BIT;
+ break;
+ case p2m_access_x:
+ flags &= ~_PAGE_RW;
+ break;
+ case p2m_access_rx:
+ case p2m_access_rx2rw:
+ flags &= ~_PAGE_RW;
+ break;
+ case p2m_access_wx:
+ break;
+ case p2m_access_rw:
+ flags |= _PAGE_NX_BIT;
+ break;
+ case p2m_access_rwx:
+ break;
+ }
+
+ return flags;
}
+p2m_type_t p2m_flags_to_type(unsigned long flags)
+{
+ /* For AMD IOMMUs we need to use type 0 for plain RAM, but we need
+ * to make sure that an entirely empty PTE doesn't have RAM type */
+ if ( flags == 0 )
+ return p2m_invalid;
+#ifdef __x86_64__
+ /* AMD IOMMUs use bits 9-11 to encode next io page level and bits
+ * 59-62 for iommu flags so we can't use them to store p2m type info. */
+ return (flags >> 12) & 0x7f;
+#else
+ return (flags >> 9) & 0x7;
+#endif
+}
+
+p2m_access_t p2m_flags_to_access(unsigned long flags)
+{
+ p2m_access_t access;
+
+ access = p2m_access_rx;
+ if (flags & _PAGE_NX_BIT)
+ access = p2m_access_r;
+ if (flags & _PAGE_RW)
+ access = p2m_access_rwx;
+ if ((flags & (_PAGE_RW | _PAGE_NX_BIT)) == (_PAGE_RW | _PAGE_NX_BIT))
+ access = p2m_access_rw;
+
+ return access;
+}
// Find the next level's P2M entry, checking for out-of-range gfn's...
// Returns NULL on error.
@@ -351,7 +420,7 @@ p2m_set_entry(struct p2m_domain *p2m, un
ASSERT(!mfn_valid(mfn) || p2mt != p2m_mmio_direct);
l3e_content = mfn_valid(mfn)
? l3e_from_pfn(mfn_x(mfn),
- p2m_type_to_flags(p2mt, mfn) | _PAGE_PSE)
+ p2m_type_to_flags(p2mt, mfn, p2ma) | _PAGE_PSE)
: l3e_empty();
entry_content.l1 = l3e_content.l3;
@@ -397,7 +466,7 @@ p2m_set_entry(struct p2m_domain *p2m, un
if ( mfn_valid(mfn) || (p2mt == p2m_mmio_direct)
|| p2m_is_paging(p2mt) )
entry_content = p2m_l1e_from_pfn(mfn_x(mfn),
- p2m_type_to_flags(p2mt, mfn));
+ p2m_type_to_flags(p2mt, mfn, p2ma));
else
entry_content = l1e_empty();
@@ -430,7 +499,7 @@ p2m_set_entry(struct p2m_domain *p2m, un
ASSERT(!mfn_valid(mfn) || p2mt != p2m_mmio_direct);
if ( mfn_valid(mfn) || p2m_is_magic(p2mt) )
l2e_content = l2e_from_pfn(mfn_x(mfn),
- p2m_type_to_flags(p2mt, mfn) |
+ p2m_type_to_flags(p2mt, mfn, p2ma) |
_PAGE_PSE);
else
l2e_content = l2e_empty();
@@ -544,6 +613,7 @@ pod_retry_l3:
if ( l3e_get_flags(l3e) & _PAGE_PSE )
{
p2mt = p2m_flags_to_type(l3e_get_flags(l3e));
+ *a = p2m_flags_to_access(l3e_get_flags(l3e));
ASSERT(l3e_get_pfn(l3e) != INVALID_MFN || !p2m_is_ram(p2mt));
if (p2m_is_valid(p2mt) )
mfn = _mfn(l3e_get_pfn(l3e) +
@@ -600,6 +670,7 @@ pod_retry_l2:
if (l2e_get_flags(l2e) & _PAGE_PSE)
{
p2mt = p2m_flags_to_type(l2e_get_flags(l2e));
+ *a = p2m_flags_to_access(l2e_get_flags(l2e));
ASSERT(l2e_get_pfn(l2e) != INVALID_MFN || !p2m_is_ram(p2mt));
if ( p2m_is_valid(p2mt) )
@@ -628,6 +699,7 @@ pod_retry_l1:
if ( ret == 0 ) {
unsigned long l1e_mfn = l1e_get_pfn(l1e);
p2mt = p2m_flags_to_type(l1e_get_flags(l1e));
+ *a = p2m_flags_to_access(l1e_get_flags(l1e));
ASSERT( mfn_valid(_mfn(l1e_mfn)) || !p2m_is_ram(p2mt) ||
p2m_is_paging(p2mt) );
@@ -685,8 +757,7 @@ p2m_gfn_to_mfn(struct p2m_domain *p2m, u
* XXX Once we start explicitly registering MMIO regions in the p2m
* XXX we will return p2m_invalid for unmapped gfns */
*t = p2m_mmio_dm;
- /* Not implemented except with EPT */
- *a = p2m_access_rwx;
+ *a = p2m_access_n;
if ( gfn > p2m->max_mapped_pfn )
/* This pfn is higher than the highest the p2m map currently holds */
@@ -746,6 +817,7 @@ pod_retry_l3:
l2_table_offset(addr) * L1_PAGETABLE_ENTRIES +
l1_table_offset(addr));
*t = p2m_flags_to_type(l3e_get_flags(*l3e));
+ *a = p2m_flags_to_access(l3e_get_flags(*l3e));
unmap_domain_page(l3e);
ASSERT(mfn_valid(mfn) || !p2m_is_ram(*t));
@@ -781,6 +853,7 @@ pod_retry_l2:
{
mfn = _mfn(l2e_get_pfn(*l2e) + l1_table_offset(addr));
*t = p2m_flags_to_type(l2e_get_flags(*l2e));
+ *a = p2m_flags_to_access(l2e_get_flags(*l2e));
unmap_domain_page(l2e);
ASSERT(mfn_valid(mfn) || !p2m_is_ram(*t));
@@ -814,6 +887,7 @@ pod_retry_l1:
}
mfn = _mfn(l1e_get_pfn(*l1e));
*t = l1t;
+ *a = p2m_flags_to_access(l1e_get_flags(*l1e));
unmap_domain_page(l1e);
ASSERT(mfn_valid(mfn) || !p2m_is_ram(*t) || p2m_is_paging(*t));
@@ -835,6 +909,7 @@ static void p2m_change_type_global(struc
mfn_t l1mfn, l2mfn, l3mfn;
unsigned long i1, i2, i3;
l3_pgentry_t *l3e;
+ p2m_access_t p2ma;
#if CONFIG_PAGING_LEVELS == 4
l4_pgentry_t *l4e;
unsigned long i4;
@@ -883,7 +958,8 @@ static void p2m_change_type_global(struc
continue;
mfn = l3e_get_pfn(l3e[i3]);
gfn = get_gpfn_from_mfn(mfn);
- flags = p2m_type_to_flags(nt, _mfn(mfn));
+ p2ma = p2m_flags_to_access(flags);
+ flags = p2m_type_to_flags(nt, _mfn(mfn), p2ma);
l1e_content = l1e_from_pfn(mfn, flags | _PAGE_PSE);
p2m->write_p2m_entry(p2m, gfn,
(l1_pgentry_t *)&l3e[i3],
@@ -914,7 +990,8 @@ static void p2m_change_type_global(struc
#endif
)
* L2_PAGETABLE_ENTRIES) * L1_PAGETABLE_ENTRIES;
- flags = p2m_type_to_flags(nt, _mfn(mfn));
+ p2ma = p2m_flags_to_access(flags);
+ flags = p2m_type_to_flags(nt, _mfn(mfn), p2ma);
l1e_content = l1e_from_pfn(mfn, flags | _PAGE_PSE);
p2m->write_p2m_entry(p2m, gfn,
(l1_pgentry_t *)&l2e[i2],
@@ -938,7 +1015,8 @@ static void p2m_change_type_global(struc
)
* L2_PAGETABLE_ENTRIES) * L1_PAGETABLE_ENTRIES;
/* create a new 1le entry with the new type */
- flags = p2m_type_to_flags(nt, _mfn(mfn));
+ p2ma = p2m_flags_to_access(flags);
+ flags = p2m_type_to_flags(nt, _mfn(mfn), p2ma);
l1e_content = p2m_l1e_from_pfn(mfn, flags);
p2m->write_p2m_entry(p2m, gfn, &l1e[i1],
l1mfn, l1e_content, 1);
diff -r 5fdb3f48b970 -r d71ce8cd7e8a xen/arch/x86/mm/p2m.c
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1588,9 +1588,15 @@ unsigned long paging_gva_to_gfn(struct v
mode = paging_get_nestedmode(v);
gfn = mode->gva_to_gfn(v, p2m, va, pfec);
+ /* if l2 guest has access to the l1's own devices
+ * then the translation failed because l1 mmio pages
+ * are not in the nested p2m. */
+ if (gfn == INVALID_GFN)
+ return INVALID_GFN;
+
/* translate l2 guest gfn into l1 guest gfn */
return hostmode->p2m_ga_to_gfn(v, hostp2m, ncr3,
- gfn << PAGE_SHIFT, pfec, NULL);
+ gfn << PAGE_SHIFT, NULL, pfec, NULL);
}
return hostmode->gva_to_gfn(v, hostp2m, va, pfec);
diff -r 5fdb3f48b970 -r d71ce8cd7e8a xen/include/asm-x86/guest_pt.h
--- a/xen/include/asm-x86/guest_pt.h
+++ b/xen/include/asm-x86/guest_pt.h
@@ -289,6 +289,27 @@ guest_walk_to_page_order(walk_t *gw)
return GUEST_L1_PAGETABLE_SHIFT - PAGE_SHIFT;
}
+/* Given a walk_t from a successful walk, return the flags of the
+ * page or superpage that the virtual address is in. */
+static inline unsigned long
+guest_walk_to_flags(walk_t *gw)
+{
+ unsigned long flags;
+
+ /* This is only valid for successful walks - otherwise the
+ * PSE bits might be invalid. */
+ ASSERT(guest_l1e_get_flags(gw->l1e) & _PAGE_PRESENT);
+#if GUEST_PAGING_LEVELS >= 3
+ flags = guest_l3e_get_flags(gw->l3e);
+ if ( flags & _PAGE_PSE )
+ return flags;
+#endif
+ flags = guest_l2e_get_flags(gw->l2e);
+ if ( flags & _PAGE_PSE )
+ return flags;
+ return guest_l1e_get_flags(gw->l1e);
+}
+
/* Walk the guest pagetables, after the manner of a hardware walker.
*
diff -r 5fdb3f48b970 -r d71ce8cd7e8a xen/include/asm-x86/hvm/nestedhvm.h
--- a/xen/include/asm-x86/hvm/nestedhvm.h
+++ b/xen/include/asm-x86/hvm/nestedhvm.h
@@ -47,11 +47,13 @@ bool_t nestedhvm_vcpu_in_guestmode(struc
vcpu_nestedhvm(v).nv_guestmode = 0
/* Nested paging */
-#define NESTEDHVM_PAGEFAULT_DONE 0
-#define NESTEDHVM_PAGEFAULT_INJECT 1
-#define NESTEDHVM_PAGEFAULT_ERROR 2
-#define NESTEDHVM_PAGEFAULT_MMIO 3
-int nestedhvm_hap_nested_page_fault(struct vcpu *v, paddr_t L2_gpa);
+#define NESTEDHVM_PAGEFAULT_DONE 0
+#define NESTEDHVM_PAGEFAULT_INJECT 1
+#define NESTEDHVM_PAGEFAULT_ERROR 2
+#define NESTEDHVM_PAGEFAULT_MMIO 3
+#define NESTEDHVM_PAGEFAULT_READONLY 4
+int nestedhvm_hap_nested_page_fault(struct vcpu *v, paddr_t L2_gpa,
+ bool_t access_r, bool_t access_w, bool_t access_x);
/* IO permission map */
unsigned long *nestedhvm_vcpu_iomap_get(bool_t ioport_80, bool_t ioport_ed);
diff -r 5fdb3f48b970 -r d71ce8cd7e8a xen/include/asm-x86/p2m.h
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -693,20 +693,10 @@ p2m_pod_demand_populate(struct p2m_domai
*/
/* Extract the type from the PTE flags that store it */
-static inline p2m_type_t p2m_flags_to_type(unsigned long flags)
-{
- /* For AMD IOMMUs we need to use type 0 for plain RAM, but we need
- * to make sure that an entirely empty PTE doesn't have RAM type */
- if ( flags == 0 )
- return p2m_invalid;
-#ifdef __x86_64__
- /* AMD IOMMUs use bits 9-11 to encode next io page level and bits
- * 59-62 for iommu flags so we can't use them to store p2m type info. */
- return (flags >> 12) & 0x7f;
-#else
- return (flags >> 9) & 0x7;
-#endif
-}
+p2m_type_t p2m_flags_to_type(unsigned long flags);
+
+/* Extract the PTE flags from the type */
+unsigned long p2m_type_to_flags(p2m_type_t t, mfn_t mfn, p2m_access_t access);
/*
* Nested p2m: shadow p2m tables used for nested HVM virtualization
diff -r 5fdb3f48b970 -r d71ce8cd7e8a xen/include/asm-x86/paging.h
--- a/xen/include/asm-x86/paging.h
+++ b/xen/include/asm-x86/paging.h
@@ -114,8 +114,9 @@ struct paging_mode {
uint32_t *pfec);
unsigned long (*p2m_ga_to_gfn )(struct vcpu *v,
struct p2m_domain *p2m,
- unsigned long cr3,
- paddr_t ga, uint32_t *pfec,
+ unsigned long cr3, paddr_t ga,
+ unsigned long *flags,
+ uint32_t *pfec,
unsigned int *page_order);
void (*update_cr3 )(struct vcpu *v, int do_locking);
void (*update_paging_modes )(struct vcpu *v);
@@ -277,11 +278,12 @@ unsigned long paging_gva_to_gfn(struct v
static inline unsigned long paging_ga_to_gfn_cr3(struct vcpu *v,
unsigned long cr3,
paddr_t ga,
+ unsigned long *flags,
uint32_t *pfec,
unsigned int *page_order)
{
struct p2m_domain *p2m = v->domain->arch.p2m;
- return paging_get_hostmode(v)->p2m_ga_to_gfn(v, p2m, cr3, ga, pfec,
+ return paging_get_hostmode(v)->p2m_ga_to_gfn(v, p2m, cr3, ga, flags, pfec,
page_order);
}
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: [PATCH] nestedsvm: fix l2 guest display refresh issue
2012-07-06 13:36 [PATCH] nestedsvm: fix l2 guest display refresh issue Christoph Egger
@ 2012-07-12 11:26 ` Tim Deegan
0 siblings, 0 replies; 2+ messages in thread
From: Tim Deegan @ 2012-07-12 11:26 UTC (permalink / raw)
To: Christoph Egger; +Cc: xen-devel@lists.xen.org
At 15:36 +0200 on 06 Jul (1341588980), Christoph Egger wrote:
> Fix l2 guest refresh problem.
> When l2 guest does a write access the l1 hypervisor
> mapped read only then inject VMEXIT(#NPF) into l1
> hypervisor.
> When l2 guest does a write access the host mapped
> as read only then let the host handle the NPF.
> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
Thanks for the fix, but I see two problems with this patch:
First, I think that trying to extract the flags from the page walk this
way is the wrong approach. Everywhere else, we pass a PFEC in to the
walker and let the walker handle the access control logic correctly;
its return code is the set of flags that were missing at _any_ level in
the walk, not just the leaf entry.
In fact nestedhap_walk_L1_p2m() already passes a PFEC in to the walker,
and from my reading of it, passes an uninitialized value, so presumably
it's only luck that you haven't been seeing spurious failures already.
So you could pass the r/w/x bits in to nestedhvm_hap_nested_page_fault()
as you do below, and then have nestedhvm_hap_nested_page_fault() or
nestedhap_walk_L1_p2m() make the right PFEC to pass to the walker. Do
you think that would work?
Second, you've made a bunch of changes to p2m_type_to_flags(); it looks
like an attempt to support p2m_access on AMD, which is excellent but
doesn't relate to the patch description, and is probably too late for
4.2. Can you split it out into a separate patch, please?
Also, it's not correct. The EPT implementation of this feature stores
the access type in some available bits of the EPT entry, so it can be
recovered later. Your p2m_flags_to_access() tries to infer the access
type from the actual access-control bits, but there's not enough
information there to do that -- an entry might have _PAGE_RW clear based
on its p2mt, even if the p2ma is p2m_access_rw, and a magic access type
like p2m_access_n2rwx can't be distinguished from p2m_access_n. So
you'll need to find 4 bits to stash the p2ma in. And I think this runs
up against the problem we had with p2mt -- if you're using an IOMMU you
can't use bits 9-11 or 52-62 in a present entry. So if you want to
supprt the p2m_access_type logic, you have to get rid of the shared
NPT/IOMMU tables.
The change to add _PAGE_NX to grant-map types seems like a good one that
could be split out and go in separately; it brings this code in line
with PV and EPT. But I think it should use _PAGE_NX_BIT rather than
defining a new flag.
Cheers,
Tim.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-07-12 11:26 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-06 13:36 [PATCH] nestedsvm: fix l2 guest display refresh issue Christoph Egger
2012-07-12 11:26 ` Tim Deegan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).