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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Fri, 20 Jun 2014, Julien Grall wrote:
> On 20 Jun 2014 18:41, "Stefano Stabellini" <stefano.stabellini@xxxxxxxxxxxxx> 
> wrote:
> >
> > On Fri, 20 Jun 2014, Julien Grall wrote:
> > > 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.
> >
> > That's true.
> >
> >
> >
> > > > > 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.
> >
> > Which 2 clear bit?
> 
> GUEST_VISIBLE and GUEST_ACTIVE see the code after eoi_irq.

The code is protected by the vgic lock.

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