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.
> >>
> >>
>
>

Reply via email to