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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 3 Jul 2014, Ian Campbell wrote:
> 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.

Not a problem: a new interrupt can still come to the old processor and
the gic driver would know how to inject it properly. It could even
arrive on a processor that is nethier the old or the new one and
everything would still work.
The only case that needs care is when an irq is still on an LR register
on the old processor: the MIGRATING bit is for that.


> 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.

I don't think that is an issue for us either way, as long as we take the
right vcpu lock and we have code for that now.


> > 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.

OK


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