diff mbox series

[v3,7/7] arm: fix aa64_generate_debug_exceptions to work with EL2

Message ID 20181109152119.9242-8-alex.bennee@linaro.org
State Superseded
Headers show
Series KVM Guest Debug fixes (plus TCG EL2 debug tweaks) | expand

Commit Message

Alex Bennée Nov. 9, 2018, 3:21 p.m. UTC 
The test was incomplete and incorrectly caused debug exceptions to be
generated when returning to EL2 after a failed attempt to single-step
an EL1 instruction. Fix this while cleaning up the function a little.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>


---
v3
  - further re-arrangement as suggested by rth
---
 target/arm/cpu.h | 39 ++++++++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 15 deletions(-)

-- 
2.17.1

Comments

Richard Henderson Nov. 11, 2018, 2 p.m. UTC | #1 
On 11/9/18 4:21 PM, Alex Bennée wrote:
> The test was incomplete and incorrectly caused debug exceptions to be

> generated when returning to EL2 after a failed attempt to single-step

> an EL1 instruction. Fix this while cleaning up the function a little.

> 

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

> 

> ---

> v3

>   - further re-arrangement as suggested by rth

> ---

>  target/arm/cpu.h | 39 ++++++++++++++++++++++++---------------

>  1 file changed, 24 insertions(+), 15 deletions(-)


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



r~
Peter Maydell Nov. 11, 2018, 2:47 p.m. UTC | #2 
On 9 November 2018 at 15:21, Alex Bennée <alex.bennee@linaro.org> wrote:
> The test was incomplete and incorrectly caused debug exceptions to be

> generated when returning to EL2 after a failed attempt to single-step

> an EL1 instruction. Fix this while cleaning up the function a little.


> @@ -2833,9 +2845,6 @@ static inline bool aa32_generate_debug_exceptions(CPUARMState *env)

>   * since the pseudocode has it at all callsites except for the one in

>   * CheckSoftwareStep(), where it is elided because both branches would

>   * always return the same value.

> - *

> - * Parts of the pseudocode relating to EL2 and EL3 are omitted because we

> - * don't yet implement those exception levels or their associated trap bits.

>   */


In hindsight I regret not standardizing on a greppable tag for
marking these kinds of "we don't do X because we don't implement
feature Y" comments...

thanks
-- PMM
diff mbox series

Patch

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 1efff21a18..814ff69bc2 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2764,23 +2764,35 @@  static inline bool arm_v7m_csselr_razwi(ARMCPU *cpu)
     return (cpu->clidr & R_V7M_CLIDR_CTYPE_ALL_MASK) != 0;
 }
 
+/* See AArch64.GenerateDebugExceptionsFrom() in ARM ARM pseudocode */
 static inline bool aa64_generate_debug_exceptions(CPUARMState *env)
 {
-    if (arm_is_secure(env)) {
-        /* MDCR_EL3.SDD disables debug events from Secure state */
-        if (extract32(env->cp15.mdcr_el3, 16, 1) != 0
-            || arm_current_el(env) == 3) {
-            return false;
-        }
+    int cur_el = arm_current_el(env);
+    int debug_el;
+
+    if (cur_el == 3) {
+        return false;
     }
 
-    if (arm_current_el(env) == arm_debug_target_el(env)) {
-        if ((extract32(env->cp15.mdscr_el1, 13, 1) == 0)
-            || (env->daif & PSTATE_D)) {
-            return false;
-        }
+    /* MDCR_EL3.SDD disables debug events from Secure state */
+    if (arm_is_secure_below_el3(env)
+        && extract32(env->cp15.mdcr_el3, 16, 1)) {
+        return false;
     }
-    return true;
+
+    /*
+     * Same EL to same EL debug exceptions need MDSCR_KDE enabled
+     * while not masking the (D)ebug bit in DAIF.
+     */
+    debug_el = arm_debug_target_el(env);
+
+    if (cur_el == debug_el) {
+        return extract32(env->cp15.mdscr_el1, 13, 1)
+            && !(env->daif & PSTATE_D);
+    }
+
+    /* Otherwise the debug target needs to be a higher EL */
+    return debug_el > cur_el;
 }
 
 static inline bool aa32_generate_debug_exceptions(CPUARMState *env)
@@ -2833,9 +2845,6 @@  static inline bool aa32_generate_debug_exceptions(CPUARMState *env)
  * since the pseudocode has it at all callsites except for the one in
  * CheckSoftwareStep(), where it is elided because both branches would
  * always return the same value.
- *
- * Parts of the pseudocode relating to EL2 and EL3 are omitted because we
- * don't yet implement those exception levels or their associated trap bits.
  */
 static inline bool arm_generate_debug_exceptions(CPUARMState *env)
 {