Skip to contentSkip to conversation

ARM Cortex R5 Support

PATCH Superseded Peter Crosthwaite 2 participants 22 messages Jun 12, 2015 – Jun 16, 2015 Arm/Zynq architecture

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Peter Crosthwaite

author
#0 lore raw

v2 cover ARM Cortex R5 Support

Hi Peter and all,

This patch series adds ARM Cortex R5 processor support. The PMSAv7 MPU is implemented. Two R5s are added to the Xilinx ZynqMP SoC.

Changed since v1:
Addressed PMM and Alistair reviews (see indiv. patches) Adding prepatory refactorings to target-arm (new patches)
- TLBTR VMSA conditional (1)
- V7MP CP regs VMSA conditional (2)
- Refactor get_phys_addr FSR return path (4)
- Add MPUIR.U config (5)
- Improved cpu configurability around MPUs (6-7)

Regards,
Peter

Peter Crosthwaite (13):
arm: Do not define TLBTR in PMSA systems
arm: Don't add v7mp registers in MPU systems
arm: helper: Factor out CP regs common to [pv]msa
arm: Refactor get_phys_addr FSR return mechanism
arm: Implement uniprocessor with MP config
arm: Add has-mpu property target-arm/helper.c: define MPUIR register
arm: helper: rename get_phys_addr_mpu
target-arm: Add registers for PMSAv7
target-arm: Implement PMSAv7 MPU
target-arm: Add support for Cortex-R5
arm: xlnx-zynqmp: Preface CPU variables with "apu"
arm: xlnx-zynqmp: Add 2xCortexR5 CPUs

 hw/arm/xlnx-ep108.c          |   2 +-
 hw/arm/xlnx-zynqmp.c         |  53 ++++--
 include/hw/arm/xlnx-zynqmp.h |   6 +-
 target-arm/cpu-qom.h         |   8 +
 target-arm/cpu.c             |  69 +++++++
 target-arm/cpu.h             |  11 ++
 target-arm/helper.c          | 439 +++++++++++++++++++++++++++++++++++--------
 target-arm/internals.h       |   3 +-
 target-arm/machine.c         |  35 ++++
 target-arm/op_helper.c       |  11 +-
 10 files changed, 533 insertions(+), 104 deletions(-)
Signature · 1 line
2.4.3.3.g905f831
author
#1 lore raw

PATCH v2 1/13 arm: Do not define TLBTR in PMSA systems

If doing a PMSA (MPU) system do not define the VMSA specific TLBTR CP. The def is done separately from VMSA registers group as it is affected by both the OMAP/STRONGARM RW errata and the MIDR backgrounding.

Signed-off-by Peter Crosthwaite <[email protected]>
---
 target-arm/helper.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)
target-arm/helper.c +10 -3
index 3da0c05..1e5c4c1 100644
@@ -3448,11 +3448,14 @@ void register_cp_regs_for_features(ARMCPU *cpu)
             { .name = "TCMTR",
               .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 2,
               .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 },
-            { .name = "TLBTR",
-              .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 3,
-              .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 },
             REGINFO_SENTINEL
         };
+        /* TLBTR is specific to VMSA */
+        ARMCPRegInfo id_tlbtr_reginfo = {
+              .name = "TLBTR",
+              .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 3,
+              .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0,
+        };
         ARMCPRegInfo crn0_wi_reginfo = {
             .name = "CRN0_WI", .cp = 15, .crn = 0, .crm = CP_ANY,
             .opc1 = CP_ANY, .opc2 = CP_ANY, .access = PL1_W,

@@ -3474,6 +3477,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
             for (r = id_cp_reginfo; r->type != ARM_CP_SENTINEL; r++) {
                 r->access = PL1_RW;
             }
+            id_tlbtr_reginfo.access = PL1_RW;
         }
         if (arm_feature(env, ARM_FEATURE_V8)) {
             define_arm_cp_regs(cpu, id_v8_midr_cp_reginfo);

@@ -3481,6 +3485,9 @@ void register_cp_regs_for_features(ARMCPU *cpu)
             define_arm_cp_regs(cpu, id_pre_v8_midr_cp_reginfo);
         }
         define_arm_cp_regs(cpu, id_cp_reginfo);
