diff mbox

target-mips: Output CP0.Config2-5 in the register dump

Message ID alpine.DEB.1.10.1411180311390.2881@tp.orcam.me.uk
State New
Headers show

Commit Message

Maciej W. Rozycki Nov. 18, 2014, 3:20 a.m. UTC
Include CP0.Config2 through CP0.Config5 registers in the register dump 
produced with the `info registers' monitor command.  Align vertically 
with the registers already output.

Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
---
Hi,

 This proved useful in debugging a CP0.Config3.ISAOnExc problem, fixed 
with the next patch.  Eventually I think we should produce a complete 
dump of CP0 registers, but for now this is an improvement.  Ideally we'd 
produce a suitable XML description so that GDB can access these 
registers natively, but for that XML description support will have to be 
implemented for bare-iron MIPS targets in GDB first.

 Meanwhile, please apply.

  Maciej

qemu-mips-info-regs-config.diff

Comments

Leon Alrae Dec. 2, 2014, 10:42 a.m. UTC | #1
On 18/11/2014 03:20, Maciej W. Rozycki wrote:
> @@ -19276,6 +19276,10 @@ void mips_cpu_dump_state(CPUState *cs, F
>                  env->CP0_Status, env->CP0_Cause, env->CP0_EPC);
>      cpu_fprintf(f, "    Config0 0x%08x Config1 0x%08x LLAddr 0x" TARGET_FMT_lx "\n",
>                  env->CP0_Config0, env->CP0_Config1, env->lladdr);
> +    cpu_fprintf(f, "    Config2 0x%08x Config3 0x%08x\n",
> +                env->CP0_Config2, env->CP0_Config3);
> +    cpu_fprintf(f, "    Config4 0x%08x Config5 0x%08x\n",
> +                env->CP0_Config4, env->CP0_Config5);

Wouldn't it be better to order these registers for example by CP0
Register and Select number rather than putting them at the end? I think
it doesn't matter at the moment as there are just few registers printed
out, but once we start adding more then probably we would like to avoid
having them placed arbitrarily in the output.

Regards,
Leon
Maciej W. Rozycki Dec. 2, 2014, 11:37 a.m. UTC | #2
On Tue, 2 Dec 2014, Leon Alrae wrote:

> > @@ -19276,6 +19276,10 @@ void mips_cpu_dump_state(CPUState *cs, F
> >                  env->CP0_Status, env->CP0_Cause, env->CP0_EPC);
> >      cpu_fprintf(f, "    Config0 0x%08x Config1 0x%08x LLAddr 0x" TARGET_FMT_lx "\n",
> >                  env->CP0_Config0, env->CP0_Config1, env->lladdr);
> > +    cpu_fprintf(f, "    Config2 0x%08x Config3 0x%08x\n",
> > +                env->CP0_Config2, env->CP0_Config3);
> > +    cpu_fprintf(f, "    Config4 0x%08x Config5 0x%08x\n",
> > +                env->CP0_Config4, env->CP0_Config5);
> 
> Wouldn't it be better to order these registers for example by CP0
> Register and Select number rather than putting them at the end? I think
> it doesn't matter at the moment as there are just few registers printed
> out, but once we start adding more then probably we would like to avoid
> having them placed arbitrarily in the output.

 I placed the registers such that output renders in a well-formatted 
readable way.  Please note that we print 64-bit registers at the right 
side and I avoided intermixing 32-bit registers with 64-bit registers.

 I agree we might reformat output according to CP0 Register and Select 
numbers as for example our (Mentor's) GDB does in `info registers' (the 
change to dump all available CP0 registers in GDB hasn't made its way 
upstream yet, partially due to the XML register description issue I 
mentioned previously).  Hovewer we need to think how to format output in a 
readable way as the receiver will most often be a human being.

 I think deciding on a uniform register width, i.e. the native machine 
word size of the CPU, might help here, after all in the execution stream 
you can access them with DMFC0 on 64-bit processors and get the expected 
result (with "don't-care's" in the upper 32 bits).  The drawback is more 
screen space is used for data which can make it all harder to comprehend.  
For example I find the output of `info registers' produced by GDB on a 
64-bit target more difficult to process in my head than the corresponding 
output on a 32-bit target.  Maybe we can leave the upper 32 bits padded 
with spaces rather than zeros though.

  Maciej
diff mbox

Patch

Index: qemu-git-trunk/target-mips/translate.c
===================================================================
--- qemu-git-trunk.orig/target-mips/translate.c	2014-11-17 04:04:36.000000000 +0000
+++ qemu-git-trunk/target-mips/translate.c	2014-11-17 04:12:25.137964436 +0000
@@ -19276,6 +19276,10 @@  void mips_cpu_dump_state(CPUState *cs, F
                 env->CP0_Status, env->CP0_Cause, env->CP0_EPC);
     cpu_fprintf(f, "    Config0 0x%08x Config1 0x%08x LLAddr 0x" TARGET_FMT_lx "\n",
                 env->CP0_Config0, env->CP0_Config1, env->lladdr);
+    cpu_fprintf(f, "    Config2 0x%08x Config3 0x%08x\n",
+                env->CP0_Config2, env->CP0_Config3);
+    cpu_fprintf(f, "    Config4 0x%08x Config5 0x%08x\n",
+                env->CP0_Config4, env->CP0_Config5);
     if (env->hflags & MIPS_HFLAG_FPU)
         fpu_dump_state(env, f, cpu_fprintf, flags);
 #if defined(TARGET_MIPS64) && defined(MIPS_DEBUG_SIGN_EXTENSIONS)