[PATCH v2 0/9] Add Cortex-R52

[PATCH v2 0/9] Add Cortex-R52

[Tobias Roehmel]

v2:
PATCH 1:
I have left the flag in for now because there there is a lot of use for it in the MPU translation code.
The PMSAv8r differs in quite a view ways from the implementation in the Cortex-M. I think using
!ARM_FEATURE_M in all of those cases might run into problems down the road when new things are added.
But I'll gladly change that if those concerns are not valid.
PATCH 2:
Patch was moved and I removed the ATCM... registers.
PATCH 3:
The issue here is that the R52 has the MPUIR register which shares the encoding with one of the
MIDR alias registers. It's now changed to not add that register for ARM_FEATURE_V8_R.
PATCH 4:
Added RVBAR for all v8 CPUs instead of just ARMv8r
PATCH 7:
Instead of setting TTBCR_EAE to 1, change all functions that rely on that value and are
relevant for Cortex-R52
PATCH 10:
SPSR_hyp changes removed
PATCH 11:
Removed the r52_machine. The issue with adding the Cortex-R52 to the virt board is that target_page.bits
is expected to be 12 but is set to 10 for ARM_FEATURE_PMSA. Simply changing that or using
virt2.7(which doesn't have that restriction) leads to problems with memory accesses. It might be
best to model an existing board.

These patches add the ARM Cortex-R52. The biggest addition is an implementation of the armv8-r MPU.

All information is taken from:
- ARM Cortex-R52 TRM revision r1p3
- ARM Architecture Reference Manual Supplement
    -ARMv8 for the ARMv8-R AArch32 architecture profile Version A.c

Functionality that is not implemented:
- Changing between single and double precision floats
- Some hypervisor related functionality (HCR.T(R)VM,HADFSR,...)

Tobias Röhmel (9):
  target/arm: Add ARM_FEATURE_V8_R
  target/arm: Don't add all MIDR aliases for Cortex-R
  target/arm: Make RVBAR available for all ARMv8 CPUs
  target/arm: Make stage_2_format for cache attributes optional
  target/arm: Add ARMCacheAttrs to the signature of pmsav8_mpu_lookup
  target/arm: Enable TTBCR_EAE for ARMv8-R AArch32
  target/arm: Add PMSAv8r registers
  target/arm: Add PMSAv8r functionality
  target/arm: Add ARM Cortex-R52 cpu

 target/arm/cpu.c          |   6 +-
 target/arm/cpu.h          |  11 +++
 target/arm/cpu_tcg.c      |  42 +++++++++
 target/arm/debug_helper.c |   3 +-
 target/arm/helper.c       | 183 +++++++++++++++++++++++++++++++++++-
 target/arm/internals.h    |  16 ++--
 target/arm/m_helper.c     |   3 +-
 target/arm/ptw.c          | 191 ++++++++++++++++++++++++++++++++------
 target/arm/tlb_helper.c   |   3 +-
 9 files changed, 417 insertions(+), 41 deletions(-)

-- 
2.25.1

[PATCH v2 1/9] target/arm: Add ARM_FEATURE_V8_R

[Tobias Roehmel]

From: Tobias Röhmel <quic_trohmel@quicinc.com>

This flag is necessary to add features for the Cortex-R52.

Signed-off-by: Tobias Röhmel <quic_trohmel@quicinc.com>
---
 target/arm/cpu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index df677b2d5d..86e06116a9 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2287,6 +2287,7 @@ enum arm_features {
     ARM_FEATURE_M_SECURITY, /* M profile Security Extension */
     ARM_FEATURE_M_MAIN, /* M profile Main Extension */
     ARM_FEATURE_V8_1M, /* M profile extras only in v8.1M and later */
+    ARM_FEATURE_V8_R,
 };
 
 static inline int arm_feature(CPUARMState *env, int feature)
-- 
2.25.1

[PATCH v2 2/9] target/arm: Don't add all MIDR aliases for Cortex-R

[Tobias Roehmel]

From: Tobias Röhmel <quic_trohmel@quicinc.com>

Cortex-R52 has the MPUIR register which has the same encoding
has the MIDR alias with opc2=4. So we only add that alias
when we are not realizing a Cortex-R.

Signed-off-by: Tobias Röhmel <quic_trohmel@quicinc.com>
---
 target/arm/helper.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 6457e6301c..03bdc3d149 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -8189,9 +8189,6 @@ void register_cp_regs_for_features(ARMCPU *cpu)
               .fieldoffset = offsetof(CPUARMState, cp15.c0_cpuid),
               .readfn = midr_read },
             /* crn = 0 op1 = 0 crm = 0 op2 = 4,7 : AArch32 aliases of MIDR */
-            { .name = "MIDR", .type = ARM_CP_ALIAS | ARM_CP_CONST,
-              .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 4,
-              .access = PL1_R, .resetvalue = cpu->midr },
             { .name = "MIDR", .type = ARM_CP_ALIAS | ARM_CP_CONST,
               .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 7,
               .access = PL1_R, .resetvalue = cpu->midr },
@@ -8201,6 +8198,11 @@ void register_cp_regs_for_features(ARMCPU *cpu)
               .accessfn = access_aa64_tid1,
               .type = ARM_CP_CONST, .resetvalue = cpu->revidr },
         };
+        ARMCPRegInfo id_v8_midr_alias_cp_reginfo[] = {
+            { .name = "MIDR", .type = ARM_CP_ALIAS | ARM_CP_CONST,
+              .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 4,
+              .access = PL1_R, .resetvalue = cpu->midr },
+        };
         ARMCPRegInfo id_cp_reginfo[] = {
             /* These are common to v8 and pre-v8 */
             { .name = "CTR",
@@ -8264,8 +8266,12 @@ void register_cp_regs_for_features(ARMCPU *cpu)
             id_mpuir_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);
+            if (!arm_feature(env, ARM_FEATURE_V8_R)) {
+                define_arm_cp_regs(cpu, id_v8_midr_alias_cp_reginfo);
+            }
         } else {
             define_arm_cp_regs(cpu, id_pre_v8_midr_cp_reginfo);
         }
-- 
2.25.1

[PATCH v2 3/9] target/arm: Make RVBAR available for all ARMv8 CPUs

[Tobias Roehmel]

From: Tobias Röhmel <quic_trohmel@quicinc.com>

Signed-off-by: Tobias Röhmel <quic_trohmel@quicinc.com>
---
 target/arm/cpu.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 1b5d535788..9007768418 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -258,6 +258,10 @@ static void arm_cpu_reset(DeviceState *dev)
         env->cp15.cpacr_el1 = FIELD_DP64(env->cp15.cpacr_el1,
                                          CPACR, CP11, 3);
 #endif
