From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [PATCH] KVM: VMX: Judge MMIO based on PFN rather than HVA in EPT violation Date: Tue, 10 Feb 2009 10:14:32 -0200 Message-ID: <20090210121432.GA29702@amt.cnet> References: <1234259021-26007-1-git-send-email-sheng@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Avi Kivity , kvm@vger.kernel.org To: Sheng Yang Return-path: Received: from mx2.redhat.com ([66.187.237.31]:36999 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752788AbZBJMO4 (ORCPT ); Tue, 10 Feb 2009 07:14:56 -0500 Content-Disposition: inline In-Reply-To: <1234259021-26007-1-git-send-email-sheng@linux.intel.com> Sender: kvm-owner@vger.kernel.org List-ID: 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 > --- > 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 ?