[Qemu-devel] [PATCH] target/arm: Escalate to correct HardFault when AIRCR.BFHFNMINS is set

[Qemu-devel] [PATCH] target/arm: Escalate to correct HardFault when AIRCR.BFHFNMINS is set

[Peter Maydell]

When we escalate a v8M exception to HardFault, if AIRCR.BFHFNMINNS is
set then we need to decide whether it should become a secure HardFault
or a nonsecure HardFault. We should always escalate to the same
target security state as the original exception. The current code
tries to test this using the 'secure' bool, which is not right because
that flag indicates whether the target security state only for
banked exceptions; the effect was that we were incorrectly escalating
always-secure exceptions like SecureFault to a nonsecure HardFault.

Fix this by defining, logging and using a new 'targets_secure' bool
which tracks the condition we actually want.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/intc/armv7m_nvic.c | 8 ++++++--
 hw/intc/trace-events  | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index bf92fe0972c..e160b02eab4 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -529,13 +529,17 @@ static void do_armv7m_nvic_set_pending(void *opaque, int irq, bool secure,
     NVICState *s = (NVICState *)opaque;
     bool banked = exc_is_banked(irq);
     VecInfo *vec;
+    bool targets_secure;
 
     assert(irq > ARMV7M_EXCP_RESET && irq < s->num_irq);
     assert(!secure || banked);
 
     vec = (banked && secure) ? &s->sec_vectors[irq] : &s->vectors[irq];
 
-    trace_nvic_set_pending(irq, secure, derived, vec->enabled, vec->prio);
+    targets_secure = banked ? secure : exc_targets_secure(s, irq);
+
+    trace_nvic_set_pending(irq, secure, targets_secure,
+                           derived, vec->enabled, vec->prio);
 
     if (derived) {
         /* Derived exceptions are always synchronous. */
@@ -615,7 +619,7 @@ static void do_armv7m_nvic_set_pending(void *opaque, int irq, bool secure,
              */
             irq = ARMV7M_EXCP_HARD;
             if (arm_feature(&s->cpu->env, ARM_FEATURE_M_SECURITY) &&
-                (secure ||
+                (targets_secure ||
                  !(s->cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK))) {
                 vec = &s->sec_vectors[irq];
             } else {
diff --git a/hw/intc/trace-events b/hw/intc/trace-events
index b6cb5e6048d..33e932fb918 100644
--- a/hw/intc/trace-events
+++ b/hw/intc/trace-events
@@ -177,7 +177,7 @@ nvic_set_prio(int irq, bool secure, uint8_t prio) "NVIC set irq %d secure-bank %
 nvic_irq_update(int vectpending, int pendprio, int exception_prio, int level) "NVIC vectpending %d pending prio %d exception_prio %d: setting irq line to %d"
 nvic_escalate_prio(int irq, int irqprio, int runprio) "NVIC escalating irq %d to HardFault: insufficient priority %d >= %d"
 nvic_escalate_disabled(int irq) "NVIC escalating irq %d to HardFault: disabled"
-nvic_set_pending(int irq, bool secure, bool derived, int en, int prio) "NVIC set pending irq %d secure-bank %d derived %d (enabled: %d priority %d)"
+nvic_set_pending(int irq, bool secure, bool targets_secure, bool derived, int en, int prio) "NVIC set pending irq %d secure-bank %d targets_secure %d derived %d (enabled: %d priority %d)"
 nvic_clear_pending(int irq, bool secure, int en, int prio) "NVIC clear pending irq %d secure-bank %d (enabled: %d priority %d)"
 nvic_set_pending_level(int irq) "NVIC set pending: irq %d higher prio than vectpending: setting irq line to 1"
 nvic_acknowledge_irq(int irq, int prio) "NVIC acknowledge IRQ: %d now active (prio %d)"
-- 
2.17.1

Re: [Qemu-devel] [PATCH] target/arm: Escalate to correct HardFault when AIRCR.BFHFNMINS is set

[Richard Henderson]

On 07/23/2018 05:34 AM, Peter Maydell wrote:
> When we escalate a v8M exception to HardFault, if AIRCR.BFHFNMINNS is
> set then we need to decide whether it should become a secure HardFault
> or a nonsecure HardFault. We should always escalate to the same
> target security state as the original exception. The current code
> tries to test this using the 'secure' bool, which is not right because
> that flag indicates whether the target security state only for
> banked exceptions; the effect was that we were incorrectly escalating
> always-secure exceptions like SecureFault to a nonsecure HardFault.
> 
> Fix this by defining, logging and using a new 'targets_secure' bool
> which tracks the condition we actually want.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/intc/armv7m_nvic.c | 8 ++++++--
>  hw/intc/trace-events  | 2 +-
>  2 files changed, 7 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~