Re: [RFC PATCH v3 1/2] target/riscv: Add RISC-V CSR qtest support

2024-06-26 Thread Thomas Huth

On 25/06/2024 17.35, Ivan Klokov wrote:

The RISC-V architecture supports the creation of custom
CSR-mapped devices. It would be convenient to test them in the same way
as MMIO-mapped devices. To do this, a new call has been added
to read/write CSR registers.

Signed-off-by: Ivan Klokov 
---
  target/riscv/cpu.c | 14 +++
  target/riscv/cpu.h |  3 +++
  target/riscv/csr.c | 53 +-
  tests/qtest/libqtest.c | 27 +
  tests/qtest/libqtest.h | 14 +++
  5 files changed, 110 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 69a08e8c2c..55cc01bfb3 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1148,7 +1148,17 @@ void riscv_cpu_finalize_features(RISCVCPU *cpu, Error 
**errp)
  }
  }
  }
+#ifndef CONFIG_USER_ONLY
+static void riscv_cpu_register_csr_qtest_callback(void)
+{
+static gsize reinit_done;
+if (g_once_init_enter(&reinit_done)) {
+qtest_set_command_cb(csr_qtest_callback);
  
+g_once_init_leave(&reinit_done, 1);

+}
+}
+#endif
  static void riscv_cpu_realize(DeviceState *dev, Error **errp)


Could you please add an empty line before the #ifndef and after the #endif 
line? Thanks!




diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index c7f6897d78..f8c3ff15a9 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1205,6 +1205,33 @@ uint64_t qtest_rtas_call(QTestState *s, const char *name,
  return 0;
  }
  
+static void qtest_rsp_csr(QTestState *s, uint64_t *val)

