Re: [PATCH v11 00/16] target/riscv: Update QEmu for Zb[abcs] 1.0.0

[Jim Wilson]

On Mon, Sep 27, 2021 at 1:01 PM Vineet Gupta  wrote:

> So I obviously forgot to get the equivalent binutils branch, but the
> only rvb branch on sifive fork feels dated
>
>
> https://github.com/riscv-collab/riscv-binutils-gdb/tree/riscv-binutils-2.35-rvb


That is the right branch to use with the gcc that you are using.  This
stuff hasn't been actively maintained so we have old gcc and binutils
release versions.

We are in the process of putting stuff upstream now.

Jim

Re: [PATCH v5 0/4] target-riscv: support vector extension part 1

[Jim Wilson]

On Wed, Feb 26, 2020 at 2:36 PM Alistair Francis  wrote:
> On Wed, Feb 26, 2020 at 12:09 PM Jim Wilson  wrote:
> > If this rvv 0.7.1 implementation is considered a temporary solution,
> > maybe we can just remove all of this work when the official rvv spec if
> > available?  But presumably it is better if we can have both this
>
> That is generally the plan. When the final spec comes out this will be
> updated and we will only support that.

This solves my problem, but maybe creates one for Alibaba.  They have
apparently fabbed a chip using the 0.7.1 draft of the vector spec
proposal.  So for qemu to properly support their asic, the 0.7.1 draft
support will have to be retained.  But I think SiFive and everyone
else is waiting for the official spec before building asics with
vector support.  If Alibaba will update their processor as the spec
evolves, then maybe this isn't a problem for them.

Jim

Re: [PATCH v5 4/4] target/riscv: add vector configure instruction

[Jim Wilson]

On 2/21/20 1:45 AM, LIU Zhiwei wrote:

+/* Using x0 as the rs1 register specifier, encodes an infinite AVL */
+if (a->rs1 == 0) {
+/* As the mask is at least one bit, RV_VLEN_MAX is >= VLMAX */
+s1 = tcg_const_tl(RV_VLEN_MAX);


This is wrong for the current draft of the vector spec.  x0 now means 
don't change VL.  So this needs to be version specific.


Jim

Re: [PATCH v5 3/4] target/riscv: support vector extension csr

[Jim Wilson]

On 2/21/20 1:45 AM, LIU Zhiwei wrote:

+/* Vector Fixed-Point round model */
+#define FSR_VXRM_SHIFT  9
+#define FSR_VXRM(0x3 << FSR_VXRM_SHIFT)
+
+/* Vector Fixed-Point saturation flag */
+#define FSR_VXSAT_SHIFT 8
+#define FSR_VXSAT   (0x1 << FSR_VXSAT_SHIFT)


These bits have been moved into the new VCSR register.  And the offsets 
are wrong because these bits are no longer above the FP status bits in 
the FCSR.  So this needs to be rvv 0.7.1 specific.



+/* User Vector CSRs */
+#define CSR_VSTART  0x008
+#define CSR_VXSAT   0x009
+#define CSR_VXRM0x00a
+#define CSR_VL  0xc20
+#define CSR_VTYPE   0xc21


This is missing two new CSRs, VCSR and VLENB.


+/* loose check condition for fcsr in vector extension */
+if ((csrno == CSR_FCSR) && (env->misa & RVV)) {
+return 0;
+}


This is wrong for the current spec, because the vector status bits 
aren't in the FCSR anymore.  So this also needs to be rvv 0.7.1 specific.


Jim

Re: [PATCH v5 0/4] target-riscv: support vector extension part 1

[Jim Wilson]

On 2/21/20 1:45 AM, LIU Zhiwei wrote:

This is the first part of v5 patchset. The changelog of v5 is only coverd
the part1.

Features:
   * support specification riscv-v-spec-0.7.1.


I'm still concerned about versioning issues.  This implements an 
unofficial draft of the proposed RISC-V vector extension.  This draft is 
not compatible with the current draft, and will be even less compatible 
with the final official version of the vector spec.


The patch adds a version which is good, but there is only one check when 
qemu starts.  Probably something like 25% of these patches will be wrong 
for the official vector extension.  How are we going to handle this when 
someone submits patches for the official support?  It would be better if 
everything in these patches were conditional on the version number.  It 
might also be better if we stopped calling this the 'v' extension and 
maybe used another name like Xrvv071 to make it clear that it is an 
unofficial draft of the proposed vector spec.  Or maybe be we can use 
v0p7 but that isn't an officially supported extension name.


If this rvv 0.7.1 implementation is considered a temporary solution, 
maybe we can just remove all of this work when the official rvv spec if 
available?  But presumably it is better if we can have both this 
implementation and the official one, which means everything needs to be 
conditional or tied to an Xsomething extension name instead of the V 
extension name.


Jim

Re: [PATCH v3 3/4] RISC-V: support vector extension csr

[Jim Wilson]

On 1/2/20 7:33 PM, LIU Zhiwei wrote:

Until v0.7.1 specification, vector status is still not defined for
mstatus.


The v0.8 spec does define a VS bit in mstatus.


@@ -107,11 +112,6 @@ static int pmp(CPURISCVState *env, int csrno)
  /* User Floating-Point CSRs */
  static int read_fflags(CPURISCVState *env, int csrno, target_ulong *val)
  {
-#if !defined(CONFIG_USER_ONLY)
-if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
-return -1;
-}
-#endif
  *val = riscv_cpu_get_fflags(env);
  return 0;
  }


This allows reads of fflags when it doesn't exist, and hence does not 
make much sense.  Instead of removing the code, you should add a check 
for the vector extension, since the vector extension requires that fcsr 
exist even if the base architecture doesn't include FP support.  Ideally 
this should use the VS bit, but if you don't have it then you can just 
check to see if the vector extension was enabled as a command line option.


While the vector spec says that fcsr must exist, it doesn't specify that 
the FP fields in fcsr are necessarily readable or writable when there is 
no FP.  It also doesn't specify whether the other FP related shadows of 
fcsr exist, like fflags.  This appears to have been left unspecified.  I 
don't think that you should be making fflags reads and writes work for a 
target with vector but without float.  I think it would make more sense 
to have fcsr behave 3 different ways depending on whether we have only 
F, only V, or both F and V.  And then we can support reads and writes of 
only the valid fields.


Jim

Re: [PATCH v3 2/4] RISC-V: configure and turn on vector extension from command line

[Jim Wilson]

On 1/2/20 7:33 PM, LIU Zhiwei wrote:

