diff mbox

[v8,13/27] target-arm: add SCTLR_EL3 and make SCTLR banked

Message ID 1414704538-17103-14-git-send-email-greg.bellows@linaro.org
State New
Headers show

Commit Message

Greg Bellows Oct. 30, 2014, 9:28 p.m. UTC
From: Fabian Aggeler <aggelerf@ethz.ch>

Implements SCTLR_EL3 and uses secure/non-secure instance when
needed.

Signed-off-by: Fabian Aggeler <aggelerf@ethz.ch>
Signed-off-by: Greg Bellows <greg.bellows@linaro.org>

---

v5 -> v6
- Changed _el field variants to be array based
- Consolidate SCTLR and SCTLR_EL1 reginfo entries
---
 hw/arm/pxa2xx.c        |  2 +-
 target-arm/cpu.c       |  5 ++--
 target-arm/cpu.h       | 10 ++++++-
 target-arm/helper.c    | 72 +++++++++++++++++++++++++++++---------------------
 target-arm/op_helper.c |  2 +-
 5 files changed, 56 insertions(+), 35 deletions(-)

Comments

Greg Bellows Oct. 31, 2014, 9:51 p.m. UTC | #1
Not sure of the exact reasoning as I inherited the change.  However, when I
went through this code before I took it that the change was needed to
filter out the case where SCTLR may be either of the ARMv8 variants
(SCTLR_EL1 or SCTLR_EL3) as neither of them have a SCTLR_V bits.  In fact,
looking quickly through the ARMv8 ARM, I don't see any mention of hivec
support for AArch64.

I think the more appropriate check in this case is to check whether the
current EL is 32-bit instead of ARMv8.   Made this change in v9 along with
a comment.

I'll leave your review-by off until you okay this.

Greg


On 31 October 2014 09:07, Peter Maydell <peter.maydell@linaro.org> wrote:

