[PATCH] RISC-V: Correct printing of MSTATUS and MISA.

[PATCH] RISC-V: Correct printing of MSTATUS and MISA.

[Jim Wilson]

With my proposed qemu patches to make the RISC-V qemu gdbstub support work,
I noticed that a 64-bit MISA was printing wrong.  "info registers misa" would
give me RV16ACDFIMSU.  The problem here is that the MXL field is always in the
upper two bits of the register, but the code assumes it is in bits 31 and 30,
which is only correct for a 32-bit target.  I copied code from the MSTATUS
support to fix this, and also noticed a bug in it.  register_size returns
size in bytes, so we have to multiply by 8 to get size in bits.

Tested by hand with qemu with my gdbstub patches applied, for both 32-bit and
64-bit targets, printing MISA and verifying I get the right result back.  Also
testing with a riscv64-linux gdb make check, with no regressions, though that
only proves I didn't break anything, since a user mode gdb can't read machine
registers.

OK?

Jim

	gdb/
	* riscv-tdep.c (riscv_print_one_register_info): For MSTATUS, add
	comment for SD field, and correct xlen calculation.  For MISA, add
	comment for MXL field, add call to register_size, and correct base
	calculation.
---
 gdb/riscv-tdep.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 5ddec70307..41b6de1c4e 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -701,8 +701,10 @@ riscv_print_one_register_info (struct gdbarch *gdbarch,
 	      int size = register_size (gdbarch, regnum);
 	      unsigned xlen;
 
+	      /* The SD field is always in the upper bit of MSTATUS, regardless
+		 of the number of bits in MSTATUS.  */
 	      d = value_as_long (val);
-	      xlen = size * 4;
+	      xlen = size * 8;
 	      fprintf_filtered (file,
 				"\tSD:%X VM:%02X MXR:%X PUM:%X MPRV:%X XS:%X "
 				"FS:%X MPP:%x HPP:%X SPP:%X MPIE:%X HPIE:%X "
@@ -731,9 +733,13 @@ riscv_print_one_register_info (struct gdbarch *gdbarch,
 	      int base;
 	      unsigned xlen, i;
 	      LONGEST d;
+	      int size = register_size (gdbarch, regnum);
 
+	      /* The MXL field is always in the upper two bits of MISA,
+		 regardless of the number of bits in MISA.  Mask out other
+		 bits to ensure we have a positive value.  */
 	      d = value_as_long (val);
-	      base = d >> 30;
+	      base = (d >> ((size * 8) - 2)) & 0x3;
 	      xlen = 16;
 
 	      for (; base > 0; base--)
-- 
2.17.1

Re: [PATCH] RISC-V: Correct printing of MSTATUS and MISA.

[Andrew Burgess]

* Jim Wilson <jimw@sifive.com> [2018-12-12 18:49:43 -0800]:

> With my proposed qemu patches to make the RISC-V qemu gdbstub support work,
> I noticed that a 64-bit MISA was printing wrong.  "info registers misa" would
> give me RV16ACDFIMSU.  The problem here is that the MXL field is always in the
> upper two bits of the register, but the code assumes it is in bits 31 and 30,
> which is only correct for a 32-bit target.  I copied code from the MSTATUS
> support to fix this, and also noticed a bug in it.  register_size returns
> size in bytes, so we have to multiply by 8 to get size in bits.
> 
> Tested by hand with qemu with my gdbstub patches applied, for both 32-bit and
> 64-bit targets, printing MISA and verifying I get the right result back.  Also
> testing with a riscv64-linux gdb make check, with no regressions, though that
> only proves I didn't break anything, since a user mode gdb can't read machine
> registers.
> 
> OK?
> 
> Jim
> 
> 	gdb/
> 	* riscv-tdep.c (riscv_print_one_register_info): For MSTATUS, add
> 	comment for SD field, and correct xlen calculation.  For MISA, add
> 	comment for MXL field, add call to register_size, and correct base
> 	calculation.

Looks good to me.

Thanks,
Andrew


> ---
>  gdb/riscv-tdep.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> index 5ddec70307..41b6de1c4e 100644
> --- a/gdb/riscv-tdep.c
> +++ b/gdb/riscv-tdep.c
> @@ -701,8 +701,10 @@ riscv_print_one_register_info (struct gdbarch *gdbarch,
>  	      int size = register_size (gdbarch, regnum);
>  	      unsigned xlen;
>  
> +	      /* The SD field is always in the upper bit of MSTATUS, regardless
> +		 of the number of bits in MSTATUS.  */
>  	      d = value_as_long (val);
> -	      xlen = size * 4;
> +	      xlen = size * 8;
>  	      fprintf_filtered (file,
>  				"\tSD:%X VM:%02X MXR:%X PUM:%X MPRV:%X XS:%X "
>  				"FS:%X MPP:%x HPP:%X SPP:%X MPIE:%X HPIE:%X "
> @@ -731,9 +733,13 @@ riscv_print_one_register_info (struct gdbarch *gdbarch,
>  	      int base;
>  	      unsigned xlen, i;
>  	      LONGEST d;
> +	      int size = register_size (gdbarch, regnum);
>  
> +	      /* The MXL field is always in the upper two bits of MISA,
> +		 regardless of the number of bits in MISA.  Mask out other
> +		 bits to ensure we have a positive value.  */
>  	      d = value_as_long (val);
> -	      base = d >> 30;
> +	      base = (d >> ((size * 8) - 2)) & 0x3;
>  	      xlen = 16;
>  
>  	      for (; base > 0; base--)
> -- 
> 2.17.1
> 

Re: [PATCH] RISC-V: Correct printing of MSTATUS and MISA.

[Jim Wilson]

On Thu, Dec 13, 2018 at 1:53 AM Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
> Looks good to me.

Thanks.  Committed.

Jim