nestedsvm: fix l2 guest display refresh issue
PATCH 1/1 nestedsvm: fix l2 guest display refresh issue
Signature · 7 lines
-- ---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
# 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.
xen/arch/x86/hvm/hvm.c +4 -1
@@ -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 lif ( !handle_mmio () )hvm_inject_hw_exception (TRAP_gp_fault,0 );return 1 ; +case NESTEDHVM_PAGEFAULT_READONLY : +break ; } }
xen/arch/x86/mm/hap/guest_walk.c +9 -2
@@ -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_PAunsigned 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_PAif ( page_order ) *page_order =guest_walk_to_page_order (&gw); +if ( flags ) + *flags =guest_walk_to_flags (&gw); +return gfn_x (gfn); }
xen/arch/x86/mm/hap/hap.c +3 -1
@@ -908,10 +908,12 @@ static unsigned long hap_gva_to_gfn_realstatic 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 ); }
xen/arch/x86/mm/hap/nested_hap.c +60 -29
161 lines not on this page · raw message
xen/arch/x86/mm/hap/private.h +6 -3
@@ -40,12 +40,15 @@ unsigned long hap_gva_to_gfn_4_levels(stunsigned 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__ */
xen/arch/x86/mm/p2m-pt.c +94 -16
223 lines not on this page · raw message
xen/arch/x86/mm/p2m.c +7 -1
@@ -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);
xen/include/asm-x86/guest_pt.h +21 -0
@@ -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. *
xen/include/asm-x86/hvm/nestedhvm.h +7 -5
@@ -47,11 +47,13 @@ bool_t nestedhvm_vcpu_in_guestmode(strucvcpu_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);
xen/include/asm-x86/p2m.h +4 -14
@@ -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
xen/include/asm-x86/paging.h +5 -3
@@ -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 vstatic 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); }
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Quoted 7 lines · Christoph Egger
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 <[email protected]>
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.