diff mbox

[14/20] target/arm: Make MPU_RNR register banked for v8M

Message ID 1503414539-28762-15-git-send-email-peter.maydell@linaro.org
State New
Headers show

Commit Message

Peter Maydell Aug. 22, 2017, 3:08 p.m. UTC 
Make the MPU_RNR register banked if v8M security extensions are
enabled.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/cpu.h      |  2 +-
 hw/intc/armv7m_nvic.c | 18 +++++++++---------
 target/arm/cpu.c      |  3 ++-
 target/arm/helper.c   |  6 +++---
 target/arm/machine.c  | 13 +++++++++++--
 5 files changed, 26 insertions(+), 16 deletions(-)

Comments

Richard Henderson Aug. 29, 2017, 4:05 p.m. UTC | #1 
On 08/22/2017 08:08 AM, Peter Maydell wrote:
> +        env->pmsav7.rnr[M_REG_NS] = 0;
> +        env->pmsav7.rnr[M_REG_S] = 0;
>          memset(env->pmsav8.mair0, 0, sizeof(env->pmsav8.mair0));
>          memset(env->pmsav8.mair1, 0, sizeof(env->pmsav8.mair1));

Why some memset and some by-element?

Otherwise,

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~
Peter Maydell Aug. 29, 2017, 4:06 p.m. UTC | #2 
On 29 August 2017 at 17:05, Richard Henderson
<richard.henderson@linaro.org> wrote:
> On 08/22/2017 08:08 AM, Peter Maydell wrote:
>> +        env->pmsav7.rnr[M_REG_NS] = 0;
>> +        env->pmsav7.rnr[M_REG_S] = 0;
>>          memset(env->pmsav8.mair0, 0, sizeof(env->pmsav8.mair0));
>>          memset(env->pmsav8.mair1, 0, sizeof(env->pmsav8.mair1));
>
> Why some memset and some by-element?

