From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751742AbdHQH6K (ORCPT ); Thu, 17 Aug 2017 03:58:10 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42524 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750812AbdHQH6J (ORCPT ); Thu, 17 Aug 2017 03:58:09 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com F0DBEE1854 Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=david@redhat.com Subject: Re: [PATCH 2/3] KVM: x86: Avoid guest page table walk when gpa_available is set To: Paolo Bonzini , linux-kernel@vger.kernel.org, kvm@vger.kernel.org Cc: wanpeng.li@hotmail.com, rkrcmar@redhat.com, Brijesh Singh References: <1502470352-19447-1-git-send-email-pbonzini@redhat.com> <1502470352-19447-3-git-send-email-pbonzini@redhat.com> From: David Hildenbrand Organization: Red Hat GmbH Message-ID: <24a65134-308b-755b-049a-701fe6f215e7@redhat.com> Date: Thu, 17 Aug 2017 09:58:06 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <1502470352-19447-3-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Thu, 17 Aug 2017 07:58:09 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11.08.2017 18:52, Paolo Bonzini wrote: > From: Brijesh Singh > > When a guest causes a page fault which requires emulation, the > vcpu->arch.gpa_available flag is set to indicate that cr2 contains a > valid GPA. > > Currently, emulator_read_write_onepage() makes use of gpa_available flag > to avoid a guest page walk for a known MMIO regions. Lets not limit > the gpa_available optimization to just MMIO region. The patch extends > the check to avoid page walk whenever gpa_available flag is set. Can we move that to a separate patch? > > Signed-off-by: Brijesh Singh > [Fix EPT=0 according to Wanpeng Li's fix, plus ensure VMX also uses the > new code. - Paolo] > Signed-off-by: Paolo Bonzini > --- [...] > +++ b/arch/x86/kvm/svm.c > @@ -4235,8 +4235,7 @@ static int handle_exit(struct kvm_vcpu *vcpu) > u32 exit_code = svm->vmcb->control.exit_code; > > trace_kvm_exit(exit_code, vcpu, KVM_ISA_SVM); > - > - vcpu->arch.gpa_available = (exit_code == SVM_EXIT_NPF); > + vcpu->arch.gpa_available = false; Can we move resetting to false to vcpu_enter_guest()? It should be reset before handle_exit() is called for both cases. (maybe an additional patch) > > if (!is_cr_intercept(svm, INTERCEPT_CR0_WRITE)) > vcpu->arch.cr0 = svm->vmcb->save.cr0; > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 45fb0ea78ee8..79efb00dd70d 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -6393,9 +6393,7 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu) > error_code |= (exit_qualification & 0x100) != 0 ? > PFERR_GUEST_FINAL_MASK : PFERR_GUEST_PAGE_MASK; > > - vcpu->arch.gpa_available = true; > vcpu->arch.exit_qualification = exit_qualification; > - > return kvm_mmu_page_fault(vcpu, gpa, error_code, NULL, 0); > } > > @@ -6410,7 +6408,6 @@ static int handle_ept_misconfig(struct kvm_vcpu *vcpu) > return kvm_skip_emulated_instruction(vcpu); > } > > - vcpu->arch.gpa_available = true; > ret = kvm_mmu_page_fault(vcpu, gpa, PFERR_RSVD_MASK, NULL, 0); > if (ret >= 0) > return ret; > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index e40a779711a9..bb05b705c295 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -4657,25 +4657,18 @@ static int emulator_read_write_onepage(unsigned long addr, void *val, > */ > if (vcpu->arch.gpa_available && > emulator_can_use_gpa(ctxt) && > - vcpu_is_mmio_gpa(vcpu, addr, exception->address, write) && > - (addr & ~PAGE_MASK) == (exception->address & ~PAGE_MASK)) { > - gpa = exception->address; > - goto mmio; > + (addr & ~PAGE_MASK) == (vcpu->arch.gpa_val & ~PAGE_MASK)) { > + gpa = vcpu->arch.gpa_val; > + ret = vcpu_is_mmio_gpa(vcpu, addr, gpa, write); > + } else { > + ret = vcpu_mmio_gva_to_gpa(vcpu, addr, &gpa, exception, write); > } > > - ret = vcpu_mmio_gva_to_gpa(vcpu, addr, &gpa, exception, write); > - > if (ret < 0) > return X86EMUL_PROPAGATE_FAULT; > - > - /* For APIC access vmexit */ > - if (ret) > - goto mmio; > - > - if (ops->read_write_emulate(vcpu, gpa, val, bytes)) > + if (!ret && ops->read_write_emulate(vcpu, gpa, val, bytes)) > return X86EMUL_CONTINUE; > > -mmio: > /* > * Is this MMIO handled locally? > */ > Looks good to me. -- Thanks, David