From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Zhang, Yang Z" Subject: RE: [PATCH 2/2] x86, apicv: Add Posted Interrupt supporting Date: Fri, 25 Jan 2013 00:40:21 +0000 Message-ID: References: <1355383780-1367-1-git-send-email-yang.z.zhang@intel.com> <1355383780-1367-3-git-send-email-yang.z.zhang@intel.com> <20130124234339.GA18719@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT Cc: "kvm@vger.kernel.org" , "gleb@redhat.com" , "Shan, Haitao" , "Zhang, Xiantao" , "Nakajima, Jun" , "Anvin, H Peter" To: Marcelo Tosatti Return-path: Received: from mga09.intel.com ([134.134.136.24]:18965 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755220Ab3AYAk0 convert rfc822-to-8bit (ORCPT ); Thu, 24 Jan 2013 19:40:26 -0500 In-Reply-To: <20130124234339.GA18719@amt.cnet> Content-Language: en-US Sender: kvm-owner@vger.kernel.org List-ID: Marcelo Tosatti wrote on 2013-01-25: > On Thu, Dec 13, 2012 at 03:29:40PM +0800, Yang Zhang wrote: >> From: Yang Zhang >> >> Posted Interrupt allows APIC interrupts to inject into guest directly >> without any vmexit. >> >> - When delivering a interrupt to guest, if target vcpu is running, >> update Posted-interrupt requests bitmap and send a notification event >> to the vcpu. Then the vcpu will handle this interrupt automatically, >> without any software involvemnt. >> - If target vcpu is not running or there already a notification event >> pending in the vcpu, do nothing. The interrupt will be handled by >> next vm entry. >> Signed-off-by: Yang Zhang >> --- >> arch/x86/include/asm/entry_arch.h | 1 + >> arch/x86/include/asm/hw_irq.h | 1 + arch/x86/include/asm/irq.h >> | 1 + arch/x86/include/asm/irq_vectors.h | 4 + >> arch/x86/include/asm/kvm_host.h | 3 + arch/x86/include/asm/vmx.h >> | 4 + arch/x86/kernel/entry_64.S | 2 + >> arch/x86/kernel/irq.c | 25 +++++++ >> arch/x86/kernel/irqinit.c | 2 + arch/x86/kvm/lapic.c >> | 16 +++- arch/x86/kvm/lapic.h | 1 + >> arch/x86/kvm/vmx.c | 133 >> +++++++++++++++++++++++++++++++++--- 12 files changed, 180 >> insertions(+), 13 deletions(-) >> diff --git a/arch/x86/include/asm/entry_arch.h >> b/arch/x86/include/asm/entry_arch.h index 40afa00..7b0a29e 100644 --- >> a/arch/x86/include/asm/entry_arch.h +++ >> b/arch/x86/include/asm/entry_arch.h @@ -18,6 +18,7 @@ >> BUILD_INTERRUPT(reboot_interrupt,REBOOT_VECTOR) >> #endif >> >> BUILD_INTERRUPT(x86_platform_ipi, X86_PLATFORM_IPI_VECTOR) >> +BUILD_INTERRUPT(posted_intr_ipi, POSTED_INTR_VECTOR) >> >> /* >> * every pentium local APIC has two 'local interrupts', with a >> diff --git a/arch/x86/include/asm/hw_irq.h b/arch/x86/include/asm/hw_irq.h >> index eb92a6e..ee61af3 100644 >> --- a/arch/x86/include/asm/hw_irq.h >> +++ b/arch/x86/include/asm/hw_irq.h >> @@ -28,6 +28,7 @@ >> /* Interrupt handlers registered during init_IRQ */ extern void >> apic_timer_interrupt(void); extern void x86_platform_ipi(void); >> +extern void posted_intr_ipi(void); extern void error_interrupt(void); >> extern void irq_work_interrupt(void); >> diff --git a/arch/x86/include/asm/irq.h b/arch/x86/include/asm/irq.h >> index ba870bb..cff9933 100644 >> --- a/arch/x86/include/asm/irq.h >> +++ b/arch/x86/include/asm/irq.h >> @@ -30,6 +30,7 @@ extern void irq_force_complete_move(int); >> #endif >> >> extern void (*x86_platform_ipi_callback)(void); +extern void >> (*posted_intr_callback)(void); extern void native_init_IRQ(void); >> extern bool handle_irq(unsigned irq, struct pt_regs *regs); >> diff --git a/arch/x86/include/asm/irq_vectors.h >> b/arch/x86/include/asm/irq_vectors.h index 1508e51..8f2e383 100644 --- >> a/arch/x86/include/asm/irq_vectors.h +++ >> b/arch/x86/include/asm/irq_vectors.h @@ -102,6 +102,10 @@ >> */ >> #define X86_PLATFORM_IPI_VECTOR 0xf7 >> +#ifdef CONFIG_HAVE_KVM >> +#define POSTED_INTR_VECTOR 0xf2 >> +#endif >> + >> /* >> * IRQ work vector: >> */ >> diff --git a/arch/x86/include/asm/kvm_host.h >> b/arch/x86/include/asm/kvm_host.h index 7e26d1a..82423a8 100644 --- >> a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h >> @@ -700,6 +700,9 @@ struct kvm_x86_ops { >> int (*has_virtual_interrupt_delivery)(struct kvm_vcpu *vcpu); >> void (*update_irq)(struct kvm_vcpu *vcpu); >> void (*update_eoi_exitmap)(struct kvm_vcpu *vcpu, int vector, bool set); >> + int (*has_posted_interrupt)(struct kvm_vcpu *vcpu); >> + int (*send_nv)(struct kvm_vcpu *vcpu, int vector); >> + void (*update_irr)(struct kvm_vcpu *vcpu); >> int (*set_tss_addr)(struct kvm *kvm, unsigned int addr); >> int (*get_tdp_level)(void); >> u64 (*get_mt_mask)(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio); >> diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h >> index 1003341..7b9e1d0 100644 >> --- a/arch/x86/include/asm/vmx.h >> +++ b/arch/x86/include/asm/vmx.h >> @@ -152,6 +152,7 @@ >> #define PIN_BASED_EXT_INTR_MASK 0x00000001 >> #define PIN_BASED_NMI_EXITING 0x00000008 >> #define PIN_BASED_VIRTUAL_NMIS 0x00000020 >> +#define PIN_BASED_POSTED_INTR 0x00000080 >> >> #define VM_EXIT_SAVE_DEBUG_CONTROLS 0x00000002 #define >> VM_EXIT_HOST_ADDR_SPACE_SIZE 0x00000200 @@ -174,6 +175,7 @@ >> /* VMCS Encodings */ enum vmcs_field { VIRTUAL_PROCESSOR_ID >> = 0x00000000, + POSTED_INTR_NV = 0x00000002, >> GUEST_ES_SELECTOR = 0x00000800, GUEST_CS_SELECTOR >> = 0x00000802, GUEST_SS_SELECTOR = 0x00000804, >> @@ -208,6 +210,8 @@ enum vmcs_field { VIRTUAL_APIC_PAGE_ADDR_HIGH >> = 0x00002013, APIC_ACCESS_ADDR = 0x00002014, >> APIC_ACCESS_ADDR_HIGH = 0x00002015, >> + POSTED_INTR_DESC_ADDR = 0x00002016, >> + POSTED_INTR_DESC_ADDR_HIGH = 0x00002017, >> EPT_POINTER = 0x0000201a, >> EPT_POINTER_HIGH = 0x0000201b, >> EOI_EXIT_BITMAP0 = 0x0000201c, >> diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S >> index b51b2c7..d06eea1 100644 >> --- a/arch/x86/kernel/entry_64.S >> +++ b/arch/x86/kernel/entry_64.S >> @@ -1160,6 +1160,8 @@ apicinterrupt LOCAL_TIMER_VECTOR \ >> apic_timer_interrupt smp_apic_timer_interrupt >> apicinterrupt X86_PLATFORM_IPI_VECTOR \ >> x86_platform_ipi smp_x86_platform_ipi >> +apicinterrupt POSTED_INTR_VECTOR \ >> + posted_intr_ipi smp_posted_intr_ipi >> >> apicinterrupt THRESHOLD_APIC_VECTOR \ >> threshold_interrupt smp_threshold_interrupt >> diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c >> index e4595f1..781d324 100644 >> --- a/arch/x86/kernel/irq.c >> +++ b/arch/x86/kernel/irq.c >> @@ -22,6 +22,9 @@ atomic_t irq_err_count; >> >> /* Function pointer for generic interrupt vector handling */ >> void (*x86_platform_ipi_callback)(void) = NULL; >> +/* Function pointer for posted interrupt vector handling */ >> +void (*posted_intr_callback)(void) = NULL; >> +EXPORT_SYMBOL_GPL(posted_intr_callback); >> >> /* >> * 'what should we do if we get a hw irq event on an illegal vector'. >> @@ -228,6 +231,28 @@ void smp_x86_platform_ipi(struct pt_regs *regs) >> set_irq_regs(old_regs); >> } >> +/* >> + * Handler for POSTED_INTERRUPT_VECTOR. >> + */ >> +void smp_posted_intr_ipi(struct pt_regs *regs) >> +{ >> + struct pt_regs *old_regs = set_irq_regs(regs); >> + >> + ack_APIC_irq(); >> + >> + irq_enter(); >> + >> + exit_idle(); >> + >> + if (posted_intr_callback) >> + posted_intr_callback(); >> + >> + irq_exit(); >> + >> + set_irq_regs(old_regs); >> +} >> + >> + >> EXPORT_SYMBOL_GPL(vector_used_by_percpu_irq); >> >> #ifdef CONFIG_HOTPLUG_CPU >> diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c >> index 6e03b0d..d15ca4f 100644 >> --- a/arch/x86/kernel/irqinit.c >> +++ b/arch/x86/kernel/irqinit.c >> @@ -205,6 +205,8 @@ static void __init apic_intr_init(void) >> >> /* IPI for X86 platform specific use */ >> alloc_intr_gate(X86_PLATFORM_IPI_VECTOR, x86_platform_ipi); >> + /* IPI for posted interrupt use */ >> + alloc_intr_gate(POSTED_INTR_VECTOR, posted_intr_ipi); >> >> /* IPI vectors for APIC spurious and error interrupts */ >> alloc_intr_gate(SPURIOUS_APIC_VECTOR, spurious_interrupt); >> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c >> index 2109a6a..d660b9d 100644 >> --- a/arch/x86/kvm/lapic.c >> +++ b/arch/x86/kvm/lapic.c >> @@ -350,6 +350,7 @@ static inline int apic_find_highest_irr(struct kvm_lapic > *apic) >> if (!apic->irr_pending) >> return -1; >> + kvm_x86_ops->update_irr(apic->vcpu); >> result = apic_search_irr(apic); >> ASSERT(result == -1 || result >= 16); >> @@ -725,18 +726,25 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int > delivery_mode, >> if (trig_mode) { >> apic_debug("level trig mode for vector %d", vector); >> apic_set_vector(vector, apic->regs + APIC_TMR); >> - } else >> + } else { >> apic_clear_vector(vector, apic->regs + APIC_TMR); >> - >> + if (kvm_x86_ops->has_posted_interrupt(vcpu)) { >> + result = 1; >> + apic->irr_pending = true; >> + kvm_x86_ops->send_nv(vcpu, vector); >> + goto out; >> + } > > Hi, > > Steps 4, 5 and 6 of section 29.6 are executed in both VMX root/non-root > modes, or only non-root mode? SDM doesn't tell. But we don't need know this in software level. > > > If only non-root mode, there is a problem if target vcpu<->pcpu vm-exits > before receiving and acking the interrupt. In that case PIR set bits are > not transferred to VIRR. > > It would be necessary to read notification bit on VM-exit and, if set, > do PIR->VIRR transfer in software. The downside, is lack of an atomic In current implementation, it will sync PIR to VIRR before vmentry. > (VIRR |= PIR; PIR = 0) in software. So it would require synchronization > to KVM APIC injection (which ATM relies on atomic test_and_set of IRR). Best regards, Yang