[v3,07/22] target/arm: Fetch GICv3 state directly from CPUARMState

Commit Message

Aaron Lindsay March 16, 2018, 8:31 p.m. UTC

This eliminates the need for fetching it from el_change_hook_opaque, and
allows for supporting multiple el_change_hooks without having to hack
something together to find the registered opaque belonging to GICv3.

Signed-off-by: Aaron Lindsay <[e-mail]>
---
 hw/intc/arm_gicv3_cpuif.c | 10 ++--------
 target/arm/cpu.h          | 10 ----------
 2 files changed, 2 insertions(+), 18 deletions(-)

Comments

Peter Maydell April 12, 2018, 4:28 p.m. UTC | #1

On 16 March 2018 at 20:31, Aaron Lindsay <[e-mail]> wrote:
> This eliminates the need for fetching it from el_change_hook_opaque, and
> allows for supporting multiple el_change_hooks without having to hack
> something together to find the registered opaque belonging to GICv3.
>
> Signed-off-by: Aaron Lindsay <[e-mail]>
> ---
>  hw/intc/arm_gicv3_cpuif.c | 10 ++--------
>  target/arm/cpu.h          | 10 ----------
>  2 files changed, 2 insertions(+), 18 deletions(-)

I'm not wonderfully happy about this, because the original aim
here was to try to keep the GICv3 device end reasonably decoupled
from the CPU proper. On the other hand since d3a3e529626fb we've had
this pointer because we ended up needing it in the KVM reset codepath,
so we might as well use it here too. Maybe one day I'll come back
to this and clean it up.

Reviewed-by: Peter Maydell <[e-mail]>

thanks
-- PMM

Patch

diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index 5cbafaf..801f91b 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -29,11 +29,7 @@  void gicv3_set_gicv3state(CPUState *cpu, GICv3CPUState *s)
 
 static GICv3CPUState *icc_cs_from_env(CPUARMState *env)
 {
-    /* Given the CPU, find the right GICv3CPUState struct.
-     * Since we registered the CPU interface with the EL change hook as
-     * the opaque pointer, we can just directly get from the CPU to it.
-     */
-    return arm_get_el_change_hook_opaque(arm_env_get_cpu(env));
+    return env->gicv3state;
 }
 
 static bool gicv3_use_ns_bank(CPUARMState *env)
@@ -2615,9 +2611,7 @@  void gicv3_init_cpuif(GICv3State *s)
          * it might be with code translated by CPU 0 but run by CPU 1, in
          * which case we'd get the wrong value.
          * So instead we define the regs with no ri->opaque info, and
-         * get back to the GICv3CPUState from the ARMCPU by reading back
-         * the opaque pointer from the el_change_hook, which we're going
-         * to need to register anyway.
+         * get back to the GICv3CPUState from the CPUARMState.
          */
         define_arm_cp_regs(cpu, gicv3_cpuif_reginfo);
         if (arm_feature(&cpu->env, ARM_FEATURE_EL2)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 1e7e1f8..f17592b 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2904,16 +2904,6 @@  void arm_register_el_change_hook(ARMCPU *cpu, ARMELChangeHook *hook,
                                  void *opaque);
 
 /**
- * arm_get_el_change_hook_opaque:
- * Return the opaque data that will be used by the el_change_hook
- * for this CPU.
- */
-static inline void *arm_get_el_change_hook_opaque(ARMCPU *cpu)
-{
-    return cpu->el_change_hook_opaque;
-}
-
-/**
  * aa32_vfp_dreg:
  * Return a pointer to the Dn register within env in 32-bit mode.
  */