diff mbox

[3/3] armv7-m: add MPU to cortex-m3 and cortex-m4

Message ID c0d3b1169ee079ef9a2bd00d62d6720c8367ae41.1444396783.git.mdavidsaver@gmail.com
State New
Headers show

Commit Message

Michael Davidsaver Oct. 9, 2015, 1:28 p.m. UTC 
The M series MPU is almost the same as the already
implemented R series MPU.  So use the M series
and translate as best we can.

The HFNMIENA bit in MPU_CTRL is not implemented.

Implement CFSR and MMFAR to report fault address
to MemManage handler.

Add MPU feature flag to cortex-m3 and -m4.
---
 hw/intc/armv7m_nvic.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++++--
 target-arm/cpu-qom.h  |   4 ++
 target-arm/cpu.c      |  14 +++++
 target-arm/helper.c   |   7 +++
 4 files changed, 174 insertions(+), 5 deletions(-)

Comments

Peter Crosthwaite Oct. 11, 2015, 3:23 p.m. UTC | #1 
On Fri, Oct 9, 2015 at 6:28 AM, Michael Davidsaver
<mdavidsaver@gmail.com> wrote:
> The M series MPU is almost the same as the already
> implemented R series MPU.  So use the M series
> and translate as best we can.
>

There is some work on list for this that never got a respin:

https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg01945.html

> The HFNMIENA bit in MPU_CTRL is not implemented.
>
> Implement CFSR and MMFAR to report fault address
> to MemManage handler.
>
> Add MPU feature flag to cortex-m3 and -m4.
> ---
>  hw/intc/armv7m_nvic.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  target-arm/cpu-qom.h  |   4 ++
>  target-arm/cpu.c      |  14 +++++
>  target-arm/helper.c   |   7 +++
>  4 files changed, 174 insertions(+), 5 deletions(-)
>
> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
> index a671d84..94011cf 100644
> --- a/hw/intc/armv7m_nvic.c
> +++ b/hw/intc/armv7m_nvic.c
> @@ -245,12 +245,11 @@ static uint32_t nvic_readl(nvic_state *s, uint32_t offset)
>          if (s->gic.irq_state[ARMV7M_EXCP_USAGE].enabled) val |= (1 << 18);
>          return val;
>      case 0xd28: /* Configurable Fault Status.  */
> -        /* TODO: Implement Fault Status.  */
> -        qemu_log_mask(LOG_UNIMP, "Configurable Fault Status unimplemented\n");
> -        return 0;
> +        return ARM_CPU(current_cpu)->pmsav7_cfsr;

You should avoid dereferenced inline QOM casts and create a local variable.

> +    case 0xd34: /* MMFAR MemManage Fault Address */
> +        return ARM_CPU(current_cpu)->pmsav7_mmfar;

Why reorder the addresses in the switch?

