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

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(-)

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) {

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