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 <ivan.klokov@syntacore.com>
---
target/riscv/cpu.c | 13 +++++++++
target/riscv/cpu.h | 3 +++
target/riscv/csr.c | 49 +++++++++++++++++++++++++++++++++-
tests/qtest/libqos/csr.c | 42 +++++++++++++++++++++++++++++
tests/qtest/libqos/csr.h | 16 +++++++++++
tests/qtest/libqos/meson.build | 3 +++
tests/qtest/libqtest.c | 27 +++++++++++++++++++
tests/qtest/libqtest.h | 14 ++++++++++
8 files changed, 166 insertions(+), 1 deletion(-)
create mode 100644 tests/qtest/libqos/csr.c
create mode 100644 tests/qtest/libqos/csr.h
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 69a08e8c2c..f1df0f4de0 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1149,6 +1149,16 @@ void riscv_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
}
}
+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);
+ }
+}
+
static void riscv_cpu_realize(DeviceState *dev, Error **errp)
{
CPUState *cs = CPU(dev);
@@ -1175,6 +1185,9 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
riscv_cpu_register_gdb_regs_for_features(cs);
+ /* register callback for csr qtests */
+ riscv_cpu_register_csr_qtest_callback();
+
#ifndef CONFIG_USER_ONLY
if (cpu->cfg.debug) {
riscv_trigger_realize(&cpu->env);
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..82540ae5dc 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,53 @@ static RISCVException write_jvt(CPURISCVState *env, int csrno,
return RISCV_EXCP_NONE;
}
+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;
+ }
+
+ 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;
+}
+
/*
* Control and Status Register function table
* riscv_csr_operations::predicate() must be provided for an implemented CSR
diff --git a/tests/qtest/libqos/csr.c b/tests/qtest/libqos/csr.c
new file mode 100644
index 0000000000..2dc52fc442
--- /dev/null
+++ b/tests/qtest/libqos/csr.c
@@ -0,0 +1,42 @@
+/*
+ * QTest RISC-V CSR driver
+ *
+ * Copyright (c) 2024 Syntacore
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "../libqtest.h"
+#include "csr.h"
+
+static uint64_t qcsr_call(QTestState *qts, const char *name, uint64_t cpu,
+ int csrno, uint64_t *val)
+{
+ uint64_t res = 0;
+
+ res = qtest_csr_call(qts, name, cpu, csrno, val);
+
+ return res;
+}
+
+int qcsr_get_csr(QTestState *qts, uint64_t cpu,
+ int csrno, uint64_t *val)
+{
+ int res;
+
+ res = qcsr_call(qts, "get_csr", cpu, csrno, val);
+
+ return res;
+}
+
+int qcsr_set_csr(QTestState *qts, uint64_t cpu,
+ int csrno, uint64_t *val)
+{
+ int res;
+
+ res = qcsr_call(qts, "set_csr", cpu, csrno, val);
+
+ return res;
+}
diff --git a/tests/qtest/libqos/csr.h b/tests/qtest/libqos/csr.h
new file mode 100644
index 0000000000..d953735fe8
--- /dev/null
+++ b/tests/qtest/libqos/csr.h
@@ -0,0 +1,16 @@
+/*
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef LIBQOS_CSR_H
+#define LIBQOS_CSR_H
+
+int qcsr_get_csr(QTestState *qts, uint64_t cpu,
+ int csrno, uint64_t *val);
+
+int qcsr_set_csr(QTestState *qts, uint64_t cpu,
+ int csrno, uint64_t *val);
+
+
+#endif /* LIBQOS_CSR_H */
diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build
index 558eb4c24b..a944febbd8 100644
--- a/tests/qtest/libqos/meson.build
+++ b/tests/qtest/libqos/meson.build
@@ -25,6 +25,9 @@ libqos_srcs = files(
# usb
'usb.c',
+ #riscv csr
+ 'csr.c',
+
# qgraph devices:
'e1000e.c',
'i2c.c',
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 18e2f7f282..4667d8d873 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1200,6 +1200,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 beb96b18eb..a3336a0ea4 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -575,6 +575,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
+ */
+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.
--
2.34.1On 18/06/2024 08.44, 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 <ivan.klokov@syntacore.com>
> ---
...
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 58ef7079dc..82540ae5dc 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,53 @@ static RISCVException write_jvt(CPURISCVState *env, int csrno,
> return RISCV_EXCP_NONE;
> }
>
> +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;
> + }
Is there a reason for ignoring errors here? If not, I'd rather replace that
final if-statement with:
else {
g_assert_not_reached();
}
to make sure that mistakes in setting the right sub-command don't get
ignored without any error message.
> + 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;
> +}
> +
> /*
> * Control and Status Register function table
> * riscv_csr_operations::predicate() must be provided for an implemented CSR
> diff --git a/tests/qtest/libqos/csr.c b/tests/qtest/libqos/csr.c
> new file mode 100644
> index 0000000000..2dc52fc442
> --- /dev/null
> +++ b/tests/qtest/libqos/csr.c
> @@ -0,0 +1,42 @@
> +/*
> + * QTest RISC-V CSR driver
> + *
> + * Copyright (c) 2024 Syntacore
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "../libqtest.h"
> +#include "csr.h"
> +
> +static uint64_t qcsr_call(QTestState *qts, const char *name, uint64_t cpu,
> + int csrno, uint64_t *val)
> +{
> + uint64_t res = 0;
> +
> + res = qtest_csr_call(qts, name, cpu, csrno, val);
> +
> + return res;
> +}
> +
> +int qcsr_get_csr(QTestState *qts, uint64_t cpu,
> + int csrno, uint64_t *val)
> +{
> + int res;
> +
> + res = qcsr_call(qts, "get_csr", cpu, csrno, val);
> +
> + return res;
> +}
> +
> +int qcsr_set_csr(QTestState *qts, uint64_t cpu,
> + int csrno, uint64_t *val)
> +{
> + int res;
> +
> + res = qcsr_call(qts, "set_csr", cpu, csrno, val);
> +
> + return res;
> +}
Technically, there does not seem to be anything related to libqos in your
patch set. libqos is a framework for executing tests on various buses, e.g.
to test PCI devices on various host PCI bus implementations. All that is
triggered via qos-test.c. Your CSR test does not seem to fit into that
catogory, so please put that code rather directly in your riscv-csr-test.c
file instead. (unless you want to use it in a lot of other tests in the
future, too, then maybe you could move them as static inlines into the csr.h
header instead).
> diff --git a/tests/qtest/libqos/csr.h b/tests/qtest/libqos/csr.h
> new file mode 100644
> index 0000000000..d953735fe8
> --- /dev/null
> +++ b/tests/qtest/libqos/csr.h
Again, not related to libqos, please move it up to the qtest folder itself.
> @@ -0,0 +1,16 @@
> +/*
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef LIBQOS_CSR_H
> +#define LIBQOS_CSR_H
> +
> +int qcsr_get_csr(QTestState *qts, uint64_t cpu,
> + int csrno, uint64_t *val);
> +
> +int qcsr_set_csr(QTestState *qts, uint64_t cpu,
> + int csrno, uint64_t *val);
> +
> +
> +#endif /* LIBQOS_CSR_H */
> diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build
> index 558eb4c24b..a944febbd8 100644
> --- a/tests/qtest/libqos/meson.build
> +++ b/tests/qtest/libqos/meson.build
> @@ -25,6 +25,9 @@ libqos_srcs = files(
> # usb
> 'usb.c',
>
> + #riscv csr
> + 'csr.c',
> +
> # qgraph devices:
> 'e1000e.c',
> 'i2c.c',
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 18e2f7f282..4667d8d873 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -1200,6 +1200,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);
Just a matter of taste, but I think I'd rather inline the contents of
qtest_rsp_csr() here since both functions are not very big yet.
(unless you need qtest_rsp_csr() in another function later, then it's of
course better to keep it separate)
> + return 0;
> +}
Thomas
© 2016 - 2024 Red Hat, Inc.