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, 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.


> > > 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