Re: [Qemu-devel] [PATCH v2 16/22] hw/intc/arm_gicv3: Implement gicv3_cpuif_update()

[Peter Maydell]

On 13 June 2016 at 08:56, Shannon Zhao <address@hidden> wrote:
>
>
> On 2016/5/26 22:55, Peter Maydell wrote:
>> +void gicv3_cpuif_update(GICv3CPUState *cs)
>> +{
>> +    /* Tell the CPU about its highest priority pending interrupt */
>> +    int irqlevel = 0;
>> +    int fiqlevel = 0;
>> +    ARMCPU *cpu = ARM_CPU(cs->cpu);
>> +    CPUARMState *env = &cpu->env;
>> +
>> +    trace_gicv3_cpuif_update(gicv3_redist_affid(cs), cs->hppi.irq,
>> +                             cs->hppi.grp, cs->hppi.prio);
>> +
>> +    if (cs->hppi.grp == GICV3_G1 && !arm_feature(env, ARM_FEATURE_EL3)) {
>> +        /* If a Security-enabled GIC sends a G1S interrupt to a
>> +         * Security-disabled CPU, we must treat it as if it were G0.
>> +         */
>> +        cs->hppi.grp = GICV3_G0;
>> +    }
>> +
>> +    if (icc_hppi_can_preempt(cs)) {
>> +        /* We have an interrupt: should we signal it as IRQ or FIQ?
>> +         * This is described in the GICv3 spec section 4.6.2.
>> +         */
>> +        bool isfiq;
>> +
>> +        switch (cs->hppi.grp) {
>> +        case GICV3_G0:
>> +            isfiq = true;
>> +            break;
>> +        case GICV3_G1:
>> +            isfiq = (!arm_is_secure(env) ||
>> +                     (arm_current_el(env) == 3 && arm_el_is_aa64(env, 3)));
>> +            break;
>> +        case GICV3_G1NS:
>> +            isfiq = arm_is_secure(env);
>> +            break;
>> +        default:
>> +            g_assert_not_reached();
>> +        }
>> +
>> +        if (isfiq) {
>> +            fiqlevel = 1;
>> +        } else {
>> +            irqlevel = 1;
>> +        }
>> +    }
>> +
>> +    trace_gicv3_cpuif_set_irqs(gicv3_redist_affid(cs), fiqlevel, irqlevel);
>> +
>> +    qemu_set_irq(cs->parent_fiq, fiqlevel);
>> +    qemu_set_irq(cs->parent_irq, irqlevel);
> Does it need to set both fiq and irq for one interrupt? I think it
> should be called differently based on the value of isfiq.

If the state of the CPU changes then an asserted interrupt may
switch from "must assert FIQ" to "must assert IRQ". This means
we need to call qemu_set_irq() for both (in one case with a 0
level and in the other case with a 1 level); we can't just ignore
the IRQ/FIQ line which doesn't apply to this interrupt.
There's also the case of "icc_hppi_can_preempt() returns false" where
we need to make both calls with a 0 level, so we can't simplify to
one call with isfiq and one call with !isfiq.

thanks
-- PMM