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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 22 May 2014, Julien Grall wrote:
> On 22/05/14 18:45, Stefano Stabellini wrote:
> > On Thu, 22 May 2014, Julien Grall wrote:
> > > Hi Stefano,
> > > 
> > > On 22/05/14 13:32, Stefano Stabellini wrote:
> > > > At the moment gic_remove_from_queues doesn't handle the case where the
> > > > guest kernel disables an irq on a different vcpu compared to the one
> > > > currently receiving the interrupt.
> > > > Make sure to take the right vcpu lock before removing the irq from
> > > > lr_queue.
> > > 
> > > I see the same issue with vgic_enable_irqs. We may inject to the wrong
> > > VCPU
> > > (i.e other than 0).
> > > 
> > > I think we should have the same case in vgic_enable_irqs.
> > 
> > I think it would make more sense to print a warning in
> > vgic_distr_mmio_write GICD_ITARGETSR rather than vgic_enable_irqs.
> 
> IHMO the warning is not enougth. We may screw your state machine.

That cannot happen: rank->itargets is actually unused at the moment.


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

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