Re: [Xen-devel] [PATCH v4 11/17] xen/arm: ITS: Add GICR register emulation

[Vijay Kilari <vijay.kilari@xxxxxxxxx>]

On Thu, Jul 16, 2015 at 8:11 PM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
> Hi Vijay,
>
> On 16/07/2015 16:15, Vijay Kilari wrote:
>>
>> On Wed, Jul 15, 2015 at 11:02 PM, Julien Grall <julien.grall@xxxxxxxxxx>
>> wrote:
>>>
>>> On 10/07/2015 09:42, vijay.kilari@xxxxxxxxx wrote:
>>>>
>>>>
>>>> From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxxxxxxxxxx>
>>>>
>>>> Emulate LPI related changes to GICR registers
>>
>> [..]
>>>
>>>
>>>> +    p = irq_to_pending(v, vlpi);
>>>> +
>>>> +    set_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
>>>> +
>>>> +    spin_lock_irqsave(&v->arch.vgic.lock, flags);
>>>> +
>>>> +    /*XXX: raise on right vcpu */
>>>
>>>
>>>
>>> /* XXX: ... */
>>>
>>> I guess you added this comment because I mentioned possible locking
>>> problem
>>> here?
>>
>>
>>     I meant to find out Collection (VCPU) for this vlpi and inject on that
>> VCPU
>
>
> I'm nearly sure there is locking problem if you don't do it know.
>
>>>
>>> If so, did you think how this would affect the vLPI handling to not do
>>> the
>>> correct locking for Xen 4.6?
>>>
>>> Note for the others: AFAICT the locking of the pending_irq structure is
>>> done
>>> using the lock of the vCPU where the IRQ is routed. Stefano, please
>>> confirm
>>> if it's true.
>
>
> [...]
>
>
>>>> +
>>>> +    spin_lock(&v->domain->arch.vits->prop_lock);
>>>
>>>
>>>
>>> I don't understand why you need to take the prop_lock here. You only use
>>> them in the handler which you don't have yet registered.
>>>
>>      lock is taken as property table can be changed. However as I am
>> not handling change in property table in this series, It can be dropped.
>>
>>>> +    if ( id_bits > gic_nr_id_bits() )
>>>> +        id_bits = gic_nr_id_bits();
>>>
>>>
>>>
>>> What happen if the property configuration table is smaller?
>>
>>
>> Spec does not say anything about it
>
>
> So you have to handle it properly to avoid the helper reading out of the LPI
> configuration table.

  the check on size lpi_size manages this.

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