From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1174606AbdDXUxA (ORCPT ); Mon, 24 Apr 2017 16:53:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52344 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1172632AbdDXUwm (ORCPT ); Mon, 24 Apr 2017 16:52:42 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 85CFB7E9C1 Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=rkrcmar@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 85CFB7E9C1 Date: Mon, 24 Apr 2017 22:52:36 +0200 From: Radim =?utf-8?B?S3LEjW3DocWZ?= To: Brijesh Singh Cc: pbonzini@redhat.com, joro@8bytes.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com, x86@kernel.org, Thomas.Lendacky@amd.com Subject: Re: [PATCH] x86: kvm: Avoid guest page table walk when gpa_available is set Message-ID: <20170424205236.GE5713@potion> References: <1493049146-19261-1-git-send-email-brijesh.singh@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1493049146-19261-1-git-send-email-brijesh.singh@amd.com> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Mon, 24 Apr 2017 20:52:41 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 2017-04-24 11:52-0400, Brijesh Singh: > On AMD hardware when a guest causes a NFP 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. > > Signed-off-by: Brijesh Singh > --- > arch/x86/include/asm/kvm_host.h | 1 + > arch/x86/kvm/svm.c | 4 ++++ > arch/x86/kvm/x86.c | 26 +++++++++++++++----------- > 3 files changed, 20 insertions(+), 11 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 74ef58c..491326d 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -677,6 +677,7 @@ struct kvm_vcpu_arch { > > /* GPA available (AMD only) */ > bool gpa_available; > + gpa_t gpa_val; Can't we pass this information through function parameters? (I'd rather avoid intractable variables.) > }; > > struct kvm_lpage_info { > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index 5fba706..8827e4b 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -4159,6 +4159,10 @@ static int handle_exit(struct kvm_vcpu *vcpu) > > vcpu->arch.gpa_available = (exit_code == SVM_EXIT_NPF); > > + /* On #NPF, exit_info_2 contain a valid GPA */ > + if (vcpu->arch.gpa_available) > + vcpu->arch.gpa_val = svm->vmcb->control.exit_info_2; How is vcpu->arch.gpa_val used between here and the NPF handler? > + > if (!is_cr_intercept(svm, INTERCEPT_CR0_WRITE)) > vcpu->arch.cr0 = svm->vmcb->save.cr0; > if (npt_enabled) > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > @@ -4653,18 +4653,16 @@ static int emulator_read_write_onepage(unsigned long addr, void *val, > * occurred. > */ > 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)) { > + emulator_can_use_gpa(ctxt) && > + (addr & ~PAGE_MASK) == (exception->address & ~PAGE_MASK)) { > gpa = exception->address; > - goto mmio; > + ret = vcpu_is_mmio_gpa(vcpu, addr, gpa, write); > + } else { > + ret = vcpu_mmio_gva_to_gpa(vcpu, addr, &gpa, exception, write); > + if (ret < 0) > + return X86EMUL_PROPAGATE_FAULT; > } > > - 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; > @@ -5675,8 +5673,14 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, > } > > restart: > - /* Save the faulting GPA (cr2) in the address field */ > - ctxt->exception.address = cr2; > + /* > + * Save the faulting GPA (cr2) in the address field > + * NOTE: If gpa_available is set then gpa_val will contain a valid GPA > + */ > + if (vcpu->arch.gpa_available) > + ctxt->exception.address = vcpu->arch.gpa_val; > + else > + ctxt->exception.address = cr2; And related, shouldn't vcpu->arch.gpa_val be in cr2? Thanks.