[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/4] target-arm: Change reset to highest availab
From: |
Greg Bellows |
Subject: |
Re: [Qemu-devel] [PATCH 3/4] target-arm: Change reset to highest available EL |
Date: |
Fri, 23 Jan 2015 09:25:42 -0600 |
On Fri, Jan 23, 2015 at 9:05 AM, Peter Maydell <address@hidden> wrote:
> 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...
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