diff mbox

[v3,07/32] target-arm: add non-secure Translation Block flag

Message ID 1402444514-19658-8-git-send-email-aggelerf@ethz.ch
State New
Headers show

Commit Message

Fabian Aggeler June 10, 2014, 11:54 p.m. UTC 
From: Sergey Fedorov <s.fedorov@samsung.com>

This patch is based on idea found in patch at
git://github.com/jowinter/qemu-trustzone.git
f3d955c6c0ed8c46bc0eb10b634201032a651dd2 by
Johannes Winter <johannes.winter@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.fedorov@samsung.com>
Signed-off-by: Fabian Aggeler <aggelerf@ethz.ch>
---
 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(+)

Comments

Edgar E. Iglesias June 17, 2014, 9:15 a.m. UTC | #1 
On Wed, Jun 11, 2014 at 01:54:49AM +0200, Fabian Aggeler wrote:
> From: Sergey Fedorov <s.fedorov@samsung.com>
> 
> This patch is based on idea found in patch at
> git://github.com/jowinter/qemu-trustzone.git
> f3d955c6c0ed8c46bc0eb10b634201032a651dd2 by
> Johannes Winter <johannes.winter@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.

Hi, I think the patch looks OK but I'm unsure if it brings any benefits
unless we add separate TLBs for S and NS.

I noticed that TTBR0 gets banked in the series, but are changes to
SCR.NS flushing the TLBs? I might have missed that from the patches.
You'll need it unless we add separate S/NS TLBs.

Considering that changes to SCR.NS will flush the TLBs, the
use of a per TB ns flag is limited, unless I am missing something...

Cheers,
Edgar


> 
> Signed-off-by: Sergey Fedorov <s.fedorov@samsung.com>
> Signed-off-by: Fabian Aggeler <aggelerf@ethz.ch>
> ---
>  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 14007a9..661bfbe 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1275,6 +1275,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
> @@ -1305,6 +1307,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)
> @@ -1318,6 +1322,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];
> @@ -1334,6 +1341,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;
> +        }
>          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 8cb326f..16b706c 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -10878,6 +10878,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
>      bool cpacr_fpen; /* FP enabled via CPACR.FPEN */
>      bool vfp_enabled; /* FP enabled via FPSCR.EN */
> -- 
> 1.8.3.2
>
Sergey Fedorov June 17, 2014, 10:07 a.m. UTC | #2 
On 17.06.2014 13:15, Edgar E. Iglesias wrote:
> Hi, I think the patch looks OK but I'm unsure if it brings any benefits
> unless we add separate TLBs for S and NS.
>
> I noticed that TTBR0 gets banked in the series, but are changes to
> SCR.NS flushing the TLBs? I might have missed that from the patches.
> You'll need it unless we add separate S/NS TLBs.
>
> Considering that changes to SCR.NS will flush the TLBs, the
> use of a per TB ns flag is limited, unless I am missing something...

Hi Edgar,

This seems to be used in patch 19/32.

// Sergey

>
> Cheers,
> Edgar
Edgar E. Iglesias June 19, 2014, 5:30 a.m. UTC | #3 
On Tue, Jun 17, 2014 at 02:07:11PM +0400, Sergey Fedorov wrote:
> On 17.06.2014 13:15, Edgar E. Iglesias wrote:
> > Hi, I think the patch looks OK but I'm unsure if it brings any benefits
> > unless we add separate TLBs for S and NS.
> >
> > I noticed that TTBR0 gets banked in the series, but are changes to
> > SCR.NS flushing the TLBs? I might have missed that from the patches.
> > You'll need it unless we add separate S/NS TLBs.
> >
> > Considering that changes to SCR.NS will flush the TLBs, the
> > use of a per TB ns flag is limited, unless I am missing something...
> 
> Hi Edgar,
> 
> This seems to be used in patch 19/32.

Yes, I actually meant limited use as in having the NS flag in translation
time brings limited performance win or none.

But looking more carefuly, the way we handle cp regs requires the ns bit
at translation time to support direct loads of cpregs (without calling
out to tcg helpers). That is probably enough to motivate the tb flag.

Cheers,
Edgar

> 
> // Sergey
> 
> >
> > Cheers,
> > Edgar
>
Edgar E. Iglesias June 25, 2014, 4:15 a.m. UTC | #4 
On Wed, Jun 11, 2014 at 01:54:49AM +0200, Fabian Aggeler wrote:
> From: Sergey Fedorov <s.fedorov@samsung.com>
> 
> This patch is based on idea found in patch at
> git://github.com/jowinter/qemu-trustzone.git
> f3d955c6c0ed8c46bc0eb10b634201032a651dd2 by
> Johannes Winter <johannes.winter@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.
>


Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

 
> Signed-off-by: Sergey Fedorov <s.fedorov@samsung.com>
> Signed-off-by: Fabian Aggeler <aggelerf@ethz.ch>
> ---
>  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 14007a9..661bfbe 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1275,6 +1275,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
> @@ -1305,6 +1307,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)
> @@ -1318,6 +1322,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];
> @@ -1334,6 +1341,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;
> +        }
>          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 8cb326f..16b706c 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -10878,6 +10878,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
>      bool cpacr_fpen; /* FP enabled via CPACR.FPEN */
>      bool vfp_enabled; /* FP enabled via FPSCR.EN */
> -- 
> 1.8.3.2
>
diff mbox

Patch

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 14007a9..661bfbe 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1275,6 +1275,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
@@ -1305,6 +1307,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)
@@ -1318,6 +1322,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];
@@ -1334,6 +1341,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;
+        }
         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 8cb326f..16b706c 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -10878,6 +10878,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
     bool cpacr_fpen; /* FP enabled via CPACR.FPEN */
     bool vfp_enabled; /* FP enabled via FPSCR.EN */