From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754791AbcKUPHS (ORCPT ); Mon, 21 Nov 2016 10:07:18 -0500 Received: from mx1.redhat.com ([209.132.183.28]:54150 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754200AbcKUPHO (ORCPT ); Mon, 21 Nov 2016 10:07:14 -0500 Subject: Re: [PATCH v1 3/3] kvm: svm: Use the hardware provided GPA instead of page walk To: Brijesh Singh , kvm@vger.kernel.org References: <147916172660.16347.15695649975899246333.stgit@brijesh-build-machine> <147916176259.16347.7828367075943432152.stgit@brijesh-build-machine> Cc: Thomas.Lendacky@amd.com, rkrcmar@redhat.com, joro@8bytes.org, x86@kernel.org, linux-kernel@vger.kernel.org, mingo@redhat.com, hpa@zytor.com, tglx@linutronix.de, bp@suse.de From: Paolo Bonzini Message-ID: <40c0e826-e8ef-2270-17e1-1b35e121ab1e@redhat.com> Date: Mon, 21 Nov 2016 16:07:03 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <147916176259.16347.7828367075943432152.stgit@brijesh-build-machine> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Mon, 21 Nov 2016 15:07:09 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 14/11/2016 23:16, Brijesh Singh wrote: > From: Tom Lendacky > > When a guest causes a NPF which requires emulation, KVM sometimes walks > the guest page tables to translate the GVA to a GPA. This is unnecessary > most of the time on AMD hardware since the hardware provides the GPA in > EXITINFO2. > > The only exception cases involve string operations involving rep or > operations that use two memory locations. With rep, the GPA will only be > the value of the initial NPF and with dual memory locations we won't know > which memory address was translated into EXITINFO2. > > Signed-off-by: Tom Lendacky > Reviewed-by: Borislav Petkov > Signed-off-by: Brijesh Singh > --- > arch/x86/include/asm/kvm_emulate.h | 3 +++ > arch/x86/include/asm/kvm_host.h | 3 +++ > arch/x86/kvm/svm.c | 9 ++++++++- > arch/x86/kvm/x86.c | 17 ++++++++++++++++- > 4 files changed, 30 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h > index e9cd7be..2d1ac09 100644 > --- a/arch/x86/include/asm/kvm_emulate.h > +++ b/arch/x86/include/asm/kvm_emulate.h > @@ -344,6 +344,9 @@ struct x86_emulate_ctxt { > struct read_cache mem_read; > }; > > +/* String operation identifier (matches the definition in emulate.c) */ > +#define CTXT_STRING_OP (1 << 13) > + > /* Repeat String Operation Prefix */ > #define REPE_PREFIX 0xf3 > #define REPNE_PREFIX 0xf2 > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 77cb3f9..fd5b1c8 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -668,6 +668,9 @@ struct kvm_vcpu_arch { > > int pending_ioapic_eoi; > int pending_external_vector; > + > + /* GPA available (AMD only) */ > + bool gpa_available; > }; > > struct kvm_lpage_info { > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index 5e64e656..b442c5a 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -275,6 +275,9 @@ static int avic; > module_param(avic, int, S_IRUGO); > #endif > > +/* EXITINFO2 contains valid GPA */ > +static bool gpa_avail = true; > + > /* AVIC VM ID bit masks and lock */ > static DECLARE_BITMAP(avic_vm_id_bitmap, AVIC_VM_ID_NR); > static DEFINE_SPINLOCK(avic_vm_id_lock); > @@ -1055,8 +1058,10 @@ static __init int svm_hardware_setup(void) > goto err; > } > > - if (!boot_cpu_has(X86_FEATURE_NPT)) > + if (!boot_cpu_has(X86_FEATURE_NPT)) { > npt_enabled = false; > + gpa_avail = false; This is not necessary, since you will never have exit_code == SVM_EXIT_NPF && !gpa_avail. > + } > > if (npt_enabled && !npt) { > printk(KERN_INFO "kvm: Nested Paging disabled\n"); > @@ -4192,6 +4197,8 @@ static int handle_exit(struct kvm_vcpu *vcpu) > vcpu->arch.cr0 = svm->vmcb->save.cr0; > if (npt_enabled) > vcpu->arch.cr3 = svm->vmcb->save.cr3; > + if (gpa_avail) > + vcpu->arch.gpa_available = (exit_code == SVM_EXIT_NPF); The "if", and the body moved just before return svm_exit_handlers[exit_code](svm); I'll try doing a similar patch for Intel too, for better testing. > > if (unlikely(svm->nested.exit_required)) { > nested_svm_vmexit(svm); > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index d02aeff..c290794 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -4420,7 +4420,19 @@ static int vcpu_mmio_gva_to_gpa(struct kvm_vcpu *vcpu, unsigned long gva, > return 1; > } > > - *gpa = vcpu->arch.walk_mmu->gva_to_gpa(vcpu, gva, access, exception); > + /* > + * If the exit was due to a NPF we may already have a GPA. > + * If the GPA is present, use it to avoid the GVA to GPA table > + * walk. Note, this cannot be used on string operations since > + * string operation using rep will only have the initial GPA > + * from when the NPF occurred. > + */ > + if (vcpu->arch.gpa_available && > + !(vcpu->arch.emulate_ctxt.d & CTXT_STRING_OP)) > + *gpa = exception->address; > + else > + *gpa = vcpu->arch.walk_mmu->gva_to_gpa(vcpu, gva, access, > + exception); > > if (*gpa == UNMAPPED_GVA) > return -1; > @@ -5542,6 +5554,9 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, > } > > restart: > + /* Save the faulting GPA (cr2) in the address field */ > + ctxt->exception.address = cr2; > + > r = x86_emulate_insn(ctxt); > > if (r == EMULATION_INTERCEPTED) >