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

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

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

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

Regards,


-- 
Julien Grall