Re: [Xen-devel] [PATCH v9 03/28] ARM: GIC: Add checks for NULL pointer pending_irq's

[Andre Przywara <andre.przywara@xxxxxxx>]

Hi,

On 22/05/17 18:15, Julien Grall wrote:
> 
> 
> On 22/05/17 17:49, Andre Przywara wrote:
>> Hi,
> 
> Hi Andre,
> 
>> On 12/05/17 15:19, Julien Grall wrote:
>>> Hi Andre,
>>>
>>> On 11/05/17 18:53, Andre Przywara wrote:
>>>> For LPIs the struct pending_irq's are dynamically allocated and the
>>>> pointers will be stored in a radix tree. Since an LPI can be "unmapped"
>>>> at any time, teach the VGIC how to deal with irq_to_pending() returning
>>>> a NULL pointer.
>>>> We just do nothing in this case or clean up the LR if the virtual LPI
>>>> number was still in an LR.
>>>>
>>>> Those are all call sites for irq_to_pending(), as per:
>>>> "git grep irq_to_pending", and their evaluations:
>>>> (PROTECTED means: added NULL check and bailing out)
>>>>
>>>>     xen/arch/arm/gic.c:
>>>> gic_route_irq_to_guest(): only called for SPIs, added ASSERT()
>>>> gic_remove_irq_from_guest(): only called for SPIs, added ASSERT()
>>>> gic_remove_from_queues(): PROTECTED, called within VCPU VGIC lock
>>>> gic_raise_inflight_irq(): PROTECTED, called under VCPU VGIC lock
>>>> gic_raise_guest_irq(): PROTECTED, called under VCPU VGIC lock
>>>> gic_update_one_lr(): PROTECTED, called under VCPU VGIC lock
>>>
>>> Even they are protected, an ASSERT would be useful.
>>
>> I am not sure I get what you mean here.
>> With PROTECTED I meant that the code checks for a irq_to_pending()
>> returning NULL and reacts accordingly.
>> ASSERTs are only for making sure that those functions are never called
>> for LPIs(), but the other functions can be called with an LPI, and they
>> can now cope with a NULL pending_irq.
>>
>> So what do I miss here?
> 
> I mean adding an ASSERT(spin_is_locked(vgic->vcpu)) in those functions
> if it is not done yet.

Well, that's what I meant with PROTECTED: all of them have that ASSERT
already.
So I consider this done then.

Cheers,
Andre.

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