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 <[email protected]>
> >>>>>> 
> >>>>>> 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 <[email protected]>
> >>>>>> ---
> >>>>>>  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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to