> On 30 October 2014 21:28, Greg Bellows <greg.bellows@linaro.org> wrote:
> > From: Fabian Aggeler <aggelerf@ethz.ch>
> >
> > Implements SCTLR_EL3 and uses secure/non-secure instance when
> > needed.
> >
> > Signed-off-by: Fabian Aggeler <aggelerf@ethz.ch>
> > Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
>
> > diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> > index e0b82a6..18f4726 100644
> > --- a/target-arm/cpu.c
> > +++ b/target-arm/cpu.c
> > @@ -109,7 +109,7 @@ static void arm_cpu_reset(CPUState *s)
> >  #if defined(CONFIG_USER_ONLY)
> >          env->pstate = PSTATE_MODE_EL0t;
> >          /* Userspace expects access to DC ZVA, CTL_EL0 and the cache
> ops */
> > -        env->cp15.c1_sys |= SCTLR_UCT | SCTLR_UCI | SCTLR_DZE;
> > +        env->cp15.sctlr_el[1] |= SCTLR_UCT | SCTLR_UCI | SCTLR_DZE;
> >          /* and to the FP/Neon instructions */
> >          env->cp15.c1_coproc = deposit64(env->cp15.c1_coproc, 20, 2, 3);
> >  #else
> > @@ -167,7 +167,8 @@ static void arm_cpu_reset(CPUState *s)
> >          env->thumb = initial_pc & 1;
> >      }
> >
> > -    if (env->cp15.c1_sys & SCTLR_V) {
> > +    if (!arm_feature(env, ARM_FEATURE_V8)
> > +            && (A32_BANKED_CURRENT_REG_GET(env, sctlr) & SCTLR_V)) {
> >          env->regs[15] = 0xFFFF0000;
>
> Why has this condition had an "if not v8" added to it? The v8
> spec doesn't drop support for hivecs as far as I can tell...
>
> Patch looks good otherwise.
>
> -- PMM
>
Peter Maydell Oct. 31, 2014, 11:26 p.m. UTC | #2
On 31 October 2014 21:51, Greg Bellows <greg.bellows@linaro.org> wrote:
> Not sure of the exact reasoning as I inherited the change.  However, when I
> went through this code before I took it that the change was needed to filter
> out the case where SCTLR may be either of the ARMv8 variants (SCTLR_EL1 or
> SCTLR_EL3) as neither of them have a SCTLR_V bits.  In fact, looking quickly
> through the ARMv8 ARM, I don't see any mention of hivec support for AArch64.
>
> I think the more appropriate check in this case is to check whether the
> current EL is 32-bit instead of ARMv8.   Made this change in v9 along with a
> comment.

You don't need any extra check, for two reasons:
 (1) if our EL out of reset is 64 bit then this is SCTLR_EL3 or
 SCTLR_EL1, and in either case the V bit is RES0 so it will always
 be zero
 (2) setting env->regs[15] is harmless anyway because the AArch64
 PC is stored elsewhere

thanks
-- PMM
diff mbox

Patch

diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
index 693dfec..11d51af 100644
--- a/hw/arm/pxa2xx.c
+++ b/hw/arm/pxa2xx.c
@@ -273,7 +273,7 @@  static void pxa2xx_pwrmode_write(CPUARMState *env, const ARMCPRegInfo *ri,
     case 3:
         s->cpu->env.uncached_cpsr = ARM_CPU_MODE_SVC;
         s->cpu->env.daif = PSTATE_A | PSTATE_F | PSTATE_I;
-        s->cpu->env.cp15.c1_sys = 0;
+        s->cpu->env.cp15.sctlr_ns = 0;
         s->cpu->env.cp15.c1_coproc = 0;
         s->cpu->env.cp15.ttbr0_el1 = 0;
         s->cpu->env.cp15.c3 = 0;
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index e0b82a6..18f4726 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -109,7 +109,7 @@  static void arm_cpu_reset(CPUState *s)
 #if defined(CONFIG_USER_ONLY)
         env->pstate = PSTATE_MODE_EL0t;
         /* Userspace expects access to DC ZVA, CTL_EL0 and the cache ops */
-        env->cp15.c1_sys |= SCTLR_UCT | SCTLR_UCI | SCTLR_DZE;
+        env->cp15.sctlr_el[1] |= SCTLR_UCT | SCTLR_UCI | SCTLR_DZE;
         /* and to the FP/Neon instructions */
         env->cp15.c1_coproc = deposit64(env->cp15.c1_coproc, 20, 2, 3);
 #else
@@ -167,7 +167,8 @@  static void arm_cpu_reset(CPUState *s)
         env->thumb = initial_pc & 1;
     }
 
-    if (env->cp15.c1_sys & SCTLR_V) {
+    if (!arm_feature(env, ARM_FEATURE_V8)
+            && (A32_BANKED_CURRENT_REG_GET(env, sctlr) & SCTLR_V)) {
         env->regs[15] = 0xFFFF0000;
     }
 
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 596bfae..8acc2b0 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -178,7 +178,15 @@  typedef struct CPUARMState {
     struct {
         uint32_t c0_cpuid;
         uint64_t c0_cssel; /* Cache size selection.  */
-        uint64_t c1_sys; /* System control register.  */
+        union { /* System control register. */
+            struct {
+                uint64_t _unused_sctlr;
+                uint64_t sctlr_ns;
+                uint64_t hsctlr;
+                uint64_t sctlr_s;
+            };
+            uint64_t sctlr_el[4];
+        };
         uint64_t c1_coproc; /* Coprocessor access register.  */
         uint32_t c1_xscaleauxcr; /* XScale auxiliary control register.  */
         uint64_t sder; /* Secure debug enable register. */
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 3c56d8f..466459b 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -1908,7 +1908,7 @@  static void aa64_fpsr_write(CPUARMState *env, const ARMCPRegInfo *ri,
 
 static CPAccessResult aa64_daif_access(CPUARMState *env, const ARMCPRegInfo *ri)
 {
-    if (arm_current_el(env) == 0 && !(env->cp15.c1_sys & SCTLR_UMA)) {
+    if (arm_current_el(env) == 0 && !(env->cp15.sctlr_el[1] & SCTLR_UMA)) {
         return CP_ACCESS_TRAP;
     }
     return CP_ACCESS_OK;
@@ -1926,7 +1926,7 @@  static CPAccessResult aa64_cacheop_access(CPUARMState *env,
     /* Cache invalidate/clean: NOP, but EL0 must UNDEF unless
      * SCTLR_EL1.UCI is set.
      */
-    if (arm_current_el(env) == 0 && !(env->cp15.c1_sys & SCTLR_UCI)) {
+    if (arm_current_el(env) == 0 && !(env->cp15.sctlr_el[1] & SCTLR_UCI)) {
         return CP_ACCESS_TRAP;
     }
     return CP_ACCESS_OK;
@@ -2003,7 +2003,7 @@  static CPAccessResult aa64_zva_access(CPUARMState *env, const ARMCPRegInfo *ri)
     /* We don't implement EL2, so the only control on DC ZVA is the
      * bit in the SCTLR which can prohibit access for EL0.
      */
-    if (arm_current_el(env) == 0 && !(env->cp15.c1_sys & SCTLR_DZE)) {
+    if (arm_current_el(env) == 0 && !(env->cp15.sctlr_el[1] & SCTLR_DZE)) {
         return CP_ACCESS_TRAP;
     }
     return CP_ACCESS_OK;
@@ -2042,6 +2042,24 @@  static void spsel_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t val)
     update_spsel(env, val);
 }
 
+static void sctlr_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                        uint64_t value)
+{
+    ARMCPU *cpu = arm_env_get_cpu(env);
+
+    if (raw_read(env, ri) == value) {
+        /* Skip the TLB flush if nothing actually changed; Linux likes
+         * to do a lot of pointless SCTLR writes.
+         */
+        return;
+    }
+
+    raw_write(env, ri, value);
+    /* ??? Lots of these bits are not implemented.  */
+    /* This may enable/disable the MMU, so do a TLB flush.  */
+    tlb_flush(CPU(cpu), 1);
+}
+
 static const ARMCPRegInfo v8_cp_reginfo[] = {
     /* Minimal set of EL0-visible registers. This will need to be expanded
      * significantly for system emulation of AArch64 CPUs.
@@ -2311,6 +2329,10 @@  static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
 };
 
 static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
+    { .name = "SCTLR_EL3", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .crn = 1, .crm = 0, .opc1 = 6, .opc2 = 0,
+      .access = PL3_RW, .raw_writefn = raw_write, .writefn = sctlr_write,
+      .fieldoffset = offsetof(CPUARMState, cp15.sctlr_el[3]) },
     { .name = "ELR_EL3", .state = ARM_CP_STATE_AA64,
       .type = ARM_CP_NO_MIGRATE,
       .opc0 = 3, .opc1 = 6, .crn = 4, .crm = 0, .opc2 = 1,
@@ -2361,30 +2383,12 @@  static const ARMCPRegInfo el3_cp_reginfo[] = {
     REGINFO_SENTINEL
 };
 
-static void sctlr_write(CPUARMState *env, const ARMCPRegInfo *ri,
-                        uint64_t value)
-{
-    ARMCPU *cpu = arm_env_get_cpu(env);
-
-    if (raw_read(env, ri) == value) {
-        /* Skip the TLB flush if nothing actually changed; Linux likes
-         * to do a lot of pointless SCTLR writes.
-         */
-        return;
-    }
-
-    raw_write(env, ri, value);
-    /* ??? Lots of these bits are not implemented.  */
-    /* This may enable/disable the MMU, so do a TLB flush.  */
-    tlb_flush(CPU(cpu), 1);
-}
-
 static CPAccessResult ctr_el0_access(CPUARMState *env, const ARMCPRegInfo *ri)
 {
     /* Only accessible in EL0 if SCTLR.UCT is set (and only in AArch64,
      * but the AArch32 CTR has its own reginfo struct)
      */
-    if (arm_current_el(env) == 0 && !(env->cp15.c1_sys & SCTLR_UCT)) {
+    if (arm_current_el(env) == 0 && !(env->cp15.sctlr_el[1] & SCTLR_UCT)) {
         return CP_ACCESS_TRAP;
     }
     return CP_ACCESS_OK;
@@ -3181,8 +3185,10 @@  void register_cp_regs_for_features(ARMCPU *cpu)
     {
         ARMCPRegInfo sctlr = {
             .name = "SCTLR", .state = ARM_CP_STATE_BOTH,
-            .opc0 = 3, .crn = 1, .crm = 0, .opc1 = 0, .opc2 = 0,
-            .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.c1_sys),
+            .cp = 15, .crn = 1, .crm = 0, .opc1 = 0, .opc2 = 0,
+            .access = PL1_RW,
+            .bank_fieldoffsets = { offsetof(CPUARMState, cp15.sctlr_s),
+                                   offsetof(CPUARMState, cp15.sctlr_ns) },
             .writefn = sctlr_write, .resetvalue = cpu->reset_sctlr,
             .raw_writefn = raw_write,
         };
@@ -4271,7 +4277,7 @@  void arm_cpu_do_interrupt(CPUState *cs)
 
     if (new_mode == ARM_CPU_MODE_MON) {
         addr += env->cp15.mvbar;
-    } else if (env->cp15.c1_sys & SCTLR_V) {
+    } else if (A32_BANKED_CURRENT_REG_GET(env, sctlr) & SCTLR_V) {
         /* High vectors. When enabled, base address cannot be remapped. */
         addr += 0xffff0000;
     } else {
@@ -4301,7 +4307,7 @@  void arm_cpu_do_interrupt(CPUState *cs)
     /* this is a lie, as the was no c1_sys on V4T/V5, but who cares
      * and we should just guard the thumb mode on V4 */
     if (arm_feature(env, ARM_FEATURE_V4T)) {
-        env->thumb = (env->cp15.c1_sys & SCTLR_TE) != 0;
+        env->thumb = (A32_BANKED_CURRENT_REG_GET(env, sctlr) & SCTLR_TE) != 0;
     }
     env->regs[14] = env->regs[15] + offset;
     env->regs[15] = addr;
@@ -4332,7 +4338,7 @@  static inline int check_ap(CPUARMState *env, int ap, int domain_prot,
       }
       if (access_type == 1)
           return 0;
-      switch (env->cp15.c1_sys & (SCTLR_S | SCTLR_R)) {
+      switch (A32_BANKED_CURRENT_REG_GET(env, sctlr) & (SCTLR_S | SCTLR_R)) {
       case SCTLR_S:
           return is_user ? 0 : PAGE_READ;
       case SCTLR_R:
@@ -4581,7 +4587,8 @@  static int get_phys_addr_v6(CPUARMState *env, uint32_t address, int access_type,
             goto do_fault;
 
         /* The simplified model uses AP[0] as an access control bit.  */
-        if ((env->cp15.c1_sys & SCTLR_AFE) && (ap & 1) == 0) {
+        if ((A32_BANKED_CURRENT_REG_GET(env, sctlr) & SCTLR_AFE)
+                && (ap & 1) == 0) {
             /* Access flag fault.  */
             code = (code == 15) ? 6 : 3;
             goto do_fault;
@@ -4912,11 +4919,16 @@  static inline int get_phys_addr(CPUARMState *env, target_ulong address,
                                 hwaddr *phys_ptr, int *prot,
                                 target_ulong *page_size)
 {
+    /* This is not entirely correct as get_phys_addr() can also be called
+     * from ats_write() for an address translation of a specific regime.
+     */
+    uint32_t sctlr = A32_BANKED_CURRENT_REG_GET(env, sctlr);
+
     /* Fast Context Switch Extension.  */
     if (address < 0x02000000)
         address += env->cp15.c13_fcse;
 
-    if ((env->cp15.c1_sys & SCTLR_M) == 0) {
+    if ((sctlr & SCTLR_M) == 0) {
         /* MMU/MPU disabled.  */
         *phys_ptr = address;
         *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
@@ -4929,7 +4941,7 @@  static inline int get_phys_addr(CPUARMState *env, target_ulong address,
     } else if (extended_addresses_enabled(env)) {
         return get_phys_addr_lpae(env, address, access_type, is_user, phys_ptr,
                                   prot, page_size);
-    } else if (env->cp15.c1_sys & SCTLR_XP) {
+    } else if (sctlr & SCTLR_XP) {
         return get_phys_addr_v6(env, address, access_type, is_user, phys_ptr,
                                 prot, page_size);
     } else {
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 62012c3..a8dea5a 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -361,7 +361,7 @@  void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
      * Note that SPSel is never OK from EL0; we rely on handle_msr_i()
      * to catch that case at translate time.
      */
-    if (arm_current_el(env) == 0 && !(env->cp15.c1_sys & SCTLR_UMA)) {
+    if (arm_current_el(env) == 0 && !(env->cp15.sctlr_el[1] & SCTLR_UMA)) {
         raise_exception(env, EXCP_UDEF);
     }