[PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver

Konrad Schwarz posted 5 patches 4 years, 1 month ago
Maintainers: "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Alistair Francis <alistair.francis@wdc.com>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Palmer Dabbelt <palmer@dabbelt.com>, "Alex Bennée" <alex.bennee@linaro.org>, Bin Meng <bin.meng@windriver.com>
[PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver
Posted by Konrad Schwarz 4 years, 1 month ago
GDB target descriptions support typed registers;
such that `info register X' displays not only the hex value of
register `X', but also the individual bitfields the register
comprises (if any), using textual labels if possible.

This patch includes type information for GDB for
a large subset of the RISC-V Control and Status Registers (CSRs).

Signed-off-by: Konrad Schwarz <konrad.schwarz@siemens.com>
---
 target/riscv/csr.c                |   2 +
 target/riscv/csr32-op-gdbserver.h | 109 ++++++++++
 target/riscv/csr64-op-gdbserver.h |  76 +++++++
 target/riscv/gdb_csr_type_group.c |  16 ++
 target/riscv/gdb_csr_type_group.h |   3 +
 target/riscv/gdb_csr_types.c      | 333 ++++++++++++++++++++++++++++++
 target/riscv/gdb_csr_types.h      |   3 +
 target/riscv/gdbstub.c            |  26 ++-
 target/riscv/meson.build          |   4 +-
 9 files changed, 566 insertions(+), 6 deletions(-)
 create mode 100644 target/riscv/csr32-op-gdbserver.h
 create mode 100644 target/riscv/csr64-op-gdbserver.h
 create mode 100644 target/riscv/gdb_csr_type_group.c
 create mode 100644 target/riscv/gdb_csr_type_group.h
 create mode 100644 target/riscv/gdb_csr_types.c
 create mode 100644 target/riscv/gdb_csr_types.h

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 9f41954894..557b4afe0e 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3,6 +3,7 @@
  *
  * Copyright (c) 2016-2017 Sagar Karandikar, sagark@eecs.berkeley.edu
  * Copyright (c) 2017-2018 SiFive, Inc.
+ * Copyright (c) 2021 Siemens AG, konrad.schwarz@siemens.com
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -2094,5 +2095,6 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     [CSR_MHPMCOUNTER29H] = { "mhpmcounter29h", any32,  read_zero },
     [CSR_MHPMCOUNTER30H] = { "mhpmcounter30h", any32,  read_zero },
     [CSR_MHPMCOUNTER31H] = { "mhpmcounter31h", any32,  read_zero },
+
 #endif /* !CONFIG_USER_ONLY */
 };
diff --git a/target/riscv/csr32-op-gdbserver.h b/target/riscv/csr32-op-gdbserver.h
new file mode 100644
index 0000000000..e8ec527f23
--- /dev/null
+++ b/target/riscv/csr32-op-gdbserver.h
@@ -0,0 +1,109 @@
+/* Copyright (c) 2021 Siemens AG, konrad.schwarz@siemens.com */
+
+  [CSR_USTATUS] { .gdb_type = "sstatus-fields", .gdb_group = "user" },
+  [CSR_UIE] { .gdb_type = "sie-fields", .gdb_group = "user" },
+  [CSR_UTVEC] { .gdb_type = "code_ptr", .gdb_group = "user" },
+  [CSR_USCRATCH] { .gdb_type = "data_ptr", .gdb_group = "user" },
+  [CSR_UEPC] { .gdb_type = "code_ptr", .gdb_group = "user" },
+  [CSR_UCAUSE] { .gdb_type = "scause-fields", .gdb_group = "user" },
+  [CSR_UTVAL] { .gdb_type = "data_ptr", .gdb_group = "user" },
+  [CSR_UIP] { .gdb_type = "code_ptr", .gdb_group = "user" },
+  [CSR_CYCLE] { .gdb_type = "uint32", .gdb_group = "user" },
+  [CSR_TIME] { .gdb_type = "uint32", .gdb_group = "user" },
+  [CSR_INSTRET] { .gdb_type = "uint32", .gdb_group = "user" },
+  [CSR_HPMCOUNTER3] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER4] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER5] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER6] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER7] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER8] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER9] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER10] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER11] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER12] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER13] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER14] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER15] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER16] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER17] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER18] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER19] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER20] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER21] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER22] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER23] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER24] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER25] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER26] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER27] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER28] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER29] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER30] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER31] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_CYCLEH] { .gdb_type = "uint32", .gdb_group = "user" },
+  [CSR_TIMEH] { .gdb_type = "uint32", .gdb_group = "user" },
+  [CSR_INSTRETH] { .gdb_type = "uint32", .gdb_group = "user" },
+  [CSR_HPMCOUNTER3H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER4H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER5H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER6H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER7H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER8H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER9H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER10H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER11H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER12H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER13H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER14H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER15H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER16H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER17H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER18H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER19H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER20H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER21H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER22H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER23H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER24H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER25H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER26H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER27H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER28H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER29H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER30H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER31H] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_SSTATUS] { .gdb_type = "sstatus-fields", .gdb_group = "supervisor" },
+  [CSR_SEDELEG] { .gdb_type = "uint32", .gdb_group = "supervisor" },
+  [CSR_SIDELEG] { .gdb_type = "sie-fields", .gdb_group = "supervisor" },
+  [CSR_SIE] { .gdb_type = "sie-fields", .gdb_group = "supervisor" },
+  [CSR_STVEC] { .gdb_type = "stvec-fields", .gdb_group = "supervisor" },
+  [CSR_SCOUNTEREN] { .gdb_type = "scounteren-fields", .gdb_group = "supervisor" },
+  [CSR_SSCRATCH] { .gdb_type = "data_ptr", .gdb_group = "supervisor" },
+  [CSR_SEPC] { .gdb_type = "code_ptr", .gdb_group = "supervisor" },
+  [CSR_SCAUSE] { .gdb_type = "scause-fields", .gdb_group = "supervisor" },
+  [CSR_STVAL] { .gdb_type = "data_ptr", .gdb_group = "supervisor" },
+  [CSR_SIP] { .gdb_type = "sip-fields", .gdb_group = "supervisor" },
+  [CSR_SATP] { .gdb_type = "satp-fields", .gdb_group = "supervisor" },
+  [CSR_HSTATUS] { .gdb_type = "hstatus-fields", .gdb_group = "hypervisor" },
+  [CSR_HEDELEG] { .gdb_type = "hedeleg-fields", .gdb_group = "hypervisor" },
+  [CSR_HIDELEG] { .gdb_type = "hie-fields", .gdb_group = "hypervisor" },
+  [CSR_HIE] { .gdb_type = "hie-fields", .gdb_group = "hypervisor" },
+  [CSR_HCOUNTEREN] { .gdb_type = "int", .gdb_group = "hypervisor" },
+  [CSR_HGEIE] { .gdb_type = "uint32", .gdb_group = "hypervisor" },
+  [CSR_HGEIP] { .gdb_type = "uint32", .gdb_group = "hypervisor" },
+  [CSR_HTVAL] { .gdb_type = "data_ptr", .gdb_group = "hypervisor" },
+  [CSR_HIP] { .gdb_type = "hip-fields", .gdb_group = "hypervisor" },
+  [CSR_HVIP] { .gdb_type = "hvip-fields", .gdb_group = "hypervisor" },
+  [CSR_HGATP] { .gdb_type = "hgatp-fields", .gdb_group = "hypervisor" },
+  [CSR_HTIMEDELTA] { .gdb_type = "int", .gdb_group = "hypervisor" },
+  [CSR_HTIMEDELTAH] { .gdb_type = "int", .gdb_group = "hypervisor" },
+  [CSR_HTINST] { .gdb_type = "uint32", .gdb_group = "hypervisor" },
+  [CSR_VSSTATUS] { .gdb_type = "sstatus-fields", .gdb_group = "virtual-supervisor" },
+  [CSR_VSIE] { .gdb_type = "sie-fields", .gdb_group = "virtual-supervisor" },
+  [CSR_VSTVEC] { .gdb_type = "stvec-fields", .gdb_group = "virtual-supervisor" },
+  [CSR_VSSCRATCH] { .gdb_type = "data_ptr", .gdb_group = "virtual-supervisor" },
+  [CSR_VSEPC] { .gdb_type = "code_ptr", .gdb_group = "virtual-supervisor" },
+  [CSR_VSCAUSE] { .gdb_type = "scause-fields", .gdb_group = "virtual-supervisor" },
+  [CSR_VSTVAL] { .gdb_type = "data_ptr", .gdb_group = "virtual-supervisor" },
+  [CSR_VSIP] { .gdb_type = "sip-fields", .gdb_group = "virtual-supervisor" },
+  [CSR_VSATP] { .gdb_type = "satp-fields", .gdb_group = "virtual-supervisor" },
diff --git a/target/riscv/csr64-op-gdbserver.h b/target/riscv/csr64-op-gdbserver.h
new file mode 100644
index 0000000000..fc4bc62d9e
--- /dev/null
+++ b/target/riscv/csr64-op-gdbserver.h
@@ -0,0 +1,76 @@
+/* Copyright (c) 2021 Siemens AG, konrad.schwarz@siemens.com */
+
+  [CSR_USTATUS] { .gdb_type = "sstatus-fields", .gdb_group = "user" },
+  [CSR_UIE] { .gdb_type = "sie-fields", .gdb_group = "user" },
+  [CSR_UTVEC] { .gdb_type = "code_ptr", .gdb_group = "user" },
+  [CSR_USCRATCH] { .gdb_type = "data_ptr", .gdb_group = "user" },
+  [CSR_UEPC] { .gdb_type = "code_ptr", .gdb_group = "user" },
+  [CSR_UCAUSE] { .gdb_type = "scause-fields", .gdb_group = "user" },
+  [CSR_UTVAL] { .gdb_type = "data_ptr", .gdb_group = "user" },
+  [CSR_UIP] { .gdb_type = "code_ptr", .gdb_group = "user" },
+  [CSR_CYCLE] { .gdb_type = "uint64", .gdb_group = "user" },
+  [CSR_TIME] { .gdb_type = "uint64", .gdb_group = "user" },
+  [CSR_INSTRET] { .gdb_type = "uint64", .gdb_group = "user" },
+  [CSR_HPMCOUNTER3] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER4] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER5] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER6] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER7] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER8] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER9] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER10] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER11] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER12] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER13] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER14] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER15] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER16] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER17] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER18] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER19] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER20] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER21] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER22] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER23] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER24] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER25] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER26] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER27] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER28] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER29] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER30] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_HPMCOUNTER31] { .gdb_type = "int", .gdb_group = "user" },
+  [CSR_SSTATUS] { .gdb_type = "sstatus-fields", .gdb_group = "supervisor" },
+  [CSR_SEDELEG] { .gdb_type = "uint64", .gdb_group = "supervisor" },
+  [CSR_SIDELEG] { .gdb_type = "sie-fields", .gdb_group = "supervisor" },
+  [CSR_SIE] { .gdb_type = "sie-fields", .gdb_group = "supervisor" },
+  [CSR_STVEC] { .gdb_type = "stvec-fields", .gdb_group = "supervisor" },
+  [CSR_SCOUNTEREN] { .gdb_type = "scounteren-fields", .gdb_group = "supervisor" },
+  [CSR_SSCRATCH] { .gdb_type = "data_ptr", .gdb_group = "supervisor" },
+  [CSR_SEPC] { .gdb_type = "code_ptr", .gdb_group = "supervisor" },
+  [CSR_SCAUSE] { .gdb_type = "scause-fields", .gdb_group = "supervisor" },
+  [CSR_STVAL] { .gdb_type = "data_ptr", .gdb_group = "supervisor" },
+  [CSR_SIP] { .gdb_type = "sip-fields", .gdb_group = "supervisor" },
+  [CSR_SATP] { .gdb_type = "satp-fields", .gdb_group = "supervisor" },
+  [CSR_HSTATUS] { .gdb_type = "hstatus-fields", .gdb_group = "hypervisor" },
+  [CSR_HEDELEG] { .gdb_type = "hedeleg-fields", .gdb_group = "hypervisor" },
+  [CSR_HIDELEG] { .gdb_type = "hie-fields", .gdb_group = "hypervisor" },
+  [CSR_HIE] { .gdb_type = "hie-fields", .gdb_group = "hypervisor" },
+  [CSR_HCOUNTEREN] { .gdb_type = "int", .gdb_group = "hypervisor" },
+  [CSR_HGEIE] { .gdb_type = "uint64", .gdb_group = "hypervisor" },
+  [CSR_HGEIP] { .gdb_type = "uint64", .gdb_group = "hypervisor" },
+  [CSR_HTVAL] { .gdb_type = "data_ptr", .gdb_group = "hypervisor" },
+  [CSR_HIP] { .gdb_type = "hip-fields", .gdb_group = "hypervisor" },
+  [CSR_HVIP] { .gdb_type = "hvip-fields", .gdb_group = "hypervisor" },
+  [CSR_HGATP] { .gdb_type = "hgatp-fields", .gdb_group = "hypervisor" },
+  [CSR_HTIMEDELTA] { .gdb_type = "int", .gdb_group = "hypervisor" },
+  [CSR_HTINST] { .gdb_type = "uint64", .gdb_group = "hypervisor" },
+  [CSR_VSSTATUS] { .gdb_type = "sstatus-fields", .gdb_group = "virtual-supervisor" },
+  [CSR_VSIE] { .gdb_type = "sie-fields", .gdb_group = "virtual-supervisor" },
+  [CSR_VSTVEC] { .gdb_type = "stvec-fields", .gdb_group = "virtual-supervisor" },
+  [CSR_VSSCRATCH] { .gdb_type = "data_ptr", .gdb_group = "virtual-supervisor" },
+  [CSR_VSEPC] { .gdb_type = "code_ptr", .gdb_group = "virtual-supervisor" },
+  [CSR_VSCAUSE] { .gdb_type = "scause-fields", .gdb_group = "virtual-supervisor" },
+  [CSR_VSTVAL] { .gdb_type = "data_ptr", .gdb_group = "virtual-supervisor" },
+  [CSR_VSIP] { .gdb_type = "sip-fields", .gdb_group = "virtual-supervisor" },
+  [CSR_VSATP] { .gdb_type = "satp-fields", .gdb_group = "virtual-supervisor" },
diff --git a/target/riscv/gdb_csr_type_group.c b/target/riscv/gdb_csr_type_group.c
new file mode 100644
index 0000000000..af394de302
--- /dev/null
+++ b/target/riscv/gdb_csr_type_group.c
@@ -0,0 +1,16 @@
+/* Copyright 2021 Siemens AG */
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "gdb_csr_type_group.h"
+
+struct riscv_gdb_csr_tg const riscv_gdb_csr_type_group[] = {
+
+#if !defined(CONFIG_USER_ONLY)
+#  ifdef TARGET_RISCV64
+#    include "csr64-op-gdbserver.h"
+#  elif defined TARGET_RISCV64
+#    include "csr32-op-gdbserver.h"
+#  endif
+#endif /* !CONFIG_USER_ONLY */
+
+};
diff --git a/target/riscv/gdb_csr_type_group.h b/target/riscv/gdb_csr_type_group.h
new file mode 100644
index 0000000000..e044913bd7
--- /dev/null
+++ b/target/riscv/gdb_csr_type_group.h
@@ -0,0 +1,3 @@
+extern struct riscv_gdb_csr_tg {
+    char const *gdb_type, *gdb_group;
+} const riscv_gdb_csr_type_group[CSR_TABLE_SIZE];
diff --git a/target/riscv/gdb_csr_types.c b/target/riscv/gdb_csr_types.c
new file mode 100644
index 0000000000..48b1db2b88
--- /dev/null
+++ b/target/riscv/gdb_csr_types.c
@@ -0,0 +1,333 @@
+/* Copyright (c) 2021 Siemens AG, konrad.schwarz@siemens.com */
+
+#include "qemu/osdep.h"
+#include "gdb_csr_types.h"
+#define STR(X) #X
+
+char const riscv_gdb_csr_types[] =
+#ifdef TARGET_RISCV32
+   STR(
+<enum id="sstatus-fs-type" size="4">
+  <evalue name="off" value="0"/>
+  <evalue name="initial" value="1"/>
+  <evalue name="clean" value="2"/>
+  <evalue name="dirty" value="3"/>
+</enum><enum id="sstatus-xs-type" size="4">
+  <evalue name="off" value="0"/>
+  <evalue name="initial" value="1"/>
+  <evalue name="clean" value="2"/>
+  <evalue name="dirty" value="3"/>
+</enum><enum id="sstatus-uxl-type" size="4">
+  <evalue name="32" value="1"/>
+  <evalue name="64" value="2"/>
+  <evalue name="128" value="3"/>
+</enum><enum id="stvec-mode-type" size="4">
+  <evalue name="direct" value="0"/>
+  <evalue name="vectored" value="1"/>
+</enum><enum id="scause-exc-type" size="4">
+  <evalue name="instruction_address_misaligned" value="0"/>
+  <evalue name="instruction_access_fault" value="1"/>
+  <evalue name="illegal_instruction" value="2"/>
+  <evalue name="breakpoint" value="3"/>
+  <evalue name="load_address_misaligned" value="4"/>
+  <evalue name="load_access_fault" value="5"/>
+  <evalue name="store_address_misaligned" value="6"/>
+  <evalue name="store_access_fault" value="7"/>
+  <evalue name="enironment_call_from_U_mode" value="8"/>
+  <evalue name="enironment_call_from_S_mode" value="9"/>
+  <evalue name="enironment_call_from_VS_mode" value="10"/>
+  <evalue name="enironment_call_from_M_mode" value="11"/>
+  <evalue name="instruction_page_fault" value="12"/>
+  <evalue name="load_page_fault" value="13"/>
+  <evalue name="store_page_fault" value="15"/>
+  <evalue name="instruction_guest_page_fault" value="20"/>
+  <evalue name="load_guest_page_fault" value="21"/>
+  <evalue name="virtual_instruction" value="22"/>
+  <evalue name="store_guest_page_fault" value="23"/>
+</enum><enum id="satp-mode-type" size="4">
+  <evalue name="bare" value="0"/>
+  <evalue name="sv32" value="1"/>
+  <evalue name="sv39" value="8"/>
+  <evalue name="sv48" value="9"/>
+  <evalue name="sv57" value="10"/>
+  <evalue name="sv64" value="11"/>
+</enum><enum id="hgatp-mode-type" size="4">
+  <evalue name="bare" value="0"/>
+  <evalue name="sv32x4" value="1"/>
+  <evalue name="sv39x4" value="8"/>
+  <evalue name="sv48x4" value="9"/>
+  <evalue name="sv57x4" value="10"/>
+</enum><flags id="sstatus-fields" size="4">
+  <field name="sie" start="1" end="1"/>
+  <field name="mie" start="3" end="3"/>
+  <field name="spie" start="5" end="5"/>
+  <field name="ube" start="6" end="6"/>
+  <field name="mpie" start="7" end="7"/>
+  <field name="spp" start="8" end="8"/>
+  <field name="mpp" start="11" end="12"/>
+  <field name="fs" start="13" end="14" type="sstatus-fs-type"/>
+  <field name="xs" start="15" end="16" type="sstatus-xs-type"/>
+  <field name="mprv" start="17" end="17"/>
+  <field name="sum" start="18" end="18"/>
+  <field name="mxr" start="19" end="19"/>
+  <field name="tvm" start="20" end="20"/>
+  <field name="tw" start="21" end="21"/>
+  <field name="tsr" start="22" end="23"/>
+  <field name="uxl" start="32" end="33" type="sstatus-uxl-type"/>
+  <field name="sxl" start="34" end="35"/>
+  <field name="sbe" start="36" end="36"/>
+  <field name="mbe" start="37" end="37"/>
+  <field name="gva" start="38" end="38"/>
+  <field name="mpv" start="39" end="39"/>
+  <field name="sd" start="63" end="63"/>
+</flags><flags id="sie-fields" size="4">
+  <field name="ssie" start="1" end="1"/>
+  <field name="vssie" start="2" end="2"/>
+  <field name="msie" start="3" end="3"/>
+  <field name="stie" start="5" end="5"/>
+  <field name="vstie" start="6" end="6"/>
+  <field name="mtie" start="7" end="7"/>
+  <field name="seie" start="9" end="9"/>
+  <field name="vseie" start="10" end="10"/>
+  <field name="meie" start="11" end="11"/>
+  <field name="sgeie" start="12" end="12"/>
+</flags><flags id="stvec-fields" size="4">
+  <field name="mode" start="0" end="1" type="stvec-mode-type"/>
+  <field name="base" start="2" end="63"/>
+</flags><flags id="scounteren-fields" size="4">
+  <field name="cy" start="0" end="0"/>
+  <field name="tm" start="1" end="1"/>
+  <field name="ir" start="2" end="2"/>
+  <field name="hpm" start="3" end="31"/>
+</flags><flags id="scause-fields" size="4">
+  <field name="exc" start="0" end="30" type="scause-exc-type"/>
+  <field name="interrupt" start="31" end="31"/>
+</flags><flags id="sip-fields" size="4">
+  <field name="ssip" start="1" end="1"/>
+  <field name="vssip" start="2" end="2"/>
+  <field name="msip" start="3" end="3"/>
+  <field name="stip" start="5" end="5"/>
+  <field name="vstip" start="6" end="6"/>
+  <field name="mtip" start="7" end="7"/>
+  <field name="seip" start="9" end="9"/>
+  <field name="vseip" start="10" end="10"/>
+  <field name="meip" start="11" end="11"/>
+  <field name="sgeip" start="12" end="12"/>
+</flags><flags id="satp-fields" size="4">
+  <field name="ppn" start="0" end="43"/>
+  <field name="asid" start="44" end="59"/>
+  <field name="mode" start="60" end="63" type="satp-mode-type"/>
+</flags><flags id="hstatus-fields" size="4">
+  <field name="vsbe" start="5" end="5"/>
+  <field name="gva" start="6" end="6"/>
+  <field name="spv" start="7" end="7"/>
+  <field name="spvp" start="8" end="8"/>
+  <field name="hu" start="9" end="9"/>
+  <field name="vgein" start="12" end="17"/>
+  <field name="vtvm" start="20" end="20"/>
+  <field name="vtsr" start="22" end="22"/>
+  <field name="vsxl" start="32" end="33"/>
+</flags><flags id="hedeleg-fields" size="4">
+  <field name="instruction_address_misaligned" start="0" end="0"/>
+  <field name="instruction_access_fault" start="1" end="1"/>
+  <field name="illegal_instruction" start="2" end="2"/>
+  <field name="breakpoint" start="3" end="3"/>
+  <field name="load_address_misaligned" start="4" end="4"/>
+  <field name="load_access_fault" start="5" end="5"/>
+  <field name="store_address_misaligned" start="6" end="6"/>
+  <field name="store_access_fault" start="7" end="7"/>
+  <field name="enironment_call_from_U_mode" start="8" end="8"/>
+  <field name="enironment_call_from_S_mode" start="9" end="9"/>
+  <field name="enironment_call_from_VS_mode" start="10" end="10"/>
+  <field name="enironment_call_from_M_mode" start="11" end="11"/>
+  <field name="instruction_page_fault" start="12" end="12"/>
+  <field name="load_page_fault" start="13" end="13"/>
+  <field name="store_page_fault" start="15" end="15"/>
+  <field name="instruction_guest_page_fault" start="20" end="20"/>
+  <field name="load_guest_page_fault" start="21" end="21"/>
+  <field name="virtual_instruction" start="22" end="22"/>
+  <field name="store_guest_page_fault" start="23" end="23"/>
+</flags><flags id="hie-fields" size="4">
+  <field name="vssie" start="2" end="2"/>
+  <field name="vstie" start="6" end="6"/>
+  <field name="vseie" start="10" end="10"/>
+  <field name="sgeie" start="12" end="12"/>
+</flags><flags id="hip-fields" size="4">
+  <field name="vssip" start="2" end="2"/>
+  <field name="vstip" start="6" end="6"/>
+  <field name="vseip" start="10" end="10"/>
+  <field name="sgeip" start="12" end="12"/>
+</flags><flags id="hvip-fields" size="4">
+  <field name="vssip" start="2" end="2"/>
+  <field name="vstip" start="6" end="6"/>
+  <field name="vseip" start="10" end="10"/>
+</flags><flags id="hgatp-fields" size="4">
+  <field name="ppn" start="0" end="43"/>
+  <field name="vmid" start="44" end="57"/>
+  <field name="mode" start="60" end="63" type="hgatp-mode-type"/>
+</flags>
+)
+#elif defined TARGET_RISCV64
+   STR(
+<enum id="sstatus-fs-type" size="8">
+  <evalue name="off" value="0"/>
+  <evalue name="initial" value="1"/>
+  <evalue name="clean" value="2"/>
+  <evalue name="dirty" value="3"/>
+</enum><enum id="sstatus-xs-type" size="8">
+  <evalue name="off" value="0"/>
+  <evalue name="initial" value="1"/>
+  <evalue name="clean" value="2"/>
+  <evalue name="dirty" value="3"/>
+</enum><enum id="sstatus-uxl-type" size="8">
+  <evalue name="32" value="1"/>
+  <evalue name="64" value="2"/>
+  <evalue name="128" value="3"/>
+</enum><enum id="stvec-mode-type" size="8">
+  <evalue name="direct" value="0"/>
+  <evalue name="vectored" value="1"/>
+</enum><enum id="scause-exc-type" size="8">
+  <evalue name="instruction_address_misaligned" value="0"/>
+  <evalue name="instruction_access_fault" value="1"/>
+  <evalue name="illegal_instruction" value="2"/>
+  <evalue name="breakpoint" value="3"/>
+  <evalue name="load_address_misaligned" value="4"/>
+  <evalue name="load_access_fault" value="5"/>
+  <evalue name="store_address_misaligned" value="6"/>
+  <evalue name="store_access_fault" value="7"/>
+  <evalue name="enironment_call_from_U_mode" value="8"/>
+  <evalue name="enironment_call_from_S_mode" value="9"/>
+  <evalue name="enironment_call_from_VS_mode" value="10"/>
+  <evalue name="enironment_call_from_M_mode" value="11"/>
+  <evalue name="instruction_page_fault" value="12"/>
+  <evalue name="load_page_fault" value="13"/>
+  <evalue name="store_page_fault" value="15"/>
+  <evalue name="instruction_guest_page_fault" value="20"/>
+  <evalue name="load_guest_page_fault" value="21"/>
+  <evalue name="virtual_instruction" value="22"/>
+  <evalue name="store_guest_page_fault" value="23"/>
+</enum><enum id="satp-mode-type" size="8">
+  <evalue name="bare" value="0"/>
+  <evalue name="sv32" value="1"/>
+  <evalue name="sv39" value="8"/>
+  <evalue name="sv48" value="9"/>
+  <evalue name="sv57" value="10"/>
+  <evalue name="sv64" value="11"/>
+</enum><enum id="hgatp-mode-type" size="8">
+  <evalue name="bare" value="0"/>
+  <evalue name="sv32x4" value="1"/>
+  <evalue name="sv39x4" value="8"/>
+  <evalue name="sv48x4" value="9"/>
+  <evalue name="sv57x4" value="10"/>
+</enum><flags id="sstatus-fields" size="8">
+  <field name="sie" start="1" end="1"/>
+  <field name="mie" start="3" end="3"/>
+  <field name="spie" start="5" end="5"/>
+  <field name="ube" start="6" end="6"/>
+  <field name="mpie" start="7" end="7"/>
+  <field name="spp" start="8" end="8"/>
+  <field name="mpp" start="11" end="12"/>
+  <field name="fs" start="13" end="14" type="sstatus-fs-type"/>
+  <field name="xs" start="15" end="16" type="sstatus-xs-type"/>
+  <field name="mprv" start="17" end="17"/>
+  <field name="sum" start="18" end="18"/>
+  <field name="mxr" start="19" end="19"/>
+  <field name="tvm" start="20" end="20"/>
+  <field name="tw" start="21" end="21"/>
+  <field name="tsr" start="22" end="23"/>
+  <field name="uxl" start="32" end="33" type="sstatus-uxl-type"/>
+  <field name="sxl" start="34" end="35"/>
+  <field name="sbe" start="36" end="36"/>
+  <field name="mbe" start="37" end="37"/>
+  <field name="gva" start="38" end="38"/>
+  <field name="mpv" start="39" end="39"/>
+  <field name="sd" start="63" end="63"/>
+</flags><flags id="sie-fields" size="8">
+  <field name="ssie" start="1" end="1"/>
+  <field name="vssie" start="2" end="2"/>
+  <field name="msie" start="3" end="3"/>
+  <field name="stie" start="5" end="5"/>
+  <field name="vstie" start="6" end="6"/>
+  <field name="mtie" start="7" end="7"/>
+  <field name="seie" start="9" end="9"/>
+  <field name="vseie" start="10" end="10"/>
+  <field name="meie" start="11" end="11"/>
+  <field name="sgeie" start="12" end="12"/>
+</flags><flags id="stvec-fields" size="8">
+  <field name="mode" start="0" end="1" type="stvec-mode-type"/>
+  <field name="base" start="2" end="63"/>
+</flags><flags id="scounteren-fields" size="8">
+  <field name="cy" start="0" end="0"/>
+  <field name="tm" start="1" end="1"/>
+  <field name="ir" start="2" end="2"/>
+  <field name="hpm" start="3" end="31"/>
+</flags><flags id="scause-fields" size="8">
+  <field name="exc" start="0" end="62" type="scause-exc-type"/>
+  <field name="interrupt" start="63" end="63"/>
+</flags><flags id="sip-fields" size="8">
+  <field name="ssip" start="1" end="1"/>
+  <field name="vssip" start="2" end="2"/>
+  <field name="msip" start="3" end="3"/>
+  <field name="stip" start="5" end="5"/>
+  <field name="vstip" start="6" end="6"/>
+  <field name="mtip" start="7" end="7"/>
+  <field name="seip" start="9" end="9"/>
+  <field name="vseip" start="10" end="10"/>
+  <field name="meip" start="11" end="11"/>
+  <field name="sgeip" start="12" end="12"/>
+</flags><flags id="satp-fields" size="8">
+  <field name="ppn" start="0" end="43"/>
+  <field name="asid" start="44" end="59"/>
+  <field name="mode" start="60" end="63" type="satp-mode-type"/>
+</flags><flags id="hstatus-fields" size="8">
+  <field name="vsbe" start="5" end="5"/>
+  <field name="gva" start="6" end="6"/>
+  <field name="spv" start="7" end="7"/>
+  <field name="spvp" start="8" end="8"/>
+  <field name="hu" start="9" end="9"/>
+  <field name="vgein" start="12" end="17"/>
+  <field name="vtvm" start="20" end="20"/>
+  <field name="vtsr" start="22" end="22"/>
+  <field name="vsxl" start="32" end="33"/>
+</flags><flags id="hedeleg-fields" size="8">
+  <field name="instruction_address_misaligned" start="0" end="0"/>
+  <field name="instruction_access_fault" start="1" end="1"/>
+  <field name="illegal_instruction" start="2" end="2"/>
+  <field name="breakpoint" start="3" end="3"/>
+  <field name="load_address_misaligned" start="4" end="4"/>
+  <field name="load_access_fault" start="5" end="5"/>
+  <field name="store_address_misaligned" start="6" end="6"/>
+  <field name="store_access_fault" start="7" end="7"/>
+  <field name="enironment_call_from_U_mode" start="8" end="8"/>
+  <field name="enironment_call_from_S_mode" start="9" end="9"/>
+  <field name="enironment_call_from_VS_mode" start="10" end="10"/>
+  <field name="enironment_call_from_M_mode" start="11" end="11"/>
+  <field name="instruction_page_fault" start="12" end="12"/>
+  <field name="load_page_fault" start="13" end="13"/>
+  <field name="store_page_fault" start="15" end="15"/>
+  <field name="instruction_guest_page_fault" start="20" end="20"/>
+  <field name="load_guest_page_fault" start="21" end="21"/>
+  <field name="virtual_instruction" start="22" end="22"/>
+  <field name="store_guest_page_fault" start="23" end="23"/>
+</flags><flags id="hie-fields" size="8">
+  <field name="vssie" start="2" end="2"/>
+  <field name="vstie" start="6" end="6"/>
+  <field name="vseie" start="10" end="10"/>
+  <field name="sgeie" start="12" end="12"/>
+</flags><flags id="hip-fields" size="8">
+  <field name="vssip" start="2" end="2"/>
+  <field name="vstip" start="6" end="6"/>
+  <field name="vseip" start="10" end="10"/>
+  <field name="sgeip" start="12" end="12"/>
+</flags><flags id="hvip-fields" size="8">
+  <field name="vssip" start="2" end="2"/>
+  <field name="vstip" start="6" end="6"/>
+  <field name="vseip" start="10" end="10"/>
+</flags><flags id="hgatp-fields" size="8">
+  <field name="ppn" start="0" end="43"/>
+  <field name="vmid" start="44" end="57"/>
+  <field name="mode" start="60" end="63" type="hgatp-mode-type"/>
+</flags>
+)
+# endif
+;
diff --git a/target/riscv/gdb_csr_types.h b/target/riscv/gdb_csr_types.h
new file mode 100644
index 0000000000..e55c978ac8
--- /dev/null
+++ b/target/riscv/gdb_csr_types.h
@@ -0,0 +1,3 @@
+/* Copyright (c) 2021 Siemens AG, konrad.schwarz@siemens.com */
+
+extern char const riscv_gdb_csr_types[];
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index 23429179e2..9c3f68eeaf 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -2,6 +2,7 @@
  * RISC-V GDB Server Stub
  *
  * Copyright (c) 2016-2017 Sagar Karandikar, sagark@eecs.berkeley.edu
