Re: [PATCH] KVM: APIC: avoid instruction emulation for EOI writes

[Jan Kiszka <jan.kiszka@siemens.com>]

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 <kevin.tian@intel.com>
>     <Based on earlier work from>:
>     Signed-off-by: Eddie Dong <eddie.dong@intel.com>
>     Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> 
> 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