Roughly, memset where I would otherwise have to write a
for loop (ie where there's more than 2 elements).

thanks
-- PMM
Richard Henderson Aug. 29, 2017, 4:09 p.m. UTC | #3 
On 08/29/2017 09:06 AM, Peter Maydell wrote:
> On 29 August 2017 at 17:05, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>> On 08/22/2017 08:08 AM, Peter Maydell wrote:
>>> +        env->pmsav7.rnr[M_REG_NS] = 0;
>>> +        env->pmsav7.rnr[M_REG_S] = 0;
>>>          memset(env->pmsav8.mair0, 0, sizeof(env->pmsav8.mair0));
>>>          memset(env->pmsav8.mair1, 0, sizeof(env->pmsav8.mair1));
>>
>> Why some memset and some by-element?
> 
> Roughly, memset where I would otherwise have to write a
> for loop (ie where there's more than 2 elements).

There's only 2 elements of mair0 and mair1...


r~
Peter Maydell Sept. 5, 2017, 4:41 p.m. UTC | #4 
On 29 August 2017 at 17:09, Richard Henderson
<richard.henderson@linaro.org> wrote:
> On 08/29/2017 09:06 AM, Peter Maydell wrote:
>> On 29 August 2017 at 17:05, Richard Henderson
>> <richard.henderson@linaro.org> wrote:
>>> On 08/22/2017 08:08 AM, Peter Maydell wrote:
>>>> +        env->pmsav7.rnr[M_REG_NS] = 0;
>>>> +        env->pmsav7.rnr[M_REG_S] = 0;
>>>>          memset(env->pmsav8.mair0, 0, sizeof(env->pmsav8.mair0));
>>>>          memset(env->pmsav8.mair1, 0, sizeof(env->pmsav8.mair1));
>>>
>>> Why some memset and some by-element?
>>
>> Roughly, memset where I would otherwise have to write a
>> for loop (ie where there's more than 2 elements).
>
> There's only 2 elements of mair0 and mair1...

So there are. Let's go with assignments for mair0/mair1 then.

thanks
-- PMM
diff mbox

Patch

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 12fa95e..43d36d6 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -533,7 +533,7 @@  typedef struct CPUARMState {
         uint32_t *drbar;
         uint32_t *drsr;
         uint32_t *dracr;
-        uint32_t rnr;
+        uint32_t rnr[2];
     } pmsav7;
 
     /* PMSAv8 MPU */
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 9ced7af..c3c214c 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -543,13 +543,13 @@  static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs)
     case 0xd94: /* MPU_CTRL */
         return cpu->env.v7m.mpu_ctrl;
     case 0xd98: /* MPU_RNR */
-        return cpu->env.pmsav7.rnr;
+        return cpu->env.pmsav7.rnr[attrs.secure];
     case 0xd9c: /* MPU_RBAR */
     case 0xda4: /* MPU_RBAR_A1 */
     case 0xdac: /* MPU_RBAR_A2 */
     case 0xdb4: /* MPU_RBAR_A3 */
     {
-        int region = cpu->env.pmsav7.rnr;
+        int region = cpu->env.pmsav7.rnr[attrs.secure];
 
         if (arm_feature(&cpu->env, ARM_FEATURE_V8)) {
             /* PMSAv8M handling of the aliases is different from v7M:
@@ -577,7 +577,7 @@  static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs)
     case 0xdb0: /* MPU_RASR_A2 (v7M), MPU_RLAR_A2 (v8M) */
     case 0xdb8: /* MPU_RASR_A3 (v7M), MPU_RLAR_A3 (v8M) */
     {
-        int region = cpu->env.pmsav7.rnr;
+        int region = cpu->env.pmsav7.rnr[attrs.secure];
 
         if (arm_feature(&cpu->env, ARM_FEATURE_V8)) {
             /* PMSAv8M handling of the aliases is different from v7M:
@@ -731,7 +731,7 @@  static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value,
                           PRIu32 "/%" PRIu32 "\n",
                           value, cpu->pmsav7_dregion);
         } else {
-            cpu->env.pmsav7.rnr = value;
+            cpu->env.pmsav7.rnr[attrs.secure] = value;
         }
         break;
     case 0xd9c: /* MPU_RBAR */
@@ -749,7 +749,7 @@  static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value,
              */
             int aliasno = (offset - 0xd9c) / 8; /* 0..3 */
 
-            region = cpu->env.pmsav7.rnr;
+            region = cpu->env.pmsav7.rnr[attrs.secure];
             if (aliasno) {
                 region = deposit32(region, 0, 2, aliasno);
             }
@@ -772,9 +772,9 @@  static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value,
                               region, cpu->pmsav7_dregion);
                 return;
             }
-            cpu->env.pmsav7.rnr = region;
+            cpu->env.pmsav7.rnr[attrs.secure] = region;
         } else {
-            region = cpu->env.pmsav7.rnr;
+            region = cpu->env.pmsav7.rnr[attrs.secure];
         }
 
         if (region >= cpu->pmsav7_dregion) {
@@ -790,7 +790,7 @@  static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value,
     case 0xdb0: /* MPU_RASR_A2 (v7M), MPU_RLAR_A2 (v8M) */
     case 0xdb8: /* MPU_RASR_A3 (v7M), MPU_RLAR_A3 (v8M) */
     {
-        int region = cpu->env.pmsav7.rnr;
+        int region = cpu->env.pmsav7.rnr[attrs.secure];
 
         if (arm_feature(&cpu->env, ARM_FEATURE_V8)) {
             /* PMSAv8M handling of the aliases is different from v7M:
@@ -799,7 +799,7 @@  static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value,
              */
             int aliasno = (offset - 0xd9c) / 8; /* 0..3 */
 
-            region = cpu->env.pmsav7.rnr;
+            region = cpu->env.pmsav7.rnr[attrs.secure];
             if (aliasno) {
                 region = deposit32(region, 0, 2, aliasno);
             }
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 333029c..11038b8 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -258,7 +258,8 @@  static void arm_cpu_reset(CPUState *s)
                        sizeof(*env->pmsav7.dracr) * cpu->pmsav7_dregion);
             }
         }
-        env->pmsav7.rnr = 0;
+        env->pmsav7.rnr[M_REG_NS] = 0;
+        env->pmsav7.rnr[M_REG_S] = 0;
         memset(env->pmsav8.mair0, 0, sizeof(env->pmsav8.mair0));
         memset(env->pmsav8.mair1, 0, sizeof(env->pmsav8.mair1));
     }
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 5394cef..48e0fc6 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2385,7 +2385,7 @@  static uint64_t pmsav7_read(CPUARMState *env, const ARMCPRegInfo *ri)
         return 0;
     }
 
-    u32p += env->pmsav7.rnr;
+    u32p += env->pmsav7.rnr[M_REG_NS];
     return *u32p;
 }
 
@@ -2399,7 +2399,7 @@  static void pmsav7_write(CPUARMState *env, const ARMCPRegInfo *ri,
         return;
     }
 
-    u32p += env->pmsav7.rnr;
+    u32p += env->pmsav7.rnr[M_REG_NS];
     tlb_flush(CPU(cpu)); /* Mappings may have changed - purge! */
     *u32p = value;
 }
@@ -2442,7 +2442,7 @@  static const ARMCPRegInfo pmsav7_cp_reginfo[] = {
       .resetfn = arm_cp_reset_ignore },
     { .name = "RGNR", .cp = 15, .crn = 6, .opc1 = 0, .crm = 2, .opc2 = 0,
       .access = PL1_RW,
-      .fieldoffset = offsetof(CPUARMState, pmsav7.rnr),
+      .fieldoffset = offsetof(CPUARMState, pmsav7.rnr[M_REG_NS]),
       .writefn = pmsav7_rgnr_write,
       .resetfn = arm_cp_reset_ignore },
     REGINFO_SENTINEL
diff --git a/target/arm/machine.c b/target/arm/machine.c
index 05c6c7a..6941e35 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -167,7 +167,7 @@  static bool pmsav7_rgnr_vmstate_validate(void *opaque, int version_id)
 {
     ARMCPU *cpu = opaque;
 
-    return cpu->env.pmsav7.rnr < cpu->pmsav7_dregion;
+    return cpu->env.pmsav7.rnr[M_REG_NS] < cpu->pmsav7_dregion;
 }
 
 static const VMStateDescription vmstate_pmsav7 = {
@@ -205,7 +205,7 @@  static const VMStateDescription vmstate_pmsav7_rnr = {
     .minimum_version_id = 1,
     .needed = pmsav7_rnr_needed,
     .fields = (VMStateField[]) {
-        VMSTATE_UINT32(env.pmsav7.rnr, ARMCPU),
+        VMSTATE_UINT32(env.pmsav7.rnr[M_REG_NS], ARMCPU),
         VMSTATE_END_OF_LIST()
     }
 };
@@ -235,6 +235,13 @@  static const VMStateDescription vmstate_pmsav8 = {
     }
 };
 
+static bool s_rnr_vmstate_validate(void *opaque, int version_id)
+{
+    ARMCPU *cpu = opaque;
+
+    return cpu->env.pmsav7.rnr[M_REG_S] < cpu->pmsav7_dregion;
+}
+
 static bool m_security_needed(void *opaque)
 {
     ARMCPU *cpu = opaque;
@@ -261,6 +268,8 @@  static const VMStateDescription vmstate_m_security = {
                               0, vmstate_info_uint32, uint32_t),
         VMSTATE_VARRAY_UINT32(env.pmsav8.rlar[M_REG_S], ARMCPU, pmsav7_dregion,
                               0, vmstate_info_uint32, uint32_t),
+        VMSTATE_UINT32(env.pmsav7.rnr[M_REG_S], ARMCPU),
+        VMSTATE_VALIDATE("secure MPU_RNR is valid", s_rnr_vmstate_validate),
         VMSTATE_END_OF_LIST()
     }
 };