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

Re: [Xen-devel] [PATCH] xen/arm: introduce platform_need_explicit_eoi





On 20/06/14 17:48, Stefano Stabellini wrote:

Your patch "physical irq follow virtual irq" won't even solve this problem if
the maintenance interrupt is request to EOI the IRQ.

I don't understand this statement.
The maintenance interrupt is requested to make sure that the vcpu is
interrupted as soon as it EOIs the virtual irq, so that
gic_update_one_lr can run.

I agree with that but ... the following step can happen

1) Xen receives the interrupt
2) Xen writes EOIR
3) Xen inject the IRQ to the guest
4) The guest will receive the IRQ (i.e read IAR)
5) Xen migrates the VCPU to another physical CPU
6) The guest writes into GIC_DIR and a maintenance IRQ is fired.

In a such case, the IRQ will be EOIed to another physical CPU. If we assume that we should always write to GICC_DIR of the physical CPU that receive the interrupt, then even your patch "physcial irq follow virtual irq" won't solve the problem.

The VGIC lock is per-vcpu.

If this is happening while we migrate, nothing protect p->lr and the different
bit anymore.

The patch series
alpine.DEB.2.02.1406111607450.13771@xxxxxxxxxxxxxxxxxxxxxxx takes
care of vcpu migration and locking.


Indeed, the vgic_inject_irq will use the new VCPU. This can happen as soon as
we EOI the IRQ.

Yes, but at that point we have also already migrated the corresponding
physical irq to the new pcpu.

Even tho we the migration has been done, the 2 clear bit are not atomic. Let's imagine that the IRQ has fired between the two clear bit. In this case we may clear the ACTIVE bit by mistake.

I don't see anything in this code which prevents a such configuration.

Regards,

--
Julien Grall

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


 


Rackspace

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