Re: [Xen-devel] [PATCH] x86/apicv: enhance posted-interrupt processing

["Xuquan (Quan Xu)" <xuquan8@xxxxxxxxxx>]

On February 20, 2017 4:24 PM, Chao Gao wrote:
>On Mon, Feb 20, 2017 at 11:25:29AM +0000, Xuquan (Quan Xu) wrote:
>>On February 18, 2017 12:33 AM, Jan Beulich wrote:
>>>>>> On 17.02.17 at 09:49, <chao.gao@xxxxxxxxx> wrote:
>>>> On Fri, Feb 17, 2017 at 09:37:45AM +0000, Xuquan (Quan Xu) wrote:
>>>>>From a589074281cc22a30ed75a5bccba60e83d2312a6 Mon Sep 17
>>>00:00:00 2001
>>>>>From: Quan Xu <xuquan8@xxxxxxxxxx>
>>>>>Date: Sat, 18 Feb 2017 09:27:37 +0800
>>>>>Subject: [PATCH] x86/apicv: enhance posted-interrupt processing
>>>>>
>>>>>If guest is already in non-root mode, an posted interrupt will be
>>>>>directly delivered to guest (leaving softirq being set w/o actually
>>>>>incurring a VM-Exit - breaking desired softirq behavior).
>>>>>Then further posted interrupts will skip the IPI, stay in PIR and
>>>>>not noted until another VM-Exit happens.
>>>>>
>>>>>Remove the softirq set. Actually since it's an optimization for less
>>>>>IPIs, check softirq_pending(cpu) directly instead of sticking to one
>>>>>bit only.
>>>>>
>>>>>Signed-off-by: Quan Xu <xuquan8@xxxxxxxxxx>
>>>>>---
>>>>> xen/arch/x86/hvm/vmx/vmx.c | 3 +--
>>>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>>>
>>>>>diff --git a/xen/arch/x86/hvm/vmx/vmx.c
>b/xen/arch/x86/hvm/vmx/vmx.c
>>>>>index 61925cf..3887c32 100644
>>>>>--- a/xen/arch/x86/hvm/vmx/vmx.c
>>>>>+++ b/xen/arch/x86/hvm/vmx/vmx.c
>>>>>@@ -1846,8 +1846,7 @@ 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 ( !softirq_pending(cpu) && (cpu != smp_processor_id()) )
>>>> HI, Quan.
>>>> Is there a situation that we need set VCPU_KICK_SOFTIRQ. For
>>>> example, after vmx_intr_assist(), a interrupt happened and its
>>>> handler called this function to deliver interrupt to current vcpu.
>>>> In that case, the interrupt would not be injected to guest before
>>>> this VM-entry for we don't generate a softirq and don't send a self-IPI
>to current vcpu.
>>>
>>
>>Chao, __iiuc__, your question may be from the comments of
>xen/arch/x86/hvm/vmx/vmx.c :: pi_notification_interrupt() ..
>>IF VT-d PI is enabled,
>>   VCPU_KICK_SOFTIRQ bit is set by ' raise_softirq(VCPU_KICK_SOFTIRQ)',
>at the end of pi_notification_interrupt()..
>>Else
>>  Is it possible for your case?
>>
>If vcpu is in root mode and is to do VM-entry, it has synced PIR to vIRR.
>Now a interrupt (e.g. PMU_APIC_VECTOR) happens. Thus it goes following
>the path pmu_apic_interrupt->vpmu_do_interrupt->vlapic_set_irq(assume
>it will inject a interrupt to current vcpu)
>-> vmx_deliver_posted_intr( set one bit in PIR )->
>-> __vmx_deliver_posted_interrupt
>Assuming that there is no softirq pending, the code after change doesn't
>generate a IPI for (cpu == smp_processor_id()). In this case, this interrupt
>would not be injected to guest before this VM-entry.
>Although there are many assumption in the explaination, I think it may be
>possible.
>

So far, I agree to add VCPU_KICK_SOFTIRQ bit in a nice way.. 
Even we have checked whether the vCPU is running or not at the beginning of 
__vmx_deliver_posted_interrupt(),
we can't grantee the vcpu is still in guest mode at the point to call this IPI..
as in an extreme case, at the point to call this IPI, all of vCPUs are in 
root-mode, the posted-interrupt
won't be delivered..

btw, we better drop these local variables('running' / 'cpu') in 
__vmx_deliver_posted_interrupt(), as we may check
a stale status..

for the cpu == smp_processor_id() case, as mentioned as below, the vCPUs (cpu = 
v->processor) pending to this cpu, are indeed not in guest-mode..
could you explain more?


Quan



>Thanks,
>Chao
>>I hope Kevin could help us to check/correct it.
>>
>>
>>>Good point, I think we indeed want to retain the old behavior (but in
>>>a not open coded fashion) for the cpu == smp_processor_id() case.
>>>
>>__iiuc__
>>for the cpu == smp_processor_id() case, the vCPUs (cpu = v->processor)
>pending to this cpu, are indeed not in guest-mode..
>>
>>Quan
>>

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