diff mbox

[4/7] target-arm: Update arm_generate_debug_exceptions() to handle EL2/EL3

Message ID 1454506721-11843-5-git-send-email-peter.maydell@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Maydell Feb. 3, 2016, 1:38 p.m. UTC
The arm_generate_debug_exceptions() function as originally implemented
assumes no EL2 or EL3. Since we now have much more of an implementation
of those now, fix this assumption.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu.h | 48 +++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 43 insertions(+), 5 deletions(-)

Comments

Alex Bennée Feb. 5, 2016, 2:09 p.m. UTC | #1
Peter Maydell <peter.maydell@linaro.org> writes:

> The arm_generate_debug_exceptions() function as originally implemented
> assumes no EL2 or EL3. Since we now have much more of an implementation
> of those now, fix this assumption.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/cpu.h | 48 +++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 43 insertions(+), 5 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index cf2df50..0fb79d0 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1742,9 +1742,7 @@ typedef enum ARMASIdx {
>      ARMASIdx_S = 1,
>  } ARMASIdx;
>
> -/* Return the Exception Level targeted by debug exceptions;
> - * currently always EL1 since we don't implement EL2 or EL3.
> - */
> +/* Return the Exception Level targeted by debug exceptions. */
>  static inline int arm_debug_target_el(CPUARMState *env)
>  {
>      bool secure = arm_is_secure(env);
> @@ -1767,6 +1765,14 @@ static inline int arm_debug_target_el(CPUARMState *env)
>
>  static inline bool aa64_generate_debug_exceptions(CPUARMState *env)
>  {
> +    if (arm_is_secure(env)) {
> +        /* MDCR_EL3.SDD disables debug events from Secure state */

Is it worth commenting that BRK still works?

> +        if (extract32(env->cp15.mdcr_el3,ctct 16, 1) != 0

The != 0 is superfluous here.

> +            || arm_current_el(env) == 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)) {
> @@ -1778,10 +1784,42 @@ static inline bool aa64_generate_debug_exceptions(CPUARMState *env)
>
>  static inline bool aa32_generate_debug_exceptions(CPUARMState *env)
>  {
> -    if (arm_current_el(env) == 0 && arm_el_is_aa64(env, 1)) {
> +    int el = arm_current_el(env);
> +
> +    if (el == 0 && arm_el_is_aa64(env, 1)) {
>          return aa64_generate_debug_exceptions(env);
>      }
> -    return arm_current_el(env) != 2;
> +
> +    if (arm_is_secure(env)) {
> +        int spd;
> +
> +        if (el == 0 && (env->cp15.sder & 1)) {
> +            /* SDER.SUIDEN means debug exceptions from Secure EL0
> +             * are always enabled. Otherwise they are controlled by
> +             * SDCR.SPD like those from other Secure ELs.
> +             */
> +            return true;
> +        }
> +
> +        spd = extract32(env->cp15.mdcr_el3, 14, 2);
> +        switch (spd) {
> +        case 1:
> +            /* SPD == 0b01 is reserved, but behaves as 0b00. */
> +        case 0:
> +            /* For 0b00 we return true if external secure invasive debug
> +             * is enabled. On real hardware this is controlled by external
> +             * signals to the core. QEMU always permits debug, and behaves
> +             * as if DBGEN, SPIDEN, NIDEN and SPNIDEN are all tied high.
> +             */
> +            return true;
> +        case 2:
> +            return false;
> +        case 3:
> +            return true;
> +        }
> +    }
> +
> +    return el != 2;
>  }
>
>  /* Return true if debugging exceptions are currently enabled.

Otherwise:

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

--
Alex Bennée
Peter Maydell Feb. 5, 2016, 3:55 p.m. UTC | #2
On 5 February 2016 at 14:09, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>> @@ -1767,6 +1765,14 @@ static inline int arm_debug_target_el(CPUARMState *env)
>>
>>  static inline bool aa64_generate_debug_exceptions(CPUARMState *env)
>>  {
>> +    if (arm_is_secure(env)) {
>> +        /* MDCR_EL3.SDD disables debug events from Secure state */
>
> Is it worth commenting that BRK still works?

This is true for all the checks made by this family of functions
so if we were going to say anything about BRK then this would not
be the right spot for it.

thanks
-- PMM
Sergey Fedorov Feb. 6, 2016, 6:43 p.m. UTC | #3
On 03.02.2016 16:38, Peter Maydell wrote:
> The arm_generate_debug_exceptions() function as originally implemented
> assumes no EL2 or EL3. Since we now have much more of an implementation
> of those now, fix this assumption.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com>

> ---
>  target-arm/cpu.h | 48 +++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 43 insertions(+), 5 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index cf2df50..0fb79d0 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1742,9 +1742,7 @@ typedef enum ARMASIdx {
>      ARMASIdx_S = 1,
>  } ARMASIdx;
>  
> -/* Return the Exception Level targeted by debug exceptions;
> - * currently always EL1 since we don't implement EL2 or EL3.
> - */
> +/* Return the Exception Level targeted by debug exceptions. */
>  static inline int arm_debug_target_el(CPUARMState *env)
>  {
>      bool secure = arm_is_secure(env);
> @@ -1767,6 +1765,14 @@ static inline int arm_debug_target_el(CPUARMState *env)
>  
>  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;
> +        }
> +    }
> +
>      if (arm_current_el(env) == arm_debug_target_el(env)) {
>          if ((extract32(env->cp15.mdscr_el1, 13, 1) == 0)
>              || (env->daif & PSTATE_D)) {
> @@ -1778,10 +1784,42 @@ static inline bool aa64_generate_debug_exceptions(CPUARMState *env)
>  
>  static inline bool aa32_generate_debug_exceptions(CPUARMState *env)
>  {
> -    if (arm_current_el(env) == 0 && arm_el_is_aa64(env, 1)) {
> +    int el = arm_current_el(env);
> +
> +    if (el == 0 && arm_el_is_aa64(env, 1)) {
>          return aa64_generate_debug_exceptions(env);
>      }
> -    return arm_current_el(env) != 2;
> +
> +    if (arm_is_secure(env)) {
> +        int spd;
> +
> +        if (el == 0 && (env->cp15.sder & 1)) {
> +            /* SDER.SUIDEN means debug exceptions from Secure EL0
> +             * are always enabled. Otherwise they are controlled by
> +             * SDCR.SPD like those from other Secure ELs.
> +             */
> +            return true;
> +        }
> +
> +        spd = extract32(env->cp15.mdcr_el3, 14, 2);
> +        switch (spd) {
> +        case 1:
> +            /* SPD == 0b01 is reserved, but behaves as 0b00. */
> +        case 0:
> +            /* For 0b00 we return true if external secure invasive debug
> +             * is enabled. On real hardware this is controlled by external
> +             * signals to the core. QEMU always permits debug, and behaves
> +             * as if DBGEN, SPIDEN, NIDEN and SPNIDEN are all tied high.
> +             */
> +            return true;
> +        case 2:
> +            return false;
> +        case 3:
> +            return true;
> +        }
> +    }
> +
> +    return el != 2;
>  }
>  
>  /* Return true if debugging exceptions are currently enabled.
diff mbox

Patch

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index cf2df50..0fb79d0 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1742,9 +1742,7 @@  typedef enum ARMASIdx {
     ARMASIdx_S = 1,
 } ARMASIdx;
 
-/* Return the Exception Level targeted by debug exceptions;
- * currently always EL1 since we don't implement EL2 or EL3.
- */
+/* Return the Exception Level targeted by debug exceptions. */
 static inline int arm_debug_target_el(CPUARMState *env)
 {
     bool secure = arm_is_secure(env);
@@ -1767,6 +1765,14 @@  static inline int arm_debug_target_el(CPUARMState *env)
 
 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;
+        }
+    }
+
     if (arm_current_el(env) == arm_debug_target_el(env)) {
         if ((extract32(env->cp15.mdscr_el1, 13, 1) == 0)
             || (env->daif & PSTATE_D)) {
@@ -1778,10 +1784,42 @@  static inline bool aa64_generate_debug_exceptions(CPUARMState *env)
 
 static inline bool aa32_generate_debug_exceptions(CPUARMState *env)
 {
-    if (arm_current_el(env) == 0 && arm_el_is_aa64(env, 1)) {
+    int el = arm_current_el(env);
+
+    if (el == 0 && arm_el_is_aa64(env, 1)) {
         return aa64_generate_debug_exceptions(env);
     }
-    return arm_current_el(env) != 2;
+
+    if (arm_is_secure(env)) {
+        int spd;
+
+        if (el == 0 && (env->cp15.sder & 1)) {
+            /* SDER.SUIDEN means debug exceptions from Secure EL0
+             * are always enabled. Otherwise they are controlled by
+             * SDCR.SPD like those from other Secure ELs.
+             */
+            return true;
+        }
+
+        spd = extract32(env->cp15.mdcr_el3, 14, 2);
+        switch (spd) {
+        case 1:
+            /* SPD == 0b01 is reserved, but behaves as 0b00. */
+        case 0:
+            /* For 0b00 we return true if external secure invasive debug
+             * is enabled. On real hardware this is controlled by external
+             * signals to the core. QEMU always permits debug, and behaves
+             * as if DBGEN, SPIDEN, NIDEN and SPNIDEN are all tied high.
+             */
+            return true;
+        case 2:
+            return false;
+        case 3:
+            return true;
+        }
+    }
+
+    return el != 2;
 }
 
 /* Return true if debugging exceptions are currently enabled.