* [Qemu-devel] [PATCH v2] target-ppc: improve "info registers" by printing SPRs
@ 2014-03-22 12:25 Alexey Kardashevskiy
2014-03-22 14:43 ` Stuart Brady
0 siblings, 1 reply; 10+ messages in thread
From: Alexey Kardashevskiy @ 2014-03-22 12:25 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, qemu-ppc, Alexander Graf, Fabien Chouteau
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;
+ }
+ 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);
--
1.8.4.rc4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2] target-ppc: improve "info registers" by printing SPRs
2014-03-22 12:25 [Qemu-devel] [PATCH v2] target-ppc: improve "info registers" by printing SPRs Alexey Kardashevskiy
@ 2014-03-22 14:43 ` Stuart Brady
2014-03-24 6:24 ` Alexey Kardashevskiy
0 siblings, 1 reply; 10+ messages in thread
From: Stuart Brady @ 2014-03-22 14:43 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: qemu-ppc, qemu-devel, Fabien Chouteau, Alexander Graf
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2] target-ppc: improve "info registers" by printing SPRs
2014-03-22 14:43 ` Stuart Brady
@ 2014-03-24 6:24 ` Alexey Kardashevskiy
2014-03-31 1:25 ` Alexey Kardashevskiy
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Alexey Kardashevskiy @ 2014-03-24 6:24 UTC (permalink / raw)
To: Stuart Brady
Cc: Andreas Färber, qemu-ppc, qemu-devel, Fabien Chouteau,
Alexander Graf
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.
--
Alexey
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2] target-ppc: improve "info registers" by printing SPRs
2014-03-24 6:24 ` Alexey Kardashevskiy
@ 2014-03-31 1:25 ` Alexey Kardashevskiy
2014-03-31 8:24 ` Andreas Färber
2014-03-31 13:27 ` Alexander Graf
2014-03-31 19:59 ` Stuart Brady
2 siblings, 1 reply; 10+ messages in thread
From: Alexey Kardashevskiy @ 2014-03-31 1:25 UTC (permalink / raw)
To: Stuart Brady
Cc: Andreas Färber, qemu-ppc, qemu-devel, Fabien Chouteau,
Alexander Graf
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.
>
>
--
Alexey
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2] target-ppc: improve "info registers" by printing SPRs
2014-03-31 1:25 ` Alexey Kardashevskiy
@ 2014-03-31 8:24 ` Andreas Färber
2014-03-31 8:50 ` Alexey Kardashevskiy
0 siblings, 1 reply; 10+ messages in thread
From: Andreas Färber @ 2014-03-31 8:24 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Peter Maydell, qemu-devel, Fabien Chouteau, Alexander Graf,
Stuart Brady, qemu-ppc
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.
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2] target-ppc: improve "info registers" by printing SPRs
2014-03-31 8:24 ` Andreas Färber
@ 2014-03-31 8:50 ` Alexey Kardashevskiy
2014-03-31 10:07 ` Peter Maydell
0 siblings, 1 reply; 10+ messages in thread
From: Alexey Kardashevskiy @ 2014-03-31 8:50 UTC (permalink / raw)
To: Andreas Färber
Cc: Peter Maydell, qemu-devel, Fabien Chouteau, Alexander Graf,
Stuart Brady, qemu-ppc
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.
>
--
Alexey
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2] target-ppc: improve "info registers" by printing SPRs
2014-03-31 8:50 ` Alexey Kardashevskiy
@ 2014-03-31 10:07 ` Peter Maydell
2014-03-31 10:48 ` Alexey Kardashevskiy
0 siblings, 1 reply; 10+ messages in thread
From: Peter Maydell @ 2014-03-31 10:07 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Alexander Graf, Fabien Chouteau, QEMU Developers, Stuart Brady,
qemu-ppc, Andreas Färber
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2] target-ppc: improve "info registers" by printing SPRs
2014-03-31 10:07 ` Peter Maydell
@ 2014-03-31 10:48 ` Alexey Kardashevskiy
0 siblings, 0 replies; 10+ messages in thread
From: Alexey Kardashevskiy @ 2014-03-31 10:48 UTC (permalink / raw)
To: Peter Maydell
Cc: Alexander Graf, Fabien Chouteau, QEMU Developers, Stuart Brady,
qemu-ppc, Andreas Färber
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.
--
Alexey
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2] target-ppc: improve "info registers" by printing SPRs
2014-03-24 6:24 ` Alexey Kardashevskiy
2014-03-31 1:25 ` Alexey Kardashevskiy
@ 2014-03-31 13:27 ` Alexander Graf
2014-03-31 19:59 ` Stuart Brady
2 siblings, 0 replies; 10+ messages in thread
From: Alexander Graf @ 2014-03-31 13:27 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Stuart Brady, qemu-ppc, qemu-devel, Fabien Chouteau, Andreas Färber
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2] target-ppc: improve "info registers" by printing SPRs
2014-03-24 6:24 ` Alexey Kardashevskiy
2014-03-31 1:25 ` Alexey Kardashevskiy
2014-03-31 13:27 ` Alexander Graf
@ 2014-03-31 19:59 ` Stuart Brady
2 siblings, 0 replies; 10+ messages in thread
From: Stuart Brady @ 2014-03-31 19:59 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Andreas Färber, qemu-ppc, qemu-devel, Fabien Chouteau,
Alexander Graf
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.
--
Cheers,
Stuart
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-03-31 19:59 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-22 12:25 [Qemu-devel] [PATCH v2] target-ppc: improve "info registers" by printing SPRs Alexey Kardashevskiy
2014-03-22 14:43 ` Stuart Brady
2014-03-24 6:24 ` Alexey Kardashevskiy
2014-03-31 1:25 ` Alexey Kardashevskiy
2014-03-31 8:24 ` Andreas Färber
2014-03-31 8:50 ` Alexey Kardashevskiy
2014-03-31 10:07 ` Peter Maydell
2014-03-31 10:48 ` Alexey Kardashevskiy
2014-03-31 13:27 ` Alexander Graf
2014-03-31 19:59 ` Stuart Brady
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.