+        if (arm_feature(env, ARM_FEATURE_V8)) {
+            env->cp15.rvbar = cpu->rvbar_prop;
+            env->regs[15] = cpu->rvbar_prop;
+        }
     }
 
 #if defined(CONFIG_USER_ONLY)
@@ -1273,7 +1277,7 @@ void arm_cpu_post_init(Object *obj)
         qdev_property_add_static(DEVICE(obj), &arm_cpu_reset_hivecs_property);
     }
 
-    if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
+    if (arm_feature(&cpu->env, ARM_FEATURE_V8)) {
         object_property_add_uint64_ptr(obj, "rvbar",
                                        &cpu->rvbar_prop,
                                        OBJ_PROP_FLAG_READWRITE);
-- 
2.25.1

[PATCH v2 4/9] target/arm: Make stage_2_format for cache attributes optional

[Tobias Roehmel]

From: Tobias Röhmel <quic_trohmel@quicinc.com>

The Cortex-R52 has a 2 stage MPU translation process but doesn't have the FEAT_S2FWB feature. This makes it neccessary to allow for the old cache attribut combination.
This is facilitated by changing the control path of combine_cacheattrs instead of failing if the second cache attributes struct is not in that format.

Signed-off-by: Tobias Röhmel <quic_trohmel@quicinc.com>
---
 target/arm/ptw.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 4d97a24808..8b037c1f55 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -2108,7 +2108,11 @@ static uint8_t combined_attrs_nofwb(CPUARMState *env,
 {
     uint8_t s1lo, s2lo, s1hi, s2hi, s2_mair_attrs, ret_attrs;
 
-    s2_mair_attrs = convert_stage2_attrs(env, s2.attrs);
+    if (s2.is_s2_format) {
+        s2_mair_attrs = convert_stage2_attrs(env, s2.attrs);
+    } else {
+        s2_mair_attrs = s2.attrs;
+    }
 
     s1lo = extract32(s1.attrs, 0, 4);
     s2lo = extract32(s2_mair_attrs, 0, 4);
@@ -2166,6 +2170,8 @@ static uint8_t force_cacheattr_nibble_wb(uint8_t attr)
 static uint8_t combined_attrs_fwb(CPUARMState *env,
                                   ARMCacheAttrs s1, ARMCacheAttrs s2)
 {
+    assert(s2.is_s2_format && !s1.is_s2_format);
+
     switch (s2.attrs) {
     case 7:
         /* Use stage 1 attributes */
@@ -2215,7 +2221,6 @@ static ARMCacheAttrs combine_cacheattrs(CPUARMState *env,
     ARMCacheAttrs ret;
     bool tagged = false;
 
-    assert(s2.is_s2_format && !s1.is_s2_format);
     ret.is_s2_format = false;
 
     if (s1.attrs == 0xf0) {
-- 
2.25.1

[PATCH v2 5/9] target/arm: Add ARMCacheAttrs to the signature of pmsav8_mpu_lookup

[Tobias Roehmel]

From: Tobias Röhmel <quic_trohmel@quicinc.com>

Add ARMCacheAttrs to the signature of pmsav8_mpu_lookup to prepare for the Cortex-R52 MPU which uses and combines cache attributes of different translation levels.

Signed-off-by: Tobias Röhmel <quic_trohmel@quicinc.com>
---
 target/arm/internals.h | 13 +++++++------
 target/arm/m_helper.c  |  3 ++-
 target/arm/ptw.c       | 11 +++++++----
 3 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/target/arm/internals.h b/target/arm/internals.h
index 6f94f3019d..b03049d920 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1109,12 +1109,6 @@ void v8m_security_lookup(CPUARMState *env, uint32_t address,
                          MMUAccessType access_type, ARMMMUIdx mmu_idx,
                          V8M_SAttributes *sattrs);
 
-bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address,
-                       MMUAccessType access_type, ARMMMUIdx mmu_idx,
-                       hwaddr *phys_ptr, MemTxAttrs *txattrs,
-                       int *prot, bool *is_subpage,
-                       ARMMMUFaultInfo *fi, uint32_t *mregion);
-
 /* Cacheability and shareability attributes for a memory access */
 typedef struct ARMCacheAttrs {
     /*
@@ -1126,6 +1120,13 @@ typedef struct ARMCacheAttrs {
     bool is_s2_format:1;
 } ARMCacheAttrs;
 
+bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address,
+                       MMUAccessType access_type, ARMMMUIdx mmu_idx,
+                       hwaddr *phys_ptr, MemTxAttrs *txattrs,
+                       int *prot, bool *is_subpage,
+                       ARMMMUFaultInfo *fi, uint32_t *mregion,
+                       ARMCacheAttrs *cacheattrs);
+
 bool get_phys_addr(CPUARMState *env, target_ulong address,
                    MMUAccessType access_type, ARMMMUIdx mmu_idx,
                    hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot,
diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c
index a740c3e160..44c80d733a 100644
--- a/target/arm/m_helper.c
+++ b/target/arm/m_helper.c
@@ -2829,10 +2829,11 @@ uint32_t HELPER(v7m_tt)(CPUARMState *env, uint32_t addr, uint32_t op)
      * inspecting the other MPU state.
      */
     if (arm_current_el(env) != 0 || alt) {
+        ARMCacheAttrs cacheattrs = {0};
         /* We can ignore the return value as prot is always set */
         pmsav8_mpu_lookup(env, addr, MMU_DATA_LOAD, mmu_idx,
                           &phys_addr, &attrs, &prot, &is_subpage,
-                          &fi, &mregion);
+                          &fi, &mregion, &cacheattrs);
         if (mregion == -1) {
             mrvalid = false;
             mregion = 0;
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 8b037c1f55..c4f5721012 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -1702,7 +1702,8 @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address,
                        MMUAccessType access_type, ARMMMUIdx mmu_idx,
                        hwaddr *phys_ptr, MemTxAttrs *txattrs,
                        int *prot, bool *is_subpage,
-                       ARMMMUFaultInfo *fi, uint32_t *mregion)
+                       ARMMMUFaultInfo *fi, uint32_t *mregion,
+                       ARMCacheAttrs *cacheattrs)
 {
     /*
      * Perform a PMSAv8 MPU lookup (without also doing the SAU check
@@ -1968,7 +1969,7 @@ static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address,
                                  MMUAccessType access_type, ARMMMUIdx mmu_idx,
                                  hwaddr *phys_ptr, MemTxAttrs *txattrs,
                                  int *prot, target_ulong *page_size,
-                                 ARMMMUFaultInfo *fi)
+                                 ARMMMUFaultInfo *fi, ARMCacheAttrs *cacheattrs)
 {
     uint32_t secure = regime_is_secure(env, mmu_idx);
     V8M_SAttributes sattrs = {};
@@ -2036,7 +2037,8 @@ static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address,
     }
 
     ret = pmsav8_mpu_lookup(env, address, access_type, mmu_idx, phys_ptr,
-                            txattrs, prot, &mpu_is_subpage, fi, NULL);
+                            txattrs, prot, &mpu_is_subpage, fi,
+                            NULL, cacheattrs);
     *page_size = sattrs.subpage || mpu_is_subpage ? 1 : TARGET_PAGE_SIZE;
     return ret;
 }
@@ -2416,7 +2418,8 @@ bool get_phys_addr(CPUARMState *env, target_ulong address,
         if (arm_feature(env, ARM_FEATURE_V8)) {
             /* PMSAv8 */
             ret = get_phys_addr_pmsav8(env, address, access_type, mmu_idx,
-                                       phys_ptr, attrs, prot, page_size, fi);
+                                       phys_ptr, attrs, prot, page_size,
+                                       fi, cacheattrs);
         } else if (arm_feature(env, ARM_FEATURE_V7)) {
             /* PMSAv7 */
             ret = get_phys_addr_pmsav7(env, address, access_type, mmu_idx,
-- 
2.25.1

[PATCH v2 6/9] target/arm: Enable TTBCR_EAE for ARMv8-R AArch32

[Tobias Roehmel]

From: Tobias Röhmel <quic_trohmel@quicinc.com>

ARMv8-R AArch32 CPUs behave as if TTBCR.EAE is always 1 even
tough they don't have the TTBCR register.
See ARM Architecture Reference Manual Supplement - ARMv8, for the ARMv8-R
AArch32 architecture profile Version:A.c section C1.2.

Signed-off-by: Tobias Röhmel <quic_trohmel@quicinc.com>
---
 target/arm/debug_helper.c | 3 ++-
 target/arm/internals.h    | 3 ++-
 target/arm/tlb_helper.c   | 3 ++-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index b18a6bd3a2..44b1e32974 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -434,7 +434,8 @@ static uint32_t arm_debug_exception_fsr(CPUARMState *env)
         using_lpae = true;
     } else {
         if (arm_feature(env, ARM_FEATURE_LPAE) &&
-            (env->cp15.tcr_el[target_el].raw_tcr & TTBCR_EAE)) {
+            ((env->cp15.tcr_el[target_el].raw_tcr & TTBCR_EAE)
+            || arm_feature(env, ARM_FEATURE_V8_R))) {
             using_lpae = true;
         }
     }
diff --git a/target/arm/internals.h b/target/arm/internals.h
index b03049d920..e2a2b03d41 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -254,7 +254,8 @@ static inline bool extended_addresses_enabled(CPUARMState *env)
 {
     TCR *tcr = &env->cp15.tcr_el[arm_is_secure(env) ? 3 : 1];
     return arm_el_is_aa64(env, 1) ||
-           (arm_feature(env, ARM_FEATURE_LPAE) && (tcr->raw_tcr & TTBCR_EAE));
+           (arm_feature(env, ARM_FEATURE_LPAE) && ((tcr->raw_tcr & TTBCR_EAE)
+           || arm_feature(env, ARM_FEATURE_V8_R)));
 }
 
 /* Update a QEMU watchpoint based on the information the guest has set in the
diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c
index 7d8a86b3c4..891326edb8 100644
--- a/target/arm/tlb_helper.c
+++ b/target/arm/tlb_helper.c
@@ -20,7 +20,8 @@ bool regime_using_lpae_format(CPUARMState *env, ARMMMUIdx mmu_idx)
         return true;
     }
     if (arm_feature(env, ARM_FEATURE_LPAE)
-        && (regime_tcr(env, mmu_idx)->raw_tcr & TTBCR_EAE)) {
+        && ((regime_tcr(env, mmu_idx)->raw_tcr & TTBCR_EAE)
+        || arm_feature(env, ARM_FEATURE_V8_R))) {
         return true;
     }
     return false;
-- 
2.25.1

[PATCH v2 7/9] target/arm: Add PMSAv8r registers

[Tobias Roehmel]

From: Tobias Röhmel <quic_trohmel@quicinc.com>

Signed-off-by: Tobias Röhmel <quic_trohmel@quicinc.com>
---
 target/arm/cpu.h    |  10 +++
 target/arm/helper.c | 171 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 181 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 86e06116a9..632d0d13c6 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -726,8 +726,18 @@ typedef struct CPUArchState {
          */
         uint32_t *rbar[M_REG_NUM_BANKS];
         uint32_t *rlar[M_REG_NUM_BANKS];
+        uint32_t prbarn[255];
+        uint32_t prlarn[255];
+        uint32_t hprbarn[255];
+        uint32_t hprlarn[255];
         uint32_t mair0[M_REG_NUM_BANKS];
         uint32_t mair1[M_REG_NUM_BANKS];
+        uint32_t prbar;
+        uint32_t prlar;
+        uint32_t prselr;
+        uint32_t hprbar;
+        uint32_t hprlar;
+        uint32_t hprselr;
     } pmsav8;
 
     /* v8M SAU */
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 03bdc3d149..f9ed2bd5c3 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -7422,6 +7422,78 @@ static CPAccessResult access_joscr_jmcr(CPUARMState *env,
     return CP_ACCESS_OK;
 }
 
+static void prbar_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                          uint64_t value)
+{
+    env->pmsav8.prbarn[env->pmsav8.prselr] = value;
+}
+
+static void prlar_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                          uint64_t value)
+{
+    env->pmsav8.prlarn[env->pmsav8.prselr] = value;
+}
+
+static uint64_t prbar_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    return env->pmsav8.prbarn[env->pmsav8.prselr];
+}
+
+static uint64_t prlar_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    return env->pmsav8.prlarn[env->pmsav8.prselr];
+}
+
+static void hprbar_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                          uint64_t value)
+{
+    env->pmsav8.hprbarn[env->pmsav8.hprselr] = value;
+}
+
+static void hprlar_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                          uint64_t value)
+{
+    env->pmsav8.hprlarn[env->pmsav8.hprselr] = value;
+}
+
+static void hprenr_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                          uint64_t value)
+{
+    uint32_t n;
+    ARMCPU *cpu = env_archcpu(env);
+    for (n = 0; n < (int)cpu->pmsav7_dregion; ++n) {
+        if (value & (1 << n)) {
+            env->pmsav8.hprlarn[n] |= 0x1;
+        } else {
+            env->pmsav8.hprlarn[n] &= (~0x1);
+        }
+    }
+}
+
+static uint64_t hprbar_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    return env->pmsav8.hprbarn[env->pmsav8.hprselr];
+}
+
+static uint64_t hprlar_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    return env->pmsav8.hprlarn[env->pmsav8.hprselr];
+}
+
+static uint64_t hprenr_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    uint32_t n;
+    uint32_t result = 0x0;
+    ARMCPU *cpu = env_archcpu(env);
+
+    for (n = 0; n < (int)cpu->pmsav7_dregion; ++n) {
+        if (env->pmsav8.hprlarn[n] & 0x1) {
+            result |= (0x1 << n);
+        }
+    }
+    return result;
+}
+
 static const ARMCPRegInfo jazelle_regs[] = {
     { .name = "JIDR",
       .cp = 14, .crn = 0, .crm = 0, .opc1 = 7, .opc2 = 0,
@@ -8235,6 +8307,46 @@ void register_cp_regs_for_features(ARMCPU *cpu)
               .access = PL1_R, .type = ARM_CP_CONST,
               .resetvalue = cpu->pmsav7_dregion << 8
         };
+        /* PMSAv8-R registers*/
+        ARMCPRegInfo id_pmsav8_r_reginfo[] = {
+            { .name = "HMPUIR",
+              .cp = 15, .crn = 0, .crm = 0, .opc1 = 4, .opc2 = 4,
+              .access = PL2_R, .type = ARM_CP_CONST,
+              .resetvalue = cpu->pmsav7_dregion},
+             /* PMSAv8-R registers */
+            { .name = "PRBAR",
+              .cp = 15, .opc1 = 0, .crn = 6, .crm = 3, .opc2 = 0,
+              .access = PL1_RW, .resetvalue = 0,
+              .readfn = prbar_read, .writefn = prbar_write,
+              .fieldoffset = offsetof(CPUARMState, pmsav8.prbar)},
+            { .name = "PRLAR",
+              .cp = 15, .opc1 = 0, .crn = 6, .crm = 3, .opc2 = 1,
+              .access = PL1_RW, .resetvalue = 0,
+              .readfn = prlar_read, .writefn = prlar_write,
+              .fieldoffset = offsetof(CPUARMState, pmsav8.prlar)},
+            { .name = "PRSELR", .resetvalue = 0,
+              .cp = 15, .opc1 = 0, .crn = 6, .crm = 2, .opc2 = 1,
+              .access = PL1_RW, .accessfn = access_tvm_trvm,
+              .fieldoffset = offsetof(CPUARMState, pmsav8.prselr)},
+            { .name = "HPRBAR", .resetvalue = 0,
+              .readfn = hprbar_read, .writefn = hprbar_write,
+              .cp = 15, .opc1 = 4, .crn = 6, .crm = 3, .opc2 = 0,
+              .access = PL2_RW, .resetvalue = 0,
+              .fieldoffset = offsetof(CPUARMState, pmsav8.hprbar)},
+            { .name = "HPRLAR",
+              .cp = 15, .opc1 = 4, .crn = 6, .crm = 3, .opc2 = 1,
+              .access = PL2_RW, .resetvalue = 0,
+              .readfn = hprlar_read, .writefn = hprlar_write,
+              .fieldoffset = offsetof(CPUARMState, pmsav8.hprlar)},
+            { .name = "HPRSELR", .resetvalue = 0,
+              .cp = 15, .opc1 = 4, .crn = 6, .crm = 2, .opc2 = 1,
+              .access = PL2_RW, .accessfn = access_tvm_trvm,
+              .fieldoffset = offsetof(CPUARMState, pmsav8.hprselr)},
+            { .name = "HPRENR",
+              .cp = 15, .opc1 = 4, .crn = 6, .crm = 1, .opc2 = 1,
+              .access = PL2_RW, .resetvalue = 0,
+              .readfn = hprenr_read, .writefn = hprenr_write},
+        };
         static const ARMCPRegInfo crn0_wi_reginfo = {
             .name = "CRN0_WI", .cp = 15, .crn = 0, .crm = CP_ANY,
             .opc1 = CP_ANY, .opc2 = CP_ANY, .access = PL1_W,
@@ -8278,6 +8390,65 @@ void register_cp_regs_for_features(ARMCPU *cpu)
         define_arm_cp_regs(cpu, id_cp_reginfo);
         if (!arm_feature(env, ARM_FEATURE_PMSA)) {
             define_one_arm_cp_reg(cpu, &id_tlbtr_reginfo);
+        } else if (arm_feature(env, ARM_FEATURE_V8_R)) {
+            uint32_t i = 0;
+            char hprbar_string[] = "HPRBAR%u";
+            char hprlar_string[] = "HPRLAR%u";
+
+            char prbar_string[] = "PRBAR%u";
+            char prlar_string[] = "PRLAR%u";
+            char tmp_string[50];
+            define_one_arm_cp_reg(cpu, &id_mpuir_reginfo);
+            define_arm_cp_regs(cpu, id_pmsav8_r_reginfo);
+            for (i = 0; i < cpu->pmsav7_dregion; ++i) {
+                uint8_t crm = 0b1000 | ((i & 0b1110) >> 1);
+                uint8_t opc2 = (i & 0x1) << 2;
+
+                sprintf(tmp_string, hprbar_string, i);
+                ARMCPRegInfo tmp_hprbarn_reginfo = {
+                    .name = tmp_string,
+                    .cp = 15, .opc1 = 4, .crn = 6, .crm = crm, .opc2 = opc2,
+                    .access = PL2_RW, .resetvalue = 0,
+                    .accessfn = access_tvm_trvm,
+                    .fieldoffset = offsetof(CPUARMState, pmsav8.hprbarn)
+                    + i * sizeof(env->pmsav8.hprbarn[0])
+                };
+                define_one_arm_cp_reg(cpu, &tmp_hprbarn_reginfo);
+
+                sprintf(tmp_string, prbar_string, i);
+                ARMCPRegInfo tmp_prbarn_reginfo = {
+                    .name = tmp_string,
+                    .cp = 15, .opc1 = 0, .crn = 6, .crm = crm, .opc2 = opc2,
+                    .access = PL1_RW, .resetvalue = 0,
+                    .accessfn = access_tvm_trvm,
+                    .fieldoffset = offsetof(CPUARMState, pmsav8.prbarn)
+                    + i * sizeof(env->pmsav8.prbarn[0])
+                };
+                define_one_arm_cp_reg(cpu, &tmp_prbarn_reginfo);
+
+                opc2 = (i & 0x1) << 2 | 0x1;
+                sprintf(tmp_string, hprlar_string, i);
+                ARMCPRegInfo tmp_hprlarn_reginfo = {
+                    .name = tmp_string,
+                    .cp = 15, .opc1 = 4, .crn = 6, .crm = crm, .opc2 = opc2,
+                    .access = PL2_RW, .resetvalue = 0,
+                    .accessfn = access_tvm_trvm,
+                    .fieldoffset = offsetof(CPUARMState, pmsav8.hprlarn)
+                    + i * sizeof(env->pmsav8.hprlarn[0])
+                };
+                define_one_arm_cp_reg(cpu, &tmp_hprlarn_reginfo);
+
+                sprintf(tmp_string, prlar_string, i);
+                ARMCPRegInfo tmp_prlarn_reginfo = {
+                    .name = tmp_string,
+                    .cp = 15, .opc1 = 0, .crn = 6, .crm = crm, .opc2 = opc2,
+                    .access = PL1_RW, .resetvalue = 0,
+                    .accessfn = access_tvm_trvm,
+                    .fieldoffset = offsetof(CPUARMState, pmsav8.prlarn)
+                    + i * sizeof(env->pmsav8.prlarn[0])
+                };
+                define_one_arm_cp_reg(cpu, &tmp_prlarn_reginfo);
+            }
         } else if (arm_feature(env, ARM_FEATURE_V7)) {
             define_one_arm_cp_reg(cpu, &id_mpuir_reginfo);
         }
-- 
2.25.1

[PATCH v2 8/9] target/arm: Add PMSAv8r functionality

[Tobias Roehmel]

From: Tobias Röhmel <quic_trohmel@quicinc.com>

Add PMSAv8r translation that is used by the ARM Cortex-R52.

Signed-off-by: Tobias Röhmel <quic_trohmel@quicinc.com>
---
 target/arm/ptw.c | 171 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 150 insertions(+), 21 deletions(-)

diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index c4f5721012..c7e37c66d0 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -140,6 +140,9 @@ static bool regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx)
              */
             return true;
         }
