I suggested to combine arm_is_secure() and arm_is_secure_below_el3(). Since the code follows the v8 pseudo code my suggestion is really not a good idea. So I apologize twice for wasting your time :)
// Sergey 12.06.2014 22:35, Greg Bellows wrote: > To make sure I understand, you are proposing that we simply use the > return from arm_current_pl of 3 to indicate isSecure? > > Fabian's code closely follows the v8 spec pseudo code. I believe the > case that would be omitted if we let a return of 3 mean "secure" is > the case where we are in EL0/1 with SCR.NS=0. > > So, the to functions are not quite identical. In the case of > arm_current_pl, it returns the PL regardless of the SCR.NS bit. In > the case of arm_is_secure, the secure state takes into consideration > both the PL and the SCR.NS setting. > > > On 12 June 2014 12:26, Sergey Fedorov <serge.f...@gmail.com > <mailto:serge.f...@gmail.com>> wrote: > > Hi Greg, > > I'm sorry, I wasn't thoughtful enough and missed that. > I would just suggest to combine that functions since they have a > common part, i.e.: > > if (arm_feature(env, ARM_FEATURE_EL3)) { > ... > > What do you think? > > // Sergey > > 12.06.2014 20:26, Greg Bellows пишет: > >> Hi Sergey, >> >> I think I am missing your point. In patch 6 arm_current_pl calls >> arm_is_secure. Can you elaborate? >> >> Greg >> >> >> On 11 June 2014 07:17, Sergey Fedorov <serge.f...@gmail.com >> <mailto:serge.f...@gmail.com>> wrote: >> >> On 11.06.2014 03:54, Fabian Aggeler wrote: >> > arm_is_secure() function allows to determine CPU security state >> > if the CPU implements Security Extensions/EL3. >> > arm_is_secure_below_el3() returns true if CPU is in secure >> state >> > below EL3. >> > >> > Signed-off-by: Sergey Fedorov <s.fedo...@samsung.com >> <mailto:s.fedo...@samsung.com>> >> > Signed-off-by: Fabian Aggeler <aggel...@ethz.ch >> <mailto:aggel...@ethz.ch>> >> > --- >> > target-arm/cpu.h | 38 ++++++++++++++++++++++++++++++++++++++ >> > 1 file changed, 38 insertions(+) >> > >> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h >> > index 903aa01..cb0da6b 100644 >> > --- a/target-arm/cpu.h >> > +++ b/target-arm/cpu.h >> > @@ -710,6 +710,44 @@ 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) >> > +{ >> > +#if !defined(CONFIG_USER_ONLY) >> > + if (arm_feature(env, ARM_FEATURE_EL3)) { >> > + return !(env->cp15.scr_el3 & SCR_NS); >> > + } else if (arm_feature(env, ARM_FEATURE_EL2)) { >> > + return false; >> > + } else { >> > + /* IMPDEF: QEMU defaults to non-secure */ >> > + return false; >> > + } >> > +#else >> > + return false; >> > +#endif >> > +} >> > + >> > +/* Return true if the processor is in secure state */ >> > +static inline bool arm_is_secure(CPUARMState *env) >> > +{ >> > +#if !defined(CONFIG_USER_ONLY) >> > + if (arm_feature(env, ARM_FEATURE_EL3)) { >> > + if (env->aarch64 && extract32(env->pstate, 2, 2) >> == 3) { >> > + /* CPU currently in Aarch64 state and EL3 */ >> > + return true; >> > + } else if (!env->aarch64 && >> > + (env->uncached_cpsr & CPSR_M) == >> ARM_CPU_MODE_MON) { >> > + /* CPU currently in Aarch32 state and monitor >> mode */ >> > + return true; >> > + } >> >> Hi Fabian, >> >> Why don't use arm_current_pl() from patch 6 to determine EL here? >> >> Best regards, >> Sergey >> >> > + } >> > + return arm_is_secure_below_el3(env); >> > +#else >> > + return false; >> > +#endif >> > +} >> > + >> > /* Return true if the specified exception level is running >> in AArch64 state. */ >> > static inline bool arm_el_is_aa64(CPUARMState *env, int el) >> > { >> >> > >
