From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41203) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WzSn7-0007RW-AT for qemu-devel@nongnu.org; Tue, 24 Jun 2014 11:37:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WzSn1-0000CF-D0 for qemu-devel@nongnu.org; Tue, 24 Jun 2014 11:37:49 -0400 Received: from mail-qc0-f172.google.com ([209.85.216.172]:43625) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WzSn1-0000BZ-7A for qemu-devel@nongnu.org; Tue, 24 Jun 2014 11:37:43 -0400 Received: by mail-qc0-f172.google.com with SMTP id o8so446952qcw.3 for ; Tue, 24 Jun 2014 08:37:42 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <4F288BC8-B2D9-4944-A25D-32B56B60A968@ethz.ch> References: <1402444514-19658-1-git-send-email-aggelerf@ethz.ch> <1402444514-19658-16-git-send-email-aggelerf@ethz.ch> <4F288BC8-B2D9-4944-A25D-32B56B60A968@ethz.ch> Date: Tue, 24 Jun 2014 10:37:42 -0500 Message-ID: From: Greg Bellows Content-Type: multipart/alternative; boundary=001a113a6e868d2cf804fc96bddb Subject: Re: [Qemu-devel] [PATCH v3 15/32] target-arm: add NSACR register List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aggeler Fabian Cc: Peter Maydell , Peter Crosthwaite , QEMU Developers , Sergey Fedorov , "Edgar E. Iglesias" , Christoffer Dall --001a113a6e868d2cf804fc96bddb Content-Type: text/plain; charset=UTF-8 On 17 June 2014 02:41, Aggeler Fabian wrote: > > On 13 Jun 2014, at 20:27, Greg Bellows greg.bellows@linaro.org>> wrote: > > > > > On 10 June 2014 18:54, Fabian Aggeler aggelerf@ethz.ch>> wrote: > Implements NSACR register with corresponding read/write functions > for ARMv7 and ARMv8. > > Signed-off-by: Sergey Fedorov s.fedorov@samsung.com>> > Signed-off-by: Fabian Aggeler > > --- > target-arm/cpu.h | 6 +++++ > target-arm/helper.c | 68 > ++++++++++++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 73 insertions(+), 1 deletion(-) > > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > index 52e679f..bc9edaa 100644 > --- a/target-arm/cpu.h > +++ b/target-arm/cpu.h > @@ -182,6 +182,7 @@ typedef struct CPUARMState { > uint64_t c1_coproc; /* Coprocessor access register. */ > uint32_t c1_xscaleauxcr; /* XScale auxiliary control register. */ > uint32_t c1_scr; /* secure config register. */ > + uint32_t c1_nsacr; /* Non-secure access control register. */ > uint64_t ttbr0_el1; /* MMU translation table base 0. */ > uint64_t ttbr1_el1; /* MMU translation table base 1. */ > uint64_t c2_control; /* MMU translation table base control. */ > @@ -593,6 +594,11 @@ static inline void xpsr_write(CPUARMState *env, > uint32_t val, uint32_t mask) > #define SCR_RES1_MASK (3U << 4) > #define SCR_MASK (0x3fff & ~SCR_RES1_MASK) > > +#define NSACR_NSTRCDIS (1U << 20) > +#define NSACR_RFR (1U << 19) > +#define NSACR_NSASEDIS (1U << 15) > +#define NSACR_NSD32DIS (1U << 14) > + > /* Return the current FPSCR value. */ > uint32_t vfp_get_fpscr(CPUARMState *env); > void vfp_set_fpscr(CPUARMState *env, uint32_t val); > diff --git a/target-arm/helper.c b/target-arm/helper.c > index f6ff4aa..9671f9f 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -489,7 +489,19 @@ static void cpacr_write(CPUARMState *env, const > ARMCPRegInfo *ri, > /* VFP coprocessor: cp10 & cp11 [23:20] */ > mask |= (1 << 31) | (1 << 30) | (0xf << 20); > > - if (!arm_feature(env, ARM_FEATURE_NEON)) { > + if (arm_feature(env, ARM_FEATURE_NEON)) { > + /* NSACR can disable non-secure writes to > + * ASEDIS [31] or D32DIS [30] > + */ > + if (arm_feature(env, ARM_FEATURE_EL3) && > !arm_is_secure(env)) { > + if ((env->cp15.c1_nsacr & NSACR_NSASEDIS)) { > + mask &= ~(1 << 31); > + } > + if ((env->cp15.c1_nsacr & NSACR_NSD32DIS)) { > + mask &= ~(1 << 30); > + } > + } > + } else { > /* ASEDIS [31] bit is RAO/WI */ > value |= (1 << 31); > } > @@ -501,6 +513,7 @@ static void cpacr_write(CPUARMState *env, const > ARMCPRegInfo *ri, > !arm_feature(env, ARM_FEATURE_VFP3)) { > /* D32DIS [30] is RAO/WI if D16-31 are not implemented. */ > value |= (1 << 30); > + mask |= (1 << 30); > } > } > value &= mask; > @@ -2184,6 +2197,55 @@ static void scr_write(CPUARMState *env, const > ARMCPRegInfo *ri, uint64_t value) > raw_write(env, ri, value); > } > > +static void nsacr_write(CPUARMState *env, const ARMCPRegInfo *ri, > + uint64_t value) > +{ > + uint32_t mask = 0; > + > + /* Pre ARMv8 some bits are RAO or UNK/SBZP */ > + if (!arm_feature(env, ARM_FEATURE_V8)) { > + > + if (arm_feature(env, ARM_FEATURE_VFP)) { > + mask |= NSACR_NSASEDIS | NSACR_NSD32DIS; > + > + if (!arm_feature(env, ARM_FEATURE_NEON)) { > + /* NSASEDIS are RAO/WI */ > + value |= NSACR_NSASEDIS; > + } > + > + /* VFPv3 and upwards with NEON implement 32 double precision > + * registers (D0-D31). > + */ > + if (!arm_feature(env, ARM_FEATURE_NEON) || > + !arm_feature(env, ARM_FEATURE_VFP3)) { > + /* NSD32DIS is RAO/WI if D16-31 are not implemented. */ > + value |= NSACR_NSD32DIS; > + } > + } > + > + /* cpn bits [13:0] */ > + mask = 0x3fff; > + > + value &= mask; > + } > + > + raw_write(env, ri, value); > +} > + > +static uint64_t nsacr_read(CPUARMState *env, const ARMCPRegInfo *ri) > +{ > + uint64_t ret = raw_read(env, ri); > + > + if (arm_feature(env, ARM_FEATURE_V8)) { > + if (!arm_feature(env, ARM_FEATURE_EL3) || ( > + arm_el_is_aa64(env, 3) && !is_a64(env) && > + arm_current_pl(env) != 3)) { > + ret = 0x0000C00; > + } > > It appears we are missing a case where 0xc00 is returned because we check > for the non-existence of EL3. > > Right, this case is missing. > > Not sure what I was thinking at the time but the case I thought was missing is covered. > > + } > + return ret; > +} > + > > The ARMv8 spec suggests that if EL3 is aarch64 that a read or write of > this register occurs from secure EL1 when aarch32 it is trapped as an > exception to EL3. Is this omitted? > > It seems I skipped that line when reading the ARMv8 spec. Should be added > of course. > > I will add this as a separate commit because it is only a v8 concern. > > static const ARMCPRegInfo v8_el3_cp_reginfo[] = { > { .name = "ELR_EL3", .state = ARM_CP_STATE_AA64, > .type = ARM_CP_NO_MIGRATE, > @@ -2217,6 +2279,10 @@ static const ARMCPRegInfo security_cp_reginfo[] = { > { .name = "SCR", .cp = 15, .crn = 1, .crm = 1, .opc1 = 0, .opc2 = 0, > .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, > cp15.scr_el3), > .resetvalue = 0, }, > + { .name = "NSACR", .cp = 15, .crn = 1, .crm = 1, .opc1 = 0, .opc2 = 2, > + .access = PL3_RW | PL1_R, .resetvalue = 0, > + .writefn = nsacr_write, .readfn = nsacr_read, > + .fieldoffset = offsetof(CPUARMState, cp15.c1_nsacr) }, > REGINFO_SENTINEL > }; > > -- > 1.8.3.2 > > > > --001a113a6e868d2cf804fc96bddb Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable



On 17 June 2014 02:41, Aggeler Fabian <aggelerf@student.et= hz.ch> wrote:

On 13 Jun 2014, at 20:27, Greg Bellows <greg.bellows@linaro.org<mailto:greg.bellows@linaro.org>> wrote:




On 10 June 2014 18:54, Fabian Aggeler <aggelerf@ethz.ch<mailto:aggel= erf@ethz.ch>> wrote:
Implements NSACR register with corresponding read/write functions
for ARMv7 and ARMv8.

Signed-off-by: Sergey Fedorov <s.fedorov@samsung.com<mailto:s.fedorov@samsung.com>>
Signed-off-by: Fabian Aggeler <aggel= erf@ethz.ch<mailto:aggelerf@ethz= .ch>>
---
=C2=A0target-arm/cpu.h =C2=A0 =C2=A0| =C2=A06 +++++
=C2=A0target-arm/helper.c | 68 ++++++++++++++++++++++++++++++++++++++++++++= ++++++++-
=C2=A02 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 52e679f..bc9edaa 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -182,6 +182,7 @@ typedef struct CPUARMState {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0uint64_t c1_coproc; /* Coprocessor access= register. =C2=A0*/
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0uint32_t c1_xscaleauxcr; /* XScale auxili= ary control register. =C2=A0*/
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0uint32_t c1_scr; /* secure config registe= r. =C2=A0*/
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0uint32_t c1_nsacr; /* Non-secure access contro= l register. */
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0uint64_t ttbr0_el1; /* MMU translation ta= ble base 0. */
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0uint64_t ttbr1_el1; /* MMU translation ta= ble base 1. */
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0uint64_t c2_control; /* MMU translation t= able base control. =C2=A0*/
@@ -593,6 +594,11 @@ static inline void xpsr_write(CPUARMState *env, uint32= _t val, uint32_t mask)
=C2=A0#define SCR_RES1_MASK (3U << 4)
=C2=A0#define SCR_MASK =C2=A0 =C2=A0 =C2=A0(0x3fff & ~SCR_RES1_MASK)
+#define NSACR_NSTRCDIS (1U << 20)
+#define NSACR_RFR =C2=A0 =C2=A0 =C2=A0(1U << 19)
+#define NSACR_NSASEDIS (1U << 15)
+#define NSACR_NSD32DIS (1U << 14)
+
=C2=A0/* Return the current FPSCR value. =C2=A0*/
=C2=A0uint32_t vfp_get_fpscr(CPUARMState *env);
=C2=A0void vfp_set_fpscr(CPUARMState *env, uint32_t val);
diff --git a/target-arm/helper.c b/target-arm/helper.c
index f6ff4aa..9671f9f 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -489,7 +489,19 @@ static void cpacr_write(CPUARMState *env, const ARMCPR= egInfo *ri,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* VFP coprocessor: cp10 &a= mp; cp11 [23:20] */
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0mask |=3D (1 << 31) |= (1 << 30) | (0xf << 20);

- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (!arm_feature(env, ARM_FEATUR= E_NEON)) {
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (arm_feature(env, ARM_FEATURE= _NEON)) {
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* NSACR can disab= le non-secure writes to
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 * ASEDIS [31] or = D32DIS [30]
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 */
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (arm_feature(en= v, ARM_FEATURE_EL3) && !arm_is_secure(env)) {
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (= (env->cp15.c1_nsacr & NSACR_NSASEDIS)) {
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0mask &=3D ~(1 << 31);
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0} + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (= (env->cp15.c1_nsacr & NSACR_NSD32DIS)) {
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0mask &=3D ~(1 << 30);
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0} + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0} else {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* ASEDIS [31= ] bit is RAO/WI */
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0value |=3D (1= << 31);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
@@ -501,6 +513,7 @@ static void cpacr_write(CPUARMState *env, const ARMCPRe= gInfo *ri,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0!arm_feature(env, ARM_FEATURE_VFP3)) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* D32DIS [30= ] is RAO/WI if D16-31 are not implemented. */
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0value |=3D (1= << 30);
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0mask |=3D (1 <&= lt; 30);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0value &=3D mask;
@@ -2184,6 +2197,55 @@ static void scr_write(CPUARMState *env, const ARMCPR= egInfo *ri, uint64_t value)
=C2=A0 =C2=A0 =C2=A0raw_write(env, ri, value);
=C2=A0}

+static void nsacr_write(CPUARMState *env, const ARMCPRegInfo *ri,
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0uint64_t value)
+{
+ =C2=A0 =C2=A0uint32_t mask =3D 0;
+
+ =C2=A0 =C2=A0/* Pre ARMv8 some bits are RAO or UNK/SBZP */
+ =C2=A0 =C2=A0if (!arm_feature(env, ARM_FEATURE_V8)) {
+
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0if (arm_feature(env, ARM_FEATURE_VFP)) {
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0mask |=3D NSACR_NSASEDIS | NSACR= _NSD32DIS;
+
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (!arm_feature(env, ARM_FEATUR= E_NEON)) {
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* NSASEDIS are RA= O/WI */
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0value |=3D NSACR_N= SASEDIS;
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
+
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* VFPv3 and upwards with NEON i= mplement 32 double precision
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 * registers (D0-D31).
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 */
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (!arm_feature(env, ARM_FEATUR= E_NEON) ||
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0!arm= _feature(env, ARM_FEATURE_VFP3)) {
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* NSD32DIS is RAO= /WI if D16-31 are not implemented. */
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0value |=3D NSACR_N= SD32DIS;
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0}
+
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0/* cpn bits [13:0] */
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0mask =3D 0x3fff;
+
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0value &=3D mask;
+ =C2=A0 =C2=A0}
+
+ =C2=A0 =C2=A0raw_write(env, ri, value);
+}
+
+static uint64_t nsacr_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+ =C2=A0 =C2=A0uint64_t ret =3D raw_read(env, ri);
+
+ =C2=A0 =C2=A0if (arm_feature(env, ARM_FEATURE_V8)) {
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0if (!arm_feature(env, ARM_FEATURE_EL3) || ( + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0arm_el_is_aa64(env= , 3) && !is_a64(env) &&
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0arm_current_pl(env= ) !=3D 3)) {
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ret =3D 0x0000C00;
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0}

It appears we are missing a case where 0xc00 is returned because we check f= or the non-existence of EL3.

Right, this case is missing.


Not sure what I = was thinking at the time but the case I thought was missing is covered.
=C2=A0

+ =C2=A0 =C2=A0}
+ =C2=A0 =C2=A0return ret;
+}
+

The ARMv8 spec suggests that if EL3 is aarch64 that a read or write of this= register occurs from secure EL1 when aarch32 it is trapped as an exception= to EL3. =C2=A0Is this omitted?

It seems I skipped that line when reading the ARMv8 spec. Should be a= dded of course.

<= br>
I will add this as a separate commit because it is only a v8 = concern.
=C2=A0

=C2=A0static const ARMCPRegInfo v8_el3_cp_reginfo[] =3D {
=C2=A0 =C2=A0 =C2=A0{ .name =3D "ELR_EL3", .state =3D ARM_CP_STAT= E_AA64,
=C2=A0 =C2=A0 =C2=A0 =C2=A0.type =3D ARM_CP_NO_MIGRATE,
@@ -2217,6 +2279,10 @@ static const ARMCPRegInfo security_cp_reginfo[] =3D = {
=C2=A0 =C2=A0 =C2=A0{ .name =3D "SCR", .cp =3D 15, .crn =3D 1, .c= rm =3D 1, .opc1 =3D 0, .opc2 =3D 0,
=C2=A0 =C2=A0 =C2=A0 =C2=A0.access =3D PL3_RW, .fieldoffset =3D offsetof(CP= UARMState, cp15.scr_el3),
=C2=A0 =C2=A0 =C2=A0 =C2=A0.resetvalue =3D 0, },
+ =C2=A0 =C2=A0{ .name =3D "NSACR", .cp =3D 15, .crn =3D 1, .crm = =3D 1, .opc1 =3D 0, .opc2 =3D 2,
+ =C2=A0 =C2=A0 =C2=A0.access =3D PL3_RW | PL1_R, .resetvalue =3D 0,
+ =C2=A0 =C2=A0 =C2=A0.writefn =3D nsacr_write, .readfn =3D nsacr_read,
+ =C2=A0 =C2=A0 =C2=A0.fieldoffset =3D offsetof(CPUARMState, cp15.c1_nsacr)= },
=C2=A0 =C2=A0 =C2=A0REGINFO_SENTINEL
=C2=A0};

--
1.8.3.2




--001a113a6e868d2cf804fc96bddb--