Re: [PATCH v2 5/6] x86: Enable ack interrupt on vmexit

[Gleb Natapov <gleb@redhat.com>]

On Mon, Nov 26, 2012 at 05:44:29AM +0000, Zhang, Yang Z wrote:
> Avi Kivity wrote on 2012-11-25:
> > On 11/25/2012 03:03 PM, Gleb Natapov wrote:
> >> On Sun, Nov 25, 2012 at 02:55:26PM +0200, Avi Kivity wrote:
> >>> On 11/22/2012 05:22 PM, Gleb Natapov wrote:
> >>>> 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.
> >>>>> 
> >>>>> 
> >>>>> 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.
> >>> 
> >>> Why not always?  Better to have a single code path for host interrupts
> >>> (and as Yang notes, the new path is faster as well).
> >>> 
> >> Is it? The current path is:
> >> 
> >> vm exit -> KVM vmexit handler(interrupt disabled) -> KVM re-enable
> >> interrupt -> cpu ack the interrupt and interrupt deliver through the
> >> host IDT.
> >> 
> >> The proposed path is:
> >> 
> >> CPU acks interrupt -> vm exit -> KVM vmexit handler(interrupt disabled)
> >> -> eoi -> self IPI -> KVM re-enable interrupt -> cpu ack the interrupt
> >> and interrupt deliver through the host IDT.
> >> 
> >> Am I missing something?
> > 
> > Yes, you're missing the part where I didn't write that the new path
> > should avoid the IDT and dispatch the interrupt directly, by emulating
> > an interrupt frame directly.  Can be as simple as pushf; push cs; call
> > interrupt_table[vector * 8].  Of course we need to verify that no
> > interrupt uses the IST or a task gate.
> 
> How can we call interrupt table directly? I don't think we can expose the idt_table to a module.
No, but we can add function to entry_(64|32).S that despatch via
idt_table and expose it. Avi's idea is worth to explore before going
self IPI way.

> Anyway, to simply the implementation, I will follow gleb's suggestion: only enable "ack intr on exit" when PI is enabled and self ipi should be enough. Any comments?
> 
> Best regards,
> Yang
> 

--
			Gleb.