From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH] KVM: APIC: avoid instruction emulation for EOI writes Date: Mon, 29 Aug 2011 12:24:26 +0200 Message-ID: <4E5B68DA.1090208@siemens.com> References: <625BA99ED14B2D499DC4E29D8138F15063045B0C0C@shsmsx502.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Avi Kivity , "kvm@vger.kernel.org" , "Nakajima, Jun" , "Dong, Eddie" , Marcelo Tosatti To: "Tian, Kevin" Return-path: Received: from goliath.siemens.de ([192.35.17.28]:28910 "EHLO goliath.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753387Ab1H2KYg (ORCPT ); Mon, 29 Aug 2011 06:24:36 -0400 In-Reply-To: <625BA99ED14B2D499DC4E29D8138F15063045B0C0C@shsmsx502.ccr.corp.intel.com> Sender: kvm-owner@vger.kernel.org List-ID: On 2011-08-29 08:09, Tian, Kevin wrote: > Hi, Avi, > > Here comes the patch: > > KVM: APIC: avoid instruction emulation for EOI writes > > Instruction emulation for EOI writes can be skipped, since sane > guest simply uses MOV instead of string operations. This is a nice > improvement when guest doesn't support x2apic or hyper-V EOI > support. > > a single VM bandwidth is observed with ~8% bandwidth improvement > (7.4Gbps->8Gbps), by saving ~5% cycles from EOI emulation. > > Signed-off-by: Kevin Tian > : > Signed-off-by: Eddie Dong > Signed-off-by: Marcelo Tosatti > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index 57dcbd4..933187e 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -864,6 +864,15 @@ static int apic_mmio_write(struct kvm_io_device *this, > return 0; > } > > +void kvm_lapic_set_eoi(struct kvm_vcpu *vcpu) > +{ > + struct kvm_lapic *apic = vcpu->arch.apic; > + > + if (apic) > + apic_set_eoi(vcpu->arch.apic); > +} > +EXPORT_SYMBOL_GPL(kvm_lapic_set_eoi); > + > void kvm_free_lapic(struct kvm_vcpu *vcpu) > { > if (!vcpu->arch.apic) > diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h > index 52c9e6b..8287243 100644 > --- a/arch/x86/kvm/lapic.h > +++ b/arch/x86/kvm/lapic.h > @@ -26,6 +26,7 @@ int kvm_get_apic_interrupt(struct kvm_vcpu *vcpu); > void kvm_lapic_reset(struct kvm_vcpu *vcpu); > u64 kvm_lapic_get_cr8(struct kvm_vcpu *vcpu); > void kvm_lapic_set_tpr(struct kvm_vcpu *vcpu, unsigned long cr8); > +void kvm_lapic_set_eoi(struct kvm_vcpu *vcpu); > void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value); > u64 kvm_lapic_get_base(struct kvm_vcpu *vcpu); > void kvm_apic_set_version(struct kvm_vcpu *vcpu); > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 5e8d411..35e4af7 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -4540,6 +4540,22 @@ static int handle_xsetbv(struct kvm_vcpu *vcpu) > > static int handle_apic_access(struct kvm_vcpu *vcpu) > { > + unsigned long exit_qualification = vmcs_readl(EXIT_QUALIFICATION); > + int access_type, offset; > + > + access_type = (exit_qualification >> 12) & 0xf; > + offset = exit_qualification & 0xfff; > + /* > + * Sane guest uses MOV instead of string operations to > + * write EOI, with written value not cared. So make a > + * short-circuit here by avoiding heavy instruction > + * emulation. > + */ Is there no cheap way to validate this assumption and fall back to the slow path in case it doesn't apply? E.g. reading the first instruction byte and matching it against a whitelist? Even if the ignored scenarios are highly unlikely, I think we so far tried hard to provide both fast and accurate results to the guest in all cases. > + if ((access_type == 1) && (offset == APIC_EOI)) { > + kvm_lapic_set_eoi(vcpu); > + skip_emulated_instruction(vcpu); > + return 1; > + } > return emulate_instruction(vcpu, 0) == EMULATE_DONE; > } > > Thanks > Kevin Nice optimization otherwise! Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux