Re: [Xen-devel] [PATCH v6 2/5] xen/arm: inflight irqs during migration

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 23 Jun 2014, Julien Grall wrote:
> Hi Stefano,
> 
> On 23/06/14 17:37, Stefano Stabellini wrote:
> > @@ -786,9 +837,14 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int 
> > irq)
> >   
> >       spin_lock_irqsave(&v->arch.vgic.lock, flags);
> >   
> > +    set_bit(GIC_IRQ_GUEST_QUEUED, &n->status);
> > +    /* update QUEUED before MIGRATING */
> > +    smp_wmb();
> > +    if ( test_bit(GIC_IRQ_GUEST_MIGRATING, &n->status) )
> > +        goto out;
> 
> Why do you kick the current VCPU here? It looks like useless because the 
> migration will take care of it.

With the physical follow virtual patch, the vcpu_unblock below is going
to be mostly useless but harmless as vgic_vcpu_inject_irq is going to be called 
on
the pcpu running the destination vcpu. smp_send_event_check_mask won't be 
called as
v == current.

On the other hand without that patch, the pcpu receiving the physical
interrupt could be different from any of the pcpus running the vcpus
involved in vcpu migration, therefore we would need the kick to wake up
the destination vcpu.


> > +
> >       if ( !list_empty(&n->inflight) )
> >       {
> > -        set_bit(GIC_IRQ_GUEST_QUEUED, &n->status);
> >           gic_raise_inflight_irq(v, irq);
> >           goto out;
> >       }
> > @@ -796,6 +852,7 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int 
> > irq)
> >       /* vcpu offline */
> >       if ( test_bit(_VPF_down, &v->pause_flags) )
> >       {
> > +        clear_bit(GIC_IRQ_GUEST_QUEUED, &n->status);
> >           spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
> >           return;
> 
> Rather than setting & clearing the GUEST_QUEUED bit. Wouldn't it be better to 
> move the if (test_bit(_VPF_down,...)) before the spin_lock?
> 
> If the VCPU is down here, it will likely be down before. Hence, the lock 
> doesn't protect the pause_flags. This would also make the code clearer.

Good suggestion

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