On 03/24/2014 05:24 PM, Alexey Kardashevskiy wrote:
> 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 <chout...@adacore.com>
>>> Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru>
>>> ---
>>> 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()):
Noone has an opinion? Come on! :)
>
> 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
