diff mbox

[1/8] target-arm: Properly support EL2 and EL3 in arm_el_is_aa64()

Message ID 20160115153737.GL29396@toto (mailing list archive)
State New, archived
Headers show

Commit Message

Edgar E. Iglesias Jan. 15, 2016, 3:37 p.m. UTC 
On Fri, Jan 15, 2016 at 02:50:24PM +0000, Peter Maydell wrote:
> On 15 January 2016 at 14:38, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > On Thu, Jan 14, 2016 at 06:34:04PM +0000, Peter Maydell wrote:
> >> Support EL2 and EL3 in arm_el_is_aa64() by implementing the
> >> logic for checking the SCR_EL3 and HCR_EL2 register-width bits
> >> as appropriate to determine the register width of lower exception
> >> levels.
> >>
> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> >
> > Hi Peter,
> >
> > On the ZynqMP we've got the Cortex-A53 EL3 RW configurable at reset
> > time. At some later point we'll likely have to implement that
> > runtime option...
> 
> That might be tricky, we fairly well bake in "AARCH64 feature means
> 64-bit highest EL" at the moment. The KVM code takes the approach
> of "if it's not going to reset in AArch64 then unset the feature bit".
> 
> Anyway, we'll cross that bridge when we get to it.
> 
> Do you have much locally extra that you needed for enabling
> EL3 in the Cortex-A53? I have an ARM Trusted Firmware + OP-TEE
> setup now that I'm going to use to work through the missing bits,
> but if you've already gone through that effort there's no need
> my duplicating work...

I don't have anything immediate for EL3 beyond enabling it and some
boot thing for a15/aarch32 to allow me to run my tests. I haven't
really looked at the boot in detail for aa32 so I haven't bothered
submitting it. This is it:

commit b30c7102624241a67ebb2d3df70e88a4148f68a4
Author: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Date:   Sun Sep 13 09:52:01 2015 +0200

    target-arm: Start EL3 capable ARMv7 cores in MON mode
    
    Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Comments

Peter Maydell Jan. 15, 2016, 3:47 p.m. UTC | #1 
On 15 January 2016 at 15:37, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> On Fri, Jan 15, 2016 at 02:50:24PM +0000, Peter Maydell wrote:
>> Do you have much locally extra that you needed for enabling
>> EL3 in the Cortex-A53? I have an ARM Trusted Firmware + OP-TEE
>> setup now that I'm going to use to work through the missing bits,
>> but if you've already gone through that effort there's no need
>> my duplicating work...
>
> I don't have anything immediate for EL3 beyond enabling it and some
> boot thing for a15/aarch32 to allow me to run my tests.

Cool. I'm not sure at what point to add the patch that turns
on the EL3 feature bit, but I guess in the not too distant future :-)

