* [PATCH] KVM: VMX: Judge MMIO based on PFN rather than HVA in EPT violation @ 2009-02-10 9:43 Sheng Yang 2009-02-10 12:14 ` Marcelo Tosatti 0 siblings, 1 reply; 4+ messages in thread From: Sheng Yang @ 2009-02-10 9:43 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm, Sheng Yang One page can be unmapped from userspace, then HVA seems legal, but in fact, PFN is illegal. Signed-off-by: Sheng Yang <sheng@linux.intel.com> --- arch/x86/kvm/vmx.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 9913a1d..a4fa1b5 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -3061,7 +3061,7 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) u64 exit_qualification; enum emulation_result er; gpa_t gpa; - unsigned long hva; + pfn_t pfn; int gla_validity; int r; @@ -3086,8 +3086,8 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) } gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS); - hva = gfn_to_hva(vcpu->kvm, gpa >> PAGE_SHIFT); - if (!kvm_is_error_hva(hva)) { + pfn = gfn_to_pfn(vcpu->kvm, gpa >> PAGE_SHIFT); + if (!is_error_pfn(pfn)) { r = kvm_mmu_page_fault(vcpu, gpa & PAGE_MASK, 0); if (r < 0) { printk(KERN_ERR "EPT: Not enough memory!\n"); -- 1.5.4.5 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: VMX: Judge MMIO based on PFN rather than HVA in EPT violation 2009-02-10 9:43 [PATCH] KVM: VMX: Judge MMIO based on PFN rather than HVA in EPT violation Sheng Yang @ 2009-02-10 12:14 ` Marcelo Tosatti 2009-02-11 2:43 ` Sheng Yang 0 siblings, 1 reply; 4+ messages in thread From: Marcelo Tosatti @ 2009-02-10 12:14 UTC (permalink / raw) To: Sheng Yang; +Cc: Avi Kivity, kvm Hi Sheng, On Tue, Feb 10, 2009 at 05:43:41PM +0800, Sheng Yang wrote: > One page can be unmapped from userspace, then HVA seems legal, but in fact, > PFN is illegal. > > Signed-off-by: Sheng Yang <sheng@linux.intel.com> > --- > arch/x86/kvm/vmx.c | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 9913a1d..a4fa1b5 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -3061,7 +3061,7 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) > u64 exit_qualification; > enum emulation_result er; > gpa_t gpa; > - unsigned long hva; > + pfn_t pfn; > int gla_validity; > int r; > > @@ -3086,8 +3086,8 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) > } > > gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS); > - hva = gfn_to_hva(vcpu->kvm, gpa >> PAGE_SHIFT); > - if (!kvm_is_error_hva(hva)) { > + pfn = gfn_to_pfn(vcpu->kvm, gpa >> PAGE_SHIFT); > + if (!is_error_pfn(pfn)) { > r = kvm_mmu_page_fault(vcpu, gpa & PAGE_MASK, 0); > if (r < 0) { > printk(KERN_ERR "EPT: Not enough memory!\n"); Forgot to drop the additional reference acquired by gfn_to_pfn ? Hum, gfn_to_hva assumes the slot information is accurate, which is not the case if you simply munmap a page in the middle of a slot... we should provide a helper in userspace, and make sure nothing bad can happen otherwise. Anyway, why don't you just let kvm_mmu_page_fault handle instruction emulation instead of opencoding it in handle_ept_violation ? ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: VMX: Judge MMIO based on PFN rather than HVA in EPT violation 2009-02-10 12:14 ` Marcelo Tosatti @ 2009-02-11 2:43 ` Sheng Yang 2009-02-11 3:29 ` Sheng Yang 0 siblings, 1 reply; 4+ messages in thread From: Sheng Yang @ 2009-02-11 2:43 UTC (permalink / raw) To: Marcelo Tosatti; +Cc: Avi Kivity, kvm On Tuesday 10 February 2009 20:14:32 Marcelo Tosatti wrote: > Hi Sheng, > > On Tue, Feb 10, 2009 at 05:43:41PM +0800, Sheng Yang wrote: > > One page can be unmapped from userspace, then HVA seems legal, but in > > fact, PFN is illegal. > > > > Signed-off-by: Sheng Yang <sheng@linux.intel.com> > > --- > > arch/x86/kvm/vmx.c | 6 +++--- > > 1 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > > index 9913a1d..a4fa1b5 100644 > > --- a/arch/x86/kvm/vmx.c > > +++ b/arch/x86/kvm/vmx.c > > @@ -3061,7 +3061,7 @@ static int handle_ept_violation(struct kvm_vcpu > > *vcpu, struct kvm_run *kvm_run) u64 exit_qualification; > > enum emulation_result er; > > gpa_t gpa; > > - unsigned long hva; > > + pfn_t pfn; > > int gla_validity; > > int r; > > > > @@ -3086,8 +3086,8 @@ static int handle_ept_violation(struct kvm_vcpu > > *vcpu, struct kvm_run *kvm_run) } > > > > gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS); > > - hva = gfn_to_hva(vcpu->kvm, gpa >> PAGE_SHIFT); > > - if (!kvm_is_error_hva(hva)) { > > + pfn = gfn_to_pfn(vcpu->kvm, gpa >> PAGE_SHIFT); > > + if (!is_error_pfn(pfn)) { > > r = kvm_mmu_page_fault(vcpu, gpa & PAGE_MASK, 0); > > if (r < 0) { > > printk(KERN_ERR "EPT: Not enough memory!\n"); > > Forgot to drop the additional reference acquired by gfn_to_pfn ? Sadly, yeah... :( > > Hum, gfn_to_hva assumes the slot information is accurate, which is not > the case if you simply munmap a page in the middle of a slot... we > should provide a helper in userspace, and make sure nothing bad can > happen otherwise. Yes. But maintain a list in kernel seems a little complex for it. I checked the callings of gfn_to_hva, seems most of them are safe with a inaccurate result(not sure about host_largepage_backed, seems this would be affected if one 4k page was unmapped inside a large page). > > Anyway, why don't you just let kvm_mmu_page_fault handle instruction > emulation instead of opencoding it in handle_ept_violation ? It didn't work as expect. Checking it. -- regards Yang, Sheng ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: VMX: Judge MMIO based on PFN rather than HVA in EPT violation 2009-02-11 2:43 ` Sheng Yang @ 2009-02-11 3:29 ` Sheng Yang 0 siblings, 0 replies; 4+ messages in thread From: Sheng Yang @ 2009-02-11 3:29 UTC (permalink / raw) To: kvm; +Cc: Marcelo Tosatti, Avi Kivity On Wednesday 11 February 2009 10:43:43 Sheng Yang wrote: > On Tuesday 10 February 2009 20:14:32 Marcelo Tosatti wrote: > > Hi Sheng, > > > > On Tue, Feb 10, 2009 at 05:43:41PM +0800, Sheng Yang wrote: > > > One page can be unmapped from userspace, then HVA seems legal, but in > > > fact, PFN is illegal. > > > > > > Signed-off-by: Sheng Yang <sheng@linux.intel.com> > > > --- > > > arch/x86/kvm/vmx.c | 6 +++--- > > > 1 files changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > > > index 9913a1d..a4fa1b5 100644 > > > --- a/arch/x86/kvm/vmx.c > > > +++ b/arch/x86/kvm/vmx.c > > > @@ -3061,7 +3061,7 @@ static int handle_ept_violation(struct kvm_vcpu > > > *vcpu, struct kvm_run *kvm_run) u64 exit_qualification; > > > enum emulation_result er; > > > gpa_t gpa; > > > - unsigned long hva; > > > + pfn_t pfn; > > > int gla_validity; > > > int r; > > > > > > @@ -3086,8 +3086,8 @@ static int handle_ept_violation(struct kvm_vcpu > > > *vcpu, struct kvm_run *kvm_run) } > > > > > > gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS); > > > - hva = gfn_to_hva(vcpu->kvm, gpa >> PAGE_SHIFT); > > > - if (!kvm_is_error_hva(hva)) { > > > + pfn = gfn_to_pfn(vcpu->kvm, gpa >> PAGE_SHIFT); > > > + if (!is_error_pfn(pfn)) { > > > r = kvm_mmu_page_fault(vcpu, gpa & PAGE_MASK, 0); > > > if (r < 0) { > > > printk(KERN_ERR "EPT: Not enough memory!\n"); > > > > Forgot to drop the additional reference acquired by gfn_to_pfn ? > > Sadly, yeah... :( > > > Hum, gfn_to_hva assumes the slot information is accurate, which is not > > the case if you simply munmap a page in the middle of a slot... we > > should provide a helper in userspace, and make sure nothing bad can > > happen otherwise. > > Yes. But maintain a list in kernel seems a little complex for it. I checked > the callings of gfn_to_hva, seems most of them are safe with a inaccurate > result(not sure about host_largepage_backed, seems this would be affected > if one 4k page was unmapped inside a large page). > > > Anyway, why don't you just let kvm_mmu_page_fault handle instruction > > emulation instead of opencoding it in handle_ept_violation ? > > It didn't work as expect. Checking it. Oh, seems it didn't work due to my fault... Update the patch, thanks for reminder. :) -- regards Yang, Sheng ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-02-11 3:29 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-02-10 9:43 [PATCH] KVM: VMX: Judge MMIO based on PFN rather than HVA in EPT violation Sheng Yang 2009-02-10 12:14 ` Marcelo Tosatti 2009-02-11 2:43 ` Sheng Yang 2009-02-11 3:29 ` Sheng Yang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox