[3/4] target-arm: Change reset to highest available EL

Update to arm_cpu_reset() to reset into the highest available exception level
based on the set ARM features.

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
---
 hw/arm/boot.c    | 10 ++++++++++
 target-arm/cpu.c | 10 +++++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

On 23 January 2015 at 14:49, Greg Bellows <greg.bellows@linaro.org> 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 <greg.bellows@linaro.org>
> ---
>  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

On Fri, Jan 23, 2015 at 9:05 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 23 January 2015 at 14:49, Greg Bellows <greg.bellows@linaro.org> 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 <greg.bellows@linaro.org>
>> ---
>>  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...

Added case.

>
>> +                /* 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.

Added.

>
>> +
>> +                /* 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?

I was thinking that I needed to reset it, but that should be taken
care of by the CP reg reset.  Removed.

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

Yes, Fixed.

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