From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42620) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wwpbv-0005xl-Ca for qemu-devel@nongnu.org; Tue, 17 Jun 2014 05:23:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wwpbr-00075F-6I for qemu-devel@nongnu.org; Tue, 17 Jun 2014 05:23:23 -0400 Received: from mail-ig0-x230.google.com ([2607:f8b0:4001:c05::230]:50020) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wwpbq-000755-Vj for qemu-devel@nongnu.org; Tue, 17 Jun 2014 05:23:19 -0400 Received: by mail-ig0-f176.google.com with SMTP id a13so3928819igq.9 for ; Tue, 17 Jun 2014 02:23:18 -0700 (PDT) Date: Tue, 17 Jun 2014 11:15:06 +0200 From: "Edgar E. Iglesias" Message-ID: <20140617091506.GH10398@toto> References: <1402444514-19658-1-git-send-email-aggelerf@ethz.ch> <1402444514-19658-8-git-send-email-aggelerf@ethz.ch> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1402444514-19658-8-git-send-email-aggelerf@ethz.ch> Subject: Re: [Qemu-devel] [PATCH v3 07/32] target-arm: add non-secure Translation Block flag List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fabian Aggeler Cc: peter.maydell@linaro.org, peter.crosthwaite@xilinx.com, qemu-devel@nongnu.org, greg.bellows@linaro.org, serge.fdrv@gmail.com, christoffer.dall@linaro.org On Wed, Jun 11, 2014 at 01:54:49AM +0200, Fabian Aggeler wrote: > From: Sergey Fedorov > > This patch is based on idea found in patch at > git://github.com/jowinter/qemu-trustzone.git > f3d955c6c0ed8c46bc0eb10b634201032a651dd2 by > Johannes Winter . > > 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 > Signed-off-by: Fabian Aggeler > --- > 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 >