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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>
>>>>>>>> 
>>>>>>>> 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@xxxxxxxxx>
>>>>>>>> ---
>>>>>>>>  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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux