On 19 May 2016 at 13:59, Shannon Zhao <zhaoshengl...@huawei.com> 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
