[PATCH v2] target/arm: Add cortex-m0+ support

Matthieu Castet posted 1 patch 1 week, 2 days ago
hw/intc/armv7m_nvic.c    | 51 ++++++++++++++++++++++++++++++++++++++--
target/arm/cpu.c         |  5 ++--
target/arm/ptw.c         | 14 +++++++++--
target/arm/tcg/cpu-v7m.c | 17 ++++++++++++++
4 files changed, 81 insertions(+), 6 deletions(-)
[PATCH v2] target/arm: Add cortex-m0+ support
Posted by Matthieu Castet 1 week, 2 days ago
Signed-off-by: Matthieu Castet<castet.matthieu@free.fr>
---
 hw/intc/armv7m_nvic.c    | 51 ++++++++++++++++++++++++++++++++++++++--
 target/arm/cpu.c         |  5 ++--
 target/arm/ptw.c         | 14 +++++++++--
 target/arm/tcg/cpu-v7m.c | 17 ++++++++++++++
 4 files changed, 81 insertions(+), 6 deletions(-)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 98f3cf59bc..41d2b98ee4 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -1386,7 +1386,7 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs)
         }
         return (cpu->env.pmsav7.drbar[region] & ~0x1f) | (region & 0xf);
     }
-    case 0xda0: /* MPU_RASR (v7M), MPU_RLAR (v8M) */
+    case 0xda0: /* MPU_RASR (v6M/v7M), MPU_RLAR (v8M) */
     case 0xda8: /* MPU_RASR_A1 (v7M), MPU_RLAR_A1 (v8M) */
     case 0xdb0: /* MPU_RASR_A2 (v7M), MPU_RLAR_A2 (v8M) */
     case 0xdb8: /* MPU_RASR_A3 (v7M), MPU_RLAR_A3 (v8M) */
@@ -1407,6 +1407,14 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs)
             }
             return cpu->env.pmsav8.rlar[attrs.secure][region];
         }
+        if (!arm_feature(&s->cpu->env, ARM_FEATURE_V7)) {
+            /*
+             * armv6-m do not support A alias
+             */
+            if (offset != 0xda0) {
+                goto bad_offset;
+            }
+        }
 
         if (region >= cpu->pmsav7_dregion) {
             return 0;
@@ -1876,6 +1884,21 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value,
             return;
         }
 
+        if (!arm_feature(&s->cpu->env, ARM_FEATURE_V7)) {
+            if (offset != 0xd9c) {
+                /*
+                 * armv6-m do not support A alias
+                 */
+                goto bad_offset;
+            }
+
+            /*
+             * armv6-m do not support region address with alignement
+             * less than 256. Force alignement.
+             */
+            value &= ~0xe0;
+        }
+
         if (value & (1 << 4)) {
             /* VALID bit means use the region number specified in this
              * value and also update MPU_RNR.REGION with that value.
@@ -1900,12 +1923,14 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value,
         tlb_flush(CPU(cpu));
         break;
     }
-    case 0xda0: /* MPU_RASR (v7M), MPU_RLAR (v8M) */
+    case 0xda0: /* MPU_RASR (v6M/v7M), MPU_RLAR (v8M) */
     case 0xda8: /* MPU_RASR_A1 (v7M), MPU_RLAR_A1 (v8M) */
     case 0xdb0: /* MPU_RASR_A2 (v7M), MPU_RLAR_A2 (v8M) */
     case 0xdb8: /* MPU_RASR_A3 (v7M), MPU_RLAR_A3 (v8M) */
     {
         int region = cpu->env.pmsav7.rnr[attrs.secure];
+        int rsize;
+        int rsize_min;
 
         if (arm_feature(&cpu->env, ARM_FEATURE_V8)) {
             /* PMSAv8M handling of the aliases is different from v7M:
@@ -1926,6 +1951,28 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value,
             return;
         }
 
+        if (!arm_feature(&s->cpu->env, ARM_FEATURE_V7)) {
+            if (offset != 0xda0) {
+                /*
+                 * armv6-m do not support A alias
+                 */
+                goto bad_offset;
+            }
+        }
+
+        rsize = extract32(value, 1, 5);
+        /*
+         * for armv6-m rsize >= 7 (min 256)
+         * for armv7-m rsize >= 4 (min 32)
+         */
+        rsize_min = arm_feature(&s->cpu->env, ARM_FEATURE_V7) ? 4 : 7;
+
+        if (rsize < rsize_min) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                    "MPU region size too small %d\n", rsize);
+            return;
+        }
+
         if (region >= cpu->pmsav7_dregion) {
             return;
         }
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 6938161b95..818c93ed2c 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -520,7 +520,8 @@ static void arm_cpu_reset_hold(Object *obj, ResetType type)
                            sizeof(*env->pmsav8.rlar[M_REG_S])
                            * cpu->pmsav7_dregion);
                 }
