On 24 October 2014 11:40, Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 21 October 2014 17:55, Greg Bellows <greg.bell...@linaro.org> wrote:
> > From: Sergey Fedorov <s.fedo...@samsung.com>
> >
> > This patch is based on idea found in patch at
> > git://github.com/jowinter/qemu-trustzone.git
> > f3d955c6c0ed8c46bc0eb10b634201032a651dd2 by
> > Johannes Winter <johannes.win...@iaik.tugraz.at>.
> >
> > The TBFLAG captures the SCR NS secure state at the time when a TB is
> created so
> > the correct bank is accessed on system register accesses. It also
> allows to
> > generate different TCG code depending on CPU secure state.
>
> This last sentence is wrong and should just be deleted.
>
Done.
>
> > Signed-off-by: Sergey Fedorov <s.fedo...@samsung.com>
> > Signed-off-by: Fabian Aggeler <aggel...@ethz.ch>
> > Signed-off-by: Greg Bellows <greg.bell...@linaro.org>
> >
> > ==========
> >
> > v5 -> v6
> > - Removed 64-bit NS TBFLAG macros as they are not needed
> > - Added comment on DisasContext ns field
> > - Replaced use of USE_SECURE_REG with use_secure_reg
> >
> > v4 -> v5
> > - Merge changes
> > - Fixed issue where TB secure state flag was incorrectly being set based
> on
> > secure state rather than NS setting. This caused an issue where
> monitor mode
> > MRC/MCR accesses were always secure rather than being based on NS bit
> > setting.
> > - Added separate 64/32 TB secure state flags
> > - Unconditionalized the setting of the DC ns bit
> > - Removed IS_NS macro and replaced with direct usage.
> >
> > Signed-off-by: Greg Bellows <greg.bell...@linaro.org>
> > ---
> > target-arm/cpu.h | 7 +++++++
> > target-arm/translate.c | 1 +
> > target-arm/translate.h | 1 +
> > 3 files changed, 9 insertions(+)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index b48b81a..e041437 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -1551,6 +1551,8 @@ static inline bool
> arm_singlestep_active(CPUARMState *env)
> > */
> > #define ARM_TBFLAG_XSCALE_CPAR_SHIFT 20
> > #define ARM_TBFLAG_XSCALE_CPAR_MASK (3 << ARM_TBFLAG_XSCALE_CPAR_SHIFT)
>
> /* Indicates whether cp register reads and writes by guest code should
> access
> * the secure or nonsecure bank of banked registers; note that this is not
> * the same thing as the current security state of the processor!
> */
>
> If you add this comment and move and rename the use_secure_reg()
> function from the previous patch as I suggest, you can add my
> reviewed-by tag.
>
>
Function moved and renamed and comment added in v8.
> > +#define ARM_TBFLAG_NS_SHIFT 22
> > +#define ARM_TBFLAG_NS_MASK (1 << ARM_TBFLAG_NS_SHIFT)
> >
> > /* Bit usage when in AArch64 state */
> > #define ARM_TBFLAG_AA64_EL_SHIFT 0
> > @@ -1595,6 +1597,8 @@ static inline bool
> arm_singlestep_active(CPUARMState *env)
> > (((F) & ARM_TBFLAG_AA64_SS_ACTIVE_MASK) >>
> ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT)
> > #define ARM_TBFLAG_AA64_PSTATE_SS(F) \
> > (((F) & ARM_TBFLAG_AA64_PSTATE_SS_MASK) >>
> ARM_TBFLAG_AA64_PSTATE_SS_SHIFT)
> > +#define ARM_TBFLAG_NS(F) \
> > + (((F) & ARM_TBFLAG_NS_MASK) >> ARM_TBFLAG_NS_SHIFT)
> >
> > static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong
> *pc,
> > target_ulong *cs_base, int
> *flags)
> > @@ -1644,6 +1648,9 @@ static inline void
> cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
> > if (privmode) {
> > *flags |= ARM_TBFLAG_PRIV_MASK;
> > }
> > + if (!(use_secure_reg(env))) {
> > + *flags |= ARM_TBFLAG_NS_MASK;
> > + }
> > if (env->vfp.xregs[ARM_VFP_FPEXC] & (1 << 30)
> > || arm_el_is_aa64(env, 1)) {
> > *flags |= ARM_TBFLAG_VFPEN_MASK;
> > diff --git a/target-arm/translate.c b/target-arm/translate.c
> > index 91958b6..aa17a20 100644
> > --- a/target-arm/translate.c
> > +++ b/target-arm/translate.c
> > @@ -11002,6 +11002,7 @@ static inline void
> gen_intermediate_code_internal(ARMCPU *cpu,
> > #if !defined(CONFIG_USER_ONLY)
> > dc->user = (ARM_TBFLAG_PRIV(tb->flags) == 0);
> > #endif
> > + dc->ns = ARM_TBFLAG_NS(tb->flags);
> > dc->cpacr_fpen = ARM_TBFLAG_CPACR_FPEN(tb->flags);
> > dc->vfp_enabled = ARM_TBFLAG_VFPEN(tb->flags);
> > dc->vec_len = ARM_TBFLAG_VECLEN(tb->flags);
> > diff --git a/target-arm/translate.h b/target-arm/translate.h
> > index 41a9071..f6ee789 100644
> > --- a/target-arm/translate.h
> > +++ b/target-arm/translate.h
> > @@ -20,6 +20,7 @@ typedef struct DisasContext {
> > #if !defined(CONFIG_USER_ONLY)
> > int user;
> > #endif
> > + bool ns; /* Use non-secure CPREG bank on access */
> > bool cpacr_fpen; /* FP enabled via CPACR.FPEN */
> > bool vfp_enabled; /* FP enabled via FPSCR.EN */
> > int vec_len;
> > --
> > 1.8.3.2
> >
>
>
> thanks
> -- PMM
>
