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 ?
> +{
> + 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
