All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/4] target/arm: Implement tailchaining for M profile cores
@ 2018-07-20 14:56 Peter Maydell
  2018-07-20 14:56 ` [Qemu-devel] [PATCH 1/4] target/arm: Improve exception-taken logging Peter Maydell
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Peter Maydell @ 2018-07-20 14:56 UTC (permalink / raw)
  To: qemu-arm, qemu-devel; +Cc: patches

Tailchaining is an optimization in handling of exception return
for M-profile cores: if we are about to pop the exception stack
for an exception return, but there is a pending exception which
is higher priority than the priority we are returning to, then
instead of unstacking and then immediately taking the exception
and stacking registers again, we can chain to the pending
exception without unstacking and stacking.

For v6M and v7M it is IMPDEF whether tailchaining happens for pending
exceptions; for v8M this is architecturally required.  Implement it
in QEMU for all M-profile cores, since in practice v6M and v7M
hardware implementations generally do have it.

(We were already doing tailchaining for derived exceptions which
happened during exception return, like the validity checks and
stack access failures; these have always been required to be
tailchained for all versions of the architecture.)

The first few patches here do some minor cleanup and bug fixing
that I noticed while working on this; patch 4 is the actual
implementation, which turns out to be pretty trivial.

thanks
-- PMM

Peter Maydell (4):
  target/arm: Improve exception-taken logging
  target/arm: Initialize exc_secure correctly in do_v7m_exception_exit()
  target/arm: Restore M-profile CONTROL.SPSEL before any tailchaining
  target/arm: Implement tailchaining for M profile cores

 target/arm/helper.c | 47 ++++++++++++++++++++++++++++++++++-----------
 1 file changed, 36 insertions(+), 11 deletions(-)

-- 
2.17.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 1/4] target/arm: Improve exception-taken logging
  2018-07-20 14:56 [Qemu-devel] [PATCH 0/4] target/arm: Implement tailchaining for M profile cores Peter Maydell
@ 2018-07-20 14:56 ` Peter Maydell
  2018-07-20 16:15   ` Richard Henderson
  2018-07-21 19:47   ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
  2018-07-20 14:56 ` [Qemu-devel] [PATCH 2/4] target/arm: Initialize exc_secure correctly in do_v7m_exception_exit() Peter Maydell
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 11+ messages in thread
From: Peter Maydell @ 2018-07-20 14:56 UTC (permalink / raw)
  To: qemu-arm, qemu-devel; +Cc: patches

Improve the exception-taken logging by logging in
v7m_exception_taken() the exception we're going to take
and whether it is secure/nonsecure.

This requires us to move logging at many callsites from after the
call to before it, so that the logging appears in a sensible order.

(This will make tail-chaining produce more useful logs; for the
current callers of v7m_exception_taken() we know which exception
we're going to take, so custom log messages at the callsite sufficed;
for tail-chaining only v7m_exception_taken() knows the exception
number that we're going to tail-chain to.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/helper.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 6f5bb198c3b..3ec6114599b 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6840,6 +6840,8 @@ static void v7m_exception_taken(ARMCPU *cpu, uint32_t lr, bool dotailchain,
     bool push_failed = false;
 
     armv7m_nvic_get_pending_irq_info(env->nvic, &exc, &targets_secure);
+    qemu_log_mask(CPU_LOG_INT, "...taking pending %s exception %d\n",
+                  targets_secure ? "secure" : "nonsecure", exc);
 
     if (arm_feature(env, ARM_FEATURE_V8)) {
         if (arm_feature(env, ARM_FEATURE_M_SECURITY) &&
@@ -6913,12 +6915,15 @@ static void v7m_exception_taken(ARMCPU *cpu, uint32_t lr, bool dotailchain,
          * we might now want to take a different exception which
          * targets a different security state, so try again from the top.
          */
+        qemu_log_mask(CPU_LOG_INT,
+                      "...derived exception on callee-saves register stacking");
         v7m_exception_taken(cpu, lr, true, true);
         return;
     }
 
     if (!arm_v7m_load_vector(cpu, exc, targets_secure, &addr)) {
         /* Vector load failed: derived exception */
+        qemu_log_mask(CPU_LOG_INT, "...derived exception on vector table load");
         v7m_exception_taken(cpu, lr, true, true);
         return;
     }
@@ -7129,9 +7134,9 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
     if (sfault) {
         env->v7m.sfsr |= R_V7M_SFSR_INVER_MASK;
         armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_SECURE, false);
-        v7m_exception_taken(cpu, excret, true, false);
         qemu_log_mask(CPU_LOG_INT, "...taking SecureFault on existing "
                       "stackframe: failed EXC_RETURN.ES validity check\n");
+        v7m_exception_taken(cpu, excret, true, false);
         return;
     }
 
@@ -7141,9 +7146,9 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
          */
         env->v7m.cfsr[env->v7m.secure] |= R_V7M_CFSR_INVPC_MASK;
         armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE, env->v7m.secure);
-        v7m_exception_taken(cpu, excret, true, false);
         qemu_log_mask(CPU_LOG_INT, "...taking UsageFault on existing "
                       "stackframe: failed exception return integrity check\n");
+        v7m_exception_taken(cpu, excret, true, false);
         return;
     }
 
