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()?
[...] > +/* Update the GIC status after state in the distributor has > + * changed affecting @len interrupts starting at @start, > + * but don't tell the CPU i/f. > + */ > +static void gicv3_update_noirqset(GICv3State *s, int start, int len) > +{ > + int i; > + uint8_t prio; > + uint32_t pend = 0; > + > + for (i = 0; i < s->num_cpu; i++) { > + s->cpu[i].seenbetter = false; > + } > + > + /* Find the highest priority pending interrupt in this range. */ > + for (i = start; i < start + len; i++) { > + GICv3CPUState *cs; > + The i should start from GIC_INTERNAL or you should change the parameter of the callers. > + if (i == start || (i & 0x1f) == 0) { > + /* Calculate the next 32 bits worth of pending status */ > + pend = gicd_int_pending(s, i & ~0x1f); > + } > + [...] > + > + /* Note that we can guarantee that these functions will not > + * recursively call back into gicv3_full_update(), because > + * at each point the "previous best" is always outside the > + * range we ask them to update. > + */ > + gicv3_update_noirqset(s, 0, s->num_irq); > + use GIC_INTERNAL instead of 0? > /** > + * 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? Thanks, -- Shannon