[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v10 26/32] ARM: vITS: handle MOVI command

Hi Stefano,

On 31/05/2017 18:53, Stefano Stabellini wrote:
On Wed, 31 May 2017, Julien Grall wrote:
Hi Stefano,

On 30/05/17 23:35, Stefano Stabellini wrote:
+    /*
+     * TODO: lookup currently-in-guest virtual IRQs and migrate them,
+     * as the locking may be fragile otherwise.
+     * This is not easy to do at the moment, but should become easier
+     * with the introduction of a per-IRQ lock.
+     */

Sure but at least we can handle the inflight, but not in guest, case. It
is just a matter of adding (withing the arch.vgic.lock locked region):

    if ( !list_empty(&p->lr_queue) )
        gic_remove_irq(old, p);
        clear_bit(GIC_IRQ_GUEST_QUEUED, &p->status);

        spin_unlock_irqrestore(&old->arch.vgic.lock, flags);
        vgic_vcpu_inject_irq(new, irq);

Please no more hardcoding of migration code. If we are going to support
migration we should rework the current function vgic_migrate_irq to support

I agree, it is true, in fact that is what I actually suggested last
time. My preference would be to just call vgic_migrate_irq, providing an
empty implementation of irq_set_affinity for LPIs.

But ... I don't see any drawback to not support this today. Per the
specification, when you migrate an interrupt the only things you have to
ensure if the pending interrupt only fire once either on the old or new vCPU.

IHMO, if the interrupt has already been queued then it is too late. We should
aim at simplifying the code if there are no drawback to do it. In this case,
what would be the drawback to leave pending on the old vCPU?

It is inconsistent with what we do elsewhere (vgic_migrate_irq).

So? It is not because the current code does something that we should keep the same behavior here which BTW cannot be noticed by a guest.

It is
also inconsistent with the TODO comment (!list_empty(&p->lr_queue)
interrupts are not yet currently-in-guest).

A TODO can easily be updated.

I don't want to introduce any more hardcoding, I just would like the
existing vgic_migrate_irq to be called.

It is going to need a bit of rework to get it working with LPI as the code is currently gated with (p->desc). For what benefits?

Not much as the current code is already working well on migration...


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.