diff mbox

[for-2.5,2/2] target-arm: Report S/NS status in the CPU debug logs

Message ID 1445883178-576-3-git-send-email-peter.maydell@linaro.org
State Accepted
Headers show

Commit Message

Peter Maydell Oct. 26, 2015, 6:12 p.m. UTC 
If this CPU supports EL3, enhance the printing of the current
CPU mode in debug logging to distinguish S from NS modes as
appropriate.

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

---
 target-arm/translate-a64.c | 11 ++++++++++-
 target-arm/translate.c     | 12 +++++++++++-
 2 files changed, 21 insertions(+), 2 deletions(-)

-- 
1.9.1

Comments

Alex Bennée Oct. 29, 2015, 3:15 p.m. UTC | #1 
Peter Maydell <peter.maydell@linaro.org> writes:

> If this CPU supports EL3, enhance the printing of the current

> CPU mode in debug logging to distinguish S from NS modes as

> appropriate.

>

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

> ---

>  target-arm/translate-a64.c | 11 ++++++++++-

>  target-arm/translate.c     | 12 +++++++++++-

>  2 files changed, 21 insertions(+), 2 deletions(-)

>

> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c

> index ccefa7b..8ebdcb7 100644

> --- a/target-arm/translate-a64.c

> +++ b/target-arm/translate-a64.c

> @@ -127,6 +127,7 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,

>      uint32_t psr = pstate_read(env);

>      int i;

>      int el = arm_current_el(env);

> +    const char *ns_status;

>  

>      cpu_fprintf(f, "PC=%016"PRIx64"  SP=%016"PRIx64"\n",

>              env->pc, env->xregs[31]);

> @@ -138,12 +139,20 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,

>              cpu_fprintf(f, " ");

>          }

>      }

> -    cpu_fprintf(f, "\nPSTATE=%08x %c%c%c%c EL%d%c\n",

> +

> +    if (arm_feature(env, ARM_FEATURE_EL3) && el != 3) {

> +        ns_status = env->cp15.scr_el3 & SCR_NS ? "NS " : "S ";

> +    } else {

> +        ns_status = "";

> +    }


Looks fine to me. I might of gone for a default:

const char *ns_status = "legacy";
...
    if (arm_feature(env, ARM_FEATURE_EL3) && el != 3) {
        ns_status = env->cp15.scr_el3 & SCR_NS ? "NS " : "S ";
    }

But otherwise:

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



>                  psr,

>                  psr & PSTATE_N ? 'N' : '-',

>                  psr & PSTATE_Z ? 'Z' : '-',

>                  psr & PSTATE_C ? 'C' : '-',

>                  psr & PSTATE_V ? 'V' : '-',

> +                ns_status,

>                  el,

>                  psr & PSTATE_SP ? 'h' : 't');

>  

> diff --git a/target-arm/translate.c b/target-arm/translate.c

> index 9f1d740..5f2346a 100644

> --- a/target-arm/translate.c

> +++ b/target-arm/translate.c

> @@ -11556,6 +11556,7 @@ void arm_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,

>      CPUARMState *env = &cpu->env;

>      int i;

>      uint32_t psr;

> +    const char *ns_status;

>  

>      if (is_a64(env)) {

>          aarch64_cpu_dump_state(cs, f, cpu_fprintf, flags);

> @@ -11570,13 +11571,22 @@ void arm_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,

>              cpu_fprintf(f, " ");

>      }

>      psr = cpsr_read(env);

> -    cpu_fprintf(f, "PSR=%08x %c%c%c%c %c %s%d\n",

> +

> +    if (arm_feature(env, ARM_FEATURE_EL3) &&

> +        (psr & CPSR_M) != ARM_CPU_MODE_MON) {

> +        ns_status = env->cp15.scr_el3 & SCR_NS ? "NS " : "S ";

> +    } else {

> +        ns_status = "";

> +    }

> +

> +    cpu_fprintf(f, "PSR=%08x %c%c%c%c %c %s%s%d\n",

>                  psr,

>                  psr & (1 << 31) ? 'N' : '-',

>                  psr & (1 << 30) ? 'Z' : '-',

>                  psr & (1 << 29) ? 'C' : '-',

>                  psr & (1 << 28) ? 'V' : '-',

>                  psr & CPSR_T ? 'T' : 'A',

> +                ns_status,

>                  cpu_mode_names[psr & 0xf], (psr & 0x10) ? 32 : 26);

>  

>      if (flags & CPU_DUMP_FPU) {



-- 
Alex Bennée
Peter Maydell Oct. 29, 2015, 3:21 p.m. UTC | #2 
On 29 October 2015 at 15:15, Alex Bennée <alex.bennee@linaro.org> wrote:
>

> Peter Maydell <peter.maydell@linaro.org> writes:

>

>> If this CPU supports EL3, enhance the printing of the current

>> CPU mode in debug logging to distinguish S from NS modes as

>> appropriate.

>>

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

>> ---

>>  target-arm/translate-a64.c | 11 ++++++++++-

>>  target-arm/translate.c     | 12 +++++++++++-

>>  2 files changed, 21 insertions(+), 2 deletions(-)

>>

>> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c

>> index ccefa7b..8ebdcb7 100644

>> --- a/target-arm/translate-a64.c

>> +++ b/target-arm/translate-a64.c

>> @@ -127,6 +127,7 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,

>>      uint32_t psr = pstate_read(env);

>>      int i;

>>      int el = arm_current_el(env);

>> +    const char *ns_status;

>>

>>      cpu_fprintf(f, "PC=%016"PRIx64"  SP=%016"PRIx64"\n",

>>              env->pc, env->xregs[31]);

>> @@ -138,12 +139,20 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,

>>              cpu_fprintf(f, " ");

>>          }

>>      }

