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

[Andre Przywara <andre.przywara@arm.com>]

Hi,

On 08/06/17 10:45, Julien Grall wrote:
> 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.

Thanks for catching this! Fixed as hinted below.

Cheers,
Andre.


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

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