Re: [PATCH v10 24/32] ARM: GICv3: handle unmapped LPIs

[Stefano Stabellini <sstabellini@kernel.org>]

On Fri, 2 Jun 2017, Julien Grall wrote:
> Hi Andre,
> 
> On 05/26/2017 06:35 PM, Andre Przywara wrote:
> > @@ -441,6 +443,40 @@ void gic_raise_inflight_irq(struct vcpu *v, unsigned
> > int virtual_irq)
> >   #endif
> >   }
> >   +/*
> > + * Find an unused LR to insert an IRQ into, starting with the LR given
> > + * by @lr. If this new interrupt is a PRISTINE LPI, scan the other LRs to
> > + * avoid inserting the same IRQ twice. This situation can occur when an
> > + * event gets discarded while the LPI is in an LR, and a new LPI with the
> > + * same number gets mapped quickly afterwards.
> > + */
> > +static unsigned int gic_find_unused_lr(struct vcpu *v,
> > +                                       struct pending_irq *p,
> > +                                       unsigned int lr)
> > +{
> > +    unsigned int nr_lrs = gic_hw_ops->info->nr_lrs;
> > +    unsigned long *lr_mask = (unsigned long *) &this_cpu(lr_mask);
> > +    struct gic_lr lr_val;
> > +
> > +    ASSERT(spin_is_locked(&v->arch.vgic.lock));
> > +
> > +    if ( test_bit(GIC_IRQ_GUEST_PRISTINE_LPI, &p->status) )
> 
> Stefano suggested to put an unlikely and ...
> 
> > @@ -479,8 +516,14 @@ static void gic_update_one_lr(struct vcpu *v, int i)
> >       gic_hw_ops->read_lr(i, &lr_val);
> >       irq = lr_val.virq;
> >       p = irq_to_pending(v, irq);
> > -    /* An LPI might have been unmapped, in which case we just clean up
> > here. */
> > -    if ( unlikely(!p) )
> > +    /*
> > +     * An LPI might have been unmapped, in which case we just clean up
> > here.
> > +     * If that LPI is marked as PRISTINE, the information in the LR is
> > bogus,
> > +     * as it belongs to a previous, already unmapped LPI. So we discard it
> > +     * here as well.
> > +     */
> > +    if ( unlikely(!p) ||
> > +         test_and_clear_bit(GIC_IRQ_GUEST_PRISTINE_LPI, &p->status) )
> 
> ... I think we should put one here too.
> 
> With that, I think the patch looks good. I will let Stefano confirm he is
> happy with that too.

Yes, I think it is OK

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