-            } else if (arm_feature(env, ARM_FEATURE_V7)) {
+            } else if (arm_feature(env, ARM_FEATURE_V7) ||
+                       arm_feature(env, ARM_FEATURE_M)) {
                 memset(env->pmsav7.drbar, 0,
                        sizeof(*env->pmsav7.drbar) * cpu->pmsav7_dregion);
                 memset(env->pmsav7.drsr, 0,
@@ -2471,7 +2472,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
     }
 
     if (arm_feature(env, ARM_FEATURE_PMSA) &&
-        arm_feature(env, ARM_FEATURE_V7)) {
+        (arm_feature(env, ARM_FEATURE_V7) || arm_feature(env, ARM_FEATURE_M))) {
         uint32_t nr = cpu->pmsav7_dregion;
 
         if (nr > 0xff) {
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 9849949508..4203a94d54 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -3517,8 +3517,18 @@ static bool get_phys_addr_nogpc(CPUARMState *env, S1Translate *ptw,
             /* PMSAv8 */
             ret = get_phys_addr_pmsav8(env, ptw, address, access_type,
                                        result, fi);
-        } else if (arm_feature(env, ARM_FEATURE_V7)) {
-            /* PMSAv7 */
+        } else if (arm_feature(env, ARM_FEATURE_V7) ||
+                   arm_feature(env, ARM_FEATURE_M)) {
+            /*
+             * PMSAv7 or armv6-m PMSAv6
+             *
+             * armv6-m PMSAv6 is mostly compatible with PMSAv7,
+             * main difference :
+             * - min region size is 256 instead of 32
+             * - TEX can be only 0 (Tex not used by qemu)
+             * - no alias register
+             * - HardFault instead of MemManage
+             */
             ret = get_phys_addr_pmsav7(env, ptw, address, access_type,
                                        result, fi);
         } else {
diff --git a/target/arm/tcg/cpu-v7m.c b/target/arm/tcg/cpu-v7m.c
index 58e54578d6..1570a2cf13 100644
--- a/target/arm/tcg/cpu-v7m.c
+++ b/target/arm/tcg/cpu-v7m.c
@@ -76,6 +76,21 @@ static void cortex_m0_initfn(Object *obj)
     cpu->isar.id_isar6 = 0x00000000;
 }
 
+static void cortex_m0p_initfn(Object *obj)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+
+    /*
+     * cortex-m0p is a cortex-m0 with
+     * vtor and mpu extension
+     */
+    cortex_m0_initfn(obj);
+
+    cpu->midr = 0x410cc601;
+    cpu->pmsav7_dregion = 8;
+}
+
+
 static void cortex_m3_initfn(Object *obj)
 {
     ARMCPU *cpu = ARM_CPU(obj);
@@ -267,6 +282,8 @@ static void arm_v7m_class_init(ObjectClass *oc, void *data)
 static const ARMCPUInfo arm_v7m_cpus[] = {
     { .name = "cortex-m0",   .initfn = cortex_m0_initfn,
                              .class_init = arm_v7m_class_init },
+    { .name = "cortex-m0p",  .initfn = cortex_m0p_initfn,
+                             .class_init = arm_v7m_class_init },
     { .name = "cortex-m3",   .initfn = cortex_m3_initfn,
                              .class_init = arm_v7m_class_init },
     { .name = "cortex-m4",   .initfn = cortex_m4_initfn,
-- 
2.39.5
Re: [PATCH v2] target/arm: Add cortex-m0+ support
Posted by Peter Maydell 1 week, 1 day ago
On Wed, 13 Nov 2024 at 17:36, Matthieu Castet <castet.matthieu@free.fr> wrote:
>
> Signed-off-by: Matthieu Castet<castet.matthieu@free.fr>

Hi; thanks for version 2; this looks pretty good and I just
have a couple of small nits.

Firstly, there's still no commit message body text here.
Commit messages should explain the intention of the commit
and roughly what the change is doing. If you look at
"git log" you'll see that subject-line-only commit messages
are the exception, not the norm (and are usually only for
pretty mechanical changes done as part of a whole series
that's doing the same kind of change).

> ---
>  hw/intc/armv7m_nvic.c    | 51 ++++++++++++++++++++++++++++++++++++++--
>  target/arm/cpu.c         |  5 ++--
>  target/arm/ptw.c         | 14 +++++++++--
>  target/arm/tcg/cpu-v7m.c | 17 ++++++++++++++
>  4 files changed, 81 insertions(+), 6 deletions(-)
>
> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
> index 98f3cf59bc..41d2b98ee4 100644
> --- a/hw/intc/armv7m_nvic.c
> +++ b/hw/intc/armv7m_nvic.c
> @@ -1386,7 +1386,7 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs)
>          }
>          return (cpu->env.pmsav7.drbar[region] & ~0x1f) | (region & 0xf);
>      }
> -    case 0xda0: /* MPU_RASR (v7M), MPU_RLAR (v8M) */
> +    case 0xda0: /* MPU_RASR (v6M/v7M), MPU_RLAR (v8M) */
>      case 0xda8: /* MPU_RASR_A1 (v7M), MPU_RLAR_A1 (v8M) */
>      case 0xdb0: /* MPU_RASR_A2 (v7M), MPU_RLAR_A2 (v8M) */
>      case 0xdb8: /* MPU_RASR_A3 (v7M), MPU_RLAR_A3 (v8M) */
> @@ -1407,6 +1407,14 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs)
>              }
>              return cpu->env.pmsav8.rlar[attrs.secure][region];
>          }
> +        if (!arm_feature(&s->cpu->env, ARM_FEATURE_V7)) {
> +            /*
> +             * armv6-m do not support A alias
> +             */
> +            if (offset != 0xda0) {
> +                goto bad_offset;
> +            }
> +        }