@@ -7198,10 +7203,10 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
                 /* Take a SecureFault on the current stack */
                 env->v7m.sfsr |= R_V7M_SFSR_INVIS_MASK;
                 armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_SECURE, false);
-                v7m_exception_taken(cpu, excret, true, false);
                 qemu_log_mask(CPU_LOG_INT, "...taking SecureFault on existing "
                               "stackframe: failed exception return integrity "
                               "signature check\n");
+                v7m_exception_taken(cpu, excret, true, false);
                 return;
             }
 
@@ -7234,6 +7239,7 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
             /* v7m_stack_read() pended a fault, so take it (as a tail
              * chained exception on the same stack frame)
              */
+            qemu_log_mask(CPU_LOG_INT, "...derived exception on unstacking\n");
             v7m_exception_taken(cpu, excret, true, false);
             return;
         }
@@ -7270,10 +7276,10 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
                 armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE,
                                         env->v7m.secure);
                 env->v7m.cfsr[env->v7m.secure] |= R_V7M_CFSR_INVPC_MASK;
-                v7m_exception_taken(cpu, excret, true, false);
                 qemu_log_mask(CPU_LOG_INT, "...taking UsageFault on existing "
                               "stackframe: failed exception return integrity "
                               "check\n");
+                v7m_exception_taken(cpu, excret, true, false);
                 return;
             }
         }
@@ -7309,9 +7315,9 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
         armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE, false);
         env->v7m.cfsr[env->v7m.secure] |= R_V7M_CFSR_INVPC_MASK;
         ignore_stackfaults = v7m_push_stack(cpu);
-        v7m_exception_taken(cpu, excret, false, ignore_stackfaults);
         qemu_log_mask(CPU_LOG_INT, "...taking UsageFault on new stackframe: "
                       "failed exception return integrity check\n");
+        v7m_exception_taken(cpu, excret, false, ignore_stackfaults);
         return;
     }
 
@@ -7727,7 +7733,6 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
 
     ignore_stackfaults = v7m_push_stack(cpu);
     v7m_exception_taken(cpu, lr, false, ignore_stackfaults);
