Re: [Qemu-devel] [PATCH] arm: Fix SMC reporting to EL2 when QEMU provides PSCI

[Jan Kiszka]

On 2017-09-18 09:51, Jan Kiszka wrote:
> From: Jan Kiszka <address@hidden>
> 
> This properly forwards SMC events to EL2 when PSCI is provided by QEMU
> itself and, thus, ARM_FEATURE_EL3 is off.
> 
> Found and tested with the Jailhouse hypervisor.
> 
> Signed-off-by: Jan Kiszka <address@hidden>
> ---
>  target/arm/helper.c    | 2 +-
>  target/arm/op_helper.c | 8 ++++----
>  target/arm/psci.c      | 6 ++++++
>  3 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 4f41841ef6..8c3929762c 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -3717,7 +3717,7 @@ static void hcr_write(CPUARMState *env, const 
> ARMCPRegInfo *ri, uint64_t value)
>  
>      if (arm_feature(env, ARM_FEATURE_EL3)) {
>          valid_mask &= ~HCR_HCD;
> -    } else {
> +    } else if (cpu->psci_conduit != QEMU_PSCI_CONDUIT_SMC) {
>          valid_mask &= ~HCR_TSC;
>      }
>  
> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
> index 6a60464ab9..4b0ef6a234 100644
> --- a/target/arm/op_helper.c
> +++ b/target/arm/op_helper.c
> @@ -960,12 +960,12 @@ void HELPER(pre_smc)(CPUARMState *env, uint32_t 
> syndrome)
>          return;
>      }
>  
> -    if (!arm_feature(env, ARM_FEATURE_EL3)) {
> -        /* If we have no EL3 then SMC always UNDEFs */
> -        undef = true;
> -    } else if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {
> +    if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {
>          /* In NS EL1, HCR controlled routing to EL2 has priority over SMD. */
>          raise_exception(env, EXCP_HYP_TRAP, syndrome, 2);
> +    } else if (!arm_feature(env, ARM_FEATURE_EL3)) {
> +        /* If we have no EL3 then SMC always UNDEFs */
> +        undef = true;
>      }
>  
>      if (undef) {
> diff --git a/target/arm/psci.c b/target/arm/psci.c
> index fc34b263d3..637987ff46 100644
> --- a/target/arm/psci.c
> +++ b/target/arm/psci.c
> @@ -35,6 +35,8 @@ bool arm_is_psci_call(ARMCPU *cpu, int excp_type)
>       */
>      CPUARMState *env = &cpu->env;
>      uint64_t param = is_a64(env) ? env->xregs[0] : env->regs[0];
> +    int cur_el = arm_current_el(env);
> +    bool secure = arm_is_secure(env);
>  
>      switch (excp_type) {
>      case EXCP_HVC:
> @@ -46,6 +48,10 @@ bool arm_is_psci_call(ARMCPU *cpu, int excp_type)
>          if (cpu->psci_conduit != QEMU_PSCI_CONDUIT_SMC) {
>              return false;
>          }
> +        if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {
> +            /* The EL2 will handle this. */
> +            return false;
> +        }
>          break;
>      default:
>          return false;
> 

FWIW, we've now a stable (and fast!) QEMU setup running Jailhouse for
aarch64. We just got bitten by a deficit that this setup revealed in our
device tree overlay. See also

https://groups.google.com/forum/#!topic/jailhouse-dev/ZqWpFyMXtZE

Looking forward to eventually expand this to ARMv7, GICv2, or ITS.
Anyone working on those edges already or plan to do so?

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux