[Qemu-devel] [PATCH for-2.5 0/2] target-arm: improve debug log display of PSTATE/PSR

[Qemu-devel] [PATCH for-2.5 0/2] target-arm: improve debug log display of PSTATE/PSR

[Peter Maydell]

These are two small patches which I put together while working
on support for TZ in ARM, which improve the display of the
PSTATE and PSR in the -d cpu debug logging.

The first one brings the handling of PSTATE in AArch64 into line with
the handling of PSR in AArch32, by putting it on its own line and
with a similar format to the 32-bit version.

The second one adds NS/S indicators as appropriate so you can distinguish
NS svc32 from S svc32, and so on.

Sample output:

Trace 0x7f9563dc3b00 [4001031c] 
R00=400100b4 R01=4120c0f1 R02=000f0000 R03=00000008
R04=40008000 R05=00000000 R06=0000000e R07=ffffffff
R08=48000000 R09=412fc0f1 R10=00000000 R11=10201105
R12=400100c8 R13=00000000 R14=400103ec R15=4001031c
PSR=200001d3 --C- A NS svc32


Trace 0x7f303d6b7028 [0000000040000000] 
PC=0000000040000000  SP=0000000000000000
X00=0000000000000000 X01=0000000000000000 X02=0000000000000000 X03=0000000000000000
X04=0000000000000000 X05=0000000000000000 X06=0000000000000000 X07=0000000000000000
X08=0000000000000000 X09=0000000000000000 X10=0000000000000000 X11=0000000000000000
X12=0000000000000000 X13=0000000000000000 X14=0000000000000000 X15=0000000000000000
X16=0000000000000000 X17=0000000000000000 X18=0000000000000000 X19=0000000000000000
X20=0000000000000000 X21=0000000000000000 X22=0000000000000000 X23=0000000000000000
X24=0000000000000000 X25=0000000000000000 X26=0000000000000000 X27=0000000000000000
X28=0000000000000000 X29=0000000000000000 X30=0000000000000000 
PSTATE=400003c5 -Z-- EL1h

(The S/NS indicator is only displayed if appropriate, ie if the
CPU supports EL3 at all and if we're not in EL3 or Monitor mode.)

These don't strictly speaking have to go into 2.5, but given they're
pretty small and safe it doesn't seem too unreasonable to put them
there.

Peter Maydell (2):
  target-arm: Bring AArch64 debug CPU display of PSTATE into line with
    AArch32
  target-arm: Report S/NS status in the CPU debug logs

 target-arm/translate-a64.c | 17 ++++++++++++++---
 target-arm/translate.c     | 12 +++++++++++-
 2 files changed, 25 insertions(+), 4 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH for-2.5 1/2] target-arm: Bring AArch64 debug CPU display of PSTATE into line with AArch32

[Peter Maydell]

The AArch64 debug CPU display of PSTATE as "PSTATE=200003c5 (flags --C-)"
on the end of the same line as the last of the general purpose registers
is unnecessarily different from the AArch32 display of PSR as
"PSR=200001d3 --C- A svc32" on its own line. Update the AArch64
code to put PSTATE in its own line and in the same format, including
printing the exception level (mode).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 19f9d8d..ccefa7b 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -126,6 +126,7 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
     CPUARMState *env = &cpu->env;
     uint32_t psr = pstate_read(env);
     int i;
+    int el = arm_current_el(env);
 
     cpu_fprintf(f, "PC=%016"PRIx64"  SP=%016"PRIx64"\n",
             env->pc, env->xregs[31]);
@@ -137,13 +138,14 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
             cpu_fprintf(f, " ");
         }
     }
-    cpu_fprintf(f, "PSTATE=%08x (flags %c%c%c%c)\n",
+    cpu_fprintf(f, "\nPSTATE=%08x %c%c%c%c EL%d%c\n",
                 psr,
                 psr & PSTATE_N ? 'N' : '-',
                 psr & PSTATE_Z ? 'Z' : '-',
                 psr & PSTATE_C ? 'C' : '-',
-                psr & PSTATE_V ? 'V' : '-');
-    cpu_fprintf(f, "\n");
+                psr & PSTATE_V ? 'V' : '-',
+                el,
+                psr & PSTATE_SP ? 'h' : 't');
 
     if (flags & CPU_DUMP_FPU) {
         int numvfpregs = 32;
-- 
1.9.1

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

[Peter Maydell]

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 = "";
+    }
+
+    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) {
-- 
1.9.1

Re: [Qemu-devel] [PATCH for-2.5 0/2] target-arm: improve debug log display of PSTATE/PSR

[Edgar E. Iglesias]

On Mon, Oct 26, 2015 at 06:12:56PM +0000, Peter Maydell wrote:
> These are two small patches which I put together while working
> on support for TZ in ARM, which improve the display of the
> PSTATE and PSR in the -d cpu debug logging.

Looks good to me:

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Cheers,
Edgar

> 
> The first one brings the handling of PSTATE in AArch64 into line with
> the handling of PSR in AArch32, by putting it on its own line and
> with a similar format to the 32-bit version.
> 
> The second one adds NS/S indicators as appropriate so you can distinguish
> NS svc32 from S svc32, and so on.
> 
> Sample output:
> 
> Trace 0x7f9563dc3b00 [4001031c] 
> R00=400100b4 R01=4120c0f1 R02=000f0000 R03=00000008
> R04=40008000 R05=00000000 R06=0000000e R07=ffffffff
> R08=48000000 R09=412fc0f1 R10=00000000 R11=10201105
> R12=400100c8 R13=00000000 R14=400103ec R15=4001031c
> PSR=200001d3 --C- A NS svc32
> 
> 
> Trace 0x7f303d6b7028 [0000000040000000] 
> PC=0000000040000000  SP=0000000000000000
> X00=0000000000000000 X01=0000000000000000 X02=0000000000000000 X03=0000000000000000
> X04=0000000000000000 X05=0000000000000000 X06=0000000000000000 X07=0000000000000000
> X08=0000000000000000 X09=0000000000000000 X10=0000000000000000 X11=0000000000000000
> X12=0000000000000000 X13=0000000000000000 X14=0000000000000000 X15=0000000000000000
> X16=0000000000000000 X17=0000000000000000 X18=0000000000000000 X19=0000000000000000
> X20=0000000000000000 X21=0000000000000000 X22=0000000000000000 X23=0000000000000000
> X24=0000000000000000 X25=0000000000000000 X26=0000000000000000 X27=0000000000000000
> X28=0000000000000000 X29=0000000000000000 X30=0000000000000000 
> PSTATE=400003c5 -Z-- EL1h
> 
> (The S/NS indicator is only displayed if appropriate, ie if the
> CPU supports EL3 at all and if we're not in EL3 or Monitor mode.)
> 
> These don't strictly speaking have to go into 2.5, but given they're
> pretty small and safe it doesn't seem too unreasonable to put them
> there.
> 
> Peter Maydell (2):
>   target-arm: Bring AArch64 debug CPU display of PSTATE into line with
>     AArch32
>   target-arm: Report S/NS status in the CPU debug logs
> 
>  target-arm/translate-a64.c | 17 ++++++++++++++---
>  target-arm/translate.c     | 12 +++++++++++-
>  2 files changed, 25 insertions(+), 4 deletions(-)
> 
> -- 
> 1.9.1
> 

Re: [Qemu-devel] [PATCH for-2.5 1/2] target-arm: Bring AArch64 debug CPU display of PSTATE into line with AArch32

[Alex Bennée]

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

> The AArch64 debug CPU display of PSTATE as "PSTATE=200003c5 (flags --C-)"
> on the end of the same line as the last of the general purpose registers
> is unnecessarily different from the AArch32 display of PSR as
> "PSR=200001d3 --C- A svc32" on its own line. Update the AArch64
> code to put PSTATE in its own line and in the same format, including
> printing the exception level (mode).
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

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

> ---
>  target-arm/translate-a64.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index 19f9d8d..ccefa7b 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -126,6 +126,7 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
>      CPUARMState *env = &cpu->env;
>      uint32_t psr = pstate_read(env);
>      int i;
> +    int el = arm_current_el(env);
>  
>      cpu_fprintf(f, "PC=%016"PRIx64"  SP=%016"PRIx64"\n",
>              env->pc, env->xregs[31]);
> @@ -137,13 +138,14 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
>              cpu_fprintf(f, " ");
>          }
>      }
> -    cpu_fprintf(f, "PSTATE=%08x (flags %c%c%c%c)\n",
> +    cpu_fprintf(f, "\nPSTATE=%08x %c%c%c%c EL%d%c\n",
>                  psr,
>                  psr & PSTATE_N ? 'N' : '-',
>                  psr & PSTATE_Z ? 'Z' : '-',
>                  psr & PSTATE_C ? 'C' : '-',
> -                psr & PSTATE_V ? 'V' : '-');
> -    cpu_fprintf(f, "\n");
> +                psr & PSTATE_V ? 'V' : '-',
> +                el,
> +                psr & PSTATE_SP ? 'h' : 't');
>  
>      if (flags & CPU_DUMP_FPU) {
>          int numvfpregs = 32;


-- 
Alex Bennée

Re: [Qemu-devel] [PATCH for-2.5 2/2] target-arm: Report S/NS status in the CPU debug logs

[Alex Bennée]

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

Re: [Qemu-devel] [PATCH for-2.5 2/2] target-arm: Report S/NS status in the CPU debug logs

[Peter Maydell]

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