+if (cpu->cfg.vlen > RV_VLEN_MAX) {
+error_setg(errp,
+   "Vector extension VLEN must <= %d", RV_VLEN_MAX);
+return;


There is no architectural maximum for VLEN.  This is simply an 
implementation choice so you can use static arrays instead of malloc.  I 
think this error should be reworded to something like "Vector extension 
implementation only supports VLEN <= %d."


The other errors here are for architecture requirements and are OK.

Jim

Re: [PATCH v2] target/riscv: Expose "priv" register for GDB

[Jim Wilson]

On Tue, Oct 8, 2019 at 2:00 AM Bin Meng  wrote:
> My gdb does not list "priv" register after applying this patch.

I didn't try the patch, I didn't have time for that.  I would expect
priv to be in the "info registers" output if you are adding it to the
cpu register set.  Shrug.  Anyways, defining priv as a virtual
register is the right way to do this, as it isn't a cpu register, and
gdb already has support for virtual registers like priv.

Jim

Re: [PATCH v2] target/riscv: Expose "priv" register for GDB

[Jim Wilson]

On 10/4/19 8:16 AM, Jonathan Behrens wrote:

diff --git a/gdb-xml/riscv-32bit-cpu.xml b/gdb-xml/riscv-32bit-cpu.xml
index 0d07aaec85..d6d76aafd8 100644
--- a/gdb-xml/riscv-32bit-cpu.xml
+++ b/gdb-xml/riscv-32bit-cpu.xml
@@ -44,4 +44,5 @@



+  
  


Adding this to the cpu register set means that the gdb "info registers" 
command will now list a value for the mysterious undocumented "priv" 
register.  That is likely to result in user confusion, and a few gdb bug 
reports.


Gdb incidentally already has support for a virtual priv register.  From 
gdb/riscv-tdep.c:


static const struct riscv_register_feature riscv_virtual_feature =
{
 "org.gnu.gdb.riscv.virtual",
 {
   { RISCV_PRIV_REGNUM, { "priv" }, false }
 }
};

So the correct way to fix this is to add a 
gdb-xml/riscv-32bit-virtual.xml file, along with code to handle this new 
xml file and the registers in it.  Likewise for the 64-bit support.


The main advantage of doing things this way is that only people that 
care about the priv register will see it, and this will interoperate 
with other RISC-V debuggers and targets (if any) that already have 
virtual priv register support.


Jim

Re: [Qemu-devel] [PATCH v3] RISC-V: Select FPU gdb xml file based on the supported extensions

[Jim Wilson]

On 8/21/19 9:28 AM, Georg Kotheimer wrote:

The size of the FPU registers depends solely on the floating point
extensions supported by the target architecture.
However, in the previous implementation the floating point register
size was derived from whether the target architecture is 32-bit or
64-bit.

To allow RVF without RVD, changes to riscv_gdb_get_fpu() and
riscv_gdb_set_fpu() were necessary.


Reviewed-by: Jim Wilson 

Jim

Re: [Qemu-devel] [PATCH v2] RISC-V: Select FPU gdb xml file based on the supported extensions

[Jim Wilson]

On 8/20/19 7:39 AM, Georg Kotheimer wrote:

The size of the FPU registers depends solely on the floating point
extensions supported by the target architecture.
However, in the previous implementation the floating point register
size was derived from whether the target architecture is 32-bit or
64-bit.

To allow RVF without RVD, changes to riscv_gdb_get_fpu() and
riscv_gdb_set_fpu() were necessary.

In addition fflags, frm and fcsr were removed from
riscv-XXbit-csr.xml, as the floating point csr registers are only
available, if a FPU is present.


The current XML files were identical to the XML files in gdb when 
implemented.  This seems to be existing practice, as this is true of all 
of the other targets I looked at when I implemented this.  Also, the 
file names are the same with a / replaced with a -.  These files are in 
the gdb/features/riscv dir in a gdb source tree.  It would be a shame to 
break this.  I'm not sure if they are still identical.  The gdb copies 
might have been updated since then, and may need to be copied into qemu 
to update qemu, but we don't have a dedicated gdb/qemu maintainer to do 
this.


I don't see a need to remove the fp csr's from the csr list.  There are 
other extension dependent CSRs, like hypervisor ones. I think it makes 
more sense for the csr list to contain all of the csrs, and then disable 
access to them if that extension is not enabled.  If there is a good 
reason to require changes to the csr XML files, then it probably should 
be discussed with the gdb developers too, so that the gdb and qemu 
copies of the files remain consistent.


Fixing the rvf/rvd 32/64-bit support is good.  That is a bug in my 
original implementation.


Jim

Re: [Qemu-devel] [PATCH v16 1/5] linux-user: Add support for translation of statx() syscall

[Jim Wilson]

On Fri, Jun 28, 2019 at 5:53 PM Aleksandar Markovic
 wrote:
> This patch went trough several transformations in last few days, and I am a 
> little worried that we forgot the primary reasons/scenarios why want it in 
> the first place. In that light, may I ask you to recheck this latest version 
> of the patch, v16, against your scenarios (you mentioned earlier you have two 
> significantly different flavors of your scenario, one with Ubuntu 16, and 
> another with Ubuntu 19)?

My use case is that I want 32-bit RISC-V user mode to work.  This
requires a riscv syscall list patch that Palmer Dabbelt added to his
patch set, and the statx patch that is part of your patch set.  The
statx strace support is not required for this use case, but should be
added for completeness as all of the other stat family functions have
strace support, so statx should too.  Since the statx strace patch
needs statx macros that old systems don't have, I test on Ubuntu 16
(no host statx) and Ubuntu 19 (with host statx).  On Ubuntu 19, statx
strace should be fully pretty printed.  On Ubuntu 16, qemu should
still build despite the missing macros, and statx strace should be
partially pretty printed because of the missing macros.

I removed the old patches, updated qemu, added the new patches,
rebuilt qemu, and reran the gcc testsuite for rv32/rv64 Ubuntu 16/19
and it still works as expected.  I also manually checked strace for
rv32/rv64 Ubuntu 16/19 and that also still works as expected.  So this
looks good to me.

I'm leaving on a trip tomorrow, and only taking one laptop with me, so
I won't have access to my Ubuntu 19 machine anymore.

Jim

Re: [Qemu-devel] [PATCH v10 3/3] linux-user: Add support for statx() syscall

[Jim Wilson]

On Tue, Jun 18, 2019 at 4:13 PM Aleksandar Markovic
 wrote:
> I am waiting on him to send a new version of the series. Meanwhile you can 
> send strace patch to the list, and I can even incude it in my series after 
> and together with Aleksandar's patch, if you don't object.

I submitted it the usual way, so it is on the mailing list now.  If
you want to include it with your patch series that is fine.
https://lists.nongnu.org/archive/html/qemu-devel/2019-06/msg04087.html

Jim

Re: [Qemu-devel] [PATCH] linux-user: Add strace support for statx.

[Jim Wilson]

On Tue, Jun 18, 2019 at 5:09 PM  wrote:
> === OUTPUT BEGIN ===
> ERROR: storage class should be at the beginning of the declaration
> #25: FILE: linux-user/strace.c:979:
> +UNUSED static struct flags statx_flags[] = {

It is complaining about UNUSED, which is a macro that expands to
attribute unused, but this is the exact same C syntax used in the rest
of the file.  I think this is a bug in the checkpatch script as this
looks like it should be allowed.  Or maybe there is an exception for
this one file?

Jim

[Qemu-devel] [PATCH] linux-user: Add strace support for statx.

[Jim Wilson]

All of the flags need to be conditional as old systems don't have statx
support.  Otherwise it works the same as other stat family syscalls.
This requires the pending patch to add statx support.

Tested on Ubuntu 16.04 (no host statx) and Ubuntu 19.04 (with host statx)
using a riscv32-linux toolchain.

Signed-off-by: Jim Wilson 
---
 linux-user/strace.c| 86 ++
 linux-user/strace.list |  3 ++
 2 files changed, 89 insertions(+)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 6f72a74..c80e93b 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -976,6 +976,76 @@ UNUSED static struct flags msg_flags[] = {
 FLAG_END,
 };
 
+UNUSED static struct flags statx_flags[] = {
+#ifdef AT_EMPTY_PATH
+FLAG_GENERIC(AT_EMPTY_PATH),
+#endif
+#ifdef AT_NO_AUTOMOUNT
+FLAG_GENERIC(AT_NO_AUTOMOUNT),
+#endif
+#ifdef AT_SYMLINK_NOFOLLOW
+FLAG_GENERIC(AT_SYMLINK_NOFOLLOW),
+#endif
+#ifdef AT_STATX_SYNC_AS_STAT
+FLAG_GENERIC(AT_STATX_SYNC_AS_STAT),
+#endif
+#ifdef AT_STATX_FORCE_SYNC
+FLAG_GENERIC(AT_STATX_FORCE_SYNC),
+#endif
+#ifdef AT_STATX_DONT_SYNC
+FLAG_GENERIC(AT_STATX_DONT_SYNC),
+#endif
+FLAG_END,
+};
+
+UNUSED static struct flags statx_mask[] = {
+/* This must come first, because it includes everything.  */
+#ifdef STATX_ALL
+FLAG_GENERIC(STATX_ALL),
+#endif
+/* This must come second; it includes everything except STATX_BTIME.  */
+#ifdef STATX_BASIC_STATS
+FLAG_GENERIC(STATX_BASIC_STATS),
+#endif
+#ifdef STATX_TYPE
+FLAG_GENERIC(STATX_TYPE),
+#endif
+#ifdef STATX_MODE
+FLAG_GENERIC(STATX_MODE),
+#endif
+#ifdef STATX_NLINK
+FLAG_GENERIC(STATX_NLINK),
+#endif
+#ifdef STATX_UID
+FLAG_GENERIC(STATX_UID),
+#endif
+#ifdef STATX_GID
+FLAG_GENERIC(STATX_GID),
+#endif
+#ifdef STATX_ATIME
+FLAG_GENERIC(STATX_ATIME),
+#endif
+#ifdef STATX_MTIME
+FLAG_GENERIC(STATX_MTIME),
+#endif
+#ifdef STATX_CTIME
+FLAG_GENERIC(STATX_CTIME),
+#endif
+#ifdef STATX_INO
+FLAG_GENERIC(STATX_INO),
+#endif
+#ifdef STATX_SIZE
+FLAG_GENERIC(STATX_SIZE),
+#endif
+#ifdef STATX_BLOCKS
+FLAG_GENERIC(STATX_BLOCKS),
+#endif
+#ifdef STATX_BTIME
+FLAG_GENERIC(STATX_BTIME),
+#endif
+FLAG_END,
+};
+
 /*
  * print_xxx utility functions.  These are used to print syscall
  * parameters in certain format.  All of these have parameter
@@ -2611,6 +2681,22 @@ print_tgkill(const struct syscallname *name,
 }
 #endif
 
+#ifdef TARGET_NR_statx
+static void
+print_statx(const struct syscallname *name,
+abi_long arg0, abi_long arg1, abi_long arg2,
+abi_long arg3, abi_long arg4, abi_long arg5)
+{
+print_syscall_prologue(name);
+print_at_dirfd(arg0, 0);
+print_string(arg1, 0);
+print_flags(statx_flags, arg2, 0);
+print_flags(statx_mask, arg3, 0);
+print_pointer(arg4, 1);
+print_syscall_epilogue(name);
+}
+#endif
+
 /*
  * An array of all of the syscalls we know about
  */
diff --git a/linux-user/strace.list b/linux-user/strace.list
index db21ce4..63a9466 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -1650,3 +1650,6 @@
 #ifdef TARGET_NR_atomic_barrier
 { TARGET_NR_atomic_barrier, "atomic_barrier", NULL, NULL, NULL },
 #endif
+#ifdef TARGET_NR_statx
+{ TARGET_NR_statx, "statx", NULL, print_statx, NULL },
+#endif
-- 
2.7.4

[Qemu-devel] [PATCH] RISC-V: Update syscall list for 32-bit support.

[Jim Wilson]

32-bit RISC-V uses _llseek instead of lseek as syscall number 62.
Update syscall list from open-embedded build, primarily because
32-bit RISC-V requires statx support.

Tested with cross gcc testsuite runs for rv32 and rv64, with the
pending statx patch also applied.

Signed-off-by: Jim Wilson 
---
 linux-user/riscv/syscall_nr.h | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/linux-user/riscv/syscall_nr.h b/linux-user/riscv/syscall_nr.h
index dab6509..5c87282 100644
--- a/linux-user/riscv/syscall_nr.h
+++ b/linux-user/riscv/syscall_nr.h
@@ -72,7 +72,11 @@
 #define TARGET_NR_pipe2 59
 #define TARGET_NR_quotactl 60
 #define TARGET_NR_getdents64 61
+#ifdef TARGET_RISCV32
+#define TARGET_NR__llseek 62
+#else
 #define TARGET_NR_lseek 62
+#endif
 #define TARGET_NR_read 63
 #define TARGET_NR_write 64
 #define TARGET_NR_readv 65
@@ -286,7 +290,16 @@
 #define TARGET_NR_membarrier 283
 #define TARGET_NR_mlock2 284
 #define TARGET_NR_copy_file_range 285
+#define TARGET_NR_preadv2 286
+#define TARGET_NR_pwritev2 287
+#define TARGET_NR_pkey_mprotect 288
+#define TARGET_NR_pkey_alloc 289
+#define TARGET_NR_pkey_free 290
+#define TARGET_NR_statx 291
+#define TARGET_NR_io_pgetevents 292
+#define TARGET_NR_rseq 293
+#define TARGET_NR_kexec_file_load 294
 
-#define TARGET_NR_syscalls (TARGET_NR_copy_file_range + 1)
+#define TARGET_NR_syscalls (TARGET_NR_kexec_file_load + 1)
 
 #endif
-- 
2.7.4

Re: [Qemu-devel] [PATCH v10 3/3] linux-user: Add support for statx() syscall

[Jim Wilson]

On 6/7/19 3:35 AM, Aleksandar Markovic wrote:

Implement support for translation of system call statx().


I also need these patches for 32-bit RISC-V linux user mode support.

glibc ld.so calls statx if fstatat is not supported.  Apparently new 
linux architecture ports aren't allowed to define __ARCH_WANT_NEW_STAT 
which enables fstatat because this is already obsolete.  64-bit RISC-V 
linux does have fstatat, but apparently this was a mistake which we 
can't fix now because the ABI is already frozen.  The 32-bit RISC-V ABI 
is not frozen yet, so it won't have fstatat.  Anyways, without statx, 
ld.so doesn't work, which makes user mode qemu pretty useless, so we do 
need this emulated in qemu to make the 32-bit RISC-V linux user mode 
support work properly.


I started with the August 2018 version of the patch a few weeks ago, and 
just noticed that it has been resubmitted.  I had to modify the patch 
slightly to apply to current sources, and had to fix one bug to make it 
work.  The line

+if (ret != TARGET_ENOSYS) {
needs to instead be
+if (ret != -TARGET_ENOSYS) {
I see that Laurent has already pointed that out.

Incidentally, I also have strace patches for statx that work on top of 
this patch, since I didn't see that in the nanomips patch set I started 
with.  That helped me debug the 32-bit RISC-V user mode support.


I've tested this on Ubuntu 16.04 (no host statx) and Ubuntu 19.10 (with 
host statx) and it worked well for me running the gcc testsuite for a 
riscv32-linux target.  I haven't tried testing the latest version of the 
patch yet.  I can do that if this is helpful.


Jim

[Qemu-devel] [PATCH v4 4/5] RISC-V: Add debug support for accessing CSRs.

[Jim Wilson]

Add a debugger field to CPURISCVState.  Add riscv_csrrw_debug function
to set it.  Disable mode checks when debugger field true.

Signed-off-by: Jim Wilson 
Reviewed-by: Alistair Francis 
---
 target/riscv/cpu.h |  5 +
 target/riscv/csr.c | 34 ++
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 743f02c..04a050e 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -170,6 +170,9 @@ struct CPURISCVState {
 
 /* physical memory protection */
 pmp_table_t pmp_state;
+
+/* True if in debugger mode.  */
+bool debugger;
 #endif
 
 float_status fp_status;
@@ -292,6 +295,8 @@ static inline void cpu_get_tb_cpu_state(CPURISCVState *env, 
target_ulong *pc,
 
 int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
 target_ulong new_value, target_ulong write_mask);
+int riscv_csrrw_debug(CPURISCVState *env, int csrno, target_ulong *ret_value,
+  target_ulong new_value, target_ulong write_mask);
 
 static inline void csr_write_helper(CPURISCVState *env, target_ulong val,
 int csrno)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 5e7e7d1..de28a5d 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -46,7 +46,7 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
 static int fs(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)
-if (!(env->mstatus & MSTATUS_FS)) {
+if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
 return -1;
 }
 #endif
@@ -58,7 +58,7 @@ static int ctr(CPURISCVState *env, int csrno)
 #if !defined(CONFIG_USER_ONLY)
 target_ulong ctr_en = env->priv == PRV_U ? env->scounteren :
   env->priv == PRV_S ? env->mcounteren : -1U;
-if (!(ctr_en & (1 << (csrno & 31 {
+if (!env->debugger && !(ctr_en & (1 << (csrno & 31 {
 return -1;
 }
 #endif
@@ -86,7 +86,7 @@ static int pmp(CPURISCVState *env, int csrno)
 static int read_fflags(CPURISCVState *env, int csrno, target_ulong *val)
 {
 #if !defined(CONFIG_USER_ONLY)
-if (!(env->mstatus & MSTATUS_FS)) {
+if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
 return -1;
 }
 #endif
@@ -97,7 +97,7 @@ static int read_fflags(CPURISCVState *env, int csrno, 
target_ulong *val)
 static int write_fflags(CPURISCVState *env, int csrno, target_ulong val)
 {
 #if !defined(CONFIG_USER_ONLY)
-if (!(env->mstatus & MSTATUS_FS)) {
+if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
 return -1;
 }
 env->mstatus |= MSTATUS_FS;
@@ -109,7 +109,7 @@ static int write_fflags(CPURISCVState *env, int csrno, 
target_ulong val)
 static int read_frm(CPURISCVState *env, int csrno, target_ulong *val)
 {
 #if !defined(CONFIG_USER_ONLY)
-if (!(env->mstatus & MSTATUS_FS)) {
+if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
 return -1;
 }
 #endif
@@ -120,7 +120,7 @@ static int read_frm(CPURISCVState *env, int csrno, 
target_ulong *val)
 static int write_frm(CPURISCVState *env, int csrno, target_ulong val)
 {
 #if !defined(CONFIG_USER_ONLY)
-if (!(env->mstatus & MSTATUS_FS)) {
+if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
 return -1;
 }
 env->mstatus |= MSTATUS_FS;
@@ -132,7 +132,7 @@ static int write_frm(CPURISCVState *env, int csrno, 
target_ulong val)
 static int read_fcsr(CPURISCVState *env, int csrno, target_ulong *val)
 {
 #if !defined(CONFIG_USER_ONLY)
-if (!(env->mstatus & MSTATUS_FS)) {
+if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
 return -1;
 }
 #endif
@@ -144,7 +144,7 @@ static int read_fcsr(CPURISCVState *env, int csrno, 
target_ulong *val)
 static int write_fcsr(CPURISCVState *env, int csrno, target_ulong val)
 {
 #if !defined(CONFIG_USER_ONLY)
-if (!(env->mstatus & MSTATUS_FS)) {
+if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
 return -1;
 }
 env->mstatus |= MSTATUS_FS;
@@ -772,6 +772,24 @@ int riscv_csrrw(CPURISCVState *env, int csrno, 
target_ulong *ret_value,
 return 0;
 }
 
+/*
+ * Debugger support.  If not in user mode, set env->debugger before the
+ * riscv_csrrw call and clear it after the call.
+ */
+int riscv_csrrw_debug(CPURISCVState *env, int csrno, target_ulong *ret_value,
+target_ulong new_value, target_ulong write_mask)
+{
+int ret;
+#if !defined(CONFIG_USER_ONLY)
+env->debugger = true;
+#endif
+ret = riscv_csrrw(env, csrno, ret_value, new_value, write_mask);
+#if !defined(CONFIG_USER_ONLY)
+env->debugger = false;
+#endif
+return ret;
+}
+
 /* Control and Status Register function table */
 static riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
 /* User Floating-Point CSRs */
-- 
2.7.4

[Qemu-devel] [PATCH v4 2/5] RISC-V: Add 64-bit gdb xml files.

[Jim Wilson]

Signed-off-by: Jim Wilson 
Reviewed-by: Alistair Francis 
---
 configure   |   1 +
 gdb-xml/riscv-64bit-cpu.xml |  43 
 gdb-xml/riscv-64bit-csr.xml | 250 
 gdb-xml/riscv-64bit-fpu.xml |  52 +
 4 files changed, 346 insertions(+)
 create mode 100644 gdb-xml/riscv-64bit-cpu.xml
 create mode 100644 gdb-xml/riscv-64bit-csr.xml
 create mode 100644 gdb-xml/riscv-64bit-fpu.xml

diff --git a/configure b/configure
index febe292..d7cae4e 100755
--- a/configure
+++ b/configure
@@ -7258,6 +7258,7 @@ case "$target_name" in
 TARGET_BASE_ARCH=riscv
 TARGET_ABI_DIR=riscv
 mttcg=yes
+gdb_xml_files="riscv-64bit-cpu.xml riscv-64bit-fpu.xml riscv-64bit-csr.xml"
 target_compiler=$cross_cc_riscv64
   ;;
   sh4|sh4eb)
diff --git a/gdb-xml/riscv-64bit-cpu.xml b/gdb-xml/riscv-64bit-cpu.xml
new file mode 100644
index 000..f37d7f3
--- /dev/null
+++ b/gdb-xml/riscv-64bit-cpu.xml
@@ -0,0 +1,43 @@
+
+
+
+
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
diff --git a/gdb-xml/riscv-64bit-csr.xml b/gdb-xml/riscv-64bit-csr.xml
new file mode 100644
index 000..a3de834
--- /dev/null
+++ b/gdb-xml/riscv-64bit-csr.xml
@@ -0,0 +1,250 @@
+
+
+
+
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
diff --git a/gdb-xml/riscv-64bit-fpu.xml b/gdb-xml/riscv-64bit-fpu.xml
new file mode 100644
index 000..fb24b72
--- /dev/null
+++ b/gdb-xml/riscv-64bit-fpu.xml
@@ -0,0 +1,52 @@
+
+
+
+
+
+
+  
+
+
+  
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
+  
+  
+  
+
-- 
2.7.4

[Qemu-devel] [PATCH v4 5/5] RISC-V: Add hooks to use the gdb xml files.

[Jim Wilson]

The gdb CSR xml file has registers in documentation order, not numerical
order, so we need a table to map the register numbers.  This also adds
fairly standard gdb hooks to access xml specified registers.

Signed-off-by: Jim Wilson 
---
 target/riscv/cpu.c |   9 +-
 target/riscv/cpu.h |   2 +
 target/riscv/gdbstub.c | 348 +++--
 3 files changed, 347 insertions(+), 12 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 28d7e53..c23bd01 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -311,6 +311,8 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
+riscv_cpu_register_gdb_regs_for_features(cs);
+
 qemu_init_vcpu(cs);
 cpu_reset(cs);
 
@@ -351,7 +353,12 @@ static void riscv_cpu_class_init(ObjectClass *c, void 
*data)
 cc->synchronize_from_tb = riscv_cpu_synchronize_from_tb;
 cc->gdb_read_register = riscv_cpu_gdb_read_register;
 cc->gdb_write_register = riscv_cpu_gdb_write_register;
-cc->gdb_num_core_regs = 65;
+cc->gdb_num_core_regs = 33;
+#if defined(TARGET_RISCV32)
+cc->gdb_core_xml_file = "riscv-32bit-cpu.xml";
+#elif defined(TARGET_RISCV64)
+cc->gdb_core_xml_file = "riscv-64bit-cpu.xml";
+#endif
 cc->gdb_stop_before_watchpoint = true;
 cc->disas_set_info = riscv_cpu_disas_set_info;
 #ifdef CONFIG_USER_ONLY
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 04a050e..c10e86c 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -329,6 +329,8 @@ typedef struct {
 void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops);
 void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops);
 
+void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
+
 #include "exec/cpu-all.h"
 
 #endif /* RISCV_CPU_H */
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index 3cabb21..621206d 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -21,6 +21,255 @@
 #include "exec/gdbstub.h"
 #include "cpu.h"
 
+/*
+ * The GDB CSR xml files list them in documentation order, not numerical order,
+ * and are missing entries for unnamed CSRs.  So we need to map the gdb numbers
+ * to the hardware numbers.
+ */
+
+static int csr_register_map[] = {
+CSR_USTATUS,
+CSR_UIE,
+CSR_UTVEC,
+CSR_USCRATCH,
+CSR_UEPC,
+CSR_UCAUSE,
+CSR_UTVAL,
+CSR_UIP,
+CSR_FFLAGS,
+CSR_FRM,
+CSR_FCSR,
+CSR_CYCLE,
+CSR_TIME,
+CSR_INSTRET,
+CSR_HPMCOUNTER3,
+CSR_HPMCOUNTER4,
+CSR_HPMCOUNTER5,
+CSR_HPMCOUNTER6,
+CSR_HPMCOUNTER7,
+CSR_HPMCOUNTER8,
+CSR_HPMCOUNTER9,
+CSR_HPMCOUNTER10,
+CSR_HPMCOUNTER11,
+CSR_HPMCOUNTER12,
+CSR_HPMCOUNTER13,
+CSR_HPMCOUNTER14,
+CSR_HPMCOUNTER15,
+CSR_HPMCOUNTER16,
+CSR_HPMCOUNTER17,
+CSR_HPMCOUNTER18,
+CSR_HPMCOUNTER19,
+CSR_HPMCOUNTER20,
+CSR_HPMCOUNTER21,
+CSR_HPMCOUNTER22,
+CSR_HPMCOUNTER23,
+CSR_HPMCOUNTER24,
+CSR_HPMCOUNTER25,
+CSR_HPMCOUNTER26,
+CSR_HPMCOUNTER27,
+CSR_HPMCOUNTER28,
+CSR_HPMCOUNTER29,
+CSR_HPMCOUNTER30,
+CSR_HPMCOUNTER31,
+CSR_CYCLEH,
+CSR_TIMEH,
+CSR_INSTRETH,
+CSR_HPMCOUNTER3H,
+CSR_HPMCOUNTER4H,
+CSR_HPMCOUNTER5H,
+CSR_HPMCOUNTER6H,
+CSR_HPMCOUNTER7H,
+CSR_HPMCOUNTER8H,
+CSR_HPMCOUNTER9H,
+CSR_HPMCOUNTER10H,
+CSR_HPMCOUNTER11H,
+CSR_HPMCOUNTER12H,
+CSR_HPMCOUNTER13H,
+CSR_HPMCOUNTER14H,
+CSR_HPMCOUNTER15H,
+CSR_HPMCOUNTER16H,
+CSR_HPMCOUNTER17H,
+CSR_HPMCOUNTER18H,
+CSR_HPMCOUNTER19H,
+CSR_HPMCOUNTER20H,
+CSR_HPMCOUNTER21H,
+CSR_HPMCOUNTER22H,
+CSR_HPMCOUNTER23H,
+CSR_HPMCOUNTER24H,
+CSR_HPMCOUNTER25H,
+CSR_HPMCOUNTER26H,
+CSR_HPMCOUNTER27H,
+CSR_HPMCOUNTER28H,
+CSR_HPMCOUNTER29H,
+CSR_HPMCOUNTER30H,
+CSR_HPMCOUNTER31H,
+CSR_SSTATUS,
+CSR_SEDELEG,
+CSR_SIDELEG,
+CSR_SIE,
+CSR_STVEC,
+CSR_SCOUNTEREN,
+CSR_SSCRATCH,
+CSR_SEPC,
+CSR_SCAUSE,
+CSR_STVAL,
+CSR_SIP,
+CSR_SATP,
+CSR_MVENDORID,
+CSR_MARCHID,
+CSR_MIMPID,
+CSR_MHARTID,
+CSR_MSTATUS,
+CSR_MISA,
+CSR_MEDELEG,
+CSR_MIDELEG,
+CSR_MIE,
+CSR_MTVEC,
+CSR_MCOUNTEREN,
+CSR_MSCRATCH,
+CSR_MEPC,
+CSR_MCAUSE,
+CSR_MTVAL,
+CSR_MIP,
+CSR_PMPCFG0,
+CSR_PMPCFG1,
+CSR_PMPCFG2,
+CSR_PMPCFG3,
+CSR_PMPADDR0,
+CSR_PMPADDR1,
+CSR_PMPADDR2,
+CSR_PMPADDR3,
+CSR_PMPADDR4,
+CSR_PMPADDR5,
+CSR_PMPADDR6,
+CSR_PMPADDR7,
+CSR_PMPADDR8,
+CSR_PMPADDR9,
+CSR_PMPADDR10,
+CSR_PMPADDR11,
+CSR_PMPADDR12,
+CSR_PMPADDR13,
+CSR_PMPADDR14,
+CSR_PMPADDR15,
+CSR_MCYCLE,
+CSR_MINSTRET,
+CSR_MHPMCOUNTER3,
+CSR_MHPMCOUNTER4,
+CSR_MHPMCOUNTER5,
+CSR_MHPMCOUNTER6,
+CSR_MHPMCOUNTER7,
+CSR_MHPMCO

[Qemu-devel] [PATCH v4 3/5] RISC-V: Fixes to CSR_* register macros.

[Jim Wilson]

This adds some missing CSR_* register macros, and documents some as being
priv v1.9.1 specific.

Signed-off-by: Jim Wilson 
Reviewed-by: Alistair Francis 
---
 target/riscv/cpu_bits.h | 35 +--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 5439f47..316d500 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -135,16 +135,22 @@
 /* Legacy Counter Setup (priv v1.9.1) */
 #define CSR_MUCOUNTEREN 0x320
 #define CSR_MSCOUNTEREN 0x321
+#define CSR_MHCOUNTEREN 0x322
 
 /* Machine Trap Handling */
 #define CSR_MSCRATCH0x340
 #define CSR_MEPC0x341
 #define CSR_MCAUSE  0x342
-#define CSR_MBADADDR0x343
+#define CSR_MTVAL   0x343
 #define CSR_MIP 0x344
 
+/* Legacy Machine Trap Handling (priv v1.9.1) */
+#define CSR_MBADADDR0x343
+
 /* Supervisor Trap Setup */
 #define CSR_SSTATUS 0x100
+#define CSR_SEDELEG 0x102
+#define CSR_SIDELEG 0x103
 #define CSR_SIE 0x104
 #define CSR_STVEC   0x105
 #define CSR_SCOUNTEREN  0x106
@@ -153,9 +159,12 @@
 #define CSR_SSCRATCH0x140
 #define CSR_SEPC0x141
 #define CSR_SCAUSE  0x142
-#define CSR_SBADADDR0x143
+#define CSR_STVAL   0x143
 #define CSR_SIP 0x144
 
+/* Legacy Supervisor Trap Handling (priv v1.9.1) */
+#define CSR_SBADADDR0x143
+
 /* Supervisor Protection and Translation */
 #define CSR_SPTBR   0x180
 #define CSR_SATP0x180
@@ -282,6 +291,28 @@
 #define CSR_MHPMCOUNTER30H  0xb9e
 #define CSR_MHPMCOUNTER31H  0xb9f
 
+/* Legacy Hypervisor Trap Setup (priv v1.9.1) */
+#define CSR_HSTATUS 0x200
+#define CSR_HEDELEG 0x202
+#define CSR_HIDELEG 0x203
+#define CSR_HIE 0x204
+#define CSR_HTVEC   0x205
+
+/* Legacy Hypervisor Trap Handling (priv v1.9.1) */
+#define CSR_HSCRATCH0x240
+#define CSR_HEPC0x241
+#define CSR_HCAUSE  0x242
+#define CSR_HBADADDR0x243
+#define CSR_HIP 0x244
+
+/* Legacy Machine Protection and Translation (priv v1.9.1) */
+#define CSR_MBASE   0x380
+#define CSR_MBOUND  0x381
+#define CSR_MIBASE  0x382
+#define CSR_MIBOUND 0x383
+#define CSR_MDBASE  0x384
+#define CSR_MDBOUND 0x385
+
 /* mstatus CSR bits */
 #define MSTATUS_UIE 0x0001
 #define MSTATUS_SIE 0x0002
-- 
2.7.4

[Qemu-devel] [PATCH v4 1/5] RISC-V: Add 32-bit gdb xml files.

[Jim Wilson]

Signed-off-by: Jim Wilson 
Reviewed-by: Alistair Francis 
---
 configure   |   1 +
 gdb-xml/riscv-32bit-cpu.xml |  43 
 gdb-xml/riscv-32bit-csr.xml | 250 
 gdb-xml/riscv-32bit-fpu.xml |  46 
 4 files changed, 340 insertions(+)
 create mode 100644 gdb-xml/riscv-32bit-cpu.xml
 create mode 100644 gdb-xml/riscv-32bit-csr.xml
 create mode 100644 gdb-xml/riscv-32bit-fpu.xml

diff --git a/configure b/configure
index fbd0825..febe292 100755
--- a/configure
+++ b/configure
@@ -7251,6 +7251,7 @@ case "$target_name" in
 TARGET_BASE_ARCH=riscv
 TARGET_ABI_DIR=riscv
 mttcg=yes
+gdb_xml_files="riscv-32bit-cpu.xml riscv-32bit-fpu.xml riscv-32bit-csr.xml"
 target_compiler=$cross_cc_riscv32
   ;;
   riscv64)
diff --git a/gdb-xml/riscv-32bit-cpu.xml b/gdb-xml/riscv-32bit-cpu.xml
new file mode 100644
index 000..c02f86c
--- /dev/null
+++ b/gdb-xml/riscv-32bit-cpu.xml
@@ -0,0 +1,43 @@
+
+
+
+
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
diff --git a/gdb-xml/riscv-32bit-csr.xml b/gdb-xml/riscv-32bit-csr.xml
new file mode 100644
index 000..4aea9e6
--- /dev/null
+++ b/gdb-xml/riscv-32bit-csr.xml
@@ -0,0 +1,250 @@
+
+
+
+
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
diff --git a/gdb-xml/riscv-32bit-fpu.xml b/gdb-xml/riscv-32bit-fpu.xml
new file mode 100644
index 000..783287d
--- /dev/null
+++ b/gdb-xml/riscv-32bit-fpu.xml
@@ -0,0 +1,46 @@
+
+
+
+
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
+  
+  
+  
+
-- 
2.7.4

[Qemu-devel] [PATCH v4 0/5] RISC-V: Add gdb xml files and gdbstub support.

[Jim Wilson]

This is the 4th version of the patch set.  Updated as per the review
from Alistair, it has the riscv_csrrw_debug function added, and
Reviewed-By lines added.  Otherwise it is the same as the 3rd version.

Jim

Re: [Qemu-devel] [PATCH 5/5 v3] RISC-V: Add hooks to use the gdb xml files.

[Jim Wilson]

On Fri, Feb 8, 2019 at 10:17 AM Alistair Francis  wrote:
> Can we just write a wrapper function then that sets and unsets the variable?
> Something like this:
>
> riscv_csrrw_debug(...) {
> #if !defined(CONFIG_USER_ONLY)
> env->debugger = true;
> #endif
> result = riscv_csrrw(env, ...);
> #if !defined(CONFIG_USER_ONLY)
> env->debugger = false;
> #endif
> }

Yes, that would work.  Do you want me to resubmit a fixed part 5 patch?

Jim

Re: [Qemu-devel] [PATCH 5/5 v3] RISC-V: Add hooks to use the gdb xml files.

[Jim Wilson]

On Wed, Feb 6, 2019 at 4:04 PM Alistair Francis  wrote:
> Would it not be easier to add an extra argument to the functions
> intstead of setting and unsetting this?
>
> That's what you had in the earlier version of this set.

The csr support was rewritten, and is now a table of functions.  If I
change the type signature for one function in a table column I have to
change them all.  This means I have to change the type signature of
about 50 functions, the vast majority of which will never use the new
argument.  I was hoping to avoid that, so I added a variable into the
cpu state instead.  In the old patch, before the csr rewrite, I only
had to change the type signature of a couple of functions, most of
which did use the new argument, so that was reasonable at the time.

If I do need to change a lot of type signatures, what about attribute
((unused))?  I see that there is a ATTRIBUTE_UNUSED in the disas dir
but it is apparently not used outside there, and only defined in
include/disas/bfd.h.  I see a few scattered uses of
attribute((unused)) but that seems unwise for portability reasons.
Maybe qemu doesn't care about unused arguments/parameters?

Jim

[Qemu-devel] [PATCH 5/5 v3] RISC-V: Add hooks to use the gdb xml files.

[Jim Wilson]

The gdb CSR xml file has registers in documentation order, not numerical
order, so we need a table to map the register numbers.  This also adds
fairly standard gdb hooks to access xml specified registers.

Signed-off-by: Jim Wilson 
---
 target/riscv/cpu.c |   9 +-
 target/riscv/cpu.h |   2 +
 target/riscv/gdbstub.c | 373 +++--
 3 files changed, 372 insertions(+), 12 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 28d7e53..c23bd01 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -311,6 +311,8 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
+riscv_cpu_register_gdb_regs_for_features(cs);
+
 qemu_init_vcpu(cs);
 cpu_reset(cs);
 
@@ -351,7 +353,12 @@ static void riscv_cpu_class_init(ObjectClass *c, void 
*data)
 cc->synchronize_from_tb = riscv_cpu_synchronize_from_tb;
 cc->gdb_read_register = riscv_cpu_gdb_read_register;
 cc->gdb_write_register = riscv_cpu_gdb_write_register;
-cc->gdb_num_core_regs = 65;
+cc->gdb_num_core_regs = 33;
+#if defined(TARGET_RISCV32)
+cc->gdb_core_xml_file = "riscv-32bit-cpu.xml";
+#elif defined(TARGET_RISCV64)
+cc->gdb_core_xml_file = "riscv-64bit-cpu.xml";
+#endif
 cc->gdb_stop_before_watchpoint = true;
 cc->disas_set_info = riscv_cpu_disas_set_info;
 #ifdef CONFIG_USER_ONLY
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index faa46d0..a7dcdb6 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -327,6 +327,8 @@ typedef struct {
 void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops);
 void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops);
 
+void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
+
 #include "exec/cpu-all.h"
 
 #endif /* RISCV_CPU_H */
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index 3cabb21..44e19a2 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -21,6 +21,255 @@
 #include "exec/gdbstub.h"
 #include "cpu.h"
 
+/*
+ * The GDB CSR xml files list them in documentation order, not numerical order,
+ * and are missing entries for unnamed CSRs.  So we need to map the gdb numbers
+ * to the hardware numbers.
+ */
+
+static int csr_register_map[] = {
+CSR_USTATUS,
+CSR_UIE,
+CSR_UTVEC,
+CSR_USCRATCH,
+CSR_UEPC,
+CSR_UCAUSE,
+CSR_UTVAL,
+CSR_UIP,
+CSR_FFLAGS,
+CSR_FRM,
+CSR_FCSR,
+CSR_CYCLE,
+CSR_TIME,
+CSR_INSTRET,
+CSR_HPMCOUNTER3,
+CSR_HPMCOUNTER4,
+CSR_HPMCOUNTER5,
+CSR_HPMCOUNTER6,
+CSR_HPMCOUNTER7,
+CSR_HPMCOUNTER8,
+CSR_HPMCOUNTER9,
+CSR_HPMCOUNTER10,
+CSR_HPMCOUNTER11,
+CSR_HPMCOUNTER12,
+CSR_HPMCOUNTER13,
+CSR_HPMCOUNTER14,
+CSR_HPMCOUNTER15,
+CSR_HPMCOUNTER16,
+CSR_HPMCOUNTER17,
+CSR_HPMCOUNTER18,
+CSR_HPMCOUNTER19,
+CSR_HPMCOUNTER20,
+CSR_HPMCOUNTER21,
+CSR_HPMCOUNTER22,
+CSR_HPMCOUNTER23,
+CSR_HPMCOUNTER24,
+CSR_HPMCOUNTER25,
+CSR_HPMCOUNTER26,
+CSR_HPMCOUNTER27,
+CSR_HPMCOUNTER28,
+CSR_HPMCOUNTER29,
+CSR_HPMCOUNTER30,
+CSR_HPMCOUNTER31,
+CSR_CYCLEH,
+CSR_TIMEH,
+CSR_INSTRETH,
+CSR_HPMCOUNTER3H,
+CSR_HPMCOUNTER4H,
+CSR_HPMCOUNTER5H,
+CSR_HPMCOUNTER6H,
+CSR_HPMCOUNTER7H,
+CSR_HPMCOUNTER8H,
+CSR_HPMCOUNTER9H,
+CSR_HPMCOUNTER10H,
+CSR_HPMCOUNTER11H,
+CSR_HPMCOUNTER12H,
+CSR_HPMCOUNTER13H,
+CSR_HPMCOUNTER14H,
+CSR_HPMCOUNTER15H,
+CSR_HPMCOUNTER16H,
+CSR_HPMCOUNTER17H,
+CSR_HPMCOUNTER18H,
+CSR_HPMCOUNTER19H,
+CSR_HPMCOUNTER20H,
+CSR_HPMCOUNTER21H,
+CSR_HPMCOUNTER22H,
+CSR_HPMCOUNTER23H,
+CSR_HPMCOUNTER24H,
+CSR_HPMCOUNTER25H,
+CSR_HPMCOUNTER26H,
+CSR_HPMCOUNTER27H,
+CSR_HPMCOUNTER28H,
+CSR_HPMCOUNTER29H,
+CSR_HPMCOUNTER30H,
+CSR_HPMCOUNTER31H,
+CSR_SSTATUS,
+CSR_SEDELEG,
+CSR_SIDELEG,
+CSR_SIE,
+CSR_STVEC,
+CSR_SCOUNTEREN,
+CSR_SSCRATCH,
+CSR_SEPC,
+CSR_SCAUSE,
+CSR_STVAL,
+CSR_SIP,
+CSR_SATP,
+CSR_MVENDORID,
+CSR_MARCHID,
+CSR_MIMPID,
+CSR_MHARTID,
+CSR_MSTATUS,
+CSR_MISA,
+CSR_MEDELEG,
+CSR_MIDELEG,
+CSR_MIE,
+CSR_MTVEC,
+CSR_MCOUNTEREN,
+CSR_MSCRATCH,
+CSR_MEPC,
+CSR_MCAUSE,
+CSR_MTVAL,
+CSR_MIP,
+CSR_PMPCFG0,
+CSR_PMPCFG1,
+CSR_PMPCFG2,
+CSR_PMPCFG3,
+CSR_PMPADDR0,
+CSR_PMPADDR1,
+CSR_PMPADDR2,
+CSR_PMPADDR3,
+CSR_PMPADDR4,
+CSR_PMPADDR5,
+CSR_PMPADDR6,
+CSR_PMPADDR7,
+CSR_PMPADDR8,
+CSR_PMPADDR9,
+CSR_PMPADDR10,
+CSR_PMPADDR11,
+CSR_PMPADDR12,
+CSR_PMPADDR13,
+CSR_PMPADDR14,
+CSR_PMPADDR15,
+CSR_MCYCLE,
+CSR_MINSTRET,
+CSR_MHPMCOUNTER3,
+CSR_MHPMCOUNTER4,
+CSR_MHPMCOUNTER5,
+CSR_MHPMCOUNTER6,
+CSR_MHPMCOUNTER7,
+CSR_MHPMCO

[Qemu-devel] [PATCH 4/5 v3] RISC-V: Add debug support for accessing CSRs.

[Jim Wilson]

Adds a debugger field to CPURISCVState.  Disable mode checks in riscv_csrrw
when true.

Signed-off-by: Jim Wilson 
---
 target/riscv/cpu.h |  3 +++
 target/riscv/csr.c | 16 
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 743f02c..faa46d0 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -170,6 +170,9 @@ struct CPURISCVState {
 
 /* physical memory protection */
 pmp_table_t pmp_state;
+
+/* True if in debugger mode.  */
+bool debugger;
 #endif
 
 float_status fp_status;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 5e7e7d1..04e6b59 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -46,7 +46,7 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
 static int fs(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)
-if (!(env->mstatus & MSTATUS_FS)) {
+if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
 return -1;
 }
 #endif
@@ -58,7 +58,7 @@ static int ctr(CPURISCVState *env, int csrno)
 #if !defined(CONFIG_USER_ONLY)
 target_ulong ctr_en = env->priv == PRV_U ? env->scounteren :
   env->priv == PRV_S ? env->mcounteren : -1U;
-if (!(ctr_en & (1 << (csrno & 31 {
+if (!env->debugger && !(ctr_en & (1 << (csrno & 31 {
 return -1;
 }
 #endif
@@ -86,7 +86,7 @@ static int pmp(CPURISCVState *env, int csrno)
 static int read_fflags(CPURISCVState *env, int csrno, target_ulong *val)
 {
 #if !defined(CONFIG_USER_ONLY)
-if (!(env->mstatus & MSTATUS_FS)) {
+if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
 return -1;
 }
 #endif
@@ -97,7 +97,7 @@ static int read_fflags(CPURISCVState *env, int csrno, 
target_ulong *val)
 static int write_fflags(CPURISCVState *env, int csrno, target_ulong val)
 {
 #if !defined(CONFIG_USER_ONLY)
-if (!(env->mstatus & MSTATUS_FS)) {
+if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
 return -1;
 }
 env->mstatus |= MSTATUS_FS;
@@ -109,7 +109,7 @@ static int write_fflags(CPURISCVState *env, int csrno, 
target_ulong val)
 static int read_frm(CPURISCVState *env, int csrno, target_ulong *val)
 {
 #if !defined(CONFIG_USER_ONLY)
-if (!(env->mstatus & MSTATUS_FS)) {
+if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
 return -1;
 }
 #endif
@@ -120,7 +120,7 @@ static int read_frm(CPURISCVState *env, int csrno, 
target_ulong *val)
 static int write_frm(CPURISCVState *env, int csrno, target_ulong val)
 {
 #if !defined(CONFIG_USER_ONLY)
-if (!(env->mstatus & MSTATUS_FS)) {
+if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
 return -1;
 }
 env->mstatus |= MSTATUS_FS;
@@ -132,7 +132,7 @@ static int write_frm(CPURISCVState *env, int csrno, 
target_ulong val)
 static int read_fcsr(CPURISCVState *env, int csrno, target_ulong *val)
 {
 #if !defined(CONFIG_USER_ONLY)
-if (!(env->mstatus & MSTATUS_FS)) {
+if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
 return -1;
 }
 #endif
@@ -144,7 +144,7 @@ static int read_fcsr(CPURISCVState *env, int csrno, 
target_ulong *val)
 static int write_fcsr(CPURISCVState *env, int csrno, target_ulong val)
 {
 #if !defined(CONFIG_USER_ONLY)
-if (!(env->mstatus & MSTATUS_FS)) {
+if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
 return -1;
 }
 env->mstatus |= MSTATUS_FS;
-- 
2.7.4

[Qemu-devel] [PATCH 3/5 v3] RISC-V: Fixes to CSR_* register macros.

[Jim Wilson]

This adds some missing CSR_* register macros, and documents some as being
priv v1.9.1 specific.

Signed-off-by: Jim Wilson 
---
 target/riscv/cpu_bits.h | 35 +--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 5439f47..316d500 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -135,16 +135,22 @@
 /* Legacy Counter Setup (priv v1.9.1) */
 #define CSR_MUCOUNTEREN 0x320
 #define CSR_MSCOUNTEREN 0x321
+#define CSR_MHCOUNTEREN 0x322
 
 /* Machine Trap Handling */
 #define CSR_MSCRATCH0x340
 #define CSR_MEPC0x341
 #define CSR_MCAUSE  0x342
-#define CSR_MBADADDR0x343
+#define CSR_MTVAL   0x343
 #define CSR_MIP 0x344
 
+/* Legacy Machine Trap Handling (priv v1.9.1) */
+#define CSR_MBADADDR0x343
+
 /* Supervisor Trap Setup */
 #define CSR_SSTATUS 0x100
+#define CSR_SEDELEG 0x102
+#define CSR_SIDELEG 0x103
 #define CSR_SIE 0x104
 #define CSR_STVEC   0x105
 #define CSR_SCOUNTEREN  0x106
@@ -153,9 +159,12 @@
 #define CSR_SSCRATCH0x140
 #define CSR_SEPC0x141
 #define CSR_SCAUSE  0x142
-#define CSR_SBADADDR0x143
+#define CSR_STVAL   0x143
 #define CSR_SIP 0x144
 
+/* Legacy Supervisor Trap Handling (priv v1.9.1) */
+#define CSR_SBADADDR0x143
+
 /* Supervisor Protection and Translation */
 #define CSR_SPTBR   0x180
 #define CSR_SATP0x180
@@ -282,6 +291,28 @@
 #define CSR_MHPMCOUNTER30H  0xb9e
 #define CSR_MHPMCOUNTER31H  0xb9f
 
+/* Legacy Hypervisor Trap Setup (priv v1.9.1) */
+#define CSR_HSTATUS 0x200
+#define CSR_HEDELEG 0x202
+#define CSR_HIDELEG 0x203
+#define CSR_HIE 0x204
+#define CSR_HTVEC   0x205
+
+/* Legacy Hypervisor Trap Handling (priv v1.9.1) */
+#define CSR_HSCRATCH0x240
+#define CSR_HEPC0x241
+#define CSR_HCAUSE  0x242
+#define CSR_HBADADDR0x243
+#define CSR_HIP 0x244
+
+/* Legacy Machine Protection and Translation (priv v1.9.1) */
+#define CSR_MBASE   0x380
+#define CSR_MBOUND  0x381
+#define CSR_MIBASE  0x382
+#define CSR_MIBOUND 0x383
+#define CSR_MDBASE  0x384
+#define CSR_MDBOUND 0x385
+
 /* mstatus CSR bits */
 #define MSTATUS_UIE 0x0001
 #define MSTATUS_SIE 0x0002
-- 
2.7.4

[Qemu-devel] [PATCH 2/5 v3] RISC-V: Add 64-bit gdb xml files.

[Jim Wilson]

Signed-off-by: Jim Wilson 
---
 configure   |   1 +
 gdb-xml/riscv-64bit-cpu.xml |  43 
 gdb-xml/riscv-64bit-csr.xml | 250 
 gdb-xml/riscv-64bit-fpu.xml |  52 +
 4 files changed, 346 insertions(+)
 create mode 100644 gdb-xml/riscv-64bit-cpu.xml
 create mode 100644 gdb-xml/riscv-64bit-csr.xml
 create mode 100644 gdb-xml/riscv-64bit-fpu.xml

diff --git a/configure b/configure
index f30369a..5e71d28 100755
--- a/configure
+++ b/configure
@@ -7290,6 +7290,7 @@ case "$target_name" in
 TARGET_BASE_ARCH=riscv
 TARGET_ABI_DIR=riscv
 mttcg=yes
+gdb_xml_files="riscv-64bit-cpu.xml riscv-64bit-fpu.xml riscv-64bit-csr.xml"
 target_compiler=$cross_cc_riscv64
   ;;
   sh4|sh4eb)
diff --git a/gdb-xml/riscv-64bit-cpu.xml b/gdb-xml/riscv-64bit-cpu.xml
new file mode 100644
index 000..f37d7f3
--- /dev/null
+++ b/gdb-xml/riscv-64bit-cpu.xml
@@ -0,0 +1,43 @@
+
+
+
+
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
diff --git a/gdb-xml/riscv-64bit-csr.xml b/gdb-xml/riscv-64bit-csr.xml
new file mode 100644
index 000..a3de834
--- /dev/null
+++ b/gdb-xml/riscv-64bit-csr.xml
@@ -0,0 +1,250 @@
+
+
+
+
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
diff --git a/gdb-xml/riscv-64bit-fpu.xml b/gdb-xml/riscv-64bit-fpu.xml
new file mode 100644
index 000..fb24b72
--- /dev/null
+++ b/gdb-xml/riscv-64bit-fpu.xml
@@ -0,0 +1,52 @@
+
+
+
+
+
+
+  
+
+
+  
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
+  
+  
+  
+
-- 
2.7.4

[Qemu-devel] [PATCH 1/5 v3] RISC-V: Add 32-bit gdb xml files.

[Jim Wilson]

Signed-off-by: Jim Wilson 
---
 configure   |   1 +
 gdb-xml/riscv-32bit-cpu.xml |  43 
 gdb-xml/riscv-32bit-csr.xml | 250 
 gdb-xml/riscv-32bit-fpu.xml |  46 
 4 files changed, 340 insertions(+)
 create mode 100644 gdb-xml/riscv-32bit-cpu.xml
 create mode 100644 gdb-xml/riscv-32bit-csr.xml
 create mode 100644 gdb-xml/riscv-32bit-fpu.xml

diff --git a/configure b/configure
index b18281c..f30369a 100755
--- a/configure
+++ b/configure
@@ -7283,6 +7283,7 @@ case "$target_name" in
 TARGET_BASE_ARCH=riscv
 TARGET_ABI_DIR=riscv
 mttcg=yes
+gdb_xml_files="riscv-32bit-cpu.xml riscv-32bit-fpu.xml riscv-32bit-csr.xml"
 target_compiler=$cross_cc_riscv32
   ;;
   riscv64)
diff --git a/gdb-xml/riscv-32bit-cpu.xml b/gdb-xml/riscv-32bit-cpu.xml
new file mode 100644
index 000..c02f86c
--- /dev/null
+++ b/gdb-xml/riscv-32bit-cpu.xml
@@ -0,0 +1,43 @@
+
+
+
+
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
diff --git a/gdb-xml/riscv-32bit-csr.xml b/gdb-xml/riscv-32bit-csr.xml
new file mode 100644
index 000..4aea9e6
--- /dev/null
+++ b/gdb-xml/riscv-32bit-csr.xml
@@ -0,0 +1,250 @@
+
+
+
+
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
diff --git a/gdb-xml/riscv-32bit-fpu.xml b/gdb-xml/riscv-32bit-fpu.xml
new file mode 100644
index 000..783287d
--- /dev/null
+++ b/gdb-xml/riscv-32bit-fpu.xml
@@ -0,0 +1,46 @@
+
+
+
+
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
+  
+  
+  
+
-- 
2.7.4

[Qemu-devel] [PATCH 0/5 v3] RISC-V: Add gdb xml files and gdbstub support.

[Jim Wilson]

This is the 3rd version of the patch set, updated as per the review
from Richard and Alistair, and updated for current top of tree.  Parts
1 and 2 are the same.  Part 3 is smaller because some of it was moved
to part 5.  Part 5 is bigger because it received part of part 3.
Parts 4 and 5 have changed because the csr support was rewritten.  The
illegal instruction traps are no longer inside the csr support code,
but it still has mode checks that I need to disable, so I still need a
debugger mode, but it doesn't do as much as before.  Also, what was
one function is now about one hundred functions, so rather than add a
debugger parameter to lots of functions that don't need it, I put it
in the CPURISCVState struct.  This also helps fix a bug, because
debugger mode is only appropriate for system qemu, and should not be
enabled for user qemu.  The new patch set gets this right.  As a
result of this, part 4 also ends up smaller, and part 5 is rewritten a
bit to use the new interface to the csr support.  Otherwise it is
effectively the same code as before.

Jim

Re: [Qemu-devel] [PATCH 3/5 v2] RISC-V: Map gdb CSR reg numbers to hw reg numbers.

[Jim Wilson]

On Tue, Jan 22, 2019 at 1:45 PM Alistair Francis  wrote:
> I think it makes more sense to just define the variable in the
> gdbstubs.c file then. Can you move it to patch 5?

Yes, that is no problem.  That makes patch 3 a lot smaller and patch 5
a lot bigger, but it is the same code as before, just arranged
differently, so this shouldn't complicate the review too much.

Jim

Re: [Qemu-devel] [PATCH 5/5 v2] RISC-V: Add hooks to use the gdb xml files.

[Jim Wilson]

On Tue, Jan 22, 2019 at 1:52 PM Alistair Francis  wrote:
> You can get env and then check for floating point support:
>
> CPURISCVState *env = >env;
> if (env->misa_mask & RVF) {
> ...

I needed this which wasn't hard to figure out.
RISCVCPU *cpu = RISCV_CPU(cs);
CPURISCVState *env = >env;
if (env->misa & RVF) {

The tricky bit was figuring out how to test it, because I wasn't sure
if making registers conditional would actually work.  I figured out
that using -machine sifive_e gives me a target with no fpu, and
playing with that a bit I get the expected result, which is that the
FP regs don't print anymore.  The FP related CSRs still do, but that
would require gdb fixes I think, because gdb knows that they are both
FP regs and CSR, and tries to print them both ways.  That leads to a
more general problem of figuring out exactly which CSRs a particular
target implements, which is a bigger problem than I have time to fix
at the moment, and should be handled as a separate problem.

Since my patch set is now a month old, I'll rebase onto current master
and post a version 3 patch set.

Jim

Re: [Qemu-devel] [PATCH 1/5 v2] RISC-V: Add 32-bit gdb xml files.

[Jim Wilson]

On Sat, Dec 29, 2018 at 2:20 PM Richard Henderson
 wrote:
> On 12/29/18 9:07 AM, Jim Wilson wrote:
> Don't the csr's vary between priv-1.9.1 and priv-1.10?

There are a few csr's that disappear in 1.10, but there is no known
hardware that implements them.  There are a few csr's new in 1.10, but
I don't know of any 1.9.1 systems that can run the software that
requires the new registers.  There are a few that change name; the
toolchain supports both the old and new names.  On the toolchain side,
currently, none of the tools care about the privilege spec version.
And the linux kernel doesn't care about the priv spec version either.
This will eventually have to change, maybe with 1.11, but so far it
hasn't been an issue with any software I've seen.

qemu has an option to specify the priv spec version.  But looking at
the csr's, this affects two of the registers that disappeared, which
were never implemented in hardware, and aren't supported by gdb.  This
also affects a few registers that have new fields in 1.10, but gdb
doesn't know about those internal register fields, so this doesn't
affect gdb either.

> I wonder if it would be better to auto-generate these, via the
> gdb_get_dynamic_xml hook.  That way you don't need near identical copies for
> 32-bit vs 64-bit.

I haven't tried looking at this, but I don't see a convenient list of
implemented csr's.  This list would be every csr that the
csr_read_helper function in target/riscv/op_helper.c supports  That
function requires some machine state that includes the register
values, so it doesn't look convenient to use, unless we add more
hackery to it.

Long term, it would be useful to have target specific csr register
lists.  Most csr's are optional, and most hardware only implements a
subset of them.  This is somewhat orthogonal to what I'm trying to do
here, but this would require a list of supported csr's in the target
hardware file, then this would have to be fed back into
csr_read_helper somehow, and then we could also use this to generate
an appropriate xml csr register file for that target for use with gdb.
Openocd already does something like this for a few supported targets.

Jim

Re: [Qemu-devel] [PATCH 3/5 v2] RISC-V: Map gdb CSR reg numbers to hw reg numbers.

[Jim Wilson]

On Sat, Dec 29, 2018 at 2:23 PM Richard Henderson
 wrote:
> On 12/29/18 9:09 AM, Jim Wilson wrote:
> > +int csr_register_map[] = {
>
> static const?

If I add static const here, then I get a build error if this patch is
applied to the tree but the following patch #5 that uses the variable
is not applied.  Though I suppose I could fix that if I put the static
const in patch 5.  That would look a little funny but would work.

> Putting an initialized variable in a header file doesn't seem right.  Is this
> supposed to be a declaration that is shared between c files?

I did it this way for two reasons.  It makes it easier to keep the
register mapping consistent with the gdb csr file, if the gdb csr file
happens to change in the future.  We can just do a line by line
comparison of the gdb csr file against the csr-map.h file to verify
that the csr names match.  And it makes for a cleaner patch if the gdb
csr file register numbering gets fixed in the future, in that case we
can just delete this file and change a few lines to stop using this
variable.

This variable is only meant to be used in one file, the
target/riscv/gdbstubs.c file.

Jim

[Qemu-devel] [PATCH 0/5 v2] RISC-V: Add gdb xml files and gdbstub support.

[Jim Wilson]

This is the second version of the patch set.  I haven't gotten any bug
reports for the patches in the several weeks that they have been
available, and no review yet, so the only significant difference from
the first version is that I ran them through checkpatch to fix style
issues, and I sent them to the right mailing list this time.

With these patches, I can run system qemu with -s -S, connect to it
with gdb, and then print any integer, FP, or CSR register.  This
support is currently broken.

The first two patches in the series are just dropping in xml files
from gdb unchanged, with configure support to point at them.  The next
two patches are the inconvenient stuff.  The gdb CSR files are in
documentation order not numerical order, so I need an array to map the
gdb numbers to the actual hardware numbers.  The gdb files should
probably be improved, but I don't know when that will happen.  That is
the third patch.  The fourth patch is to disable all of the illegal
instruction checks when accessing CSRs from the debugger, as it needs
to be able to access them without traps.  The last patch adds the
gdbstub support to use the new xml files which is pretty simple,
though there is an unanswered question in there about what to do with
the FP registers.  Should we try to configure them based on the
target?  Right now I just always enable them which is the simple
solution.  I'm not sure if enabling them based on the target will even
work with gdb or not.

Jim

[Qemu-devel] [PATCH 1/5 v2] RISC-V: Add 32-bit gdb xml files.

[Jim Wilson]

Signed-off-by: Jim Wilson 
---
 configure   |   1 +
 gdb-xml/riscv-32bit-cpu.xml |  43 
 gdb-xml/riscv-32bit-csr.xml | 250 
 gdb-xml/riscv-32bit-fpu.xml |  46 
 4 files changed, 340 insertions(+)
 create mode 100644 gdb-xml/riscv-32bit-cpu.xml
 create mode 100644 gdb-xml/riscv-32bit-csr.xml
 create mode 100644 gdb-xml/riscv-32bit-fpu.xml

diff --git a/configure b/configure
index 224d307..4e05eed 100755
--- a/configure
+++ b/configure
@@ -7208,6 +7208,7 @@ case "$target_name" in
 TARGET_BASE_ARCH=riscv
 TARGET_ABI_DIR=riscv
 mttcg=yes
+gdb_xml_files="riscv-32bit-cpu.xml riscv-32bit-fpu.xml riscv-32bit-csr.xml"
 target_compiler=$cross_cc_riscv32
   ;;
   riscv64)
diff --git a/gdb-xml/riscv-32bit-cpu.xml b/gdb-xml/riscv-32bit-cpu.xml
new file mode 100644
index 000..c02f86c
--- /dev/null
+++ b/gdb-xml/riscv-32bit-cpu.xml
@@ -0,0 +1,43 @@
+
+
+
+
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
diff --git a/gdb-xml/riscv-32bit-csr.xml b/gdb-xml/riscv-32bit-csr.xml
new file mode 100644
index 000..4aea9e6
--- /dev/null
+++ b/gdb-xml/riscv-32bit-csr.xml
@@ -0,0 +1,250 @@
+
+
+
+
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
diff --git a/gdb-xml/riscv-32bit-fpu.xml b/gdb-xml/riscv-32bit-fpu.xml
new file mode 100644
index 000..783287d
--- /dev/null
+++ b/gdb-xml/riscv-32bit-fpu.xml
@@ -0,0 +1,46 @@
+
+
+
+
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
+  
+  
+  
+
-- 
2.7.4

[Qemu-devel] [PATCH 4/5 v2] RISC-V: Add debug support for accessing CSRs.

[Jim Wilson]

Adds a debugger parameter to csr_read_helper and csr_write_helper.  When
this is true, we disable illegal instruction checks.

Signed-off-by: Jim Wilson 
---
 linux-user/riscv/signal.c |  5 ++-
 target/riscv/cpu.h|  7 +++-
 target/riscv/cpu_helper.c |  4 +-
 target/riscv/gdbstub.c|  4 +-
 target/riscv/op_helper.c  | 93 ---
 5 files changed, 76 insertions(+), 37 deletions(-)

diff --git a/linux-user/riscv/signal.c b/linux-user/riscv/signal.c
index f598d41..ed76f23 100644
--- a/linux-user/riscv/signal.c
+++ b/linux-user/riscv/signal.c
@@ -83,7 +83,8 @@ static void setup_sigcontext(struct target_sigcontext *sc, 
CPURISCVState *env)
 __put_user(env->fpr[i], >fpr[i]);
 }
 
-uint32_t fcsr = csr_read_helper(env, CSR_FCSR); /*riscv_get_fcsr(env);*/
+/*riscv_get_fcsr(env);*/
+uint32_t fcsr = csr_read_helper(env, CSR_FCSR, false);
 __put_user(fcsr, >fcsr);
 }
 
@@ -159,7 +160,7 @@ static void restore_sigcontext(CPURISCVState *env, struct 
target_sigcontext *sc)
 
 uint32_t fcsr;
 __get_user(fcsr, >fcsr);
-csr_write_helper(env, fcsr, CSR_FCSR);
+csr_write_helper(env, fcsr, CSR_FCSR, false);
 }
 
 static void restore_ucontext(CPURISCVState *env, struct target_ucontext *uc)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 4ee09b9..29361ca 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -290,8 +290,11 @@ static inline void cpu_get_tb_cpu_state(CPURISCVState 
*env, target_ulong *pc,
 }
 
 void csr_write_helper(CPURISCVState *env, target_ulong val_to_write,
-target_ulong csrno);
-target_ulong csr_read_helper(CPURISCVState *env, target_ulong csrno);
+  target_ulong csrno, bool debugger);
+target_ulong csr_read_helper(CPURISCVState *env, target_ulong csrno,
+ bool debugger);
+
+void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
 
 #include "exec/cpu-all.h"
 
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 86f9f47..1abad94 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -526,7 +526,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
 get_field(s, MSTATUS_SIE) : get_field(s, MSTATUS_UIE << 
env->priv));
 s = set_field(s, MSTATUS_SPP, env->priv);
 s = set_field(s, MSTATUS_SIE, 0);
-csr_write_helper(env, s, CSR_MSTATUS);
+csr_write_helper(env, s, CSR_MSTATUS, false);
 riscv_set_mode(env, PRV_S);
 } else {
 /* No need to check MTVEC for misaligned - lower 2 bits cannot be set 
*/
@@ -551,7 +551,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
 get_field(s, MSTATUS_MIE) : get_field(s, MSTATUS_UIE << 
env->priv));
 s = set_field(s, MSTATUS_MPP, env->priv);
 s = set_field(s, MSTATUS_MIE, 0);
-csr_write_helper(env, s, CSR_MSTATUS);
+csr_write_helper(env, s, CSR_MSTATUS, false);
 riscv_set_mode(env, PRV_M);
 }
 /* TODO yield load reservation  */
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index 71c3eb1..b06f0fa 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -34,7 +34,7 @@ int riscv_cpu_gdb_read_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 } else if (n < 65) {
 return gdb_get_reg64(mem_buf, env->fpr[n - 33]);
 } else if (n < 4096 + 65) {
-return gdb_get_regl(mem_buf, csr_read_helper(env, n - 65));
+return gdb_get_regl(mem_buf, csr_read_helper(env, n - 65, true));
 }
 return 0;
 }
@@ -57,7 +57,7 @@ int riscv_cpu_gdb_write_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 env->fpr[n - 33] = ldq_p(mem_buf); /* always 64-bit */
 return sizeof(uint64_t);
 } else if (n < 4096 + 65) {
-csr_write_helper(env, ldtul_p(mem_buf), n - 65);
+csr_write_helper(env, ldtul_p(mem_buf), n - 65, true);
 }
 return 0;
 }
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 3726299..3c5641e 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -87,7 +87,7 @@ static void validate_mstatus_fs(CPURISCVState *env, uintptr_t 
ra)
  * Adapted from Spike's processor_t::set_csr
  */
 void csr_write_helper(CPURISCVState *env, target_ulong val_to_write,
-target_ulong csrno)
+  target_ulong csrno, bool debugger)
 {
 #ifndef CONFIG_USER_ONLY
 uint64_t delegable_ints = MIP_SSIP | MIP_STIP | MIP_SEIP;
@@ -96,15 +96,21 @@ void csr_write_helper(CPURISCVState *env, target_ulong 
val_to_write,
 
 switch (csrno) {
 case CSR_FFLAGS:
-validate_mstatus_fs(env, GETPC());
+if (!debugger) {
+validate_mstatus_fs(env, GETPC());
+}
 cpu_riscv_set_fflags(env, val_to_write & (FSR_AEXC >> FSR_AEXC_SHIFT));
 break;
 case CSR_FRM:
-validate_mstatus_fs(env, GETPC());
+if (!debugger) {
+

[Qemu-devel] [PATCH 2/5 v2] RISC-V: Add 64-bit gdb xml files.

[Jim Wilson]

Signed-off-by: Jim Wilson 
---
 configure   |   1 +
 gdb-xml/riscv-64bit-cpu.xml |  43 
 gdb-xml/riscv-64bit-csr.xml | 250 
 gdb-xml/riscv-64bit-fpu.xml |  52 +
 4 files changed, 346 insertions(+)
 create mode 100644 gdb-xml/riscv-64bit-cpu.xml
 create mode 100644 gdb-xml/riscv-64bit-csr.xml
 create mode 100644 gdb-xml/riscv-64bit-fpu.xml

diff --git a/configure b/configure
index 4e05eed..00b7495 100755
--- a/configure
+++ b/configure
@@ -7215,6 +7215,7 @@ case "$target_name" in
 TARGET_BASE_ARCH=riscv
 TARGET_ABI_DIR=riscv
 mttcg=yes
+gdb_xml_files="riscv-64bit-cpu.xml riscv-64bit-fpu.xml riscv-64bit-csr.xml"
 target_compiler=$cross_cc_riscv64
   ;;
   sh4|sh4eb)
diff --git a/gdb-xml/riscv-64bit-cpu.xml b/gdb-xml/riscv-64bit-cpu.xml
new file mode 100644
index 000..f37d7f3
--- /dev/null
+++ b/gdb-xml/riscv-64bit-cpu.xml
@@ -0,0 +1,43 @@
+
+
+
+
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
diff --git a/gdb-xml/riscv-64bit-csr.xml b/gdb-xml/riscv-64bit-csr.xml
new file mode 100644
index 000..a3de834
--- /dev/null
+++ b/gdb-xml/riscv-64bit-csr.xml
@@ -0,0 +1,250 @@
+
+
+
+
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
diff --git a/gdb-xml/riscv-64bit-fpu.xml b/gdb-xml/riscv-64bit-fpu.xml
new file mode 100644
index 000..fb24b72
--- /dev/null
+++ b/gdb-xml/riscv-64bit-fpu.xml
@@ -0,0 +1,52 @@
+
+
+
+
+
+
+  
+
+
+  
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
+  
+  
+  
+
-- 
2.7.4

[Qemu-devel] [PATCH 5/5 v2] RISC-V: Add hooks to use the gdb xml files.

[Jim Wilson]

Signed-off-by: Jim Wilson 
---
 target/riscv/cpu.c |  9 ++-
 target/riscv/gdbstub.c | 73 --
 2 files changed, 73 insertions(+), 9 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index a025a0a..b248e3e 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -305,6 +305,8 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
+riscv_cpu_register_gdb_regs_for_features(cs);
+
 qemu_init_vcpu(cs);
 cpu_reset(cs);
 
@@ -345,7 +347,12 @@ static void riscv_cpu_class_init(ObjectClass *c, void 
*data)
 cc->synchronize_from_tb = riscv_cpu_synchronize_from_tb;
 cc->gdb_read_register = riscv_cpu_gdb_read_register;
 cc->gdb_write_register = riscv_cpu_gdb_write_register;
-cc->gdb_num_core_regs = 65;
+cc->gdb_num_core_regs = 33;
+#if defined(TARGET_RISCV32)
+cc->gdb_core_xml_file = "riscv-32bit-cpu.xml";
+#elif defined(TARGET_RISCV64)
+cc->gdb_core_xml_file = "riscv-64bit-cpu.xml";
+#endif
 cc->gdb_stop_before_watchpoint = true;
 cc->disas_set_info = riscv_cpu_disas_set_info;
 #ifdef CONFIG_USER_ONLY
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index b06f0fa..9558d80 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -31,10 +31,6 @@ int riscv_cpu_gdb_read_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 return gdb_get_regl(mem_buf, env->gpr[n]);
 } else if (n == 32) {
 return gdb_get_regl(mem_buf, env->pc);
-} else if (n < 65) {
-return gdb_get_reg64(mem_buf, env->fpr[n - 33]);
-} else if (n < 4096 + 65) {
-return gdb_get_regl(mem_buf, csr_read_helper(env, n - 65, true));
 }
 return 0;
 }
@@ -53,11 +49,72 @@ int riscv_cpu_gdb_write_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 } else if (n == 32) {
 env->pc = ldtul_p(mem_buf);
 return sizeof(target_ulong);
-} else if (n < 65) {
-env->fpr[n - 33] = ldq_p(mem_buf); /* always 64-bit */
+}
+return 0;
+}
+
+static int riscv_gdb_get_fpu(CPURISCVState *env, uint8_t *mem_buf, int n)
+{
+if (n < 32) {
+return gdb_get_reg64(mem_buf, env->fpr[n]);
+} else if (n < 35) {
+/*
+ * CSR_FFLAGS is 0x001, and gdb says it is FP register 32, so we
+ * subtract 31 to map the gdb FP register number to the CSR number.
+ * This also works for CSR_FRM and CSR_FCSR.
+ */
+return gdb_get_regl(mem_buf, csr_read_helper(env, n - 31, true));
+}
+return 0;
+}
+
+static int riscv_gdb_set_fpu(CPURISCVState *env, uint8_t *mem_buf, int n)
+{
+if (n < 32) {
+env->fpr[n] = ldq_p(mem_buf); /* always 64-bit */
 return sizeof(uint64_t);
-} else if (n < 4096 + 65) {
-csr_write_helper(env, ldtul_p(mem_buf), n - 65, true);
+} else if (n < 35) {
+/*
+ * CSR_FFLAGS is 0x001, and gdb says it is FP register 32, so we
+ * subtract 31 to map the gdb FP register number to the CSR number.
+ * This also works for CSR_FRM and CSR_FCSR.
+ */
+csr_write_helper(env, ldtul_p(mem_buf), n - 31, true);
 }
 return 0;
 }
+
+static int riscv_gdb_get_csr(CPURISCVState *env, uint8_t *mem_buf, int n)
+{
+if (n < ARRAY_SIZE(csr_register_map)) {
+return gdb_get_regl(mem_buf, csr_read_helper(env, csr_register_map[n],
+ true));
+}
+return 0;
+}
+
+static int riscv_gdb_set_csr(CPURISCVState *env, uint8_t *mem_buf, int n)
+{
+if (n < ARRAY_SIZE(csr_register_map)) {
+csr_write_helper(env, ldtul_p(mem_buf), csr_register_map[n], true);
+}
+return 0;
+}
+
+void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
+{
+/* ??? Assume all targets have FPU regs for now.  */
+#if defined(TARGET_RISCV32)
+gdb_register_coprocessor(cs, riscv_gdb_get_fpu, riscv_gdb_set_fpu,
+ 35, "riscv-32bit-fpu.xml", 0);
+
+gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr,
+ 4096, "riscv-32bit-csr.xml", 0);
+#elif defined(TARGET_RISCV64)
+gdb_register_coprocessor(cs, riscv_gdb_get_fpu, riscv_gdb_set_fpu,
+ 35, "riscv-64bit-fpu.xml", 0);
+
+gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr,
+ 4096, "riscv-64bit-csr.xml", 0);
+#endif
+}
-- 
2.7.4

[Qemu-devel] [PATCH 3/5 v2] RISC-V: Map gdb CSR reg numbers to hw reg numbers.

[Jim Wilson]

The gdb CSR xml file has registers in documentation order, not numerical
order, so we need a table to map the register numbers.  This also adds
some missing CSR_* register macros.

Signed-off-by: Jim Wilson 
---
 target/riscv/cpu_bits.h |  35 ++-
 target/riscv/csr-map.h  | 248 
 target/riscv/gdbstub.c  |   1 +
 3 files changed, 282 insertions(+), 2 deletions(-)
 create mode 100644 target/riscv/csr-map.h

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 5439f47..316d500 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -135,16 +135,22 @@
 /* Legacy Counter Setup (priv v1.9.1) */
 #define CSR_MUCOUNTEREN 0x320
 #define CSR_MSCOUNTEREN 0x321
+#define CSR_MHCOUNTEREN 0x322
 
 /* Machine Trap Handling */
 #define CSR_MSCRATCH0x340
 #define CSR_MEPC0x341
 #define CSR_MCAUSE  0x342
-#define CSR_MBADADDR0x343
+#define CSR_MTVAL   0x343
 #define CSR_MIP 0x344
 
+/* Legacy Machine Trap Handling (priv v1.9.1) */
+#define CSR_MBADADDR0x343
+
 /* Supervisor Trap Setup */
 #define CSR_SSTATUS 0x100
+#define CSR_SEDELEG 0x102
+#define CSR_SIDELEG 0x103
 #define CSR_SIE 0x104
 #define CSR_STVEC   0x105
 #define CSR_SCOUNTEREN  0x106
@@ -153,9 +159,12 @@
 #define CSR_SSCRATCH0x140
 #define CSR_SEPC0x141
 #define CSR_SCAUSE  0x142
-#define CSR_SBADADDR0x143
+#define CSR_STVAL   0x143
 #define CSR_SIP 0x144
 
+/* Legacy Supervisor Trap Handling (priv v1.9.1) */
+#define CSR_SBADADDR0x143
+
 /* Supervisor Protection and Translation */
 #define CSR_SPTBR   0x180
 #define CSR_SATP0x180
@@ -282,6 +291,28 @@
 #define CSR_MHPMCOUNTER30H  0xb9e
 #define CSR_MHPMCOUNTER31H  0xb9f
 
+/* Legacy Hypervisor Trap Setup (priv v1.9.1) */
+#define CSR_HSTATUS 0x200
+#define CSR_HEDELEG 0x202
+#define CSR_HIDELEG 0x203
+#define CSR_HIE 0x204
+#define CSR_HTVEC   0x205
+
+/* Legacy Hypervisor Trap Handling (priv v1.9.1) */
+#define CSR_HSCRATCH0x240
+#define CSR_HEPC0x241
+#define CSR_HCAUSE  0x242
+#define CSR_HBADADDR0x243
+#define CSR_HIP 0x244
+
+/* Legacy Machine Protection and Translation (priv v1.9.1) */
+#define CSR_MBASE   0x380
+#define CSR_MBOUND  0x381
+#define CSR_MIBASE  0x382
+#define CSR_MIBOUND 0x383
+#define CSR_MDBASE  0x384
+#define CSR_MDBOUND 0x385
+
 /* mstatus CSR bits */
 #define MSTATUS_UIE 0x0001
 #define MSTATUS_SIE 0x0002
diff --git a/target/riscv/csr-map.h b/target/riscv/csr-map.h
new file mode 100644
index 000..cce32fd
--- /dev/null
+++ b/target/riscv/csr-map.h
@@ -0,0 +1,248 @@
+/*
+ * The GDB CSR xml files list them in documentation order, not numerical order,
+ * and are missing entries for unnamed CSRs.  So we need to map the gdb numbers
+ * to the hardware numbers.
+ */
+
+int csr_register_map[] = {
+CSR_USTATUS,
+CSR_UIE,
+CSR_UTVEC,
+CSR_USCRATCH,
+CSR_UEPC,
+CSR_UCAUSE,
+CSR_UTVAL,
+CSR_UIP,
+CSR_FFLAGS,
+CSR_FRM,
+CSR_FCSR,
+CSR_CYCLE,
+CSR_TIME,
+CSR_INSTRET,
+CSR_HPMCOUNTER3,
+CSR_HPMCOUNTER4,
+CSR_HPMCOUNTER5,
+CSR_HPMCOUNTER6,
+CSR_HPMCOUNTER7,
+CSR_HPMCOUNTER8,
+CSR_HPMCOUNTER9,
+CSR_HPMCOUNTER10,
+CSR_HPMCOUNTER11,
+CSR_HPMCOUNTER12,
+CSR_HPMCOUNTER13,
+CSR_HPMCOUNTER14,
+CSR_HPMCOUNTER15,
+CSR_HPMCOUNTER16,
+CSR_HPMCOUNTER17,
+CSR_HPMCOUNTER18,
+CSR_HPMCOUNTER19,
+CSR_HPMCOUNTER20,
+CSR_HPMCOUNTER21,
+CSR_HPMCOUNTER22,
+CSR_HPMCOUNTER23,
+CSR_HPMCOUNTER24,
+CSR_HPMCOUNTER25,
+CSR_HPMCOUNTER26,
+CSR_HPMCOUNTER27,
+CSR_HPMCOUNTER28,
+CSR_HPMCOUNTER29,
+CSR_HPMCOUNTER30,
+CSR_HPMCOUNTER31,
+CSR_CYCLEH,
+CSR_TIMEH,
+CSR_INSTRETH,
+CSR_HPMCOUNTER3H,
+CSR_HPMCOUNTER4H,
+CSR_HPMCOUNTER5H,
+CSR_HPMCOUNTER6H,
+CSR_HPMCOUNTER7H,
+CSR_HPMCOUNTER8H,
+CSR_HPMCOUNTER9H,
+CSR_HPMCOUNTER10H,
+CSR_HPMCOUNTER11H,
+CSR_HPMCOUNTER12H,
+CSR_HPMCOUNTER13H,
+CSR_HPMCOUNTER14H,
+CSR_HPMCOUNTER15H,
+CSR_HPMCOUNTER16H,
+CSR_HPMCOUNTER17H,
+CSR_HPMCOUNTER18H,
+CSR_HPMCOUNTER19H,
+CSR_HPMCOUNTER20H,
+CSR_HPMCOUNTER21H,
+CSR_HPMCOUNTER22H,
+CSR_HPMCOUNTER23H,
+CSR_HPMCOUNTER24H,
+CSR_HPMCOUNTER25H,
+CSR_HPMCOUNTER26H,
+CSR_HPMCOUNTER27H,
+CSR_HPMCOUNTER28H,
+CSR_HPMCOUNTER29H,
+CSR_HPMCOUNTER30H,
+CSR_HPMCOUNTER31H,
+CSR_SSTATUS,
+CSR_SEDELEG,
+CSR_SIDELEG,
+CSR_SIE,
+CSR_STVEC,
+CSR_SCOUNTEREN,
+CSR_SSCRATCH,
+CSR_SEPC,
+CSR_SCAUSE,
+CSR_STVAL,
+CSR_SIP,
+CSR_SATP,
+CSR_MVENDORID,
+CSR_MARCHID,
+CSR_MIMPID

Re: [Qemu-devel] [PATCH v1 06/21] RISC-V FPU Support

[Jim Wilson]

On Wed, Jan 24, 2018 at 3:47 PM, Richard Henderson
<richard.hender...@linaro.org> wrote:
> On 01/24/2018 10:58 AM, Jim Wilson wrote:
>> Although, looking at this again, I see another statement in a
>> different place that says:
>>
>> Except when otherwise stated, if the result of a floating-point operation is
>> NaN, it is the canonical NaN. The canonical NaN has a positive sign and all
>> significand bits clear except the MSB, a.k.a. the quiet bit. For
>> single-precision floating-point, this corresponds to the pattern
>> 0x7fc0.
> Yes, I had read this before as well.  I had assumed that your patch 
> constituted
> an intended change to this text.
>
>> This could take a little time to sort out.
> Ok.  I don't see this as a blocking issue for merging.

So after looking at this a bit more, I've come to the conclusion that
my patch to remove the default/canonical nan support from RISC-V qemu
was wrong.  We will have to fix this on the gcc/glibc side.

Michael, please revert my change

https://github.com/riscv/riscv-qemu/commit/4223d89b0c5c671332d66bcd649db5c6f46559f5

Jim

Re: [Qemu-devel] [PATCH v1 06/21] RISC-V FPU Support

[Jim Wilson]

On Tue, Jan 23, 2018 at 4:15 PM, Richard Henderson
 wrote:
> Ok.  Now it depends on what result you care about for madd specifically.
>
> If, like x86 and Power, fmsub returns the (silenced) original input NaN, you
> want the float_muladd_* flags.
>
> If, like ARM, fmsub returns the (silenced) negated input NaN, then you do need
> to change sign externally.  If this is the case, please use float32_chs 
> instead
> of open-coding it with xor.

The ISA spec is a little ambiguous here.  There is text that says we
multiply, optionally negate the result, and then add or subtract the
addend.  However, this is followed by a sentence that gives equations,
and for fnmsub it says -rs1*rs2+rs3.  If we assume C semantics, then
they are negating rs1 not the multiply result.  This could potentially
give a different result if rs2 is a +qNaN and rs1/rs3 are not NaNs.
qemu is implementing what the equation says, not what the text says.
The definitive source would be the Spike simulator, and it agrees with
qemu and the equations.  The ISA spec does not specify what happens
when one of the operands is a NaN, except to mention that operations
comply with the IEEE 754 2008 standard.

I think that qemu is correct here, and that you want to use float32_chs.

Although, looking at this again, I see another statement in a
different place that says:

Except when otherwise stated, if the result of a floating-point
operation is NaN, it is the canonical
NaN. The canonical NaN has a positive sign and all significand bits
clear except the MSB, a.k.a.
the quiet bit. For single-precision floating-point, this corresponds
to the pattern 0x7fc0.

So it sounds like maybe we do want default NaN support.  It appears
that spike is using default NaNs.  Unfortunately, enabling default NaN
support causes gcc and glibc testsuite failures which complicates
upstreaming glibc support, as they won't accept it unless we get
failures below a certain number.  Also, not having hardware to compare
against makes it impossible to determine if the simulator is doing
exactly what the hardware does.  This could take a little time to sort
out.

Jim

Re: [Qemu-devel] [PATCH v1 06/21] RISC-V FPU Support

[Jim Wilson]

On Tue, Jan 23, 2018 at 3:35 PM, Michael Clark  wrote:
> We want minimum number (minnum). It's been added to the draft spec and will
> be in riscv-spec-v2.3.pdf

In the preface of the draft, it says
• Defined the signed-zero behavior of FMIN.fmt and FMAX.fmt, and
changed their behavior on
signaling-NaN inputs to conform to the minimumNumber and maximumNumber
operations
in the proposed IEEE 754-201x specification.

But if qemu doesn't have minimumNumber support yet, then yes minNum is correct.

This is discussed a bit here
https://github.com/riscv/riscv-isa-manual/issues/65

Jim