-    qemu_log_mask(CPU_LOG_INT, "... as %d\n", env->v7m.exception);
 }
 
 /* Function used to synchronize QEMU's AArch64 register set with AArch32
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 2/4] target/arm: Initialize exc_secure correctly in do_v7m_exception_exit()
  2018-07-20 14:56 [Qemu-devel] [PATCH 0/4] target/arm: Implement tailchaining for M profile cores Peter Maydell
  2018-07-20 14:56 ` [Qemu-devel] [PATCH 1/4] target/arm: Improve exception-taken logging Peter Maydell
@ 2018-07-20 14:56 ` Peter Maydell
  2018-07-20 16:17   ` Richard Henderson
  2018-07-21 19:48   ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
  2018-07-20 14:56 ` [Qemu-devel] [PATCH 3/4] target/arm: Restore M-profile CONTROL.SPSEL before any tailchaining Peter Maydell
  2018-07-20 14:56 ` [Qemu-devel] [PATCH 4/4] target/arm: Implement tailchaining for M profile cores Peter Maydell
  3 siblings, 2 replies; 11+ messages in thread
From: Peter Maydell @ 2018-07-20 14:56 UTC (permalink / raw)
  To: qemu-arm, qemu-devel; +Cc: patches

In do_v7m_exception_exit(), we use the exc_secure variable to track
whether the exception we're returning from is secure or non-secure.
Unfortunately the statement initializing this was accidentally
inside an "if (env->v7m.exception != ARMV7M_EXCP_NMI)" conditional,
which meant that we were using the wrong value for NMI handlers.
Move the initialization out to the right place.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 3ec6114599b..efcadfc7fa9 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -7052,6 +7052,7 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
             /* For all other purposes, treat ES as 0 (R_HXSR) */
             excret &= ~R_V7M_EXCRET_ES_MASK;
         }
