[Qemu-devel] [PATCH qemu 0/2] monitor/ppc: Print correct SPRs

qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed

* [Qemu-devel] [PATCH qemu 0/2] monitor/ppc: Print correct SPRs
@ 2015-08-06  5:25 Alexey Kardashevskiy
  2015-08-06  5:25 ` [Qemu-devel] [PATCH qemu 1/2] monitor: Add CPU class callback to read registers for monitor Alexey Kardashevskiy
  2015-08-06  5:25 ` [Qemu-devel] [PATCH qemu 2/2] target-ppc: Define get_monitor_def Alexey Kardashevskiy
  0 siblings, 2 replies; 13+ messages in thread
From: Alexey Kardashevskiy @ 2015-08-06  5:25 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alexey Kardashevskiy, Markus Armbruster, Alexander Graf,
	Luiz Capitulino, qemu-ppc, Scott Wood, Andreas Färber,
	David Gibson

QEMU's monitor can only print registers described in monitor.c but
in fact we got lot, lot more in POWER7/8, instead of adding all of these
into monitor, we better print only registered SPR so here are the patches.

Please comment. Thanks!



ps. about coding style.
In PPC code we name CPUPPCState* variables as "cpu" and "CPUState" - "cs".
However include/qom/cpu.h uses "cpu" for "CPUState".
What is the correct way of naming these?


Alexey Kardashevskiy (2):
  monitor: Add CPU class callback to read registers for monitor
  target-ppc: Define get_monitor_def

 include/qom/cpu.h           |   1 +
 monitor.c                   | 229 +++-----------------------------------------
 target-ppc/cpu-qom.h        |   2 +
 target-ppc/translate.c      |  72 ++++++++++++++
 target-ppc/translate_init.c |   1 +
 5 files changed, 90 insertions(+), 215 deletions(-)

-- 
2.4.0.rc3.8.gfb3e7d5

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH qemu 1/2] monitor: Add CPU class callback to read registers for monitor
  2015-08-06  5:25 [Qemu-devel] [PATCH qemu 0/2] monitor/ppc: Print correct SPRs Alexey Kardashevskiy
@ 2015-08-06  5:25 ` Alexey Kardashevskiy
  2015-08-12  1:12   ` David Gibson
  2015-08-06  5:25 ` [Qemu-devel] [PATCH qemu 2/2] target-ppc: Define get_monitor_def Alexey Kardashevskiy
  1 sibling, 1 reply; 13+ messages in thread
From: Alexey Kardashevskiy @ 2015-08-06  5:25 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alexey Kardashevskiy, Markus Armbruster, Alexander Graf,
	Luiz Capitulino, qemu-ppc, Andreas Färber, David Gibson

At the moment the monitor only prints registers from monitor_defs.
Some may not be supported but it will print those anyway, other
may be missing in the list so monitor_defs needs an update every time
new register is added.

This defines a CPUClass callback to read various registers from CPU.

Next patch makes use of it.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 include/qom/cpu.h |  1 +
 monitor.c         | 14 +++++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 20aabc9..fcf981f 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -144,6 +144,7 @@ typedef struct CPUClass {
                        int flags);
     void (*dump_statistics)(CPUState *cpu, FILE *f,
                             fprintf_function cpu_fprintf, int flags);
+    int (*get_monitor_def)(CPUState *cs, const char *name, uint64_t *pval);
     int64_t (*get_arch_id)(CPUState *cpu);
     bool (*get_paging_enabled)(const CPUState *cpu);
     void (*get_memory_mapping)(CPUState *cpu, MemoryMappingList *list,
diff --git a/monitor.c b/monitor.c
index aeea2b5..bdfcacc 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3303,13 +3303,25 @@ static int get_monitor_def(target_long *pval, const char *name)
 {
     const MonitorDef *md;
     void *ptr;
+    CPUState *cs = mon_get_cpu();
+    CPUClass *cc = CPU_GET_CLASS(cs);
+
+    if (cc->get_monitor_def) {
+        uint64_t tmp = 0;
+        int ret = cc->get_monitor_def(cs, name, &tmp);
+
+        if (!ret) {
+            *pval = (target_long) tmp;
+        }
+        return ret;
+    }
 
     for(md = monitor_defs; md->name != NULL; md++) {
         if (compare_cmd(name, md->name)) {
             if (md->get_value) {
                 *pval = md->get_value(md, md->offset);
             } else {
-                CPUArchState *env = mon_get_cpu_env();
+                CPUArchState *env = cs->env_ptr;
                 ptr = (uint8_t *)env + md->offset;
                 switch(md->type) {
                 case MD_I32:
-- 
2.4.0.rc3.8.gfb3e7d5

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH qemu 2/2] target-ppc: Define get_monitor_def
  2015-08-06  5:25 [Qemu-devel] [PATCH qemu 0/2] monitor/ppc: Print correct SPRs Alexey Kardashevskiy
  2015-08-06  5:25 ` [Qemu-devel] [PATCH qemu 1/2] monitor: Add CPU class callback to read registers for monitor Alexey Kardashevskiy
@ 2015-08-06  5:25 ` Alexey Kardashevskiy
  2015-08-06  6:33   ` Thomas Huth
  2015-08-12  1:21   ` David Gibson
  1 sibling, 2 replies; 13+ messages in thread
From: Alexey Kardashevskiy @ 2015-08-06  5:25 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alexey Kardashevskiy, Markus Armbruster, Alexander Graf,
	Luiz Capitulino, qemu-ppc, Andreas Färber, David Gibson

At the moment get_monitor_def() prints only registers from monitor_defs.
However there is a lot of BOOK3S SPRs which are not in the list and
cannot be printed.

This makes use of the new get_monitor_def() callback and prints all
registered SPRs and fails on unregistered ones proving the user
information on what is actually supported in the running CPU.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 monitor.c                   | 215 +-------------------------------------------
 target-ppc/cpu-qom.h        |   2 +
 target-ppc/translate.c      |  72 +++++++++++++++
 target-ppc/translate_init.c |   1 +
 4 files changed, 76 insertions(+), 214 deletions(-)

diff --git a/monitor.c b/monitor.c
index bdfcacc..07ca678 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2916,51 +2916,6 @@ static target_long monitor_get_pc (const struct MonitorDef *md, int val)
 }
 #endif
 
-#if defined(TARGET_PPC)
-static target_long monitor_get_ccr (const struct MonitorDef *md, int val)
-{
-    CPUArchState *env = mon_get_cpu_env();
-    unsigned int u;
-    int i;
-
-    u = 0;
-    for (i = 0; i < 8; i++)
-        u |= env->crf[i] << (32 - (4 * (i + 1)));
-
-    return u;
-}
-
-static target_long monitor_get_msr (const struct MonitorDef *md, int val)
-{
-    CPUArchState *env = mon_get_cpu_env();
-    return env->msr;
-}
-
-static target_long monitor_get_xer (const struct MonitorDef *md, int val)
-{
-    CPUArchState *env = mon_get_cpu_env();
-    return env->xer;
-}
-
-static target_long monitor_get_decr (const struct MonitorDef *md, int val)
-{
-    CPUArchState *env = mon_get_cpu_env();
-    return cpu_ppc_load_decr(env);
-}
-
-static target_long monitor_get_tbu (const struct MonitorDef *md, int val)
-{
-    CPUArchState *env = mon_get_cpu_env();
-    return cpu_ppc_load_tbu(env);
-}
-
-static target_long monitor_get_tbl (const struct MonitorDef *md, int val)
-{
-    CPUArchState *env = mon_get_cpu_env();
-    return cpu_ppc_load_tbl(env);
-}
-#endif
-
 #if defined(TARGET_SPARC)
 #ifndef TARGET_SPARC64
 static target_long monitor_get_psr (const struct MonitorDef *md, int val)
@@ -3014,175 +2969,7 @@ static const MonitorDef monitor_defs[] = {
     SEG("gs", R_GS)
     { "pc", 0, monitor_get_pc, },
 #elif defined(TARGET_PPC)
-    /* General purpose registers */
-    { "r0", offsetof(CPUPPCState, gpr[0]) },
-    { "r1", offsetof(CPUPPCState, gpr[1]) },
-    { "r2", offsetof(CPUPPCState, gpr[2]) },
-    { "r3", offsetof(CPUPPCState, gpr[3]) },
-    { "r4", offsetof(CPUPPCState, gpr[4]) },
-    { "r5", offsetof(CPUPPCState, gpr[5]) },
-    { "r6", offsetof(CPUPPCState, gpr[6]) },
-    { "r7", offsetof(CPUPPCState, gpr[7]) },
-    { "r8", offsetof(CPUPPCState, gpr[8]) },
-    { "r9", offsetof(CPUPPCState, gpr[9]) },
-    { "r10", offsetof(CPUPPCState, gpr[10]) },
-    { "r11", offsetof(CPUPPCState, gpr[11]) },
-    { "r12", offsetof(CPUPPCState, gpr[12]) },
-    { "r13", offsetof(CPUPPCState, gpr[13]) },
-    { "r14", offsetof(CPUPPCState, gpr[14]) },
-    { "r15", offsetof(CPUPPCState, gpr[15]) },
-    { "r16", offsetof(CPUPPCState, gpr[16]) },
-    { "r17", offsetof(CPUPPCState, gpr[17]) },
-    { "r18", offsetof(CPUPPCState, gpr[18]) },
-    { "r19", offsetof(CPUPPCState, gpr[19]) },
-    { "r20", offsetof(CPUPPCState, gpr[20]) },
-    { "r21", offsetof(CPUPPCState, gpr[21]) },
-    { "r22", offsetof(CPUPPCState, gpr[22]) },
-    { "r23", offsetof(CPUPPCState, gpr[23]) },
-    { "r24", offsetof(CPUPPCState, gpr[24]) },
-    { "r25", offsetof(CPUPPCState, gpr[25]) },
-    { "r26", offsetof(CPUPPCState, gpr[26]) },
-    { "r27", offsetof(CPUPPCState, gpr[27]) },
-    { "r28", offsetof(CPUPPCState, gpr[28]) },
-    { "r29", offsetof(CPUPPCState, gpr[29]) },
-    { "r30", offsetof(CPUPPCState, gpr[30]) },
-    { "r31", offsetof(CPUPPCState, gpr[31]) },
-    /* Floating point registers */
-    { "f0", offsetof(CPUPPCState, fpr[0]) },
-    { "f1", offsetof(CPUPPCState, fpr[1]) },
-    { "f2", offsetof(CPUPPCState, fpr[2]) },
-    { "f3", offsetof(CPUPPCState, fpr[3]) },
-    { "f4", offsetof(CPUPPCState, fpr[4]) },
-    { "f5", offsetof(CPUPPCState, fpr[5]) },
-    { "f6", offsetof(CPUPPCState, fpr[6]) },
-    { "f7", offsetof(CPUPPCState, fpr[7]) },
-    { "f8", offsetof(CPUPPCState, fpr[8]) },
-    { "f9", offsetof(CPUPPCState, fpr[9]) },
-    { "f10", offsetof(CPUPPCState, fpr[10]) },
-    { "f11", offsetof(CPUPPCState, fpr[11]) },
-    { "f12", offsetof(CPUPPCState, fpr[12]) },
-    { "f13", offsetof(CPUPPCState, fpr[13]) },
-    { "f14", offsetof(CPUPPCState, fpr[14]) },
-    { "f15", offsetof(CPUPPCState, fpr[15]) },
-    { "f16", offsetof(CPUPPCState, fpr[16]) },
-    { "f17", offsetof(CPUPPCState, fpr[17]) },
-    { "f18", offsetof(CPUPPCState, fpr[18]) },
-    { "f19", offsetof(CPUPPCState, fpr[19]) },
-    { "f20", offsetof(CPUPPCState, fpr[20]) },
-    { "f21", offsetof(CPUPPCState, fpr[21]) },
-    { "f22", offsetof(CPUPPCState, fpr[22]) },
-    { "f23", offsetof(CPUPPCState, fpr[23]) },
-    { "f24", offsetof(CPUPPCState, fpr[24]) },
-    { "f25", offsetof(CPUPPCState, fpr[25]) },
-    { "f26", offsetof(CPUPPCState, fpr[26]) },
-    { "f27", offsetof(CPUPPCState, fpr[27]) },
-    { "f28", offsetof(CPUPPCState, fpr[28]) },
-    { "f29", offsetof(CPUPPCState, fpr[29]) },
-    { "f30", offsetof(CPUPPCState, fpr[30]) },
-    { "f31", offsetof(CPUPPCState, fpr[31]) },
-    { "fpscr", offsetof(CPUPPCState, fpscr) },
-    /* Next instruction pointer */
-    { "nip|pc", offsetof(CPUPPCState, nip) },
-    { "lr", offsetof(CPUPPCState, lr) },
-    { "ctr", offsetof(CPUPPCState, ctr) },
-    { "decr", 0, &monitor_get_decr, },
-    { "ccr", 0, &monitor_get_ccr, },
-    /* Machine state register */
-    { "msr", 0, &monitor_get_msr, },
-    { "xer", 0, &monitor_get_xer, },
-    { "tbu", 0, &monitor_get_tbu, },
-    { "tbl", 0, &monitor_get_tbl, },
-    /* Segment registers */
-    { "sdr1", offsetof(CPUPPCState, spr[SPR_SDR1]) },
-    { "sr0", offsetof(CPUPPCState, sr[0]) },
-    { "sr1", offsetof(CPUPPCState, sr[1]) },
-    { "sr2", offsetof(CPUPPCState, sr[2]) },
-    { "sr3", offsetof(CPUPPCState, sr[3]) },
-    { "sr4", offsetof(CPUPPCState, sr[4]) },
-    { "sr5", offsetof(CPUPPCState, sr[5]) },
-    { "sr6", offsetof(CPUPPCState, sr[6]) },
-    { "sr7", offsetof(CPUPPCState, sr[7]) },
-    { "sr8", offsetof(CPUPPCState, sr[8]) },
-    { "sr9", offsetof(CPUPPCState, sr[9]) },
-    { "sr10", offsetof(CPUPPCState, sr[10]) },
-    { "sr11", offsetof(CPUPPCState, sr[11]) },
-    { "sr12", offsetof(CPUPPCState, sr[12]) },
-    { "sr13", offsetof(CPUPPCState, sr[13]) },
-    { "sr14", offsetof(CPUPPCState, sr[14]) },
-    { "sr15", offsetof(CPUPPCState, sr[15]) },
-    /* Too lazy to put BATs... */
-    { "pvr", offsetof(CPUPPCState, spr[SPR_PVR]) },
-
-    { "srr0", offsetof(CPUPPCState, spr[SPR_SRR0]) },
-    { "srr1", offsetof(CPUPPCState, spr[SPR_SRR1]) },
-    { "dar", offsetof(CPUPPCState, spr[SPR_DAR]) },
-    { "dsisr", offsetof(CPUPPCState, spr[SPR_DSISR]) },
-    { "cfar", offsetof(CPUPPCState, spr[SPR_CFAR]) },
-    { "sprg0", offsetof(CPUPPCState, spr[SPR_SPRG0]) },
-    { "sprg1", offsetof(CPUPPCState, spr[SPR_SPRG1]) },
-    { "sprg2", offsetof(CPUPPCState, spr[SPR_SPRG2]) },
-    { "sprg3", offsetof(CPUPPCState, spr[SPR_SPRG3]) },
-    { "sprg4", offsetof(CPUPPCState, spr[SPR_SPRG4]) },
-    { "sprg5", offsetof(CPUPPCState, spr[SPR_SPRG5]) },
-    { "sprg6", offsetof(CPUPPCState, spr[SPR_SPRG6]) },
-    { "sprg7", offsetof(CPUPPCState, spr[SPR_SPRG7]) },
-    { "pid", offsetof(CPUPPCState, spr[SPR_BOOKE_PID]) },
-    { "csrr0", offsetof(CPUPPCState, spr[SPR_BOOKE_CSRR0]) },
-    { "csrr1", offsetof(CPUPPCState, spr[SPR_BOOKE_CSRR1]) },
-    { "esr", offsetof(CPUPPCState, spr[SPR_BOOKE_ESR]) },
-    { "dear", offsetof(CPUPPCState, spr[SPR_BOOKE_DEAR]) },
-    { "mcsr", offsetof(CPUPPCState, spr[SPR_BOOKE_MCSR]) },
-    { "tsr", offsetof(CPUPPCState, spr[SPR_BOOKE_TSR]) },
-    { "tcr", offsetof(CPUPPCState, spr[SPR_BOOKE_TCR]) },
-    { "vrsave", offsetof(CPUPPCState, spr[SPR_VRSAVE]) },
-    { "pir", offsetof(CPUPPCState, spr[SPR_BOOKE_PIR]) },
-    { "mcsrr0", offsetof(CPUPPCState, spr[SPR_BOOKE_MCSRR0]) },
-    { "mcsrr1", offsetof(CPUPPCState, spr[SPR_BOOKE_MCSRR1]) },
-    { "decar", offsetof(CPUPPCState, spr[SPR_BOOKE_DECAR]) },
-    { "ivpr", offsetof(CPUPPCState, spr[SPR_BOOKE_IVPR]) },
-    { "epcr", offsetof(CPUPPCState, spr[SPR_BOOKE_EPCR]) },
-    { "sprg8", offsetof(CPUPPCState, spr[SPR_BOOKE_SPRG8]) },
-    { "ivor0", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR0]) },
-    { "ivor1", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR1]) },
-    { "ivor2", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR2]) },
-    { "ivor3", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR3]) },
-    { "ivor4", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR4]) },
-    { "ivor5", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR5]) },
-    { "ivor6", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR6]) },
-    { "ivor7", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR7]) },
-    { "ivor8", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR8]) },
-    { "ivor9", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR9]) },
-    { "ivor10", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR10]) },
-    { "ivor11", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR11]) },
-    { "ivor12", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR12]) },
-    { "ivor13", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR13]) },
-    { "ivor14", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR14]) },
-    { "ivor15", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR15]) },
-    { "ivor32", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR32]) },
-    { "ivor33", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR33]) },
-    { "ivor34", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR34]) },
-    { "ivor35", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR35]) },
-    { "ivor36", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR36]) },
-    { "ivor37", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR37]) },
-    { "mas0", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS0]) },
-    { "mas1", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS1]) },
-    { "mas2", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS2]) },
-    { "mas3", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS3]) },
-    { "mas4", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS4]) },
-    { "mas6", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS6]) },
-    { "mas7", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS7]) },
-    { "mmucfg", offsetof(CPUPPCState, spr[SPR_MMUCFG]) },
-    { "tlb0cfg", offsetof(CPUPPCState, spr[SPR_BOOKE_TLB0CFG]) },
-    { "tlb1cfg", offsetof(CPUPPCState, spr[SPR_BOOKE_TLB1CFG]) },
-    { "epr", offsetof(CPUPPCState, spr[SPR_BOOKE_EPR]) },
-    { "eplc", offsetof(CPUPPCState, spr[SPR_BOOKE_EPLC]) },
-    { "epsc", offsetof(CPUPPCState, spr[SPR_BOOKE_EPSC]) },
-    { "svr", offsetof(CPUPPCState, spr[SPR_E500_SVR]) },
-    { "mcar", offsetof(CPUPPCState, spr[SPR_Exxx_MCAR]) },
-    { "pid1", offsetof(CPUPPCState, spr[SPR_BOOKE_PID1]) },
-    { "pid2", offsetof(CPUPPCState, spr[SPR_BOOKE_PID2]) },
-    { "hid0", offsetof(CPUPPCState, spr[SPR_HID0]) },
-
+    /* Uses get_monitor_def() */
 #elif defined(TARGET_SPARC)
     { "g0", offsetof(CPUSPARCState, gregs[0]) },
     { "g1", offsetof(CPUSPARCState, gregs[1]) },
diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
index 6967a80..bc20504 100644
--- a/target-ppc/cpu-qom.h
+++ b/target-ppc/cpu-qom.h
@@ -118,6 +118,8 @@ void ppc_cpu_dump_state(CPUState *cpu, FILE *f, fprintf_function cpu_fprintf,
                         int flags);
 void ppc_cpu_dump_statistics(CPUState *cpu, FILE *f,
                              fprintf_function cpu_fprintf, int flags);
+int ppc_cpu_get_monitor_def(CPUState *cs, const char *name,
+                            uint64_t *pval);
 hwaddr ppc_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 int ppc_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
 int ppc_cpu_gdb_read_register_apple(CPUState *cpu, uint8_t *buf, int reg);
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 84c5cea..f4acafb 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -11401,6 +11401,78 @@ void ppc_cpu_dump_statistics(CPUState *cs, FILE*f,
 #endif
 }
 
+static int ppc_cpu_get_reg(target_ulong *regs, const char *numstr, int maxnum,
+                           uint64_t *pval)
+{
+    char *endptr = NULL;
+    int regnum = strtoul(numstr, &endptr, 10);
+
+    if ((endptr && *endptr) || (regnum >= maxnum)) {
+        return -EINVAL;
+    }
+    *pval = regs[regnum];
+
+    return 0;
+}
+
+int ppc_cpu_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval)
+{
+    int i;
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    CPUPPCState *env = &cpu->env;
+
+#define MONREG(s, f) \
+    if ((strcasecmp((s), name) == 0)) { \
+        *pval = (f); \
+        return 0; \
+    }
+    MONREG("pc", env->nip)
+    MONREG("nip", env->nip)
+    MONREG("lr", env->lr)
+    MONREG("ctr", env->ctr)
+    MONREG("xer", env->xer)
+    MONREG("decr", cpu_ppc_load_decr(env))
+    MONREG("msr",  env->msr)
+    MONREG("tbu",  cpu_ppc_load_tbu(env))
+    MONREG("tbl", cpu_ppc_load_tbl(env))
+
+    if (strcasecmp("ccr", name) == 0) {
+        unsigned int u = 0;
+
+        for (i = 0; i < 8; i++)
+            u |= env->crf[i] << (32 - (4 * (i + 1)));
+
+        return u;
+    }
+
+    /* General purpose registers */
+    if (name[0] == 'r') {
+        return ppc_cpu_get_reg(env->gpr, name + 1, ARRAY_SIZE(env->gpr), pval);
+    }
+
+    /* Floating point registers */
+    if (name[0] == 'f') {
+        return ppc_cpu_get_reg(env->fpr, name + 1, ARRAY_SIZE(env->fpr), pval);
+    }
+
+    /* Segment registers */
+    if (strncmp(name, "sr", 2) == 0) {
+        return ppc_cpu_get_reg(env->sr, name + 2, ARRAY_SIZE(env->sr), pval);
+    }
+
+    /* Special purpose registers */
+    for (i = 0; i < ARRAY_SIZE(env->spr_cb); ++i) {
+        ppc_spr_t *spr = &env->spr_cb[i];
+
+        if (spr->name && (strcasecmp(name, spr->name) == 0)) {
+            *pval = env->spr[i];
+            return 0;
+        }
+    }
+
+    return -EINVAL;
+}
+
 /*****************************************************************************/
 static inline void gen_intermediate_code_internal(PowerPCCPU *cpu,
                                                   TranslationBlock *tb,
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 16d7b16..038674a 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9706,6 +9706,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
     cc->cpu_exec_interrupt = ppc_cpu_exec_interrupt;
     cc->dump_state = ppc_cpu_dump_state;
     cc->dump_statistics = ppc_cpu_dump_statistics;
+    cc->get_monitor_def = ppc_cpu_get_monitor_def;
     cc->set_pc = ppc_cpu_set_pc;
     cc->gdb_read_register = ppc_cpu_gdb_read_register;
     cc->gdb_write_register = ppc_cpu_gdb_write_register;
-- 
2.4.0.rc3.8.gfb3e7d5

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH qemu 2/2] target-ppc: Define get_monitor_def
  2015-08-06  5:25 ` [Qemu-devel] [PATCH qemu 2/2] target-ppc: Define get_monitor_def Alexey Kardashevskiy
@ 2015-08-06  6:33   ` Thomas Huth
  2015-08-06  7:00     ` Alexey Kardashevskiy
  2015-08-12  1:21   ` David Gibson
  1 sibling, 1 reply; 13+ messages in thread
From: Thomas Huth @ 2015-08-06  6:33 UTC (permalink / raw)
  To: Alexey Kardashevskiy, qemu-devel
  Cc: Alexander Graf, Markus Armbruster, Luiz Capitulino, qemu-ppc,
	Andreas Färber, David Gibson

On 06/08/15 07:25, Alexey Kardashevskiy wrote:
> At the moment get_monitor_def() prints only registers from monitor_defs.
> However there is a lot of BOOK3S SPRs which are not in the list and
> cannot be printed.
> 
> This makes use of the new get_monitor_def() callback and prints all
> registered SPRs and fails on unregistered ones proving the user
> information on what is actually supported in the running CPU.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>  monitor.c                   | 215 +-------------------------------------------
>  target-ppc/cpu-qom.h        |   2 +
>  target-ppc/translate.c      |  72 +++++++++++++++
>  target-ppc/translate_init.c |   1 +
>  4 files changed, 76 insertions(+), 214 deletions(-)
...
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 84c5cea..f4acafb 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -11401,6 +11401,78 @@ void ppc_cpu_dump_statistics(CPUState *cs, FILE*f,
>  #endif
>  }
>  
> +static int ppc_cpu_get_reg(target_ulong *regs, const char *numstr, int maxnum,
> +                           uint64_t *pval)

Don't you break the 32-bit QEMU (ppc-softmmu instead of ppc64-softmmu)
here? Since pval is uint64_t but the registers are target_ulong = 32 bit ?

> +{
> +    char *endptr = NULL;
> +    int regnum = strtoul(numstr, &endptr, 10);
> +
> +    if ((endptr && *endptr) || (regnum >= maxnum)) {

I'll never get used to your bracketism...

> +        return -EINVAL;
> +    }
> +    *pval = regs[regnum];
> +
> +    return 0;
> +}
> +
> +int ppc_cpu_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval)
> +{
> +    int i;
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    CPUPPCState *env = &cpu->env;
> +
> +#define MONREG(s, f) \
> +    if ((strcasecmp((s), name) == 0)) { \

Remove at least here the outermost round brackets?

> +        *pval = (f); \
> +        return 0; \
> +    }

... also defining a macro with parameters and code within a function
looks somewhat strange to me. Maybe you could consider moving this in
front of the function?

> +    MONREG("pc", env->nip)
> +    MONREG("nip", env->nip)
> +    MONREG("lr", env->lr)
> +    MONREG("ctr", env->ctr)
> +    MONREG("xer", env->xer)
> +    MONREG("decr", cpu_ppc_load_decr(env))
> +    MONREG("msr",  env->msr)
> +    MONREG("tbu",  cpu_ppc_load_tbu(env))
> +    MONREG("tbl", cpu_ppc_load_tbl(env))
> +
> +    if (strcasecmp("ccr", name) == 0) {
> +        unsigned int u = 0;
> +
> +        for (i = 0; i < 8; i++)
> +            u |= env->crf[i] << (32 - (4 * (i + 1)));
> +
> +        return u;
> +    }
> +
> +    /* General purpose registers */
> +    if (name[0] == 'r') {
> +        return ppc_cpu_get_reg(env->gpr, name + 1, ARRAY_SIZE(env->gpr), pval);
> +    }
> +
> +    /* Floating point registers */
> +    if (name[0] == 'f') {
> +        return ppc_cpu_get_reg(env->fpr, name + 1, ARRAY_SIZE(env->fpr), pval);
> +    }
> +
> +    /* Segment registers */
> +    if (strncmp(name, "sr", 2) == 0) {
> +        return ppc_cpu_get_reg(env->sr, name + 2, ARRAY_SIZE(env->sr), pval);
> +    }
> +
> +    /* Special purpose registers */
> +    for (i = 0; i < ARRAY_SIZE(env->spr_cb); ++i) {
> +        ppc_spr_t *spr = &env->spr_cb[i];
> +
> +        if (spr->name && (strcasecmp(name, spr->name) == 0)) {
> +            *pval = env->spr[i];
> +            return 0;
> +        }
> +    }
> +
> +    return -EINVAL;
> +}

Since translate.c is very overcrowded already ... maybe you could put
this code into a separate file instead? target-ppc/monitor.c ? Or maybe
target-ppc/cpu.c ?

 Thomas

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH qemu 2/2] target-ppc: Define get_monitor_def
  2015-08-06  6:33   ` Thomas Huth
@ 2015-08-06  7:00     ` Alexey Kardashevskiy
  2015-08-06  7:07       ` Thomas Huth
  0 siblings, 1 reply; 13+ messages in thread
From: Alexey Kardashevskiy @ 2015-08-06  7:00 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel
  Cc: Alexander Graf, Markus Armbruster, Luiz Capitulino, qemu-ppc,
	Andreas Färber, David Gibson

On 08/06/2015 04:33 PM, Thomas Huth wrote:
> On 06/08/15 07:25, Alexey Kardashevskiy wrote:
>> At the moment get_monitor_def() prints only registers from monitor_defs.
>> However there is a lot of BOOK3S SPRs which are not in the list and
>> cannot be printed.
>>
>> This makes use of the new get_monitor_def() callback and prints all
>> registered SPRs and fails on unregistered ones proving the user
>> information on what is actually supported in the running CPU.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>   monitor.c                   | 215 +-------------------------------------------
>>   target-ppc/cpu-qom.h        |   2 +
>>   target-ppc/translate.c      |  72 +++++++++++++++
>>   target-ppc/translate_init.c |   1 +
>>   4 files changed, 76 insertions(+), 214 deletions(-)
> ...
>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>> index 84c5cea..f4acafb 100644
>> --- a/target-ppc/translate.c
>> +++ b/target-ppc/translate.c
>> @@ -11401,6 +11401,78 @@ void ppc_cpu_dump_statistics(CPUState *cs, FILE*f,
>>   #endif
>>   }
>>
>> +static int ppc_cpu_get_reg(target_ulong *regs, const char *numstr, int maxnum,
>> +                           uint64_t *pval)
>
> Don't you break the 32-bit QEMU (ppc-softmmu instead of ppc64-softmmu)
> here? Since pval is uint64_t but the registers are target_ulong = 32 bit ?


I cannot see how I break it - 64bit is enough for both, 32bit will just 
have upper bits set to zero.


>
>> +{
>> +    char *endptr = NULL;
>> +    int regnum = strtoul(numstr, &endptr, 10);
>> +
>> +    if ((endptr && *endptr) || (regnum >= maxnum)) {
>
> I'll never get used to your bracketism...
>
>> +        return -EINVAL;
>> +    }
>> +    *pval = regs[regnum];
>> +
>> +    return 0;
>> +}
>> +
>> +int ppc_cpu_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval)
>> +{
>> +    int i;
>> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
>> +    CPUPPCState *env = &cpu->env;
>> +
>> +#define MONREG(s, f) \
>> +    if ((strcasecmp((s), name) == 0)) { \
>
> Remove at least here the outermost round brackets?

Oh, leftovers...


>> +        *pval = (f); \
>> +        return 0; \
>> +    }
>
> ... also defining a macro with parameters and code within a function
> looks somewhat strange to me. Maybe you could consider moving this in
> front of the function?

Why? It is very very local and not intended to be used anywhere else, why 
to push people to look for their definitions somewhere else?


>> +    MONREG("pc", env->nip)
>> +    MONREG("nip", env->nip)
>> +    MONREG("lr", env->lr)
>> +    MONREG("ctr", env->ctr)
>> +    MONREG("xer", env->xer)
>> +    MONREG("decr", cpu_ppc_load_decr(env))
>> +    MONREG("msr",  env->msr)
>> +    MONREG("tbu",  cpu_ppc_load_tbu(env))
>> +    MONREG("tbl", cpu_ppc_load_tbl(env))
>> +
>> +    if (strcasecmp("ccr", name) == 0) {
>> +        unsigned int u = 0;
>> +
>> +        for (i = 0; i < 8; i++)
>> +            u |= env->crf[i] << (32 - (4 * (i + 1)));
>> +
>> +        return u;
>> +    }
>> +
>> +    /* General purpose registers */
>> +    if (name[0] == 'r') {
>> +        return ppc_cpu_get_reg(env->gpr, name + 1, ARRAY_SIZE(env->gpr), pval);
>> +    }
>> +
>> +    /* Floating point registers */
>> +    if (name[0] == 'f') {
>> +        return ppc_cpu_get_reg(env->fpr, name + 1, ARRAY_SIZE(env->fpr), pval);
>> +    }
>> +
>> +    /* Segment registers */
>> +    if (strncmp(name, "sr", 2) == 0) {
>> +        return ppc_cpu_get_reg(env->sr, name + 2, ARRAY_SIZE(env->sr), pval);
>> +    }
>> +
>> +    /* Special purpose registers */
>> +    for (i = 0; i < ARRAY_SIZE(env->spr_cb); ++i) {
>> +        ppc_spr_t *spr = &env->spr_cb[i];
>> +
>> +        if (spr->name && (strcasecmp(name, spr->name) == 0)) {
>> +            *pval = env->spr[i];
>> +            return 0;
>> +        }
>> +    }
>> +
>> +    return -EINVAL;
>> +}
>
> Since translate.c is very overcrowded already ... maybe you could put
> this code into a separate file instead? target-ppc/monitor.c ? Or maybe
> target-ppc/cpu.c ?

Well, I will do that as well (and move ppc_cpu_dump_state&co to this new 
file) if the whole approach will be ack'ed.




-- 
Alexey

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH qemu 2/2] target-ppc: Define get_monitor_def
  2015-08-06  7:00     ` Alexey Kardashevskiy
@ 2015-08-06  7:07       ` Thomas Huth
  0 siblings, 0 replies; 13+ messages in thread
From: Thomas Huth @ 2015-08-06  7:07 UTC (permalink / raw)
  To: Alexey Kardashevskiy, qemu-devel
  Cc: Alexander Graf, Markus Armbruster, Luiz Capitulino, qemu-ppc,
	Andreas Färber, David Gibson

On 06/08/15 09:00, Alexey Kardashevskiy wrote:
> On 08/06/2015 04:33 PM, Thomas Huth wrote:
>> On 06/08/15 07:25, Alexey Kardashevskiy wrote:
>>> At the moment get_monitor_def() prints only registers from monitor_defs.
>>> However there is a lot of BOOK3S SPRs which are not in the list and
>>> cannot be printed.
>>>
>>> This makes use of the new get_monitor_def() callback and prints all
>>> registered SPRs and fails on unregistered ones proving the user
>>> information on what is actually supported in the running CPU.
>>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>>   monitor.c                   | 215
>>> +-------------------------------------------
>>>   target-ppc/cpu-qom.h        |   2 +
>>>   target-ppc/translate.c      |  72 +++++++++++++++
>>>   target-ppc/translate_init.c |   1 +
>>>   4 files changed, 76 insertions(+), 214 deletions(-)
>> ...
>>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>>> index 84c5cea..f4acafb 100644
>>> --- a/target-ppc/translate.c
>>> +++ b/target-ppc/translate.c
>>> @@ -11401,6 +11401,78 @@ void ppc_cpu_dump_statistics(CPUState *cs,
>>> FILE*f,
>>>   #endif
>>>   }
>>>
>>> +static int ppc_cpu_get_reg(target_ulong *regs, const char *numstr,
>>> int maxnum,
>>> +                           uint64_t *pval)
>>
>> Don't you break the 32-bit QEMU (ppc-softmmu instead of ppc64-softmmu)
>> here? Since pval is uint64_t but the registers are target_ulong = 32
>> bit ?
> 
> 
> I cannot see how I break it - 64bit is enough for both, 32bit will just
> have upper bits set to zero.

Ah, stupid me, I somehow mixed up the pval and the regs pointer ...
never mind!

 Thomas

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH qemu 1/2] monitor: Add CPU class callback to read registers for monitor
  2015-08-06  5:25 ` [Qemu-devel] [PATCH qemu 1/2] monitor: Add CPU class callback to read registers for monitor Alexey Kardashevskiy
@ 2015-08-12  1:12   ` David Gibson
  0 siblings, 0 replies; 13+ messages in thread
From: David Gibson @ 2015-08-12  1:12 UTC (permalink / raw)
  To: Alexey Kardashevskiy
  Cc: Alexander Graf, qemu-devel, Markus Armbruster, Luiz Capitulino,
	qemu-ppc, Andreas Färber

[-- Attachment #1: Type: text/plain, Size: 2542 bytes --]

On Thu, Aug 06, 2015 at 03:25:56PM +1000, Alexey Kardashevskiy wrote:
> At the moment the monitor only prints registers from monitor_defs.
> Some may not be supported but it will print those anyway, other
> may be missing in the list so monitor_defs needs an update every time
> new register is added.
> 
> This defines a CPUClass callback to read various registers from CPU.
> 
> Next patch makes use of it.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>


> ---
>  include/qom/cpu.h |  1 +
>  monitor.c         | 14 +++++++++++++-
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 20aabc9..fcf981f 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -144,6 +144,7 @@ typedef struct CPUClass {
>                         int flags);
>      void (*dump_statistics)(CPUState *cpu, FILE *f,
>                              fprintf_function cpu_fprintf, int flags);
> +    int (*get_monitor_def)(CPUState *cs, const char *name, uint64_t *pval);
>      int64_t (*get_arch_id)(CPUState *cpu);
>      bool (*get_paging_enabled)(const CPUState *cpu);
>      void (*get_memory_mapping)(CPUState *cpu, MemoryMappingList *list,
> diff --git a/monitor.c b/monitor.c
> index aeea2b5..bdfcacc 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -3303,13 +3303,25 @@ static int get_monitor_def(target_long *pval, const char *name)
>  {
>      const MonitorDef *md;
>      void *ptr;
> +    CPUState *cs = mon_get_cpu();
> +    CPUClass *cc = CPU_GET_CLASS(cs);
> +
> +    if (cc->get_monitor_def) {
> +        uint64_t tmp = 0;
> +        int ret = cc->get_monitor_def(cs, name, &tmp);
> +
> +        if (!ret) {
> +            *pval = (target_long) tmp;
> +        }
> +        return ret;
> +    }
>  
>      for(md = monitor_defs; md->name != NULL; md++) {
>          if (compare_cmd(name, md->name)) {
>              if (md->get_value) {
>                  *pval = md->get_value(md, md->offset);
>              } else {
> -                CPUArchState *env = mon_get_cpu_env();
> +                CPUArchState *env = cs->env_ptr;
>                  ptr = (uint8_t *)env + md->offset;
>                  switch(md->type) {
>                  case MD_I32:

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH qemu 2/2] target-ppc: Define get_monitor_def
  2015-08-06  5:25 ` [Qemu-devel] [PATCH qemu 2/2] target-ppc: Define get_monitor_def Alexey Kardashevskiy
  2015-08-06  6:33   ` Thomas Huth
@ 2015-08-12  1:21   ` David Gibson
  2015-08-13 15:52     ` [Qemu-devel] [PATCH qemu v2] " Alexey Kardashevskiy
  1 sibling, 1 reply; 13+ messages in thread
From: David Gibson @ 2015-08-12  1:21 UTC (permalink / raw)
  To: Alexey Kardashevskiy
  Cc: Alexander Graf, qemu-devel, Markus Armbruster, Luiz Capitulino,
	qemu-ppc, Andreas Färber

[-- Attachment #1: Type: text/plain, Size: 4435 bytes --]

On Thu, Aug 06, 2015 at 03:25:57PM +1000, Alexey Kardashevskiy wrote:
> At the moment get_monitor_def() prints only registers from monitor_defs.
> However there is a lot of BOOK3S SPRs which are not in the list and
> cannot be printed.
> 
> This makes use of the new get_monitor_def() callback and prints all
> registered SPRs and fails on unregistered ones proving the user
> information on what is actually supported in the running CPU.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

The idea looks sound, but..

[snip]
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 84c5cea..f4acafb 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -11401,6 +11401,78 @@ void ppc_cpu_dump_statistics(CPUState *cs, FILE*f,
>  #endif
>  }
>  
> +static int ppc_cpu_get_reg(target_ulong *regs, const char *numstr, int maxnum,
> +                           uint64_t *pval)
> +{
> +    char *endptr = NULL;
> +    int regnum = strtoul(numstr, &endptr, 10);
> +
> +    if ((endptr && *endptr) || (regnum >= maxnum)) {
> +        return -EINVAL;
> +    }
> +    *pval = regs[regnum];
> +
> +    return 0;
> +}
> +
> +int ppc_cpu_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval)
> +{
> +    int i;
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    CPUPPCState *env = &cpu->env;
> +
> +#define MONREG(s, f) \
> +    if ((strcasecmp((s), name) == 0)) { \
> +        *pval = (f); \
> +        return 0; \
> +    }
> +    MONREG("pc", env->nip)
> +    MONREG("nip", env->nip)
> +    MONREG("lr", env->lr)
> +    MONREG("ctr", env->ctr)
> +    MONREG("xer", env->xer)
> +    MONREG("decr", cpu_ppc_load_decr(env))
> +    MONREG("msr",  env->msr)
> +    MONREG("tbu",  cpu_ppc_load_tbu(env))
> +    MONREG("tbl", cpu_ppc_load_tbl(env))
> +
> +    if (strcasecmp("ccr", name) == 0) {

Probably want to recognize "cr" here as well as "ccr".

> +        unsigned int u = 0;
> +
> +        for (i = 0; i < 8; i++)
> +            u |= env->crf[i] << (32 - (4 * (i + 1)));
> +
> +        return u;
> +    }
> +
> +    /* General purpose registers */
> +    if (name[0] == 'r') {
> +        return ppc_cpu_get_reg(env->gpr, name + 1, ARRAY_SIZE(env->gpr), pval);

This means you won't be able to display SPRs whose names start with
"r" (there aren't many, but there are a few).

Well.. ok, you can by using a capital "R".  But being able to see some
SPRs with lower-case names, but not others definitely violates the
least surprise principle.

> +    }
> +
> +    /* Floating point registers */
> +    if (name[0] == 'f') {
> +        return ppc_cpu_get_reg(env->fpr, name + 1,
> ARRAY_SIZE(env->fpr), pval);

Likewise SPRs beginning with "f" (e.g. fscr).

> +    }
> +
> +    /* Segment registers */
> +    if (strncmp(name, "sr", 2) == 0) {
> +        return ppc_cpu_get_reg(env->sr, name + 2, ARRAY_SIZE(env->sr), pval);

And "sr.." such as the rather important srr0 and srr1.

> +    }
> +
> +    /* Special purpose registers */
> +    for (i = 0; i < ARRAY_SIZE(env->spr_cb); ++i) {
> +        ppc_spr_t *spr = &env->spr_cb[i];
> +
> +        if (spr->name && (strcasecmp(name, spr->name) == 0)) {
> +            *pval = env->spr[i];
> +            return 0;
> +        }
> +    }
> +
> +    return -EINVAL;
> +}
> +
>  /*****************************************************************************/
>  static inline void gen_intermediate_code_internal(PowerPCCPU *cpu,
>                                                    TranslationBlock *tb,
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 16d7b16..038674a 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -9706,6 +9706,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
>      cc->cpu_exec_interrupt = ppc_cpu_exec_interrupt;
>      cc->dump_state = ppc_cpu_dump_state;
>      cc->dump_statistics = ppc_cpu_dump_statistics;
> +    cc->get_monitor_def = ppc_cpu_get_monitor_def;
>      cc->set_pc = ppc_cpu_set_pc;
>      cc->gdb_read_register = ppc_cpu_gdb_read_register;
>      cc->gdb_write_register = ppc_cpu_gdb_write_register;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH qemu v2] target-ppc: Define get_monitor_def
  2015-08-12  1:21   ` David Gibson
@ 2015-08-13 15:52     ` Alexey Kardashevskiy
  2015-08-13 22:39       ` David Gibson
  0 siblings, 1 reply; 13+ messages in thread
From: Alexey Kardashevskiy @ 2015-08-13 15:52 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, qemu-ppc, David Gibson

At the moment get_monitor_def() prints only registers from monitor_defs.
However there is a lot of BOOK3S SPRs which are not in the list and
cannot be printed.

This makes use of the new get_monitor_def() callback and prints all
registered SPRs and fails on unregistered ones proving the user
information on what is actually supported in the running CPU.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v2:
* handles r**, f**, sr** if their numbers  were parsed completely and correctly
* added "cr" as synonym of "ccr"
---
 monitor.c                   | 215 +-------------------------------------------
 target-ppc/cpu-qom.h        |   2 +
 target-ppc/translate.c      |  75 ++++++++++++++++
 target-ppc/translate_init.c |   1 +
 4 files changed, 79 insertions(+), 214 deletions(-)

diff --git a/monitor.c b/monitor.c
index bdfcacc..07ca678 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2916,51 +2916,6 @@ static target_long monitor_get_pc (const struct MonitorDef *md, int val)
 }
 #endif
 
-#if defined(TARGET_PPC)
-static target_long monitor_get_ccr (const struct MonitorDef *md, int val)
-{
-    CPUArchState *env = mon_get_cpu_env();
-    unsigned int u;
-    int i;
-
-    u = 0;
-    for (i = 0; i < 8; i++)
-        u |= env->crf[i] << (32 - (4 * (i + 1)));
-
-    return u;
-}
-
-static target_long monitor_get_msr (const struct MonitorDef *md, int val)
-{
-    CPUArchState *env = mon_get_cpu_env();
-    return env->msr;
-}
-
-static target_long monitor_get_xer (const struct MonitorDef *md, int val)
-{
-    CPUArchState *env = mon_get_cpu_env();
-    return env->xer;
-}
-
-static target_long monitor_get_decr (const struct MonitorDef *md, int val)
-{
-    CPUArchState *env = mon_get_cpu_env();
-    return cpu_ppc_load_decr(env);
-}
-
-static target_long monitor_get_tbu (const struct MonitorDef *md, int val)
-{
-    CPUArchState *env = mon_get_cpu_env();
-    return cpu_ppc_load_tbu(env);
-}
-
-static target_long monitor_get_tbl (const struct MonitorDef *md, int val)
-{
-    CPUArchState *env = mon_get_cpu_env();
-    return cpu_ppc_load_tbl(env);
-}
-#endif
-
 #if defined(TARGET_SPARC)
 #ifndef TARGET_SPARC64
 static target_long monitor_get_psr (const struct MonitorDef *md, int val)
@@ -3014,175 +2969,7 @@ static const MonitorDef monitor_defs[] = {
     SEG("gs", R_GS)
     { "pc", 0, monitor_get_pc, },
 #elif defined(TARGET_PPC)
-    /* General purpose registers */
-    { "r0", offsetof(CPUPPCState, gpr[0]) },
-    { "r1", offsetof(CPUPPCState, gpr[1]) },
-    { "r2", offsetof(CPUPPCState, gpr[2]) },
-    { "r3", offsetof(CPUPPCState, gpr[3]) },
-    { "r4", offsetof(CPUPPCState, gpr[4]) },
-    { "r5", offsetof(CPUPPCState, gpr[5]) },
-    { "r6", offsetof(CPUPPCState, gpr[6]) },
-    { "r7", offsetof(CPUPPCState, gpr[7]) },
-    { "r8", offsetof(CPUPPCState, gpr[8]) },
-    { "r9", offsetof(CPUPPCState, gpr[9]) },
-    { "r10", offsetof(CPUPPCState, gpr[10]) },
-    { "r11", offsetof(CPUPPCState, gpr[11]) },
-    { "r12", offsetof(CPUPPCState, gpr[12]) },
-    { "r13", offsetof(CPUPPCState, gpr[13]) },
-    { "r14", offsetof(CPUPPCState, gpr[14]) },
-    { "r15", offsetof(CPUPPCState, gpr[15]) },
-    { "r16", offsetof(CPUPPCState, gpr[16]) },
-    { "r17", offsetof(CPUPPCState, gpr[17]) },
-    { "r18", offsetof(CPUPPCState, gpr[18]) },
-    { "r19", offsetof(CPUPPCState, gpr[19]) },
-    { "r20", offsetof(CPUPPCState, gpr[20]) },
-    { "r21", offsetof(CPUPPCState, gpr[21]) },
-    { "r22", offsetof(CPUPPCState, gpr[22]) },
-    { "r23", offsetof(CPUPPCState, gpr[23]) },
-    { "r24", offsetof(CPUPPCState, gpr[24]) },
-    { "r25", offsetof(CPUPPCState, gpr[25]) },
-    { "r26", offsetof(CPUPPCState, gpr[26]) },
-    { "r27", offsetof(CPUPPCState, gpr[27]) },
-    { "r28", offsetof(CPUPPCState, gpr[28]) },
-    { "r29", offsetof(CPUPPCState, gpr[29]) },
-    { "r30", offsetof(CPUPPCState, gpr[30]) },
-    { "r31", offsetof(CPUPPCState, gpr[31]) },
-    /* Floating point registers */
-    { "f0", offsetof(CPUPPCState, fpr[0]) },
-    { "f1", offsetof(CPUPPCState, fpr[1]) },
-    { "f2", offsetof(CPUPPCState, fpr[2]) },
-    { "f3", offsetof(CPUPPCState, fpr[3]) },
-    { "f4", offsetof(CPUPPCState, fpr[4]) },
-    { "f5", offsetof(CPUPPCState, fpr[5]) },
-    { "f6", offsetof(CPUPPCState, fpr[6]) },
-    { "f7", offsetof(CPUPPCState, fpr[7]) },
-    { "f8", offsetof(CPUPPCState, fpr[8]) },
-    { "f9", offsetof(CPUPPCState, fpr[9]) },
-    { "f10", offsetof(CPUPPCState, fpr[10]) },
-    { "f11", offsetof(CPUPPCState, fpr[11]) },
-    { "f12", offsetof(CPUPPCState, fpr[12]) },
-    { "f13", offsetof(CPUPPCState, fpr[13]) },
-    { "f14", offsetof(CPUPPCState, fpr[14]) },
-    { "f15", offsetof(CPUPPCState, fpr[15]) },
-    { "f16", offsetof(CPUPPCState, fpr[16]) },
-    { "f17", offsetof(CPUPPCState, fpr[17]) },
-    { "f18", offsetof(CPUPPCState, fpr[18]) },
-    { "f19", offsetof(CPUPPCState, fpr[19]) },
-    { "f20", offsetof(CPUPPCState, fpr[20]) },
-    { "f21", offsetof(CPUPPCState, fpr[21]) },
-    { "f22", offsetof(CPUPPCState, fpr[22]) },
-    { "f23", offsetof(CPUPPCState, fpr[23]) },
-    { "f24", offsetof(CPUPPCState, fpr[24]) },
-    { "f25", offsetof(CPUPPCState, fpr[25]) },
-    { "f26", offsetof(CPUPPCState, fpr[26]) },
-    { "f27", offsetof(CPUPPCState, fpr[27]) },
-    { "f28", offsetof(CPUPPCState, fpr[28]) },
-    { "f29", offsetof(CPUPPCState, fpr[29]) },
-    { "f30", offsetof(CPUPPCState, fpr[30]) },
-    { "f31", offsetof(CPUPPCState, fpr[31]) },
-    { "fpscr", offsetof(CPUPPCState, fpscr) },
-    /* Next instruction pointer */
-    { "nip|pc", offsetof(CPUPPCState, nip) },
-    { "lr", offsetof(CPUPPCState, lr) },
-    { "ctr", offsetof(CPUPPCState, ctr) },
-    { "decr", 0, &monitor_get_decr, },
-    { "ccr", 0, &monitor_get_ccr, },
-    /* Machine state register */
-    { "msr", 0, &monitor_get_msr, },
-    { "xer", 0, &monitor_get_xer, },
-    { "tbu", 0, &monitor_get_tbu, },
-    { "tbl", 0, &monitor_get_tbl, },
-    /* Segment registers */
-    { "sdr1", offsetof(CPUPPCState, spr[SPR_SDR1]) },
-    { "sr0", offsetof(CPUPPCState, sr[0]) },
-    { "sr1", offsetof(CPUPPCState, sr[1]) },
-    { "sr2", offsetof(CPUPPCState, sr[2]) },
-    { "sr3", offsetof(CPUPPCState, sr[3]) },
-    { "sr4", offsetof(CPUPPCState, sr[4]) },
-    { "sr5", offsetof(CPUPPCState, sr[5]) },
-    { "sr6", offsetof(CPUPPCState, sr[6]) },
-    { "sr7", offsetof(CPUPPCState, sr[7]) },
-    { "sr8", offsetof(CPUPPCState, sr[8]) },
-    { "sr9", offsetof(CPUPPCState, sr[9]) },
-    { "sr10", offsetof(CPUPPCState, sr[10]) },
-    { "sr11", offsetof(CPUPPCState, sr[11]) },
-    { "sr12", offsetof(CPUPPCState, sr[12]) },
-    { "sr13", offsetof(CPUPPCState, sr[13]) },
-    { "sr14", offsetof(CPUPPCState, sr[14]) },
-    { "sr15", offsetof(CPUPPCState, sr[15]) },
-    /* Too lazy to put BATs... */
-    { "pvr", offsetof(CPUPPCState, spr[SPR_PVR]) },
-
-    { "srr0", offsetof(CPUPPCState, spr[SPR_SRR0]) },
-    { "srr1", offsetof(CPUPPCState, spr[SPR_SRR1]) },
-    { "dar", offsetof(CPUPPCState, spr[SPR_DAR]) },
-    { "dsisr", offsetof(CPUPPCState, spr[SPR_DSISR]) },
-    { "cfar", offsetof(CPUPPCState, spr[SPR_CFAR]) },
-    { "sprg0", offsetof(CPUPPCState, spr[SPR_SPRG0]) },
-    { "sprg1", offsetof(CPUPPCState, spr[SPR_SPRG1]) },
-    { "sprg2", offsetof(CPUPPCState, spr[SPR_SPRG2]) },
-    { "sprg3", offsetof(CPUPPCState, spr[SPR_SPRG3]) },
-    { "sprg4", offsetof(CPUPPCState, spr[SPR_SPRG4]) },
-    { "sprg5", offsetof(CPUPPCState, spr[SPR_SPRG5]) },
-    { "sprg6", offsetof(CPUPPCState, spr[SPR_SPRG6]) },
-    { "sprg7", offsetof(CPUPPCState, spr[SPR_SPRG7]) },
-    { "pid", offsetof(CPUPPCState, spr[SPR_BOOKE_PID]) },
-    { "csrr0", offsetof(CPUPPCState, spr[SPR_BOOKE_CSRR0]) },
-    { "csrr1", offsetof(CPUPPCState, spr[SPR_BOOKE_CSRR1]) },
-    { "esr", offsetof(CPUPPCState, spr[SPR_BOOKE_ESR]) },
-    { "dear", offsetof(CPUPPCState, spr[SPR_BOOKE_DEAR]) },
-    { "mcsr", offsetof(CPUPPCState, spr[SPR_BOOKE_MCSR]) },
-    { "tsr", offsetof(CPUPPCState, spr[SPR_BOOKE_TSR]) },
-    { "tcr", offsetof(CPUPPCState, spr[SPR_BOOKE_TCR]) },
-    { "vrsave", offsetof(CPUPPCState, spr[SPR_VRSAVE]) },
-    { "pir", offsetof(CPUPPCState, spr[SPR_BOOKE_PIR]) },
-    { "mcsrr0", offsetof(CPUPPCState, spr[SPR_BOOKE_MCSRR0]) },
-    { "mcsrr1", offsetof(CPUPPCState, spr[SPR_BOOKE_MCSRR1]) },
-    { "decar", offsetof(CPUPPCState, spr[SPR_BOOKE_DECAR]) },
-    { "ivpr", offsetof(CPUPPCState, spr[SPR_BOOKE_IVPR]) },
-    { "epcr", offsetof(CPUPPCState, spr[SPR_BOOKE_EPCR]) },
-    { "sprg8", offsetof(CPUPPCState, spr[SPR_BOOKE_SPRG8]) },
-    { "ivor0", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR0]) },
-    { "ivor1", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR1]) },
-    { "ivor2", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR2]) },
-    { "ivor3", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR3]) },
-    { "ivor4", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR4]) },
-    { "ivor5", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR5]) },
-    { "ivor6", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR6]) },
-    { "ivor7", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR7]) },
-    { "ivor8", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR8]) },
-    { "ivor9", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR9]) },
-    { "ivor10", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR10]) },
-    { "ivor11", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR11]) },
-    { "ivor12", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR12]) },
-    { "ivor13", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR13]) },
-    { "ivor14", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR14]) },
-    { "ivor15", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR15]) },
-    { "ivor32", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR32]) },
-    { "ivor33", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR33]) },
-    { "ivor34", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR34]) },
-    { "ivor35", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR35]) },
-    { "ivor36", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR36]) },
-    { "ivor37", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR37]) },
-    { "mas0", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS0]) },
-    { "mas1", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS1]) },
-    { "mas2", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS2]) },
-    { "mas3", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS3]) },
-    { "mas4", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS4]) },
-    { "mas6", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS6]) },
-    { "mas7", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS7]) },
-    { "mmucfg", offsetof(CPUPPCState, spr[SPR_MMUCFG]) },
-    { "tlb0cfg", offsetof(CPUPPCState, spr[SPR_BOOKE_TLB0CFG]) },
-    { "tlb1cfg", offsetof(CPUPPCState, spr[SPR_BOOKE_TLB1CFG]) },
-    { "epr", offsetof(CPUPPCState, spr[SPR_BOOKE_EPR]) },
-    { "eplc", offsetof(CPUPPCState, spr[SPR_BOOKE_EPLC]) },
-    { "epsc", offsetof(CPUPPCState, spr[SPR_BOOKE_EPSC]) },
-    { "svr", offsetof(CPUPPCState, spr[SPR_E500_SVR]) },
-    { "mcar", offsetof(CPUPPCState, spr[SPR_Exxx_MCAR]) },
-    { "pid1", offsetof(CPUPPCState, spr[SPR_BOOKE_PID1]) },
-    { "pid2", offsetof(CPUPPCState, spr[SPR_BOOKE_PID2]) },
-    { "hid0", offsetof(CPUPPCState, spr[SPR_HID0]) },
-
+    /* Uses get_monitor_def() */
 #elif defined(TARGET_SPARC)
     { "g0", offsetof(CPUSPARCState, gregs[0]) },
     { "g1", offsetof(CPUSPARCState, gregs[1]) },
diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
index 6967a80..bc20504 100644
--- a/target-ppc/cpu-qom.h
+++ b/target-ppc/cpu-qom.h
@@ -118,6 +118,8 @@ void ppc_cpu_dump_state(CPUState *cpu, FILE *f, fprintf_function cpu_fprintf,
                         int flags);
 void ppc_cpu_dump_statistics(CPUState *cpu, FILE *f,
                              fprintf_function cpu_fprintf, int flags);
+int ppc_cpu_get_monitor_def(CPUState *cs, const char *name,
+                            uint64_t *pval);
 hwaddr ppc_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 int ppc_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
 int ppc_cpu_gdb_read_register_apple(CPUState *cpu, uint8_t *buf, int reg);
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 84c5cea..a988959 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -11401,6 +11401,81 @@ void ppc_cpu_dump_statistics(CPUState *cs, FILE*f,
 #endif
 }
 
+static bool ppc_cpu_get_reg(target_ulong *regs, const char *numstr, int maxnum,
+                            uint64_t *pval)
+{
+    char *endptr = NULL;
+    int regnum = strtoul(numstr, &endptr, 10);
+
+    if ((endptr && *endptr) || (regnum >= maxnum)) {
+        return false;
+    }
+    *pval = regs[regnum];
+
+    return true;
+}
+
+int ppc_cpu_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval)
+{
+    int i;
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    CPUPPCState *env = &cpu->env;
+
+#define MONREG(s, f) \
+    if ((strcasecmp((s), name) == 0)) { \
+        *pval = (f); \
+        return 0; \
+    }
+    MONREG("pc", env->nip)
+    MONREG("nip", env->nip)
+    MONREG("lr", env->lr)
+    MONREG("ctr", env->ctr)
+    MONREG("xer", env->xer)
+    MONREG("decr", cpu_ppc_load_decr(env))
+    MONREG("msr",  env->msr)
+    MONREG("tbu",  cpu_ppc_load_tbu(env))
+    MONREG("tbl", cpu_ppc_load_tbl(env))
+
+    if ((strcasecmp("ccr", name) == 0) || (strcasecmp("cr", name) == 0)) {
+        unsigned int u = 0;
+
+        for (i = 0; i < 8; i++)
+            u |= env->crf[i] << (32 - (4 * (i + 1)));
+
+        return u;
+    }
+
+    /* General purpose registers */
+    if ((name[0] == 'r') &&
+        ppc_cpu_get_reg(env->gpr, name + 1, ARRAY_SIZE(env->gpr), pval)) {
+        return 0;
+    }
+
+    /* Floating point registers */
+    if ((name[0] == 'f') &&
+        ppc_cpu_get_reg(env->fpr, name + 1, ARRAY_SIZE(env->fpr), pval)) {
+        return 0;
+    }
+
+    /* Segment registers */
+    if ((strncmp(name, "sr", 2) == 0) &&
+        ppc_cpu_get_reg(env->sr, name + 2, ARRAY_SIZE(env->sr), pval)) {
+        return 0;
+    }
+
+    /* Special purpose registers */
+    for (i = 0; i < ARRAY_SIZE(env->spr_cb); ++i) {
+        ppc_spr_t *spr = &env->spr_cb[i];
+
+        if (spr->name && (strcasecmp(name, spr->name) == 0)) {
+            *pval = env->spr[i];
+            return 0;
+        }
+    }
+
+    return -EINVAL;
+}
+
 /*****************************************************************************/
 static inline void gen_intermediate_code_internal(PowerPCCPU *cpu,
                                                   TranslationBlock *tb,
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 16d7b16..038674a 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9706,6 +9706,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
     cc->cpu_exec_interrupt = ppc_cpu_exec_interrupt;
     cc->dump_state = ppc_cpu_dump_state;
     cc->dump_statistics = ppc_cpu_dump_statistics;
+    cc->get_monitor_def = ppc_cpu_get_monitor_def;
     cc->set_pc = ppc_cpu_set_pc;
     cc->gdb_read_register = ppc_cpu_gdb_read_register;
     cc->gdb_write_register = ppc_cpu_gdb_write_register;
-- 
2.4.0.rc3.8.gfb3e7d5

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH qemu v2] target-ppc: Define get_monitor_def
  2015-08-13 15:52     ` [Qemu-devel] [PATCH qemu v2] " Alexey Kardashevskiy
@ 2015-08-13 22:39       ` David Gibson
  2015-08-14  3:34         ` [Qemu-devel] [PATCH qemu v3] " Alexey Kardashevskiy
  0 siblings, 1 reply; 13+ messages in thread
From: David Gibson @ 2015-08-13 22:39 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: qemu-ppc, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 4358 bytes --]

On Fri, Aug 14, 2015 at 01:52:18AM +1000, Alexey Kardashevskiy wrote:
> At the moment get_monitor_def() prints only registers from monitor_defs.
> However there is a lot of BOOK3S SPRs which are not in the list and
> cannot be printed.
> 
> This makes use of the new get_monitor_def() callback and prints all
> registered SPRs and fails on unregistered ones proving the user
> information on what is actually supported in the running CPU.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> Changes:
> v2:
> * handles r**, f**, sr** if their numbers  were parsed completely and correctly
> * added "cr" as synonym of "ccr"

[snip]
> +static bool ppc_cpu_get_reg(target_ulong *regs, const char *numstr, int maxnum,
> +                            uint64_t *pval)
> +{
> +    char *endptr = NULL;
> +    int regnum = strtoul(numstr, &endptr, 10);
> +
> +    if ((endptr && *endptr) || (regnum >= maxnum)) {

Sorry I didn't pick this up in v1, but I think these conditiojns
aren't quite right.  *endptr is ok (fail if there were invalid
characters).  But AFAICT from strtoul(3), endptr will *always* be
non-NULL (somewhere between numstr and the end of the string).  You
also need to check if *numstr == '\0' to catch the case of no number
at all.

> +        return false;
> +    }
> +    *pval = regs[regnum];
> +
> +    return true;
> +}
> +
> +int ppc_cpu_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval)
> +{
> +    int i;
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    CPUPPCState *env = &cpu->env;
> +
> +#define MONREG(s, f) \
> +    if ((strcasecmp((s), name) == 0)) { \
> +        *pval = (f); \
> +        return 0; \
> +    }
> +    MONREG("pc", env->nip)
> +    MONREG("nip", env->nip)
> +    MONREG("lr", env->lr)
> +    MONREG("ctr", env->ctr)
> +    MONREG("xer", env->xer)
> +    MONREG("decr", cpu_ppc_load_decr(env))
> +    MONREG("msr",  env->msr)
> +    MONREG("tbu",  cpu_ppc_load_tbu(env))
> +    MONREG("tbl", cpu_ppc_load_tbl(env))
> +
> +    if ((strcasecmp("ccr", name) == 0) || (strcasecmp("cr", name) == 0)) {
> +        unsigned int u = 0;
> +
> +        for (i = 0; i < 8; i++)
> +            u |= env->crf[i] << (32 - (4 * (i + 1)));
> +
> +        return u;
> +    }
> +
> +    /* General purpose registers */
> +    if ((name[0] == 'r') &&

These cases will catch "r3" but not "R3", whereas for sprs it will
handle any case.

> +        ppc_cpu_get_reg(env->gpr, name + 1, ARRAY_SIZE(env->gpr), pval)) {
> +        return 0;
> +    }
> +
> +    /* Floating point registers */
> +    if ((name[0] == 'f') &&
> +        ppc_cpu_get_reg(env->fpr, name + 1, ARRAY_SIZE(env->fpr), pval)) {
> +        return 0;
> +    }
> +
> +    /* Segment registers */
> +    if ((strncmp(name, "sr", 2) == 0) &&
> +        ppc_cpu_get_reg(env->sr, name + 2, ARRAY_SIZE(env->sr), pval)) {
> +        return 0;
> +    }
> +
> +    /* Special purpose registers */
> +    for (i = 0; i < ARRAY_SIZE(env->spr_cb); ++i) {
> +        ppc_spr_t *spr = &env->spr_cb[i];
> +
> +        if (spr->name && (strcasecmp(name, spr->name) == 0)) {
> +            *pval = env->spr[i];
> +            return 0;
> +        }
> +    }
> +
> +    return -EINVAL;
> +}
> +
>  /*****************************************************************************/
>  static inline void gen_intermediate_code_internal(PowerPCCPU *cpu,
>                                                    TranslationBlock *tb,
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 16d7b16..038674a 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -9706,6 +9706,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
>      cc->cpu_exec_interrupt = ppc_cpu_exec_interrupt;
>      cc->dump_state = ppc_cpu_dump_state;
>      cc->dump_statistics = ppc_cpu_dump_statistics;
> +    cc->get_monitor_def = ppc_cpu_get_monitor_def;
>      cc->set_pc = ppc_cpu_set_pc;
>      cc->gdb_read_register = ppc_cpu_gdb_read_register;
>      cc->gdb_write_register = ppc_cpu_gdb_write_register;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH qemu v3] target-ppc: Define get_monitor_def
  2015-08-13 22:39       ` David Gibson
@ 2015-08-14  3:34         ` Alexey Kardashevskiy
  2015-09-07  1:26           ` Alexey Kardashevskiy
  2015-09-23  3:40           ` David Gibson
  0 siblings, 2 replies; 13+ messages in thread
From: Alexey Kardashevskiy @ 2015-08-14  3:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, qemu-ppc, David Gibson

At the moment get_monitor_def() prints only registers from monitor_defs.
However there is a lot of BOOK3S SPRs which are not in the list and
cannot be printed.

This makes use of the new get_monitor_def() callback and prints all
registered SPRs and fails on unregistered ones proving the user
information on what is actually supported in the running CPU.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v3:
* removed the check for endptr as strtoul() always initializes it
* check if there is any number after r/f/sr and fail if none
* added tolower()/strncasecmp() to support both r/f/sr and R/F/SR

v2:
* handles r**, f**, sr** if their numbers  were parsed completely and correctly
* added "cr" as synonym of "ccr"
---
 monitor.c                   | 215 +-------------------------------------------
 target-ppc/cpu-qom.h        |   2 +
 target-ppc/translate.c      |  80 +++++++++++++++++
 target-ppc/translate_init.c |   1 +
 4 files changed, 84 insertions(+), 214 deletions(-)

diff --git a/monitor.c b/monitor.c
index bdfcacc..07ca678 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2916,51 +2916,6 @@ static target_long monitor_get_pc (const struct MonitorDef *md, int val)
 }
 #endif
 
-#if defined(TARGET_PPC)
-static target_long monitor_get_ccr (const struct MonitorDef *md, int val)
-{
-    CPUArchState *env = mon_get_cpu_env();
-    unsigned int u;
-    int i;
-
-    u = 0;
-    for (i = 0; i < 8; i++)
-        u |= env->crf[i] << (32 - (4 * (i + 1)));
-
-    return u;
-}
-
-static target_long monitor_get_msr (const struct MonitorDef *md, int val)
-{
-    CPUArchState *env = mon_get_cpu_env();
-    return env->msr;
-}
-
-static target_long monitor_get_xer (const struct MonitorDef *md, int val)
-{
-    CPUArchState *env = mon_get_cpu_env();
-    return env->xer;
-}
-
-static target_long monitor_get_decr (const struct MonitorDef *md, int val)
-{
-    CPUArchState *env = mon_get_cpu_env();
-    return cpu_ppc_load_decr(env);
-}
-
-static target_long monitor_get_tbu (const struct MonitorDef *md, int val)
-{
-    CPUArchState *env = mon_get_cpu_env();
-    return cpu_ppc_load_tbu(env);
-}
-
-static target_long monitor_get_tbl (const struct MonitorDef *md, int val)
-{
-    CPUArchState *env = mon_get_cpu_env();
-    return cpu_ppc_load_tbl(env);
-}
-#endif
-
 #if defined(TARGET_SPARC)
 #ifndef TARGET_SPARC64
 static target_long monitor_get_psr (const struct MonitorDef *md, int val)
@@ -3014,175 +2969,7 @@ static const MonitorDef monitor_defs[] = {
     SEG("gs", R_GS)
     { "pc", 0, monitor_get_pc, },
 #elif defined(TARGET_PPC)
-    /* General purpose registers */
-    { "r0", offsetof(CPUPPCState, gpr[0]) },
-    { "r1", offsetof(CPUPPCState, gpr[1]) },
-    { "r2", offsetof(CPUPPCState, gpr[2]) },
-    { "r3", offsetof(CPUPPCState, gpr[3]) },
-    { "r4", offsetof(CPUPPCState, gpr[4]) },
-    { "r5", offsetof(CPUPPCState, gpr[5]) },
-    { "r6", offsetof(CPUPPCState, gpr[6]) },
-    { "r7", offsetof(CPUPPCState, gpr[7]) },
-    { "r8", offsetof(CPUPPCState, gpr[8]) },
-    { "r9", offsetof(CPUPPCState, gpr[9]) },
-    { "r10", offsetof(CPUPPCState, gpr[10]) },
-    { "r11", offsetof(CPUPPCState, gpr[11]) },
-    { "r12", offsetof(CPUPPCState, gpr[12]) },
-    { "r13", offsetof(CPUPPCState, gpr[13]) },
-    { "r14", offsetof(CPUPPCState, gpr[14]) },
-    { "r15", offsetof(CPUPPCState, gpr[15]) },
-    { "r16", offsetof(CPUPPCState, gpr[16]) },
-    { "r17", offsetof(CPUPPCState, gpr[17]) },
-    { "r18", offsetof(CPUPPCState, gpr[18]) },
-    { "r19", offsetof(CPUPPCState, gpr[19]) },
-    { "r20", offsetof(CPUPPCState, gpr[20]) },
-    { "r21", offsetof(CPUPPCState, gpr[21]) },
-    { "r22", offsetof(CPUPPCState, gpr[22]) },
-    { "r23", offsetof(CPUPPCState, gpr[23]) },
-    { "r24", offsetof(CPUPPCState, gpr[24]) },
-    { "r25", offsetof(CPUPPCState, gpr[25]) },
-    { "r26", offsetof(CPUPPCState, gpr[26]) },
-    { "r27", offsetof(CPUPPCState, gpr[27]) },
-    { "r28", offsetof(CPUPPCState, gpr[28]) },
-    { "r29", offsetof(CPUPPCState, gpr[29]) },
-    { "r30", offsetof(CPUPPCState, gpr[30]) },
-    { "r31", offsetof(CPUPPCState, gpr[31]) },
-    /* Floating point registers */
-    { "f0", offsetof(CPUPPCState, fpr[0]) },
-    { "f1", offsetof(CPUPPCState, fpr[1]) },
-    { "f2", offsetof(CPUPPCState, fpr[2]) },
-    { "f3", offsetof(CPUPPCState, fpr[3]) },
-    { "f4", offsetof(CPUPPCState, fpr[4]) },
-    { "f5", offsetof(CPUPPCState, fpr[5]) },
-    { "f6", offsetof(CPUPPCState, fpr[6]) },
-    { "f7", offsetof(CPUPPCState, fpr[7]) },
-    { "f8", offsetof(CPUPPCState, fpr[8]) },
-    { "f9", offsetof(CPUPPCState, fpr[9]) },
-    { "f10", offsetof(CPUPPCState, fpr[10]) },
-    { "f11", offsetof(CPUPPCState, fpr[11]) },
-    { "f12", offsetof(CPUPPCState, fpr[12]) },
-    { "f13", offsetof(CPUPPCState, fpr[13]) },
-    { "f14", offsetof(CPUPPCState, fpr[14]) },
-    { "f15", offsetof(CPUPPCState, fpr[15]) },
-    { "f16", offsetof(CPUPPCState, fpr[16]) },
-    { "f17", offsetof(CPUPPCState, fpr[17]) },
-    { "f18", offsetof(CPUPPCState, fpr[18]) },
-    { "f19", offsetof(CPUPPCState, fpr[19]) },
-    { "f20", offsetof(CPUPPCState, fpr[20]) },
-    { "f21", offsetof(CPUPPCState, fpr[21]) },
-    { "f22", offsetof(CPUPPCState, fpr[22]) },
-    { "f23", offsetof(CPUPPCState, fpr[23]) },
-    { "f24", offsetof(CPUPPCState, fpr[24]) },
-    { "f25", offsetof(CPUPPCState, fpr[25]) },
-    { "f26", offsetof(CPUPPCState, fpr[26]) },
-    { "f27", offsetof(CPUPPCState, fpr[27]) },
-    { "f28", offsetof(CPUPPCState, fpr[28]) },
-    { "f29", offsetof(CPUPPCState, fpr[29]) },
-    { "f30", offsetof(CPUPPCState, fpr[30]) },
-    { "f31", offsetof(CPUPPCState, fpr[31]) },
-    { "fpscr", offsetof(CPUPPCState, fpscr) },
-    /* Next instruction pointer */
-    { "nip|pc", offsetof(CPUPPCState, nip) },
-    { "lr", offsetof(CPUPPCState, lr) },
-    { "ctr", offsetof(CPUPPCState, ctr) },
-    { "decr", 0, &monitor_get_decr, },
-    { "ccr", 0, &monitor_get_ccr, },
-    /* Machine state register */
-    { "msr", 0, &monitor_get_msr, },
-    { "xer", 0, &monitor_get_xer, },
-    { "tbu", 0, &monitor_get_tbu, },
-    { "tbl", 0, &monitor_get_tbl, },
-    /* Segment registers */
-    { "sdr1", offsetof(CPUPPCState, spr[SPR_SDR1]) },
-    { "sr0", offsetof(CPUPPCState, sr[0]) },
-    { "sr1", offsetof(CPUPPCState, sr[1]) },
-    { "sr2", offsetof(CPUPPCState, sr[2]) },
-    { "sr3", offsetof(CPUPPCState, sr[3]) },
-    { "sr4", offsetof(CPUPPCState, sr[4]) },
-    { "sr5", offsetof(CPUPPCState, sr[5]) },
-    { "sr6", offsetof(CPUPPCState, sr[6]) },
-    { "sr7", offsetof(CPUPPCState, sr[7]) },
-    { "sr8", offsetof(CPUPPCState, sr[8]) },
-    { "sr9", offsetof(CPUPPCState, sr[9]) },
-    { "sr10", offsetof(CPUPPCState, sr[10]) },
-    { "sr11", offsetof(CPUPPCState, sr[11]) },
-    { "sr12", offsetof(CPUPPCState, sr[12]) },
-    { "sr13", offsetof(CPUPPCState, sr[13]) },
-    { "sr14", offsetof(CPUPPCState, sr[14]) },
-    { "sr15", offsetof(CPUPPCState, sr[15]) },
-    /* Too lazy to put BATs... */
-    { "pvr", offsetof(CPUPPCState, spr[SPR_PVR]) },
-
-    { "srr0", offsetof(CPUPPCState, spr[SPR_SRR0]) },
-    { "srr1", offsetof(CPUPPCState, spr[SPR_SRR1]) },
-    { "dar", offsetof(CPUPPCState, spr[SPR_DAR]) },
-    { "dsisr", offsetof(CPUPPCState, spr[SPR_DSISR]) },
-    { "cfar", offsetof(CPUPPCState, spr[SPR_CFAR]) },
-    { "sprg0", offsetof(CPUPPCState, spr[SPR_SPRG0]) },
-    { "sprg1", offsetof(CPUPPCState, spr[SPR_SPRG1]) },
-    { "sprg2", offsetof(CPUPPCState, spr[SPR_SPRG2]) },
-    { "sprg3", offsetof(CPUPPCState, spr[SPR_SPRG3]) },
-    { "sprg4", offsetof(CPUPPCState, spr[SPR_SPRG4]) },
-    { "sprg5", offsetof(CPUPPCState, spr[SPR_SPRG5]) },
-    { "sprg6", offsetof(CPUPPCState, spr[SPR_SPRG6]) },
-    { "sprg7", offsetof(CPUPPCState, spr[SPR_SPRG7]) },
-    { "pid", offsetof(CPUPPCState, spr[SPR_BOOKE_PID]) },
-    { "csrr0", offsetof(CPUPPCState, spr[SPR_BOOKE_CSRR0]) },
-    { "csrr1", offsetof(CPUPPCState, spr[SPR_BOOKE_CSRR1]) },
-    { "esr", offsetof(CPUPPCState, spr[SPR_BOOKE_ESR]) },
-    { "dear", offsetof(CPUPPCState, spr[SPR_BOOKE_DEAR]) },
-    { "mcsr", offsetof(CPUPPCState, spr[SPR_BOOKE_MCSR]) },
-    { "tsr", offsetof(CPUPPCState, spr[SPR_BOOKE_TSR]) },
-    { "tcr", offsetof(CPUPPCState, spr[SPR_BOOKE_TCR]) },
-    { "vrsave", offsetof(CPUPPCState, spr[SPR_VRSAVE]) },
-    { "pir", offsetof(CPUPPCState, spr[SPR_BOOKE_PIR]) },
-    { "mcsrr0", offsetof(CPUPPCState, spr[SPR_BOOKE_MCSRR0]) },
-    { "mcsrr1", offsetof(CPUPPCState, spr[SPR_BOOKE_MCSRR1]) },
-    { "decar", offsetof(CPUPPCState, spr[SPR_BOOKE_DECAR]) },
-    { "ivpr", offsetof(CPUPPCState, spr[SPR_BOOKE_IVPR]) },
-    { "epcr", offsetof(CPUPPCState, spr[SPR_BOOKE_EPCR]) },
-    { "sprg8", offsetof(CPUPPCState, spr[SPR_BOOKE_SPRG8]) },
-    { "ivor0", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR0]) },
-    { "ivor1", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR1]) },
-    { "ivor2", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR2]) },
-    { "ivor3", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR3]) },
-    { "ivor4", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR4]) },
-    { "ivor5", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR5]) },
-    { "ivor6", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR6]) },
-    { "ivor7", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR7]) },
-    { "ivor8", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR8]) },
-    { "ivor9", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR9]) },
-    { "ivor10", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR10]) },
-    { "ivor11", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR11]) },
-    { "ivor12", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR12]) },
-    { "ivor13", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR13]) },
-    { "ivor14", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR14]) },
-    { "ivor15", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR15]) },
-    { "ivor32", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR32]) },
-    { "ivor33", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR33]) },
-    { "ivor34", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR34]) },
-    { "ivor35", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR35]) },
-    { "ivor36", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR36]) },
-    { "ivor37", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR37]) },
-    { "mas0", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS0]) },
-    { "mas1", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS1]) },
-    { "mas2", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS2]) },
-    { "mas3", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS3]) },
-    { "mas4", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS4]) },
-    { "mas6", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS6]) },
-    { "mas7", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS7]) },
-    { "mmucfg", offsetof(CPUPPCState, spr[SPR_MMUCFG]) },
-    { "tlb0cfg", offsetof(CPUPPCState, spr[SPR_BOOKE_TLB0CFG]) },
-    { "tlb1cfg", offsetof(CPUPPCState, spr[SPR_BOOKE_TLB1CFG]) },
-    { "epr", offsetof(CPUPPCState, spr[SPR_BOOKE_EPR]) },
-    { "eplc", offsetof(CPUPPCState, spr[SPR_BOOKE_EPLC]) },
-    { "epsc", offsetof(CPUPPCState, spr[SPR_BOOKE_EPSC]) },
-    { "svr", offsetof(CPUPPCState, spr[SPR_E500_SVR]) },
-    { "mcar", offsetof(CPUPPCState, spr[SPR_Exxx_MCAR]) },
-    { "pid1", offsetof(CPUPPCState, spr[SPR_BOOKE_PID1]) },
-    { "pid2", offsetof(CPUPPCState, spr[SPR_BOOKE_PID2]) },
-    { "hid0", offsetof(CPUPPCState, spr[SPR_HID0]) },
-
+    /* Uses get_monitor_def() */
 #elif defined(TARGET_SPARC)
     { "g0", offsetof(CPUSPARCState, gregs[0]) },
     { "g1", offsetof(CPUSPARCState, gregs[1]) },
diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
index 6967a80..bc20504 100644
--- a/target-ppc/cpu-qom.h
+++ b/target-ppc/cpu-qom.h
@@ -118,6 +118,8 @@ void ppc_cpu_dump_state(CPUState *cpu, FILE *f, fprintf_function cpu_fprintf,
                         int flags);
 void ppc_cpu_dump_statistics(CPUState *cpu, FILE *f,
                              fprintf_function cpu_fprintf, int flags);
+int ppc_cpu_get_monitor_def(CPUState *cs, const char *name,
+                            uint64_t *pval);
 hwaddr ppc_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 int ppc_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
 int ppc_cpu_gdb_read_register_apple(CPUState *cpu, uint8_t *buf, int reg);
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 84c5cea..2c6f772 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -11401,6 +11401,86 @@ void ppc_cpu_dump_statistics(CPUState *cs, FILE*f,
 #endif
 }
 
+static bool ppc_cpu_get_reg(target_ulong *regs, const char *numstr, int maxnum,
+                            uint64_t *pval)
+{
+    char *endptr = NULL;
+    int regnum;
+
+    if (!*numstr) {
+        return false;
+    }
+
+    regnum = strtoul(numstr, &endptr, 10);
+    if (*endptr || (regnum >= maxnum)) {
+        return false;
+    }
+    *pval = regs[regnum];
+
+    return true;
+}
+
+int ppc_cpu_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval)
+{
+    int i;
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    CPUPPCState *env = &cpu->env;
+
+#define MONREG(s, f) \
+    if ((strcasecmp((s), name) == 0)) { \
+        *pval = (f); \
+        return 0; \
+    }
+    MONREG("pc", env->nip)
+    MONREG("nip", env->nip)
+    MONREG("lr", env->lr)
+    MONREG("ctr", env->ctr)
+    MONREG("xer", env->xer)
+    MONREG("decr", cpu_ppc_load_decr(env))
+    MONREG("msr",  env->msr)
+    MONREG("tbu",  cpu_ppc_load_tbu(env))
+    MONREG("tbl", cpu_ppc_load_tbl(env))
+
+    if ((strcasecmp("ccr", name) == 0) || (strcasecmp("cr", name) == 0)) {
+        unsigned int u = 0;
+
+        for (i = 0; i < 8; i++)
+            u |= env->crf[i] << (32 - (4 * (i + 1)));
+
+        return u;
+    }
+
+    /* General purpose registers */
+    if ((tolower(name[0]) == 'r') &&
+        ppc_cpu_get_reg(env->gpr, name + 1, ARRAY_SIZE(env->gpr), pval)) {
+        return 0;
+    }
+
+    /* Floating point registers */
+    if ((tolower(name[0]) == 'f') &&
+        ppc_cpu_get_reg(env->fpr, name + 1, ARRAY_SIZE(env->fpr), pval)) {
+        return 0;
+    }
+
+    /* Segment registers */
+    if ((strncasecmp(name, "sr", 2) == 0) &&
+        ppc_cpu_get_reg(env->sr, name + 2, ARRAY_SIZE(env->sr), pval)) {
+        return 0;
+    }
+
+    /* Special purpose registers */
+    for (i = 0; i < ARRAY_SIZE(env->spr_cb); ++i) {
+        ppc_spr_t *spr = &env->spr_cb[i];
+
+        if (spr->name && (strcasecmp(name, spr->name) == 0)) {
+            *pval = env->spr[i];
+            return 0;
+        }
+    }
+
+    return -EINVAL;
+}
+
 /*****************************************************************************/
 static inline void gen_intermediate_code_internal(PowerPCCPU *cpu,
                                                   TranslationBlock *tb,
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 16d7b16..038674a 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9706,6 +9706,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
     cc->cpu_exec_interrupt = ppc_cpu_exec_interrupt;
     cc->dump_state = ppc_cpu_dump_state;
     cc->dump_statistics = ppc_cpu_dump_statistics;
+    cc->get_monitor_def = ppc_cpu_get_monitor_def;
     cc->set_pc = ppc_cpu_set_pc;
     cc->gdb_read_register = ppc_cpu_gdb_read_register;
     cc->gdb_write_register = ppc_cpu_gdb_write_register;
-- 
2.4.0.rc3.8.gfb3e7d5

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH qemu v3] target-ppc: Define get_monitor_def
  2015-08-14  3:34         ` [Qemu-devel] [PATCH qemu v3] " Alexey Kardashevskiy
@ 2015-09-07  1:26           ` Alexey Kardashevskiy
  2015-09-23  3:40           ` David Gibson
  1 sibling, 0 replies; 13+ messages in thread
From: Alexey Kardashevskiy @ 2015-09-07  1:26 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, David Gibson

On 08/14/2015 01:34 PM, Alexey Kardashevskiy wrote:
> At the moment get_monitor_def() prints only registers from monitor_defs.
> However there is a lot of BOOK3S SPRs which are not in the list and
> cannot be printed.
>
> This makes use of the new get_monitor_def() callback and prints all
> registered SPRs and fails on unregistered ones proving the user

ping?

and also s/proving the user/providing the user with/ (not sure what I 
thought about when I wrote it)? :)




> information on what is actually supported in the running CPU.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> Changes:
> v3:
> * removed the check for endptr as strtoul() always initializes it
> * check if there is any number after r/f/sr and fail if none
> * added tolower()/strncasecmp() to support both r/f/sr and R/F/SR
>
> v2:
> * handles r**, f**, sr** if their numbers  were parsed completely and correctly
> * added "cr" as synonym of "ccr"
> ---
>   monitor.c                   | 215 +-------------------------------------------
>   target-ppc/cpu-qom.h        |   2 +
>   target-ppc/translate.c      |  80 +++++++++++++++++
>   target-ppc/translate_init.c |   1 +
>   4 files changed, 84 insertions(+), 214 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index bdfcacc..07ca678 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2916,51 +2916,6 @@ static target_long monitor_get_pc (const struct MonitorDef *md, int val)
>   }
>   #endif
>
> -#if defined(TARGET_PPC)
> -static target_long monitor_get_ccr (const struct MonitorDef *md, int val)
> -{
> -    CPUArchState *env = mon_get_cpu_env();
> -    unsigned int u;
> -    int i;
> -
> -    u = 0;
> -    for (i = 0; i < 8; i++)
> -        u |= env->crf[i] << (32 - (4 * (i + 1)));
> -
> -    return u;
> -}
> -
> -static target_long monitor_get_msr (const struct MonitorDef *md, int val)
> -{
> -    CPUArchState *env = mon_get_cpu_env();
> -    return env->msr;
> -}
> -
> -static target_long monitor_get_xer (const struct MonitorDef *md, int val)
> -{
> -    CPUArchState *env = mon_get_cpu_env();
> -    return env->xer;
> -}
> -
> -static target_long monitor_get_decr (const struct MonitorDef *md, int val)
> -{
> -    CPUArchState *env = mon_get_cpu_env();
> -    return cpu_ppc_load_decr(env);
> -}
> -
> -static target_long monitor_get_tbu (const struct MonitorDef *md, int val)
> -{
> -    CPUArchState *env = mon_get_cpu_env();
> -    return cpu_ppc_load_tbu(env);
> -}
> -
> -static target_long monitor_get_tbl (const struct MonitorDef *md, int val)
> -{
> -    CPUArchState *env = mon_get_cpu_env();
> -    return cpu_ppc_load_tbl(env);
> -}
> -#endif
> -
>   #if defined(TARGET_SPARC)
>   #ifndef TARGET_SPARC64
>   static target_long monitor_get_psr (const struct MonitorDef *md, int val)
> @@ -3014,175 +2969,7 @@ static const MonitorDef monitor_defs[] = {
>       SEG("gs", R_GS)
>       { "pc", 0, monitor_get_pc, },
>   #elif defined(TARGET_PPC)
> -    /* General purpose registers */
> -    { "r0", offsetof(CPUPPCState, gpr[0]) },
> -    { "r1", offsetof(CPUPPCState, gpr[1]) },
> -    { "r2", offsetof(CPUPPCState, gpr[2]) },
> -    { "r3", offsetof(CPUPPCState, gpr[3]) },
> -    { "r4", offsetof(CPUPPCState, gpr[4]) },
> -    { "r5", offsetof(CPUPPCState, gpr[5]) },
> -    { "r6", offsetof(CPUPPCState, gpr[6]) },
> -    { "r7", offsetof(CPUPPCState, gpr[7]) },
> -    { "r8", offsetof(CPUPPCState, gpr[8]) },
> -    { "r9", offsetof(CPUPPCState, gpr[9]) },
> -    { "r10", offsetof(CPUPPCState, gpr[10]) },
> -    { "r11", offsetof(CPUPPCState, gpr[11]) },
> -    { "r12", offsetof(CPUPPCState, gpr[12]) },
> -    { "r13", offsetof(CPUPPCState, gpr[13]) },
> -    { "r14", offsetof(CPUPPCState, gpr[14]) },
> -    { "r15", offsetof(CPUPPCState, gpr[15]) },
> -    { "r16", offsetof(CPUPPCState, gpr[16]) },
> -    { "r17", offsetof(CPUPPCState, gpr[17]) },
> -    { "r18", offsetof(CPUPPCState, gpr[18]) },
> -    { "r19", offsetof(CPUPPCState, gpr[19]) },
> -    { "r20", offsetof(CPUPPCState, gpr[20]) },
> -    { "r21", offsetof(CPUPPCState, gpr[21]) },
> -    { "r22", offsetof(CPUPPCState, gpr[22]) },
> -    { "r23", offsetof(CPUPPCState, gpr[23]) },
> -    { "r24", offsetof(CPUPPCState, gpr[24]) },
> -    { "r25", offsetof(CPUPPCState, gpr[25]) },
> -    { "r26", offsetof(CPUPPCState, gpr[26]) },
> -    { "r27", offsetof(CPUPPCState, gpr[27]) },
> -    { "r28", offsetof(CPUPPCState, gpr[28]) },
> -    { "r29", offsetof(CPUPPCState, gpr[29]) },
> -    { "r30", offsetof(CPUPPCState, gpr[30]) },
> -    { "r31", offsetof(CPUPPCState, gpr[31]) },
> -    /* Floating point registers */
> -    { "f0", offsetof(CPUPPCState, fpr[0]) },
> -    { "f1", offsetof(CPUPPCState, fpr[1]) },
> -    { "f2", offsetof(CPUPPCState, fpr[2]) },
> -    { "f3", offsetof(CPUPPCState, fpr[3]) },
> -    { "f4", offsetof(CPUPPCState, fpr[4]) },
> -    { "f5", offsetof(CPUPPCState, fpr[5]) },
> -    { "f6", offsetof(CPUPPCState, fpr[6]) },
> -    { "f7", offsetof(CPUPPCState, fpr[7]) },
> -    { "f8", offsetof(CPUPPCState, fpr[8]) },
> -    { "f9", offsetof(CPUPPCState, fpr[9]) },
> -    { "f10", offsetof(CPUPPCState, fpr[10]) },
> -    { "f11", offsetof(CPUPPCState, fpr[11]) },
> -    { "f12", offsetof(CPUPPCState, fpr[12]) },
> -    { "f13", offsetof(CPUPPCState, fpr[13]) },
> -    { "f14", offsetof(CPUPPCState, fpr[14]) },
> -    { "f15", offsetof(CPUPPCState, fpr[15]) },
> -    { "f16", offsetof(CPUPPCState, fpr[16]) },
> -    { "f17", offsetof(CPUPPCState, fpr[17]) },
> -    { "f18", offsetof(CPUPPCState, fpr[18]) },
> -    { "f19", offsetof(CPUPPCState, fpr[19]) },
> -    { "f20", offsetof(CPUPPCState, fpr[20]) },
> -    { "f21", offsetof(CPUPPCState, fpr[21]) },
> -    { "f22", offsetof(CPUPPCState, fpr[22]) },
> -    { "f23", offsetof(CPUPPCState, fpr[23]) },
> -    { "f24", offsetof(CPUPPCState, fpr[24]) },
> -    { "f25", offsetof(CPUPPCState, fpr[25]) },
> -    { "f26", offsetof(CPUPPCState, fpr[26]) },
> -    { "f27", offsetof(CPUPPCState, fpr[27]) },
> -    { "f28", offsetof(CPUPPCState, fpr[28]) },
> -    { "f29", offsetof(CPUPPCState, fpr[29]) },
> -    { "f30", offsetof(CPUPPCState, fpr[30]) },
> -    { "f31", offsetof(CPUPPCState, fpr[31]) },
> -    { "fpscr", offsetof(CPUPPCState, fpscr) },
> -    /* Next instruction pointer */
> -    { "nip|pc", offsetof(CPUPPCState, nip) },
> -    { "lr", offsetof(CPUPPCState, lr) },
> -    { "ctr", offsetof(CPUPPCState, ctr) },
> -    { "decr", 0, &monitor_get_decr, },
> -    { "ccr", 0, &monitor_get_ccr, },
> -    /* Machine state register */
> -    { "msr", 0, &monitor_get_msr, },
> -    { "xer", 0, &monitor_get_xer, },
> -    { "tbu", 0, &monitor_get_tbu, },
> -    { "tbl", 0, &monitor_get_tbl, },
> -    /* Segment registers */
> -    { "sdr1", offsetof(CPUPPCState, spr[SPR_SDR1]) },
> -    { "sr0", offsetof(CPUPPCState, sr[0]) },
> -    { "sr1", offsetof(CPUPPCState, sr[1]) },
> -    { "sr2", offsetof(CPUPPCState, sr[2]) },
> -    { "sr3", offsetof(CPUPPCState, sr[3]) },
> -    { "sr4", offsetof(CPUPPCState, sr[4]) },
> -    { "sr5", offsetof(CPUPPCState, sr[5]) },
> -    { "sr6", offsetof(CPUPPCState, sr[6]) },
> -    { "sr7", offsetof(CPUPPCState, sr[7]) },
> -    { "sr8", offsetof(CPUPPCState, sr[8]) },
> -    { "sr9", offsetof(CPUPPCState, sr[9]) },
> -    { "sr10", offsetof(CPUPPCState, sr[10]) },
> -    { "sr11", offsetof(CPUPPCState, sr[11]) },
> -    { "sr12", offsetof(CPUPPCState, sr[12]) },
> -    { "sr13", offsetof(CPUPPCState, sr[13]) },
> -    { "sr14", offsetof(CPUPPCState, sr[14]) },
> -    { "sr15", offsetof(CPUPPCState, sr[15]) },
> -    /* Too lazy to put BATs... */
> -    { "pvr", offsetof(CPUPPCState, spr[SPR_PVR]) },
> -
> -    { "srr0", offsetof(CPUPPCState, spr[SPR_SRR0]) },
> -    { "srr1", offsetof(CPUPPCState, spr[SPR_SRR1]) },
> -    { "dar", offsetof(CPUPPCState, spr[SPR_DAR]) },
> -    { "dsisr", offsetof(CPUPPCState, spr[SPR_DSISR]) },
> -    { "cfar", offsetof(CPUPPCState, spr[SPR_CFAR]) },
> -    { "sprg0", offsetof(CPUPPCState, spr[SPR_SPRG0]) },
> -    { "sprg1", offsetof(CPUPPCState, spr[SPR_SPRG1]) },
> -    { "sprg2", offsetof(CPUPPCState, spr[SPR_SPRG2]) },
> -    { "sprg3", offsetof(CPUPPCState, spr[SPR_SPRG3]) },
> -    { "sprg4", offsetof(CPUPPCState, spr[SPR_SPRG4]) },
> -    { "sprg5", offsetof(CPUPPCState, spr[SPR_SPRG5]) },
> -    { "sprg6", offsetof(CPUPPCState, spr[SPR_SPRG6]) },
> -    { "sprg7", offsetof(CPUPPCState, spr[SPR_SPRG7]) },
> -    { "pid", offsetof(CPUPPCState, spr[SPR_BOOKE_PID]) },
> -    { "csrr0", offsetof(CPUPPCState, spr[SPR_BOOKE_CSRR0]) },
> -    { "csrr1", offsetof(CPUPPCState, spr[SPR_BOOKE_CSRR1]) },
> -    { "esr", offsetof(CPUPPCState, spr[SPR_BOOKE_ESR]) },
> -    { "dear", offsetof(CPUPPCState, spr[SPR_BOOKE_DEAR]) },
> -    { "mcsr", offsetof(CPUPPCState, spr[SPR_BOOKE_MCSR]) },
> -    { "tsr", offsetof(CPUPPCState, spr[SPR_BOOKE_TSR]) },
> -    { "tcr", offsetof(CPUPPCState, spr[SPR_BOOKE_TCR]) },
> -    { "vrsave", offsetof(CPUPPCState, spr[SPR_VRSAVE]) },
> -    { "pir", offsetof(CPUPPCState, spr[SPR_BOOKE_PIR]) },
> -    { "mcsrr0", offsetof(CPUPPCState, spr[SPR_BOOKE_MCSRR0]) },
> -    { "mcsrr1", offsetof(CPUPPCState, spr[SPR_BOOKE_MCSRR1]) },
> -    { "decar", offsetof(CPUPPCState, spr[SPR_BOOKE_DECAR]) },
> -    { "ivpr", offsetof(CPUPPCState, spr[SPR_BOOKE_IVPR]) },
> -    { "epcr", offsetof(CPUPPCState, spr[SPR_BOOKE_EPCR]) },
> -    { "sprg8", offsetof(CPUPPCState, spr[SPR_BOOKE_SPRG8]) },
> -    { "ivor0", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR0]) },
> -    { "ivor1", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR1]) },
> -    { "ivor2", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR2]) },
> -    { "ivor3", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR3]) },
> -    { "ivor4", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR4]) },
> -    { "ivor5", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR5]) },
> -    { "ivor6", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR6]) },
> -    { "ivor7", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR7]) },
> -    { "ivor8", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR8]) },
> -    { "ivor9", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR9]) },
> -    { "ivor10", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR10]) },
> -    { "ivor11", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR11]) },
> -    { "ivor12", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR12]) },
> -    { "ivor13", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR13]) },
> -    { "ivor14", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR14]) },
> -    { "ivor15", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR15]) },
> -    { "ivor32", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR32]) },
> -    { "ivor33", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR33]) },
> -    { "ivor34", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR34]) },
> -    { "ivor35", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR35]) },
> -    { "ivor36", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR36]) },
> -    { "ivor37", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR37]) },
> -    { "mas0", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS0]) },
> -    { "mas1", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS1]) },
> -    { "mas2", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS2]) },
> -    { "mas3", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS3]) },
> -    { "mas4", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS4]) },
> -    { "mas6", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS6]) },
> -    { "mas7", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS7]) },
> -    { "mmucfg", offsetof(CPUPPCState, spr[SPR_MMUCFG]) },
> -    { "tlb0cfg", offsetof(CPUPPCState, spr[SPR_BOOKE_TLB0CFG]) },
> -    { "tlb1cfg", offsetof(CPUPPCState, spr[SPR_BOOKE_TLB1CFG]) },
> -    { "epr", offsetof(CPUPPCState, spr[SPR_BOOKE_EPR]) },
> -    { "eplc", offsetof(CPUPPCState, spr[SPR_BOOKE_EPLC]) },
> -    { "epsc", offsetof(CPUPPCState, spr[SPR_BOOKE_EPSC]) },
> -    { "svr", offsetof(CPUPPCState, spr[SPR_E500_SVR]) },
> -    { "mcar", offsetof(CPUPPCState, spr[SPR_Exxx_MCAR]) },
> -    { "pid1", offsetof(CPUPPCState, spr[SPR_BOOKE_PID1]) },
> -    { "pid2", offsetof(CPUPPCState, spr[SPR_BOOKE_PID2]) },
> -    { "hid0", offsetof(CPUPPCState, spr[SPR_HID0]) },
> -
> +    /* Uses get_monitor_def() */
>   #elif defined(TARGET_SPARC)
>       { "g0", offsetof(CPUSPARCState, gregs[0]) },
>       { "g1", offsetof(CPUSPARCState, gregs[1]) },
> diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
> index 6967a80..bc20504 100644
> --- a/target-ppc/cpu-qom.h
> +++ b/target-ppc/cpu-qom.h
> @@ -118,6 +118,8 @@ void ppc_cpu_dump_state(CPUState *cpu, FILE *f, fprintf_function cpu_fprintf,
>                           int flags);
>   void ppc_cpu_dump_statistics(CPUState *cpu, FILE *f,
>                                fprintf_function cpu_fprintf, int flags);
> +int ppc_cpu_get_monitor_def(CPUState *cs, const char *name,
> +                            uint64_t *pval);
>   hwaddr ppc_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
>   int ppc_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
>   int ppc_cpu_gdb_read_register_apple(CPUState *cpu, uint8_t *buf, int reg);
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 84c5cea..2c6f772 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -11401,6 +11401,86 @@ void ppc_cpu_dump_statistics(CPUState *cs, FILE*f,
>   #endif
>   }
>
> +static bool ppc_cpu_get_reg(target_ulong *regs, const char *numstr, int maxnum,
> +                            uint64_t *pval)
> +{
> +    char *endptr = NULL;
> +    int regnum;
> +
> +    if (!*numstr) {
> +        return false;
> +    }
> +
> +    regnum = strtoul(numstr, &endptr, 10);
> +    if (*endptr || (regnum >= maxnum)) {
> +        return false;
> +    }
> +    *pval = regs[regnum];
> +
> +    return true;
> +}
> +
> +int ppc_cpu_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval)
> +{
> +    int i;
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    CPUPPCState *env = &cpu->env;
> +
> +#define MONREG(s, f) \
> +    if ((strcasecmp((s), name) == 0)) { \
> +        *pval = (f); \
> +        return 0; \
> +    }
> +    MONREG("pc", env->nip)
> +    MONREG("nip", env->nip)
> +    MONREG("lr", env->lr)
> +    MONREG("ctr", env->ctr)
> +    MONREG("xer", env->xer)
> +    MONREG("decr", cpu_ppc_load_decr(env))
> +    MONREG("msr",  env->msr)
> +    MONREG("tbu",  cpu_ppc_load_tbu(env))
> +    MONREG("tbl", cpu_ppc_load_tbl(env))
> +
> +    if ((strcasecmp("ccr", name) == 0) || (strcasecmp("cr", name) == 0)) {
> +        unsigned int u = 0;
> +
> +        for (i = 0; i < 8; i++)
> +            u |= env->crf[i] << (32 - (4 * (i + 1)));
> +
> +        return u;
> +    }
> +
> +    /* General purpose registers */
> +    if ((tolower(name[0]) == 'r') &&
> +        ppc_cpu_get_reg(env->gpr, name + 1, ARRAY_SIZE(env->gpr), pval)) {
> +        return 0;
> +    }
> +
> +    /* Floating point registers */
> +    if ((tolower(name[0]) == 'f') &&
> +        ppc_cpu_get_reg(env->fpr, name + 1, ARRAY_SIZE(env->fpr), pval)) {
> +        return 0;
> +    }
> +
> +    /* Segment registers */
> +    if ((strncasecmp(name, "sr", 2) == 0) &&
> +        ppc_cpu_get_reg(env->sr, name + 2, ARRAY_SIZE(env->sr), pval)) {
> +        return 0;
> +    }
> +
> +    /* Special purpose registers */
> +    for (i = 0; i < ARRAY_SIZE(env->spr_cb); ++i) {
> +        ppc_spr_t *spr = &env->spr_cb[i];
> +
> +        if (spr->name && (strcasecmp(name, spr->name) == 0)) {
> +            *pval = env->spr[i];
> +            return 0;
> +        }
> +    }
> +
> +    return -EINVAL;
> +}
> +
>   /*****************************************************************************/
>   static inline void gen_intermediate_code_internal(PowerPCCPU *cpu,
>                                                     TranslationBlock *tb,
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 16d7b16..038674a 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -9706,6 +9706,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
>       cc->cpu_exec_interrupt = ppc_cpu_exec_interrupt;
>       cc->dump_state = ppc_cpu_dump_state;
>       cc->dump_statistics = ppc_cpu_dump_statistics;
> +    cc->get_monitor_def = ppc_cpu_get_monitor_def;
>       cc->set_pc = ppc_cpu_set_pc;
>       cc->gdb_read_register = ppc_cpu_gdb_read_register;
>       cc->gdb_write_register = ppc_cpu_gdb_write_register;
>


-- 
Alexey

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH qemu v3] target-ppc: Define get_monitor_def
  2015-08-14  3:34         ` [Qemu-devel] [PATCH qemu v3] " Alexey Kardashevskiy
  2015-09-07  1:26           ` Alexey Kardashevskiy
@ 2015-09-23  3:40           ` David Gibson
  1 sibling, 0 replies; 13+ messages in thread
From: David Gibson @ 2015-09-23  3:40 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: qemu-ppc, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 2527 bytes --]

On Fri, Aug 14, 2015 at 01:34:30PM +1000, Alexey Kardashevskiy wrote:
> At the moment get_monitor_def() prints only registers from monitor_defs.
> However there is a lot of BOOK3S SPRs which are not in the list and
> cannot be printed.
> 
> This makes use of the new get_monitor_def() callback and prints all
> registered SPRs and fails on unregistered ones proving the user
> information on what is actually supported in the running CPU.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Sorry it's taken me so long to review this.

[snip]
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 84c5cea..2c6f772 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -11401,6 +11401,86 @@ void ppc_cpu_dump_statistics(CPUState *cs, FILE*f,
>  #endif
>  }
>  
> +static bool ppc_cpu_get_reg(target_ulong *regs, const char *numstr, int maxnum,
> +                            uint64_t *pval)
> +{
> +    char *endptr = NULL;
> +    int regnum;
> +
> +    if (!*numstr) {
> +        return false;
> +    }
> +
> +    regnum = strtoul(numstr, &endptr, 10);
> +    if (*endptr || (regnum >= maxnum)) {
> +        return false;
> +    }
> +    *pval = regs[regnum];
> +
> +    return true;
> +}
> +
> +int ppc_cpu_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval)
> +{
> +    int i;
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    CPUPPCState *env = &cpu->env;
> +
> +#define MONREG(s, f) \
> +    if ((strcasecmp((s), name) == 0)) { \
> +        *pval = (f); \
> +        return 0; \
> +    }
> +    MONREG("pc", env->nip)
> +    MONREG("nip", env->nip)
> +    MONREG("lr", env->lr)
> +    MONREG("ctr", env->ctr)
> +    MONREG("xer", env->xer)
> +    MONREG("decr", cpu_ppc_load_decr(env))
> +    MONREG("msr",  env->msr)
> +    MONREG("tbu",  cpu_ppc_load_tbu(env))
> +    MONREG("tbl", cpu_ppc_load_tbl(env))
> +
> +    if ((strcasecmp("ccr", name) == 0) || (strcasecmp("cr", name) == 0)) {
> +        unsigned int u = 0;
> +
> +        for (i = 0; i < 8; i++)
> +            u |= env->crf[i] << (32 - (4 * (i + 1)));
> +
> +        return u;

The other branches here set *pval and return 0, this one is returning
the register contents directly.  Is that a bug?

Apart from that the patch looks good.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2015-09-23  5:19 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-06  5:25 [Qemu-devel] [PATCH qemu 0/2] monitor/ppc: Print correct SPRs Alexey Kardashevskiy
2015-08-06  5:25 ` [Qemu-devel] [PATCH qemu 1/2] monitor: Add CPU class callback to read registers for monitor Alexey Kardashevskiy
2015-08-12  1:12   ` David Gibson
2015-08-06  5:25 ` [Qemu-devel] [PATCH qemu 2/2] target-ppc: Define get_monitor_def Alexey Kardashevskiy
2015-08-06  6:33   ` Thomas Huth
2015-08-06  7:00     ` Alexey Kardashevskiy
2015-08-06  7:07       ` Thomas Huth
2015-08-12  1:21   ` David Gibson
2015-08-13 15:52     ` [Qemu-devel] [PATCH qemu v2] " Alexey Kardashevskiy
2015-08-13 22:39       ` David Gibson
2015-08-14  3:34         ` [Qemu-devel] [PATCH qemu v3] " Alexey Kardashevskiy
2015-09-07  1:26           ` Alexey Kardashevskiy
2015-09-23  3:40           ` David Gibson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).