Re: [PATCH 1/2] x86: Enable ack interrupt on vmexit

[Gleb Natapov <gleb@redhat.com>]

On Thu, Dec 13, 2012 at 08:03:06AM +0000, Zhang, Yang Z wrote:
> Gleb Natapov wrote on 2012-12-13:
> > On Thu, Dec 13, 2012 at 07:54:35AM +0000, Zhang, Yang Z wrote:
> >> Gleb Natapov wrote on 2012-12-13:
> >>> On Thu, Dec 13, 2012 at 03:29:39PM +0800, Yang Zhang wrote:
> >>>> From: Yang Zhang <yang.z.zhang@Intel.com>
> >>>> 
> >>>> 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. Only
> >>>> enable it when posted interrupt is enabled.
> >>>> 
> >>>> 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 |   20 +++++++++++++++++---
> >>>>  1 files changed, 17 insertions(+), 3 deletions(-)
> >>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> >>>> index 8cd9eb7..6b6bd03 100644
> >>>> --- a/arch/x86/kvm/vmx.c
> >>>> +++ b/arch/x86/kvm/vmx.c
> >>>> @@ -2549,7 +2549,7 @@ 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;
> >>>>  	if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_EXIT_CTLS,
> >>>>  				&_vmexit_control) < 0) 		return -EIO; @@ -3913,6 +3913,7 @@
> >>>>  static int vmx_vcpu_setup(struct vcpu_vmx *vmx) 	unsigned long a;
> >>>>  #endif 	int i;
> >>>> +	u32 vmexit_ctrl = vmcs_config.vmexit_ctrl;
> >>>> 
> >>>>  	/* I/O */ 	vmcs_write64(IO_BITMAP_A, __pa(vmx_io_bitmap_a)); @@
> >>>>  -3996,8 +3997,10 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx)
> >>>>  		vmx->guest_msrs[j].mask = -1ull; 		++vmx->nmsrs; 	}
> >>>> -
> >>>> -	vmcs_write32(VM_EXIT_CONTROLS, vmcs_config.vmexit_ctrl);
> >>>> +
> >>>> +	if(!enable_apicv_pi)
> >>>> +		vmexit_ctrl &= ~VM_EXIT_ACK_INTR_ON_EXIT;
> >>>> +	vmcs_write32(VM_EXIT_CONTROLS, vmexit_ctrl);
> >>>> 
> >>>>  	/* 22.2.1, 20.8.1 */
> >>>>  	vmcs_write32(VM_ENTRY_CONTROLS, vmcs_config.vmentry_ctrl);
> >>>> @@ -6267,6 +6270,17 @@ static void vmx_complete_atomic_exit(struct
> >>> vcpu_vmx *vmx)
> >>>>  		asm("int $2");
> >>>>  		kvm_after_handle_nmi(&vmx->vcpu);
> >>>>  	}
> >>>> +	if ((exit_intr_info & INTR_INFO_INTR_TYPE_MASK) ==
> >>>> INTR_TYPE_EXT_INTR && +	    (exit_intr_info & INTR_INFO_VALID_MASK)
> >>>> && enable_apicv_pi) { +		unsigned int vector, tmr; + +		vector = 
> >>>> exit_intr_info & INTR_INFO_VECTOR_MASK; +		tmr = apic_read(APIC_TMR +
> >>>> ((vector &
> > ~0x1f)
> >>>>>> 1)); +		apic_eoi(); +		if ( !((1 << (vector % 32)) & tmr) )
> >>>> +			apic->send_IPI_self(vector); +	}
> >>> What happen with the idea to dispatch interrupt through idt without IPI?
> >> I am not sure upstream guys will allow to export idt to a module. If it
> >> is not a problem, then can do it as you suggested.
> >> 
> > I replied to that before. No need to export idt to modules. Add function
> > to entry_32/64.S that does dispatching and export it instead.
> It still need to touch common code. Do you think upstream guys will buy-in this?
>
What's the problem with touching common code? Show the code, get the
acks. But wait for merge window to close.
 
> >>>> +
> >>>>  }
> >>>>  
> >>>>  static void vmx_recover_nmi_blocking(struct vcpu_vmx *vmx)
> >>>> --
> >>>> 1.7.1
> >>> 
> >>> --
> >>> 			Gleb.
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe kvm" in
> >>> the body of a message to majordomo@vger.kernel.org
> >>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >> 
> >> 
> >> Best regards,
> >> Yang
> >> 
> > 
> > --
> > 			Gleb.
> 
> 
> Best regards,
> Yang
> 

--
			Gleb.