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

[Fabien Chouteau]

On 03/19/2014 03:17 PM, 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.
> 
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> ---
> 

Very useful patch Alexey,

I have few comments:

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

When the number of register is not a multiple of 4 a "\n" is missing.

e.g.

XER    00000000 LR     00000000 CTR    00000000 DECR   00000000
SRR0   0010c4d4 SRR1   00001030 PID    00000001 DECAR  000a7d8c
CSRR0  00000000 CSRR1  00000000 DEAR   01c7201c ESR    00000000
IVPR   00000000 USPRG0 00000000 USPRG4 00000000 USPRG5 00000000
USPRG6 00000000 USPRG7 00000000 TBL    00000000 TBU    00000000
SPRG0  02069030 SPRG1  00000000 SPRG2  002724bc SPRG3  80804080
SPRG4  00000000 SPRG5  00000000 SPRG6  00000000 SPRG7  00000000
TBL    00000000 TBU    00000000 PIR    00000000 PVR    80210030
DBSR   00000000 DBCR0  00000000 DBCR1  00000000 DBCR2  00000000
IAC1   00000000 IAC2   00000000 DAC1   00000000 DAC2   00000000
TSR    00000000 TCR    05c16000 IVOR0  00000100 IVOR1  00000200
IVOR2  00000300 IVOR3  00000400 IVOR4  00000500 IVOR5  00000600
IVOR6  00000700 IVOR7  00000800 IVOR8  00000900 IVOR9  00000a00
IVOR10 00000b00 IVOR11 00000c00 IVOR12 00000d00 IVOR13 00000e00
IVOR14 00000f00 IVOR15 00001000 SPEFSCR 00000000 BBEAR  00000000
BBTAR  00000000 L1CFG0 00003820 NPIDR  00000000 IVOR32 00001100
IVOR33 00001200 IVOR34 00001300 IVOR35 00001400 MCSRR0 00000000
MCSRR1 00000000 MCSR   00000000 MCAR   00000000 MAS0   00010002
MAS1   80001000 MAS2   01c72004 MAS3   01c72015 MAS4   00000000
MAS6   00010001 PID1   00000000 PID2   00000000 TLB0CFG 04110200
TLB1CFG 101cc010 EPR    00000000 MAS7   00000000 HID0   80804080
HID1   00023000 L1CSR0 00000001 L1CSR1 00000001 MMUCSR0 00000000
BUCSR  00000201 MMUCFG 00000000 SVR    00000000 (qemu) 


> +#if !defined(CONFIG_USER_ONLY)
>  
>  #if defined(TARGET_PPC64)
>      if (env->flags & POWERPC_FLAG_CFAR) {
> @@ -11233,25 +11201,8 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, 
> fprintf_function cpu_fprintf,
>      case POWERPC_MMU_2_06a:
>      case POWERPC_MMU_2_06d:
>  #endif
> -        cpu_fprintf(f, " SDR1 " TARGET_FMT_lx "   DAR " TARGET_FMT_lx
> -                       "  DSISR " TARGET_FMT_lx "\n", env->spr[SPR_SDR1],
> -                    env->spr[SPR_DAR], env->spr[SPR_DSISR]);
>          break;
>      case POWERPC_MMU_BOOKE206:
> -        cpu_fprintf(f, " MAS0 " TARGET_FMT_lx "  MAS1 " TARGET_FMT_lx
> -                       "   MAS2 " TARGET_FMT_lx "   MAS3 " TARGET_FMT_lx 
> "\n",
> -                    env->spr[SPR_BOOKE_MAS0], env->spr[SPR_BOOKE_MAS1],
> -                    env->spr[SPR_BOOKE_MAS2], env->spr[SPR_BOOKE_MAS3]);
> -
> -        cpu_fprintf(f, " MAS4 " TARGET_FMT_lx "  MAS6 " TARGET_FMT_lx
> -                       "   MAS7 " TARGET_FMT_lx "    PID " TARGET_FMT_lx 
> "\n",
> -                    env->spr[SPR_BOOKE_MAS4], env->spr[SPR_BOOKE_MAS6],
> -                    env->spr[SPR_BOOKE_MAS7], env->spr[SPR_BOOKE_PID]);
> -
> -        cpu_fprintf(f, "MMUCFG " TARGET_FMT_lx " TLB0CFG " TARGET_FMT_lx
> -                       " TLB1CFG " TARGET_FMT_lx "\n",
> -                    env->spr[SPR_MMUCFG], env->spr[SPR_BOOKE_TLB0CFG],
> -                    env->spr[SPR_BOOKE_TLB1CFG]);
>          break;

If you remove those lines, the "switch (env->mmu_model)" is empty. You
should remove it entirely then.

Regards,