From mboxrd@z Thu Jan  1 00:00:00 1970
From: Gleb Natapov <gleb@redhat.com>
Subject: Re: [PATCH v2 5/6] x86: Enable ack interrupt on vmexit
Date: Thu, 22 Nov 2012 17:22:01 +0200
Message-ID: <20121122152201.GH21208@redhat.com>
References: <1353485379-6823-1-git-send-email-yang.z.zhang@intel.com>
 <1353485379-6823-6-git-send-email-yang.z.zhang@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm@vger.kernel.org, mtosatti@redhat.com
To: Yang Zhang <yang.z.zhang@intel.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:18320 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1752387Ab2KVTCi (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 22 Nov 2012 14:02:38 -0500
Content-Disposition: inline
In-Reply-To: <1353485379-6823-6-git-send-email-yang.z.zhang@intel.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Wed, Nov 21, 2012 at 04:09:38PM +0800, Yang Zhang wrote:
> Ack interrupt on vmexit is required by Posted Interrupt. With it,
> when external interrupt caused vmexit, the cpu will acknowledge the
> interrupt controller and save the interrupt's vector in vmcs.
> 
> There are several approaches to enable it. This patch uses a simply
> way: re-generate an interrupt via self ipi.
> 
> Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
> ---
>  arch/x86/kvm/vmx.c |   11 ++++++++++-
>  1 files changed, 10 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 7949d21..f6ef090 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -2525,7 +2525,8 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
>  #ifdef CONFIG_X86_64
>  	min |= VM_EXIT_HOST_ADDR_SPACE_SIZE;
>  #endif
> -	opt = VM_EXIT_SAVE_IA32_PAT | VM_EXIT_LOAD_IA32_PAT;
> +	opt = VM_EXIT_SAVE_IA32_PAT | VM_EXIT_LOAD_IA32_PAT |
> +		VM_EXIT_ACK_INTR_ON_EXIT;
Always? Do it only if posted interrupts are actually available
and going to be used.

>  	if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_EXIT_CTLS,
>  				&_vmexit_control) < 0)
>  		return -EIO;
> @@ -4457,6 +4458,14 @@ static int handle_exception(struct kvm_vcpu *vcpu)
>  
>  static int handle_external_interrupt(struct kvm_vcpu *vcpu)
>  {
> +	unsigned int vector;
> +
> +	vector = vmcs_read32(VM_EXIT_INTR_INFO);
> +	vector &= INTR_INFO_VECTOR_MASK;
Valid bit is guarantied to be set here?

> +
> +	apic_eoi();
This is way to late. handle_external_interrupt() is called longs after
preemption and local irqs are enabled. vcpu process may be scheduled out
and apic_eoi() will not be called for a long time leaving interrupt
stuck in ISR and blocking other interrupts.

> +	apic->send_IPI_self(vector);
For level interrupt this is not needed, no?

> +
>  	++vcpu->stat.irq_exits;
>  	return 1;
>  }
> -- 
> 1.7.1

--
			Gleb.