From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751228AbdEBQYX (ORCPT ); Tue, 2 May 2017 12:24:23 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46852 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750853AbdEBQYV (ORCPT ); Tue, 2 May 2017 12:24:21 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 86ABEC1FF076 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=pbonzini@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 86ABEC1FF076 Subject: Re: [PATCH] x86: kvm: Avoid guest page table walk when gpa_available is set To: Brijesh Singh , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= References: <1493049146-19261-1-git-send-email-brijesh.singh@amd.com> <20170424205236.GE5713@potion> <77f51978-5937-0c94-13b6-885345921b03@amd.com> <20170425140351.GF5713@potion> <6e453f35-cd26-1df8-5f8e-68fa09c6a1a3@amd.com> <20170426204427.GA7135@potion> <7e27af5e-5978-5b57-f008-58515b218cce@amd.com> Cc: 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 From: Paolo Bonzini Message-ID: Date: Tue, 2 May 2017 18:24:13 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <7e27af5e-5978-5b57-f008-58515b218cce@amd.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Tue, 02 May 2017 16:24:20 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 28/04/2017 21:15, Brijesh Singh wrote: > Hi Radim, > >> >> This will probably return false negatives when then vcpu->arch.gpa_val >> couldn't be used anyway (possibly after a VM exits), so it is hard to >> draw a conclusion. >> >> I would really like if we had a solution that passed the gpa inside >> function parameters. >> >> (Btw. cr2 can also be a virtual address, so we can call it gpa directly) >> > > I've tried the below patch and it seems to work fine. This does not > consider > PIO case and as you rightly pointed PIO should trigger #NPF relatively > rarely. > At least so far in my runs I have not seen PIO causing #NPF. If this sounds > acceptable approach then I can submit v2 with these changes and remove > gpa_val > additional. PIO can certainly cause #NPF, albeit rarely, so this is not a viable one. Paolo > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 13c132b..c040e38 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -4662,17 +4662,17 @@ 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; > + ret = vcpu_is_mmio_gpa(vcpu, addr, gpa, write); > + } else { > + dump_stack(); > + 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; > @@ -7056,11 +7056,11 @@ static int vcpu_run(struct kvm_vcpu *vcpu) > return r; > } > > -static inline int complete_emulated_io(struct kvm_vcpu *vcpu) > +static inline int complete_emulated_io(struct kvm_vcpu *vcpu, unsigned > long cr2) > { > int r; > vcpu->srcu_idx = srcu_read_lock(&vcpu->kvm->srcu); > - r = emulate_instruction(vcpu, EMULTYPE_NO_DECODE); > + r = x86_emulate_instruction(vcpu, cr2, EMULTYPE_NO_DECODE, NULL, > 0); > srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx); > if (r != EMULATE_DONE) > return 0; > @@ -7071,7 +7071,7 @@ static int complete_emulated_pio(struct kvm_vcpu > *vcpu) > { > BUG_ON(!vcpu->arch.pio.count); > > - return complete_emulated_io(vcpu); > + return complete_emulated_io(vcpu, 0); > } > /* > @@ -7097,6 +7097,7 @@ static int complete_emulated_mmio(struct kvm_vcpu > *vcpu) > struct kvm_run *run = vcpu->run; > struct kvm_mmio_fragment *frag; > unsigned len; > + gpa_t gpa; > > BUG_ON(!vcpu->mmio_needed); > > @@ -7106,6 +7107,12 @@ static int complete_emulated_mmio(struct kvm_vcpu > *vcpu) > if (!vcpu->mmio_is_write) > memcpy(frag->data, run->mmio.data, len); > > + /* > + * lets use the GPA from previous guest page table walk to avoid yet > + * another guest page table walk when completing the MMIO > page-fault. > + */ > + gpa = frag->gpa; > + > if (frag->len <= 8) { > /* Switch to the next fragment. */ > frag++; > @@ -7124,7 +7131,7 @@ static int complete_emulated_mmio(struct kvm_vcpu > *vcpu) > if (vcpu->mmio_is_write) > return 1; > vcpu->mmio_read_completed = 1; > - return complete_emulated_io(vcpu); > + return complete_emulated_io(vcpu, gpa); > } > > run->exit_reason = KVM_EXIT_MMIO; >