Re: [Xen-devel] [PATCH] Remove a set operation for VCPU_KICK_SOFTIRQ when post interrupt to vm.

["Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx>]

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 Jan and Dario,

I have a quick check on current code. I am curious that is current Xen 
preemptive? The variable preempt_count is only checked in in_atomic() which 
never use in latest Xen. Also, when return from an interrupt handler, 
hypervisor didn't check whether reschedule is needed if the interrupt is 
occurred in kernel context.
ENTRY(ret_from_intr)
        GET_CURRENT(%rbx)
        testb $3,UREGS_cs(%rsp)
        jz    restore_all_xen              //call iret directly to restore 
previous context where interrupt occur if it is in kernel space.

If Xen isn't preemptive, the above case I mentioned should never happen since 
the VCPU still run in the same PCPU. Am I right?

Best regards,
Yang

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel