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

[Andre Przywara <andre.przywara@xxxxxxx>]

Hi,

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?

Cheers,
Andre.

>>
>>     xen/arch/arm/vgic.c:
>> vgic_migrate_irq(): not called for LPIs (virtual IRQs), added ASSERT()
>> arch_move_irqs(): not iterating over LPIs, added ASSERT()
>> vgic_disable_irqs(): not called for LPIs, added ASSERT()
>> vgic_enable_irqs(): not called for LPIs, added ASSERT()
>> vgic_vcpu_inject_irq(): PROTECTED, moved under VCPU VGIC lock
>>
>>     xen/include/asm-arm/event.h:
>> local_events_need_delivery_nomask(): only called for a PPI, added
>> ASSERT()
>>
>>     xen/include/asm-arm/vgic.h:
>> (prototype)
>>

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