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

[Julien Grall <julien.grall@linaro.org>]

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.


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

Regards,

-- 
Julien Grall