[Qemu-devel] [PATCH 16/18] armv7m: check exception return consistency

[Michael Davidsaver <mdavidsaver@gmail.com>]

Detect use of reserved exception return codes
and return to thread mode from nested
exception handler.

Also check consistency between NVIC and CPU
wrt. the active exception.

Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
---
 hw/intc/armv7m_nvic.c |  7 +++-
 target-arm/cpu.h      |  2 +-
 target-arm/helper.c   | 97 ++++++++++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 96 insertions(+), 10 deletions(-)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 734f6f8..a75dd3c 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -342,7 +342,7 @@ void armv7m_nvic_acknowledge_irq(void *opaque)
     assert(env->v7m.exception > 0); /* spurious exception? */
 }
 
-void armv7m_nvic_complete_irq(void *opaque, int irq)
+bool armv7m_nvic_complete_irq(void *opaque, int irq)
 {
     nvic_state *s = (nvic_state *)opaque;
     vec_info *I;
@@ -352,12 +352,17 @@ void armv7m_nvic_complete_irq(void *opaque, int irq)
 
     I = &s->vectors[irq];
 
+    if (!I->active) {
+        return true;
+    }
+
     I->active = 0;
     I->pending = I->level;
     assert(!I->level || irq >= 16);
 
     nvic_irq_update(s, 1);
     DPRINTF(0, "EOI %d\n", irq);
+    return false;
 }
 
 /* Only called for external interrupt (vector>=16) */
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index e98bca0..72b0b32 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1063,7 +1063,7 @@ int armv7m_excp_unmasked(ARMCPU *cpu)
 /* Interface between CPU and Interrupt controller.  */
 void armv7m_nvic_set_pending(void *opaque, int irq);
 void armv7m_nvic_acknowledge_irq(void *opaque);
-void armv7m_nvic_complete_irq(void *opaque, int irq);
+bool armv7m_nvic_complete_irq(void *opaque, int irq);
 
 /* Interface for defining coprocessor registers.
  * Registers are defined in tables of arm_cp_reginfo structs
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 83af528..3993f77 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -5375,18 +5375,65 @@ static void switch_v7m_sp(CPUARMState *env, int process)
 
 static void do_v7m_exception_exit(CPUARMState *env)
 {
+    unsigned ufault = 0;
     uint32_t type;
     uint32_t xpsr;
+    uint32_t nvic_active;
+
+    if (env->v7m.exception == 0) {
+        hw_error("Return from exception w/o active exception.  Logic error.");
+    }
 
-    type = env->regs[15];
     if (env->v7m.exception != ARMV7M_EXCP_NMI) {
         /* Auto-clear FAULTMASK on return from other than NMI */
         env->daif &= ~PSTATE_F;
     }
-    if (env->v7m.exception != 0) {
-        armv7m_nvic_complete_irq(env->nvic, env->v7m.exception);
+
+    if (armv7m_nvic_complete_irq(env->nvic, env->v7m.exception)) {
+        qemu_log_mask(LOG_GUEST_ERROR, "Requesting return from exception "
+                      "from inactive exception %d\n",
+                      env->v7m.exception);
+        ufault = 1;
+    }
+    /* env->v7m.exception and friends updated by armv7m_nvic_complete_irq() */
+
+    type = env->regs[15]&0xf;
+    /* QEMU seems to clear the LSB at some point. */
+    type |= 1;
+
+    switch (type) {
+    case 0x1: /* Return to Handler mode */
+        if (env->v7m.exception == 0) {
+            qemu_log_mask(LOG_GUEST_ERROR, "Requesting return from exception "
+                          "to Handler mode not allowed at base level of "
+                          "activation");
+            ufault = 1;
+        }
+        break;
+    case 0x9: /* Return to Thread mode w/ Main stack */
+    case 0xd: /* Return to Thread mode w/ Process stack */
+        if (env->v7m.exception != 0) {
+            /* Attempt to return to Thread mode
+             * from nested handler while NONBASETHRDENA not set.
+             */
+            qemu_log_mask(LOG_GUEST_ERROR, "Nested exception return to %d w/"
+                          " Thread mode while NONBASETHRDENA not set\n",
+                          env->v7m.exception);
+            ufault = 1;
+        }
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "Exception return w/ reserved code"
+                                       " %02x\n", (unsigned)type);
+        ufault = 1;
     }
 
+    /* TODO? if ufault==1 ARM calls for entering exception handler
+     * directly w/o popping stack.
+     * We pop anyway since the active UsageFault will push on entry
+     * which should happen before execution resumes?
+     */
+
     /* Switch to the target stack.  */
     switch_v7m_sp(env, (type & 4) != 0);
     /* Pop registers.  */
@@ -5409,15 +5456,49 @@ static void do_v7m_exception_exit(CPUARMState *env)
         env->regs[15] &= ~1U;
     }
     xpsr = v7m_pop(env);
+    nvic_active = env->v7m.exception;
     xpsr_write(env, xpsr, 0xfffffdff);
+
     /* Undo stack alignment.  */
     if (xpsr & 0x200)
         env->regs[13] |= 4;
-    /* ??? The exception return type specifies Thread/Handler mode.  However
-       this is also implied by the xPSR value. Not sure what to do
-       if there is a mismatch.  */
-    /* ??? Likewise for mismatches between the CONTROL register and the stack
-       pointer.  */
+
+    /* TODO? if v7m.exception>0 && nvic_active != v7m.exception
+     * Adjust v7m.exception_prio for the exception being returned to?
+     * This could be different now that v7m.exception is restored
+     * from the guest stack.
+     * This can happen if the guest adjusts exception priorities
+     * while in a handler.
+     * At present retain the priority of the highest active
+     * exception.
+     */
+
+    if (!ufault) {
+        if (nvic_active != 0 && env->v7m.exception == 0) {
+            ufault = 1;
+        } else if (nvic_active == 0 && env->v7m.exception != 0) {
+            ufault = 1;
+        }
+        if (ufault) {
+            qemu_log_mask(LOG_GUEST_ERROR, "Return from exception with "
+                          "inconsistency between Exception return code "
+                          "and stacked xPSR\n");
+        }
+        /* ARM calls for PushStack() here, which should happen
+         * went we return with a pending exception
+         */
+    }
+
+    if (ufault) {
+        armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE);
+        env->v7m.cfsr |= 1<<18; /* INVPC */
+    }
+
+    /* Ensure that priority is consistent.  Clear for Thread mode
+     * and set for Handler mode
+     */
+    assert((env->v7m.exception == 0 && env->v7m.exception_prio > 0xff)
+           || (env->v7m.exception != 0 && env->v7m.exception_prio <= 0xff));
 }
 
 void arm_v7m_cpu_do_interrupt(CPUState *cs)
-- 
2.1.4