On 13/10/26 10:38, Peter Maydell wrote:
Currently the GICv5 cpu interface directly calls cpu_interrupt() and
cpu_reset_interrupt() to raise and lower IRQ and FIQ. This is almost
but not exactly what we do for GICv2/GICv3 external GPIO IRQ/FIQ line
changes. Those are handled by arm_cpu_set_irq(), and as well as
calling cpu_interrupt() or cpu_reset_interrupt() they also update
env->irq_line_state.
For IRQ and FIQ it doesn't matter whether we set and clear the bits
in irq_line_state, because nothing looks at them. But for VIRQ and
VFIQ it will matter, because VIRQ and VFIQ are the logical OR of the
line state and some bits in HCR_EL2. We're going to need to call
arm_cpu_set_irq() for VIRQ and VFIQ; for consistency, handle IRQ and
FIQ that way too.
Since arm_cpu_set_irq() is now a function called from other parts of
the CPU code, we make its first argument a properly typed one, with a
wrapper taking void* for when we need to use it as a
qemu_irq_handler.
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Signed-off-by: Peter Maydell <[email protected]>
---
target/arm/cpu.c | 14 +++++++++++---
target/arm/internals.h | 14 ++++++++++++++
target/arm/tcg/gicv5-cpuif.c | 35 ++++-------------------------------
3 files changed, 29 insertions(+), 34 deletions(-)
diff --git a/target/arm/tcg/gicv5-cpuif.c b/target/arm/tcg/gicv5-cpuif.c
index a6e6c440849..517c780d1f7 100644
--- a/target/arm/tcg/gicv5-cpuif.c
+++ b/target/arm/tcg/gicv5-cpuif.c
@@ -173,33 +173,6 @@ static GICv5PendingIrq gic_hppi(CPUARMState *env,
GICv5Domain domain)
return best;
}
-static void cpu_interrupt_update(CPUARMState *env, int irqtype, bool new_state)
-{
- CPUState *cs = env_cpu(env);
-
- /*
- * OPT: calling cpu_interrupt() and cpu_reset_interrupt() has the
- * correct behaviour, but is not optimal for the case where we're
- * setting the interrupt line to the same level it already has.
- *
- * Clearing an already clear interrupt is free (it's just doing an
- * atomic AND operation). Signalling an already set interrupt is a
- * bit less ideal (it might unnecessarily kick the CPU).
- *
- * We could potentially use cpu_test_interrupt(), like
- * arm_cpu_update_{virq,vfiq,vinmi,vserr}, since we always hold
- * the BQL here; or perhaps there is an abstraction we could
- * provide in the core code that all these places could call.
- *
- * For now, this is simple and definitely correct.
- */
- if (new_state) {
- cpu_interrupt(cs, irqtype);
- } else {
- cpu_reset_interrupt(cs, irqtype);
- }
-}
-
static void gicv5_update_irq_fiq(CPUARMState *env)
{
/*
Revisiting this comment:
* For the moment, we do not consider preemptive interrupts,
* because these only occur when there is a HPPI of sufficient
* priority for another interrupt domain, and we only support EL1
* and the NonSecure interrupt domain currently.
Is there a way to assert preemptive interrupts are not accessed
by the guest?
@@ -243,12 +216,12 @@ static void gicv5_update_irq_fiq(CPUARMState *env)
/*
* Unlike a GICv3 or GICv2, there is no external IRQ or FIQ line
* to the CPU. Instead we directly signal the interrupt via
- * cpu_interrupt()/cpu_reset_interrupt().
+ * arm_cpu_set_irq().
*/
trace_gicv5_update_irq_fiq(irq, fiq, superpriority);
- cpu_interrupt_update(env, CPU_INTERRUPT_HARD, irq);
- cpu_interrupt_update(env, CPU_INTERRUPT_FIQ, fiq);
- cpu_interrupt_update(env, CPU_INTERRUPT_NMI, superpriority);
+ arm_cpu_set_irq(env_archcpu(env), ARM_CPU_IRQ, irq);
+ arm_cpu_set_irq(env_archcpu(env), ARM_CPU_FIQ, fiq);
+ arm_cpu_set_irq(env_archcpu(env), ARM_CPU_NMI, superpriority);
}
static void gic_recalc_ppi_hppi(CPUARMState *env)