>      case 0xd2c: /* Hard Fault Status.  */
>      case 0xd30: /* Debug Fault Status.  */
> -    case 0xd34: /* Mem Manage Address.  */
>      case 0xd38: /* Bus Fault Address.  */
>      case 0xd3c: /* Aux Fault Status.  */
>          /* TODO: Implement fault status registers.  */
> @@ -283,6 +282,55 @@ static uint32_t nvic_readl(nvic_state *s, uint32_t offset)
>      case 0xd70: /* ISAR4.  */
>          return 0x01310102;
>      /* TODO: Implement debug registers.  */
> +    case 0xd90: /* MPU_TYPE */
> +        cpu = ARM_CPU(current_cpu);
> +        return cpu->has_mpu ? (cpu->pmsav7_dregion<<8) : 0;
> +        break;
> +    case 0xd94: /* MPU_CTRL */
> +        val = 0;
> +        cpu = ARM_CPU(current_cpu);
> +        if(cpu->env.cp15.sctlr_el[0] & SCTLR_M)
> +            val |= 1; /* ENABLE */
> +        /* HFNMIENA not implemented, see nvic_writel() */
> +        if(cpu->env.cp15.sctlr_el[0] & SCTLR_BR)
> +            val |= 4; /* PRIVDEFENA */
> +        return val;
> +    case 0xd98: /* MPU_RNR */
> +        return ARM_CPU(current_cpu)->env.cp15.c6_rgnr;
> +    case 0xd9c: /* MPU_RBAR */
> +    case 0xda4: /* MPU_RBAR_A1 */
> +    case 0xdaC: /* MPU_RBAR_A2 */
> +    case 0xdb4: /* MPU_RBAR_A3 */
> +    {
> +        uint32_t range;
> +        cpu = ARM_CPU(current_cpu);
> +        if(offset==0xd9c)

spaces around ==

> +            range = cpu->env.cp15.c6_rgnr;
> +        else
> +            range = (offset-0xda4)/8;
> +
> +        if(range>=cpu->pmsav7_dregion) return 0;

{} for if body, return on new line. If you run your patch through
scripts/checkpatch.pl it will detect some of these conventions.

> +
> +        return (cpu->env.pmsav7.drbar[range]&(0x1f)) | (range&0xf);

Spaces around &, parentheses around hex constant not needed.

> +    }
> +    case 0xda0: /* MPU_RASR */
> +    case 0xda8: /* MPU_RASR_A1 */
> +    case 0xdb0: /* MPU_RASR_A2 */
> +    case 0xdb8: /* MPU_RASR_A3 */
> +    {
> +        uint32_t range;
> +        cpu = ARM_CPU(current_cpu);
> +
> +        if(offset==0xda0)
> +            range = cpu->env.cp15.c6_rgnr;
> +        else
> +            range = (offset-0xda8)/8;
> +
> +        if(range>=cpu->pmsav7_dregion) return 0;
> +
> +        return ((cpu->env.pmsav7.dracr[range]&0xffff)<<16)
> +                | (cpu->env.pmsav7.drsr[range]&0xffff);
> +    }

More style nits here.

Regards,
Peter

>      default:
>          qemu_log_mask(LOG_GUEST_ERROR, "NVIC: Bad read offset 0x%x\n", offset);
>          return 0;
> @@ -376,14 +424,110 @@ static void nvic_writel(nvic_state *s, uint32_t offset, uint32_t value)
>          s->gic.irq_state[ARMV7M_EXCP_USAGE].enabled = (value & (1 << 18)) != 0;
>          break;
Michael Davidsaver Oct. 12, 2015, 3:51 a.m. UTC | #2 
On 10/11/2015 11:23 AM, Peter Crosthwaite wrote:
> On Fri, Oct 9, 2015 at 6:28 AM, Michael Davidsaver
> <mdavidsaver@gmail.com> wrote:
>> The M series MPU is almost the same as the already
>> implemented R series MPU.  So use the M series
>> and translate as best we can.
>>
> There is some work on list for this that never got a respin:
>
> https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg01945.html

Well, I totally missed that.  I'll have look.

> ...
>> +    case 0xd34: /* MMFAR MemManage Fault Address */
>> +        return ARM_CPU(current_cpu)->pmsav7_mmfar;
> Why reorder the addresses in the switch?

I was thinking to avoid duplicating the qemu_log_mask() for the
unimplemented registers.  I take it that this to you is not the lesser
evil :)

> ... If you run your patch through scripts/checkpatch.pl it will detect
> some of these conventions. 

Will do.

> ... More style nits here....

All noted.

> Regards, Peter 

Michael
diff mbox

Patch

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index a671d84..94011cf 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -245,12 +245,11 @@  static uint32_t nvic_readl(nvic_state *s, uint32_t offset)
         if (s->gic.irq_state[ARMV7M_EXCP_USAGE].enabled) val |= (1 << 18);
         return val;
     case 0xd28: /* Configurable Fault Status.  */
