I suppose it depends on how true we want to be to the specification and whether our default is NS=0 or NS=1 when the security extension is present or not. The code currently assumes non-secure as the default state.
Is there a convention in qemu? How closely do we attempt to stay to the pseudo code provided in the spec? On 14 May 2014 13:35, Fedorov Sergey <serge.f...@gmail.com> wrote: > > 14.05.2014 18:42, Greg Bellows пишет: > > On 14 May 2014 00:53, Sergey Fedorov <serge.f...@gmail.com> wrote: > > > >> On 13.05.2014 20:15, Fabian Aggeler wrote: > >>> arm_is_secure() function allows to determine CPU security state > >>> if the CPU implements Security Extensions. > >>> > >>> Signed-off-by: Sergey Fedorov <s.fedo...@samsung.com> > >>> Signed-off-by: Fabian Aggeler <aggel...@ethz.ch> > >>> --- > >>> target-arm/cpu.h | 15 +++++++++++++++ > >>> 1 file changed, 15 insertions(+) > >>> > >>> diff --git a/target-arm/cpu.h b/target-arm/cpu.h > >>> index a56d3d6..6ea0432 100644 > >>> --- a/target-arm/cpu.h > >>> +++ b/target-arm/cpu.h > >>> @@ -640,6 +640,21 @@ static inline int arm_feature(CPUARMState *env, > int > >> feature) > >>> return (env->features & (1ULL << feature)) != 0; > >>> } > >>> > >>> +/* 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_SECURITY_EXTENSIONS)) { > >> I think feature test can be safely avoided here. Without this feature > >> that should be no way to switch to monitor mode and to access SCR > register. > >> > > I agree with the feature check here. For correctness, we should only be > > examining c1_scr if the security extension is enabled. This is > consistent > > with only registering the SCR register if the feature is enabled. > > So this check will be done every time arm_is_secure() is called, e.g. on > each MMU table walk. > > Moreover I've noticed that this function deviates from ARM ARM v7-AR > description in section B1.5.1 which states: "The IsSecure() function > returns TRUE if the processor is in Secure state, or if the > implementation does not include > the Security Extensions, and FALSE otherwise." Then there is a pseudo > code for that function. > > > > >>> + return ((env->uncached_cpsr & CPSR_M) == ARM_CPU_MODE_MON) || > >>> + !(env->cp15.c1_scr & 1); > >>> + } else { > >>> + return false; > >>> + } > >>> +#else > >>> + return false; > >> That is a good question how to treat user emulation: secure or > >> non-secure. Perhaps assuming user emulation in secure state may simplify > >> code in the following patches. > > > >>> +#endif > >>> +} > >>> + > >>> /* Return true if the specified exception level is running in AArch64 > >> state. */ > >>> static inline bool arm_el_is_aa64(CPUARMState *env, int el) > >>> { > >> Thanks, > >> Sergey. > >> > >> > >