+    } else if (arm_feature(env, ARM_FEATURE_V8_R)) {
+        return !(regime_sctlr(env, mmu_idx) & SCTLR_M) ||
+        (!(regime_el(env, mmu_idx) == 2) && arm_hcr_el2_eff(env) & HCR_TGE);
     }
 
     hcr_el2 = arm_hcr_el2_eff(env);
@@ -1504,6 +1507,8 @@ static bool pmsav7_use_background_region(ARMCPU *cpu, ARMMMUIdx mmu_idx,
     if (arm_feature(env, ARM_FEATURE_M)) {
         return env->v7m.mpu_ctrl[regime_is_secure(env, mmu_idx)]
             & R_V7M_MPU_CTRL_PRIVDEFENA_MASK;
+    } else if (arm_feature(env, ARM_FEATURE_V8_R)) {
+        return false;
     } else {
         return regime_sctlr(env, mmu_idx) & SCTLR_BR;
     }
@@ -1698,6 +1703,77 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
     return !(*prot & (1 << access_type));
 }
 
+static uint32_t *regime_rbar(CPUARMState *env, ARMMMUIdx mmu_idx,
+                             uint32_t secure)
+{
+    if (arm_feature(env, ARM_FEATURE_V8_R)) {
+        if (regime_el(env, mmu_idx) == 2) {
+            return env->pmsav8.hprbarn;
+        } else {
+            return env->pmsav8.prbarn;
+        }
+    } else {
+         return env->pmsav8.rbar[secure];
+    }
+}
+
+static uint32_t *regime_rlar(CPUARMState *env, ARMMMUIdx mmu_idx,
+                             uint32_t secure)
+{
+    if (arm_feature(env, ARM_FEATURE_V8_R)) {
+        if (regime_el(env, mmu_idx) == 2) {
+            return env->pmsav8.hprlarn;
+        } else {
+            return env->pmsav8.prlarn;
+        }
+    } else {
+        return env->pmsav8.rlar[secure];
+    }
+}
+
+static inline void get_phys_addr_pmsav8_default(CPUARMState *env,
+                                                ARMMMUIdx mmu_idx,
+                                                uint32_t address, int *prot)
+{
+    if (arm_feature(env, ARM_FEATURE_V8_R)) {
+        *prot = PAGE_READ | PAGE_WRITE;
+        if (address <= 0x7FFFFFFF) {
+            *prot |= PAGE_EXEC;
+        }
+        if ((regime_el(env, mmu_idx) == 2)
+            && (regime_sctlr(env, mmu_idx) & SCTLR_WXN)
+            && (regime_sctlr(env, mmu_idx) & SCTLR_M)) {
+            *prot &= ~PAGE_EXEC;
+        }
+    } else {
+        get_phys_addr_pmsav7_default(env, mmu_idx, address, prot);
+    }
+}
+
+static bool pmsav8_fault(bool hit, CPUARMState *env, ARMMMUIdx mmu_idx)
+{
+    if (arm_feature(env, ARM_FEATURE_V8_R)) {
+        if (regime_el(env, mmu_idx) == 2) {
+            if (!hit && (mmu_idx != ARMMMUIdx_E2)) {
+                return true;
+            } else if (!hit && (mmu_idx == ARMMMUIdx_E2)
+                       &&!(regime_sctlr(env, mmu_idx) & SCTLR_BR)) {
+                return true;
+            }
+        } else {
+            if (!hit && (mmu_idx != ARMMMUIdx_Stage1_E1)) {
+                return true;
+            } else if (!hit && (mmu_idx == ARMMMUIdx_Stage1_E1)
+                       &&!(regime_sctlr(env, mmu_idx) & SCTLR_BR)) {
+                return true;
+            }
+        }
+        return false;
+    } else {
+        return !hit;
+    }
+}
+
 bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address,
                        MMUAccessType access_type, ARMMMUIdx mmu_idx,
                        hwaddr *phys_ptr, MemTxAttrs *txattrs,
