qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v4 07/33] target-arm: add non-secure Translation


From: Greg Bellows
Subject: Re: [Qemu-devel] [PATCH v4 07/33] target-arm: add non-secure Translation Block flag
Date: Tue, 2 Sep 2014 11:43:15 -0500

Thanks Peter.  I'll fix these in v5.


On 2 September 2014 11:11, Peter Maydell <address@hidden> wrote:
On 1 July 2014 00:09,  <address@hidden> wrote:
> From: Sergey Fedorov <address@hidden>
>
> This patch is based on idea found in patch at
> git://github.com/jowinter/qemu-trustzone.git
> f3d955c6c0ed8c46bc0eb10b634201032a651dd2 by
> Johannes Winter <address@hidden>.
>
> 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 <address@hidden>
> Signed-off-by: Fabian Aggeler <address@hidden>
> Signed-off-by: Greg Bellows <address@hidden>
> ---
>  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


reply via email to

[Prev in Thread] Current Thread [Next in Thread]