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

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Xuquan (Quan Xu) [mailto:xuquan8@xxxxxxxxxx]
> Sent: Tuesday, February 21, 2017 12:12 PM
> 
> On February 21, 2017 11:07 AM, Tian, Kevin wrote:
> >> From: Xuquan (Quan Xu) [mailto:xuquan8@xxxxxxxxxx]
> >> Sent: Tuesday, February 21, 2017 10:49 AM
> >>
> >> 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..
> >
> >I don't understand of your concern of whether guest is in guest mode here.
> 
> If the vCPUs are in root-mode, whatever we do we can't deliver posted 
> interrupt
> Successfully.

The point is not to 100% guarantee this IPI is posted when target vcpu is
in non-root mode. The goal is to kick vcpu about pending interrupts. If
it happens to be in non-root mode (in most time), then posted way is 
triggered. Otherwise a physical IPI triggered in root mode with corresponding 
interrupt handler to mark out this situation.

> 
> ...
> 
> >The purpose of this function is not to guarantee posted-interrupt is always
> >used (cannot unless you pause remote cpu). It's just a best effort.
> 
> ...
> 
> the best effort, you mentioned here, __iiuc__, we will sync PIRR to vIRR
> before vmentry..
> 
> if we don't set VCPU_KICK_SOFTIRQ bit, the pending interrupt in PIRR will be 
> not synced
> to vIRR before vm-entry in time.
> In an extreme case, if there is only one interrupt pending interrupt in PIRR 
> (no other caller
> to set VCPU_KICK_SOFTIRQ bit),
> The pending interrupt in PIRR will never be delivered to guest later..
> 
> ...
> 
> > If target
> >vcpu is in root mode, then this IPI causes a real interrupt on remote cpu as
> >notification (then the handler pi_notification_interrupt you copied earlier
> >will jump in to help).
> >
> >
> ...
> The posted_intr_vector handler is not always pi_notification_interrupt. If 
> the vt-d PI is not
> enabled, the handler
> is event_check_interrupt..
> The VCPU_KICK_SOFTIRQ bit is set in  pi_notification_interrupt , but not
> event_check_interrupt..
> 
> 

As Chao explained, likely event_check_interrupt is used in pair with original
delivery logic. Now since delivery path is changed, we should move to 
pi_notification_interrupt always.

Thanks
Kevin

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