@@ -1730,6 +1806,12 @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address,
         *mregion = -1;
     }
 
+    if (arm_feature(env, ARM_FEATURE_V8_R)) {
+        if (mmu_idx == ARMMMUIdx_Stage2) {
+            fi->stage2 = true;
+        }
+    }
+
     /*
      * Unlike the ARM ARM pseudocode, we don't need to check whether this
      * was an exception vector read from the vector table (which is always
@@ -1746,17 +1828,26 @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address,
             hit = true;
         }
 
+        uint32_t bitmask;
+        if (arm_feature(env, ARM_FEATURE_V8_R)) {
+            bitmask = 0x3f;
+        } else {
+            bitmask = 0x1f;
+        }
+
+
         for (n = (int)cpu->pmsav7_dregion - 1; n >= 0; n--) {
             /* region search */
             /*
-             * Note that the base address is bits [31:5] from the register
-             * with bits [4:0] all zeroes, but the limit address is bits
-             * [31:5] from the register with bits [4:0] all ones.
+             * Note that the base address is bits [31:x] from the register
+             * with bits [x-1:0] all zeroes, but the limit address is bits
+             * [31:x] from the register with bits [x:0] all ones. Where x is
+             * 5 for Cortex-M and 6 for Cortex-R
              */
-            uint32_t base = env->pmsav8.rbar[secure][n] & ~0x1f;
-            uint32_t limit = env->pmsav8.rlar[secure][n] | 0x1f;
+            uint32_t base = regime_rbar(env, mmu_idx, secure)[n] & ~bitmask;
+            uint32_t limit = regime_rlar(env, mmu_idx, secure)[n] | bitmask;
 