-        /* TODO: Implement Fault Status.  */
-        qemu_log_mask(LOG_UNIMP, "Configurable Fault Status unimplemented\n");
-        return 0;
+        return ARM_CPU(current_cpu)->pmsav7_cfsr;
+    case 0xd34: /* MMFAR MemManage Fault Address */
+        return ARM_CPU(current_cpu)->pmsav7_mmfar;
     case 0xd2c: /* Hard Fault Status.  */
     case 0xd30: /* Debug Fault Status.  */
-    case 0xd34: /* Mem Manage Address.  */
     case 0xd38: /* Bus Fault Address.  */
     case 0xd3c: /* Aux Fault Status.  */
         /* TODO: Implement fault status registers.  */
@@ -283,6 +282,55 @@  static uint32_t nvic_readl(nvic_state *s, uint32_t offset)
     case 0xd70: /* ISAR4.  */
         return 0x01310102;
     /* TODO: Implement debug registers.  */
+    case 0xd90: /* MPU_TYPE */
+        cpu = ARM_CPU(current_cpu);
+        return cpu->has_mpu ? (cpu->pmsav7_dregion<<8) : 0;
+        break;
+    case 0xd94: /* MPU_CTRL */
+        val = 0;
+        cpu = ARM_CPU(current_cpu);
+        if(cpu->env.cp15.sctlr_el[0] & SCTLR_M)
+            val |= 1; /* ENABLE */
+        /* HFNMIENA not implemented, see nvic_writel() */
+        if(cpu->env.cp15.sctlr_el[0] & SCTLR_BR)
+            val |= 4; /* PRIVDEFENA */
+        return val;
+    case 0xd98: /* MPU_RNR */
+        return ARM_CPU(current_cpu)->env.cp15.c6_rgnr;
+    case 0xd9c: /* MPU_RBAR */
+    case 0xda4: /* MPU_RBAR_A1 */
+    case 0xdaC: /* MPU_RBAR_A2 */
+    case 0xdb4: /* MPU_RBAR_A3 */
+    {
+        uint32_t range;
+        cpu = ARM_CPU(current_cpu);
+        if(offset==0xd9c)
+            range = cpu->env.cp15.c6_rgnr;
+        else
+            range = (offset-0xda4)/8;
+
+        if(range>=cpu->pmsav7_dregion) return 0;
+
+        return (cpu->env.pmsav7.drbar[range]&(0x1f)) | (range&0xf);
+    }
+    case 0xda0: /* MPU_RASR */
+    case 0xda8: /* MPU_RASR_A1 */
+    case 0xdb0: /* MPU_RASR_A2 */
+    case 0xdb8: /* MPU_RASR_A3 */
+    {
+        uint32_t range;
+        cpu = ARM_CPU(current_cpu);
+
+        if(offset==0xda0)
+            range = cpu->env.cp15.c6_rgnr;
+        else
+            range = (offset-0xda8)/8;
+
+        if(range>=cpu->pmsav7_dregion) return 0;
+
+        return ((cpu->env.pmsav7.dracr[range]&0xffff)<<16)
+                | (cpu->env.pmsav7.drsr[range]&0xffff);
+    }
     default:
         qemu_log_mask(LOG_GUEST_ERROR, "NVIC: Bad read offset 0x%x\n", offset);
         return 0;
@@ -376,14 +424,110 @@  static void nvic_writel(nvic_state *s, uint32_t offset, uint32_t value)
         s->gic.irq_state[ARMV7M_EXCP_USAGE].enabled = (value & (1 << 18)) != 0;
         break;
     case 0xd28: /* Configurable Fault Status.  */
+    case 0xd34: /* Mem Manage Address.  */
+        return;
     case 0xd2c: /* Hard Fault Status.  */
     case 0xd30: /* Debug Fault Status.  */
-    case 0xd34: /* Mem Manage Address.  */
     case 0xd38: /* Bus Fault Address.  */
     case 0xd3c: /* Aux Fault Status.  */
         qemu_log_mask(LOG_UNIMP,
                       "NVIC: fault status registers unimplemented\n");
         break;
