From: Yang Zhang <yang.z.zhang@Intel.com> The follwoing patches are adding the Posted Interrupt supporting to Xen: Posted Interrupt allows vAPIC 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 Refer to Intel SDM vol3, 29.6 to get more information. Changes from v1 to v2: * Allocate posted interrupt vector dynamically. * Use vlapic_set_vector() to sync pir to irr. * Refine vlapic_set_irq() and call it to deliver virtual interrupt. * Rebase on top of Xen. Yang Zhang (5): VMX: Detect posted interrupt capability VMX: Turn on posted interrupt bit in vmcs VMX: Add posted interrupt supporting Call vlapic_set_irq() to delivery virtual interrupt VMX: Use posted interrupt to deliver virutal interrupt xen/arch/x86/hvm/vioapic.c | 3 +- xen/arch/x86/hvm/vlapic.c | 38 +++++--------- xen/arch/x86/hvm/vmsi.c | 3 +- xen/arch/x86/hvm/vmx/vmcs.c | 18 ++++++- xen/arch/x86/hvm/vmx/vmx.c | 70 ++++++++++++++++++++++++ xen/include/asm-x86/bitops.h | 10 ++++ xen/include/asm-x86/hvm/hvm.h | 2 + xen/include/asm-x86/hvm/vlapic.h | 20 +++++++- xen/include/asm-x86/hvm/vmx/vmcs.h | 13 +++++ xen/include/asm-x86/hvm/vmx/vmx.h | 27 +++++++++ xen/include/asm-x86/mach-default/irq_vectors.h | 3 +- 11 files changed, 175 insertions(+), 32 deletions(-)
From: Yang Zhang <yang.z.zhang@Intel.com> Check whether the Hardware supports posted interrupt capability. Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com> Reviewed-by: Jun Nakajima <jun.nakajima@intel.com> --- xen/arch/x86/hvm/vmx/vmcs.c | 12 +++++++++++- xen/include/asm-x86/hvm/vmx/vmcs.h | 6 ++++++ 2 files changed, 17 insertions(+), 1 deletions(-) diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c index 9926ffb..a88a548 100644 --- a/xen/arch/x86/hvm/vmx/vmcs.c +++ b/xen/arch/x86/hvm/vmx/vmcs.c @@ -92,6 +92,7 @@ static void __init vmx_display_features(void) P(cpu_has_vmx_unrestricted_guest, "Unrestricted Guest"); P(cpu_has_vmx_apic_reg_virt, "APIC Register Virtualization"); P(cpu_has_vmx_virtual_intr_delivery, "Virtual Interrupt Delivery"); + P(cpu_has_vmx_posted_intr_processing, "Posted Interrupt Processing"); P(cpu_has_vmx_vmcs_shadowing, "VMCS shadowing"); #undef P @@ -143,7 +144,8 @@ static int vmx_init_vmcs_config(void) min = (PIN_BASED_EXT_INTR_MASK | PIN_BASED_NMI_EXITING); - opt = PIN_BASED_VIRTUAL_NMIS; + opt = (PIN_BASED_VIRTUAL_NMIS | + PIN_BASED_POSTED_INTERRUPT); _vmx_pin_based_exec_control = adjust_vmx_controls( "Pin-Based Exec Control", min, opt, MSR_IA32_VMX_PINBASED_CTLS, &mismatch); @@ -269,6 +271,14 @@ static int vmx_init_vmcs_config(void) _vmx_vmexit_control = adjust_vmx_controls( "VMExit Control", min, opt, MSR_IA32_VMX_EXIT_CTLS, &mismatch); + /* + * "Process posted interrupt" can be set only when "virtual-interrupt + * delivery" and "acknowledge interrupt on exit" is set + */ + if ( !(_vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY) + || !(_vmx_vmexit_control & VM_EXIT_ACK_INTR_ON_EXIT) ) + _vmx_pin_based_exec_control &= ~ PIN_BASED_POSTED_INTERRUPT; + min = 0; opt = VM_ENTRY_LOAD_GUEST_PAT; _vmx_vmentry_control = adjust_vmx_controls( diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h index 37e6734..3a5c91a 100644 --- a/xen/include/asm-x86/hvm/vmx/vmcs.h +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h @@ -165,6 +165,7 @@ extern u32 vmx_cpu_based_exec_control; #define PIN_BASED_NMI_EXITING 0x00000008 #define PIN_BASED_VIRTUAL_NMIS 0x00000020 #define PIN_BASED_PREEMPT_TIMER 0x00000040 +#define PIN_BASED_POSTED_INTERRUPT 0x00000080 extern u32 vmx_pin_based_exec_control; #define VM_EXIT_SAVE_DEBUG_CNTRLS 0x00000004 @@ -256,6 +257,8 @@ extern bool_t cpu_has_vmx_ins_outs_instr_info; (vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY) #define cpu_has_vmx_virtualize_x2apic_mode \ (vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE) +#define cpu_has_vmx_posted_intr_processing \ + (vmx_pin_based_exec_control & PIN_BASED_POSTED_INTERRUPT) #define cpu_has_vmx_vmcs_shadowing \ (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VMCS_SHADOWING) @@ -280,6 +283,7 @@ extern bool_t cpu_has_vmx_ins_outs_instr_info; /* VMCS field encodings. */ enum vmcs_field { VIRTUAL_PROCESSOR_ID = 0x00000000, + POSTED_INTR_NOTIFICATION_VECTOR = 0x00000002, GUEST_ES_SELECTOR = 0x00000800, GUEST_CS_SELECTOR = 0x00000802, GUEST_SS_SELECTOR = 0x00000804, @@ -314,6 +318,8 @@ enum vmcs_field { VIRTUAL_APIC_PAGE_ADDR_HIGH = 0x00002013, APIC_ACCESS_ADDR = 0x00002014, APIC_ACCESS_ADDR_HIGH = 0x00002015, + PI_DESC_ADDR = 0x00002016, + PI_DESC_ADDR_HIGH = 0x00002017, EPT_POINTER = 0x0000201a, EPT_POINTER_HIGH = 0x0000201b, EOI_EXIT_BITMAP0 = 0x0000201c, -- 1.7.1
From: Yang Zhang <yang.z.zhang@Intel.com> Turn on posted interrupt for vcpu if posted interrupt is avaliable. Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com> Reviewed-by: Jun Nakajima <jun.nakajima@intel.com> --- xen/arch/x86/hvm/vmx/vmcs.c | 6 ++++++ xen/arch/x86/hvm/vmx/vmx.c | 5 +++++ xen/include/asm-x86/hvm/vmx/vmcs.h | 7 +++++++ xen/include/asm-x86/mach-default/irq_vectors.h | 3 ++- 4 files changed, 20 insertions(+), 1 deletions(-) diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c index a88a548..fb1be8d 100644 --- a/xen/arch/x86/hvm/vmx/vmcs.c +++ b/xen/arch/x86/hvm/vmx/vmcs.c @@ -926,6 +926,12 @@ static int construct_vmcs(struct vcpu *v) __vmwrite(GUEST_INTR_STATUS, 0); } + if ( cpu_has_vmx_posted_intr_processing ) + { + __vmwrite(PI_DESC_ADDR, virt_to_maddr(&v->arch.hvm_vmx.pi_desc)); + __vmwrite(POSTED_INTR_NOTIFICATION_VECTOR, POSTED_INTERRUPT_VECTOR); + } + /* Host data selectors. */ __vmwrite(HOST_SS_SELECTOR, __HYPERVISOR_DS); __vmwrite(HOST_DS_SELECTOR, __HYPERVISOR_DS); diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index a869ed4..8e1c06f 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -75,6 +75,8 @@ static int vmx_msr_read_intercept(unsigned int msr, uint64_t *msr_content); static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content); static void vmx_invlpg_intercept(unsigned long vaddr); +uint8_t posted_intr_vector; + static int vmx_domain_initialise(struct domain *d) { int rc; @@ -1523,6 +1525,9 @@ struct hvm_function_table * __init start_vmx(void) setup_ept_dump(); } + + if ( cpu_has_vmx_posted_intr_processing ) + alloc_direct_apic_vector(&posted_intr_vector, event_check_interrupt); setup_vmcs_dump(); diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h index 3a5c91a..c77eca1 100644 --- a/xen/include/asm-x86/hvm/vmx/vmcs.h +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h @@ -73,6 +73,12 @@ struct vmx_domain { unsigned long apic_access_mfn; }; +struct pi_desc { + u64 pir[4]; + u32 control; + u32 rsvd[7]; +} __attribute__ ((aligned (64))); + #define ept_get_wl(ept) ((ept)->ept_wl) #define ept_get_asr(ept) ((ept)->asr) #define ept_get_eptp(ept) ((ept)->eptp) @@ -113,6 +119,7 @@ struct arch_vmx_struct { uint32_t eoi_exitmap_changed; uint64_t eoi_exit_bitmap[4]; + struct pi_desc pi_desc; unsigned long host_cr0; diff --git a/xen/include/asm-x86/mach-default/irq_vectors.h b/xen/include/asm-x86/mach-default/irq_vectors.h index 992e00c..34c2f44 100644 --- a/xen/include/asm-x86/mach-default/irq_vectors.h +++ b/xen/include/asm-x86/mach-default/irq_vectors.h @@ -9,12 +9,13 @@ #define CALL_FUNCTION_VECTOR 0xfb #define LOCAL_TIMER_VECTOR 0xfa #define PMU_APIC_VECTOR 0xf9 +#define POSTED_INTERRUPT_VECTOR 0xf8 /* * High-priority dynamically-allocated vectors. For interrupts that * must be higher priority than any guest-bound interrupt. */ #define FIRST_HIPRIORITY_VECTOR 0xf1 -#define LAST_HIPRIORITY_VECTOR 0xf8 +#define LAST_HIPRIORITY_VECTOR 0xf7 /* IRQ0 (timer) is statically allocated but must be high priority. */ #define IRQ0_VECTOR 0xf0 -- 1.7.1
From: Yang Zhang <yang.z.zhang@Intel.com> Add the supporting of using posted interrupt to deliver interrupt. Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com> Reviewed-by: Jun Nakajima <jun.nakajima@intel.com> --- xen/arch/x86/hvm/vlapic.c | 18 ---------- xen/arch/x86/hvm/vmx/vmx.c | 65 +++++++++++++++++++++++++++++++++++++ xen/include/asm-x86/bitops.h | 10 ++++++ xen/include/asm-x86/hvm/hvm.h | 2 + xen/include/asm-x86/hvm/vlapic.h | 18 ++++++++++ xen/include/asm-x86/hvm/vmx/vmx.h | 27 +++++++++++++++ 6 files changed, 122 insertions(+), 18 deletions(-) diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c index 4b25cc8..f241a7c 100644 --- a/xen/arch/x86/hvm/vlapic.c +++ b/xen/arch/x86/hvm/vlapic.c @@ -90,24 +90,6 @@ static const unsigned int vlapic_lvt_mask[VLAPIC_LVT_NUM] ((vlapic_get_reg(vlapic, APIC_LVTT) & APIC_TIMER_MODE_MASK) \ == APIC_TIMER_MODE_TSC_DEADLINE) - -/* - * Generic APIC bitmap vector update & search routines. - */ - -#define VEC_POS(v) ((v)%32) -#define REG_POS(v) (((v)/32) * 0x10) -#define vlapic_test_and_set_vector(vec, bitmap) \ - test_and_set_bit(VEC_POS(vec), \ - (unsigned long *)((bitmap) + REG_POS(vec))) -#define vlapic_test_and_clear_vector(vec, bitmap) \ - test_and_clear_bit(VEC_POS(vec), \ - (unsigned long *)((bitmap) + REG_POS(vec))) -#define vlapic_set_vector(vec, bitmap) \ - set_bit(VEC_POS(vec), (unsigned long *)((bitmap) + REG_POS(vec))) -#define vlapic_clear_vector(vec, bitmap) \ - clear_bit(VEC_POS(vec), (unsigned long *)((bitmap) + REG_POS(vec))) - static int vlapic_find_highest_vector(void *bitmap) { uint32_t *word = bitmap; diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 8e1c06f..938e653 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -55,6 +55,7 @@ #include <asm/debugger.h> #include <asm/apic.h> #include <asm/hvm/nestedhvm.h> +#include <asm/event.h> enum handler_return { HNDL_done, HNDL_unhandled, HNDL_exception_raised }; @@ -1449,6 +1450,63 @@ static void vmx_process_isr(int isr, struct vcpu *v) vmx_vmcs_exit(v); } +static void __vmx_deliver_posted_interrupt(struct vcpu *v) +{ + bool_t running; + + running = v->is_running; + vcpu_unblock(v); + if ( running && (in_irq() || (v != current)) ) + { + unsigned int cpu = v->processor; + + if ( !test_and_set_bit(VCPU_KICK_SOFTIRQ, &softirq_pending(cpu)) + && (cpu != smp_processor_id()) ) + send_IPI_mask(cpumask_of(cpu), posted_intr_vector); + } +} + +static void vmx_deliver_posted_intr(struct vcpu *v, u8 vector) +{ + if ( pi_test_and_set_pir(vector, &v->arch.hvm_vmx.pi_desc) ) + return; + + if ( unlikely(v->arch.hvm_vmx.eoi_exitmap_changed) ) + { + /* If EOI exitbitmap needs to changed or notification vector + * can''t be allocated, interrupt will not be injected till + * VMEntry as it used to be + */ + pi_set_on(&v->arch.hvm_vmx.pi_desc); + goto out; + } + + if ( !pi_test_and_set_on(&v->arch.hvm_vmx.pi_desc) ) + { + __vmx_deliver_posted_interrupt(v); + return; + } + +out: + vcpu_kick(v); +} + +static void vmx_sync_pir_to_irr(struct vcpu *v) +{ + struct vlapic *vlapic = vcpu_vlapic(v); + u64 val[4]; + u32 group, i; + + if ( !pi_test_and_clear_on(&v->arch.hvm_vmx.pi_desc) ) + return; + + for ( group = 0; group < 4; group++ ) + val[group] = pi_get_pir(&v->arch.hvm_vmx.pi_desc, group); + + for_each_set_bit(i, val, MAX_VECTOR) + vlapic_set_vector(i, &vlapic->regs->data[APIC_IRR]); +} + static struct hvm_function_table __read_mostly vmx_function_table = { .name = "VMX", .cpu_up_prepare = vmx_cpu_up_prepare, @@ -1499,6 +1557,8 @@ static struct hvm_function_table __read_mostly vmx_function_table = { .update_eoi_exit_bitmap = vmx_update_eoi_exit_bitmap, .virtual_intr_delivery_enabled = vmx_virtual_intr_delivery_enabled, .process_isr = vmx_process_isr, + .deliver_posted_intr = vmx_deliver_posted_intr, + .sync_pir_to_irr = vmx_sync_pir_to_irr, .nhvm_hap_walk_L1_p2m = nvmx_hap_walk_L1_p2m, }; @@ -1528,6 +1588,11 @@ struct hvm_function_table * __init start_vmx(void) if ( cpu_has_vmx_posted_intr_processing ) alloc_direct_apic_vector(&posted_intr_vector, event_check_interrupt); + else + { + hvm_funcs.deliver_posted_intr = NULL; + hvm_funcs.sync_pir_to_irr = NULL; + } setup_vmcs_dump(); diff --git a/xen/include/asm-x86/bitops.h b/xen/include/asm-x86/bitops.h index 2bbd169..46b6cd1 100644 --- a/xen/include/asm-x86/bitops.h +++ b/xen/include/asm-x86/bitops.h @@ -367,6 +367,16 @@ static inline unsigned int __scanbit(unsigned long val, unsigned long max) ((off)+(__scanbit(~(((*(const unsigned long *)addr)) >> (off)), size))) : \ __find_next_zero_bit(addr,size,off))) +/** + * for_each_set_bit - iterate over every set bit in a memory region + * @bit: The integer iterator + * @addr: The address to base the search on + * @size: The maximum size to search + */ +#define for_each_set_bit(bit, addr, size) \ + for ( (bit) = find_first_bit((addr), (size)); \ + (bit) < (size); \ + (bit) = find_next_bit((addr), (size), (bit) + 1) ) /** * find_first_set_bit - find the first set bit in @word diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h index 2fa2ea5..85dc85b 100644 --- a/xen/include/asm-x86/hvm/hvm.h +++ b/xen/include/asm-x86/hvm/hvm.h @@ -184,6 +184,8 @@ struct hvm_function_table { void (*update_eoi_exit_bitmap)(struct vcpu *v, u8 vector, u8 trig); int (*virtual_intr_delivery_enabled)(void); void (*process_isr)(int isr, struct vcpu *v); + void (*deliver_posted_intr)(struct vcpu *v, u8 vector); + void (*sync_pir_to_irr)(struct vcpu *v); /*Walk nested p2m */ int (*nhvm_hap_walk_L1_p2m)(struct vcpu *v, paddr_t L2_gpa, diff --git a/xen/include/asm-x86/hvm/vlapic.h b/xen/include/asm-x86/hvm/vlapic.h index 101ef57..82a84da 100644 --- a/xen/include/asm-x86/hvm/vlapic.h +++ b/xen/include/asm-x86/hvm/vlapic.h @@ -54,6 +54,23 @@ #define vlapic_x2apic_mode(vlapic) \ ((vlapic)->hw.apic_base_msr & MSR_IA32_APICBASE_EXTD) +/* + * Generic APIC bitmap vector update & search routines. + */ + +#define VEC_POS(v) ((v)%32) +#define REG_POS(v) (((v)/32) * 0x10) +#define vlapic_test_and_set_vector(vec, bitmap) \ + test_and_set_bit(VEC_POS(vec), \ + (unsigned long *)((bitmap) + REG_POS(vec))) +#define vlapic_test_and_clear_vector(vec, bitmap) \ + test_and_clear_bit(VEC_POS(vec), \ + (unsigned long *)((bitmap) + REG_POS(vec))) +#define vlapic_set_vector(vec, bitmap) \ + set_bit(VEC_POS(vec), (unsigned long *)((bitmap) + REG_POS(vec))) +#define vlapic_clear_vector(vec, bitmap) \ + clear_bit(VEC_POS(vec), (unsigned long *)((bitmap) + REG_POS(vec))) + struct vlapic { struct hvm_hw_lapic hw; struct hvm_hw_lapic_regs *regs; @@ -104,6 +121,7 @@ void vlapic_handle_EOI_induced_exit(struct vlapic *vlapic, int vector); void vlapic_ipi(struct vlapic *vlapic, uint32_t icr_low, uint32_t icr_high); int vlapic_apicv_write(struct vcpu *v, unsigned int offset); +void vlapic_set_tmr(struct vlapic *vlapic, uint8_t vec, uint8_t trig); struct vlapic *vlapic_lowest_prio( struct domain *d, struct vlapic *source, diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h b/xen/include/asm-x86/hvm/vmx/vmx.h index d4d6feb..1bceada 100644 --- a/xen/include/asm-x86/hvm/vmx/vmx.h +++ b/xen/include/asm-x86/hvm/vmx/vmx.h @@ -99,6 +99,33 @@ void vmx_update_exception_bitmap(struct vcpu *v); void vmx_update_cpu_exec_control(struct vcpu *v); void vmx_update_secondary_exec_control(struct vcpu *v); +#define POSTED_INTR_ON 0 +static inline int pi_test_and_set_pir(int vector, struct pi_desc *pi_desc) +{ + return test_and_set_bit(vector, (unsigned long *)pi_desc->pir); +} + +static inline int pi_test_and_set_on(struct pi_desc *pi_desc) +{ + return test_and_set_bit(POSTED_INTR_ON, + (unsigned long *)&pi_desc->control); +} + +static inline void pi_set_on(struct pi_desc *pi_desc) +{ + return set_bit(POSTED_INTR_ON, (unsigned long *)&pi_desc->control); +} + +static inline int pi_test_and_clear_on(struct pi_desc *pi_desc) +{ + return test_and_clear_bit(POSTED_INTR_ON, + (unsigned long *)&pi_desc->control); +} + +static inline u64 pi_get_pir(struct pi_desc *pi_desc, int group) +{ + return xchg(&pi_desc->pir[group], 0); +} /* * Exit Reasons -- 1.7.1
Yang Zhang
2013-Apr-17 06:51 UTC
[PATCH v2 4/5] Call vlapic_set_irq() to delivery virtual interrupt
From: Yang Zhang <yang.z.zhang@Intel.com> Move kick_vcpu into vlapic_set_irq. And call it to deliver virtual interrupt instead set vIRR directly. Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com> --- xen/arch/x86/hvm/vioapic.c | 3 +-- xen/arch/x86/hvm/vlapic.c | 14 ++++++++------ xen/arch/x86/hvm/vmsi.c | 3 +-- xen/include/asm-x86/hvm/vlapic.h | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/xen/arch/x86/hvm/vioapic.c b/xen/arch/x86/hvm/vioapic.c index d3de695..d3c681b 100644 --- a/xen/arch/x86/hvm/vioapic.c +++ b/xen/arch/x86/hvm/vioapic.c @@ -263,8 +263,7 @@ static void ioapic_inj_irq( ASSERT((delivery_mode == dest_Fixed) || (delivery_mode == dest_LowestPrio)); - if ( vlapic_set_irq(target, vector, trig_mode) ) - vcpu_kick(vlapic_vcpu(target)); + vlapic_set_irq(target, vector, trig_mode); } static inline int pit_channel0_enabled(void) diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c index f241a7c..20c8fe3 100644 --- a/xen/arch/x86/hvm/vlapic.c +++ b/xen/arch/x86/hvm/vlapic.c @@ -122,16 +122,19 @@ static int vlapic_find_highest_irr(struct vlapic *vlapic) return vlapic_find_highest_vector(&vlapic->regs->data[APIC_IRR]); } -int vlapic_set_irq(struct vlapic *vlapic, uint8_t vec, uint8_t trig) +void vlapic_set_irq(struct vlapic *vlapic, uint8_t vec, uint8_t trig) { + struct vcpu *target = vlapic_vcpu(vlapic); + if ( trig ) vlapic_set_vector(vec, &vlapic->regs->data[APIC_TMR]); if ( hvm_funcs.update_eoi_exit_bitmap ) - hvm_funcs.update_eoi_exit_bitmap(vlapic_vcpu(vlapic), vec ,trig); + hvm_funcs.update_eoi_exit_bitmap(target, vec, trig); /* We may need to wake up target vcpu, besides set pending bit here */ - return !vlapic_test_and_set_irr(vec, vlapic); + if ( !vlapic_test_and_set_irr(vec, vlapic) ) + vcpu_kick(target); } static int vlapic_find_highest_isr(struct vlapic *vlapic) @@ -297,9 +300,8 @@ static void vlapic_accept_irq(struct vcpu *v, uint32_t icr_low) { case APIC_DM_FIXED: case APIC_DM_LOWEST: - if ( vlapic_enabled(vlapic) && - !vlapic_test_and_set_irr(vector, vlapic) ) - vcpu_kick(v); + if ( vlapic_enabled(vlapic) ) + vlapic_set_irq(vlapic, vector, 0); break; case APIC_DM_REMRD: diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c index cfc7c80..36de312 100644 --- a/xen/arch/x86/hvm/vmsi.c +++ b/xen/arch/x86/hvm/vmsi.c @@ -57,8 +57,7 @@ static void vmsi_inj_irq( { case dest_Fixed: case dest_LowestPrio: - if ( vlapic_set_irq(target, vector, trig_mode) ) - vcpu_kick(vlapic_vcpu(target)); + vlapic_set_irq(target, vector, trig_mode); break; default: gdprintk(XENLOG_WARNING, "error delivery mode %d\n", delivery_mode); diff --git a/xen/include/asm-x86/hvm/vlapic.h b/xen/include/asm-x86/hvm/vlapic.h index 82a84da..984b564 100644 --- a/xen/include/asm-x86/hvm/vlapic.h +++ b/xen/include/asm-x86/hvm/vlapic.h @@ -97,7 +97,7 @@ static inline void vlapic_set_reg( bool_t is_vlapic_lvtpc_enabled(struct vlapic *vlapic); -int vlapic_set_irq(struct vlapic *vlapic, uint8_t vec, uint8_t trig); +void vlapic_set_irq(struct vlapic *vlapic, uint8_t vec, uint8_t trig); int vlapic_has_pending_irq(struct vcpu *v); int vlapic_ack_pending_irq(struct vcpu *v, int vector); -- 1.7.1
Yang Zhang
2013-Apr-17 06:51 UTC
[PATCH v2 5/5] VMX: Use posted interrupt to deliver virutal interrupt
From: Yang Zhang <yang.z.zhang@Intel.com> Deliver virtual interrupt through posted way if posted interrupt is enabled. Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com> --- xen/arch/x86/hvm/vlapic.c | 10 +++++++--- 1 files changed, 7 insertions(+), 3 deletions(-) diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c index 20c8fe3..e66fdee 100644 --- a/xen/arch/x86/hvm/vlapic.c +++ b/xen/arch/x86/hvm/vlapic.c @@ -119,6 +119,9 @@ static void vlapic_clear_irr(int vector, struct vlapic *vlapic) static int vlapic_find_highest_irr(struct vlapic *vlapic) { + if ( hvm_funcs.sync_pir_to_irr ) + hvm_funcs.sync_pir_to_irr(vlapic_vcpu(vlapic)); + return vlapic_find_highest_vector(&vlapic->regs->data[APIC_IRR]); } @@ -132,9 +135,10 @@ void vlapic_set_irq(struct vlapic *vlapic, uint8_t vec, uint8_t trig) if ( hvm_funcs.update_eoi_exit_bitmap ) hvm_funcs.update_eoi_exit_bitmap(target, vec, trig); - /* We may need to wake up target vcpu, besides set pending bit here */ - if ( !vlapic_test_and_set_irr(vec, vlapic) ) - vcpu_kick(target); + if ( hvm_funcs.deliver_posted_intr ) + hvm_funcs.deliver_posted_intr(target, vec); + else if ( !vlapic_test_and_set_irr(vec, vlapic) ) + vcpu_kick(vlapic_vcpu(vlapic)); } static int vlapic_find_highest_isr(struct vlapic *vlapic) -- 1.7.1
On 17/04/2013 07:50, "Yang Zhang" <yang.z.zhang@intel.com> wrote:> From: Yang Zhang <yang.z.zhang@Intel.com>The whole series: Acked-by: Keir Fraser <keir@xen.org>> The follwoing patches are adding the Posted Interrupt supporting to Xen: > Posted Interrupt allows vAPIC 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 > > Refer to Intel SDM vol3, 29.6 to get more information. > > Changes from v1 to v2: > * Allocate posted interrupt vector dynamically. > * Use vlapic_set_vector() to sync pir to irr. > * Refine vlapic_set_irq() and call it to deliver virtual interrupt. > * Rebase on top of Xen. > > Yang Zhang (5): > VMX: Detect posted interrupt capability > VMX: Turn on posted interrupt bit in vmcs > VMX: Add posted interrupt supporting > Call vlapic_set_irq() to delivery virtual interrupt > VMX: Use posted interrupt to deliver virutal interrupt > > xen/arch/x86/hvm/vioapic.c | 3 +- > xen/arch/x86/hvm/vlapic.c | 38 +++++--------- > xen/arch/x86/hvm/vmsi.c | 3 +- > xen/arch/x86/hvm/vmx/vmcs.c | 18 ++++++- > xen/arch/x86/hvm/vmx/vmx.c | 70 > ++++++++++++++++++++++++ > xen/include/asm-x86/bitops.h | 10 ++++ > xen/include/asm-x86/hvm/hvm.h | 2 + > xen/include/asm-x86/hvm/vlapic.h | 20 +++++++- > xen/include/asm-x86/hvm/vmx/vmcs.h | 13 +++++ > xen/include/asm-x86/hvm/vmx/vmx.h | 27 +++++++++ > xen/include/asm-x86/mach-default/irq_vectors.h | 3 +- > 11 files changed, 175 insertions(+), 32 deletions(-) >
Jan Beulich
2013-Apr-17 09:02 UTC
Re: [PATCH v2 2/5] VMX: Turn on posted interrupt bit in vmcs
>>> On 17.04.13 at 08:50, Yang Zhang <yang.z.zhang@intel.com> wrote: > --- a/xen/arch/x86/hvm/vmx/vmcs.c > +++ b/xen/arch/x86/hvm/vmx/vmcs.c > @@ -926,6 +926,12 @@ static int construct_vmcs(struct vcpu *v) > __vmwrite(GUEST_INTR_STATUS, 0); > } > > + if ( cpu_has_vmx_posted_intr_processing ) > + { > + __vmwrite(PI_DESC_ADDR, virt_to_maddr(&v->arch.hvm_vmx.pi_desc)); > + __vmwrite(POSTED_INTR_NOTIFICATION_VECTOR, POSTED_INTERRUPT_VECTOR);Shouldn''t this be posted_intr_vector?> --- a/xen/include/asm-x86/mach-default/irq_vectors.h > +++ b/xen/include/asm-x86/mach-default/irq_vectors.h > @@ -9,12 +9,13 @@ > #define CALL_FUNCTION_VECTOR 0xfb > #define LOCAL_TIMER_VECTOR 0xfa > #define PMU_APIC_VECTOR 0xf9 > +#define POSTED_INTERRUPT_VECTOR 0xf8 > /* > * High-priority dynamically-allocated vectors. For interrupts that > * must be higher priority than any guest-bound interrupt. > */ > #define FIRST_HIPRIORITY_VECTOR 0xf1 > -#define LAST_HIPRIORITY_VECTOR 0xf8 > +#define LAST_HIPRIORITY_VECTOR 0xf7 > /* IRQ0 (timer) is statically allocated but must be high priority. */ > #define IRQ0_VECTOR 0xf0And these changes should both no longer be necessary. Or am I mis-reading "Allocate posted interrupt vector dynamically" in your overview mail? Jan
>>> On 17.04.13 at 08:51, Yang Zhang <yang.z.zhang@intel.com> wrote: > +static void vmx_sync_pir_to_irr(struct vcpu *v) > +{ > + struct vlapic *vlapic = vcpu_vlapic(v); > + u64 val[4];This should be DECLARE_BITMAP(), ...> + u32 group, i; > + > + if ( !pi_test_and_clear_on(&v->arch.hvm_vmx.pi_desc) ) > + return; > + > + for ( group = 0; group < 4; group++ )... and use ARRAY_SIZE() here.> + val[group] = pi_get_pir(&v->arch.hvm_vmx.pi_desc, group);Which will require adjustments to the types underlying this call.> + > + for_each_set_bit(i, val, MAX_VECTOR)I''d also favor you not adding new uses of this redundant and mis-named #define - please use NR_VECTORS.> +#define for_each_set_bit(bit, addr, size) \ > + for ( (bit) = find_first_bit((addr), (size)); \ > + (bit) < (size); \ > + (bit) = find_next_bit((addr), (size), (bit) + 1) )Please drop the pointless parentheses (but keep the ones that are needed).> --- a/xen/include/asm-x86/hvm/vmx/vmx.h > +++ b/xen/include/asm-x86/hvm/vmx/vmx.h > @@ -99,6 +99,33 @@ void vmx_update_exception_bitmap(struct vcpu *v); > void vmx_update_cpu_exec_control(struct vcpu *v); > void vmx_update_secondary_exec_control(struct vcpu *v); > > +#define POSTED_INTR_ON 0 > +static inline int pi_test_and_set_pir(int vector, struct pi_desc *pi_desc) > +{ > + return test_and_set_bit(vector, (unsigned long *)pi_desc->pir); > +} > + > +static inline int pi_test_and_set_on(struct pi_desc *pi_desc) > +{ > + return test_and_set_bit(POSTED_INTR_ON, > + (unsigned long *)&pi_desc->control); > +} > + > +static inline void pi_set_on(struct pi_desc *pi_desc) > +{ > + return set_bit(POSTED_INTR_ON, (unsigned long *)&pi_desc->control); > +} > + > +static inline int pi_test_and_clear_on(struct pi_desc *pi_desc) > +{ > + return test_and_clear_bit(POSTED_INTR_ON, > + (unsigned long *)&pi_desc->control); > +}I don''t see why any of the casts would be necessary. Please drop them. Jan
Jan Beulich
2013-Apr-17 09:17 UTC
Re: [PATCH v2 5/5] VMX: Use posted interrupt to deliver virutal interrupt
>>> On 17.04.13 at 08:51, Yang Zhang <yang.z.zhang@intel.com> wrote: > @@ -132,9 +135,10 @@ void vlapic_set_irq(struct vlapic *vlapic, uint8_t vec, uint8_t trig) > if ( hvm_funcs.update_eoi_exit_bitmap ) > hvm_funcs.update_eoi_exit_bitmap(target, vec, trig); > > - /* We may need to wake up target vcpu, besides set pending bit here */ > - if ( !vlapic_test_and_set_irr(vec, vlapic) ) > - vcpu_kick(target); > + if ( hvm_funcs.deliver_posted_intr ) > + hvm_funcs.deliver_posted_intr(target, vec); > + else if ( !vlapic_test_and_set_irr(vec, vlapic) ) > + vcpu_kick(vlapic_vcpu(vlapic));Why can''t that remain to be vcpu_kick(target); Jan
Zhang, Yang Z
2013-Apr-17 12:21 UTC
Re: [PATCH v2 2/5] VMX: Turn on posted interrupt bit in vmcs
Jan Beulich wrote on 2013-04-17:>>>> On 17.04.13 at 08:50, Yang Zhang <yang.z.zhang@intel.com> wrote: >> --- a/xen/arch/x86/hvm/vmx/vmcs.c >> +++ b/xen/arch/x86/hvm/vmx/vmcs.c >> @@ -926,6 +926,12 @@ static int construct_vmcs(struct vcpu *v) >> __vmwrite(GUEST_INTR_STATUS, 0); >> } >> + if ( cpu_has_vmx_posted_intr_processing ) + { + >> __vmwrite(PI_DESC_ADDR, virt_to_maddr(&v->arch.hvm_vmx.pi_desc)); + >> __vmwrite(POSTED_INTR_NOTIFICATION_VECTOR, > POSTED_INTERRUPT_VECTOR); > > Shouldn''t this be posted_intr_vector?Yes, it should be posted_intr_vector.> > >> --- a/xen/include/asm-x86/mach-default/irq_vectors.h >> +++ b/xen/include/asm-x86/mach-default/irq_vectors.h >> @@ -9,12 +9,13 @@ >> #define CALL_FUNCTION_VECTOR 0xfb #define LOCAL_TIMER_VECTOR 0xfa >> #define PMU_APIC_VECTOR 0xf9 +#define POSTED_INTERRUPT_VECTOR 0xf8 /* >> * High-priority dynamically-allocated vectors. For interrupts that >> * must be higher priority than any guest-bound interrupt. >> */ >> #define FIRST_HIPRIORITY_VECTOR 0xf1 >> -#define LAST_HIPRIORITY_VECTOR 0xf8 >> +#define LAST_HIPRIORITY_VECTOR 0xf7 >> /* IRQ0 (timer) is statically allocated but must be high priority. */ >> #define IRQ0_VECTOR 0xf0 > > And these changes should both no longer be necessary. > > Or am I mis-reading "Allocate posted interrupt vector dynamically" > in your overview mail?No, you are right. I may send the wrong patch. This is removed actually. I will resend it in version 3. Best regards, Yang
Zhang, Yang Z
2013-Apr-17 12:32 UTC
Re: [PATCH v2 5/5] VMX: Use posted interrupt to deliver virutal interrupt
Jan Beulich wrote on 2013-04-17:>>>> On 17.04.13 at 08:51, Yang Zhang <yang.z.zhang@intel.com> wrote: >> @@ -132,9 +135,10 @@ void vlapic_set_irq(struct vlapic *vlapic, uint8_t vec, > uint8_t trig) >> if ( hvm_funcs.update_eoi_exit_bitmap ) >> hvm_funcs.update_eoi_exit_bitmap(target, vec, trig); >> - /* We may need to wake up target vcpu, besides set pending bit here */ >> - if ( !vlapic_test_and_set_irr(vec, vlapic) ) >> - vcpu_kick(target); >> + if ( hvm_funcs.deliver_posted_intr ) >> + hvm_funcs.deliver_posted_intr(target, vec); >> + else if ( !vlapic_test_and_set_irr(vec, vlapic) ) >> + vcpu_kick(vlapic_vcpu(vlapic)); > > Why can''t that remain to be > > vcpu_kick(target);Right.. Best regards, Yang
Zhang, Yang Z
2013-Apr-17 12:40 UTC
Re: [PATCH v2 3/5] VMX: Add posted interrupt supporting
Jan Beulich wrote on 2013-04-17:>>>> On 17.04.13 at 08:51, Yang Zhang <yang.z.zhang@intel.com> wrote: >> +static void vmx_sync_pir_to_irr(struct vcpu *v) >> +{ >> + struct vlapic *vlapic = vcpu_vlapic(v); >> + u64 val[4]; > > This should be DECLARE_BITMAP(), ...Yes.> >> + u32 group, i; >> + >> + if ( !pi_test_and_clear_on(&v->arch.hvm_vmx.pi_desc) ) >> + return; >> + >> + for ( group = 0; group < 4; group++ ) > > ... and use ARRAY_SIZE() here. >Yes.>> + val[group] = pi_get_pir(&v->arch.hvm_vmx.pi_desc, group); > > Which will require adjustments to the types underlying this call.What do you mean? Do you suggest pass pir[group] instead pi_desc?> >> + >> + for_each_set_bit(i, val, MAX_VECTOR) > > I''d also favor you not adding new uses of this redundant andIs there better way to iterate the setting bit?> mis-named #define - please use NR_VECTORS. > >> +#define for_each_set_bit(bit, addr, size) \ >> + for ( (bit) = find_first_bit((addr), (size)); \ >> + (bit) < (size); \ >> + (bit) = find_next_bit((addr), (size), (bit) + 1) ) > > Please drop the pointless parentheses (but keep the ones that > are needed).Sure.> >> --- a/xen/include/asm-x86/hvm/vmx/vmx.h >> +++ b/xen/include/asm-x86/hvm/vmx/vmx.h >> @@ -99,6 +99,33 @@ void vmx_update_exception_bitmap(struct vcpu *v); >> void vmx_update_cpu_exec_control(struct vcpu *v); >> void vmx_update_secondary_exec_control(struct vcpu *v); >> +#define POSTED_INTR_ON 0 >> +static inline int pi_test_and_set_pir(int vector, struct pi_desc *pi_desc) >> +{ >> + return test_and_set_bit(vector, (unsigned long *)pi_desc->pir); >> +} >> + >> +static inline int pi_test_and_set_on(struct pi_desc *pi_desc) >> +{ >> + return test_and_set_bit(POSTED_INTR_ON, >> + (unsigned long *)&pi_desc->control); >> +} >> + >> +static inline void pi_set_on(struct pi_desc *pi_desc) >> +{ >> + return set_bit(POSTED_INTR_ON, (unsigned long *)&pi_desc->control); >> +} >> + >> +static inline int pi_test_and_clear_on(struct pi_desc *pi_desc) >> +{ >> + return test_and_clear_bit(POSTED_INTR_ON, >> + (unsigned long *)&pi_desc->control); >> +} > > I don''t see why any of the casts would be necessary. Please drop them.Sure.> > JanBest regards, Yang
On Wed, Apr 17, 2013 at 8:04 AM, Keir Fraser <keir.xen@gmail.com> wrote:> On 17/04/2013 07:50, "Yang Zhang" <yang.z.zhang@intel.com> wrote: > >> From: Yang Zhang <yang.z.zhang@Intel.com> > > The whole series: > > Acked-by: Keir Fraser <keir@xen.org>Keir / Yang, any comments on risks to the release? Yang, what kinds of testing are you guys going to be doing of this between now and the 4.3 release? From a feature / risk of slip perspective, I think this is probably good. The key thing I think is making sure that if there is a bug, it will be caught before the release. I suspect that a lot of people won''t have hardware to test this, so it will be up to Intel to make sure the code has "gone through its paces" and is ready for public consumption in 2 months'' time. -George
>>> On 17.04.13 at 14:40, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote: > Jan Beulich wrote on 2013-04-17: >>>>> On 17.04.13 at 08:51, Yang Zhang <yang.z.zhang@intel.com> wrote: >>> + val[group] = pi_get_pir(&v->arch.hvm_vmx.pi_desc, group); >> >> Which will require adjustments to the types underlying this call. > What do you mean? Do you suggest pass pir[group] instead pi_desc?Not necessarily. pir[] will need to become an array of unsigned long too, and pi_get_pir() will need to return an unsigned long. And the definitions should, no matter that we don''t support 32-bit anymore, not depend on u64 == unsigned long.>>> + >>> + for_each_set_bit(i, val, MAX_VECTOR) >> >> I''d also favor you not adding new uses of this redundant and > Is there better way to iterate the setting bit?No, the iterator is fine. MAX_VECTORS is what I dislike (and what should go away). Jan
Zhang, Yang Z
2013-Apr-17 13:19 UTC
Re: [PATCH v2 3/5] VMX: Add posted interrupt supporting
Jan Beulich wrote on 2013-04-17:>>>> On 17.04.13 at 14:40, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote: >> Jan Beulich wrote on 2013-04-17: >>>>>> On 17.04.13 at 08:51, Yang Zhang <yang.z.zhang@intel.com> wrote: >>>> + val[group] = pi_get_pir(&v->arch.hvm_vmx.pi_desc, group); >>> >>> Which will require adjustments to the types underlying this call. >> What do you mean? Do you suggest pass pir[group] instead pi_desc? > > Not necessarily. pir[] will need to become an array of unsigned long > too, and pi_get_pir() will need to return an unsigned long. And the > definitions should, no matter that we don''t support 32-bit anymore, > not depend on u64 == unsigned long.Ok, I see.> >>>> + >>>> + for_each_set_bit(i, val, MAX_VECTOR) >>> >>> I''d also favor you not adding new uses of this redundant and >> Is there better way to iterate the setting bit? > > No, the iterator is fine. MAX_VECTORS is what I dislike (and what > should go away).Yes, you are right. Best regards, Yang
On 17/04/2013 13:53, "George Dunlap" <George.Dunlap@eu.citrix.com> wrote:> On Wed, Apr 17, 2013 at 8:04 AM, Keir Fraser <keir.xen@gmail.com> wrote: >> On 17/04/2013 07:50, "Yang Zhang" <yang.z.zhang@intel.com> wrote: >> >>> From: Yang Zhang <yang.z.zhang@Intel.com> >> >> The whole series: >> >> Acked-by: Keir Fraser <keir@xen.org> > > Keir / Yang, any comments on risks to the release? > > Yang, what kinds of testing are you guys going to be doing of this > between now and the 4.3 release? > > From a feature / risk of slip perspective, I think this is probably > good. The key thing I think is making sure that if there is a bug, it > will be caught before the release. I suspect that a lot of people > won''t have hardware to test this, so it will be up to Intel to make > sure the code has "gone through its paces" and is ready for public > consumption in 2 months'' time.Personally I think it should go in, if Intel can assure they have done decent testing internally (and will continue to do so). -- Keir> -George
Keir Fraser wrote on 2013-04-18:> On 17/04/2013 13:53, "George Dunlap" <George.Dunlap@eu.citrix.com> wrote: > >> On Wed, Apr 17, 2013 at 8:04 AM, Keir Fraser <keir.xen@gmail.com> wrote: >>> On 17/04/2013 07:50, "Yang Zhang" <yang.z.zhang@intel.com> wrote: >>> >>>> From: Yang Zhang <yang.z.zhang@Intel.com> >>> >>> The whole series: >>> >>> Acked-by: Keir Fraser <keir@xen.org> >> >> Keir / Yang, any comments on risks to the release? >> >> Yang, what kinds of testing are you guys going to be doing of this >> between now and the 4.3 release? >> >> From a feature / risk of slip perspective, I think this is probably >> good. The key thing I think is making sure that if there is a bug, it >> will be caught before the release. I suspect that a lot of people >> won''t have hardware to test this, so it will be up to Intel to make >> sure the code has "gone through its paces" and is ready for public >> consumption in 2 months'' time. > > Personally I think it should go in, if Intel can assure they have done > decent testing internally (and will continue to do so).Yes, we did many testing: beside our regular testing, we also did some stress testing(boot guest and scp large file continually). And we didn''t see any issue. Also, we will put APICv testing into our nightly testing system to make sure it works well. Best regards, Yang
On 18/04/13 01:19, Zhang, Yang Z wrote:> Keir Fraser wrote on 2013-04-18: >> On 17/04/2013 13:53, "George Dunlap" <George.Dunlap@eu.citrix.com> wrote: >> >>> On Wed, Apr 17, 2013 at 8:04 AM, Keir Fraser <keir.xen@gmail.com> wrote: >>>> On 17/04/2013 07:50, "Yang Zhang" <yang.z.zhang@intel.com> wrote: >>>> >>>>> From: Yang Zhang <yang.z.zhang@Intel.com> >>>> The whole series: >>>> >>>> Acked-by: Keir Fraser <keir@xen.org> >>> Keir / Yang, any comments on risks to the release? >>> >>> Yang, what kinds of testing are you guys going to be doing of this >>> between now and the 4.3 release? >>> >>> From a feature / risk of slip perspective, I think this is probably >>> good. The key thing I think is making sure that if there is a bug, it >>> will be caught before the release. I suspect that a lot of people >>> won''t have hardware to test this, so it will be up to Intel to make >>> sure the code has "gone through its paces" and is ready for public >>> consumption in 2 months'' time. >> Personally I think it should go in, if Intel can assure they have done >> decent testing internally (and will continue to do so). > Yes, we did many testing: beside our regular testing, we also did some stress testing(boot guest and scp large file continually). And we didn''t see any issue. > Also, we will put APICv testing into our nightly testing system to make sure it works well.Sounds good. So from a release perspective: Acked-by: George Dunlap <george.dunlap@eu.citrix.com>