Re: [Qemu-devel] [PATCH 3/4] target-arm: Change reset to highest available EL

[Peter Maydell]

On 23 January 2015 at 14:49, Greg Bellows <address@hidden> wrote:
> Update to arm_cpu_reset() to reset into the highest available exception level
> based on the set ARM features.
>
> Signed-off-by: Greg Bellows <address@hidden>
> ---
>  hw/arm/boot.c    | 10 ++++++++++
>  target-arm/cpu.c | 10 +++++++++-
>  2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 52ebd8b..148011b 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -464,6 +464,16 @@ static void do_cpu_reset(void *opaque)
>               * requested.
>               */
>              if (arm_feature(env, ARM_FEATURE_EL3) && !info->secure_boot) {

We also need to handle the "have EL3, secure_boot set, AArch64" case,
for which purposes we need to boot Linux in secure EL1.
Or we could declare that invalid and complain to the user, if you
prefer; it's not a case we actually have to support right now. But
trying to boot the kernel in secure EL3 AArch64 is not going to work...

> +                /* AArch64 is defined to come out of reset into EL3 if 
> enabled.
> +                 * If we are booting Linux then we need to adjust our EL as
> +                 * Linux expects us to be EL1.  AArch32 resets into SVC, 
> which
> +                 * Linux expects, so no privilege/exception level to adjust.
> +                 */
> +                if (env->aarch64) {
> +                    env->pstate = PSTATE_MODE_EL1h;
> +                }

We might as well make this be "EL2 if you have it, else EL1"
while we're adjusting this code.

> +
> +                /* Linux expects non-secure state */
>                  env->cp15.scr_el3 |= SCR_NS;
>              }
>
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 285947f..6793596 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -113,7 +113,15 @@ static void arm_cpu_reset(CPUState *s)
>          /* and to the FP/Neon instructions */
>          env->cp15.c1_coproc = deposit64(env->cp15.c1_coproc, 20, 2, 3);
>  #else
> -        env->pstate = PSTATE_MODE_EL1h;
> +        /* Reset into the highest available EL */
> +        if (arm_feature(env, ARM_FEATURE_EL3)) {
> +            env->pstate = PSTATE_MODE_EL3h;
> +            env->cp15.scr_el3 &= ~SCR_NS;

...why is this code changing the NS bit?

> +        } else if (arm_feature(env, ARM_FEATURE_EL3)) {

ARM_FEATURE_EL2, surely?

> +            env->pstate = PSTATE_MODE_EL2h;
> +        } else {
> +            env->pstate = PSTATE_MODE_EL1h;
> +        }
>          env->pc = cpu->rvbar;
>  #endif
>      } else {
> --

thanks
-- PMM