Re: [Qemu-devel] [PATCH v2] target-ppc: improve "info registers" by printing SPRs

[Alexey Kardashevskiy]

On 03/23/2014 01:43 AM, Stuart Brady wrote:
> On Sat, Mar 22, 2014 at 11:25:49PM +1100, Alexey Kardashevskiy wrote:
>> This adds printing of all SPR registers registered for a CPU.
>>
>> This removes "SPR_" prefix from SPR name to reduce the output.
>>
>> Cc: Fabien Chouteau <address@hidden>
>> Signed-off-by: Alexey Kardashevskiy <address@hidden>
>> ---
>> Changes:
>> v2:
>> * removed "switch (env->mmu_model)"
>> * added "\n" if the last line has less than 4 registers
>> ---
>>  target-ppc/translate.c      | 96 
>> +++++++--------------------------------------
>>  target-ppc/translate_init.c | 40 +++++++++----------
>>  2 files changed, 35 insertions(+), 101 deletions(-)
>>
>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>> index e3fcb03..06f195a 100644
>> --- a/target-ppc/translate.c
>> +++ b/target-ppc/translate.c
>> @@ -11116,7 +11116,7 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, 
>> fprintf_function cpu_fprintf,
>>  
>>      PowerPCCPU *cpu = POWERPC_CPU(cs);
>>      CPUPPCState *env = &cpu->env;
>> -    int i;
>> +    int i, j;
>>  
>>      cpu_fprintf(f, "NIP " TARGET_FMT_lx "   LR " TARGET_FMT_lx " CTR "
>>                  TARGET_FMT_lx " XER " TARGET_FMT_lx "\n",
>> @@ -11167,54 +11167,22 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, 
>> fprintf_function cpu_fprintf,
>>              cpu_fprintf(f, "\n");
>>      }
>>      cpu_fprintf(f, "FPSCR " TARGET_FMT_lx "\n", env->fpscr);
>> -#if !defined(CONFIG_USER_ONLY)
>> -    cpu_fprintf(f, " SRR0 " TARGET_FMT_lx "  SRR1 " TARGET_FMT_lx
>> -                   "    PVR " TARGET_FMT_lx " VRSAVE " TARGET_FMT_lx "\n",
>> -                env->spr[SPR_SRR0], env->spr[SPR_SRR1],
>> -                env->spr[SPR_PVR], env->spr[SPR_VRSAVE]);
>>  
>> -    cpu_fprintf(f, "SPRG0 " TARGET_FMT_lx " SPRG1 " TARGET_FMT_lx
>> -                   "  SPRG2 " TARGET_FMT_lx "  SPRG3 " TARGET_FMT_lx "\n",
>> -                env->spr[SPR_SPRG0], env->spr[SPR_SPRG1],
>> -                env->spr[SPR_SPRG2], env->spr[SPR_SPRG3]);
>> -
>> -    cpu_fprintf(f, "SPRG4 " TARGET_FMT_lx " SPRG5 " TARGET_FMT_lx
>> -                   "  SPRG6 " TARGET_FMT_lx "  SPRG7 " TARGET_FMT_lx "\n",
>> -                env->spr[SPR_SPRG4], env->spr[SPR_SPRG5],
>> -                env->spr[SPR_SPRG6], env->spr[SPR_SPRG7]);
>> -
>> -    if (env->excp_model == POWERPC_EXCP_BOOKE) {
>> -        cpu_fprintf(f, "CSRR0 " TARGET_FMT_lx " CSRR1 " TARGET_FMT_lx
>> -                       " MCSRR0 " TARGET_FMT_lx " MCSRR1 " TARGET_FMT_lx 
>> "\n",
>> -                    env->spr[SPR_BOOKE_CSRR0], env->spr[SPR_BOOKE_CSRR1],
>> -                    env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSRR1]);
>> -
>> -        cpu_fprintf(f, "  TCR " TARGET_FMT_lx "   TSR " TARGET_FMT_lx
>> -                       "    ESR " TARGET_FMT_lx "   DEAR " TARGET_FMT_lx 
>> "\n",
>> -                    env->spr[SPR_BOOKE_TCR], env->spr[SPR_BOOKE_TSR],
>> -                    env->spr[SPR_BOOKE_ESR], env->spr[SPR_BOOKE_DEAR]);
>> -
>> -        cpu_fprintf(f, "  PIR " TARGET_FMT_lx " DECAR " TARGET_FMT_lx
>> -                       "   IVPR " TARGET_FMT_lx "   EPCR " TARGET_FMT_lx 
>> "\n",
>> -                    env->spr[SPR_BOOKE_PIR], env->spr[SPR_BOOKE_DECAR],
>> -                    env->spr[SPR_BOOKE_IVPR], env->spr[SPR_BOOKE_EPCR]);
>> -
>> -        cpu_fprintf(f, " MCSR " TARGET_FMT_lx " SPRG8 " TARGET_FMT_lx
>> -                       "    EPR " TARGET_FMT_lx "\n",
>> -                    env->spr[SPR_BOOKE_MCSR], env->spr[SPR_BOOKE_SPRG8],
>> -                    env->spr[SPR_BOOKE_EPR]);
>> -
>> -        /* FSL-specific */
>> -        cpu_fprintf(f, " MCAR " TARGET_FMT_lx "  PID1 " TARGET_FMT_lx
>> -                       "   PID2 " TARGET_FMT_lx "    SVR " TARGET_FMT_lx 
>> "\n",
>> -                    env->spr[SPR_Exxx_MCAR], env->spr[SPR_BOOKE_PID1],
>> -                    env->spr[SPR_BOOKE_PID2], env->spr[SPR_E500_SVR]);
>> -
>> -        /*
>> -         * IVORs are left out as they are large and do not change often --
>> -         * they can be read with "p $ivor0", "p $ivor1", etc.
>> -         */
>> +    for (i = 0, j = 0; i < ARRAY_SIZE(env->spr_cb); i++) {
>> +        ppc_spr_t *spr = &env->spr_cb[i];
>> +
>> +        if (!spr->name) {
>> +            continue;
>> +        }
> 
> This would leave the output without a trailing newline if the last spr
> doesn't have a name registered.  Is it necessary to handle unnamed sprs
> at all (maybe add an assert to the registration function)? ... or would
> we just want to warn about them here?


In the current QEMU I do not see any place where SPR would be registered
without a name so I would not bother.


> FWIW, my approach is often to write an outer loop that process one item
> of output at a time, with an inner loop to obtain the next item of data,
> and with prefixing of separators, as you then have a far simpler special
> case for 'j == 0' instead of 'i == ARRAY_SIZE(env->spr_cb) - 1'.  You can
> then unconditionally finish on a '\n'.

Oh. This is embarrassing, sorry :( This should do it, right? :)

+    for (i = 0, j = 0; i < ARRAY_SIZE(env->spr_cb); i++) {
+        ppc_spr_t *spr = &env->spr_cb[i];
+
+        if (!spr->name) {
+            continue;
+        }
+        if (j % 4) {
+            cpu_fprintf(f, " ");
+        } else if (j) {
+            cpu_fprintf(f, "\n");
+        }
+        cpu_fprintf(f, "%-6s " TARGET_FMT_lx, spr->name, env->spr[i]);
+        j++;
     }
+    cpu_fprintf(f, "\n");



btw while grepping through the code, I found dump_ppc_sprs() which prints
this (first chunk is what my patch adds and the second chunk is from
dump_ppc_sprs()):


XER    0000000000000000 LR     0000000000000000 CTR    0000000000000000
UAMR   0000000000000000
DSCR   0000000000000000 DSISR  0000000000000000 DAR    0000000000000000
DECR   0000000000000000
SDR1   0000000000000005 SRR0   0000000000000000 SRR1   0000000000000000
CFAR   0000000000000000
AMR    0000000000000000 CTRLF  0000000080800000 CTRLT  0000000080800000
UAMOR  0000000000000000
VRSAVE 0000000000000000 TBL    0000000000000000 TBU    0000000000000000
SPRG0  0000000000000000
SPRG1  0000000000000000 SPRG2  0000000000000000 SPRG3  0000000000000000 EAR
   0000000000000000
TBL    0000000000000000 TBU    0000000000000000 PVR    00000000003f0201
SPURR  0000000000000000
PURR   0000000000000000 LPCR   0000000000007005 MMCRA  0000000000000000 PPR
   0000000000000000
UMMCR0 0000000000000000 UPMC1  0000000000000000 UPMC2  0000000000000000
USIAR  0000000000000000
UMMCR1 0000000000000000 UPMC3  0000000000000000 UPMC4  0000000000000000
PMC5   0000000000000000
PMC6   0000000000000000 MMCR0  0000000000000000 PMC1   0000000000000000
PMC2   0000000000000000
SIAR   0000000000000000 MMCR1  0000000000000000 PMC3   0000000000000000
PMC4   0000000000000000
IABR   0000000000000000 DABR   0000000000000000 ICTC   0000000000000000 PIR
   0000000000000000





Special purpose registers:
SPR:    1 (001) XER      swr uwr
SPR:    8 (008) LR       swr uwr
SPR:    9 (009) CTR      swr uwr
SPR:   12 (00c) UAMR     swr uwr
SPR:   17 (011) DSCR     swr u--
SPR:   18 (012) DSISR    swr u--
SPR:   19 (013) DAR      swr u--
SPR:   22 (016) DECR     swr u--
SPR:   25 (019) SDR1     swr u--
SPR:   26 (01a) SRR0     swr u--
SPR:   27 (01b) SRR1     swr u--
SPR:   28 (01c) CFAR     swr u--
SPR:   29 (01d) AMR      swr u--
SPR:  136 (088) CTRLF    s-r u--
SPR:  152 (098) CTRLT    sw- u--
SPR:  157 (09d) UAMOR    swr u--
SPR:  256 (100) VRSAVE   swr uwr
SPR:  268 (10c) TBL      s-r u-r
SPR:  269 (10d) TBU      s-r u-r
SPR:  272 (110) SPRG0    swr u--
SPR:  273 (111) SPRG1    swr u--
SPR:  274 (112) SPRG2    swr u--
SPR:  275 (113) SPRG3    swr u--
SPR:  282 (11a) EAR      swr u--
SPR:  284 (11c) TBL      swr u-r
SPR:  285 (11d) TBU      swr u-r
SPR:  287 (11f) PVR      s-r u--
SPR:  308 (134) SPURR    s-r u-r
SPR:  309 (135) PURR     s-r u-r
SPR:  318 (13e) LPCR     swr u--
SPR:  770 (302) MMCRA    swr u--
SPR:  896 (380) PPR      swr uwr
SPR:  936 (3a8) UMMCR0   s-r u-r
SPR:  937 (3a9) UPMC1    s-r u-r
SPR:  938 (3aa) UPMC2    s-r u-r
SPR:  939 (3ab) USIAR    s-r u-r
SPR:  940 (3ac) UMMCR1   s-r u-r
SPR:  941 (3ad) UPMC3    s-r u-r
SPR:  942 (3ae) UPMC4    s-r u-r
SPR:  945 (3b1) PMC5     swr u--
SPR:  946 (3b2) PMC6     swr u--
SPR:  952 (3b8) MMCR0    swr u--
SPR:  953 (3b9) PMC1     swr u--
SPR:  954 (3ba) PMC2     swr u--
SPR:  955 (3bb) SIAR     s-r u--
SPR:  956 (3bc) MMCR1    swr u--
SPR:  957 (3bd) PMC3     swr u--
SPR:  958 (3be) PMC4     swr u--
SPR: 1010 (3f2) IABR     swr u--
SPR: 1013 (3f5) DABR     swr u--
SPR: 1019 (3fb) ICTC     swr u--
SPR: 1023 (3ff) PIR      swr u--


Which is nicer/more useful?
The characters at the end tell what handler (read/write, oea/uea) is
defined for SPR.


-- 
Alexey