+        if (!arm_feature(env, ARM_FEATURE_MPU)) {
+            define_one_arm_cp_reg(cpu, &id_tlbtr_reginfo);
+        }
     }
 
     if (arm_feature(env, ARM_FEATURE_MPIDR)) {
Signature · 1 line
2.4.3.3.g905f831
author
#2 lore raw

PATCH v2 2/13 arm: Don't add v7mp registers in MPU systems

In reply to Peter Crosthwaite

These registers are VMSA specific so they should be conditional on VMSA (i.e. !MPU).

Signed-off-by Peter Crosthwaite <[email protected]>
---
 target-arm/helper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
target-arm/helper.c +2 -1
index 1e5c4c1..ce320ed 100644
@@ -3196,7 +3196,8 @@ void register_cp_regs_for_features(ARMCPU *cpu)
     if (arm_feature(env, ARM_FEATURE_V6K)) {
         define_arm_cp_regs(cpu, v6k_cp_reginfo);
     }
-    if (arm_feature(env, ARM_FEATURE_V7MP)) {
+    if (arm_feature(env, ARM_FEATURE_V7MP) &&
+        !arm_feature(env, ARM_FEATURE_MPU)) {
         define_arm_cp_regs(cpu, v7mp_cp_reginfo);
     }
     if (arm_feature(env, ARM_FEATURE_V7)) {
Signature · 1 line
2.4.3.3.g905f831
author
#3 lore raw

PATCH v2 3/13 arm: helper: Factor out CP regs common to [pv]msa

In reply to Peter Crosthwaite

V6+ PMSA and VMSA share some common registers that are currently in the VMSA definition block. Split them out into a new def that can be shared to PMSA.

Signed-off-by Peter Crosthwaite <[email protected]>

---
Factor out FAR_EL1 as well

 target-arm/helper.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)
target-arm/helper.c +14 -9
index ce320ed..fa80884 100644
@@ -1846,7 +1846,7 @@ static void vmsa_ttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
     raw_write(env, ri, value);
 }
 
-static const ARMCPRegInfo vmsa_cp_reginfo[] = {
+static const ARMCPRegInfo vmsa_pmsa_cp_reginfo[] = {
     { .name = "DFSR", .cp = 15, .crn = 5, .crm = 0, .opc1 = 0, .opc2 = 0,
       .access = PL1_RW, .type = ARM_CP_ALIAS,
       .bank_fieldoffsets = { offsetoflow32(CPUARMState, cp15.dfsr_s),

@@ -1856,6 +1856,18 @@ static const ARMCPRegInfo vmsa_cp_reginfo[] = {
       .access = PL1_RW, .resetvalue = 0,
       .bank_fieldoffsets = { offsetoflow32(CPUARMState, cp15.ifsr_s),
                              offsetoflow32(CPUARMState, cp15.ifsr_ns) } },
+    { .name = "DFAR", .cp = 15, .opc1 = 0, .crn = 6, .crm = 0, .opc2 = 0,
+      .access = PL1_RW, .resetvalue = 0,
+      .bank_fieldoffsets = { offsetof(CPUARMState, cp15.dfar_s),
+                             offsetof(CPUARMState, cp15.dfar_ns) } },
+    { .name = "FAR_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .crn = 6, .crm = 0, .opc1 = 0, .opc2 = 0,
+      .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.far_el[1]),
+      .resetvalue = 0, },
+    REGINFO_SENTINEL
+};
+
+static const ARMCPRegInfo vmsa_cp_reginfo[] = {
     { .name = "ESR_EL1", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .crn = 5, .crm = 2, .opc1 = 0, .opc2 = 0,
       .access = PL1_RW,

@@ -1880,14 +1892,6 @@ static const ARMCPRegInfo vmsa_cp_reginfo[] = {
       .resetfn = arm_cp_reset_ignore, .raw_writefn = vmsa_ttbcr_raw_write,
       .bank_fieldoffsets = { offsetoflow32(CPUARMState, cp15.tcr_el[3]),
                              offsetoflow32(CPUARMState, cp15.tcr_el[1])} },
-    { .name = "FAR_EL1", .state = ARM_CP_STATE_AA64,
-      .opc0 = 3, .crn = 6, .crm = 0, .opc1 = 0, .opc2 = 0,
-      .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.far_el[1]),
-      .resetvalue = 0, },
-    { .name = "DFAR", .cp = 15, .opc1 = 0, .crn = 6, .crm = 0, .opc2 = 0,
-      .access = PL1_RW, .resetvalue = 0,
-      .bank_fieldoffsets = { offsetof(CPUARMState, cp15.dfar_s),
-                             offsetof(CPUARMState, cp15.dfar_ns) } },
     REGINFO_SENTINEL
 };
 

@@ -3349,6 +3353,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
         assert(!arm_feature(env, ARM_FEATURE_V6));
         define_arm_cp_regs(cpu, pmsav5_cp_reginfo);
     } else {
+        define_arm_cp_regs(cpu, vmsa_pmsa_cp_reginfo);
         define_arm_cp_regs(cpu, vmsa_cp_reginfo);
     }
     if (arm_feature(env, ARM_FEATURE_THUMB2EE)) {
Signature · 1 line
2.4.3.3.g905f831
author
#4 lore raw

PATCH v2 4/13 arm: Refactor get_phys_addr FSR return mechanism

In reply to Peter Crosthwaite

Currently, the return code for get_phys_addr is overloaded for both success/fail and FSR value return. This doesn't handle the case where there is an error with a 0 FSR. This case exists in PMSAv7.

So rework get_phys_addr and friends to return a success/failure boolean return code and populate the FSR via a caller provided uint32_t pointer.

Signed-off-by Peter Crosthwaite <[email protected]>
---
 target-arm/helper.c    | 128 +++++++++++++++++++++++++++----------------------
 target-arm/internals.h |   3 +-
 target-arm/op_helper.c |  11 +++--
 3 files changed, 78 insertions(+), 64 deletions(-)
target-arm/helper.c +70 -58
index fa80884..88e59a7 100644
@@ -12,10 +12,10 @@
 #include <zlib.h> /* For crc32 */
 
 #ifndef CONFIG_USER_ONLY
-static inline int get_phys_addr(CPUARMState *env, target_ulong address,
-                                int access_type, ARMMMUIdx mmu_idx,
-                                hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot,
-                                target_ulong *page_size);
+static inline bool get_phys_addr(CPUARMState *env, target_ulong address,
+                                 int access_type, ARMMMUIdx mmu_idx,
+                                 hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot,
+                                 target_ulong *page_size, uint32_t *fsr);
 
 /* Definitions for the PMCCNTR and PMCR registers */
 #define PMCRD   0x8

@@ -1495,19 +1495,20 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t value,
     hwaddr phys_addr;
     target_ulong page_size;
     int prot;
-    int ret;
+    uint32_t fsr;
+    bool ret;
     uint64_t par64;
     MemTxAttrs attrs = {};
 
     ret = get_phys_addr(env, value, access_type, mmu_idx,
-                        &phys_addr, &attrs, &prot, &page_size);
+                        &phys_addr, &attrs, &prot, &page_size, &fsr);
     if (extended_addresses_enabled(env)) {
-        /* ret is a DFSR/IFSR value for the long descriptor
+        /* fsr is a DFSR/IFSR value for the long descriptor
          * translation table format, but with WnR always clear.
          * Convert it to a 64-bit PAR.
          */
         par64 = (1 << 11); /* LPAE bit always set */
-        if (ret == 0) {
+        if (!ret) {
             par64 |= phys_addr & ~0xfffULL;
             if (!attrs.secure) {
                 par64 |= (1 << 9); /* NS */

@@ -1515,18 +1516,18 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t value,
             /* We don't set the ATTR or SH fields in the PAR. */
         } else {
             par64 |= 1; /* F */
-            par64 |= (ret & 0x3f) << 1; /* FS */
+            par64 |= (fsr & 0x3f) << 1; /* FS */
             /* Note that S2WLK and FSTAGE are always zero, because we don't
              * implement virtualization and therefore there can't be a stage 2
              * fault.
              */
         }
     } else {
-        /* ret is a DFSR/IFSR value for the short descriptor
+        /* fsr is a DFSR/IFSR value for the short descriptor
          * translation table format (with WnR always clear).
          * Convert it to a 32-bit PAR.
          */
-        if (ret == 0) {
+        if (!ret) {
             /* We do not set any attribute bits in the PAR */
             if (page_size == (1 << 24)
                 && arm_feature(env, ARM_FEATURE_V7)) {

@@ -1538,8 +1539,8 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t value,
                 par64 |= (1 << 9); /* NS */
             }
         } else {
-            par64 = ((ret & (1 << 10)) >> 5) | ((ret & (1 << 12)) >> 6) |
-                    ((ret & 0xf) << 1) | 1;
+            par64 = ((fsr & (1 << 10)) >> 5) | ((fsr & (1 << 12)) >> 6) |
+                    ((fsr & 0xf) << 1) | 1;
         }
     }
     return par64;

@@ -5242,9 +5243,10 @@ static uint64_t arm_ldq_ptw(CPUState *cs, hwaddr addr, bool is_secure)
     return address_space_ldq(cs->as, addr, attrs, NULL);
 }
 
-static int get_phys_addr_v5(CPUARMState *env, uint32_t address, int access_type,
-                            ARMMMUIdx mmu_idx, hwaddr *phys_ptr,
-                            int *prot, target_ulong *page_size)
+static bool get_phys_addr_v5(CPUARMState *env, uint32_t address,
+                             int access_type, ARMMMUIdx mmu_idx,
+                             hwaddr *phys_ptr, int *prot,
+                             target_ulong *page_size, uint32_t *fsr)
 {
     CPUState *cs = CPU(arm_env_get_cpu(env));
     int code;

@@ -5343,15 +5345,16 @@ static int get_phys_addr_v5(CPUARMState *env, uint32_t address, int access_type,
         goto do_fault;
     }
     *phys_ptr = phys_addr;
-    return 0;
+    return false;
 do_fault:
-    return code | (domain << 4);
+    *fsr = code | (domain << 4);
+    return true;
 }
 
-static int get_phys_addr_v6(CPUARMState *env, uint32_t address, int access_type,
-                            ARMMMUIdx mmu_idx, hwaddr *phys_ptr,
-                            MemTxAttrs *attrs,
-                            int *prot, target_ulong *page_size)
+static bool get_phys_addr_v6(CPUARMState *env, uint32_t address,
+                             int access_type, ARMMMUIdx mmu_idx,
+                             hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot,
+                             target_ulong *page_size, uint32_t *fsr)
 {
     CPUState *cs = CPU(arm_env_get_cpu(env));
     int code;

@@ -5482,9 +5485,10 @@ static int get_phys_addr_v6(CPUARMState *env, uint32_t address, int access_type,
         attrs->secure = false;
     }
     *phys_ptr = phys_addr;
-    return 0;
+    return false;
 do_fault:
-    return code | (domain << 4);
+    *fsr = code | (domain << 4);
+    return true;
 }
 
 /* Fault type for long-descriptor MMU fault reporting; this corresponds

@@ -5496,10 +5500,10 @@ typedef enum {
     permission_fault = 3,
 } MMUFaultType;
 
-static int get_phys_addr_lpae(CPUARMState *env, target_ulong address,
-                              int access_type, ARMMMUIdx mmu_idx,
-                              hwaddr *phys_ptr, MemTxAttrs *txattrs, int *prot,
-                              target_ulong *page_size_ptr)
+static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
+                               int access_type, ARMMMUIdx mmu_idx,
+                               hwaddr *phys_ptr, MemTxAttrs *txattrs, int *prot,
+                               target_ulong *page_size_ptr, uint32_t *fsr)
 {
     CPUState *cs = CPU(arm_env_get_cpu(env));
     /* Read an LPAE long-descriptor translation table. */

@@ -5738,16 +5742,17 @@ static int get_phys_addr_lpae(CPUARMState *env, target_ulong address,
     }
     *phys_ptr = descaddr;
     *page_size_ptr = page_size;
-    return 0;
+    return false;
 
 do_fault:
     /* Long-descriptor format IFSR/DFSR value */
-    return (1 << 9) | (fault_type << 2) | level;
+    *fsr = (1 << 9) | (fault_type << 2) | level;
+    return true;
 }
 
-static int get_phys_addr_mpu(CPUARMState *env, uint32_t address,
-                             int access_type, ARMMMUIdx mmu_idx,
-                             hwaddr *phys_ptr, int *prot)
+static bool get_phys_addr_mpu(CPUARMState *env, uint32_t address,
+                              int access_type, ARMMMUIdx mmu_idx,
+                              hwaddr *phys_ptr, int *prot, uint32_t *fsr)
 {
     int n;
     uint32_t mask;

@@ -5769,7 +5774,8 @@ static int get_phys_addr_mpu(CPUARMState *env, uint32_t address,
         }
     }
     if (n < 0) {
-        return 2;
+        *fsr = 2;
+        return true;
     }
 
     if (access_type == 2) {

@@ -5780,10 +5786,12 @@ static int get_phys_addr_mpu(CPUARMState *env, uint32_t address,
     mask = (mask >> (n * 4)) & 0xf;
     switch (mask) {
     case 0:
-        return 1;
+        *fsr = 1;
+        return true;
     case 1:
         if (is_user) {
-            return 1;
+            *fsr = 1;
+            return true;
         }
         *prot = PAGE_READ | PAGE_WRITE;
         break;

@@ -5798,7 +5806,8 @@ static int get_phys_addr_mpu(CPUARMState *env, uint32_t address,
         break;
     case 5:
         if (is_user) {
-            return 1;
+            *fsr = 1;
+            return true;
         }
         *prot = PAGE_READ;
         break;

@@ -5807,10 +5816,11 @@ static int get_phys_addr_mpu(CPUARMState *env, uint32_t address,
         break;
     default:
         /* Bad permission.  */
-        return 1;
+        *fsr = 1;
+        return true;
     }
     *prot |= PAGE_EXEC;
-    return 0;
+    return false;
 }
 
 /* get_phys_addr - get the physical address for this virtual address

@@ -5819,8 +5829,8 @@ static int get_phys_addr_mpu(CPUARMState *env, uint32_t address,
  * by doing a translation table walk on MMU based systems or using the
  * MPU state on MPU based systems.
  *
- * Returns 0 if the translation was successful. Otherwise, phys_ptr, attrs,
- * prot and page_size may not be filled in, and the return value provides
+ * Returns false if the translation was successful. Otherwise, phys_ptr, attrs,
+ * prot and page_size may not be filled in, and the populated fsr value provides
  * information on why the translation aborted, in the format of a
  * DFSR/IFSR fault register, with the following caveats:
  *  * we honour the short vs long DFSR format differences.

@@ -5836,11 +5846,12 @@ static int get_phys_addr_mpu(CPUARMState *env, uint32_t address,
  * @attrs: set to the memory transaction attributes to use
  * @prot: set to the permissions for the page containing phys_ptr
  * @page_size: set to the size of the page containing phys_ptr
+ * @fsr: set to the DFSR/IFSR value on failure
  */
-static inline int get_phys_addr(CPUARMState *env, target_ulong address,
-                                int access_type, ARMMMUIdx mmu_idx,
-                                hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot,
-                                target_ulong *page_size)
+static inline bool get_phys_addr(CPUARMState *env, target_ulong address,
+                                 int access_type, ARMMMUIdx mmu_idx,
+                                 hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot,
+                                 target_ulong *page_size, uint32_t *fsr)
 {
     if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) {
         /* TODO: when we support EL2 we should here call ourselves recursively

@@ -5883,27 +5894,27 @@ static inline int get_phys_addr(CPUARMState *env, target_ulong address,
     if (arm_feature(env, ARM_FEATURE_MPU)) {
         *page_size = TARGET_PAGE_SIZE;
         return get_phys_addr_mpu(env, address, access_type, mmu_idx, phys_ptr,
-                                 prot);
+                                 prot, fsr);
     }
 
     if (regime_using_lpae_format(env, mmu_idx)) {
         return get_phys_addr_lpae(env, address, access_type, mmu_idx, phys_ptr,
-                                  attrs, prot, page_size);
+                                  attrs, prot, page_size, fsr);
     } else if (regime_sctlr(env, mmu_idx) & SCTLR_XP) {
         return get_phys_addr_v6(env, address, access_type, mmu_idx, phys_ptr,
-                                attrs, prot, page_size);
+                                attrs, prot, page_size, fsr);
     } else {
         return get_phys_addr_v5(env, address, access_type, mmu_idx, phys_ptr,
-                                prot, page_size);
+                                prot, page_size, fsr);
     }
 }
 
 /* Walk the page table and (if the mapping exists) add the page
- * to the TLB. Return 0 on success, or an ARM DFSR/IFSR fault
- * register format value on failure.
+ * to the TLB. Return false on success, or true on failure. Populate
+ * fsr with ARM DFSR/IFSR fault register format value on failure.
  */
-int arm_tlb_fill(CPUState *cs, vaddr address,
-                 int access_type, int mmu_idx)
+bool arm_tlb_fill(CPUState *cs, vaddr address,
+                  int access_type, int mmu_idx, uint32_t *fsr)
 {
     ARMCPU *cpu = ARM_CPU(cs);
     CPUARMState *env = &cpu->env;

@@ -5914,8 +5925,8 @@ int arm_tlb_fill(CPUState *cs, vaddr address,
     MemTxAttrs attrs = {};
 
     ret = get_phys_addr(env, address, access_type, mmu_idx, &phys_addr,
-                        &attrs, &prot, &page_size);
-    if (ret == 0) {
+                        &attrs, &prot, &page_size, fsr);
+    if (!ret) {
         /* Map a single [sub]page.  */
         phys_addr &= TARGET_PAGE_MASK;
         address &= TARGET_PAGE_MASK;

@@ -5934,13 +5945,14 @@ hwaddr arm_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
     hwaddr phys_addr;
     target_ulong page_size;
     int prot;
-    int ret;
+    bool ret;
+    uint32_t fsr;
     MemTxAttrs attrs = {};
 
     ret = get_phys_addr(env, addr, 0, cpu_mmu_index(env), &phys_addr,
-                        &attrs, &prot, &page_size);
+                        &attrs, &prot, &page_size, &fsr);
 
-    if (ret != 0) {
+    if (ret) {
         return -1;
     }
target-arm/internals.h +2 -1
index 1e5071e..924aff9 100644
@@ -388,6 +388,7 @@ void arm_handle_psci_call(ARMCPU *cpu);
 #endif
 
 /* Do a page table walk and add page to TLB if possible */
-int arm_tlb_fill(CPUState *cpu, vaddr address, int rw, int mmu_idx);
+bool arm_tlb_fill(CPUState *cpu, vaddr address, int rw, int mmu_idx,
+                  uint32_t *fsr);
 
 #endif
target-arm/op_helper.c +6 -5
index 7583ae7..7fa32c4 100644
@@ -81,9 +81,10 @@ uint32_t HELPER(neon_tbl)(CPUARMState *env, uint32_t ireg, uint32_t def,
 void tlb_fill(CPUState *cs, target_ulong addr, int is_write, int mmu_idx,
               uintptr_t retaddr)
 {
-    int ret;
+    bool ret;
+    uint32_t fsr = 0;
 
-    ret = arm_tlb_fill(cs, addr, is_write, mmu_idx);
+    ret = arm_tlb_fill(cs, addr, is_write, mmu_idx, &fsr);
     if (unlikely(ret)) {
         ARMCPU *cpu = ARM_CPU(cs);
         CPUARMState *env = &cpu->env;

@@ -96,7 +97,7 @@ void tlb_fill(CPUState *cs, target_ulong addr, int is_write, int mmu_idx,
         }
 
         /* AArch64 syndrome does not have an LPAE bit */
-        syn = ret & ~(1 << 9);
+        syn = fsr & ~(1 << 9);
 
         /* For insn and data aborts we assume there is no instruction syndrome
          * information; this is always true for exceptions reported to EL1.

@@ -107,13 +108,13 @@ void tlb_fill(CPUState *cs, target_ulong addr, int is_write, int mmu_idx,
         } else {
             syn = syn_data_abort(same_el, 0, 0, 0, is_write == 1, syn);
             if (is_write == 1 && arm_feature(env, ARM_FEATURE_V6)) {
-                ret |= (1 << 11);
+                fsr |= (1 << 11);
             }
             exc = EXCP_DATA_ABORT;
         }
 
         env->exception.vaddress = addr;
-        env->exception.fsr = ret;
+        env->exception.fsr = fsr;
         raise_exception(env, exc, syn, exception_target_el(env));
     }
 }
Signature · 1 line
2.4.3.3.g905f831
#5 lore raw

Re: [Qemu-devel] [PATCH target-arm v2 04/13] arm: Refactor get_phys_addr FSR return mechanism

Quoted 7 lines · Peter Crosthwaite

Currently, the return code for get_phys_addr is overloaded for both success/fail and FSR value return. This doesn't handle the case where there is an error with a 0 FSR. This case exists in PMSAv7.

So rework get_phys_addr and friends to return a success/failure boolean return code and populate the FSR via a caller provided uint32_t pointer.

true-on-failure, false-on-success is an odd way round for a success/failure bool, but I guess it keeps the rest of the code from changing too much...

-- PMM

author
#6 lore raw

PATCH v2 5/13 arm: Implement uniprocessor with MP config

In reply to Peter Crosthwaite

Add a boolean for indicating uniprocessors with MP extensions. This drives the U bit in MPIDR. Prepares support for Cortex-R5.

Signed-off-by Peter Crosthwaite <[email protected]>
---
 target-arm/cpu-qom.h | 3 +++
 target-arm/helper.c  | 9 ++++++---
 2 files changed, 9 insertions(+), 3 deletions(-)
target-arm/cpu-qom.h +3 -0
index ed5a644..19c8e9c 100644
@@ -116,6 +116,9 @@ typedef struct ARMCPU {
     /* KVM init features for this CPU */
     uint32_t kvm_init_features[7];
 
+    /* Uniprocessor system with MP extensions */
+    bool mp_is_up;
+
     /* The instance init functions for implementation-specific subclasses
      * set these fields to specify the implementation-dependent values of
      * various constant registers and reset values of non-constant
target-arm/helper.c +6 -3
index 88e59a7..8c6bc0c 100644
@@ -2068,7 +2068,8 @@ static const ARMCPRegInfo strongarm_cp_reginfo[] = {
 
 static uint64_t mpidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
-    CPUState *cs = CPU(arm_env_get_cpu(env));
+    ARMCPU *cpu = arm_env_get_cpu(env);
+    CPUState *cs = CPU(cpu);
     uint32_t mpidr = cs->cpu_index;
     /* We don't support setting cluster ID ([8..11]) (known as Aff1
      * in later ARM ARM versions), or any of the higher affinity level fields,

@@ -2078,9 +2079,11 @@ static uint64_t mpidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
         mpidr |= (1U << 31);
         /* Cores which are uniprocessor (non-coherent)
          * but still implement the MP extensions set
-         * bit 30. (For instance, A9UP.) However we do
-         * not currently model any of those cores.
+         * bit 30. (For instance, Cortex-R5).
          */
+        if (cpu->mp_is_up) {
+            mpidr |= (1u << 30);
+        }
     }
     return mpidr;
 }
Signature · 1 line
2.4.3.3.g905f831
author
#7 lore raw

PATCH v2 6/13 arm: Add has-mpu property

In reply to Peter Crosthwaite

For processors that support MPUs, add a property to de-feature it. This is similar to the implementation of the EL3 feature.

The processor definition in init sets ARM_FEATURE_MPU if it can support an MPU. post_init exposes the property, defaulting to true. If cleared by the instantiator, ARM_FEATURE_MPU is then removed at realize time.

This is to support R profile processors that may or may-not have an MPU configured.

Signed-off-by Peter Crosthwaite <[email protected]>
---
 target-arm/cpu-qom.h |  3 +++
 target-arm/cpu.c     | 13 +++++++++++++
 2 files changed, 16 insertions(+)
target-arm/cpu-qom.h +3 -0
index 19c8e9c..30832d9 100644
@@ -103,6 +103,9 @@ typedef struct ARMCPU {
     /* CPU has security extension */
     bool has_el3;
 
+    /* CPU has memory protection unit */
+    bool has_mpu;
+
     /* PSCI conduit used to invoke PSCI methods
      * 0 - disabled, 1 - smc, 2 - hvc
      */
target-arm/cpu.c +13 -0
index 4a888ab..82ac52d 100644
@@ -442,6 +442,9 @@ static Property arm_cpu_rvbar_property =
 static Property arm_cpu_has_el3_property =
             DEFINE_PROP_BOOL("has_el3", ARMCPU, has_el3, true);
 
+static Property arm_cpu_has_mpu_property =
+            DEFINE_PROP_BOOL("has-mpu", ARMCPU, has_mpu, true);
+
 static void arm_cpu_post_init(Object *obj)
 {
     ARMCPU *cpu = ARM_CPU(obj);

@@ -469,6 +472,12 @@ static void arm_cpu_post_init(Object *obj)
         qdev_property_add_static(DEVICE(obj), &arm_cpu_has_el3_property,
                                  &error_abort);
     }
+
+    if (arm_feature(&cpu->env, ARM_FEATURE_MPU)) {
+        qdev_property_add_static(DEVICE(obj), &arm_cpu_has_mpu_property,
+                                 &error_abort);
+    }
+
 }
 
 static void arm_cpu_finalizefn(Object *obj)

@@ -551,6 +560,10 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
         cpu->id_aa64pfr0 &= ~0xf000;
     }
 
+    if (!cpu->has_mpu) {
+        unset_feature(env, ARM_FEATURE_MPU);
+    }
+
     register_cp_regs_for_features(cpu);
     arm_cpu_register_gdb_regs_for_features(cpu);
Signature · 1 line
2.4.3.3.g905f831
author
#8 lore raw

PATCH v2 7/13 target-arm/helper.c: define MPUIR register

In reply to Peter Crosthwaite

Define the MPUIR register for MPU supporting systems V6 onwards. Currently only support unified MPU.

The size of the unified MPU is supported via the number of "dregions". So just a single config as added to specify this size. (When split MPU is implemented iregions will accompany).

Signed-off-by Peter Crosthwaite <[email protected]>

---
changed since v1:
Add #regions configuration
conditionalise MPUIR existence

 target-arm/cpu-qom.h |  2 ++
 target-arm/cpu.c     |  8 ++++++++
 target-arm/helper.c  | 10 ++++++++++
 3 files changed, 20 insertions(+)
target-arm/cpu-qom.h +2 -0
index 30832d9..05c33ac 100644
@@ -105,6 +105,8 @@ typedef struct ARMCPU {
 
     /* CPU has memory protection unit */
     bool has_mpu;
+    /* PMSAv7 MPU number of supported regions */
+    uint32_t pmsav7_dregion;
 
     /* PSCI conduit used to invoke PSCI methods
      * 0 - disabled, 1 - smc, 2 - hvc
target-arm/cpu.c +8 -0
index 82ac52d..c967763 100644
@@ -445,6 +445,9 @@ static Property arm_cpu_has_el3_property =
 static Property arm_cpu_has_mpu_property =
             DEFINE_PROP_BOOL("has-mpu", ARMCPU, has_mpu, true);
 
+static Property arm_cpu_pmsav7_dregion_property =
+            DEFINE_PROP_UINT32("pmsav7-dregion", ARMCPU, pmsav7_dregion, 16);
+
 static void arm_cpu_post_init(Object *obj)
 {
     ARMCPU *cpu = ARM_CPU(obj);

@@ -476,6 +479,11 @@ static void arm_cpu_post_init(Object *obj)
     if (arm_feature(&cpu->env, ARM_FEATURE_MPU)) {
         qdev_property_add_static(DEVICE(obj), &arm_cpu_has_mpu_property,
                                  &error_abort);
+        if (arm_feature(&cpu->env, ARM_FEATURE_V7)) {
+            qdev_property_add_static(DEVICE(obj),
+                                     &arm_cpu_pmsav7_dregion_property,
+                                     &error_abort);
+        }
     }
 
 }
target-arm/helper.c +10 -0
index 8c6bc0c..4a2965f 100644
@@ -3466,6 +3466,13 @@ void register_cp_regs_for_features(ARMCPU *cpu)
               .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 3,
               .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0,
         };
+        /* MPUIR is specific to PMSA V6+ */
+        ARMCPRegInfo id_mpuir_reginfo = {
+              .name = "MPUIR",
+              .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 4,
+              .access = PL1_R, .type = ARM_CP_CONST,
+              .resetvalue = cpu->pmsav7_dregion << 8
+        };
         ARMCPRegInfo crn0_wi_reginfo = {
             .name = "CRN0_WI", .cp = 15, .crn = 0, .crm = CP_ANY,
             .opc1 = CP_ANY, .opc2 = CP_ANY, .access = PL1_W,

@@ -3488,6 +3495,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
                 r->access = PL1_RW;
             }
             id_tlbtr_reginfo.access = PL1_RW;
+            id_tlbtr_reginfo.access = PL1_RW;
         }
         if (arm_feature(env, ARM_FEATURE_V8)) {
             define_arm_cp_regs(cpu, id_v8_midr_cp_reginfo);

@@ -3497,6 +3505,8 @@ void register_cp_regs_for_features(ARMCPU *cpu)
         define_arm_cp_regs(cpu, id_cp_reginfo);
         if (!arm_feature(env, ARM_FEATURE_MPU)) {
             define_one_arm_cp_reg(cpu, &id_tlbtr_reginfo);
+        } else if (arm_feature(env, ARM_FEATURE_V7)) {
+            define_one_arm_cp_reg(cpu, &id_mpuir_reginfo);
         }
     }
Signature · 1 line
2.4.3.3.g905f831
#9 lore raw

Re: [Qemu-devel] [PATCH target-arm v2 07/13] target-arm/helper.c: define MPUIR register

Quoted 1 line · Peter Crosthwaite

Define the MPUIR register for MPU supporting systems V6 onwards.

"(ARMv6 and onwards)".

Quoted 1 line

Currently only support unified MPU.

"we only"

Quoted 3 lines

The size of the unified MPU is supported via the number of "dregions". So just a single config as added to specify this size. (When split MPU is implemented iregions will accompany).

Try:

The size of the unified MPU is defined via the number of "dregions". So just a single config is added to specify this size. (When split MPU is implemented we will add an extra iregions config).

Quoted 39 lines
Signed-off-by Peter Crosthwaite <[email protected]>

---
changed since v1:
Add #regions configuration
conditionalise MPUIR existence

 target-arm/cpu-qom.h |  2 ++
 target-arm/cpu.c     |  8 ++++++++
 target-arm/helper.c  | 10 ++++++++++
 3 files changed, 20 insertions(+)
target-arm/cpu-qom.h +2 -0
index 30832d9..05c33ac 100644
@@ -105,6 +105,8 @@ typedef struct ARMCPU {
 
     /* CPU has memory protection unit */
     bool has_mpu;
+    /* PMSAv7 MPU number of supported regions */
+    uint32_t pmsav7_dregion;
 
     /* PSCI conduit used to invoke PSCI methods
      * 0 - disabled, 1 - smc, 2 - hvc
target-arm/cpu.c +3 -0
index 82ac52d..c967763 100644
@@ -445,6 +445,9 @@ static Property arm_cpu_has_el3_property =
 static Property arm_cpu_has_mpu_property =
             DEFINE_PROP_BOOL("has-mpu", ARMCPU, has_mpu, true);
 
+static Property arm_cpu_pmsav7_dregion_property =
+            DEFINE_PROP_UINT32("pmsav7-dregion", ARMCPU, pmsav7_dregion, 16);
+
 static void arm_cpu_post_init(Object *obj)
 {
     ARMCPU *cpu = ARM_CPU(obj);

target-arm/cpu.c static void arm_cpu_post_init(Object *obj) · line 487

Quoted 9 lines
target-arm/cpu.c +5 -0
@@ -476,6 +479,11 @@ static void arm_cpu_post_init(Object *obj)
     if (arm_feature(&cpu->env, ARM_FEATURE_MPU)) {
         qdev_property_add_static(DEVICE(obj), &arm_cpu_has_mpu_property,
                                  &error_abort);
+        if (arm_feature(&cpu->env, ARM_FEATURE_V7)) {
+            qdev_property_add_static(DEVICE(obj),
+                                     &arm_cpu_pmsav7_dregion_property,
+                                     &error_abort);
+        }
     }

Worth making bogus values (<0, >255, at least) a realize error, I think, especially since we start allocating memory based on the number of regions later on.

Otherwise OK.

-- PMM

author
#10 lore raw

Re: [Qemu-devel] [PATCH target-arm v2 07/13] target-arm/helper.c: define MPUIR register

Quoted 14 lines · Peter Maydell
Quoted 1 line · Peter Crosthwaite

Define the MPUIR register for MPU supporting systems V6 onwards.

"(ARMv6 and onwards)".

Quoted 1 line

Currently only support unified MPU.

"we only"

Quoted 3 lines

The size of the unified MPU is supported via the number of "dregions". So just a single config as added to specify this size. (When split MPU is implemented iregions will accompany).

Quoted 6 lines

Try:

The size of the unified MPU is defined via the number of "dregions". So just a single config is added to specify this size. (When split MPU is implemented we will add an extra iregions config).

Fixed all.

Quoted 45 lines
Quoted 45 lines
Signed-off-by Peter Crosthwaite <[email protected]>

---
changed since v1:
Add #regions configuration
conditionalise MPUIR existence

 target-arm/cpu-qom.h |  2 ++
 target-arm/cpu.c     |  8 ++++++++
 target-arm/helper.c  | 10 ++++++++++
 3 files changed, 20 insertions(+)
target-arm/cpu-qom.h +2 -0
index 30832d9..05c33ac 100644
@@ -105,6 +105,8 @@ typedef struct ARMCPU {
 
     /* CPU has memory protection unit */
     bool has_mpu;
+    /* PMSAv7 MPU number of supported regions */
+    uint32_t pmsav7_dregion;
 
     /* PSCI conduit used to invoke PSCI methods
      * 0 - disabled, 1 - smc, 2 - hvc
target-arm/cpu.c +6 -0
index 82ac52d..c967763 100644
@@ -445,6 +445,9 @@ static Property arm_cpu_has_el3_property =
 static Property arm_cpu_has_mpu_property =
             DEFINE_PROP_BOOL("has-mpu", ARMCPU, has_mpu, true);
 
+static Property arm_cpu_pmsav7_dregion_property =
+            DEFINE_PROP_UINT32("pmsav7-dregion", ARMCPU, pmsav7_dregion, 16);
+
 static void arm_cpu_post_init(Object *obj)
 {
     ARMCPU *cpu = ARM_CPU(obj);

@@ -476,6 +479,11 @@ static void arm_cpu_post_init(Object *obj)
     if (arm_feature(&cpu->env, ARM_FEATURE_MPU)) {
         qdev_property_add_static(DEVICE(obj), &arm_cpu_has_mpu_property,
                                  &error_abort);
+        if (arm_feature(&cpu->env, ARM_FEATURE_V7)) {
+            qdev_property_add_static(DEVICE(obj),
+                                     &arm_cpu_pmsav7_dregion_property,
Quoted 9 lines

target-arm/cpu.c static void arm_cpu_post_init(Object *obj) · line 487

Quoted 3 lines
target-arm/cpu.c +2 -0
+                                     &error_abort);
+        }
     }

Worth making bogus values (<0, >255, at least) a realize error, I think, especially since we start allocating memory based on the number of regions later on.

Added realize error. So it occurs to me that 0 itself is valid for #dregions. I have patched the memory allocation to not happen in 0 case and added null guards to the pointer accessors. This means that a PMSA with 0 regions can be configured which will still call get_phys_addr_pmsav7 and get the default behaviour.

Regards,
Peter

Quoted 4 lines

Otherwise OK.

-- PMM

author
#11 lore raw

PATCH v2 8/13 arm: helper: rename get_phys_addr_mpu

In reply to Peter Crosthwaite

This get_phys_addr is really for pmsav5. Rename it accordingly.

Reviewed-by Peter Maydell <[email protected]>
Signed-off-by Peter Crosthwaite <[email protected]>
---
 target-arm/helper.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
target-arm/helper.c +5 -5
index 4a2965f..3f712a6 100644
@@ -5763,9 +5763,9 @@ do_fault:
     return true;
 }
 
-static bool get_phys_addr_mpu(CPUARMState *env, uint32_t address,
-                              int access_type, ARMMMUIdx mmu_idx,
-                              hwaddr *phys_ptr, int *prot, uint32_t *fsr)
+static bool get_phys_addr_pmsav5(CPUARMState *env, uint32_t address,
+                                 int access_type, ARMMMUIdx mmu_idx,
+                                 hwaddr *phys_ptr, int *prot, uint32_t *fsr)
 {
     int n;
     uint32_t mask;

@@ -5906,8 +5906,8 @@ static inline bool get_phys_addr(CPUARMState *env, target_ulong address,
 
     if (arm_feature(env, ARM_FEATURE_MPU)) {
         *page_size = TARGET_PAGE_SIZE;
-        return get_phys_addr_mpu(env, address, access_type, mmu_idx, phys_ptr,
-                                 prot, fsr);
+        return get_phys_addr_pmsav5(env, address, access_type, mmu_idx,
+                                    phys_ptr, prot, fsr);
     }
 
     if (regime_using_lpae_format(env, mmu_idx)) {
Signature · 1 line
2.4.3.3.g905f831
author
#12 lore raw

PATCH v2 9/13 target-arm: Add registers for PMSAv7

In reply to Peter Crosthwaite

Define the arm CP registers for PMSAv7 and their accessor functions. RGNR serves as a shared index that indexes into arrays storing the DRBAR, DRSR and DRACR registers. DRBAR and friends have to be VMSDd separately from the CP interface using a new PMSA specific VMSD subsection.

Signed-off-by Peter Crosthwaite <[email protected]>

---
changed since v1 (PMM review):
Use raw_ptr
Implement reset as memset
Implement VMSD support
Add out-of-bounds guard or rgnr_write Dynamically allocate registers
Move arrays out of cp15 struct into dedicated substruct

 target-arm/cpu.c     | 10 +++++++
 target-arm/cpu.h     | 10 +++++++
 target-arm/helper.c  | 78 +++++++++++++++++++++++++++++++++++++++++++++++-----
 target-arm/machine.c | 35 +++++++++++++++++++++++
 4 files changed, 126 insertions(+), 7 deletions(-)
target-arm/cpu.c +10 -0
index c967763..1ef2f03 100644
@@ -572,6 +572,16 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
         unset_feature(env, ARM_FEATURE_MPU);
     }
 
+    if (arm_feature(env, ARM_FEATURE_MPU) &&
+        arm_feature(env, ARM_FEATURE_V7)) {
+        int nr = cpu->pmsav7_dregion;
+
+        assert(nr);
+        env->pmsav7.drbar = g_new0(uint32_t, nr);
+        env->pmsav7.drsr = g_new0(uint32_t, nr);
+        env->pmsav7.dracr = g_new0(uint32_t, nr);
+    }
+
     register_cp_regs_for_features(cpu);
     arm_cpu_register_gdb_regs_for_features(cpu);
target-arm/cpu.h +10 -0
index 21b5b8e..43c1f85 100644
@@ -284,6 +284,9 @@ typedef struct CPUARMState {
             };
             uint64_t par_el[4];
         };
+
+        uint32_t c6_rgnr;
+
         uint32_t c9_insn; /* Cache lockdown registers.  */
         uint32_t c9_data;
         uint64_t c9_pmcr; /* performance monitor control register */

@@ -483,6 +486,13 @@ typedef struct CPUARMState {
     /* Internal CPU feature flags.  */
     uint64_t features;
 
+    /* PMSAv7 MPU */
+    struct {
+        uint32_t *drbar;
+        uint32_t *drsr;
+        uint32_t *dracr;
+    } pmsav7;
+
     void *nvic;
     const struct arm_boot_info *boot_info;
 } CPUARMState;
target-arm/helper.c +71 -7

90 lines not on this page · raw message

target-arm/machine.c +35 -0
index 9446e5a..ae0f7b1 100644
@@ -121,6 +121,38 @@ static const VMStateDescription vmstate_thumb2ee = {
     }
 };
 
+static bool pmsav7_needed(void *opaque)
+{
+    ARMCPU *cpu = opaque;
+    CPUARMState *env = &cpu->env;
+
+    return arm_feature(env, ARM_FEATURE_MPU) &&
+           arm_feature(env, ARM_FEATURE_V7);
+}
+
+static bool pmsav7_rgnr_vmstate_validate(void *opaque, int version_id)
+{
+    ARMCPU *cpu = opaque;
+
+    return cpu->env.cp15.c6_rgnr < cpu->pmsav7_dregion;
+}
+
+static const VMStateDescription vmstate_pmsav7 = {
+    .name = "cpu/pmsav7",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_VARRAY_UINT32(env.pmsav7.drbar, ARMCPU, pmsav7_dregion, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_VARRAY_UINT32(env.pmsav7.drsr, ARMCPU, pmsav7_dregion, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_VARRAY_UINT32(env.pmsav7.dracr, ARMCPU, pmsav7_dregion, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_VALIDATE("rgnr is valid", pmsav7_rgnr_vmstate_validate),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static int get_cpsr(QEMUFile *f, void *opaque, size_t size)
 {
     ARMCPU *cpu = opaque;

@@ -296,6 +328,9 @@ const VMStateDescription vmstate_arm_cpu = {
             .vmsd = &vmstate_thumb2ee,
             .needed = thumb2ee_needed,
         } , {
+            .vmsd = &vmstate_pmsav7,
+            .needed = pmsav7_needed,
+        } , {
             /* empty */
         }
     }
Signature · 1 line
2.4.3.3.g905f831
Reviewed-by on v2 9/13
#13 lore raw

Re: [Qemu-devel] [PATCH target-arm v2 09/13] target-arm: Add registers for PMSAv7

Quoted 41 lines · Peter Crosthwaite

Define the arm CP registers for PMSAv7 and their accessor functions. RGNR serves as a shared index that indexes into arrays storing the DRBAR, DRSR and DRACR registers. DRBAR and friends have to be VMSDd separately from the CP interface using a new PMSA specific VMSD subsection.

Signed-off-by Peter Crosthwaite <[email protected]>

---

target-arm/helper.c +32 -0
+static uint64_t pmsav7_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    uint32_t *u32p = *(uint32_t **)raw_ptr(env, ri);
+
+    u32p += env->cp15.c6_rgnr;
+    return *u32p;
+}
+
+static void pmsav7_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                         uint64_t value)
+{
+    ARMCPU *cpu = arm_env_get_cpu(env);
+    uint32_t *u32p = *(uint32_t **)raw_ptr(env, ri);
+
+    u32p += env->cp15.c6_rgnr;
+    tlb_flush(CPU(cpu), 1); /* Mappings may have changed - purge! */
+    *u32p = value;
+}
+
+static void pmsav7_reset(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    ARMCPU *cpu = arm_env_get_cpu(env);
+    uint32_t *u32p = *(uint32_t **)raw_ptr(env, ri);
+
+    memset(u32p, 0, sizeof(*u32p) * cpu->pmsav7_dregion);
+}
+
+static void pmsav7_rgnr_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                              uint64_t value)
+{
+    ARMCPU *cpu = arm_env_get_cpu(env);
+    uint32_t nrgs = cpu->pmsav7_dregion;

target-arm/helper.c static uint64_t pmsav5_insn_ap_read(CPUARMState *env, const ARMCPRegInfo *ri) · line 1754

Quoted 10 lines
target-arm/helper.c +10 -0
+
+    if (value >= nrgs) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "PMSAv7 RGNR write > # supported regions, %" PRIu32
+                      " > %" PRIu32 "\n", (uint32_t)value, nrgs);
+        return;
+    }
+
+    raw_write(env, ri, value);
+}

I'm not wild about the pointer games going on in these functions but I don't have a better suggestion.

Quoted 89 lines
target-arm/helper.c +28 -7
+static const ARMCPRegInfo pmsav7_cp_reginfo[] = {
+    { .name = "DRBAR", .cp = 15, .crn = 6, .opc1 = 0, .crm = 1, .opc2 = 0,
+      .access = PL1_RW, .type = ARM_CP_NO_RAW,
+      .fieldoffset = offsetof(CPUARMState, pmsav7.drbar),
+      .readfn = pmsav7_read, .writefn = pmsav7_write, .resetfn = pmsav7_reset },
+    { .name = "DRSR", .cp = 15, .crn = 6, .opc1 = 0, .crm = 1, .opc2 = 2,
+      .access = PL1_RW, .type = ARM_CP_NO_RAW,
+      .fieldoffset = offsetof(CPUARMState, pmsav7.drsr),
+      .readfn = pmsav7_read, .writefn = pmsav7_write, .resetfn = pmsav7_reset },
+    { .name = "DRACR", .cp = 15, .crn = 6, .opc1 = 0, .crm = 1, .opc2 = 4,
+      .access = PL1_RW, .type = ARM_CP_NO_RAW,
+      .fieldoffset = offsetof(CPUARMState, pmsav7.dracr),
+      .readfn = pmsav7_read, .writefn = pmsav7_write, .resetfn = pmsav7_reset },
+    { .name = "RGNR", .cp = 15, .crn = 6, .opc1 = 0, .crm = 2, .opc2 = 0,
+      .access = PL1_RW,
+      .fieldoffset = offsetof(CPUARMState, cp15.c6_rgnr),
+      .writefn = pmsav7_rgnr_write },
+    REGINFO_SENTINEL
+};
+
 static const ARMCPRegInfo pmsav5_cp_reginfo[] = {
     { .name = "DATA_AP", .cp = 15, .crn = 5, .crm = 0, .opc1 = 0, .opc2 = 0,
       .access = PL1_RW, .type = ARM_CP_ALIAS,

@@ -3349,13 +3412,14 @@ void register_cp_regs_for_features(ARMCPU *cpu)
         define_one_arm_cp_reg(cpu, &rvbar);
     }
     if (arm_feature(env, ARM_FEATURE_MPU)) {
-        /* These are the MPU registers prior to PMSAv6. Any new
-         * PMSA core later than the ARM946 will require that we
-         * implement the PMSAv6 or PMSAv7 registers, which are
-         * completely different.
-         */
-        assert(!arm_feature(env, ARM_FEATURE_V6));
-        define_arm_cp_regs(cpu, pmsav5_cp_reginfo);
+        if (arm_feature(env, ARM_FEATURE_V6)) {
+            /* PMSAv6 not implemented */
+            assert(arm_feature(env, ARM_FEATURE_V7));
+            define_arm_cp_regs(cpu, vmsa_pmsa_cp_reginfo);
+            define_arm_cp_regs(cpu, pmsav7_cp_reginfo);
+        } else {
+            define_arm_cp_regs(cpu, pmsav5_cp_reginfo);
+        }
     } else {
         define_arm_cp_regs(cpu, vmsa_pmsa_cp_reginfo);
         define_arm_cp_regs(cpu, vmsa_cp_reginfo);
target-arm/machine.c +32 -0
index 9446e5a..ae0f7b1 100644
@@ -121,6 +121,38 @@ static const VMStateDescription vmstate_thumb2ee = {
     }
 };
 
+static bool pmsav7_needed(void *opaque)
+{
+    ARMCPU *cpu = opaque;
+    CPUARMState *env = &cpu->env;
+
+    return arm_feature(env, ARM_FEATURE_MPU) &&
+           arm_feature(env, ARM_FEATURE_V7);
+}
+
+static bool pmsav7_rgnr_vmstate_validate(void *opaque, int version_id)
+{
+    ARMCPU *cpu = opaque;
+
+    return cpu->env.cp15.c6_rgnr < cpu->pmsav7_dregion;
+}
+
+static const VMStateDescription vmstate_pmsav7 = {
+    .name = "cpu/pmsav7",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_VARRAY_UINT32(env.pmsav7.drbar, ARMCPU, pmsav7_dregion, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_VARRAY_UINT32(env.pmsav7.drsr, ARMCPU, pmsav7_dregion, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_VARRAY_UINT32(env.pmsav7.dracr, ARMCPU, pmsav7_dregion, 0,
+                              vmstate_info_uint32, uint32_t),
+        VMSTATE_VALIDATE("rgnr is valid", pmsav7_rgnr_vmstate_validate),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static int get_cpsr(QEMUFile *f, void *opaque, size_t size)
 {
     ARMCPU *cpu = opaque;

target-arm/machine.c const VMStateDescription vmstate_arm_cpu = { · line 333

Quoted 6 lines
target-arm/machine.c +3 -0
@@ -296,6 +328,9 @@ const VMStateDescription vmstate_arm_cpu = {
             .vmsd = &vmstate_thumb2ee,
             .needed = thumb2ee_needed,
         } , {
+            .vmsd = &vmstate_pmsav7,
+            .needed = pmsav7_needed,
+        } , {

You'll find this has changed as of commit 5cd8cadae8db -- the needed function goes in the vmstate_pmsav7 VMStateDescription now.

Otherwise

Reviewed-by Peter Maydell <[email protected]>

thanks
-- PMM

author
#14 lore raw

PATCH v2 10/13 target-arm: Implement PMSAv7 MPU

In reply to Peter Crosthwaite

Unified MPU only. Uses ARM architecture major revision to switch between PMSAv5 and v7 when ARM_FEATURE_MPU is set. PMSA v6 remains unsupported and is asserted against.

Signed-off-by Peter Crosthwaite <[email protected]>

---
changed since v1 (PMM review):
Add comment about PMSAv6 non-support
Fix case where MPU is completely disabled Ignore regions with 0 size.
GUEST_ERROR invalid base address alignments UNIMP regions that are smaller than TARGET_PAGE_SIZE use extract32 to get SR disable bits
Fixed up DRACR AP bit error message.
Correct bullet point about MPU FSR format Rebased against new FSR return system removed *prot switch-case

 target-arm/cpu.h    |   1 +
 target-arm/helper.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 173 insertions(+), 1 deletion(-)
target-arm/cpu.h +1 -0
index 43c1f85..bfba377 100644
@@ -561,6 +561,7 @@ void pmccntr_sync(CPUARMState *env);
 #define SCTLR_DT      (1U << 16) /* up to ??, RAO in v6 and v7 */
 #define SCTLR_nTWI    (1U << 16) /* v8 onward */
 #define SCTLR_HA      (1U << 17)
+#define SCTLR_BR      (1U << 17) /* PMSA only */
 #define SCTLR_IT      (1U << 18) /* up to ??, RAO in v6 and v7 */
 #define SCTLR_nTWE    (1U << 18) /* v8 onward */
 #define SCTLR_WXN     (1U << 19)
target-arm/helper.c +172 -1

197 lines not on this page · raw message

Signature · 1 line
2.4.3.3.g905f831
Reviewed-by on v2 10/13
#15 lore raw

Re: [Qemu-devel] [PATCH target-arm v2 10/13] target-arm: Implement PMSAv7 MPU

Quoted 18 lines · Peter Crosthwaite

Unified MPU only. Uses ARM architecture major revision to switch between PMSAv5 and v7 when ARM_FEATURE_MPU is set. PMSA v6 remains unsupported and is asserted against.

Signed-off-by Peter Crosthwaite <[email protected]>

---
changed since v1 (PMM review):
Add comment about PMSAv6 non-support
Fix case where MPU is completely disabled Ignore regions with 0 size.
GUEST_ERROR invalid base address alignments UNIMP regions that are smaller than TARGET_PAGE_SIZE use extract32 to get SR disable bits
Fixed up DRACR AP bit error message.
Correct bullet point about MPU FSR format Rebased against new FSR return system removed *prot switch-case

Quoted 3 lines
 target-arm/cpu.h    |   1 +
 target-arm/helper.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 173 insertions(+), 1 deletion(-)
Reviewed-by Peter Maydell <[email protected]>

thanks
-- PMM

author
#16 lore raw

Re: [Qemu-devel] [PATCH target-arm v2 10/13] target-arm: Implement PMSAv7 MPU

In reply to Peter Crosthwaite
Quoted 65 lines · Peter Crosthwaite

Unified MPU only. Uses ARM architecture major revision to switch between PMSAv5 and v7 when ARM_FEATURE_MPU is set. PMSA v6 remains unsupported and is asserted against.

Signed-off-by Peter Crosthwaite <[email protected]>

---
changed since v1 (PMM review):
Add comment about PMSAv6 non-support
Fix case where MPU is completely disabled Ignore regions with 0 size.
GUEST_ERROR invalid base address alignments UNIMP regions that are smaller than TARGET_PAGE_SIZE use extract32 to get SR disable bits
Fixed up DRACR AP bit error message.
Correct bullet point about MPU FSR format Rebased against new FSR return system removed *prot switch-case

 target-arm/cpu.h    |   1 +
 target-arm/helper.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 173 insertions(+), 1 deletion(-)
target-arm/cpu.h +1 -0
index 43c1f85..bfba377 100644
@@ -561,6 +561,7 @@ void pmccntr_sync(CPUARMState *env);
 #define SCTLR_DT      (1U << 16) /* up to ??, RAO in v6 and v7 */
 #define SCTLR_nTWI    (1U << 16) /* v8 onward */
 #define SCTLR_HA      (1U << 17)
+#define SCTLR_BR      (1U << 17) /* PMSA only */
 #define SCTLR_IT      (1U << 18) /* up to ??, RAO in v6 and v7 */
 #define SCTLR_nTWE    (1U << 18) /* v8 onward */
 #define SCTLR_WXN     (1U << 19)
target-arm/helper.c +23 -0
index 588dbc9..6436c93 100644
@@ -5827,6 +5827,166 @@ do_fault:
     return true;
 }
 
+static inline void get_phys_addr_pmsav7_default(CPUARMState *env,
+                                                ARMMMUIdx mmu_idx,
+                                                int32_t address, int *prot)
+{
+    *prot = PAGE_READ | PAGE_WRITE;
+    switch (address) {
+    case 0xF0000000 ... 0xFFFFFFFF:
+        if (regime_sctlr(env, mmu_idx) & SCTLR_V) { /* hivecs execing is ok */
+            *prot |= PAGE_EXEC;
+        }
+        break;
+    case 0x00000000 ... 0x7FFFFFFF:
+        *prot |= PAGE_EXEC;
+        break;
+    }
+
+}
+
+static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
+                                 int access_type, ARMMMUIdx mmu_idx,
+                                 hwaddr *phys_ptr, int *prot, uint32_t *fsr)
+{
+    ARMCPU *cpu = arm_env_get_cpu(env);

target-arm/helper.c do_fault: · line 5862

Quoted 10 lines
target-arm/helper.c +10 -0
+    int n;
+    bool is_user = regime_is_user(env, mmu_idx);
+
+    *phys_ptr = address;
+    *prot = 0;
+
+    if (regime_translation_disabled(env, mmu_idx)) { /* MPU disabled */
+        get_phys_addr_pmsav7_default(env, mmu_idx, address, prot);
+    } else { /* MPU enabled */
+        for (n = cpu->pmsav7_dregion; n >= 0; n--) { /* region search */

-1 on initialiser. I guess I have been getting lucky with 0s in the right place on my test guest.

Quoted 63 lines
target-arm/helper.c +63 -0
+            uint32_t base = env->pmsav7.drbar[n];
+            uint32_t rsize = extract32(env->pmsav7.drsr[n], 1, 5);
+            uint32_t rmask;
+            bool srdis = false;
+
+            if (!(env->pmsav7.drsr[n] & 0x1)) {
+                continue;
+            }
+
+            if (!rsize) {
+                qemu_log_mask(LOG_GUEST_ERROR, "DRSR.Rsize field can not be 0");
+                continue;
+            }
+            rsize++;
+            rmask = (1ull << rsize) - 1;
+
+            if (base & rmask) {
+                qemu_log_mask(LOG_GUEST_ERROR, "DRBAR %" PRIx32 " misaligned "
+                              "to DRSR region size, mask = %" PRIx32,
+                              base, rmask);
+                continue;
+            }
+
+            if (address < base || address > base + rmask) {
+                continue;
+            }
+
+            /* Region matched */
+
+            if (rsize >= 8) { /* no subregions for regions < 256 bytes */
+                int i, snd;
+                uint32_t srdis_mask;
+
+                rsize -= 3; /* sub region size (power of 2) */
+                snd = ((address - base) >> rsize) & 0x7;
+                srdis = extract32(env->pmsav7.drsr[n], snd + 8, 1);
+
+                srdis_mask = srdis ? 0x3 : 0x0;
+                for (i = 2; i <= 8 && rsize < TARGET_PAGE_BITS; i *= 2) {
+                    /* This will check in groups of 2, 4 and then 8, whether
+                     * the subregion bits are consistent. rsize is incremented
+                     * back up to give the region size, considering consistent
+                     * adjacent subregions as one region. Stop testing if rsize
+                     * is already big enough for an entire QEMU page.
+                     */
+                    int snd_rounded = snd & ~(i - 1);
+                    uint32_t srdis_multi = extract32(env->pmsav7.drsr[n],
+                                                     snd_rounded + 8, i);
+                    if (srdis_mask ^ srdis_multi) {
+                        break;
+                    }
+                    srdis_mask = (srdis_mask << i) | srdis_mask;
+                    rsize++;
+                }
+            }
+            if (rsize < TARGET_PAGE_BITS) {
+                qemu_log_mask(LOG_UNIMP, "No support for MPU (sub)region"
+                              "alignment of %" PRIu32 " bits. Minimum is %d\n",
+                              rsize, TARGET_PAGE_BITS);
+                continue;
+            }
+            if (srdis) {
+                continue;

target-arm/helper.c do_fault: · line 5935

Quoted 10 lines
target-arm/helper.c +10 -0
+            }
+            break;
+        }
+
+        if (n == -1) { /* no hits */
+            if (is_user || !(regime_sctlr(env, mmu_idx) & SCTLR_BR)) {
+                /* background fault */
+                *fsr = 0;
+                return true;
+            } else {

This should be the behavior if dregions == 0 to handle a PMSA with no MPU. I have patched the if above to fallthrough to the else in this case.

else not actually needed due to short return in if. Removed.

Regards,
Peter

Quoted 94 lines
target-arm/helper.c +66 -2
+                get_phys_addr_pmsav7_default(env, mmu_idx, address, prot);
+            }
+        } else { /* a MPU hit! */
+            uint32_t ap = extract32(env->pmsav7.dracr[n], 8, 3);
+
+            if (is_user) { /* User mode AP bit decoding */
+                switch (ap) {
+                case 0:
+                case 1:
+                case 5:
+                    break; /* no access */
+                case 3:
+                    *prot |= PAGE_WRITE;
+                    /* fall through */
+                case 2:
+                case 6:
+                    *prot |= PAGE_READ | PAGE_EXEC;
+                    break;
+                default:
+                    qemu_log_mask(LOG_GUEST_ERROR,
+                                  "Bad value for AP bits in DRACR %"
+                                  PRIx32 "\n", ap);
+                }
+            } else { /* Priv. mode AP bits decoding */
+                switch (ap) {
+                case 0:
+                    break; /* no access */
+                case 1:
+                case 2:
+                case 3:
+                    *prot |= PAGE_WRITE;
+                    /* fall through */
+                case 5:
+                case 6:
+                    *prot |= PAGE_READ | PAGE_EXEC;
+                    break;
+                default:
+                    qemu_log_mask(LOG_GUEST_ERROR,
+                                  "Bad value for AP bits in DRACR %"
+                                  PRIx32 "\n", ap);
+                }
+            }
+
+            /* execute never */
+            if (env->pmsav7.dracr[n] & (1 << 12)) {
+                *prot &= ~PAGE_EXEC;
+            }
+        }
+    }
+
+    *fsr = 0x00d; /* Permission fault */
+    return !(*prot & (1 << access_type));
+}
+
 static bool get_phys_addr_pmsav5(CPUARMState *env, uint32_t address,
                                  int access_type, ARMMMUIdx mmu_idx,
                                  hwaddr *phys_ptr, int *prot, uint32_t *fsr)

@@ -5912,7 +6072,7 @@ static bool get_phys_addr_pmsav5(CPUARMState *env, uint32_t address,
  * DFSR/IFSR fault register, with the following caveats:
  *  * we honour the short vs long DFSR format differences.
  *  * the WnR bit is never set (the caller must do this).
- *  * for MPU based systems we don't bother to return a full FSR format
+ *  * for PSMAv5 based systems we don't bother to return a full FSR format
  *    value.
  *
  * @env: CPUARMState

@@ -5960,6 +6120,16 @@ static inline bool get_phys_addr(CPUARMState *env, target_ulong address,
         }
     }
 
+    /* pmsav7 has special handling for when MPU is disabled so call it before
+     * the common MMU/MPU disabled check below.
+     */
+    if (arm_feature(env, ARM_FEATURE_MPU) &&
+        arm_feature(env, ARM_FEATURE_V7)) {
+        *page_size = TARGET_PAGE_SIZE;
+        return get_phys_addr_pmsav7(env, address, access_type, mmu_idx,
+                                    phys_ptr, prot, fsr);
+    }
+
     if (regime_translation_disabled(env, mmu_idx)) {
         /* MMU/MPU disabled.  */
         *phys_ptr = address;

@@ -5969,6 +6139,7 @@ static inline bool get_phys_addr(CPUARMState *env, target_ulong address,
     }
 
     if (arm_feature(env, ARM_FEATURE_MPU)) {
+        /* Pre-v7 MPU */
         *page_size = TARGET_PAGE_SIZE;
         return get_phys_addr_pmsav5(env, address, access_type, mmu_idx,
                                     phys_ptr, prot, fsr);
--

2.4.3.3.g905f831

author
#17 lore raw

PATCH v2 11/13 target-arm: Add support for Cortex-R5

In reply to Peter Crosthwaite

Introduce a CPU model for the Cortex R5 processor. ARMv7 with MPU, and both thumb and ARM div instructions.

Also implement dummy ATCM and BTCM. These CPs are defined for R5 but don't have a lot of meaning in QEMU yet. Raz them so the guest can proceed if they are read. The TCM registers will return a size of 0, indicating no TCM.

Signed-off-by Peter Crosthwaite <[email protected]>

---
Based loosely on an old patch of Andreas' for the cortex-r4. changed since v1:
Squashed in R5 specific CP regs patch
Reordered to be after supporting patches
set mp_is_up

 target-arm/cpu.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)
target-arm/cpu.c +38 -0
index 1ef2f03..b1c20c2 100644
@@ -825,6 +825,43 @@ static void arm_v7m_class_init(ObjectClass *oc, void *data)
     cc->cpu_exec_interrupt = arm_v7m_cpu_exec_interrupt;
 }
 
+static const ARMCPRegInfo cortexr5_cp_reginfo[] = {
+    /* Dummy the TCM region regs for the moment */
+    { .name = "ATCM", .cp = 15, .opc1 = 0, .crn = 9, .crm = 1, .opc2 = 0,
+      .access = PL1_RW, .type = ARM_CP_CONST },
+    { .name = "BTCM", .cp = 15, .opc1 = 0, .crn = 9, .crm = 1, .opc2 = 1,
+      .access = PL1_RW, .type = ARM_CP_CONST },
+    REGINFO_SENTINEL
+};
+
+static void cortex_r5_initfn(Object *obj)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+
+    set_feature(&cpu->env, ARM_FEATURE_V7);
+    set_feature(&cpu->env, ARM_FEATURE_THUMB_DIV);
+    set_feature(&cpu->env, ARM_FEATURE_ARM_DIV);
+    set_feature(&cpu->env, ARM_FEATURE_V7MP);
+    set_feature(&cpu->env, ARM_FEATURE_MPU);
+    cpu->midr = 0x411fc153; /* r1p3 */
+    cpu->id_pfr0 = 0x0131;
+    cpu->id_pfr1 = 0x001;
+    cpu->id_dfr0 = 0x010400;
+    cpu->id_afr0 = 0x0;
+    cpu->id_mmfr0 = 0x0210030;
+    cpu->id_mmfr1 = 0x00000000;
+    cpu->id_mmfr2 = 0x01200000;
+    cpu->id_mmfr3 = 0x0211;
+    cpu->id_isar0 = 0x2101111;
+    cpu->id_isar1 = 0x13112111;
+    cpu->id_isar2 = 0x21232141;
+    cpu->id_isar3 = 0x01112131;
+    cpu->id_isar4 = 0x0010142;
+    cpu->id_isar5 = 0x0;
+    cpu->mp_is_up = true;
+    define_arm_cp_regs(cpu, cortexr5_cp_reginfo);
+}
+
 static const ARMCPRegInfo cortexa8_cp_reginfo[] = {
     { .name = "L2LOCKDOWN", .cp = 15, .crn = 9, .crm = 0, .opc1 = 1, .opc2 = 0,
       .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },

@@ -1216,6 +1253,7 @@ static const ARMCPUInfo arm_cpus[] = {
     { .name = "arm11mpcore", .initfn = arm11mpcore_initfn },
     { .name = "cortex-m3",   .initfn = cortex_m3_initfn,
                              .class_init = arm_v7m_class_init },
+    { .name = "cortex-r5",   .initfn = cortex_r5_initfn },
     { .name = "cortex-a8",   .initfn = cortex_a8_initfn },
     { .name = "cortex-a9",   .initfn = cortex_a9_initfn },
     { .name = "cortex-a15",  .initfn = cortex_a15_initfn },
Signature · 1 line
2.4.3.3.g905f831
Reviewed-by on v2 11/13
#18 lore raw

Re: [Qemu-devel] [PATCH target-arm v2 11/13] target-arm: Add support for Cortex-R5

Quoted 2 lines · Peter Crosthwaite

Introduce a CPU model for the Cortex R5 processor. ARMv7 with MPU, and both thumb and ARM div instructions.

Reviewed-by Peter Maydell <[email protected]>

(checked all the ID values against the TRM.)

thanks
-- PMM

author
#19 lore raw

PATCH v2 12/13 arm: xlnx-zynqmp: Preface CPU variables with "apu"

In reply to Peter Crosthwaite

The CPUs currently supported by zynqmp are the APU (application processing unit) CPUs. There are other CPUs in Zynqmp so unqualified "cpus" in ambiguous. Preface the variables with "APU" accordingly, to prepare support adding the RPU (realtime processing unit) processors.

Signed-off-by Peter Crosthwaite <[email protected]>

---
changed since v1:
s/acpu/apu-cpu/

 hw/arm/xlnx-ep108.c          |  2 +-
 hw/arm/xlnx-zynqmp.c         | 26 ++++++++++++++------------
 include/hw/arm/xlnx-zynqmp.h |  4 ++--
 3 files changed, 17 insertions(+), 15 deletions(-)
hw/arm/xlnx-ep108.c +1 -1
index b924f5e..7a98dd6 100644
@@ -65,7 +65,7 @@ static void xlnx_ep108_init(MachineState *machine)
     xlnx_ep108_binfo.kernel_cmdline = machine->kernel_cmdline;
     xlnx_ep108_binfo.initrd_filename = machine->initrd_filename;
     xlnx_ep108_binfo.loader_start = 0;
-    arm_load_kernel(&s->soc.cpu[0], &xlnx_ep108_binfo);
+    arm_load_kernel(&s->soc.apu_cpu[0], &xlnx_ep108_binfo);
 }
 
 static QEMUMachine xlnx_ep108_machine = {
hw/arm/xlnx-zynqmp.c +14 -12
index 6b01965..353ecad 100644
@@ -64,10 +64,10 @@ static void xlnx_zynqmp_init(Object *obj)
     XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
     int i;
 
-    for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) {
-        object_initialize(&s->cpu[i], sizeof(s->cpu[i]),
+    for (i = 0; i < XLNX_ZYNQMP_NUM_APU_CPUS; i++) {
+        object_initialize(&s->apu_cpu[i], sizeof(s->apu_cpu[i]),
                           "cortex-a53-" TYPE_ARM_CPU);
-        object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpu[i]),
+        object_property_add_child(obj, "apu-cpu[*]", OBJECT(&s->apu_cpu[i]),
                                   &error_abort);
     }
 

@@ -95,7 +95,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
 
     qdev_prop_set_uint32(DEVICE(&s->gic), "num-irq", GIC_NUM_SPI_INTR + 32);
     qdev_prop_set_uint32(DEVICE(&s->gic), "revision", 2);
-    qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", XLNX_ZYNQMP_NUM_CPUS);
+    qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", XLNX_ZYNQMP_NUM_APU_CPUS);
     object_property_set_bool(OBJECT(&s->gic), true, "realized", &err);
     if (err) {
         error_propagate((errp), (err));

@@ -121,38 +121,40 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
         }
     }
 
-    for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) {
+    for (i = 0; i < XLNX_ZYNQMP_NUM_APU_CPUS; i++) {
         qemu_irq irq;
 
-        object_property_set_int(OBJECT(&s->cpu[i]), QEMU_PSCI_CONDUIT_SMC,
+        object_property_set_int(OBJECT(&s->apu_cpu[i]), QEMU_PSCI_CONDUIT_SMC,
                                 "psci-conduit", &error_abort);
         if (i > 0) {
             /* Secondary CPUs start in PSCI powered-down state */
-            object_property_set_bool(OBJECT(&s->cpu[i]), true,
+            object_property_set_bool(OBJECT(&s->apu_cpu[i]), true,
                                      "start-powered-off", &error_abort);
         }
 
-        object_property_set_int(OBJECT(&s->cpu[i]), GIC_BASE_ADDR,
+        object_property_set_int(OBJECT(&s->apu_cpu[i]), GIC_BASE_ADDR,
                                 "reset-cbar", &err);
         if (err) {
             error_propagate((errp), (err));
             return;
         }
 
-        object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err);
+        object_property_set_bool(OBJECT(&s->apu_cpu[i]), true, "realized",
+                                 &err);
         if (err) {
             error_propagate((errp), (err));
             return;
         }
 
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i,
-                           qdev_get_gpio_in(DEVICE(&s->cpu[i]), ARM_CPU_IRQ));
+                           qdev_get_gpio_in(DEVICE(&s->apu_cpu[i]),
+                                            ARM_CPU_IRQ));
         irq = qdev_get_gpio_in(DEVICE(&s->gic),
                                arm_gic_ppi_index(i, ARM_PHYS_TIMER_PPI));
-        qdev_connect_gpio_out(DEVICE(&s->cpu[i]), 0, irq);
+        qdev_connect_gpio_out(DEVICE(&s->apu_cpu[i]), 0, irq);
         irq = qdev_get_gpio_in(DEVICE(&s->gic),
                                arm_gic_ppi_index(i, ARM_VIRT_TIMER_PPI));
-        qdev_connect_gpio_out(DEVICE(&s->cpu[i]), 1, irq);
+        qdev_connect_gpio_out(DEVICE(&s->apu_cpu[i]), 1, irq);
     }
 
     for (i = 0; i < GIC_NUM_SPI_INTR; i++) {
include/hw/arm/xlnx-zynqmp.h +2 -2
index 79c2b0b..d042df1 100644
@@ -27,7 +27,7 @@
 #define XLNX_ZYNQMP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
                                        TYPE_XLNX_ZYNQMP)
 
-#define XLNX_ZYNQMP_NUM_CPUS 4
+#define XLNX_ZYNQMP_NUM_APU_CPUS 4
 #define XLNX_ZYNQMP_NUM_GEMS 4
 #define XLNX_ZYNQMP_NUM_UARTS 2
 

@@ -47,7 +47,7 @@ typedef struct XlnxZynqMPState {
     DeviceState parent_obj;
 
     /*< public >*/
-    ARMCPU cpu[XLNX_ZYNQMP_NUM_CPUS];
+    ARMCPU apu_cpu[XLNX_ZYNQMP_NUM_APU_CPUS];
     GICState gic;
     MemoryRegion gic_mr[XLNX_ZYNQMP_GIC_REGIONS][XLNX_ZYNQMP_GIC_ALIASES];
     CadenceGEMState gem[XLNX_ZYNQMP_NUM_GEMS];
Signature · 1 line
2.4.3.3.g905f831
author
#20 lore raw

PATCH v2 13/13 arm: xlnx-zynqmp: Add 2xCortexR5 CPUs

In reply to Peter Crosthwaite

Add the 2xCortexR5 CPUs to zynqmp board. They are powered off on reset (this is true of real hardware).

Signed-off-by Peter Crosthwaite <[email protected]>

---
changed since v1:
s/rcpu/rpu-cpu/

 hw/arm/xlnx-zynqmp.c         | 27 +++++++++++++++++++++++++++
 include/hw/arm/xlnx-zynqmp.h |  2 ++
 2 files changed, 29 insertions(+)
hw/arm/xlnx-zynqmp.c +27 -0
index 353ecad..e69d83f 100644
@@ -71,6 +71,13 @@ static void xlnx_zynqmp_init(Object *obj)
                                   &error_abort);
     }
 
+    for (i = 0; i < XLNX_ZYNQMP_NUM_RPU_CPUS; i++) {
+        object_initialize(&s->rpu_cpu[i], sizeof(s->rpu_cpu[i]),
+                          "cortex-r5-" TYPE_ARM_CPU);
+        object_property_add_child(obj, "rpu-cpu[*]", OBJECT(&s->rpu_cpu[i]),
+                                  &error_abort);
+    }
+
     object_initialize(&s->gic, sizeof(s->gic), TYPE_ARM_GIC);
     qdev_set_parent_bus(DEVICE(&s->gic), sysbus_get_default());
 

@@ -157,6 +164,26 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
         qdev_connect_gpio_out(DEVICE(&s->apu_cpu[i]), 1, irq);
     }
 
+    for (i = 0; i < XLNX_ZYNQMP_NUM_RPU_CPUS; i++) {
+        /* RPU_CPUs and held in reset on startup, by the reset controller */
+        object_property_set_bool(OBJECT(&s->rpu_cpu[i]), true,
+                                 "start-powered-off", &error_abort);
+
+        object_property_set_bool(OBJECT(&s->rpu_cpu[i]), true, "reset-hivecs",
+                                 &err);
+        if (err != NULL) {
+            error_propagate(errp, err);
+            return;
+        }
+
+        object_property_set_bool(OBJECT(&s->rpu_cpu[i]), true, "realized",
+                                 &err);
+        if (err) {
+            error_propagate((errp), (err));
+            return;
+        }
+    }
+
     for (i = 0; i < GIC_NUM_SPI_INTR; i++) {
         gic_spi[i] = qdev_get_gpio_in(DEVICE(&s->gic), i);
     }
include/hw/arm/xlnx-zynqmp.h +2 -0
index d042df1..3c58b68 100644
@@ -28,6 +28,7 @@
                                        TYPE_XLNX_ZYNQMP)
 
 #define XLNX_ZYNQMP_NUM_APU_CPUS 4
+#define XLNX_ZYNQMP_NUM_RPU_CPUS 2
 #define XLNX_ZYNQMP_NUM_GEMS 4
 #define XLNX_ZYNQMP_NUM_UARTS 2
 

@@ -48,6 +49,7 @@ typedef struct XlnxZynqMPState {
 
     /*< public >*/
     ARMCPU apu_cpu[XLNX_ZYNQMP_NUM_APU_CPUS];
+    ARMCPU rpu_cpu[XLNX_ZYNQMP_NUM_RPU_CPUS];
     GICState gic;
     MemoryRegion gic_mr[XLNX_ZYNQMP_GIC_REGIONS][XLNX_ZYNQMP_GIC_ALIASES];
     CadenceGEMState gem[XLNX_ZYNQMP_NUM_GEMS];
Signature · 1 line
2.4.3.3.g905f831
#21 lore raw
In reply to Peter Crosthwaite
Quoted 5 lines · Peter Crosthwaite

Hi Peter and all,

This patch series adds ARM Cortex R5 processor support. The PMSAv7 MPU is implemented. Two R5s are added to the Xilinx ZynqMP SoC.

Quoted 14 lines

Peter Crosthwaite (13):
arm: Do not define TLBTR in PMSA systems
arm: Don't add v7mp registers in MPU systems
arm: helper: Factor out CP regs common to [pv]msa
arm: Refactor get_phys_addr FSR return mechanism
arm: Implement uniprocessor with MP config
arm: Add has-mpu property target-arm/helper.c: define MPUIR register
arm: helper: rename get_phys_addr_mpu
target-arm: Add registers for PMSAv7
target-arm: Implement PMSAv7 MPU
target-arm: Add support for Cortex-R5
arm: xlnx-zynqmp: Preface CPU variables with "apu"
arm: xlnx-zynqmp: Add 2xCortexR5 CPUs

Patches 1-6 and 8 all look good to me, so I've added them to target-arm.next in the interest of reducing the size of this series for the next round.

I'll send review comments on 9..13 shortly.

-- PMM