Re: [Xen-devel] [PATCH v2] gic: drop interrupts enabling on interrupts processing

[Andre Przywara <andre.przywara@xxxxxxx>]

On Tue, 28 May 2019 18:07:19 +0100
Julien Grall <julien.grall@xxxxxxx> wrote:

[ ... ]

> While looking at the code, I noticed that in the new vgic vgic_get_irq() 
> looks unsafe to be called with interrupt unmasked. This is because one 
> of the callee (vgic_get_lpi()) takes a spinlock and not a spinlock_irq. 
> Andre, what do you think?

I think you are right. In vgic_inject_irq(), right after the call to 
vgic_get_irq(), we use spin_lock_irqsave() on the irq_lock, so using the same 
irqsave version on the lpi_list_lock seems needed. But this is somewhat 
theoretical at the moment, as I think we will never LPIs through the new VGIC 
at the moment.

Cheers,
Andre.

> > Signed-off-by: Andrii Anisov <andrii_anisov@xxxxxxxx>
> > ---
> > 
> > Changes:
> > 
> >      in v2: Drop irq enabling for lpi processing as well.
> > 
> > 
> > ---
> >   xen/arch/arm/gic.c | 4 ----
> >   1 file changed, 4 deletions(-)
> > 
> > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> > index 6cc7dec..113655a 100644
> > --- a/xen/arch/arm/gic.c
> > +++ b/xen/arch/arm/gic.c
> > @@ -386,17 +386,13 @@ void gic_interrupt(struct cpu_user_regs *regs, int 
> > is_fiq)
> >   
> >           if ( likely(irq >= 16 && irq < 1020) )
> >           {
> > -            local_irq_enable();
> >               isb();
> >               do_IRQ(regs, irq, is_fiq);
> > -            local_irq_disable();
> >           }
> >           else if ( is_lpi(irq) )
> >           {
> > -            local_irq_enable();
> >               isb();
> >               gic_hw_ops->do_LPI(irq);
> > -            local_irq_disable();
> >           }
> >           else if ( unlikely(irq < 16) )
> >           {
> >   
> 
> Cheers,
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel