From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Zhang, Yang Z" Subject: RE: [PATCH 1/2] x86: Enable ack interrupt on vmexit Date: Thu, 13 Dec 2012 08:23:27 +0000 Message-ID: References: <1355383780-1367-1-git-send-email-yang.z.zhang@intel.com> <1355383780-1367-2-git-send-email-yang.z.zhang@intel.com> <20121213075119.GU11016@redhat.com> <20121213075814.GV11016@redhat.com> <20121213080548.GW11016@redhat.com> <20121213082201.GX11016@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT Cc: "kvm@vger.kernel.org" , "Shan, Haitao" , "Zhang, Xiantao" , "Nakajima, Jun" , "Anvin, H Peter" To: Gleb Natapov Return-path: Received: from mga01.intel.com ([192.55.52.88]:11466 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752416Ab2LMIXc convert rfc822-to-8bit (ORCPT ); Thu, 13 Dec 2012 03:23:32 -0500 In-Reply-To: <20121213082201.GX11016@redhat.com> Content-Language: en-US Sender: kvm-owner@vger.kernel.org List-ID: 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 >>>>>>>> >>>>>>>> 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 >>>>>>>> --- >>>>>>>> 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