| Message ID | 1404169773-20264-7-git-send-email-greg.bellows@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 1 July 2014 00:09, <greg.bellows@linaro.org> wrote: > From: Fabian Aggeler <aggelerf@ethz.ch> > > Make arm_current_pl() return PL3 for secure PL1 and monitor mode. > Increase MMU modes since mmu_index is directly infered from arm_ > current_pl(). Changes assertion in arm_el_is_aa64() to allow EL3. > @@ -963,9 +963,12 @@ static inline int arm_current_pl(CPUARMState *env) > > if ((env->uncached_cpsr & 0x1f) == ARM_CPU_MODE_USR) { > return 0; > + } else if (arm_is_secure(env)) { > + /* Secure PL1 and monitor mode are mapped to PL3 */ > + return 3; > } > - /* We don't currently implement the Virtualization or TrustZone > - * extensions, so PL2 and PL3 don't exist for us. > + /* We currently do not implement the Virtualization extensions, so PL2 does > + * not exist for us. > */ > return 1; > } This worries me a bit, because I suspect we have code that's treating arm_current_pl() as if it were arm_current_el(), ie that Secure EL1 will return 1, not 3. Perhaps we need to have both functions and check that all the callers are using the right one? thanks -- PMM
Hi Peter, Perhaps it is best to eliminate the made up "PL3" to avoid confusion with EL3. Then this function can simply always return the correct level whether it is PL or EL. Anywhere we require knowing whether we are secure or not can be checked separately, which may be clearer anyhow. As well, we could add a is_secure_pl1() function that would combine the checks. Thoughts? Regards, Greg On 26 August 2014 09:29, Peter Maydell <peter.maydell@linaro.org> wrote: > On 1 July 2014 00:09, <greg.bellows@linaro.org> wrote: > > From: Fabian Aggeler <aggelerf@ethz.ch> > > > > Make arm_current_pl() return PL3 for secure PL1 and monitor mode. > > Increase MMU modes since mmu_index is directly infered from arm_ > > current_pl(). Changes assertion in arm_el_is_aa64() to allow EL3. > > > @@ -963,9 +963,12 @@ static inline int arm_current_pl(CPUARMState *env) > > > > if ((env->uncached_cpsr & 0x1f) == ARM_CPU_MODE_USR) { > > return 0; > > + } else if (arm_is_secure(env)) { > > + /* Secure PL1 and monitor mode are mapped to PL3 */ > > + return 3; > > } > > - /* We don't currently implement the Virtualization or TrustZone > > - * extensions, so PL2 and PL3 don't exist for us. > > + /* We currently do not implement the Virtualization extensions, so > PL2 does > > + * not exist for us. > > */ > > return 1; > > } > > This worries me a bit, because I suspect we have code that's > treating arm_current_pl() as if it were arm_current_el(), ie that > Secure EL1 will return 1, not 3. Perhaps we need to have > both functions and check that all the callers are using the > right one? > > thanks > -- PMM >
diff --git a/target-arm/cpu.h b/target-arm/cpu.h index aba077b..1faf1e2 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -100,7 +100,7 @@ typedef uint32_t ARMReadCPFunc(void *opaque, int cp_info, struct arm_boot_info; -#define NB_MMU_MODES 2 +#define NB_MMU_MODES 4 /* We currently assume float and double are IEEE single and double precision respectively. @@ -726,7 +726,6 @@ static inline int arm_feature(CPUARMState *env, int feature) return (env->features & (1ULL << feature)) != 0; } - /* Return true if exception level below EL3 is in secure state */ static inline bool arm_is_secure_below_el3(CPUARMState *env) { @@ -767,11 +766,12 @@ static inline bool arm_is_secure(CPUARMState *env) /* Return true if the specified exception level is running in AArch64 state. */ static inline bool arm_el_is_aa64(CPUARMState *env, int el) { - /* We don't currently support EL2 or EL3, and this isn't valid for EL0 + /* We don't currently support EL2, and this isn't valid for EL0 * (if we're in EL0, is_a64() is what you want, and if we're not in EL0 * then the state of EL0 isn't well defined.) */ - assert(el == 1); + assert(el == 1 || el == 3); + /* AArch64-capable CPUs always run with EL1 in AArch64 mode. This * is a QEMU-imposed simplification which we may wish to change later. * If we in future support EL2 and/or EL3, then the state of lower @@ -963,9 +963,12 @@ static inline int arm_current_pl(CPUARMState *env) if ((env->uncached_cpsr & 0x1f) == ARM_CPU_MODE_USR) { return 0; + } else if (arm_is_secure(env)) { + /* Secure PL1 and monitor mode are mapped to PL3 */ + return 3; } - /* We don't currently implement the Virtualization or TrustZone - * extensions, so PL2 and PL3 don't exist for us. + /* We currently do not implement the Virtualization extensions, so PL2 does + * not exist for us. */ return 1; }