Ivan Klokov <ivan.klo...@syntacore.com> writes:
> 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.klo...@syntacore.com>
> ---
> hw/riscv/riscv_hart.c | 65 ++++++++++++++++++++++++++++++++++++++++++
> tests/qtest/libqtest.c | 27 ++++++++++++++++++
> tests/qtest/libqtest.h | 14 +++++++++
> 3 files changed, 106 insertions(+)
>
> diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
> index 613ea2aaa0..e65a1a28a1 100644
> --- a/hw/riscv/riscv_hart.c
> +++ b/hw/riscv/riscv_hart.c
> @@ -21,6 +21,8 @@
> #include "qemu/osdep.h"
> #include "qapi/error.h"
> #include "qemu/module.h"
> +#include "qemu/cutils.h"
> +#include "sysemu/qtest.h"
> #include "sysemu/reset.h"
> #include "hw/sysbus.h"
> #include "target/riscv/cpu.h"
> @@ -42,6 +44,66 @@ static void riscv_harts_cpu_reset(void *opaque)
> cpu_reset(CPU(cpu));
> }
>
> +#ifndef CONFIG_USER_ONLY
> +static uint64_t csr_call(char *cmd, uint64_t cpu_num, int csrno,
> + uint64_t *val)
Bad indentation here^
> +{
> + 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_csrr(env, csrno, (target_ulong *)val);
> + } else if (strcmp(cmd, "set_csr") == 0) {
> + ret = riscv_csrrw(env, csrno, NULL, *(target_ulong *)val,
> MAKE_64BIT_MASK(0, TARGET_LONG_BITS));
Please run scripts/checkpatch.pl before submitting:
$ ./scripts/checkpatch.pl
build/0001-target-riscv-Add-RISC-V-CSR-qtest-support.patch
ERROR: line over 90 characters
#46: FILE: hw/riscv/riscv_hart.c:58:
+ 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;
> +}
> +
> +static 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);
You could omit 'res', as it will always be 0, otherwise csr_call() will
have aborted already.
> +
> + return true;
> + }
> +
> + return false;
> +}
> +
> +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);
> + }
This seems to be the same as the simpler:
static GOnce once;
g_once(&once, (GThreadFunc)qtest_set_command_cb, csr_qtest_callback);
> +}
> +#endif
> +
> static bool riscv_hart_realize(RISCVHartArrayState *s, int idx,
> char *cpu_type, Error **errp)
> {
> @@ -49,6 +111,9 @@ static bool riscv_hart_realize(RISCVHartArrayState *s, int
> idx,
> qdev_prop_set_uint64(DEVICE(&s->harts[idx]), "resetvec", s->resetvec);
> s->harts[idx].env.mhartid = s->hartid_base + idx;
> qemu_register_reset(riscv_harts_cpu_reset, &s->harts[idx]);
> +#ifndef CONFIG_USER_ONLY
> + riscv_cpu_register_csr_qtest_callback();
> +#endif
This could be one level up at riscv_harts_realize().
> return qdev_realize(DEVICE(&s->harts[idx]), NULL, errp);
> }
>
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 9d07de1fbd..661f7f0e84 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -1202,6 +1202,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..b516a16bd4 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 RISC-V CSR read/write 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.