I think it would be neater to do the "alias registers not supported" as:

    case 0xda0: /* MPU_RASR (v7M), MPU_RLAR (v8M) */
    case 0xda8: /* MPU_RASR_A1 (v7M), MPU_RLAR_A1 (v8M) */
    case 0xdb0: /* MPU_RASR_A2 (v7M), MPU_RLAR_A2 (v8M) */
    case 0xdb8: /* MPU_RASR_A3 (v7M), MPU_RLAR_A3 (v8M) */
        if (!arm_feature(&s->cpu->env, ARM_FEATURE_V7)) {
            /* No alias registers in v6M */
            goto bad_offset;
        }
        /* fall through */
    case 0xda0: /* MPU_RASR (v7M), MPU_RLAR (v8M) */
        [code for MPU_RASR starts here]


You're also missing the "no RBAR aliases" handling in nvic_readl().

>          if (region >= cpu->pmsav7_dregion) {
>              return 0;
> @@ -1876,6 +1884,21 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value,
>              return;
>          }
>
> +        if (!arm_feature(&s->cpu->env, ARM_FEATURE_V7)) {
> +            if (offset != 0xd9c) {
> +                /*
> +                 * armv6-m do not support A alias
> +                 */
> +                goto bad_offset;
> +            }
> +
> +            /*
> +             * armv6-m do not support region address with alignement
> +             * less than 256. Force alignement.

Typos: "does not", "alignment".

> +             */
> +            value &= ~0xe0;
> +        }
> +
>          if (value & (1 << 4)) {
>              /* VALID bit means use the region number specified in this
>               * value and also update MPU_RNR.REGION with that value.

The rest of the patch looks good.

PS: there's no rush about producing a v3 -- we're currently in
freeze for the 9.2 release, so this won't be going into git
until we make the 9.2 release in late December.

thanks
-- PMM