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

[Julien Grall <julien.grall@xxxxxxxxxx>]

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