Re: [Qemu-devel] [PATCH 10/23] hw/intc/arm_gicv3: Implement functions to identify next pending irq

[Peter Maydell]

On 19 May 2016 at 13:59, Shannon Zhao <address@hidden> wrote:
>
>
> On 2016/5/10 1:29, Peter Maydell wrote:
>> +    uint32_t pend, grpmask;
>> +    uint32_t pending = *gic_bmp_ptr32(s->pending, irq - GIC_INTERNAL);
>> +    uint32_t edge_trigger = *gic_bmp_ptr32(s->edge_trigger, irq - 
>> GIC_INTERNAL);
>> +    uint32_t level = *gic_bmp_ptr32(s->level, irq - GIC_INTERNAL);
>> +    uint32_t group = *gic_bmp_ptr32(s->group, irq - GIC_INTERNAL);
>> +    uint32_t grpmod = *gic_bmp_ptr32(s->grpmod, irq - GIC_INTERNAL);
>> +    uint32_t enable = *gic_bmp_ptr32(s->enabled, irq - GIC_INTERNAL);
>> +
> Since you use "irq - GIC_INTERNAL" many times, how about moving into the
> gic_bmp_ptr32()?

The bmp* functions are supposed to be generic (with APIs matching
the bitmap.h ones), so passing it X should give you bit X in the bitmap.

Maybe it would be less confusing to make the bitmaps all have 32 unused
bits at the start, so the bit for interrupt X is bit X in the bitmap...
That only costs us five words of memory per CPU, so it seems worth
it for the code clarity and avoiding annoying bugs.

>>  /**
>> + * gicv3_irq_group:
>> + *
>> + * Return the group which this interrupt is configured as (GICV3_G0,
>> + * GICV3_G1 or GICV3_G1NS).
>> + */
>> +static inline int gicv3_irq_group(GICv3State *s, GICv3CPUState *cs, int irq)
> use uint32_t instead of int in this and other functions?

Neither interrupt numbers nor group numbers are particularly
large or specifically 32-bit, so 'int' seemed the most reasonable
type to use.

thanks
-- PMM