diff mbox

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

Message ID 1355383780-1367-2-git-send-email-yang.z.zhang@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zhang, Yang Z Dec. 13, 2012, 7:29 a.m. UTC
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(-)

Comments

Gleb Natapov Dec. 13, 2012, 7:51 a.m. UTC | #1
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?

> +    
>  }
>  
>  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
Zhang, Yang Z Dec. 13, 2012, 7:54 a.m. UTC | #2
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.

>> +
>>  }
>>  
>>  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


--
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
Gleb Natapov Dec. 13, 2012, 7:58 a.m. UTC | #3
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.

> >> +
> >>  }
> >>  
> >>  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.
--
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
Zhang, Yang Z Dec. 13, 2012, 8:03 a.m. UTC | #4
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?

>>>> +
>>>>  }
>>>>  
>>>>  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


--
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
Gleb Natapov Dec. 13, 2012, 8:05 a.m. UTC | #5
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.
--
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
Zhang, Yang Z Dec. 13, 2012, 8:19 a.m. UTC | #6
Gleb Natapov wrote on 2012-12-13:
> 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.
You are right. We hope to push the PI patch ASAP. If touch common code, it may need long time to discuss to get final decision.
As we discussion early, I will enable this feature in kvm not just when PI is enabled later. At that time, we can get some performance data and to see whether self ipi has big problem. Before the data ready, I think to limit all changes inside KVM modules should be a better way. How do you think?

>>>>>> +
>>>>>>  }
>>>>>>  
>>>>>>  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.
> --
> 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


--
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
Gleb Natapov Dec. 13, 2012, 8:22 a.m. UTC | #7
On Thu, Dec 13, 2012 at 08:19:01AM +0000, Zhang, Yang Z wrote:
> Gleb Natapov wrote on 2012-12-13:
> > 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.
> You are right. We hope to push the PI patch ASAP. If touch common code, it may need long time to discuss to get final decision.
> As we discussion early, I will enable this feature in kvm not just when PI is enabled later. At that time, we can get some performance data and to see whether self ipi has big problem. Before the data ready, I think to limit all changes inside KVM modules should be a better way. How do you think?
> 
I think we have plenty of time till 3.9. We should do it right, not
quick.

--
			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
Zhang, Yang Z Dec. 13, 2012, 8:23 a.m. UTC | #8
Gleb Natapov wrote on 2012-12-13:
> On Thu, Dec 13, 2012 at 08:19:01AM +0000, Zhang, Yang Z wrote:
>> Gleb Natapov wrote on 2012-12-13:
>>> 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.
>> You are right. We hope to push the PI patch ASAP. If touch common code,
>> it may need long time to discuss to get final decision. As we
>> discussion early, I will enable this feature in kvm not just when PI is
> enabled later. At that time, we can get some performance data and to see
> whether self ipi has big problem. Before the data ready, I think to limit all changes
> inside KVM modules should be a better way. How do you think?
>> 
> I think we have plenty of time till 3.9. We should do it right, not
> quick.
Sure. I will change it in next version.

Best regards,
Yang


--
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
Zhang, Yang Z Dec. 16, 2012, 1:26 p.m. UTC | #9
Gleb Natapov wrote on 2012-12-13:
> On Thu, Dec 13, 2012 at 08:19:01AM +0000, Zhang, Yang Z wrote:
>> Gleb Natapov wrote on 2012-12-13:
>>> 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.
>> You are right. We hope to push the PI patch ASAP. If touch common code,
>> it may need long time to discuss to get final decision. As we
>> discussion early, I will enable this feature in kvm not just when PI is
> enabled later. At that time, we can get some performance data and to see
> whether self ipi has big problem. Before the data ready, I think to limit all changes
> inside KVM modules should be a better way. How do you think?
>> 
> I think we have plenty of time till 3.9. We should do it right, not
> quick.
Another choice is to get the IDT entry through IDTR. With this way, we can achieve the same goal but limited the changes inside KVM module.

Best regards,
Yang

--
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
Gleb Natapov Dec. 18, 2012, 9:11 a.m. UTC | #10
On Sun, Dec 16, 2012 at 01:26:41PM +0000, Zhang, Yang Z wrote:
> Gleb Natapov wrote on 2012-12-13:
> > On Thu, Dec 13, 2012 at 08:19:01AM +0000, Zhang, Yang Z wrote:
> >> Gleb Natapov wrote on 2012-12-13:
> >>> 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.
> >> You are right. We hope to push the PI patch ASAP. If touch common code,
> >> it may need long time to discuss to get final decision. As we
> >> discussion early, I will enable this feature in kvm not just when PI is
> > enabled later. At that time, we can get some performance data and to see
> > whether self ipi has big problem. Before the data ready, I think to limit all changes
> > inside KVM modules should be a better way. How do you think?
> >> 
> > I think we have plenty of time till 3.9. We should do it right, not
> > quick.
> Another choice is to get the IDT entry through IDTR. With this way, we can achieve the same goal but limited the changes inside KVM module.
> 
Ture. We already get it during vcpu setup.

--
			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
diff mbox

Patch

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);
+	}
+    
 }
 
 static void vmx_recover_nmi_blocking(struct vcpu_vmx *vmx)