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