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

[Julien Grall <julien.grall@arm.com>]

Hi Andre,

On 26/05/17 18:35, Andre Przywara wrote:
> +/*
> + * 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) )
> +    {
> +        unsigned int used_lr = 0;
> +
> +        while ( (used_lr = find_next_bit(lr_mask, nr_lrs, used_lr)) < nr_lrs )

This loop is incorrect. find_next_bit will find the next set bit start 
at the offset used_lr. So if used_lr is set and does match the virq, 
this will turned into an infinite loop.

I would use the macro for_each_set_bit (see xen/bitops.h) here that will 
handle the problem for you:

for_each_set_bit( used_lr, lr_mask, nr_lrs )
{
  ....
}

Cheers,

-- 
Julien Grall

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