+ * Copyright (c) 2021 Siemens AG, konrad.schwarz@siemens.com
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -155,6 +156,9 @@ static int riscv_gdb_set_virtual(CPURISCVState *cs, uint8_t *mem_buf, int n)
     return 0;
 }
 
+#include "gdb_csr_types.h"
+#include "gdb_csr_type_group.h"
+
 static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
 {
     RISCVCPU *cpu = RISCV_CPU(cs);
@@ -163,21 +167,33 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
     riscv_csr_predicate_fn predicate;
     int bitsize = 16 << env->misa_mxl_max;
     int i;
+    riscv_csr_operations *csr_op;
+    struct riscv_gdb_csr_tg const *csr_tg;
 
     g_string_printf(s, "<?xml version=\"1.0\"?>");
     g_string_append_printf(s, "<!DOCTYPE feature SYSTEM \"gdb-target.dtd\">");
     g_string_append_printf(s, "<feature name=\"org.gnu.gdb.riscv.csr\">");
 
-    for (i = 0; i < CSR_TABLE_SIZE; i++) {
-        predicate = csr_ops[i].predicate;
+    g_string_append(s, riscv_gdb_csr_types);
+
+    for (i = 0, csr_op = csr_ops, csr_tg = riscv_gdb_csr_type_group;
+            i < CSR_TABLE_SIZE; ++csr_op, ++csr_tg, ++i) {
+        predicate = csr_op->predicate;
         if (predicate && (predicate(env, i) == RISCV_EXCP_NONE)) {
-            if (csr_ops[i].name) {
-                g_string_append_printf(s, "<reg name=\"%s\"", csr_ops[i].name);
+            if (csr_op->name) {
+                g_string_append_printf(s, "<reg name=\"%s\"", csr_op->name);
             } else {
                 g_string_append_printf(s, "<reg name=\"csr%03x\"", i);
             }
             g_string_append_printf(s, " bitsize=\"%d\"", bitsize);
-            g_string_append_printf(s, " regnum=\"%d\"/>", base_reg + i);
+            g_string_append_printf(s, " regnum=\"%d\"", base_reg + i);
+            if (csr_tg->gdb_type) {
+                g_string_append_printf(s, " type=\"%s\"", csr_tg->gdb_type);
+            }
+            if (csr_tg->gdb_group) {
+                g_string_append_printf(s, " group=\"%s\"", csr_tg->gdb_group);
+            }
+            g_string_append(s, " />\n");
         }
     }
 
diff --git a/target/riscv/meson.build b/target/riscv/meson.build
index d5e0bc93ea..e1945e54c4 100644
--- a/target/riscv/meson.build
+++ b/target/riscv/meson.build
@@ -25,7 +25,9 @@ riscv_softmmu_ss.add(files(
   'arch_dump.c',
   'pmp.c',
   'monitor.c',
-  'machine.c'
+  'machine.c',
+  'gdb_csr_types.c',
+  'gdb_csr_type_group.c'
 ))
 
 target_arch += {'riscv': riscv_ss}
-- 
Konrad Schwarz


Re: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver
Posted by Alistair Francis 4 years, 1 month ago
On Wed, Jan 5, 2022 at 1:56 AM Konrad Schwarz
<konrad.schwarz@siemens.com> wrote:
>
> GDB target descriptions support typed registers;
> such that `info register X' displays not only the hex value of
> register `X', but also the individual bitfields the register
> comprises (if any), using textual labels if possible.
>
> This patch includes type information for GDB for
> a large subset of the RISC-V Control and Status Registers (CSRs).
>
> Signed-off-by: Konrad Schwarz <konrad.schwarz@siemens.com>
> ---
>  target/riscv/csr.c                |   2 +
>  target/riscv/csr32-op-gdbserver.h | 109 ++++++++++
>  target/riscv/csr64-op-gdbserver.h |  76 +++++++
>  target/riscv/gdb_csr_type_group.c |  16 ++
>  target/riscv/gdb_csr_type_group.h |   3 +
>  target/riscv/gdb_csr_types.c      | 333 ++++++++++++++++++++++++++++++
>  target/riscv/gdb_csr_types.h      |   3 +
>  target/riscv/gdbstub.c            |  26 ++-
>  target/riscv/meson.build          |   4 +-
>  9 files changed, 566 insertions(+), 6 deletions(-)
>  create mode 100644 target/riscv/csr32-op-gdbserver.h
>  create mode 100644 target/riscv/csr64-op-gdbserver.h
>  create mode 100644 target/riscv/gdb_csr_type_group.c
>  create mode 100644 target/riscv/gdb_csr_type_group.h
>  create mode 100644 target/riscv/gdb_csr_types.c
>  create mode 100644 target/riscv/gdb_csr_types.h
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 9f41954894..557b4afe0e 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3,6 +3,7 @@
>   *
>   * Copyright (c) 2016-2017 Sagar Karandikar, sagark@eecs.berkeley.edu
>   * Copyright (c) 2017-2018 SiFive, Inc.
> + * Copyright (c) 2021 Siemens AG, konrad.schwarz@siemens.com

Please don't add these to existing files. In this case you have just
added a newline to this file

>   *
>   * This program is free software; you can redistribute it and/or modify it
>   * under the terms and conditions of the GNU General Public License,
> @@ -2094,5 +2095,6 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>      [CSR_MHPMCOUNTER29H] = { "mhpmcounter29h", any32,  read_zero },
>      [CSR_MHPMCOUNTER30H] = { "mhpmcounter30h", any32,  read_zero },
>      [CSR_MHPMCOUNTER31H] = { "mhpmcounter31h", any32,  read_zero },
> +
>  #endif /* !CONFIG_USER_ONLY */
>  };
> diff --git a/target/riscv/csr32-op-gdbserver.h b/target/riscv/csr32-op-gdbserver.h
> new file mode 100644
> index 0000000000..e8ec527f23
> --- /dev/null
> +++ b/target/riscv/csr32-op-gdbserver.h
> @@ -0,0 +1,109 @@
> +/* Copyright (c) 2021 Siemens AG, konrad.schwarz@siemens.com */

All of these files should have the usual file boiler plate

> +
> +  [CSR_USTATUS] { .gdb_type = "sstatus-fields", .gdb_group = "user" },
> +  [CSR_UIE] { .gdb_type = "sie-fields", .gdb_group = "user" },
> +  [CSR_UTVEC] { .gdb_type = "code_ptr", .gdb_group = "user" },
> +  [CSR_USCRATCH] { .gdb_type = "data_ptr", .gdb_group = "user" },
> +  [CSR_UEPC] { .gdb_type = "code_ptr", .gdb_group = "user" },
> +  [CSR_UCAUSE] { .gdb_type = "scause-fields", .gdb_group = "user" },
> +  [CSR_UTVAL] { .gdb_type = "data_ptr", .gdb_group = "user" },
> +  [CSR_UIP] { .gdb_type = "code_ptr", .gdb_group = "user" },
> +  [CSR_CYCLE] { .gdb_type = "uint32", .gdb_group = "user" },
> +  [CSR_TIME] { .gdb_type = "uint32", .gdb_group = "user" },
> +  [CSR_INSTRET] { .gdb_type = "uint32", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER3] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER4] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER5] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER6] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER7] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER8] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER9] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER10] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER11] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER12] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER13] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER14] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER15] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER16] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER17] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER18] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER19] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER20] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER21] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER22] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER23] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER24] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER25] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER26] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER27] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER28] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER29] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER30] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER31] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_CYCLEH] { .gdb_type = "uint32", .gdb_group = "user" },
> +  [CSR_TIMEH] { .gdb_type = "uint32", .gdb_group = "user" },
> +  [CSR_INSTRETH] { .gdb_type = "uint32", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER3H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER4H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER5H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER6H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER7H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER8H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER9H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER10H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER11H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER12H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER13H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER14H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER15H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER16H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER17H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER18H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER19H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER20H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER21H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER22H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER23H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER24H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER25H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER26H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER27H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER28H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER29H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER30H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER31H] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_SSTATUS] { .gdb_type = "sstatus-fields", .gdb_group = "supervisor" },
> +  [CSR_SEDELEG] { .gdb_type = "uint32", .gdb_group = "supervisor" },
> +  [CSR_SIDELEG] { .gdb_type = "sie-fields", .gdb_group = "supervisor" },
> +  [CSR_SIE] { .gdb_type = "sie-fields", .gdb_group = "supervisor" },
> +  [CSR_STVEC] { .gdb_type = "stvec-fields", .gdb_group = "supervisor" },
> +  [CSR_SCOUNTEREN] { .gdb_type = "scounteren-fields", .gdb_group = "supervisor" },
> +  [CSR_SSCRATCH] { .gdb_type = "data_ptr", .gdb_group = "supervisor" },
> +  [CSR_SEPC] { .gdb_type = "code_ptr", .gdb_group = "supervisor" },
> +  [CSR_SCAUSE] { .gdb_type = "scause-fields", .gdb_group = "supervisor" },
> +  [CSR_STVAL] { .gdb_type = "data_ptr", .gdb_group = "supervisor" },
> +  [CSR_SIP] { .gdb_type = "sip-fields", .gdb_group = "supervisor" },
> +  [CSR_SATP] { .gdb_type = "satp-fields", .gdb_group = "supervisor" },
> +  [CSR_HSTATUS] { .gdb_type = "hstatus-fields", .gdb_group = "hypervisor" },
> +  [CSR_HEDELEG] { .gdb_type = "hedeleg-fields", .gdb_group = "hypervisor" },
> +  [CSR_HIDELEG] { .gdb_type = "hie-fields", .gdb_group = "hypervisor" },
> +  [CSR_HIE] { .gdb_type = "hie-fields", .gdb_group = "hypervisor" },
> +  [CSR_HCOUNTEREN] { .gdb_type = "int", .gdb_group = "hypervisor" },
> +  [CSR_HGEIE] { .gdb_type = "uint32", .gdb_group = "hypervisor" },
> +  [CSR_HGEIP] { .gdb_type = "uint32", .gdb_group = "hypervisor" },
> +  [CSR_HTVAL] { .gdb_type = "data_ptr", .gdb_group = "hypervisor" },
> +  [CSR_HIP] { .gdb_type = "hip-fields", .gdb_group = "hypervisor" },
> +  [CSR_HVIP] { .gdb_type = "hvip-fields", .gdb_group = "hypervisor" },
> +  [CSR_HGATP] { .gdb_type = "hgatp-fields", .gdb_group = "hypervisor" },
> +  [CSR_HTIMEDELTA] { .gdb_type = "int", .gdb_group = "hypervisor" },
> +  [CSR_HTIMEDELTAH] { .gdb_type = "int", .gdb_group = "hypervisor" },
> +  [CSR_HTINST] { .gdb_type = "uint32", .gdb_group = "hypervisor" },
> +  [CSR_VSSTATUS] { .gdb_type = "sstatus-fields", .gdb_group = "virtual-supervisor" },
> +  [CSR_VSIE] { .gdb_type = "sie-fields", .gdb_group = "virtual-supervisor" },
> +  [CSR_VSTVEC] { .gdb_type = "stvec-fields", .gdb_group = "virtual-supervisor" },
> +  [CSR_VSSCRATCH] { .gdb_type = "data_ptr", .gdb_group = "virtual-supervisor" },
> +  [CSR_VSEPC] { .gdb_type = "code_ptr", .gdb_group = "virtual-supervisor" },
> +  [CSR_VSCAUSE] { .gdb_type = "scause-fields", .gdb_group = "virtual-supervisor" },
> +  [CSR_VSTVAL] { .gdb_type = "data_ptr", .gdb_group = "virtual-supervisor" },
> +  [CSR_VSIP] { .gdb_type = "sip-fields", .gdb_group = "virtual-supervisor" },
> +  [CSR_VSATP] { .gdb_type = "satp-fields", .gdb_group = "virtual-supervisor" },
> diff --git a/target/riscv/csr64-op-gdbserver.h b/target/riscv/csr64-op-gdbserver.h
> new file mode 100644
> index 0000000000..fc4bc62d9e
> --- /dev/null
> +++ b/target/riscv/csr64-op-gdbserver.h
> @@ -0,0 +1,76 @@
> +/* Copyright (c) 2021 Siemens AG, konrad.schwarz@siemens.com */
> +
> +  [CSR_USTATUS] { .gdb_type = "sstatus-fields", .gdb_group = "user" },
> +  [CSR_UIE] { .gdb_type = "sie-fields", .gdb_group = "user" },
> +  [CSR_UTVEC] { .gdb_type = "code_ptr", .gdb_group = "user" },
> +  [CSR_USCRATCH] { .gdb_type = "data_ptr", .gdb_group = "user" },
> +  [CSR_UEPC] { .gdb_type = "code_ptr", .gdb_group = "user" },
> +  [CSR_UCAUSE] { .gdb_type = "scause-fields", .gdb_group = "user" },
> +  [CSR_UTVAL] { .gdb_type = "data_ptr", .gdb_group = "user" },
> +  [CSR_UIP] { .gdb_type = "code_ptr", .gdb_group = "user" },
> +  [CSR_CYCLE] { .gdb_type = "uint64", .gdb_group = "user" },
> +  [CSR_TIME] { .gdb_type = "uint64", .gdb_group = "user" },
> +  [CSR_INSTRET] { .gdb_type = "uint64", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER3] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER4] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER5] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER6] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER7] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER8] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER9] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER10] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER11] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER12] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER13] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER14] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER15] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER16] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER17] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER18] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER19] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER20] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER21] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER22] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER23] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER24] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER25] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER26] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER27] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER28] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER29] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER30] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_HPMCOUNTER31] { .gdb_type = "int", .gdb_group = "user" },
> +  [CSR_SSTATUS] { .gdb_type = "sstatus-fields", .gdb_group = "supervisor" },
> +  [CSR_SEDELEG] { .gdb_type = "uint64", .gdb_group = "supervisor" },
> +  [CSR_SIDELEG] { .gdb_type = "sie-fields", .gdb_group = "supervisor" },
> +  [CSR_SIE] { .gdb_type = "sie-fields", .gdb_group = "supervisor" },
> +  [CSR_STVEC] { .gdb_type = "stvec-fields", .gdb_group = "supervisor" },
> +  [CSR_SCOUNTEREN] { .gdb_type = "scounteren-fields", .gdb_group = "supervisor" },
> +  [CSR_SSCRATCH] { .gdb_type = "data_ptr", .gdb_group = "supervisor" },
> +  [CSR_SEPC] { .gdb_type = "code_ptr", .gdb_group = "supervisor" },
> +  [CSR_SCAUSE] { .gdb_type = "scause-fields", .gdb_group = "supervisor" },
> +  [CSR_STVAL] { .gdb_type = "data_ptr", .gdb_group = "supervisor" },
> +  [CSR_SIP] { .gdb_type = "sip-fields", .gdb_group = "supervisor" },
> +  [CSR_SATP] { .gdb_type = "satp-fields", .gdb_group = "supervisor" },
> +  [CSR_HSTATUS] { .gdb_type = "hstatus-fields", .gdb_group = "hypervisor" },
> +  [CSR_HEDELEG] { .gdb_type = "hedeleg-fields", .gdb_group = "hypervisor" },
> +  [CSR_HIDELEG] { .gdb_type = "hie-fields", .gdb_group = "hypervisor" },
> +  [CSR_HIE] { .gdb_type = "hie-fields", .gdb_group = "hypervisor" },
> +  [CSR_HCOUNTEREN] { .gdb_type = "int", .gdb_group = "hypervisor" },
> +  [CSR_HGEIE] { .gdb_type = "uint64", .gdb_group = "hypervisor" },
> +  [CSR_HGEIP] { .gdb_type = "uint64", .gdb_group = "hypervisor" },
> +  [CSR_HTVAL] { .gdb_type = "data_ptr", .gdb_group = "hypervisor" },
> +  [CSR_HIP] { .gdb_type = "hip-fields", .gdb_group = "hypervisor" },
> +  [CSR_HVIP] { .gdb_type = "hvip-fields", .gdb_group = "hypervisor" },
> +  [CSR_HGATP] { .gdb_type = "hgatp-fields", .gdb_group = "hypervisor" },
> +  [CSR_HTIMEDELTA] { .gdb_type = "int", .gdb_group = "hypervisor" },
> +  [CSR_HTINST] { .gdb_type = "uint64", .gdb_group = "hypervisor" },
> +  [CSR_VSSTATUS] { .gdb_type = "sstatus-fields", .gdb_group = "virtual-supervisor" },
> +  [CSR_VSIE] { .gdb_type = "sie-fields", .gdb_group = "virtual-supervisor" },
> +  [CSR_VSTVEC] { .gdb_type = "stvec-fields", .gdb_group = "virtual-supervisor" },
> +  [CSR_VSSCRATCH] { .gdb_type = "data_ptr", .gdb_group = "virtual-supervisor" },
> +  [CSR_VSEPC] { .gdb_type = "code_ptr", .gdb_group = "virtual-supervisor" },
> +  [CSR_VSCAUSE] { .gdb_type = "scause-fields", .gdb_group = "virtual-supervisor" },
> +  [CSR_VSTVAL] { .gdb_type = "data_ptr", .gdb_group = "virtual-supervisor" },
> +  [CSR_VSIP] { .gdb_type = "sip-fields", .gdb_group = "virtual-supervisor" },
> +  [CSR_VSATP] { .gdb_type = "satp-fields", .gdb_group = "virtual-supervisor" },
> diff --git a/target/riscv/gdb_csr_type_group.c b/target/riscv/gdb_csr_type_group.c
> new file mode 100644
> index 0000000000..af394de302
> --- /dev/null
> +++ b/target/riscv/gdb_csr_type_group.c
> @@ -0,0 +1,16 @@
> +/* Copyright 2021 Siemens AG */
> +#include "qemu/osdep.h"
> +#include "cpu.h"
> +#include "gdb_csr_type_group.h"
> +
> +struct riscv_gdb_csr_tg const riscv_gdb_csr_type_group[] = {
> +
> +#if !defined(CONFIG_USER_ONLY)
> +#  ifdef TARGET_RISCV64
> +#    include "csr64-op-gdbserver.h"
> +#  elif defined TARGET_RISCV64
> +#    include "csr32-op-gdbserver.h"

This doesn't look right. `if defined TARGET_RISCV64` -> `include
"csr32-op-gdbserver.h"`?

Also this should be dynamic instead of based on the build time CPU, as
the user could use a 32-bit CPU on a 64-bit target build.

> +#  endif
> +#endif /* !CONFIG_USER_ONLY */
> +
> +};
> diff --git a/target/riscv/gdb_csr_type_group.h b/target/riscv/gdb_csr_type_group.h
> new file mode 100644
> index 0000000000..e044913bd7
> --- /dev/null
> +++ b/target/riscv/gdb_csr_type_group.h
> @@ -0,0 +1,3 @@
> +extern struct riscv_gdb_csr_tg {
> +    char const *gdb_type, *gdb_group;
> +} const riscv_gdb_csr_type_group[CSR_TABLE_SIZE];

I feel like all of the header files could be combined into one single
file and instead of using macros just pick the struct.

Alistair

> diff --git a/target/riscv/gdb_csr_types.c b/target/riscv/gdb_csr_types.c
> new file mode 100644
> index 0000000000..48b1db2b88
> --- /dev/null
> +++ b/target/riscv/gdb_csr_types.c
> @@ -0,0 +1,333 @@
> +/* Copyright (c) 2021 Siemens AG, konrad.schwarz@siemens.com */
> +
> +#include "qemu/osdep.h"
> +#include "gdb_csr_types.h"
> +#define STR(X) #X
> +
> +char const riscv_gdb_csr_types[] =
> +#ifdef TARGET_RISCV32
> +   STR(
> +<enum id="sstatus-fs-type" size="4">
> +  <evalue name="off" value="0"/>
> +  <evalue name="initial" value="1"/>
> +  <evalue name="clean" value="2"/>
> +  <evalue name="dirty" value="3"/>
> +</enum><enum id="sstatus-xs-type" size="4">
> +  <evalue name="off" value="0"/>
> +  <evalue name="initial" value="1"/>
> +  <evalue name="clean" value="2"/>
> +  <evalue name="dirty" value="3"/>
> +</enum><enum id="sstatus-uxl-type" size="4">
> +  <evalue name="32" value="1"/>
> +  <evalue name="64" value="2"/>
> +  <evalue name="128" value="3"/>
> +</enum><enum id="stvec-mode-type" size="4">
> +  <evalue name="direct" value="0"/>
> +  <evalue name="vectored" value="1"/>
> +</enum><enum id="scause-exc-type" size="4">
> +  <evalue name="instruction_address_misaligned" value="0"/>
> +  <evalue name="instruction_access_fault" value="1"/>
> +  <evalue name="illegal_instruction" value="2"/>
> +  <evalue name="breakpoint" value="3"/>
> +  <evalue name="load_address_misaligned" value="4"/>
> +  <evalue name="load_access_fault" value="5"/>
> +  <evalue name="store_address_misaligned" value="6"/>
> +  <evalue name="store_access_fault" value="7"/>
> +  <evalue name="enironment_call_from_U_mode" value="8"/>
> +  <evalue name="enironment_call_from_S_mode" value="9"/>
> +  <evalue name="enironment_call_from_VS_mode" value="10"/>
> +  <evalue name="enironment_call_from_M_mode" value="11"/>
> +  <evalue name="instruction_page_fault" value="12"/>
> +  <evalue name="load_page_fault" value="13"/>
> +  <evalue name="store_page_fault" value="15"/>
> +  <evalue name="instruction_guest_page_fault" value="20"/>
> +  <evalue name="load_guest_page_fault" value="21"/>
> +  <evalue name="virtual_instruction" value="22"/>
> +  <evalue name="store_guest_page_fault" value="23"/>
> +</enum><enum id="satp-mode-type" size="4">
> +  <evalue name="bare" value="0"/>
> +  <evalue name="sv32" value="1"/>
> +  <evalue name="sv39" value="8"/>
> +  <evalue name="sv48" value="9"/>
> +  <evalue name="sv57" value="10"/>
> +  <evalue name="sv64" value="11"/>
> +</enum><enum id="hgatp-mode-type" size="4">
> +  <evalue name="bare" value="0"/>
> +  <evalue name="sv32x4" value="1"/>
> +  <evalue name="sv39x4" value="8"/>
> +  <evalue name="sv48x4" value="9"/>
> +  <evalue name="sv57x4" value="10"/>
> +</enum><flags id="sstatus-fields" size="4">
> +  <field name="sie" start="1" end="1"/>
> +  <field name="mie" start="3" end="3"/>
> +  <field name="spie" start="5" end="5"/>
> +  <field name="ube" start="6" end="6"/>
> +  <field name="mpie" start="7" end="7"/>
> +  <field name="spp" start="8" end="8"/>
> +  <field name="mpp" start="11" end="12"/>
> +  <field name="fs" start="13" end="14" type="sstatus-fs-type"/>
> +  <field name="xs" start="15" end="16" type="sstatus-xs-type"/>
> +  <field name="mprv" start="17" end="17"/>
> +  <field name="sum" start="18" end="18"/>
> +  <field name="mxr" start="19" end="19"/>
> +  <field name="tvm" start="20" end="20"/>
> +  <field name="tw" start="21" end="21"/>
> +  <field name="tsr" start="22" end="23"/>
> +  <field name="uxl" start="32" end="33" type="sstatus-uxl-type"/>
> +  <field name="sxl" start="34" end="35"/>
> +  <field name="sbe" start="36" end="36"/>
> +  <field name="mbe" start="37" end="37"/>
> +  <field name="gva" start="38" end="38"/>
> +  <field name="mpv" start="39" end="39"/>
> +  <field name="sd" start="63" end="63"/>
> +</flags><flags id="sie-fields" size="4">
> +  <field name="ssie" start="1" end="1"/>
> +  <field name="vssie" start="2" end="2"/>
> +  <field name="msie" start="3" end="3"/>
> +  <field name="stie" start="5" end="5"/>
> +  <field name="vstie" start="6" end="6"/>
> +  <field name="mtie" start="7" end="7"/>
> +  <field name="seie" start="9" end="9"/>
> +  <field name="vseie" start="10" end="10"/>
> +  <field name="meie" start="11" end="11"/>
> +  <field name="sgeie" start="12" end="12"/>
> +</flags><flags id="stvec-fields" size="4">
> +  <field name="mode" start="0" end="1" type="stvec-mode-type"/>
> +  <field name="base" start="2" end="63"/>
> +</flags><flags id="scounteren-fields" size="4">
> +  <field name="cy" start="0" end="0"/>
> +  <field name="tm" start="1" end="1"/>
> +  <field name="ir" start="2" end="2"/>
> +  <field name="hpm" start="3" end="31"/>
> +</flags><flags id="scause-fields" size="4">
> +  <field name="exc" start="0" end="30" type="scause-exc-type"/>
> +  <field name="interrupt" start="31" end="31"/>
> +</flags><flags id="sip-fields" size="4">
> +  <field name="ssip" start="1" end="1"/>
> +  <field name="vssip" start="2" end="2"/>
> +  <field name="msip" start="3" end="3"/>
> +  <field name="stip" start="5" end="5"/>
> +  <field name="vstip" start="6" end="6"/>
> +  <field name="mtip" start="7" end="7"/>
> +  <field name="seip" start="9" end="9"/>
> +  <field name="vseip" start="10" end="10"/>
> +  <field name="meip" start="11" end="11"/>
> +  <field name="sgeip" start="12" end="12"/>
> +</flags><flags id="satp-fields" size="4">
> +  <field name="ppn" start="0" end="43"/>
> +  <field name="asid" start="44" end="59"/>
> +  <field name="mode" start="60" end="63" type="satp-mode-type"/>
> +</flags><flags id="hstatus-fields" size="4">
> +  <field name="vsbe" start="5" end="5"/>
> +  <field name="gva" start="6" end="6"/>
> +  <field name="spv" start="7" end="7"/>
> +  <field name="spvp" start="8" end="8"/>
> +  <field name="hu" start="9" end="9"/>
> +  <field name="vgein" start="12" end="17"/>
> +  <field name="vtvm" start="20" end="20"/>
> +  <field name="vtsr" start="22" end="22"/>
> +  <field name="vsxl" start="32" end="33"/>
> +</flags><flags id="hedeleg-fields" size="4">
> +  <field name="instruction_address_misaligned" start="0" end="0"/>
> +  <field name="instruction_access_fault" start="1" end="1"/>
> +  <field name="illegal_instruction" start="2" end="2"/>
> +  <field name="breakpoint" start="3" end="3"/>
> +  <field name="load_address_misaligned" start="4" end="4"/>
> +  <field name="load_access_fault" start="5" end="5"/>
> +  <field name="store_address_misaligned" start="6" end="6"/>
> +  <field name="store_access_fault" start="7" end="7"/>
> +  <field name="enironment_call_from_U_mode" start="8" end="8"/>
> +  <field name="enironment_call_from_S_mode" start="9" end="9"/>
> +  <field name="enironment_call_from_VS_mode" start="10" end="10"/>
> +  <field name="enironment_call_from_M_mode" start="11" end="11"/>
> +  <field name="instruction_page_fault" start="12" end="12"/>
> +  <field name="load_page_fault" start="13" end="13"/>
> +  <field name="store_page_fault" start="15" end="15"/>
> +  <field name="instruction_guest_page_fault" start="20" end="20"/>
> +  <field name="load_guest_page_fault" start="21" end="21"/>
> +  <field name="virtual_instruction" start="22" end="22"/>
> +  <field name="store_guest_page_fault" start="23" end="23"/>
> +</flags><flags id="hie-fields" size="4">
> +  <field name="vssie" start="2" end="2"/>
> +  <field name="vstie" start="6" end="6"/>
> +  <field name="vseie" start="10" end="10"/>
> +  <field name="sgeie" start="12" end="12"/>
> +</flags><flags id="hip-fields" size="4">
> +  <field name="vssip" start="2" end="2"/>
> +  <field name="vstip" start="6" end="6"/>
> +  <field name="vseip" start="10" end="10"/>
> +  <field name="sgeip" start="12" end="12"/>
> +</flags><flags id="hvip-fields" size="4">
> +  <field name="vssip" start="2" end="2"/>
> +  <field name="vstip" start="6" end="6"/>
> +  <field name="vseip" start="10" end="10"/>
> +</flags><flags id="hgatp-fields" size="4">
> +  <field name="ppn" start="0" end="43"/>
> +  <field name="vmid" start="44" end="57"/>
> +  <field name="mode" start="60" end="63" type="hgatp-mode-type"/>
> +</flags>
> +)
> +#elif defined TARGET_RISCV64
> +   STR(
> +<enum id="sstatus-fs-type" size="8">
> +  <evalue name="off" value="0"/>
> +  <evalue name="initial" value="1"/>
> +  <evalue name="clean" value="2"/>
> +  <evalue name="dirty" value="3"/>
> +</enum><enum id="sstatus-xs-type" size="8">
> +  <evalue name="off" value="0"/>
> +  <evalue name="initial" value="1"/>
> +  <evalue name="clean" value="2"/>
> +  <evalue name="dirty" value="3"/>
> +</enum><enum id="sstatus-uxl-type" size="8">
> +  <evalue name="32" value="1"/>
> +  <evalue name="64" value="2"/>
> +  <evalue name="128" value="3"/>
> +</enum><enum id="stvec-mode-type" size="8">
> +  <evalue name="direct" value="0"/>
> +  <evalue name="vectored" value="1"/>
> +</enum><enum id="scause-exc-type" size="8">
> +  <evalue name="instruction_address_misaligned" value="0"/>
> +  <evalue name="instruction_access_fault" value="1"/>
> +  <evalue name="illegal_instruction" value="2"/>
> +  <evalue name="breakpoint" value="3"/>
> +  <evalue name="load_address_misaligned" value="4"/>
> +  <evalue name="load_access_fault" value="5"/>
> +  <evalue name="store_address_misaligned" value="6"/>
> +  <evalue name="store_access_fault" value="7"/>
> +  <evalue name="enironment_call_from_U_mode" value="8"/>
> +  <evalue name="enironment_call_from_S_mode" value="9"/>
> +  <evalue name="enironment_call_from_VS_mode" value="10"/>
> +  <evalue name="enironment_call_from_M_mode" value="11"/>
> +  <evalue name="instruction_page_fault" value="12"/>
> +  <evalue name="load_page_fault" value="13"/>
> +  <evalue name="store_page_fault" value="15"/>
> +  <evalue name="instruction_guest_page_fault" value="20"/>
> +  <evalue name="load_guest_page_fault" value="21"/>
> +  <evalue name="virtual_instruction" value="22"/>
> +  <evalue name="store_guest_page_fault" value="23"/>
> +</enum><enum id="satp-mode-type" size="8">
> +  <evalue name="bare" value="0"/>
> +  <evalue name="sv32" value="1"/>
> +  <evalue name="sv39" value="8"/>
> +  <evalue name="sv48" value="9"/>
> +  <evalue name="sv57" value="10"/>
> +  <evalue name="sv64" value="11"/>
> +</enum><enum id="hgatp-mode-type" size="8">
> +  <evalue name="bare" value="0"/>
> +  <evalue name="sv32x4" value="1"/>
> +  <evalue name="sv39x4" value="8"/>
> +  <evalue name="sv48x4" value="9"/>
> +  <evalue name="sv57x4" value="10"/>
> +</enum><flags id="sstatus-fields" size="8">
> +  <field name="sie" start="1" end="1"/>
> +  <field name="mie" start="3" end="3"/>
> +  <field name="spie" start="5" end="5"/>
> +  <field name="ube" start="6" end="6"/>
> +  <field name="mpie" start="7" end="7"/>
> +  <field name="spp" start="8" end="8"/>
> +  <field name="mpp" start="11" end="12"/>
> +  <field name="fs" start="13" end="14" type="sstatus-fs-type"/>
> +  <field name="xs" start="15" end="16" type="sstatus-xs-type"/>
> +  <field name="mprv" start="17" end="17"/>
> +  <field name="sum" start="18" end="18"/>
> +  <field name="mxr" start="19" end="19"/>
> +  <field name="tvm" start="20" end="20"/>
> +  <field name="tw" start="21" end="21"/>
> +  <field name="tsr" start="22" end="23"/>
> +  <field name="uxl" start="32" end="33" type="sstatus-uxl-type"/>
> +  <field name="sxl" start="34" end="35"/>
> +  <field name="sbe" start="36" end="36"/>
> +  <field name="mbe" start="37" end="37"/>
> +  <field name="gva" start="38" end="38"/>
> +  <field name="mpv" start="39" end="39"/>
> +  <field name="sd" start="63" end="63"/>
> +</flags><flags id="sie-fields" size="8">
> +  <field name="ssie" start="1" end="1"/>
> +  <field name="vssie" start="2" end="2"/>
> +  <field name="msie" start="3" end="3"/>
> +  <field name="stie" start="5" end="5"/>
> +  <field name="vstie" start="6" end="6"/>
> +  <field name="mtie" start="7" end="7"/>
> +  <field name="seie" start="9" end="9"/>
> +  <field name="vseie" start="10" end="10"/>
> +  <field name="meie" start="11" end="11"/>
> +  <field name="sgeie" start="12" end="12"/>
> +</flags><flags id="stvec-fields" size="8">
> +  <field name="mode" start="0" end="1" type="stvec-mode-type"/>
> +  <field name="base" start="2" end="63"/>
> +</flags><flags id="scounteren-fields" size="8">
> +  <field name="cy" start="0" end="0"/>
> +  <field name="tm" start="1" end="1"/>
> +  <field name="ir" start="2" end="2"/>
> +  <field name="hpm" start="3" end="31"/>
> +</flags><flags id="scause-fields" size="8">
> +  <field name="exc" start="0" end="62" type="scause-exc-type"/>
> +  <field name="interrupt" start="63" end="63"/>
> +</flags><flags id="sip-fields" size="8">
> +  <field name="ssip" start="1" end="1"/>
> +  <field name="vssip" start="2" end="2"/>
> +  <field name="msip" start="3" end="3"/>
> +  <field name="stip" start="5" end="5"/>
> +  <field name="vstip" start="6" end="6"/>
> +  <field name="mtip" start="7" end="7"/>
> +  <field name="seip" start="9" end="9"/>
> +  <field name="vseip" start="10" end="10"/>
> +  <field name="meip" start="11" end="11"/>
> +  <field name="sgeip" start="12" end="12"/>
> +</flags><flags id="satp-fields" size="8">
> +  <field name="ppn" start="0" end="43"/>
> +  <field name="asid" start="44" end="59"/>
> +  <field name="mode" start="60" end="63" type="satp-mode-type"/>
> +</flags><flags id="hstatus-fields" size="8">
> +  <field name="vsbe" start="5" end="5"/>
> +  <field name="gva" start="6" end="6"/>
> +  <field name="spv" start="7" end="7"/>
> +  <field name="spvp" start="8" end="8"/>
> +  <field name="hu" start="9" end="9"/>
> +  <field name="vgein" start="12" end="17"/>
> +  <field name="vtvm" start="20" end="20"/>
> +  <field name="vtsr" start="22" end="22"/>
> +  <field name="vsxl" start="32" end="33"/>
> +</flags><flags id="hedeleg-fields" size="8">
> +  <field name="instruction_address_misaligned" start="0" end="0"/>
> +  <field name="instruction_access_fault" start="1" end="1"/>
> +  <field name="illegal_instruction" start="2" end="2"/>
> +  <field name="breakpoint" start="3" end="3"/>
> +  <field name="load_address_misaligned" start="4" end="4"/>
> +  <field name="load_access_fault" start="5" end="5"/>
> +  <field name="store_address_misaligned" start="6" end="6"/>
> +  <field name="store_access_fault" start="7" end="7"/>
> +  <field name="enironment_call_from_U_mode" start="8" end="8"/>
> +  <field name="enironment_call_from_S_mode" start="9" end="9"/>
> +  <field name="enironment_call_from_VS_mode" start="10" end="10"/>
> +  <field name="enironment_call_from_M_mode" start="11" end="11"/>
> +  <field name="instruction_page_fault" start="12" end="12"/>
> +  <field name="load_page_fault" start="13" end="13"/>
> +  <field name="store_page_fault" start="15" end="15"/>
> +  <field name="instruction_guest_page_fault" start="20" end="20"/>
> +  <field name="load_guest_page_fault" start="21" end="21"/>
> +  <field name="virtual_instruction" start="22" end="22"/>
> +  <field name="store_guest_page_fault" start="23" end="23"/>
> +</flags><flags id="hie-fields" size="8">
> +  <field name="vssie" start="2" end="2"/>
> +  <field name="vstie" start="6" end="6"/>
> +  <field name="vseie" start="10" end="10"/>
> +  <field name="sgeie" start="12" end="12"/>
> +</flags><flags id="hip-fields" size="8">
> +  <field name="vssip" start="2" end="2"/>
> +  <field name="vstip" start="6" end="6"/>
> +  <field name="vseip" start="10" end="10"/>
> +  <field name="sgeip" start="12" end="12"/>
> +</flags><flags id="hvip-fields" size="8">
> +  <field name="vssip" start="2" end="2"/>
> +  <field name="vstip" start="6" end="6"/>
> +  <field name="vseip" start="10" end="10"/>
> +</flags><flags id="hgatp-fields" size="8">
> +  <field name="ppn" start="0" end="43"/>
> +  <field name="vmid" start="44" end="57"/>
> +  <field name="mode" start="60" end="63" type="hgatp-mode-type"/>
> +</flags>
> +)
> +# endif
> +;
> diff --git a/target/riscv/gdb_csr_types.h b/target/riscv/gdb_csr_types.h
> new file mode 100644
> index 0000000000..e55c978ac8
> --- /dev/null
> +++ b/target/riscv/gdb_csr_types.h
> @@ -0,0 +1,3 @@
> +/* Copyright (c) 2021 Siemens AG, konrad.schwarz@siemens.com */
> +
> +extern char const riscv_gdb_csr_types[];
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index 23429179e2..9c3f68eeaf 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -2,6 +2,7 @@
>   * RISC-V GDB Server Stub
>   *
>   * Copyright (c) 2016-2017 Sagar Karandikar, sagark@eecs.berkeley.edu
> + * Copyright (c) 2021 Siemens AG, konrad.schwarz@siemens.com
>   *
>   * This program is free software; you can redistribute it and/or modify it
>   * under the terms and conditions of the GNU General Public License,
> @@ -155,6 +156,9 @@ static int riscv_gdb_set_virtual(CPURISCVState *cs, uint8_t *mem_buf, int n)
>      return 0;
>  }
>
> +#include "gdb_csr_types.h"
> +#include "gdb_csr_type_group.h"
> +
>  static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
>  {
>      RISCVCPU *cpu = RISCV_CPU(cs);
> @@ -163,21 +167,33 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
>      riscv_csr_predicate_fn predicate;
>      int bitsize = 16 << env->misa_mxl_max;
>      int i;
> +    riscv_csr_operations *csr_op;
> +    struct riscv_gdb_csr_tg const *csr_tg;
>
>      g_string_printf(s, "<?xml version=\"1.0\"?>");
>      g_string_append_printf(s, "<!DOCTYPE feature SYSTEM \"gdb-target.dtd\">");
>      g_string_append_printf(s, "<feature name=\"org.gnu.gdb.riscv.csr\">");
>
> -    for (i = 0; i < CSR_TABLE_SIZE; i++) {
> -        predicate = csr_ops[i].predicate;
> +    g_string_append(s, riscv_gdb_csr_types);
> +
> +    for (i = 0, csr_op = csr_ops, csr_tg = riscv_gdb_csr_type_group;
> +            i < CSR_TABLE_SIZE; ++csr_op, ++csr_tg, ++i) {
> +        predicate = csr_op->predicate;
>          if (predicate && (predicate(env, i) == RISCV_EXCP_NONE)) {
> -            if (csr_ops[i].name) {
> -                g_string_append_printf(s, "<reg name=\"%s\"", csr_ops[i].name);
> +            if (csr_op->name) {
> +                g_string_append_printf(s, "<reg name=\"%s\"", csr_op->name);
>              } else {
>                  g_string_append_printf(s, "<reg name=\"csr%03x\"", i);
>              }
>              g_string_append_printf(s, " bitsize=\"%d\"", bitsize);
> -            g_string_append_printf(s, " regnum=\"%d\"/>", base_reg + i);
> +            g_string_append_printf(s, " regnum=\"%d\"", base_reg + i);
> +            if (csr_tg->gdb_type) {
> +                g_string_append_printf(s, " type=\"%s\"", csr_tg->gdb_type);
> +            }
> +            if (csr_tg->gdb_group) {
> +                g_string_append_printf(s, " group=\"%s\"", csr_tg->gdb_group);
> +            }
> +            g_string_append(s, " />\n");
>          }
>      }
>
> diff --git a/target/riscv/meson.build b/target/riscv/meson.build
> index d5e0bc93ea..e1945e54c4 100644
> --- a/target/riscv/meson.build
> +++ b/target/riscv/meson.build
> @@ -25,7 +25,9 @@ riscv_softmmu_ss.add(files(
>    'arch_dump.c',
>    'pmp.c',
>    'monitor.c',
> -  'machine.c'
> +  'machine.c',
> +  'gdb_csr_types.c',
> +  'gdb_csr_type_group.c'
>  ))
>
>  target_arch += {'riscv': riscv_ss}
> --
> Konrad Schwarz
>
>

RE: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver
Posted by Schwarz, Konrad 4 years, 1 month ago
Hi,

> -----Original Message-----
> From: Alistair Francis <alistair23@gmail.com>
> Sent: Tuesday, January 4, 2022 23:12
> To: Schwarz, Konrad (T CED SES-DE) <konrad.schwarz@siemens.com>
> Subject: Re: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver
> 
> On Wed, Jan 5, 2022 at 1:56 AM Konrad Schwarz
> <konrad.schwarz@siemens.com> wrote:
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index 9f41954894..557b4afe0e 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -3,6 +3,7 @@
> >   *
> >   * Copyright (c) 2016-2017 Sagar Karandikar, sagark@eecs.berkeley.edu
> >   * Copyright (c) 2017-2018 SiFive, Inc.
> > + * Copyright (c) 2021 Siemens AG, konrad.schwarz@siemens.com
> 
> Please don't add these to existing files. In this case you have just
> added a newline to this file

Sorry, I don't know how that slipped through.
I originally didn't have any copyright messages, to which my company objected,
and I guess I overcompensated.
 
> diff --git a/target/riscv/csr32-op-gdbserver.h b/target/riscv/csr32-op-gdbserver.h
> > new file mode 100644
> > index 0000000000..e8ec527f23
> > --- /dev/null
> > +++ b/target/riscv/csr32-op-gdbserver.h
> > @@ -0,0 +1,109 @@
> > +/* Copyright (c) 2021 Siemens AG, konrad.schwarz@siemens.com */
> 
> All of these files should have the usual file boiler plate

OK, I'll try to figure out something.

> > +#if !defined(CONFIG_USER_ONLY)
> > +#  ifdef TARGET_RISCV64
> > +#    include "csr64-op-gdbserver.h"
> > +#  elif defined TARGET_RISCV64
> > +#    include "csr32-op-gdbserver.h"
> 
> This doesn't look right. `if defined TARGET_RISCV64` -> `include
> "csr32-op-gdbserver.h"`?

You are quite right. 

> Also this should be dynamic instead of based on the build time CPU, as
> the user could use a 32-bit CPU on a 64-bit target build.

I'm not sure.  The machine itself is still a 64-bit machine; its CSRs are 64 bits long.
`csr.c', which implements the CSRs contains an instance of # ifdef TARGET_RISCV64,
so that part of the implementation is not dynamic either.

Also, someone who is developing 32-bit system software can easily
sidestep the issue by using the 32-bit target.

Regards
Konrad
Re: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver
Posted by Richard Henderson 4 years, 1 month ago
On 1/4/22 7:51 AM, Konrad Schwarz wrote:
> +++ b/target/riscv/csr32-op-gdbserver.h
...
> +++ b/target/riscv/csr64-op-gdbserver.h

There is a *lot* of overlap between these two files.
Why not add this data to the main csr_ops array?
That would put all the info for each csr in one place.

> +  [CSR_CYCLE] { .gdb_type = "uint64", .gdb_group = "user" },

I think you should be able to use "unsigned long" as a proxy for the native register size.

> +char const riscv_gdb_csr_types[] =
> +#ifdef TARGET_RISCV32
...
> +#elif defined TARGET_RISCV64
...
> +# endif
> +;

Ideally we shouldn't use ifdefs for this -- we should choose one or the other depending on 
the startup env->misa_mxl_max.  We are still using an ifdef for the main 
riscv-*-virtual.xml, but that could be considered a bug to fix.


r~

RE: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver
Posted by Schwarz, Konrad 4 years, 1 month ago
Hi,

> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Wednesday, January 5, 2022 0:02
> To: Schwarz, Konrad (T CED SES-DE) <konrad.schwarz@siemens.com>; qemu-devel@nongnu.org
> Cc: Alistair Francis <alistair.francis@wdc.com>; Bin Meng <bin.meng@windriver.com>; Palmer Dabbelt
> <palmer@dabbelt.com>; Ralf Ramsauer <ralf.ramsauer@oth-regensburg.de>
> Subject: Re: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver
> 
> On 1/4/22 7:51 AM, Konrad Schwarz wrote:
> > +++ b/target/riscv/csr32-op-gdbserver.h
> ...
> > +++ b/target/riscv/csr64-op-gdbserver.h
> 
> There is a *lot* of overlap between these two files.
> Why not add this data to the main csr_ops array?
> That would put all the info for each csr in one place.

So the problem is that these files are generated -- somewhat ironically
via XSLT from complete GDB target descriptions, which are themselves
generated from a mixture of AWK and shell scripts that I have in a
different project and which you would probably not want to have
contributed.  Those scripts generate a variety of other definitions
for C and assembly besides the GDB XML target descriptions, so would
probably need to be reduced for just QEMU usage.

I did actually originally add the data directly to the csr_ops array.
Because of the large number of CSRs and the generational aspect,
it was infeasible (for me at least) to create an editing merge script to
intermingle the existing definitions and the new data.  I tried to
work around this by using C99's designated initializer syntax,
adding in the new data at the end of the table, and using specific
enough initializers to not disturb the existing data.

However, this did not work out: despite using very specific initializers,
the previously initialized CSR structures in the csr_ops array
were reset to their default values, i.e., 0, breaking the code.
This was not the way I expected this feature to work in C99 and
my reading of the C99 standard does not support this either.  But
that’s what GCC does, at least on my machine.

This is also the reason the overlap is not handled more elegantly:
GDB target description XML doesn't support alternates for different
machine word lengths and so I lose that information when transforming
from the 64 and 32-bit target description sources.
 
> > +  [CSR_CYCLE] { .gdb_type = "uint64", .gdb_group = "user" },
> 
> I think you should be able to use "unsigned long" as a proxy for the native register size.

`unsigned long' is not listed in section `G.3 Predefined Target Types'
of the GDB manual.

I also have to say that GDB does not handle the target descriptions
correctly in all cases.  In particular, I suspect a bug when
a field crosses a 32-bit boundary: GDB is showing twice the
field value.  I'm pretty sure my target description is correct
for this case, I've rechecked several times.  Also, GDB
is not picking up all of the new CSRs for some reason, but
I'm sure is reading the target description from QEMU.

Since target descriptions are rare in practice, a bug would
not surprise me; for example, the DTD for target descriptions
supplied with GDB is wrong: it is missing the `enum' element
(which was probably added later) and the xsi:include element.

Ultimately, this is a chicken and egg problem: bugs in GDB can
only be flushed out if it gets interesting
target descriptions to read.

> 
> > +char const riscv_gdb_csr_types[] =
> > +#ifdef TARGET_RISCV32
> ...
> > +#elif defined TARGET_RISCV64
> ...
> > +# endif
> > +;
> 
> Ideally we shouldn't use ifdefs for this -- we should choose one or the other depending on
> the startup env->misa_mxl_max.  We are still using an ifdef for the main
> riscv-*-virtual.xml, but that could be considered a bug to fix.

As I wrote Alistair, I'm not sure this reasoning is correct.  Even if a 64-bit
machine is running in 32-bit mode, the machine itself remains a 64-bit machine,
and it's CSRs are so too.  We can have the situation of a 64-bit kernel and 32-bit
user-mode process; would we want the CSRs to change when switching between the two?
Even if we did, the GDB remote protocol does not have the ability to say "API change,
please reread the target description" (AFAIK).  And in any case, users can easily side-step
the issue by using a 32-bit target QEMU if they are only interested in 32-bit code.

Regards
Konrad
Re: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver
Posted by Richard Henderson 4 years, 1 month ago
On 1/5/22 6:04 AM, Schwarz, Konrad wrote:
> So the problem is that these files are generated -- somewhat ironically
> via XSLT from complete GDB target descriptions, which are themselves
> generated from a mixture of AWK and shell scripts that I have in a
> different project and which you would probably not want to have
> contributed.  Those scripts generate a variety of other definitions
> for C and assembly besides the GDB XML target descriptions, so would
> probably need to be reduced for just QEMU usage.

You may be right that we don't want your original scripts, but that also  implies that the 
fact your files are generated is irrelevant to us.  Why should we care?  I think it makes 
more sense to manually edit csr.c and be done with it.

>  I tried to
> work around this by using C99's designated initializer syntax,
> adding in the new data at the end of the table, and using specific
> enough initializers to not disturb the existing data.
> 
> However, this did not work out: despite using very specific initializers,
> the previously initialized CSR structures in the csr_ops array
> were reset to their default values, i.e., 0, breaking the code.
> This was not the way I expected this feature to work in C99 and
> my reading of the C99 standard does not support this either.  But
> that’s what GCC does, at least on my machine.

I'm sure your syntax was incorrect.  You probably used

>>> +  [CSR_CYCLE] { .gdb_type = "uint64", .gdb_group = "user" },

which does indeed overwrite the entire entry in the array.  You could have used

     [CSR_CYCLE].gdb_type = "uint64"

which will just set the one field.

That said, I don't think that we want this distributed initialization.

>> I think you should be able to use "unsigned long" as a proxy for the native register size.
> 
> `unsigned long' is not listed in section `G.3 Predefined Target Types'
> of the GDB manual.

Hmm.  I didn't look at the docs; I looked at gdbtypes.h and saw the existence of 
builtin_unsigned_long.  I suppose this might not be plumbed into the xml.

In which case, since we're generating everything dynamically anyway, we could just as 
easily make NULL map to "uint<XLEN>" when generating the xml.  Or instead of a string, 
perhaps some enum which distinguishes data_ptr/code_ptr/uint<xlen>/uint<min(n,xlen)> which 
is then filled in during generation.

> I also have to say that GDB does not handle the target descriptions
> correctly in all cases.  In particular, I suspect a bug when
> a field crosses a 32-bit boundary: GDB is showing twice the
> field value.

I wonder if this is an issue of using "uint32" for the field type?  You could, for the 
moment, drop all of the bitfield descriptions and leave the interpretation to the user. 
Giving an accurate integral value for the register as a whole is more helpful than giving 
inaccurate field values.

> As I wrote Alistair, I'm not sure this reasoning is correct.  Even if a 64-bit
> machine is running in 32-bit mode, the machine itself remains a 64-bit machine,
> and it's CSRs are so too.

And that's where we're mis-communicating.

We're working toward having exactly one qemu-system-riscv binary, which will emulate both 
RV32 and RV64 (and RV128) cpus, selected by -cpu foo.  At which point there will be no 
CONFIG_RISCV32 define at all.  The way to distinguish the three cases is 
cpu->env.misa_mxl_max.

> We can have the situation of a 64-bit kernel and 32-bit
> user-mode process; would we want the CSRs to change when switching between the two?

Well ideally yes...

> Even if we did, the GDB remote protocol does not have the ability to say "API change,
> please reread the target description" (AFAIK).

... but as you rightly note, gdb can't handle it.


r~

Re: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver
Posted by Alex Bennée 4 years, 1 month ago
Konrad Schwarz <konrad.schwarz@siemens.com> writes:

> GDB target descriptions support typed registers;
> such that `info register X' displays not only the hex value of
> register `X', but also the individual bitfields the register
> comprises (if any), using textual labels if possible.
>
> This patch includes type information for GDB for
> a large subset of the RISC-V Control and Status Registers (CSRs).
>
> Signed-off-by: Konrad Schwarz <konrad.schwarz@siemens.com>
<snip>

Not withstanding my general comments (wish) to eventually get rid of
per-arch XML generation:

>  static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
>  {
>      RISCVCPU *cpu = RISCV_CPU(cs);
> @@ -163,21 +167,33 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
>      riscv_csr_predicate_fn predicate;
>      int bitsize = 16 << env->misa_mxl_max;
>      int i;
> +    riscv_csr_operations *csr_op;
> +    struct riscv_gdb_csr_tg const *csr_tg;
>  
>      g_string_printf(s, "<?xml version=\"1.0\"?>");
>      g_string_append_printf(s, "<!DOCTYPE feature SYSTEM \"gdb-target.dtd\">");
>      g_string_append_printf(s, "<feature>      name=\"org.gnu.gdb.riscv.csr\">");

With these changes does it still match the org.gnu.gdb.riscv.csr
register description in gdb? Previously for custom XML I've used the
org.qemu.ARCH.REGS form to distinguish between something GDB expects and
something we invented (changed since 797920b952ea).

>  
> -    for (i = 0; i < CSR_TABLE_SIZE; i++) {
> -        predicate = csr_ops[i].predicate;
> +    g_string_append(s, riscv_gdb_csr_types);
> +
> +    for (i = 0, csr_op = csr_ops, csr_tg = riscv_gdb_csr_type_group;
> +            i < CSR_TABLE_SIZE; ++csr_op, ++csr_tg, ++i) {
> +        predicate = csr_op->predicate;
>          if (predicate && (predicate(env, i) == RISCV_EXCP_NONE)) {
> -            if (csr_ops[i].name) {
> -                g_string_append_printf(s, "<reg name=\"%s\"", csr_ops[i].name);
> +            if (csr_op->name) {
> +                g_string_append_printf(s, "<reg name=\"%s\"", csr_op->name);
>              } else {
>                  g_string_append_printf(s, "<reg name=\"csr%03x\"", i);
>              }
>              g_string_append_printf(s, " bitsize=\"%d\"", bitsize);
> -            g_string_append_printf(s, " regnum=\"%d\"/>", base_reg + i);
> +            g_string_append_printf(s, " regnum=\"%d\"", base_reg + i);
> +            if (csr_tg->gdb_type) {
> +                g_string_append_printf(s, " type=\"%s\"", csr_tg->gdb_type);
> +            }
> +            if (csr_tg->gdb_group) {
> +                g_string_append_printf(s, " group=\"%s\"", csr_tg->gdb_group);
> +            }
> +            g_string_append(s, " />\n");
>          }
>      }
<snip>

-- 
Alex Bennée

RE: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver
Posted by Schwarz, Konrad 4 years, 1 month ago

> -----Original Message-----
> From: Alex Bennée <alex.bennee@linaro.org>

> Konrad Schwarz <konrad.schwarz@siemens.com> writes:
> 

> >  static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
> >  {
> >      RISCVCPU *cpu = RISCV_CPU(cs);
> > @@ -163,21 +167,33 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
> >      riscv_csr_predicate_fn predicate;
> >      int bitsize = 16 << env->misa_mxl_max;
> >      int i;
> > +    riscv_csr_operations *csr_op;
> > +    struct riscv_gdb_csr_tg const *csr_tg;
> >
> >      g_string_printf(s, "<?xml version=\"1.0\"?>");
> >      g_string_append_printf(s, "<!DOCTYPE feature SYSTEM \"gdb-target.dtd\">");
> >      g_string_append_printf(s, "<feature>      name=\"org.gnu.gdb.riscv.csr\">");
> 
> With these changes does it still match the org.gnu.gdb.riscv.csr
> register description in gdb? Previously for custom XML I've used the
> org.qemu.ARCH.REGS form to distinguish between something GDB expects and
> something we invented (changed since 797920b952ea).

I don't expect it to match -- but why should it?
The whole point of target descriptions is for GDBserver to inform
GDB of the precise set and layout of pre-defined register classes.
The class `org.gnu.gdb.riscv.csr' is known to a RISC-V capable
GDB as the class of all CSRs; a specific RISC-V core might
have vendor-specific CSRs, but they would still be included
in `org.gnu.gdb.riscv.csr'.

Section G.5 in the GDB manual makes this clear:
"You can add additional registers to any of the standard features --
GDB will display them just as they were added to an
unrecognized feature."

--
Konrad
Re: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver
Posted by Alex Bennée 4 years, 1 month ago
"Schwarz, Konrad" <konrad.schwarz@siemens.com> writes:

>> -----Original Message-----
>> From: Alex Bennée <alex.bennee@linaro.org>
>
>> Konrad Schwarz <konrad.schwarz@siemens.com> writes:
>> 
>
>> >  static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
>> >  {
>> >      RISCVCPU *cpu = RISCV_CPU(cs);
>> > @@ -163,21 +167,33 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
>> >      riscv_csr_predicate_fn predicate;
>> >      int bitsize = 16 << env->misa_mxl_max;
>> >      int i;
>> > +    riscv_csr_operations *csr_op;
>> > +    struct riscv_gdb_csr_tg const *csr_tg;
>> >
>> >      g_string_printf(s, "<?xml version=\"1.0\"?>");
>> >      g_string_append_printf(s, "<!DOCTYPE feature SYSTEM \"gdb-target.dtd\">");
>> >      g_string_append_printf(s, "<feature>      name=\"org.gnu.gdb.riscv.csr\">");
>> 
>> With these changes does it still match the org.gnu.gdb.riscv.csr
>> register description in gdb? Previously for custom XML I've used the
>> org.qemu.ARCH.REGS form to distinguish between something GDB expects and
>> something we invented (changed since 797920b952ea).
>
> I don't expect it to match -- but why should it?
> The whole point of target descriptions is for GDBserver to inform
> GDB of the precise set and layout of pre-defined register classes.
> The class `org.gnu.gdb.riscv.csr' is known to a RISC-V capable
> GDB as the class of all CSRs; a specific RISC-V core might
> have vendor-specific CSRs, but they would still be included
> in `org.gnu.gdb.riscv.csr'.

Certainly for ARM's SVE there is special handling code in gdb to deal
with the control of the vector length. As long as GDB doesn't make any
such assumptions for RISC-V then go right ahead.

>
> Section G.5 in the GDB manual makes this clear:
> "You can add additional registers to any of the standard features --
> GDB will display them just as they were added to an
> unrecognized feature."


-- 
Alex Bennée