>> -    cpu_fprintf(f, "\nPSTATE=%08x %c%c%c%c EL%d%c\n",

>> +

>> +    if (arm_feature(env, ARM_FEATURE_EL3) && el != 3) {

>> +        ns_status = env->cp15.scr_el3 & SCR_NS ? "NS " : "S ";

>> +    } else {

>> +        ns_status = "";

>> +    }

>

> Looks fine to me. I might of gone for a default:

>

> const char *ns_status = "legacy";

> ...

>     if (arm_feature(env, ARM_FEATURE_EL3) && el != 3) {

>         ns_status = env->cp15.scr_el3 & SCR_NS ? "NS " : "S ";

>     }


We don't want to print "legacy", as that is just confusing
for CPUs which don't have TrustZone. "" is the right
value there.

thanks
-- PMM
diff mbox

Patch

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index ccefa7b..8ebdcb7 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -127,6 +127,7 @@  void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
     uint32_t psr = pstate_read(env);
     int i;
     int el = arm_current_el(env);
+    const char *ns_status;
 
     cpu_fprintf(f, "PC=%016"PRIx64"  SP=%016"PRIx64"\n",
             env->pc, env->xregs[31]);
@@ -138,12 +139,20 @@  void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
             cpu_fprintf(f, " ");
         }
     }
-    cpu_fprintf(f, "\nPSTATE=%08x %c%c%c%c EL%d%c\n",
+
+    if (arm_feature(env, ARM_FEATURE_EL3) && el != 3) {
+        ns_status = env->cp15.scr_el3 & SCR_NS ? "NS " : "S ";
+    } else {
+        ns_status = "";
+    }
+
+    cpu_fprintf(f, "\nPSTATE=%08x %c%c%c%c %sEL%d%c\n",
                 psr,
                 psr & PSTATE_N ? 'N' : '-',
                 psr & PSTATE_Z ? 'Z' : '-',
                 psr & PSTATE_C ? 'C' : '-',
                 psr & PSTATE_V ? 'V' : '-',
+                ns_status,
                 el,
                 psr & PSTATE_SP ? 'h' : 't');
 
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 9f1d740..5f2346a 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -11556,6 +11556,7 @@  void arm_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
     CPUARMState *env = &cpu->env;
     int i;
     uint32_t psr;
+    const char *ns_status;
 
     if (is_a64(env)) {
         aarch64_cpu_dump_state(cs, f, cpu_fprintf, flags);
@@ -11570,13 +11571,22 @@  void arm_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
             cpu_fprintf(f, " ");
     }
     psr = cpsr_read(env);
-    cpu_fprintf(f, "PSR=%08x %c%c%c%c %c %s%d\n",
+
+    if (arm_feature(env, ARM_FEATURE_EL3) &&
+        (psr & CPSR_M) != ARM_CPU_MODE_MON) {
+        ns_status = env->cp15.scr_el3 & SCR_NS ? "NS " : "S ";
+    } else {
+        ns_status = "";
+    }
+
+    cpu_fprintf(f, "PSR=%08x %c%c%c%c %c %s%s%d\n",
                 psr,
                 psr & (1 << 31) ? 'N' : '-',
                 psr & (1 << 30) ? 'Z' : '-',
                 psr & (1 << 29) ? 'C' : '-',
                 psr & (1 << 28) ? 'V' : '-',
                 psr & CPSR_T ? 'T' : 'A',
+                ns_status,
                 cpu_mode_names[psr & 0xf], (psr & 0x10) ? 32 : 26);
 
     if (flags & CPU_DUMP_FPU) {