Zhang, Yang Z wrote on 2015-09-08: > Liuqiming (John) wrote on 2015-09-08: >> Ok, I will try to explain, correct me if I got anything wrong: >> >> The problem here is not interrupts lost but interrupts not delivered >> in time. >> >> there are basically two path to inject an interrupt into VM (or >> vCPU to another vCPU): >> Path 1, the traditional way: >> 1) set bit in vlapic IRR field which represent an interrupt, >> then kick vcpu 2) a VCPU_KICK_SOFTIRQ softirq raised 3) if >> VCPU_KICK_SOFTIRQ bit not set, then set it, otherwise return and >> do nothing 4) send an EVENT_CHECK_VECTOR IPI to target vcpu 5) >> target vcpu will VMEXIT due to EXIT_REASON_EXTERNAL_INTERRUPT 6) >> the interrupt handler basically do nothing 7) interrupt in IRR >> will be evaluated 8) VCPU_KICK_SOFTIRQ will be cleared when >> do_softirq 9) there will be an interrupt inject into vcpu when >> VMENTRY Path 2, the Posted-interrupt way (current logic): 1) set >> bit in posted-interrupt descriptor which represent an interrupt >> 2) if VCPU_KICK_SOFTIRQ bit not set, then set it, otherwise >> return and do nothing 3) send an POSTED_INTR_NOTIFICATION_VECTOR >> IPI to target vcpu 4) if target vcpu in non-ROOT mode it will >> receive the interrupt >> immediately otherwise interrupt will be injected when VMENTRY >> >> As the first operation in both path is setting a interrupt represent >> bit, so no interrupts will lost. >> >> The issue is: >> in path 2, the first interrupt will cause VCPU_KICK_SOFTIRQ set to >> 1, and unless a VMEXIT occured or somewhere called do_softirq >> directly, VCPU_KICK_SOFTIRQ will not cleared, that will make the >> later interrupts injection ignored at step 2), which will delay irq >> handler process in VM. >> >> And because path 2 set VCPU_KICK_SOFTIRQ to 1, the kick vcpu logic >> in path 1 will also return in step 3), which make this vcpu only can >> handle interrupt when some other reason cause VMEXIT. > > Nice catch! But without set the softirq, the interrupt delivery also will be > delay. > Look at the code in vmx_do_vmentry: > > cli > <---------------the virtual interrupt occurs here will be > delay. Because there is no softirq pending and the following posted > interrupt may consumed by another running VCPU, so the target VCPU > will not aware there is pending virtual interrupt before next vmexit. > cmp %ecx,(%rdx,%rax,1) > jnz .Lvmx_process_softirqs > > I need more time to think it.
Hi Qiming, Can you help to take a look at this patch? Also, if possible, please help to do a testing since I don't have machine to test it. Thanks very much. diff --git a/xen/arch/x86/hvm/vmx/entry.S b/xen/arch/x86/hvm/vmx/entry.S index 664ed83..1ebb5d0 100644 --- a/xen/arch/x86/hvm/vmx/entry.S +++ b/xen/arch/x86/hvm/vmx/entry.S @@ -77,6 +77,9 @@ UNLIKELY_START(ne, realmode) call vmx_enter_realmode UNLIKELY_END(realmode) + bt $0,VCPU_vmx_pi_ctrl(%rbx) + jc .Lvmx_do_vmentry + mov %rsp,%rdi call vmx_vmenter_helper mov VCPU_hvm_guest_cr2(%rbx),%rax diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index e0e9e75..315ce65 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -1678,8 +1678,9 @@ static void __vmx_deliver_posted_interrupt(struct vcpu *v) { unsigned int cpu = v->processor; - if ( !test_and_set_bit(VCPU_KICK_SOFTIRQ, &softirq_pending(cpu)) - && (cpu != smp_processor_id()) ) + if ( !test_bit(VCPU_KICK_SOFTIRQ, &softirq_pending(cpu)) + && pi_test_on(&v->arch.hvm_vmx.pi_desc) + && (cpu != smp_processor_id())) send_IPI_mask(cpumask_of(cpu), posted_intr_vector); } } diff --git a/xen/arch/x86/x86_64/asm-offsets.c b/xen/arch/x86/x86_64/asm-offsets.c index 447c650..985725f 100644 --- a/xen/arch/x86/x86_64/asm-offsets.c +++ b/xen/arch/x86/x86_64/asm-offsets.c @@ -108,6 +108,7 @@ void __dummy__(void) OFFSET(VCPU_vmx_emulate, struct vcpu, arch.hvm_vmx.vmx_emulate); OFFSET(VCPU_vm86_seg_mask, struct vcpu, arch.hvm_vmx.vm86_segment_mask); OFFSET(VCPU_hvm_guest_cr2, struct vcpu, arch.hvm_vcpu.guest_cr[2]); + OFFSET(VCPU_vmx_pi_ctrl, struct vcpu, arch.hvm_vmx.pi_desc.control); BLANK(); OFFSET(VCPU_nhvm_guestmode, struct vcpu, arch.hvm_vcpu.nvcpu.nv_guestmode); diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h b/xen/include/asm-x86/hvm/vmx/vmx.h index 35f804a..157a4fe 100644 --- a/xen/include/asm-x86/hvm/vmx/vmx.h +++ b/xen/include/asm-x86/hvm/vmx/vmx.h @@ -116,6 +116,11 @@ static inline void pi_set_on(struct pi_desc *pi_desc) set_bit(POSTED_INTR_ON, &pi_desc->control); } +static inline int pi_test_on(struct pi_desc *pi_desc) +{ + return test_bit(POSTED_INTR_ON, &pi_desc->control); +} + static inline int pi_test_and_clear_on(struct pi_desc *pi_desc) { return test_and_clear_bit(POSTED_INTR_ON, &pi_desc->control); Best regards, Yang _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel