qemu-ppc
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-ppc] [Qemu-devel] [PATCH v2] target-ppc: improve "info registe

From: Alexander Graf
Subject: Re: [Qemu-ppc] [Qemu-devel] [PATCH v2] target-ppc: improve "info registers" by printing SPRs
Date: Mon, 31 Mar 2014 15:27:34 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130910 Thunderbird/17.0.9 
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 <address@hidden>
Signed-off-by: Alexey Kardashevskiy <address@hidden>
---
Changes:
v2:
* removed "switch (env->mmu_model)"
* added "\n" if the last line has less than 4 registers
---
  target-ppc/translate.c      | 96 +++++++--------------------------------------
  target-ppc/translate_init.c | 40 +++++++++----------
  2 files changed, 35 insertions(+), 101 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index e3fcb03..06f195a 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -11116,7 +11116,7 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, 
fprintf_function cpu_fprintf,

      PowerPCCPU *cpu = POWERPC_CPU(cs);
      CPUPPCState *env = &cpu->env;
-    int i;
+    int i, j;

      cpu_fprintf(f, "NIP " TARGET_FMT_lx "   LR " TARGET_FMT_lx " CTR "
                  TARGET_FMT_lx " XER " TARGET_FMT_lx "\n",
@@ -11167,54 +11167,22 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, 
fprintf_function cpu_fprintf,
              cpu_fprintf(f, "\n");
      }
      cpu_fprintf(f, "FPSCR " TARGET_FMT_lx "\n", env->fpscr);
-#if !defined(CONFIG_USER_ONLY)
-    cpu_fprintf(f, " SRR0 " TARGET_FMT_lx "  SRR1 " TARGET_FMT_lx
-                   "    PVR " TARGET_FMT_lx " VRSAVE " TARGET_FMT_lx "\n",
-                env->spr[SPR_SRR0], env->spr[SPR_SRR1],
-                env->spr[SPR_PVR], env->spr[SPR_VRSAVE]);

-    cpu_fprintf(f, "SPRG0 " TARGET_FMT_lx " SPRG1 " TARGET_FMT_lx
-                   "  SPRG2 " TARGET_FMT_lx "  SPRG3 " TARGET_FMT_lx "\n",
-                env->spr[SPR_SPRG0], env->spr[SPR_SPRG1],
-                env->spr[SPR_SPRG2], env->spr[SPR_SPRG3]);
-
-    cpu_fprintf(f, "SPRG4 " TARGET_FMT_lx " SPRG5 " TARGET_FMT_lx
-                   "  SPRG6 " TARGET_FMT_lx "  SPRG7 " TARGET_FMT_lx "\n",
-                env->spr[SPR_SPRG4], env->spr[SPR_SPRG5],
-                env->spr[SPR_SPRG6], env->spr[SPR_SPRG7]);
-
-    if (env->excp_model == POWERPC_EXCP_BOOKE) {
-        cpu_fprintf(f, "CSRR0 " TARGET_FMT_lx " CSRR1 " TARGET_FMT_lx
-                       " MCSRR0 " TARGET_FMT_lx " MCSRR1 " TARGET_FMT_lx "\n",
-                    env->spr[SPR_BOOKE_CSRR0], env->spr[SPR_BOOKE_CSRR1],
-                    env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSRR1]);
-
-        cpu_fprintf(f, "  TCR " TARGET_FMT_lx "   TSR " TARGET_FMT_lx
-                       "    ESR " TARGET_FMT_lx "   DEAR " TARGET_FMT_lx "\n",
-                    env->spr[SPR_BOOKE_TCR], env->spr[SPR_BOOKE_TSR],
-                    env->spr[SPR_BOOKE_ESR], env->spr[SPR_BOOKE_DEAR]);
-
-        cpu_fprintf(f, "  PIR " TARGET_FMT_lx " DECAR " TARGET_FMT_lx
-                       "   IVPR " TARGET_FMT_lx "   EPCR " TARGET_FMT_lx "\n",
-                    env->spr[SPR_BOOKE_PIR], env->spr[SPR_BOOKE_DECAR],
-                    env->spr[SPR_BOOKE_IVPR], env->spr[SPR_BOOKE_EPCR]);
-
-        cpu_fprintf(f, " MCSR " TARGET_FMT_lx " SPRG8 " TARGET_FMT_lx
-                       "    EPR " TARGET_FMT_lx "\n",
-                    env->spr[SPR_BOOKE_MCSR], env->spr[SPR_BOOKE_SPRG8],
-                    env->spr[SPR_BOOKE_EPR]);
-
-        /* FSL-specific */
-        cpu_fprintf(f, " MCAR " TARGET_FMT_lx "  PID1 " TARGET_FMT_lx
-                       "   PID2 " TARGET_FMT_lx "    SVR " TARGET_FMT_lx "\n",
-                    env->spr[SPR_Exxx_MCAR], env->spr[SPR_BOOKE_PID1],
-                    env->spr[SPR_BOOKE_PID2], env->spr[SPR_E500_SVR]);
-
-        /*
-         * IVORs are left out as they are large and do not change often --
-         * they can be read with "p $ivor0", "p $ivor1", etc.
-         */
+    for (i = 0, j = 0; i < ARRAY_SIZE(env->spr_cb); i++) {
+        ppc_spr_t *spr = &env->spr_cb[i];
+
+        if (!spr->name) {
+            continue;
+        }

This would leave the output without a trailing newline if the last spr
doesn't have a name registered.  Is it necessary to handle unnamed sprs
at all (maybe add an assert to the registration function)? ... or would
we just want to warn about them here?


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



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

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

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



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


XER    0000000000000000 LR     0000000000000000 CTR    0000000000000000


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
reply via email to
[Prev in Thread] Current Thread [Next in Thread]