Thanks Peter. I'll fix these in v5.
On 2 September 2014 11:11, Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 1 July 2014 00:09, <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>.
> >
> > This flag prevents QEMU from executing TCG code generated for other CPU
> > security state. It also allows to generate different TCG code depending
> on
> > CPU secure state.
> >
> > 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>
> > ---
> > target-arm/cpu.h | 10 ++++++++++
> > target-arm/translate-a64.c | 1 +
> > target-arm/translate.c | 3 +++
> > target-arm/translate.h | 1 +
> > 4 files changed, 15 insertions(+)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index 1faf1e2..44e0943 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -1291,6 +1291,8 @@ static inline int cpu_mmu_index (CPUARMState *env)
> > #define ARM_TBFLAG_BSWAP_CODE_MASK (1 << ARM_TBFLAG_BSWAP_CODE_SHIFT)
> > #define ARM_TBFLAG_CPACR_FPEN_SHIFT 17
> > #define ARM_TBFLAG_CPACR_FPEN_MASK (1 << ARM_TBFLAG_CPACR_FPEN_SHIFT)
> > +#define ARM_TBFLAG_NS_SHIFT 18
> > +#define ARM_TBFLAG_NS_MASK (1 << ARM_TBFLAG_NS_SHIFT)
> >
> > /* Bit usage when in AArch64 state */
> > #define ARM_TBFLAG_AA64_EL_SHIFT 0
> > @@ -1321,6 +1323,8 @@ static inline int cpu_mmu_index (CPUARMState *env)
> > (((F) & ARM_TBFLAG_AA64_EL_MASK) >> ARM_TBFLAG_AA64_EL_SHIFT)
> > #define ARM_TBFLAG_AA64_FPEN(F) \
> > (((F) & ARM_TBFLAG_AA64_FPEN_MASK) >> ARM_TBFLAG_AA64_FPEN_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)
> > @@ -1334,6 +1338,9 @@ static inline void
> cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
> > if (fpen == 3 || (fpen == 1 && arm_current_pl(env) != 0)) {
> > *flags |= ARM_TBFLAG_AA64_FPEN_MASK;
> > }
> > + if (!arm_is_secure(env)) {
> > + *flags |= ARM_TBFLAG_NS_MASK;
> > + }
> > } else {
> > int privmode;
> > *pc = env->regs[15];
> > @@ -1350,6 +1357,9 @@ static inline void
> cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
> > if (privmode) {
> > *flags |= ARM_TBFLAG_PRIV_MASK;
> > }
> > + if (!arm_is_secure(env)) {
> > + *flags |= ARM_TBFLAG_NS_MASK;
> > + }
>
> You can't share the same TBFLAG between the AArch64
> and AArch32 tb flags like this -- they have different layouts
> of the bits in the flags word.
>
> > 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-a64.c b/target-arm/translate-a64.c
> > index 446d2cd..ad30903 100644
> > --- a/target-arm/translate-a64.c
> > +++ b/target-arm/translate-a64.c
> > @@ -10879,6 +10879,7 @@ void gen_intermediate_code_internal_a64(ARMCPU
> *cpu,
> > dc->condexec_cond = 0;
> > #if !defined(CONFIG_USER_ONLY)
> > dc->user = (ARM_TBFLAG_AA64_EL(tb->flags) == 0);
> > + dc->ns = ARM_TBFLAG_NS(tb->flags);
> > #endif
> > dc->cpacr_fpen = ARM_TBFLAG_AA64_FPEN(tb->flags);
> > dc->vec_len = 0;
> > diff --git a/target-arm/translate.c b/target-arm/translate.c
> > index cf4e767..bf17952 100644
> > --- a/target-arm/translate.c
> > +++ b/target-arm/translate.c
> > @@ -53,8 +53,10 @@ static uint32_t gen_opc_condexec_bits[OPC_BUF_SIZE];
> >
> > #if defined(CONFIG_USER_ONLY)
> > #define IS_USER(s) 1
> > +#define IS_NS(s) 1
> > #else
> > #define IS_USER(s) (s->user)
> > +#define IS_NS(s) (s->ns)
> > #endif
> >
> > TCGv_ptr cpu_env;
> > @@ -10904,6 +10906,7 @@ static inline void
> gen_intermediate_code_internal(ARMCPU *cpu,
> > dc->condexec_cond = ARM_TBFLAG_CONDEXEC(tb->flags) >> 4;
> > #if !defined(CONFIG_USER_ONLY)
> > dc->user = (ARM_TBFLAG_PRIV(tb->flags) == 0);
> > + dc->ns = ARM_TBFLAG_NS(tb->flags);
> > #endif
> > dc->cpacr_fpen = ARM_TBFLAG_CPACR_FPEN(tb->flags);
> > dc->vfp_enabled = ARM_TBFLAG_VFPEN(tb->flags);
> > diff --git a/target-arm/translate.h b/target-arm/translate.h
> > index 31a0104..6e8620a 100644
> > --- a/target-arm/translate.h
> > +++ b/target-arm/translate.h
> > @@ -19,6 +19,7 @@ typedef struct DisasContext {
> > int bswap_code;
> > #if !defined(CONFIG_USER_ONLY)
> > int user;
> > + int ns;
> > #endif
>
> Please don't follow the way the "user" flag is done, it's
> weird and something I might eventually tidy up. Just have
> a "bool ns" in the DisasContext (not inside the #if), and
> set it to true if CONFIG_USER_ONLY is set. Then you
> don't need to indirect via an IS_NS() macro, you can just
> use s->ns directly.
>
> > bool cpacr_fpen; /* FP enabled via CPACR.FPEN */
> > bool vfp_enabled; /* FP enabled via FPSCR.EN */
> > --
>
> thanks
> -- PMM
>