+        exc_secure = excret & R_V7M_EXCRET_ES_MASK;
     }
 
     if (env->v7m.exception != ARMV7M_EXCP_NMI) {
@@ -7062,7 +7063,6 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
          * which security state's faultmask to clear. (v8M ARM ARM R_KBNF.)
          */
         if (arm_feature(env, ARM_FEATURE_M_SECURITY)) {
-            exc_secure = excret & R_V7M_EXCRET_ES_MASK;
             if (armv7m_nvic_raw_execution_priority(env->nvic) >= 0) {
                 env->v7m.faultmask[exc_secure] = 0;
             }
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 3/4] target/arm: Restore M-profile CONTROL.SPSEL before any tailchaining
  2018-07-20 14:56 [Qemu-devel] [PATCH 0/4] target/arm: Implement tailchaining for M profile cores Peter Maydell
  2018-07-20 14:56 ` [Qemu-devel] [PATCH 1/4] target/arm: Improve exception-taken logging Peter Maydell
  2018-07-20 14:56 ` [Qemu-devel] [PATCH 2/4] target/arm: Initialize exc_secure correctly in do_v7m_exception_exit() Peter Maydell
@ 2018-07-20 14:56 ` Peter Maydell
  2018-07-20 16:19   ` Richard Henderson
  2018-07-20 14:56 ` [Qemu-devel] [PATCH 4/4] target/arm: Implement tailchaining for M profile cores Peter Maydell
  3 siblings, 1 reply; 11+ messages in thread
From: Peter Maydell @ 2018-07-20 14:56 UTC (permalink / raw)
  To: qemu-arm, qemu-devel; +Cc: patches

On exception return for M-profile, we must restore the CONTROL.SPSEL
bit from the EXCRET value before we do any kind of tailchaining,
including for the derived exceptions on integrity check failures.
Otherwise we will give the guest an incorrect EXCRET.SPSEL value on
exception entry for the tailchained exception.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/helper.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index efcadfc7fa9..ff947416c9d 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -7131,6 +7131,16 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
         }
     }
 
+    /*
+     * Set CONTROL.SPSEL from excret.SPSEL. Since we're still in
+     * Handler mode (and will be until we write the new XPSR.Interrupt
+     * field) this does not switch around the current stack pointer.
+     * We must do this before we do any kind of tailchaining, including
+     * for the derived exceptions on integrity check failures, or we will
+     * give the guest an incorrect EXCRET.SPSEL value on exception entry.
+     */
+    write_v7m_control_spsel_for_secstate(env, return_to_sp_process, exc_secure);
+
     if (sfault) {
         env->v7m.sfsr |= R_V7M_SFSR_INVER_MASK;
         armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_SECURE, false);
@@ -7152,12 +7162,6 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
         return;
     }
 
-    /* Set CONTROL.SPSEL from excret.SPSEL. Since we're still in
-     * Handler mode (and will be until we write the new XPSR.Interrupt
-     * field) this does not switch around the current stack pointer.
-     */
-    write_v7m_control_spsel_for_secstate(env, return_to_sp_process, exc_secure);
-
     switch_v7m_security_state(env, return_to_secure);
 
     {
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 4/4] target/arm: Implement tailchaining for M profile cores
  2018-07-20 14:56 [Qemu-devel] [PATCH 0/4] target/arm: Implement tailchaining for M profile cores Peter Maydell
                   ` (2 preceding siblings ...)
  2018-07-20 14:56 ` [Qemu-devel] [PATCH 3/4] target/arm: Restore M-profile CONTROL.SPSEL before any tailchaining Peter Maydell
@ 2018-07-20 14:56 ` Peter Maydell
  2018-07-20 16:22   ` Richard Henderson
  3 siblings, 1 reply; 11+ messages in thread
From: Peter Maydell @ 2018-07-20 14:56 UTC (permalink / raw)
  To: qemu-arm, qemu-devel; +Cc: patches

Tailchaining is an optimization in handling of exception return
for M-profile cores: if we are about to pop the exception stack
for an exception return, but there is a pending exception which
is higher priority than the priority we are returning to, then
instead of unstacking and then immediately taking the exception
and stacking registers again, we can chain to the pending
exception without unstacking and stacking.

For v6M and v7M it is IMPDEF whether tailchaining happens for pending
exceptions; for v8M this is architecturally required.  Implement it
in QEMU for all M-profile cores, since in practice v6M and v7M
hardware implementations generally do have it.

(We were already doing tailchaining for derived exceptions which
happened during exception return, like the validity checks and
stack access failures; these have always been required to be
tailchained for all versions of the architecture.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/helper.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index ff947416c9d..c9e9f95051c 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -7162,6 +7162,22 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
         return;
     }
 
+    /*
+     * Tailchaining: if there is currently a pending exception that
+     * is high enough priority to preempt execution at the level we're
+     * about to return to, then just directly take that exception now,
+     * avoiding an unstack-and-then-stack. Note that now we have
+     * deactivated the previous exception by calling armv7m_nvic_complete_irq()
+     * our current execution priority is already the execution priority we are
+     * returning to -- none of the state we would unstack or set based on
+     * the EXCRET value affects it.
+     */
+    if (armv7m_nvic_can_take_pending_exception(env->nvic)) {
+        qemu_log_mask(CPU_LOG_INT, "...tailchaining to pending exception\n");
+        v7m_exception_taken(cpu, excret, true, false);
+        return;
+    }
+
     switch_v7m_security_state(env, return_to_secure);
 
     {
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 1/4] target/arm: Improve exception-taken logging
  2018-07-20 14:56 ` [Qemu-devel] [PATCH 1/4] target/arm: Improve exception-taken logging Peter Maydell
@ 2018-07-20 16:15   ` Richard Henderson
  2018-07-21 19:47   ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
  1 sibling, 0 replies; 11+ messages in thread
From: Richard Henderson @ 2018-07-20 16:15 UTC (permalink / raw)
  To: Peter Maydell, qemu-arm, qemu-devel; +Cc: patches

On 07/20/2018 07:56 AM, Peter Maydell wrote:
> Improve the exception-taken logging by logging in
> v7m_exception_taken() the exception we're going to take
> and whether it is secure/nonsecure.
> 
> This requires us to move logging at many callsites from after the
> call to before it, so that the logging appears in a sensible order.
> 
> (This will make tail-chaining produce more useful logs; for the
> current callers of v7m_exception_taken() we know which exception
> we're going to take, so custom log messages at the callsite sufficed;
> for tail-chaining only v7m_exception_taken() knows the exception
> number that we're going to tail-chain to.)
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target/arm/helper.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)

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


r~

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 2/4] target/arm: Initialize exc_secure correctly in do_v7m_exception_exit()
  2018-07-20 14:56 ` [Qemu-devel] [PATCH 2/4] target/arm: Initialize exc_secure correctly in do_v7m_exception_exit() Peter Maydell
@ 2018-07-20 16:17   ` Richard Henderson
  2018-07-21 19:48   ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
  1 sibling, 0 replies; 11+ messages in thread
From: Richard Henderson @ 2018-07-20 16:17 UTC (permalink / raw)
  To: Peter Maydell, qemu-arm, qemu-devel; +Cc: patches

On 07/20/2018 07:56 AM, Peter Maydell wrote:
> In do_v7m_exception_exit(), we use the exc_secure variable to track
> whether the exception we're returning from is secure or non-secure.
> Unfortunately the statement initializing this was accidentally
> inside an "if (env->v7m.exception != ARMV7M_EXCP_NMI)" conditional,
> which meant that we were using the wrong value for NMI handlers.
> Move the initialization out to the right place.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target/arm/helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

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


r~

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 3/4] target/arm: Restore M-profile CONTROL.SPSEL before any tailchaining
  2018-07-20 14:56 ` [Qemu-devel] [PATCH 3/4] target/arm: Restore M-profile CONTROL.SPSEL before any tailchaining Peter Maydell
@ 2018-07-20 16:19   ` Richard Henderson
  0 siblings, 0 replies; 11+ messages in thread
From: Richard Henderson @ 2018-07-20 16:19 UTC (permalink / raw)
  To: Peter Maydell, qemu-arm, qemu-devel; +Cc: patches

On 07/20/2018 07:56 AM, Peter Maydell wrote:
> On exception return for M-profile, we must restore the CONTROL.SPSEL
> bit from the EXCRET value before we do any kind of tailchaining,
> including for the derived exceptions on integrity check failures.
> Otherwise we will give the guest an incorrect EXCRET.SPSEL value on
> exception entry for the tailchained exception.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target/arm/helper.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)

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


r~

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 4/4] target/arm: Implement tailchaining for M profile cores
  2018-07-20 14:56 ` [Qemu-devel] [PATCH 4/4] target/arm: Implement tailchaining for M profile cores Peter Maydell
@ 2018-07-20 16:22   ` Richard Henderson
  0 siblings, 0 replies; 11+ messages in thread
From: Richard Henderson @ 2018-07-20 16:22 UTC (permalink / raw)
  To: Peter Maydell, qemu-arm, qemu-devel; +Cc: patches

On 07/20/2018 07:56 AM, Peter Maydell wrote:
> Tailchaining is an optimization in handling of exception return
> for M-profile cores: if we are about to pop the exception stack
> for an exception return, but there is a pending exception which
> is higher priority than the priority we are returning to, then
> instead of unstacking and then immediately taking the exception
> and stacking registers again, we can chain to the pending
> exception without unstacking and stacking.
> 
> For v6M and v7M it is IMPDEF whether tailchaining happens for pending
> exceptions; for v8M this is architecturally required.  Implement it
> in QEMU for all M-profile cores, since in practice v6M and v7M
> hardware implementations generally do have it.
> 
> (We were already doing tailchaining for derived exceptions which
> happened during exception return, like the validity checks and
> stack access failures; these have always been required to be
> tailchained for all versions of the architecture.)
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target/arm/helper.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)

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


r~

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH 1/4] target/arm: Improve exception-taken logging
  2018-07-20 14:56 ` [Qemu-devel] [PATCH 1/4] target/arm: Improve exception-taken logging Peter Maydell
  2018-07-20 16:15   ` Richard Henderson
@ 2018-07-21 19:47   ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-07-21 19:47 UTC (permalink / raw)
  To: Peter Maydell, qemu-arm, qemu-devel; +Cc: patches

On 07/20/2018 11:56 AM, Peter Maydell wrote:
> Improve the exception-taken logging by logging in
> v7m_exception_taken() the exception we're going to take
> and whether it is secure/nonsecure.
> 
> This requires us to move logging at many callsites from after the
> call to before it, so that the logging appears in a sensible order.
> 
> (This will make tail-chaining produce more useful logs; for the
> current callers of v7m_exception_taken() we know which exception

Thanks for this cleanup.

> we're going to take, so custom log messages at the callsite sufficed;
> for tail-chaining only v7m_exception_taken() knows the exception
> number that we're going to tail-chain to.)
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  target/arm/helper.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 6f5bb198c3b..3ec6114599b 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -6840,6 +6840,8 @@ static void v7m_exception_taken(ARMCPU *cpu, uint32_t lr, bool dotailchain,
>      bool push_failed = false;
>  
>      armv7m_nvic_get_pending_irq_info(env->nvic, &exc, &targets_secure);
> +    qemu_log_mask(CPU_LOG_INT, "...taking pending %s exception %d\n",
> +                  targets_secure ? "secure" : "nonsecure", exc);
>  
>      if (arm_feature(env, ARM_FEATURE_V8)) {
>          if (arm_feature(env, ARM_FEATURE_M_SECURITY) &&
> @@ -6913,12 +6915,15 @@ static void v7m_exception_taken(ARMCPU *cpu, uint32_t lr, bool dotailchain,
>           * we might now want to take a different exception which
>           * targets a different security state, so try again from the top.
>           */
> +        qemu_log_mask(CPU_LOG_INT,
> +                      "...derived exception on callee-saves register stacking");
>          v7m_exception_taken(cpu, lr, true, true);
>          return;
>      }
>  
>      if (!arm_v7m_load_vector(cpu, exc, targets_secure, &addr)) {
>          /* Vector load failed: derived exception */
> +        qemu_log_mask(CPU_LOG_INT, "...derived exception on vector table load");
>          v7m_exception_taken(cpu, lr, true, true);
>          return;
>      }
> @@ -7129,9 +7134,9 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
>      if (sfault) {
>          env->v7m.sfsr |= R_V7M_SFSR_INVER_MASK;
>          armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_SECURE, false);
> -        v7m_exception_taken(cpu, excret, true, false);
>          qemu_log_mask(CPU_LOG_INT, "...taking SecureFault on existing "
>                        "stackframe: failed EXC_RETURN.ES validity check\n");
> +        v7m_exception_taken(cpu, excret, true, false);
>          return;
>      }
>  
> @@ -7141,9 +7146,9 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
>           */
>          env->v7m.cfsr[env->v7m.secure] |= R_V7M_CFSR_INVPC_MASK;
>          armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE, env->v7m.secure);
> -        v7m_exception_taken(cpu, excret, true, false);
>          qemu_log_mask(CPU_LOG_INT, "...taking UsageFault on existing "
>                        "stackframe: failed exception return integrity check\n");
> +        v7m_exception_taken(cpu, excret, true, false);
>          return;
>      }
>  
> @@ -7198,10 +7203,10 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
>                  /* Take a SecureFault on the current stack */
>                  env->v7m.sfsr |= R_V7M_SFSR_INVIS_MASK;
>                  armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_SECURE, false);
> -                v7m_exception_taken(cpu, excret, true, false);
>                  qemu_log_mask(CPU_LOG_INT, "...taking SecureFault on existing "
>                                "stackframe: failed exception return integrity "
>                                "signature check\n");
> +                v7m_exception_taken(cpu, excret, true, false);
>                  return;
>              }
>  
> @@ -7234,6 +7239,7 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
>              /* v7m_stack_read() pended a fault, so take it (as a tail
>               * chained exception on the same stack frame)
>               */
> +            qemu_log_mask(CPU_LOG_INT, "...derived exception on unstacking\n");
>              v7m_exception_taken(cpu, excret, true, false);
>              return;
>          }
> @@ -7270,10 +7276,10 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
>                  armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE,
>                                          env->v7m.secure);
>                  env->v7m.cfsr[env->v7m.secure] |= R_V7M_CFSR_INVPC_MASK;
> -                v7m_exception_taken(cpu, excret, true, false);
>                  qemu_log_mask(CPU_LOG_INT, "...taking UsageFault on existing "
>                                "stackframe: failed exception return integrity "
>                                "check\n");
> +                v7m_exception_taken(cpu, excret, true, false);
>                  return;
>              }
>          }
> @@ -7309,9 +7315,9 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
>          armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE, false);
>          env->v7m.cfsr[env->v7m.secure] |= R_V7M_CFSR_INVPC_MASK;
>          ignore_stackfaults = v7m_push_stack(cpu);
> -        v7m_exception_taken(cpu, excret, false, ignore_stackfaults);
>          qemu_log_mask(CPU_LOG_INT, "...taking UsageFault on new stackframe: "
>                        "failed exception return integrity check\n");
> +        v7m_exception_taken(cpu, excret, false, ignore_stackfaults);
>          return;
>      }
>  
> @@ -7727,7 +7733,6 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
>  
>      ignore_stackfaults = v7m_push_stack(cpu);
>      v7m_exception_taken(cpu, lr, false, ignore_stackfaults);
> -    qemu_log_mask(CPU_LOG_INT, "... as %d\n", env->v7m.exception);
>  }
>  
>  /* Function used to synchronize QEMU's AArch64 register set with AArch32
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH 2/4] target/arm: Initialize exc_secure correctly in do_v7m_exception_exit()
  2018-07-20 14:56 ` [Qemu-devel] [PATCH 2/4] target/arm: Initialize exc_secure correctly in do_v7m_exception_exit() Peter Maydell
  2018-07-20 16:17   ` Richard Henderson
@ 2018-07-21 19:48   ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-07-21 19:48 UTC (permalink / raw)
  To: Peter Maydell, qemu-arm, qemu-devel; +Cc: patches

On 07/20/2018 11:56 AM, Peter Maydell wrote:
> In do_v7m_exception_exit(), we use the exc_secure variable to track
> whether the exception we're returning from is secure or non-secure.
> Unfortunately the statement initializing this was accidentally
> inside an "if (env->v7m.exception != ARMV7M_EXCP_NMI)" conditional,
> which meant that we were using the wrong value for NMI handlers.
> Move the initialization out to the right place.

Oops

> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  target/arm/helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 3ec6114599b..efcadfc7fa9 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -7052,6 +7052,7 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
>              /* For all other purposes, treat ES as 0 (R_HXSR) */
>              excret &= ~R_V7M_EXCRET_ES_MASK;
>          }
> +        exc_secure = excret & R_V7M_EXCRET_ES_MASK;
>      }
>  
>      if (env->v7m.exception != ARMV7M_EXCP_NMI) {
> @@ -7062,7 +7063,6 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
>           * which security state's faultmask to clear. (v8M ARM ARM R_KBNF.)
>           */
>          if (arm_feature(env, ARM_FEATURE_M_SECURITY)) {
> -            exc_secure = excret & R_V7M_EXCRET_ES_MASK;
>              if (armv7m_nvic_raw_execution_priority(env->nvic) >= 0) {
>                  env->v7m.faultmask[exc_secure] = 0;
>              }
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-07-21 19:48 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-20 14:56 [Qemu-devel] [PATCH 0/4] target/arm: Implement tailchaining for M profile cores Peter Maydell
2018-07-20 14:56 ` [Qemu-devel] [PATCH 1/4] target/arm: Improve exception-taken logging Peter Maydell
2018-07-20 16:15   ` Richard Henderson
2018-07-21 19:47   ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
2018-07-20 14:56 ` [Qemu-devel] [PATCH 2/4] target/arm: Initialize exc_secure correctly in do_v7m_exception_exit() Peter Maydell
2018-07-20 16:17   ` Richard Henderson
2018-07-21 19:48   ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
2018-07-20 14:56 ` [Qemu-devel] [PATCH 3/4] target/arm: Restore M-profile CONTROL.SPSEL before any tailchaining Peter Maydell
2018-07-20 16:19   ` Richard Henderson
2018-07-20 14:56 ` [Qemu-devel] [PATCH 4/4] target/arm: Implement tailchaining for M profile cores Peter Maydell
2018-07-20 16:22   ` Richard Henderson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.