> I haven't
> really looked at the boot in detail for aa32 so I haven't bothered
> submitting it. This is it:
>
> commit b30c7102624241a67ebb2d3df70e88a4148f68a4
> Author: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> Date:   Sun Sep 13 09:52:01 2015 +0200
>
>     target-arm: Start EL3 capable ARMv7 cores in MON mode
>
>     Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index f6f5539..485965f 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -164,6 +164,9 @@ static void arm_cpu_reset(CPUState *s)
>  #else
>      /* SVC mode with interrupts disabled.  */
>      env->uncached_cpsr = ARM_CPU_MODE_SVC;
> +    if (arm_feature(env, ARM_FEATURE_EL3)) {
> +        env->uncached_cpsr = ARM_CPU_MODE_MON;
> +    }
>      env->daif = PSTATE_D | PSTATE_A | PSTATE_I | PSTATE_F;
>      /* On ARMv7-M the CPSR_I is the value of the PRIMASK register, and is
>       * clear at reset. Initial SP and PC are loaded from ROM.

This doesn't look right. A 32-bit CPU with TrustZone boots into
Secure-SVC, not Mon. This works because in the v7 security model
Secure-SVC and Mon are at the same privilege level, unlike AArch64
where EL3 is higher privilege than EL1. If guest code needs to
get into Mon mode it can do so from S-SVC (eg set MVBAR, make
sure there's sensible code at that vector entrypoint, execute
an SMC).

thanks
-- PMM
Edgar E. Iglesias Jan. 15, 2016, 8:37 p.m. UTC | #2 
On Fri, Jan 15, 2016 at 03:47:17PM +0000, Peter Maydell wrote:
> On 15 January 2016 at 15:37, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > On Fri, Jan 15, 2016 at 02:50:24PM +0000, Peter Maydell wrote:
> >> Do you have much locally extra that you needed for enabling
> >> EL3 in the Cortex-A53? I have an ARM Trusted Firmware + OP-TEE
> >> setup now that I'm going to use to work through the missing bits,
> >> but if you've already gone through that effort there's no need
> >> my duplicating work...
> >
> > I don't have anything immediate for EL3 beyond enabling it and some
> > boot thing for a15/aarch32 to allow me to run my tests.
> 
> Cool. I'm not sure at what point to add the patch that turns
> on the EL3 feature bit, but I guess in the not too distant future :-)

IMO, we should have enabled it by now :-)
I guess the EL2 needs the 2 stage MMU error handling and maybe the GIC virt features
to be reasonably useful, I don't know.

> 
> > I haven't
> > really looked at the boot in detail for aa32 so I haven't bothered
> > submitting it. This is it:
> >
> > commit b30c7102624241a67ebb2d3df70e88a4148f68a4
> > Author: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > Date:   Sun Sep 13 09:52:01 2015 +0200
> >
> >     target-arm: Start EL3 capable ARMv7 cores in MON mode
> >
> >     Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> >
> > diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> > index f6f5539..485965f 100644
> > --- a/target-arm/cpu.c
> > +++ b/target-arm/cpu.c
> > @@ -164,6 +164,9 @@ static void arm_cpu_reset(CPUState *s)
> >  #else
> >      /* SVC mode with interrupts disabled.  */
> >      env->uncached_cpsr = ARM_CPU_MODE_SVC;
> > +    if (arm_feature(env, ARM_FEATURE_EL3)) {
> > +        env->uncached_cpsr = ARM_CPU_MODE_MON;
> > +    }
> >      env->daif = PSTATE_D | PSTATE_A | PSTATE_I | PSTATE_F;
> >      /* On ARMv7-M the CPSR_I is the value of the PRIMASK register, and is
> >       * clear at reset. Initial SP and PC are loaded from ROM.
> 
> This doesn't look right. A 32-bit CPU with TrustZone boots into
> Secure-SVC, not Mon. This works because in the v7 security model
> Secure-SVC and Mon are at the same privilege level, unlike AArch64
> where EL3 is higher privilege than EL1. If guest code needs to
> get into Mon mode it can do so from S-SVC (eg set MVBAR, make
> sure there's sensible code at that vector entrypoint, execute
> an SMC).

Thanks. I figured my zero research would prove me wrong :-)
Thinking about it, I have never ran my aa32 TZ testsuite on real
HW so there are tons of stuff that could be wrong!

Best regards,
Edgar
diff mbox

Patch

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index f6f5539..485965f 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -164,6 +164,9 @@  static void arm_cpu_reset(CPUState *s)
 #else
     /* SVC mode with interrupts disabled.  */
     env->uncached_cpsr = ARM_CPU_MODE_SVC;
+    if (arm_feature(env, ARM_FEATURE_EL3)) {
+        env->uncached_cpsr = ARM_CPU_MODE_MON;
+    }
     env->daif = PSTATE_D | PSTATE_A | PSTATE_I | PSTATE_F;
     /* On ARMv7-M the CPSR_I is the value of the PRIMASK register, and is
      * clear at reset. Initial SP and PC are loaded from ROM.