+{
+gchar **args;
+uint64_t ret;
+int rc;
+
+args = qtest_rsp_args(s, 3);
+
+rc = qemu_strtou64(args[1], NULL, 16, &ret);
+g_assert(rc == 0);
+rc = qemu_strtou64(args[2], NULL, 16, val);
+g_assert(rc == 0);
+
+g_strfreev(args);
+}
+
+uint64_t qtest_csr_call(QTestState *s, const char *name,
+ uint64_t cpu, int csr,
+ uint64_t *val)
+{
+qtest_sendf(s, "csr %s 0x%"PRIx64" %d 0x%"PRIx64"\n",
+name, cpu, csr, *val);
+
+qtest_rsp_csr(s, val);
+return 0;
+}
+
  void qtest_add_func(const char *str, void (*fn)(void))
  {
  gchar *path = g_strdup_printf("/%s/%s", qtest_get_arch(), str);
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index c261b7e0b3..53cd8fe9f0 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -577,6 +577,20 @@ uint64_t qtest_rtas_call(QTestState *s, const char *name,
   uint32_t nargs, uint64_t args,
   uint32_t nret, uint64_t ret);
  
+/**

+ * qtest_csr_call:
+ * @s: #QTestState instance to operate on.
+ * @name: name of the command to call.
+ * @cpu: hart number.
+ * @csr: CSR number.
+ * @val: Value for reading/writing.
+ *
+ * Call an CSR function


Maybe mention RISC-V here in the comment?


+ */
+uint64_t qtest_csr_call(QTestState *s, const char *name,
+ uint64_t cpu, int csr,
+ unsigned long *val);
+
  /**
   * qtest_bufread:
   * @s: #QTestState instance to operate on.


For the tests/qtest part:
Acked-by: Thomas Huth 




[RFC PATCH v3 1/2] target/riscv: Add RISC-V CSR qtest support

2024-06-25 Thread Ivan Klokov
The RISC-V architecture supports the creation of custom
CSR-mapped devices. It would be convenient to test them in the same way
as MMIO-mapped devices. To do this, a new call has been added
to read/write CSR registers.

Signed-off-by: Ivan Klokov 
---
 target/riscv/cpu.c | 14 +++
 target/riscv/cpu.h |  3 +++
 target/riscv/csr.c | 53 +-
 tests/qtest/libqtest.c | 27 +
 tests/qtest/libqtest.h | 14 +++
 5 files changed, 110 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 69a08e8c2c..55cc01bfb3 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1148,7 +1148,17 @@ void riscv_cpu_finalize_features(RISCVCPU *cpu, Error 
**errp)
 }
 }
 }
+#ifndef CONFIG_USER_ONLY
+static void riscv_cpu_register_csr_qtest_callback(void)
+{
+static gsize reinit_done;
+if (g_once_init_enter(&reinit_done)) {
+qtest_set_command_cb(csr_qtest_callback);
 
+g_once_init_leave(&reinit_done, 1);
+}
+}
+#endif
 static void riscv_cpu_realize(DeviceState *dev, Error **errp)
 {
 CPUState *cs = CPU(dev);
@@ -1174,6 +1184,10 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 }
 
 riscv_cpu_register_gdb_regs_for_features(cs);
+#ifndef CONFIG_USER_ONLY
+/* register callback for csr qtests */
+riscv_cpu_register_csr_qtest_callback();
+#endif
 
 #ifndef CONFIG_USER_ONLY
 if (cpu->cfg.debug) {
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 6fe0d712b4..6d4bbec53c 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -32,6 +32,8 @@
 #include "cpu_cfg.h"
 #include "qapi/qapi-types-common.h"
 #include "cpu-qom.h"
+#include "qemu/cutils.h"
+#include "sysemu/qtest.h"
 
 typedef struct CPUArchState CPURISCVState;
 
@@ -813,6 +815,7 @@ bool riscv_cpu_accelerator_compatible(RISCVCPU *cpu);
 
 /* CSR function table */
 extern riscv_csr_operations csr_ops[CSR_TABLE_SIZE];
+bool csr_qtest_callback(CharBackend *chr, gchar **words);
 
 extern const bool valid_vm_1_10_32[], valid_vm_1_10_64[];
 
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 58ef7079dc..f4f5128c9c 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -29,7 +29,7 @@
 #include "sysemu/cpu-timers.h"
 #include "qemu/guest-random.h"
 #include "qapi/error.h"
-
+#include "tests/qtest/libqtest.h"
 
 /* CSR function table public API */
 void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops)
@@ -4549,6 +4549,57 @@ static RISCVException write_jvt(CPURISCVState *env, int 
csrno,
 return RISCV_EXCP_NONE;
 }
 
+#if !defined(CONFIG_USER_ONLY)
+static uint64_t csr_call(char *cmd, uint64_t cpu_num, int csrno,
+uint64_t *val)
+{
+RISCVCPU *cpu = RISCV_CPU(cpu_by_arch_id(cpu_num));
+CPURISCVState *env = &cpu->env;
+
+int ret = RISCV_EXCP_NONE;
+if (strcmp(cmd, "get_csr") == 0) {
+ret = riscv_csrrw(env, csrno, (target_ulong *)val, 0, 0);
+
+} else if (strcmp(cmd, "set_csr") == 0) {
+ret = riscv_csrrw(env, csrno, NULL, *(target_ulong *)val, 
MAKE_64BIT_MASK(0, TARGET_LONG_BITS));
+}
+
+if (ret == RISCV_EXCP_NONE) {
+ret = 0;
+} else {
+g_assert_not_reached();
+}
+
+return ret;
+}
+
+bool csr_qtest_callback(CharBackend *chr, gchar **words)
+{
+if (strcmp(words[0], "csr") == 0) {
+
+uint64_t res, cpu;
+
+uint64_t val;
+int rc, csr;
+
+rc = qemu_strtou64(words[2], NULL, 0, &cpu);
+g_assert(rc == 0);
+rc = qemu_strtoi(words[3], NULL, 0, &csr);
+g_assert(rc == 0);
+rc = qemu_strtou64(words[4], NULL, 0, &val);
+g_assert(rc == 0);
+res = csr_call(words[1], cpu, csr, &val);
+
+qtest_send_prefix(chr);
+qtest_sendf(chr, "OK %"PRIx64" "TARGET_FMT_lx"\n", res, 
(target_ulong)val);
+
+return true;
+}
+
+return false;
+}
+#endif
+
 /*
  * Control and Status Register function table
  * riscv_csr_operations::predicate() must be provided for an implemented CSR
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index c7f6897d78..f8c3ff15a9 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1205,6 +1205,33 @@ uint64_t qtest_rtas_call(QTestState *s, const char *name,
 return 0;
 }
 
+static void qtest_rsp_csr(QTestState *s, uint64_t *val)
+{
+gchar **args;
+uint64_t ret;
+int rc;
+
+args = qtest_rsp_args(s, 3);
+
+rc = qemu_strtou64(args[1], NULL, 16, &ret);
+g_assert(rc == 0);
+rc = qemu_strtou64(args[2], NULL, 16, val);
+g_assert(rc == 0);
+
+g_strfreev(args);
+}
+
+uint64_t qtest_csr_call(QTestState *s, const char *name,
+ uint64_t cpu, int csr,
+ uint64_t *val)
+{
+qtest_sendf(s, "csr %s 0x%"PRIx64" %d 0x%"PRIx64"\n",
+name, cpu, csr, *val);
+
+qtest_rsp_csr(s, val);
+return 0;
+}
+
 void qtest_add_func(const char *s