| Message ID | 1395491149-21432-1-git-send-email-aik@ozlabs.ru |
|---|---|
| State | New |
| Headers | show |
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 <chouteau@adacore.com> > Signed-off-by: Alexey Kardashevskiy <aik@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? 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'. Cheers, Stuart
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 <chouteau@adacore.com> >> Signed-off-by: Alexey Kardashevskiy <aik@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()): 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.
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 <chouteau@adacore.com> >>> Signed-off-by: Alexey Kardashevskiy <aik@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. > >
Am 31.03.2014 03:25, schrieb Alexey Kardashevskiy: > On 03/24/2014 05:24 PM, Alexey Kardashevskiy wrote: >> 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! :) We're in Hard Freeze!!! >:-| There's more important works than post-2.0 debug infos ATM. Anyway... >> 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? You're comparing apples to oranges. One is printing values, one is printing configuration - and IIRC only for some hidden debug #ifdef. I'd suggest to sit down with Peter and discuss whether it may make sense to turn this SPR register configuration dump into an HMP command and in this case coordinate the command naming with ARM, where there's similar dynamically configured cp15 registers that may need inspection. (But if you do, just don't expect this to be picked up by next week!) Cheers, Andreas >> The characters at the end tell what handler (read/write, oea/uea) is >> defined for SPR.
On 03/31/2014 07:24 PM, Andreas Färber wrote: > Am 31.03.2014 03:25, schrieb Alexey Kardashevskiy: >> On 03/24/2014 05:24 PM, Alexey Kardashevskiy wrote: >>> 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! :) > > We're in Hard Freeze!!! >:-| There's more important works than post-2.0 > debug infos ATM. Anyway... Are you saying I should not be posting anything which is not "for 2.0" until it is released? Oookay. >>> 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? > > You're comparing apples to oranges. One is printing values, one is > > printing configuration - and IIRC only for some hidden debug #ifdef. Yes, it is under #ifdef. I could enable it and add values if someone said it makes sense. > I'd suggest to sit down with Peter and discuss whether it may make sense > to turn this SPR register configuration dump into an HMP command and in > this case coordinate the command naming with ARM, where there's similar > dynamically configured cp15 registers that may need inspection. Why just Peter? v1 and v2 attracted attention of other folks and I want to believe they are interested in the final result. Do _you_ have an opinion of what to print here and what can break if I print everything? > (But if you do, just don't expect this to be picked up by next week!) I never ever expect whatever I do to be picked up next 2 month. Seriously. And you know that... It never makes any difference for me whether next release is coming out or not... > Cheers, > Andreas > >>> The characters at the end tell what handler (read/write, oea/uea) is >>> defined for SPR. >
On 31 March 2014 09:50, Alexey Kardashevskiy <aik@ozlabs.ru> wrote: > On 03/31/2014 07:24 PM, Andreas Färber wrote: >> Am 31.03.2014 03:25, schrieb Alexey Kardashevskiy: >>> Noone has an opinion? Come on! :) >> >> We're in Hard Freeze!!! >:-| There's more important works than post-2.0 >> debug infos ATM. Anyway... > > > Are you saying I should not be posting anything which is not "for 2.0" > until it is released? Oookay. There's nothing wrong with posting patchsets that are targeting post-2.0 now. Andreas' point is that you can't necessarily expect review of them before release, because the people whose code review you might want are likely to be busy with things which are release-relevant, and so for some of those people post-2.0 things will go on the "look at later" queue. thanks -- PMM
On 03/31/2014 09:07 PM, Peter Maydell wrote: > On 31 March 2014 09:50, Alexey Kardashevskiy <aik@ozlabs.ru> wrote: >> On 03/31/2014 07:24 PM, Andreas Färber wrote: >>> Am 31.03.2014 03:25, schrieb Alexey Kardashevskiy: >>>> Noone has an opinion? Come on! :) >>> >>> We're in Hard Freeze!!! >:-| There's more important works than post-2.0 >>> debug infos ATM. Anyway... >> >> >> Are you saying I should not be posting anything which is not "for 2.0" >> until it is released? Oookay. > > There's nothing wrong with posting patchsets that are targeting > post-2.0 now. Andreas' point is that you can't necessarily expect > review of them before release, because the people whose code > review you might want are likely to be busy with things which > are release-relevant, and so for some of those people post-2.0 > things will go on the "look at later" queue. I rather wanted Stuart and Fabien to comment, sorry for misunderstanding.
On 03/24/2014 07:24 AM, 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 <chouteau@adacore.com> >>> Signed-off-by: Alexey Kardashevskiy <aik@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()): > > > XER 0000000000000000 LR 0000000000000000 CTR 0000000000000000 These are already listed, no? > 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 TB is also explicitly printed. > 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 My main pain point with a cluttered SPR list in "info registers" is that it's also used for -d cpu. If we bloat it by too much, it becomes less useful that I'd like it to be. Could we maybe add a flag to the SPR register function to indicate special treatment? We already have a _kvm version - how about we convert that one to a flag bitmap that can be SYNC_KVM and/or DUMP_SPR? Alex > > > > > > 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 list down here really is for debugging the configuration of available SPRs. I think it is useful, but it covers vastly different information and definitely does not belong to runtime CPU state - this is CPU configuration state (more along the lines of qom properties). Alex
On Mon, Mar 24, 2014 at 05:24:35PM +1100, Alexey Kardashevskiy wrote: > On 03/23/2014 01:43 AM, Stuart Brady wrote: > > 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. Okay, although an assert probably wouldn't hurt. OTOH, if we never want an SPR without a name, removing the handling for !spr->name entirely would make sense. You could then get rid of the separate 'j' variable. > > 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? :) Not that embarrassing, to be honest. The code I once fixed that built up a string of comma-separated values, and then tried to remove the trailing comma even if the string was empty, now *that* was embarrassing. This was remarkably copied-and-pasted all over the shop and then fixed in some files but not others. Luckily, corruption of the preceding variable was harmless in all occurrences since it was held in a register. Erk. :-( In that particular case it made more sense to increment to point past the leading comma if present, and I've tended toward that style ever since. > + 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"); Looks good. > 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()): > [...] > Special purpose registers: [...] > Which is nicer/more useful? > The characters at the end tell what handler (read/write, oea/uea) is > defined for SPR. They're both perfectly fine. For the list of registers, you want detail about access type, etc. and perhaps not the current values, but for the simple register dump would want values and register names but without any extraneous information.
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; + } + cpu_fprintf(f, "%-6s " TARGET_FMT_lx, spr->name, env->spr[i]); + j++; + if (!(j % 4) || (i == ARRAY_SIZE(env->spr_cb) - 1)) { + cpu_fprintf(f, "\n"); + } else { + cpu_fprintf(f, " "); + } } +#if !defined(CONFIG_USER_ONLY) #if defined(TARGET_PPC64) if (env->flags & POWERPC_FLAG_CFAR) { @@ -11222,40 +11190,6 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf, } #endif - switch (env->mmu_model) { - case POWERPC_MMU_32B: - case POWERPC_MMU_601: - case POWERPC_MMU_SOFT_6xx: - case POWERPC_MMU_SOFT_74xx: -#if defined(TARGET_PPC64) - case POWERPC_MMU_64B: - case POWERPC_MMU_2_06: - 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; - default: - break; - } #endif #undef RGPL diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index 954dee3..4d199c1 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -3834,7 +3834,7 @@ static void init_proc_460 (CPUPPCState *env) &spr_read_generic, &spr_write_generic, 0x00000000); /* XXX : not implemented */ - spr_register(env, SPR_DCRIPR, "SPR_DCRIPR", + spr_register(env, SPR_DCRIPR, "DCRIPR", &spr_read_generic, &spr_write_generic, &spr_read_generic, &spr_write_generic, 0x00000000); @@ -3930,7 +3930,7 @@ static void init_proc_460F (CPUPPCState *env) &spr_read_generic, &spr_write_generic, 0x00000000); /* XXX : not implemented */ - spr_register(env, SPR_DCRIPR, "SPR_DCRIPR", + spr_register(env, SPR_DCRIPR, "DCRIPR", &spr_read_generic, &spr_write_generic, &spr_read_generic, &spr_write_generic, 0x00000000); @@ -6654,7 +6654,7 @@ static void init_proc_970 (CPUPPCState *env) /* Memory management */ /* XXX: not correct */ gen_low_BATs(env); - spr_register(env, SPR_HIOR, "SPR_HIOR", + spr_register(env, SPR_HIOR, "HIOR", SPR_NOACCESS, SPR_NOACCESS, &spr_read_hior, &spr_write_hior, 0x00000000); @@ -6734,19 +6734,19 @@ static void init_proc_970FX (CPUPPCState *env) /* Memory management */ /* XXX: not correct */ gen_low_BATs(env); - spr_register(env, SPR_HIOR, "SPR_HIOR", + spr_register(env, SPR_HIOR, "HIOR", SPR_NOACCESS, SPR_NOACCESS, &spr_read_hior, &spr_write_hior, 0x00000000); - spr_register(env, SPR_CTRL, "SPR_CTRL", + spr_register(env, SPR_CTRL, "CTRL", SPR_NOACCESS, SPR_NOACCESS, SPR_NOACCESS, &spr_write_generic, 0x00000000); - spr_register(env, SPR_UCTRL, "SPR_UCTRL", + spr_register(env, SPR_UCTRL, "UCTRL", SPR_NOACCESS, SPR_NOACCESS, &spr_read_generic, SPR_NOACCESS, 0x00000000); - spr_register(env, SPR_VRSAVE, "SPR_VRSAVE", + spr_register(env, SPR_VRSAVE, "VRSAVE", &spr_read_generic, &spr_write_generic, &spr_read_generic, &spr_write_generic, 0x00000000); @@ -6827,7 +6827,7 @@ static void init_proc_970MP (CPUPPCState *env) /* Memory management */ /* XXX: not correct */ gen_low_BATs(env); - spr_register(env, SPR_HIOR, "SPR_HIOR", + spr_register(env, SPR_HIOR, "HIOR", SPR_NOACCESS, SPR_NOACCESS, &spr_read_hior, &spr_write_hior, 0x00000000); @@ -6904,19 +6904,19 @@ static void init_proc_power5plus(CPUPPCState *env) /* Memory management */ /* XXX: not correct */ gen_low_BATs(env); - spr_register(env, SPR_HIOR, "SPR_HIOR", + spr_register(env, SPR_HIOR, "HIOR", SPR_NOACCESS, SPR_NOACCESS, &spr_read_hior, &spr_write_hior, 0x00000000); - spr_register(env, SPR_CTRL, "SPR_CTRL", + spr_register(env, SPR_CTRL, "CTRL", SPR_NOACCESS, SPR_NOACCESS, SPR_NOACCESS, &spr_write_generic, 0x00000000); - spr_register(env, SPR_UCTRL, "SPR_UCTRL", + spr_register(env, SPR_UCTRL, "UCTRL", SPR_NOACCESS, SPR_NOACCESS, &spr_read_generic, SPR_NOACCESS, 0x00000000); - spr_register(env, SPR_VRSAVE, "SPR_VRSAVE", + spr_register(env, SPR_VRSAVE, "VRSAVE", &spr_read_generic, &spr_write_generic, &spr_read_generic, &spr_write_generic, 0x00000000); @@ -6990,38 +6990,38 @@ static void init_proc_POWER7 (CPUPPCState *env) &spr_read_purr, SPR_NOACCESS, &spr_read_purr, SPR_NOACCESS, KVM_REG_PPC_SPURR, 0x00000000); - spr_register(env, SPR_CFAR, "SPR_CFAR", + spr_register(env, SPR_CFAR, "CFAR", SPR_NOACCESS, SPR_NOACCESS, &spr_read_cfar, &spr_write_cfar, 0x00000000); - spr_register_kvm(env, SPR_DSCR, "SPR_DSCR", + spr_register_kvm(env, SPR_DSCR, "DSCR", SPR_NOACCESS, SPR_NOACCESS, &spr_read_generic, &spr_write_generic, KVM_REG_PPC_DSCR, 0x00000000); - spr_register_kvm(env, SPR_MMCRA, "SPR_MMCRA", + spr_register_kvm(env, SPR_MMCRA, "MMCRA", SPR_NOACCESS, SPR_NOACCESS, &spr_read_generic, &spr_write_generic, KVM_REG_PPC_MMCRA, 0x00000000); - spr_register_kvm(env, SPR_PMC5, "SPR_PMC5", + spr_register_kvm(env, SPR_PMC5, "PMC5", SPR_NOACCESS, SPR_NOACCESS, &spr_read_generic, &spr_write_generic, KVM_REG_PPC_PMC5, 0x00000000); - spr_register_kvm(env, SPR_PMC6, "SPR_PMC6", + spr_register_kvm(env, SPR_PMC6, "PMC6", SPR_NOACCESS, SPR_NOACCESS, &spr_read_generic, &spr_write_generic, KVM_REG_PPC_PMC6, 0x00000000); #endif /* !CONFIG_USER_ONLY */ gen_spr_amr(env); /* XXX : not implemented */ - spr_register(env, SPR_CTRL, "SPR_CTRLT", + spr_register(env, SPR_CTRL, "CTRLT", SPR_NOACCESS, SPR_NOACCESS, SPR_NOACCESS, &spr_write_generic, 0x80800000); - spr_register(env, SPR_UCTRL, "SPR_CTRLF", + spr_register(env, SPR_UCTRL, "CTRLF", SPR_NOACCESS, SPR_NOACCESS, &spr_read_generic, SPR_NOACCESS, 0x80800000); - spr_register(env, SPR_VRSAVE, "SPR_VRSAVE", + spr_register(env, SPR_VRSAVE, "VRSAVE", &spr_read_generic, &spr_write_generic, &spr_read_generic, &spr_write_generic, 0x00000000);
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 <chouteau@adacore.com> Signed-off-by: Alexey Kardashevskiy <aik@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(-)