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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-07-02 at 19:33 +0100, Stefano Stabellini wrote:
> On Fri, 27 Jun 2014, Ian Campbell wrote:
> > On Mon, 2014-06-23 at 17:37 +0100, Stefano Stabellini wrote:
> > 
> > > +        list_del_init(&p->lr_queue);
> > > +        list_del_init(&p->inflight);
> > > +        spin_unlock_irqrestore(&old->arch.vgic.lock, flags);
> > > +        vgic_vcpu_inject_irq(new, irq);
> > > +        return;
> > > +    /* if the IRQ is in a GICH_LR register, set GIC_IRQ_GUEST_MIGRATING
> > > +     * and wait for the EOI */
> > 
> > Is it safe that MIGRATING serves dual purpose -- it indicates an irq
> > which is actively being moved but it also indicates an irq where we are
> > awaiting an EOI before migrating it. (Or have I misunderstood what is
> > going on here?)
> 
> As described in the commit message, MIGRATING is just to flag an irq
> that has been migrated but for which we are also awaiting an EOI.

Ah, for some reason I thought it was also signalling an IRQ which was
currently being migrated.

How do you handle the race between writing the real itargets register
and inflight (real) interrupts? After the write you might still see some
interrupt on the old processor I think. I thought that was what the bit
was for.

e.g. on x86 I think they don't consider the interrupt to have migrated
until they observe the first one on the target processor. Maybe x86 int
controllers and ARM ones differ wrt this sort of thing though.

> Maybe I can rename GIC_IRQ_GUEST_MIGRATING to
> GIC_IRQ_GUEST_MIGRATING_EOI to make it more obvious that is targeting a
> specific corner case.
> 
> 
> > I suspect what I'm worried about is something completing the migration
> > without realising it needs to wait for an EOI?
> 
> We carefully check the current status of the irq in vgic_migrate_irq
> before setting GIC_IRQ_GUEST_MIGRATING. Only if the irq is already in
> an LR register we set GIC_IRQ_GUEST_MIGRATING.

Assuming the above doesn't cause a rethink of the implementation then I
think just clarifying the comment:

+     * GIC_IRQ_GUEST_MIGRATING: the irq is being migrated to a different
+     * vcpu.

would be sufficient.

Ian.


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