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

[Peter Maydell]

On 8 June 2016 at 02:57, Shannon Zhao <address@hidden> wrote:
>
>
> On 2016/5/26 22:55, Peter Maydell wrote:
>> Implement the GICv3 logic to recalculate the highest priority pending
>> interrupt for each CPU after some part of the GIC state has changed.
>> We avoid unnecessary full recalculation where possible.
>>
>> Signed-off-by: Peter Maydell <address@hidden>

>> +static uint32_t gicd_int_pending(GICv3State *s, int irq)
>> +{
>> +    /* Recalculate which redistributor interrupts are actually pending
> s/redistributor/distributor/

Fixed.

>> +static inline int gicv3_irq_group(GICv3State *s, GICv3CPUState *cs, int irq)
>> +{
>> +    bool grpbit, grpmodbit;
>> +
>> +    if (irq < GIC_INTERNAL) {
>> +        grpbit = extract32(cs->gicr_igroupr0, irq, 1);
>> +        grpmodbit = extract32(cs->gicr_igrpmodr0, irq, 1);
>> +    } else {
>> +        grpbit = gicv3_gicd_group_test(s, irq);
>> +        grpmodbit = gicv3_gicd_grpmod_test(s, irq);
>> +    }
>> +    if (grpbit) {
>> +        return GICV3_G1NS;
>> +    }
>> +    if (s->gicd_ctlr & GICD_CTLR_DS) {
>> +        return GICV3_G0;
>> +    }
>> +    return grpmodbit ? GICV3_G1 : GICV3_G0;
> Maybe it could be written like below:
>     if (s->gicd_ctlr & GICD_CTLR_DS || !grpmodbit) {
>         return GICV3_G0;
>     }
>     return GICV3_G1;

I think I prefer the way it is currently, as it better
matches what we're trying to say:
 * if security is disabled, there is no grpmodbit, so always G0
 * otherwise, look at grpmodbit to determine G1 vs G0

>> +static inline void gicv3_cache_target_cpustate(GICv3State *s, int irq)
>> +{
>> +    GICv3CPUState *cs = NULL;
>> +    int i;
>> +    uint32_t tgtaff = extract64(s->gicd_irouter[irq], 0, 24) |
>> +        extract64(s->gicd_irouter[irq], 32, 8);
> This should be extract64(s->gicd_irouter[irq], 32, 8) << 24

Oops. Fixed.

thanks
-- PMM