-            if (!(env->pmsav8.rlar[secure][n] & 0x1)) {
+            if (!(regime_rlar(env, mmu_idx, secure)[n] & 0x1)) {
                 /* Region disabled */
                 continue;
             }
@@ -1799,22 +1890,25 @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address,
         }
     }
 
-    if (!hit) {
-        /* background fault */
-        fi->type = ARMFault_Background;
+    if (pmsav8_fault(hit, env, mmu_idx)) {
+        fi->type = ARMFault_Permission;
+        fi->level = 0;
         return true;
     }
 
     if (matchregion == -1) {
         /* hit using the background region */
-        get_phys_addr_pmsav7_default(env, mmu_idx, address, prot);
+        get_phys_addr_pmsav8_default(env, mmu_idx, address, prot);
     } else {
-        uint32_t ap = extract32(env->pmsav8.rbar[secure][matchregion], 1, 2);
-        uint32_t xn = extract32(env->pmsav8.rbar[secure][matchregion], 0, 1);
+        uint32_t ap = extract32(regime_rbar(env,
+                                mmu_idx, secure)[matchregion], 1, 2);
+        uint32_t xn = extract32(regime_rbar(env,
+                                mmu_idx, secure)[matchregion], 0, 1);
         bool pxn = false;
 
         if (arm_feature(env, ARM_FEATURE_V8_1M)) {
-            pxn = extract32(env->pmsav8.rlar[secure][matchregion], 4, 1);
+            pxn = extract32(regime_rlar(env,
+                            mmu_idx, secure)[matchregion], 4, 1);
         }
 
         if (m_is_system_region(env, address)) {
@@ -1822,14 +1916,42 @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t address,
             xn = 1;
         }
 
-        *prot = simple_ap_to_rw_prot(env, mmu_idx, ap);
+        if (arm_feature(env, ARM_FEATURE_V8_R)) {
+            if (regime_el(env, mmu_idx) == 2) {
+                *prot = simple_ap_to_rw_prot_is_user(ap,
+                                                mmu_idx != ARMMMUIdx_E2);
+            } else {
+                *prot = simple_ap_to_rw_prot_is_user(ap,
+                                                mmu_idx != ARMMMUIdx_Stage1_E1);
+            }
+
+            if (regime_sctlr(env, mmu_idx) & SCTLR_WXN
+                && (*prot & PAGE_WRITE)) {
+                xn = 0x1;
+            }
+
+            if ((regime_el(env, mmu_idx) == 1) && regime_sctlr(env, mmu_idx)
+                 & SCTLR_UWXN && (ap == 0x1)) {
+                xn = 0x1;
+            }
+
+            uint8_t attrindx = extract32(regime_rlar(env,
+                                         mmu_idx, secure)[matchregion], 1, 3);
+            uint64_t mair = env->cp15.mair_el[regime_el(env, mmu_idx)];
+            uint8_t sh = extract32(regime_rlar(env,
+                                   mmu_idx, secure)[matchregion], 3, 2);
+            assert(attrindx <= 4);
+            cacheattrs->is_s2_format = false;
+            cacheattrs->attrs = extract64(mair, attrindx * 8, 8);
+            cacheattrs->shareability = sh;
+        } else {
+            *prot = simple_ap_to_rw_prot(env, mmu_idx, ap);
+        }
+
         if (*prot && !xn && !(pxn && !is_user)) {
             *prot |= PAGE_EXEC;
         }
-        /*
-         * We don't need to look the attribute up in the MAIR0/MAIR1
-         * registers because that only tells us about cacheability.
-         */
+
         if (mregion) {
             *mregion = matchregion;
         }
@@ -2342,9 +2464,16 @@ bool get_phys_addr(CPUARMState *env, target_ulong address,
             is_el0 = mmu_idx == ARMMMUIdx_E10_0 || mmu_idx == ARMMMUIdx_SE10_0;
 
             /* S1 is done. Now do S2 translation.  */
-            ret = get_phys_addr_lpae(env, ipa, access_type, s2_mmu_idx, is_el0,
-                                     phys_ptr, attrs, &s2_prot,
-                                     page_size, fi, &cacheattrs2);
+            if (arm_feature(env, ARM_FEATURE_V8_R)) {
+                ret = get_phys_addr_pmsav8(env, ipa, access_type, s2_mmu_idx,
+                                       phys_ptr, attrs, &s2_prot, page_size,
+                                       fi, &cacheattrs2);
+            } else {
+                ret = get_phys_addr_lpae(env, ipa, access_type, s2_mmu_idx,
+                                      is_el0, phys_ptr, attrs, &s2_prot,
+                                      page_size, fi, &cacheattrs2);
+            }
+
             fi->s2addr = ipa;
             /* Combine the S1 and S2 perms.  */
             *prot &= s2_prot;
-- 
2.25.1

[PATCH v2 9/9] target/arm: Add ARM Cortex-R52 cpu

[Tobias Roehmel]

From: Tobias Röhmel <quic_trohmel@quicinc.com>

All constants are taken from the ARM Cortex-R52 Processor TRM Revision: r1p3

Signed-off-by: Tobias Röhmel <quic_trohmel@quicinc.com>
---
 target/arm/cpu_tcg.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
index b751a19c8a..e0f445dc91 100644
--- a/target/arm/cpu_tcg.c
+++ b/target/arm/cpu_tcg.c
@@ -843,6 +843,47 @@ static void cortex_r5_initfn(Object *obj)
     define_arm_cp_regs(cpu, cortexr5_cp_reginfo);
 }
 
+static void cortex_r52_initfn(Object *obj)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+
+    set_feature(&cpu->env, ARM_FEATURE_V8);
+    set_feature(&cpu->env, ARM_FEATURE_V8_R);
+    set_feature(&cpu->env, ARM_FEATURE_EL2);
+    set_feature(&cpu->env, ARM_FEATURE_PMSA);
+    set_feature(&cpu->env, ARM_FEATURE_NEON);
+    set_feature(&cpu->env, ARM_FEATURE_GENERIC_TIMER);
+    cpu->midr = 0x411fd133; /* r1p3 */
+    cpu->revidr = 0x00000000;
+    cpu->reset_fpsid = 0x41034023;
+    cpu->isar.mvfr0 = 0x10110222;
+    cpu->isar.mvfr1 = 0x12111111;
+    cpu->isar.mvfr2 = 0x00000043;
+    cpu->ctr = 0x8144c004;
+    cpu->reset_sctlr = 0x30c50838;
+    cpu->isar.id_pfr0 = 0x00000131;
+    cpu->isar.id_pfr1 = 0x10111001;
+    cpu->isar.id_dfr0 = 0x03010006;
+    cpu->id_afr0 = 0x00000000;
+    cpu->isar.id_mmfr0 = 0x00211040;
+    cpu->isar.id_mmfr1 = 0x40000000;
+    cpu->isar.id_mmfr2 = 0x01200000;
+    cpu->isar.id_mmfr3 = 0xf0102211;
+    cpu->isar.id_mmfr4 = 0x00000010;
+    cpu->isar.id_isar0 = 0x02101110;
+    cpu->isar.id_isar1 = 0x13112111;
+    cpu->isar.id_isar2 = 0x21232142;
+    cpu->isar.id_isar3 = 0x01112131;
+    cpu->isar.id_isar4 = 0x00010142;
+    cpu->isar.id_isar5 = 0x00010001;
+    cpu->isar.dbgdidr = 0x77168000;
+    cpu->clidr = (1 << 27) | (1 << 24) | 0x3;
+    cpu->ccsidr[0] = 0x700fe01a; /* 32KB L1 dcache */
+    cpu->ccsidr[1] = 0x201fe00a; /* 32KB L1 icache */
+
+    cpu->pmsav7_dregion = 16;
+}
+
 static void cortex_r5f_initfn(Object *obj)
 {
     ARMCPU *cpu = ARM_CPU(obj);
@@ -1149,6 +1190,7 @@ static const ARMCPUInfo arm_tcg_cpus[] = {
                              .class_init = arm_v7m_class_init },
     { .name = "cortex-r5",   .initfn = cortex_r5_initfn },
     { .name = "cortex-r5f",  .initfn = cortex_r5f_initfn },
+    { .name = "cortex-r52",  .initfn = cortex_r52_initfn },
     { .name = "ti925t",      .initfn = ti925t_initfn },
     { .name = "sa1100",      .initfn = sa1100_initfn },
     { .name = "sa1110",      .initfn = sa1110_initfn },
-- 
2.25.1

Re: [PATCH v2 0/9] Add Cortex-R52

[Tobias Roehmel]

On 18.07.22 13:54, Tobias Roehmel wrote:

> v2:
> PATCH 1:
> I have left the flag in for now because there there is a lot of use for it in the MPU translation code.
> The PMSAv8r differs in quite a view ways from the implementation in the Cortex-M. I think using
> !ARM_FEATURE_M in all of those cases might run into problems down the road when new things are added.
> But I'll gladly change that if those concerns are not valid.
> PATCH 2:
> Patch was moved and I removed the ATCM... registers.
> PATCH 3:
> The issue here is that the R52 has the MPUIR register which shares the encoding with one of the
> MIDR alias registers. It's now changed to not add that register for ARM_FEATURE_V8_R.
> PATCH 4:
> Added RVBAR for all v8 CPUs instead of just ARMv8r
> PATCH 7:
> Instead of setting TTBCR_EAE to 1, change all functions that rely on that value and are
> relevant for Cortex-R52
> PATCH 10:
> SPSR_hyp changes removed
> PATCH 11:
> Removed the r52_machine. The issue with adding the Cortex-R52 to the virt board is that target_page.bits
> is expected to be 12 but is set to 10 for ARM_FEATURE_PMSA. Simply changing that or using
> virt2.7(which doesn't have that restriction) leads to problems with memory accesses. It might be
> best to model an existing board.
>
> These patches add the ARM Cortex-R52. The biggest addition is an implementation of the armv8-r MPU.
>
> All information is taken from:
> - ARM Cortex-R52 TRM revision r1p3
> - ARM Architecture Reference Manual Supplement
>      -ARMv8 for the ARMv8-R AArch32 architecture profile Version A.c
>
> Functionality that is not implemented:
> - Changing between single and double precision floats
> - Some hypervisor related functionality (HCR.T(R)VM,HADFSR,...)
>
> Tobias Röhmel (9):
>    target/arm: Add ARM_FEATURE_V8_R
>    target/arm: Don't add all MIDR aliases for Cortex-R
>    target/arm: Make RVBAR available for all ARMv8 CPUs
>    target/arm: Make stage_2_format for cache attributes optional
>    target/arm: Add ARMCacheAttrs to the signature of pmsav8_mpu_lookup
>    target/arm: Enable TTBCR_EAE for ARMv8-R AArch32
>    target/arm: Add PMSAv8r registers
>    target/arm: Add PMSAv8r functionality
>    target/arm: Add ARM Cortex-R52 cpu
>
>   target/arm/cpu.c          |   6 +-
>   target/arm/cpu.h          |  11 +++
>   target/arm/cpu_tcg.c      |  42 +++++++++
>   target/arm/debug_helper.c |   3 +-
>   target/arm/helper.c       | 183 +++++++++++++++++++++++++++++++++++-
>   target/arm/internals.h    |  16 ++--
>   target/arm/m_helper.c     |   3 +-
>   target/arm/ptw.c          | 191 ++++++++++++++++++++++++++++++++------
>   target/arm/tlb_helper.c   |   3 +-
>   9 files changed, 417 insertions(+), 41 deletions(-)
>
ping

https://patchew.org/QEMU/20220718115433.802-1-quic._5Ftrohmel@quicinc.com/
https://lore.kernel.org/qemu-devel/20220718115433.802-1-quic_trohmel@quicinc.com/

I didn't reply to the previous review, because I did what was suggested 
and summarized it here.
I hope that did break the etiquette.

BRs

Tobias

Re: [PATCH v2 0/9] Add Cortex-R52

[Peter Maydell]

On Wed, 27 Jul 2022 at 12:43, Tobias Roehmel
<tobias.roehmel@rwth-aachen.de> wrote:
>
> On 18.07.22 13:54, Tobias Roehmel wrote:
> ping
>
> https://patchew.org/QEMU/20220718115433.802-1-quic._5Ftrohmel@quicinc.com/
> https://lore.kernel.org/qemu-devel/20220718115433.802-1-quic_trohmel@quicinc.com/
>
> I didn't reply to the previous review, because I did what was suggested
> and summarized it here.
> I hope that did break the etiquette.

That's fine. I missed this series, so thanks for the ping; I've
added it to my list to review. Note that because we're now in
freeze for the 7.1 release, I may need to prioritize for-7.1
work ahead of this, but I should get to it in the next week or so.

-- PMM

Re: [PATCH v2 2/9] target/arm: Don't add all MIDR aliases for Cortex-R

[Peter Maydell]

On Mon, 18 Jul 2022 at 12:54, Tobias Roehmel <quic_trohmel@quicinc.com> wrote:
>
> From: Tobias Röhmel <quic_trohmel@quicinc.com>
>
> Cortex-R52 has the MPUIR register which has the same encoding
> has the MIDR alias with opc2=4. So we only add that alias
> when we are not realizing a Cortex-R.
>
> Signed-off-by: Tobias Röhmel <quic_trohmel@quicinc.com>
> ---
>  target/arm/helper.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 6457e6301c..03bdc3d149 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -8189,9 +8189,6 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>                .fieldoffset = offsetof(CPUARMState, cp15.c0_cpuid),
>                .readfn = midr_read },
>              /* crn = 0 op1 = 0 crm = 0 op2 = 4,7 : AArch32 aliases of MIDR */

Moving the op2=4 register definition makes this comment out of date,
so you need to update it (ie remove the '4' here, and add a comment
similarly noting what id_v8_midr_alias_cp_reginfo[] is doing).

> -            { .name = "MIDR", .type = ARM_CP_ALIAS | ARM_CP_CONST,
> -              .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 4,
> -              .access = PL1_R, .resetvalue = cpu->midr },
>              { .name = "MIDR", .type = ARM_CP_ALIAS | ARM_CP_CONST,
>                .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 7,
>                .access = PL1_R, .resetvalue = cpu->midr },
> @@ -8201,6 +8198,11 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>                .accessfn = access_aa64_tid1,
>                .type = ARM_CP_CONST, .resetvalue = cpu->revidr },
>          };
> +        ARMCPRegInfo id_v8_midr_alias_cp_reginfo[] = {
> +            { .name = "MIDR", .type = ARM_CP_ALIAS | ARM_CP_CONST,
> +              .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 4,
> +              .access = PL1_R, .resetvalue = cpu->midr },
> +        };

