Re: [Qemu-devel] [PATCH v2 04/22] target-arm: Provide hook to tell GICv3 about changes of security state

[Shannon Zhao]

On 2016/5/26 22:55, Peter Maydell wrote:
> The GICv3 CPU interface needs to know when the CPU it is attached
> to makes an exception level or mode transition that changes the
> security state, because whether it is asserting IRQ or FIQ can change
> depending on these things. Provide a mechanism for letting the GICv3
> device register a hook to be called on such changes.
> 
> Signed-off-by: Peter Maydell <address@hidden>
Reviewed-by: Shannon Zhao <address@hidden>

> ---
>  target-arm/cpu.c       |  9 +++++++++
>  target-arm/cpu.h       | 34 ++++++++++++++++++++++++++++++++++
>  target-arm/helper.c    |  2 ++
>  target-arm/internals.h |  8 ++++++++
>  target-arm/op_helper.c |  4 ++++
>  5 files changed, 57 insertions(+)
> 
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 3fd0743..0eaa907 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -51,6 +51,15 @@ static bool arm_cpu_has_work(CPUState *cs)
>           | CPU_INTERRUPT_EXITTB);
>  }
>  
> +void arm_register_el_change_hook(ARMCPU *cpu, ARMELChangeHook *hook,
> +                                 void *opaque)
> +{
> +    /* We currently only support registering a single hook function */
> +    assert(!cpu->el_change_hook);
> +    cpu->el_change_hook = hook;
> +    cpu->el_change_hook_opaque = opaque;
> +}
> +
>  static void cp_reg_reset(gpointer key, gpointer value, gpointer opaque)
>  {
>      /* Reset a single ARMCPRegInfo register */
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 2fa1f41..9b045af 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -504,6 +504,13 @@ typedef struct CPUARMState {
>  } CPUARMState;
>  
>  /**
> + * ARMELChangeHook:
> + * type of a function which can be registered via 
> arm_register_el_change_hook()
> + * to get callbacks when the CPU changes its exception level or mode.
> + */
> +typedef void ARMELChangeHook(ARMCPU *cpu, void *opaque);
> +
> +/**
>   * ARMCPU:
>   * @env: #CPUARMState
>   *
> @@ -641,6 +648,9 @@ struct ARMCPU {
>      /* DCZ blocksize, in log_2(words), ie low 4 bits of DCZID_EL0 */
>      uint32_t dcz_blocksize;
>      uint64_t rvbar;
> +
> +    ARMELChangeHook *el_change_hook;
> +    void *el_change_hook_opaque;
>  };
>  
>  static inline ARMCPU *arm_env_get_cpu(CPUARMState *env)
> @@ -2373,4 +2383,28 @@ static inline AddressSpace *arm_addressspace(CPUState 
> *cs, MemTxAttrs attrs)
>  }
>  #endif
>  
> +/**
> + * arm_register_el_change_hook:
> + * Register a hook function which will be called back whenever this
> + * CPU changes exception level or mode. The hook function will be
> + * passed a pointer to the ARMCPU and the opaque data pointer passed
> + * to this function when the hook was registered.
> + *
> + * Note that we currently only support registering a single hook function,
> + * and will assert if this function is called twice.
> + * This facility is intended for the use of the GICv3 emulation.
> + */
> +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;
> +}
> +
>  #endif
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index e3ea26f..d907598 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -6496,6 +6496,8 @@ void arm_cpu_do_interrupt(CPUState *cs)
>          arm_cpu_do_interrupt_aarch32(cs);
>      }
>  
> +    arm_call_el_change_hook(cpu);
> +
>      if (!kvm_enabled()) {
>          cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
>      }
> diff --git a/target-arm/internals.h b/target-arm/internals.h
> index a125873..5d8ec43 100644
> --- a/target-arm/internals.h
> +++ b/target-arm/internals.h
> @@ -479,4 +479,12 @@ bool arm_s1_regime_using_lpae_format(CPUARMState *env, 
> ARMMMUIdx mmu_idx);
>  void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, int is_write,
>                                   int is_user, uintptr_t retaddr);
>  
> +/* Call the EL change hook if one has been registered */
> +static inline void arm_call_el_change_hook(ARMCPU *cpu)
> +{
> +    if (cpu->el_change_hook) {
> +        cpu->el_change_hook(cpu, cpu->el_change_hook_opaque);
> +    }
> +}
> +
>  #endif
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 0b29b9d..8021738 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -437,6 +437,8 @@ void HELPER(cpsr_write)(CPUARMState *env, uint32_t val, 
> uint32_t mask)
>  void HELPER(cpsr_write_eret)(CPUARMState *env, uint32_t val)
>  {
>      cpsr_write(env, val, CPSR_ERET_MASK, CPSRWriteExceptionReturn);
> +
> +    arm_call_el_change_hook(arm_env_get_cpu(env));
>  }
>  
>  /* Access to user mode registers from privileged modes.  */
> @@ -932,6 +934,8 @@ void HELPER(exception_return)(CPUARMState *env)
>          env->pc = env->elr_el[cur_el];
>      }
>  
> +    arm_call_el_change_hook(arm_env_get_cpu(env));
> +
>      return;
>  
>  illegal_return:
> 

-- 
Shannon