+    case 0xd90: /* MPU_TYPE (0xe000ed90) */
+        return; /* RO */
+    case 0xd94: /* MPU_CTRL */
+    {
+        unsigned q;
+        uint32_t tset=0, tclear=0;
+        if(value&1) {
+            tset |= SCTLR_M;
+        } else {
+            tclear |= SCTLR_M;
+        }
+        if(value&2) {
+            /* The M series MPU is almost functionally the same
+             * as the R series MPU, so we translate to the R series
+             * register format.
+             *
+             * One difference is that the R series MPU doesn't implement
+             * HFNMIENA (bypass MPU in some exception handlers).
+             */
+            qemu_log_mask(LOG_UNIMP, "M profile does not implement HFNMIENA bit in MPU_CTRL\n");
+        }
+        if(value&4) {
+            tset |= SCTLR_BR;
+        } else {
+            tclear |= SCTLR_BR;
+        }
+        cpu = ARM_CPU(current_cpu);
+        /* TODO, which sctlr(s) really need to be set? */
+        for(q=0; q<4; q++) {
+            cpu->env.cp15.sctlr_el[q] |= tset;
+            cpu->env.cp15.sctlr_el[q] &= ~tclear;
+        }
+        tlb_flush(CPU(cpu), 1);
+    }
+        break;
+    case 0xd98: /* MPU_RNR */
+        cpu = ARM_CPU(current_cpu);
+        if(value>=cpu->pmsav7_dregion) {
+            qemu_log_mask(LOG_GUEST_ERROR, "MPU region out of range %u/%u\n",
+                          (unsigned)value, (unsigned)cpu->pmsav7_dregion);
+        } else {
+            cpu->env.cp15.c6_rgnr = value;
+        }
+        tlb_flush(CPU(cpu), 1); /* necessary? */
+        break;
+    case 0xd9c: /* MPU_RBAR */
+    case 0xda4: /* MPU_RBAR_A1 */
+    case 0xdac: /* MPU_RBAR_A2 */
+    case 0xdb4: /* MPU_RBAR_A3 */
+    {
+        uint32_t range;
+        uint32_t base = value;
+        cpu = ARM_CPU(current_cpu);
+
+        if(offset==0xd9c)
+            range = cpu->env.cp15.c6_rgnr;
+        else
+            range = (offset-0xda4)/8;
+
+        if(value&(1<<4)) {
+            range = value&0xf;
+
+            if(range>=cpu->pmsav7_dregion) {
+                qemu_log_mask(LOG_GUEST_ERROR, "MPU region out of range %u/%u\n",
+                              (unsigned)range, (unsigned)cpu->pmsav7_dregion);
+                return;
+            }
+            cpu->env.cp15.c6_rgnr = range;
+            base &= ~0x1f;
+
+        } else if(range>=cpu->pmsav7_dregion)
+            return;
+
+        cpu->env.pmsav7.drbar[range] = base&~0x3;
+    }
+        tlb_flush(CPU(cpu), 1);
+        break;
+    case 0xda0: /* MPU_RASR */
+    case 0xda8: /* MPU_RASR_A1 */
+    case 0xdb0: /* MPU_RASR_A2 */
+    case 0xdb8: /* MPU_RASR_A3 */
+    {
+        uint32_t range;
+        cpu = ARM_CPU(current_cpu);
+
+        if(offset==0xda0)
+            range = cpu->env.cp15.c6_rgnr;
+        else
+            range = (offset-0xda8)/8;
+
+        cpu->env.pmsav7.drsr[range] = value&0xff3f;
+        cpu->env.pmsav7.dracr[range] = (value>>16)&0x173f;
+    }
+        tlb_flush(CPU(cpu), 1);
+        break;
     case 0xf00: /* Software Triggered Interrupt Register */
         if ((value & 0x1ff) < s->num_irq) {
             gic_set_pending_private(&s->gic, 0, value & 0x1ff);
diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index 25fb1ce..d75a496 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -107,6 +107,10 @@  typedef struct ARMCPU {
     bool has_mpu;
     /* PMSAv7 MPU number of supported regions */
     uint32_t pmsav7_dregion;
+    /* PMASv7 Fault status */
+    uint32_t pmsav7_cfsr;
+    /* PMASv7 Address of fault */
+    uint32_t pmsav7_mmfar;
 
     /* PSCI conduit used to invoke PSCI methods
      * 0 - disabled, 1 - smc, 2 - hvc
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index d7b4445..deb5878 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -516,6 +516,12 @@  static Property arm_cpu_has_mpu_property =
 static Property arm_cpu_pmsav7_dregion_property =
             DEFINE_PROP_UINT32("pmsav7-dregion", ARMCPU, pmsav7_dregion, 16);
 
+static Property arm_cpu_pmsav7_cfsr_property =
+            DEFINE_PROP_UINT32("pmsav7-cfsr", ARMCPU, pmsav7_cfsr, 0);
+
+static Property arm_cpu_pmsav7_mmfar_property =
+            DEFINE_PROP_UINT32("pmsav7-mmfar", ARMCPU, pmsav7_mmfar, 0);
+
 static void arm_cpu_post_init(Object *obj)
 {
     ARMCPU *cpu = ARM_CPU(obj);
@@ -551,6 +557,12 @@  static void arm_cpu_post_init(Object *obj)
             qdev_property_add_static(DEVICE(obj),
                                      &arm_cpu_pmsav7_dregion_property,
                                      &error_abort);
+            qdev_property_add_static(DEVICE(obj),
+                                     &arm_cpu_pmsav7_cfsr_property,
+                                     &error_abort);
+            qdev_property_add_static(DEVICE(obj),
+                                     &arm_cpu_pmsav7_mmfar_property,
+                                     &error_abort);
         }
     }
 
@@ -889,6 +901,7 @@  static void cortex_m3_initfn(Object *obj)
     ARMCPU *cpu = ARM_CPU(obj);
     set_feature(&cpu->env, ARM_FEATURE_V7);
     set_feature(&cpu->env, ARM_FEATURE_M);
+    set_feature(&cpu->env, ARM_FEATURE_MPU);
     cpu->midr = 0x410fc231;
 }
 
@@ -898,6 +911,7 @@  static void cortex_m4_initfn(Object *obj)
 
     set_feature(&cpu->env, ARM_FEATURE_V7);
     set_feature(&cpu->env, ARM_FEATURE_M);
+    set_feature(&cpu->env, ARM_FEATURE_MPU);
     set_feature(&cpu->env, ARM_FEATURE_THUMB_DSP);
     cpu->midr = 0x410fc240; /* r0p0 */
 }
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 56b238f..368cbc6 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -5357,11 +5357,15 @@  void arm_v7m_cpu_do_interrupt(CPUState *cs)
     case EXCP_DATA_ABORT:
         if(env->v7m.exception!=0 && env->exception.vaddress>=0xfffffff0) {
             /* this isn't a real fault, but rather a result of return from interrupt */
+            cpu->pmsav7_mmfar = 0;
+            cpu->pmsav7_cfsr = 0;
             do_v7m_exception_exit(env);
             return;
         }
         armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_MEM);
         env->v7m.exception = ARMV7M_EXCP_MEM;
+        cpu->pmsav7_mmfar = env->exception.vaddress;
+        cpu->pmsav7_cfsr = (1<<1)|(1<<7);
         break;
     case EXCP_BKPT:
         if (semihosting_enabled()) {
@@ -5382,6 +5386,9 @@  void arm_v7m_cpu_do_interrupt(CPUState *cs)
         env->v7m.exception = armv7m_nvic_acknowledge_irq(env->nvic);
         break;
     case EXCP_EXCEPTION_EXIT:
+
+        cpu->pmsav7_mmfar = 0;
+        cpu->pmsav7_cfsr = 0;
         do_v7m_exception_exit(env);
         return;
     default: