Re: [Xen-devel] [PATCH v8 13/13] gic_remove_from_queues: take a lock on the right vcpu

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Sun, 25 May 2014, Stefano Stabellini wrote:
> On Sun, 25 May 2014, Julien Grall wrote:
> > On 25/05/14 16:39, Stefano Stabellini wrote:
> >  > That is actually the problem: if vcpu1 is the one enabling an SPI, the
> > > target vcpu should still be the one specified by itarget.
> > 
> > Yes, but by enabling I mean writing in ISENABLER* on VCPU1. In this case, we
> > may inject this IRQ on this CPUs (see vgic_enable_irqs).
> 
> Yes, that is completely wrong, I have a patch for that. I'll send it out
> separately from this series.

To be clear: I'll drop this patch from this series and send out two
separate patches.


> > > > This can inject the IRQ in VCPU1 while it's already injected in
> > > > VCPU0 (assuming the IRQ what disable for a little time).
> > > > 
> > > > > 
> > > > > > BTW, for your todo:
> > > > > > 
> > > > > > > +    /* TODO: evict the irq from LRs */
> > > > > > 
> > > > > > We should not evict the IRQ from LRs. The guest may disable the IRQ
> > > > > > while he
> > > > > > is in the IRQ context (and before the IRQ has been EOI). If you drop
> > > > > > the IRQs
> > > > > > from the LRs, this can result to a maintenance interrupt:
> > > > > > 
> > > > > > "If the specified Interrupt does not exist in the
> > > > > > List registers, the GICH_HCR.EOIcount field is incremented,
> > > > > > potentially
> > > > > > generating a maintenance interrupt."
> > > > > 
> > > > > It is still better than the alternative: having an LR busy for no
> > > > > reason.
> > > > > A maintenance interrupt would be harmless.
> > > > 
> > > > Our internal representation (in the status field, still inflight) won't
> > > > be update-to-date for IRQ. We either inject a spurious IRQ (if it's a
> > > > virtual IRQ), other set active & pending is physical IRQ (which is
> > > > invalid from the GIC specification).
> > > 
> > > I think that the best behaviour would be to evict the irq from LRs if
> > > the irq is not active.
> > 
> > Right.
> 

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