If there's only one register, you don't need to define a
single-element array, you can use define_one_arm_cp_reg()
and pass it a single ARMCPRegInfo struct.

>          ARMCPRegInfo id_cp_reginfo[] = {
>              /* These are common to v8 and pre-v8 */
>              { .name = "CTR",
> @@ -8264,8 +8266,12 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>              id_mpuir_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);
> +            if (!arm_feature(env, ARM_FEATURE_V8_R)) {

You can use ARM_FEATURE_PMSA here.

> +                define_arm_cp_regs(cpu, id_v8_midr_alias_cp_reginfo);
> +            }
>          } else {
>              define_arm_cp_regs(cpu, id_pre_v8_midr_cp_reginfo);
>          }
> --

thanks
-- PMM

Re: [PATCH v2 3/9] target/arm: Make RVBAR available for all ARMv8 CPUs

[Peter Maydell]

(I've added your rwth-aachen.de address because the quicinc
one seems to be bouncing :-( )

On Mon, 18 Jul 2022 at 12:54, Tobias Roehmel <quic_trohmel@quicinc.com> wrote:
>
> From: Tobias Röhmel <quic_trohmel@quicinc.com>
>
> Signed-off-by: Tobias Röhmel <quic_trohmel@quicinc.com>

Having looked a bit more carefully at the architecture
manual, I think this is not complete. In particular
you don't actually define the AArch32 RVBAR register
(which needs to exist with permissions permitting only
EL2 access if EL2 is the highest EL, and with permissions
for EL1 access if EL1 is the highest EL, in the same way
the AArch64 RVBAR_EL2 and RVBAR_EL1 work (ie by adding
AArch32 alias registers where we define RVBAR_EL2 and
RVBAR_EL1).

(AArch32 has no equivalent of RVBAR_EL3; MVBAR is kind
of the same thing, but it's not required that it has the
actual reset address in it on reset. So we can ignore that
as we're architecturally compliant for A-profile, and for
the R52 we don't care about EL3.).

The commit message here could also usefully be expanded
to explain what we're doing and why it's the right thing.

> ---
>  target/arm/cpu.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 1b5d535788..9007768418 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -258,6 +258,10 @@ static void arm_cpu_reset(DeviceState *dev)
>          env->cp15.cpacr_el1 = FIELD_DP64(env->cp15.cpacr_el1,
>                                           CPACR, CP11, 3);
>  #endif
> +        if (arm_feature(env, ARM_FEATURE_V8)) {
> +            env->cp15.rvbar = cpu->rvbar_prop;
> +            env->regs[15] = cpu->rvbar_prop;
> +        }
>      }
>
>  #if defined(CONFIG_USER_ONLY)
> @@ -1273,7 +1277,7 @@ void arm_cpu_post_init(Object *obj)
>          qdev_property_add_static(DEVICE(obj), &arm_cpu_reset_hivecs_property);
>      }
>
> -    if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
> +    if (arm_feature(&cpu->env, ARM_FEATURE_V8)) {
>          object_property_add_uint64_ptr(obj, "rvbar",
>                                         &cpu->rvbar_prop,
>                                         OBJ_PROP_FLAG_READWRITE);

thanks
-- PMM