On Tue, 2015-06-30 at 11:11 +0100, Andrew Cooper wrote:
> On 24/06/15 06:18, Feng Wu wrote:

> > +/*
> > + * Handle VT-d posted-interrupt when VCPU is blocked.
> > + */
> > +static void pi_wakeup_interrupt(struct cpu_user_regs *regs)
> > +{
> > +    struct arch_vmx_struct *vmx;
> > +    unsigned int cpu = smp_processor_id();
> > +
> > +    spin_lock(&per_cpu(pi_blocked_vcpu_lock, cpu));
> > +
> > +    /*
> > +     * FIXME: The length of the list depends on how many
> > +     * vCPU is current blocked on this specific pCPU.
> > +     * This may hurt the interrupt latency if the list
> > +     * grows to too many entries.
> > +     */
> > +    list_for_each_entry(vmx, &per_cpu(pi_blocked_vcpu, cpu),
> > +                        pi_blocked_vcpu_list)
> > +        if ( vmx->pi_desc.on )
> > +            tasklet_schedule(&vmx->pi_vcpu_wakeup_tasklet);
> 
> There is a logical bug here.  If we have two NV's delivered to this
> pcpu, we will kick the first vcpu twice.
> 
> On finding desc.on, a kick should be scheduled, then the vcpu removed
> from this list.  With desc.on set, we know for certain that another NV
> will not arrive for it until it has been scheduled again and the
> interrupt posted.
> 
Yes, that seems a possible issue (and one that should indeed be
avoided).

I'm still unsure about the one that I raised myself but, if it is
possible to have more than one vcpu in a pcpu list, with desc.on==true,
then it looks to me that we kick all of them, for each notification.

Added what Andrew's spotted, if there are a bunch of vcpus, queued with
desc.on==ture, and a bunch of notifications arrives before the tasklet
gets executed, we'll be kicking the whole bunch of them for a bunch of
times! :-/

Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)