[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH V6 4/5] xen/mm: Clean up pfec handling in gva_to_gfn
>>> On 19.01.16 at 08:30, <huaitong.han@xxxxxxxxx> wrote: > At the moment, the pfec argument to gva_to_gfn has two functions: > > * To inform guest_walk what kind of access is happenind > > * As a value to pass back into the guest in the event of a fault. > > Unfortunately this is not quite treated consistently: the hvm_fetch_* > function will "pre-clear" the PFEC_insn_fetch flag before calling > gva_to_gfn; meaning guest_walk doesn't actually know whether a given > access is an instruction fetch or not. This works now, but will cause > issues when pkeys are introduced, since guest_walk will need to know > whether an access is an instruction fetch even if it doesn't return > PFEC_insn_fetch. > > Fix this by making a clean separation for in and out functionalities > of the pfec argument: > > 1. Always pass in the access type to gva_to_gfn > > 2. Filter out inappropriate access flags before returning from gva_to_gfn. > > (The PFEC_insn_fetch flag should only be passed to the guest if either NX or > SMEP is enabled. See Intel 64 Developer's Manual, Volume 3, Section 4.7.) As mentioned in various other contexts not so long ago - no SDM section number please (as they tend to change); use section titles instead (also below in code comments). > Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx> > Signed-off-by: Huaitong Han <huaitong.han@xxxxxxxxx> > --- > xen/arch/x86/hvm/hvm.c | 8 ++------ > xen/arch/x86/mm/hap/guest_walk.c | 10 +++++++++- > xen/arch/x86/mm/shadow/multi.c | 6 ++++++ You failed to Cc the shadow code maintainer (now added). > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -4432,11 +4432,9 @@ enum hvm_copy_result hvm_copy_from_guest_virt( > enum hvm_copy_result hvm_fetch_from_guest_virt( > void *buf, unsigned long vaddr, int size, uint32_t pfec) > { > - if ( hvm_nx_enabled(current) || hvm_smep_enabled(current) ) > - pfec |= PFEC_insn_fetch; > return __hvm_copy(buf, vaddr, size, > HVMCOPY_from_guest | HVMCOPY_fault | HVMCOPY_virt, > - PFEC_page_present | pfec); > + PFEC_page_present | PFEC_insn_fetch | pfec); > } > > enum hvm_copy_result hvm_copy_to_guest_virt_nofault( > @@ -4458,11 +4456,9 @@ enum hvm_copy_result hvm_copy_from_guest_virt_nofault( > enum hvm_copy_result hvm_fetch_from_guest_virt_nofault( > void *buf, unsigned long vaddr, int size, uint32_t pfec) > { > - if ( hvm_nx_enabled(current) || hvm_smep_enabled(current) ) > - pfec |= PFEC_insn_fetch; > return __hvm_copy(buf, vaddr, size, > HVMCOPY_from_guest | HVMCOPY_no_fault | HVMCOPY_virt, > - PFEC_page_present | pfec); > + PFEC_page_present | PFEC_insn_fetch | pfec); > } > > unsigned long copy_to_user_hvm(void *to, const void *from, unsigned int len) This part Acked-by: Jan Beulich <jbeulich@xxxxxxxx> > --- a/xen/arch/x86/mm/hap/guest_walk.c > +++ b/xen/arch/x86/mm/hap/guest_walk.c > @@ -82,7 +82,7 @@ unsigned long hap_p2m_ga_to_gfn(GUEST_PAGING_LEVELS)( > if ( !top_page ) > { > pfec[0] &= ~PFEC_page_present; > - return INVALID_GFN; > + goto out_tweak_pfec; > } > top_mfn = _mfn(page_to_mfn(top_page)); > > @@ -139,6 +139,14 @@ unsigned long hap_p2m_ga_to_gfn(GUEST_PAGING_LEVELS)( > if ( missing & _PAGE_SHARED ) > pfec[0] = PFEC_page_shared; > > +out_tweak_pfec: To avoid corrupting the context in patches, labels should be indented by at least one space please. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |