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 <a...@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
