Re: [Qemu-devel] [PATCH v2 16/18] target/arm/psci.c: If EL2 implemented, start CPUs in EL2

[Andrew Jones]

On Mon, Jan 09, 2017 at 04:05:22PM +0000, Peter Maydell wrote:
> The PSCI spec states that a CPU_ON call should cause the new
> CPU to be started in the highest implemented Non-secure
> exception level. We were incorrectly starting it at the
> exception level of the caller, which happens to be correct
> if EL2 is not implemented. Implement the correct logic
> as described in the PSCI 1.0 spec section 6.4:
>  * if EL2 exists and SCR_EL3.HCE is set: start in EL2
>  * otherwise start in EL1
> 
> Signed-off-by: Peter Maydell <address@hidden>
> ---
>  target/arm/psci.c | 25 ++++++++++++++++++-------
>  1 file changed, 18 insertions(+), 7 deletions(-)

Reviewed-by: Andrew Jones <address@hidden>
Tested-by: Andrew Jones <address@hidden>

> 
> diff --git a/target/arm/psci.c b/target/arm/psci.c
> index 14316eb..64bf82e 100644
> --- a/target/arm/psci.c
> +++ b/target/arm/psci.c
> @@ -148,17 +148,28 @@ void arm_handle_psci_call(ARMCPU *cpu)
>      case QEMU_PSCI_0_1_FN_CPU_ON:
>      case QEMU_PSCI_0_2_FN_CPU_ON:
>      case QEMU_PSCI_0_2_FN64_CPU_ON:
> +    {
> +        /* The PSCI spec mandates that newly brought up CPUs start
> +         * in the highest exception level which exists and is enabled
> +         * on the calling CPU. Since the QEMU PSCI implementation is
> +         * acting as a "fake EL3" or "fake EL2" firmware, this for us
> +         * means that we want to start at the highest NS exception level
> +         * that we are providing to the guest.
> +         * The execution mode should be that which is currently in use
> +         * by the same exception level on the calling CPU.
> +         * The CPU should be started with the context_id value
> +         * in x0 (if AArch64) or r0 (if AArch32).
> +         */
> +        int target_el = arm_feature(env, ARM_FEATURE_EL2) ? 2 : 1;
> +        bool target_aarch64 = arm_el_is_aa64(env, target_el);
> +
>          mpidr = param[1];
>          entry = param[2];
>          context_id = param[3];
> -        /*
> -         * The PSCI spec mandates that newly brought up CPUs enter the
> -         * exception level of the caller in the same execution mode as
> -         * the caller, with context_id in x0/r0, respectively.
> -         */
> -        ret = arm_set_cpu_on(mpidr, entry, context_id, arm_current_el(env),
> -                             is_a64(env));
> +        ret = arm_set_cpu_on(mpidr, entry, context_id,
> +                             target_el, target_aarch64);
>          break;
> +    }
>      case QEMU_PSCI_0_1_FN_CPU_OFF:
>      case QEMU_PSCI_0_2_FN_CPU_OFF:
>          goto cpu_off;
> -- 
> 2.7.4
> 
>