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

[Julien Grall <julien.grall@xxxxxxx>]

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

--
Julien Grall

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