* [PATCH v2 1/9] target/riscv/kvm: minor fixes/tweaks
2025-04-25 11:36 [PATCH v2 0/9] target/riscv/kvm: CSR related fixes Daniel Henrique Barboza
@ 2025-04-25 11:36 ` Daniel Henrique Barboza
2025-04-25 11:43 ` Andrew Jones
2025-04-25 11:36 ` [PATCH v2 2/9] target/riscv/kvm: fix leak in kvm_riscv_init_multiext_cfg() Daniel Henrique Barboza
` (7 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Daniel Henrique Barboza @ 2025-04-25 11:36 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, palmer,
ajones, Daniel Henrique Barboza
Remove an unused 'KVMScratchCPU' pointer argument in
kvm_riscv_check_sbi_dbcn_support().
Put kvm_riscv_reset_regs_csr() after kvm_riscv_put_regs_csr(). This will
make a future patch diff easier to read, when changes in
kvm_riscv_reset_regs_csr() and kvm_riscv_get_regs_csr() will be made.
Fixes: a6b53378f5 ("target/riscv/kvm: implement SBI debug console (DBCN) calls")
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
target/riscv/kvm/kvm-cpu.c | 29 ++++++++++++++---------------
1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 9214ce490c..accad4c28e 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -613,19 +613,6 @@ static int kvm_riscv_put_regs_core(CPUState *cs)
return ret;
}
-static void kvm_riscv_reset_regs_csr(CPURISCVState *env)
-{
- env->mstatus = 0;
- env->mie = 0;
- env->stvec = 0;
- env->sscratch = 0;
- env->sepc = 0;
- env->scause = 0;
- env->stval = 0;
- env->mip = 0;
- env->satp = 0;
-}
-
static int kvm_riscv_get_regs_csr(CPUState *cs)
{
CPURISCVState *env = &RISCV_CPU(cs)->env;
@@ -660,6 +647,19 @@ static int kvm_riscv_put_regs_csr(CPUState *cs)
return 0;
}
+static void kvm_riscv_reset_regs_csr(CPURISCVState *env)
+{
+ env->mstatus = 0;
+ env->mie = 0;
+ env->stvec = 0;
+ env->sscratch = 0;
+ env->sepc = 0;
+ env->scause = 0;
+ env->stval = 0;
+ env->mip = 0;
+ env->satp = 0;
+}
+
static int kvm_riscv_get_regs_fp(CPUState *cs)
{
int ret = 0;
@@ -1078,7 +1078,6 @@ static int uint64_cmp(const void *a, const void *b)
}
static void kvm_riscv_check_sbi_dbcn_support(RISCVCPU *cpu,
- KVMScratchCPU *kvmcpu,
struct kvm_reg_list *reglist)
{
struct kvm_reg_list *reg_search;
@@ -1197,7 +1196,7 @@ static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
kvm_riscv_read_vlenb(cpu, kvmcpu, reglist);
}
- kvm_riscv_check_sbi_dbcn_support(cpu, kvmcpu, reglist);
+ kvm_riscv_check_sbi_dbcn_support(cpu, reglist);
}
static void riscv_init_kvm_registers(Object *cpu_obj)
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 1/9] target/riscv/kvm: minor fixes/tweaks
2025-04-25 11:36 ` [PATCH v2 1/9] target/riscv/kvm: minor fixes/tweaks Daniel Henrique Barboza
@ 2025-04-25 11:43 ` Andrew Jones
0 siblings, 0 replies; 20+ messages in thread
From: Andrew Jones @ 2025-04-25 11:43 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, liwei1518, zhiwei_liu,
palmer
On Fri, Apr 25, 2025 at 08:36:57AM -0300, Daniel Henrique Barboza wrote:
> Remove an unused 'KVMScratchCPU' pointer argument in
> kvm_riscv_check_sbi_dbcn_support().
>
> Put kvm_riscv_reset_regs_csr() after kvm_riscv_put_regs_csr(). This will
> make a future patch diff easier to read, when changes in
> kvm_riscv_reset_regs_csr() and kvm_riscv_get_regs_csr() will be made.
>
> Fixes: a6b53378f5 ("target/riscv/kvm: implement SBI debug console (DBCN) calls")
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> target/riscv/kvm/kvm-cpu.c | 29 ++++++++++++++---------------
> 1 file changed, 14 insertions(+), 15 deletions(-)
>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 2/9] target/riscv/kvm: fix leak in kvm_riscv_init_multiext_cfg()
2025-04-25 11:36 [PATCH v2 0/9] target/riscv/kvm: CSR related fixes Daniel Henrique Barboza
2025-04-25 11:36 ` [PATCH v2 1/9] target/riscv/kvm: minor fixes/tweaks Daniel Henrique Barboza
@ 2025-04-25 11:36 ` Daniel Henrique Barboza
2025-04-25 11:44 ` Andrew Jones
2025-04-25 11:36 ` [PATCH v2 3/9] target/riscv/kvm: turn u32/u64 reg functions in macros Daniel Henrique Barboza
` (6 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Daniel Henrique Barboza @ 2025-04-25 11:36 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, palmer,
ajones, Daniel Henrique Barboza
'reglist' is being g+malloc'ed but never freed.
Reported-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
target/riscv/kvm/kvm-cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index accad4c28e..6ba122f360 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -1119,10 +1119,10 @@ static void kvm_riscv_read_vlenb(RISCVCPU *cpu, KVMScratchCPU *kvmcpu,
static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
{
+ g_autofree struct kvm_reg_list *reglist = NULL;
KVMCPUConfig *multi_ext_cfg;
struct kvm_one_reg reg;
struct kvm_reg_list rl_struct;
- struct kvm_reg_list *reglist;
uint64_t val, reg_id, *reg_search;
int i, ret;
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 2/9] target/riscv/kvm: fix leak in kvm_riscv_init_multiext_cfg()
2025-04-25 11:36 ` [PATCH v2 2/9] target/riscv/kvm: fix leak in kvm_riscv_init_multiext_cfg() Daniel Henrique Barboza
@ 2025-04-25 11:44 ` Andrew Jones
0 siblings, 0 replies; 20+ messages in thread
From: Andrew Jones @ 2025-04-25 11:44 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, liwei1518, zhiwei_liu,
palmer
On Fri, Apr 25, 2025 at 08:36:58AM -0300, Daniel Henrique Barboza wrote:
> 'reglist' is being g+malloc'ed but never freed.
g_malloc'ed
>
> Reported-by: Andrew Jones <ajones@ventanamicro.com>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> target/riscv/kvm/kvm-cpu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index accad4c28e..6ba122f360 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -1119,10 +1119,10 @@ static void kvm_riscv_read_vlenb(RISCVCPU *cpu, KVMScratchCPU *kvmcpu,
>
> static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
> {
> + g_autofree struct kvm_reg_list *reglist = NULL;
> KVMCPUConfig *multi_ext_cfg;
> struct kvm_one_reg reg;
> struct kvm_reg_list rl_struct;
> - struct kvm_reg_list *reglist;
> uint64_t val, reg_id, *reg_search;
> int i, ret;
>
> --
> 2.49.0
>
Otherwise,
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 3/9] target/riscv/kvm: turn u32/u64 reg functions in macros
2025-04-25 11:36 [PATCH v2 0/9] target/riscv/kvm: CSR related fixes Daniel Henrique Barboza
2025-04-25 11:36 ` [PATCH v2 1/9] target/riscv/kvm: minor fixes/tweaks Daniel Henrique Barboza
2025-04-25 11:36 ` [PATCH v2 2/9] target/riscv/kvm: fix leak in kvm_riscv_init_multiext_cfg() Daniel Henrique Barboza
@ 2025-04-25 11:36 ` Daniel Henrique Barboza
2025-04-25 11:45 ` Andrew Jones
2025-04-25 11:37 ` [PATCH v2 4/9] target/riscv/kvm: turn kvm_riscv_reg_id_ulong() in a macro Daniel Henrique Barboza
` (5 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Daniel Henrique Barboza @ 2025-04-25 11:36 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, palmer,
ajones, Daniel Henrique Barboza
This change is motivated by a future change w.r.t CSRs management. We
want to handle them the same way as KVM extensions, i.e. a static array
with KVMCPUConfig objs that will be read/write during init and so on.
But to do that properly we must be able to declare a static array that
hold KVM regs.
C does not allow to init static arrays and use functions as
initializers, e.g. we can't do:
.kvm_reg_id = kvm_riscv_reg_id_ulong(...)
When instantiating the array. We can do that with macros though, so our
goal is turn kvm_riscv_reg_ulong() in a macro. It is cleaner to turn
every other reg_id_*() function in macros, and ulong will end up using
the macros for u32 and u64, so we'll start with them.
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
target/riscv/kvm/kvm-cpu.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 6ba122f360..c91ecdfe59 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -58,6 +58,12 @@ void riscv_kvm_aplic_request(void *opaque, int irq, int level)
static bool cap_has_mp_state;
+#define KVM_RISCV_REG_ID_U32(type, idx) (KVM_REG_RISCV | KVM_REG_SIZE_U32 | \
+ type | idx)
+
+#define KVM_RISCV_REG_ID_U64(type, idx) (KVM_REG_RISCV | KVM_REG_SIZE_U64 | \
+ type | idx)
+
static uint64_t kvm_riscv_reg_id_ulong(CPURISCVState *env, uint64_t type,
uint64_t idx)
{
@@ -76,16 +82,6 @@ static uint64_t kvm_riscv_reg_id_ulong(CPURISCVState *env, uint64_t type,
return id;
}
-static uint64_t kvm_riscv_reg_id_u32(uint64_t type, uint64_t idx)
-{
- return KVM_REG_RISCV | KVM_REG_SIZE_U32 | type | idx;
-}
-
-static uint64_t kvm_riscv_reg_id_u64(uint64_t type, uint64_t idx)
-{
- return KVM_REG_RISCV | KVM_REG_SIZE_U64 | type | idx;
-}
-
static uint64_t kvm_encode_reg_size_id(uint64_t id, size_t size_b)
{
uint64_t size_ctz = __builtin_ctz(size_b);
@@ -119,12 +115,12 @@ static uint64_t kvm_riscv_vector_reg_id(RISCVCPU *cpu,
kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CONFIG, \
KVM_REG_RISCV_CONFIG_REG(name))
-#define RISCV_TIMER_REG(name) kvm_riscv_reg_id_u64(KVM_REG_RISCV_TIMER, \
+#define RISCV_TIMER_REG(name) KVM_RISCV_REG_ID_U64(KVM_REG_RISCV_TIMER, \
KVM_REG_RISCV_TIMER_REG(name))
-#define RISCV_FP_F_REG(idx) kvm_riscv_reg_id_u32(KVM_REG_RISCV_FP_F, idx)
+#define RISCV_FP_F_REG(idx) KVM_RISCV_REG_ID_U32(KVM_REG_RISCV_FP_F, idx)
-#define RISCV_FP_D_REG(idx) kvm_riscv_reg_id_u64(KVM_REG_RISCV_FP_D, idx)
+#define RISCV_FP_D_REG(idx) KVM_RISCV_REG_ID_U64(KVM_REG_RISCV_FP_D, idx)
#define RISCV_VECTOR_CSR_REG(env, name) \
kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_VECTOR, \
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 3/9] target/riscv/kvm: turn u32/u64 reg functions in macros
2025-04-25 11:36 ` [PATCH v2 3/9] target/riscv/kvm: turn u32/u64 reg functions in macros Daniel Henrique Barboza
@ 2025-04-25 11:45 ` Andrew Jones
0 siblings, 0 replies; 20+ messages in thread
From: Andrew Jones @ 2025-04-25 11:45 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, liwei1518, zhiwei_liu,
palmer
sed s/in/into/ <<<$SUBJECT
On Fri, Apr 25, 2025 at 08:36:59AM -0300, Daniel Henrique Barboza wrote:
> This change is motivated by a future change w.r.t CSRs management. We
> want to handle them the same way as KVM extensions, i.e. a static array
> with KVMCPUConfig objs that will be read/write during init and so on.
> But to do that properly we must be able to declare a static array that
> hold KVM regs.
>
> C does not allow to init static arrays and use functions as
> initializers, e.g. we can't do:
>
> .kvm_reg_id = kvm_riscv_reg_id_ulong(...)
>
> When instantiating the array. We can do that with macros though, so our
> goal is turn kvm_riscv_reg_ulong() in a macro. It is cleaner to turn
> every other reg_id_*() function in macros, and ulong will end up using
> the macros for u32 and u64, so we'll start with them.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> target/riscv/kvm/kvm-cpu.c | 22 +++++++++-------------
> 1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 6ba122f360..c91ecdfe59 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -58,6 +58,12 @@ void riscv_kvm_aplic_request(void *opaque, int irq, int level)
>
> static bool cap_has_mp_state;
>
> +#define KVM_RISCV_REG_ID_U32(type, idx) (KVM_REG_RISCV | KVM_REG_SIZE_U32 | \
> + type | idx)
> +
> +#define KVM_RISCV_REG_ID_U64(type, idx) (KVM_REG_RISCV | KVM_REG_SIZE_U64 | \
> + type | idx)
> +
> static uint64_t kvm_riscv_reg_id_ulong(CPURISCVState *env, uint64_t type,
> uint64_t idx)
> {
> @@ -76,16 +82,6 @@ static uint64_t kvm_riscv_reg_id_ulong(CPURISCVState *env, uint64_t type,
> return id;
> }
>
> -static uint64_t kvm_riscv_reg_id_u32(uint64_t type, uint64_t idx)
> -{
> - return KVM_REG_RISCV | KVM_REG_SIZE_U32 | type | idx;
> -}
> -
> -static uint64_t kvm_riscv_reg_id_u64(uint64_t type, uint64_t idx)
> -{
> - return KVM_REG_RISCV | KVM_REG_SIZE_U64 | type | idx;
> -}
> -
> static uint64_t kvm_encode_reg_size_id(uint64_t id, size_t size_b)
> {
> uint64_t size_ctz = __builtin_ctz(size_b);
> @@ -119,12 +115,12 @@ static uint64_t kvm_riscv_vector_reg_id(RISCVCPU *cpu,
> kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CONFIG, \
> KVM_REG_RISCV_CONFIG_REG(name))
>
> -#define RISCV_TIMER_REG(name) kvm_riscv_reg_id_u64(KVM_REG_RISCV_TIMER, \
> +#define RISCV_TIMER_REG(name) KVM_RISCV_REG_ID_U64(KVM_REG_RISCV_TIMER, \
> KVM_REG_RISCV_TIMER_REG(name))
>
> -#define RISCV_FP_F_REG(idx) kvm_riscv_reg_id_u32(KVM_REG_RISCV_FP_F, idx)
> +#define RISCV_FP_F_REG(idx) KVM_RISCV_REG_ID_U32(KVM_REG_RISCV_FP_F, idx)
>
> -#define RISCV_FP_D_REG(idx) kvm_riscv_reg_id_u64(KVM_REG_RISCV_FP_D, idx)
> +#define RISCV_FP_D_REG(idx) KVM_RISCV_REG_ID_U64(KVM_REG_RISCV_FP_D, idx)
>
> #define RISCV_VECTOR_CSR_REG(env, name) \
> kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_VECTOR, \
> --
> 2.49.0
>
Otherwise,
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 4/9] target/riscv/kvm: turn kvm_riscv_reg_id_ulong() in a macro
2025-04-25 11:36 [PATCH v2 0/9] target/riscv/kvm: CSR related fixes Daniel Henrique Barboza
` (2 preceding siblings ...)
2025-04-25 11:36 ` [PATCH v2 3/9] target/riscv/kvm: turn u32/u64 reg functions in macros Daniel Henrique Barboza
@ 2025-04-25 11:37 ` Daniel Henrique Barboza
2025-04-25 11:48 ` Andrew Jones
2025-04-25 11:37 ` [PATCH v2 5/9] target/riscv/kvm: add kvm_csr_cfgs[] Daniel Henrique Barboza
` (4 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Daniel Henrique Barboza @ 2025-04-25 11:37 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, palmer,
ajones, Daniel Henrique Barboza
We need the reg_id_ulong() helper to be a macro to be able to create a
static array of KVMCPUConfig that will hold CSR information.
Despite the amount of changes all of them are tedious/trivial:
- replace instances of "kvm_riscv_reg_id_ulong" with
"KVM_RISCV_REG_ID_ULONG";
- RISCV_CORE_REG(), RISCV_CSR_REG(), RISCV_CONFIG_REG() and
RISCV_VECTOR_CSR_REG() only receives one 'name' arg. Remove unneeded
'env' variables when applicable.
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
target/riscv/kvm/kvm-cpu.c | 99 ++++++++++++++++----------------------
1 file changed, 41 insertions(+), 58 deletions(-)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index c91ecdfe59..fd66bc1759 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -64,23 +64,11 @@ static bool cap_has_mp_state;
#define KVM_RISCV_REG_ID_U64(type, idx) (KVM_REG_RISCV | KVM_REG_SIZE_U64 | \
type | idx)
-static uint64_t kvm_riscv_reg_id_ulong(CPURISCVState *env, uint64_t type,
- uint64_t idx)
-{
- uint64_t id = KVM_REG_RISCV | type | idx;
-
- switch (riscv_cpu_mxl(env)) {
- case MXL_RV32:
- id |= KVM_REG_SIZE_U32;
- break;
- case MXL_RV64:
- id |= KVM_REG_SIZE_U64;
- break;
- default:
- g_assert_not_reached();
- }
- return id;
-}
+#if defined(TARGET_RISCV64)
+#define KVM_RISCV_REG_ID_ULONG(type, idx) KVM_RISCV_REG_ID_U64(type, idx)
+#else
+#define KVM_RISCV_REG_ID_ULONG(type, idx) KVM_RISCV_REG_ID_U32(type, idx)
+#endif
static uint64_t kvm_encode_reg_size_id(uint64_t id, size_t size_b)
{
@@ -103,16 +91,16 @@ static uint64_t kvm_riscv_vector_reg_id(RISCVCPU *cpu,
return kvm_encode_reg_size_id(id, size_b);
}
-#define RISCV_CORE_REG(env, name) \
- kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, \
+#define RISCV_CORE_REG(name) \
+ KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CORE, \
KVM_REG_RISCV_CORE_REG(name))
-#define RISCV_CSR_REG(env, name) \
- kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CSR, \
+#define RISCV_CSR_REG(name) \
+ KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CSR, \
KVM_REG_RISCV_CSR_REG(name))
-#define RISCV_CONFIG_REG(env, name) \
- kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CONFIG, \
+#define RISCV_CONFIG_REG(name) \
+ KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CONFIG, \
KVM_REG_RISCV_CONFIG_REG(name))
#define RISCV_TIMER_REG(name) KVM_RISCV_REG_ID_U64(KVM_REG_RISCV_TIMER, \
@@ -122,13 +110,13 @@ static uint64_t kvm_riscv_vector_reg_id(RISCVCPU *cpu,
#define RISCV_FP_D_REG(idx) KVM_RISCV_REG_ID_U64(KVM_REG_RISCV_FP_D, idx)
-#define RISCV_VECTOR_CSR_REG(env, name) \
- kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_VECTOR, \
+#define RISCV_VECTOR_CSR_REG(name) \
+ KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_VECTOR, \
KVM_REG_RISCV_VECTOR_CSR_REG(name))
#define KVM_RISCV_GET_CSR(cs, env, csr, reg) \
do { \
- int _ret = kvm_get_one_reg(cs, RISCV_CSR_REG(env, csr), ®); \
+ int _ret = kvm_get_one_reg(cs, RISCV_CSR_REG(csr), ®); \
if (_ret) { \
return _ret; \
} \
@@ -136,7 +124,7 @@ static uint64_t kvm_riscv_vector_reg_id(RISCVCPU *cpu,
#define KVM_RISCV_SET_CSR(cs, env, csr, reg) \
do { \
- int _ret = kvm_set_one_reg(cs, RISCV_CSR_REG(env, csr), ®); \
+ int _ret = kvm_set_one_reg(cs, RISCV_CSR_REG(csr), ®); \
if (_ret) { \
return _ret; \
} \
@@ -244,7 +232,7 @@ static void kvm_riscv_update_cpu_misa_ext(RISCVCPU *cpu, CPUState *cs)
/* If we're here we're going to disable the MISA bit */
reg = 0;
- id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_ISA_EXT,
+ id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_ISA_EXT,
misa_cfg->kvm_reg_id);
ret = kvm_set_one_reg(cs, id, ®);
if (ret != 0) {
@@ -430,7 +418,6 @@ static KVMCPUConfig kvm_sbi_dbcn = {
static void kvm_riscv_update_cpu_cfg_isa_ext(RISCVCPU *cpu, CPUState *cs)
{
- CPURISCVState *env = &cpu->env;
uint64_t id, reg;
int i, ret;
@@ -441,7 +428,7 @@ static void kvm_riscv_update_cpu_cfg_isa_ext(RISCVCPU *cpu, CPUState *cs)
continue;
}
- id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_ISA_EXT,
+ id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_ISA_EXT,
multi_ext_cfg->kvm_reg_id);
reg = kvm_cpu_cfg_get(cpu, multi_ext_cfg);
ret = kvm_set_one_reg(cs, id, ®);
@@ -566,14 +553,14 @@ static int kvm_riscv_get_regs_core(CPUState *cs)
target_ulong reg;
CPURISCVState *env = &RISCV_CPU(cs)->env;
- ret = kvm_get_one_reg(cs, RISCV_CORE_REG(env, regs.pc), ®);
+ ret = kvm_get_one_reg(cs, RISCV_CORE_REG(regs.pc), ®);
if (ret) {
return ret;
}
env->pc = reg;
for (i = 1; i < 32; i++) {
- uint64_t id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, i);
+ uint64_t id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CORE, i);
ret = kvm_get_one_reg(cs, id, ®);
if (ret) {
return ret;
@@ -592,13 +579,13 @@ static int kvm_riscv_put_regs_core(CPUState *cs)
CPURISCVState *env = &RISCV_CPU(cs)->env;
reg = env->pc;
- ret = kvm_set_one_reg(cs, RISCV_CORE_REG(env, regs.pc), ®);
+ ret = kvm_set_one_reg(cs, RISCV_CORE_REG(regs.pc), ®);
if (ret) {
return ret;
}
for (i = 1; i < 32; i++) {
- uint64_t id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, i);
+ uint64_t id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CORE, i);
reg = env->gpr[i];
ret = kvm_set_one_reg(cs, id, ®);
if (ret) {
@@ -796,26 +783,26 @@ static int kvm_riscv_get_regs_vector(CPUState *cs)
return 0;
}
- ret = kvm_get_one_reg(cs, RISCV_VECTOR_CSR_REG(env, vstart), ®);
+ ret = kvm_get_one_reg(cs, RISCV_VECTOR_CSR_REG(vstart), ®);
if (ret) {
return ret;
}
env->vstart = reg;
- ret = kvm_get_one_reg(cs, RISCV_VECTOR_CSR_REG(env, vl), ®);
+ ret = kvm_get_one_reg(cs, RISCV_VECTOR_CSR_REG(vl), ®);
if (ret) {
return ret;
}
env->vl = reg;
- ret = kvm_get_one_reg(cs, RISCV_VECTOR_CSR_REG(env, vtype), ®);
+ ret = kvm_get_one_reg(cs, RISCV_VECTOR_CSR_REG(vtype), ®);
if (ret) {
return ret;
}
env->vtype = reg;
if (kvm_v_vlenb.supported) {
- ret = kvm_get_one_reg(cs, RISCV_VECTOR_CSR_REG(env, vlenb), ®);
+ ret = kvm_get_one_reg(cs, RISCV_VECTOR_CSR_REG(vlenb), ®);
if (ret) {
return ret;
}
@@ -853,26 +840,26 @@ static int kvm_riscv_put_regs_vector(CPUState *cs)
}
reg = env->vstart;
- ret = kvm_set_one_reg(cs, RISCV_VECTOR_CSR_REG(env, vstart), ®);
+ ret = kvm_set_one_reg(cs, RISCV_VECTOR_CSR_REG(vstart), ®);
if (ret) {
return ret;
}
reg = env->vl;
- ret = kvm_set_one_reg(cs, RISCV_VECTOR_CSR_REG(env, vl), ®);
+ ret = kvm_set_one_reg(cs, RISCV_VECTOR_CSR_REG(vl), ®);
if (ret) {
return ret;
}
reg = env->vtype;
- ret = kvm_set_one_reg(cs, RISCV_VECTOR_CSR_REG(env, vtype), ®);
+ ret = kvm_set_one_reg(cs, RISCV_VECTOR_CSR_REG(vtype), ®);
if (ret) {
return ret;
}
if (kvm_v_vlenb.supported) {
reg = cpu->cfg.vlenb;
- ret = kvm_set_one_reg(cs, RISCV_VECTOR_CSR_REG(env, vlenb), ®);
+ ret = kvm_set_one_reg(cs, RISCV_VECTOR_CSR_REG(vlenb), ®);
for (int i = 0; i < 32; i++) {
/*
@@ -951,25 +938,24 @@ static void kvm_riscv_destroy_scratch_vcpu(KVMScratchCPU *scratch)
static void kvm_riscv_init_machine_ids(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
{
- CPURISCVState *env = &cpu->env;
struct kvm_one_reg reg;
int ret;
- reg.id = RISCV_CONFIG_REG(env, mvendorid);
+ reg.id = RISCV_CONFIG_REG(mvendorid);
reg.addr = (uint64_t)&cpu->cfg.mvendorid;
ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
if (ret != 0) {
error_report("Unable to retrieve mvendorid from host, error %d", ret);
}
- reg.id = RISCV_CONFIG_REG(env, marchid);
+ reg.id = RISCV_CONFIG_REG(marchid);
reg.addr = (uint64_t)&cpu->cfg.marchid;
ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
if (ret != 0) {
error_report("Unable to retrieve marchid from host, error %d", ret);
}
- reg.id = RISCV_CONFIG_REG(env, mimpid);
+ reg.id = RISCV_CONFIG_REG(mimpid);
reg.addr = (uint64_t)&cpu->cfg.mimpid;
ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
if (ret != 0) {
@@ -984,7 +970,7 @@ static void kvm_riscv_init_misa_ext_mask(RISCVCPU *cpu,
struct kvm_one_reg reg;
int ret;
- reg.id = RISCV_CONFIG_REG(env, isa);
+ reg.id = RISCV_CONFIG_REG(isa);
reg.addr = (uint64_t)&env->misa_ext_mask;
ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
@@ -1001,11 +987,10 @@ static void kvm_riscv_init_misa_ext_mask(RISCVCPU *cpu,
static void kvm_riscv_read_cbomz_blksize(RISCVCPU *cpu, KVMScratchCPU *kvmcpu,
KVMCPUConfig *cbomz_cfg)
{
- CPURISCVState *env = &cpu->env;
struct kvm_one_reg reg;
int ret;
- reg.id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CONFIG,
+ reg.id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CONFIG,
cbomz_cfg->kvm_reg_id);
reg.addr = (uint64_t)kvmconfig_get_cfg_addr(cpu, cbomz_cfg);
ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
@@ -1019,7 +1004,6 @@ static void kvm_riscv_read_cbomz_blksize(RISCVCPU *cpu, KVMScratchCPU *kvmcpu,
static void kvm_riscv_read_multiext_legacy(RISCVCPU *cpu,
KVMScratchCPU *kvmcpu)
{
- CPURISCVState *env = &cpu->env;
uint64_t val;
int i, ret;
@@ -1027,7 +1011,7 @@ static void kvm_riscv_read_multiext_legacy(RISCVCPU *cpu,
KVMCPUConfig *multi_ext_cfg = &kvm_multi_ext_cfgs[i];
struct kvm_one_reg reg;
- reg.id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_ISA_EXT,
+ reg.id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_ISA_EXT,
multi_ext_cfg->kvm_reg_id);
reg.addr = (uint64_t)&val;
ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
@@ -1159,7 +1143,7 @@ static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
for (i = 0; i < ARRAY_SIZE(kvm_multi_ext_cfgs); i++) {
multi_ext_cfg = &kvm_multi_ext_cfgs[i];
- reg_id = kvm_riscv_reg_id_ulong(&cpu->env, KVM_REG_RISCV_ISA_EXT,
+ reg_id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_ISA_EXT,
multi_ext_cfg->kvm_reg_id);
reg_search = bsearch(®_id, reglist->reg, reglist->n,
sizeof(uint64_t), uint64_cmp);
@@ -1338,12 +1322,11 @@ void kvm_arch_init_irq_routing(KVMState *s)
static int kvm_vcpu_set_machine_ids(RISCVCPU *cpu, CPUState *cs)
{
- CPURISCVState *env = &cpu->env;
target_ulong reg;
uint64_t id;
int ret;
- id = RISCV_CONFIG_REG(env, mvendorid);
+ id = RISCV_CONFIG_REG(mvendorid);
/*
* cfg.mvendorid is an uint32 but a target_ulong will
* be written. Assign it to a target_ulong var to avoid
@@ -1355,13 +1338,13 @@ static int kvm_vcpu_set_machine_ids(RISCVCPU *cpu, CPUState *cs)
return ret;
}
- id = RISCV_CONFIG_REG(env, marchid);
+ id = RISCV_CONFIG_REG(marchid);
ret = kvm_set_one_reg(cs, id, &cpu->cfg.marchid);
if (ret != 0) {
return ret;
}
- id = RISCV_CONFIG_REG(env, mimpid);
+ id = RISCV_CONFIG_REG(mimpid);
ret = kvm_set_one_reg(cs, id, &cpu->cfg.mimpid);
return ret;
@@ -1911,7 +1894,7 @@ void riscv_kvm_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
if (cpu->cfg.ext_zicbom &&
riscv_cpu_option_set(kvm_cbom_blocksize.name)) {
- reg.id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CONFIG,
+ reg.id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CONFIG,
kvm_cbom_blocksize.kvm_reg_id);
reg.addr = (uint64_t)&val;
ret = ioctl(kvmcpu.cpufd, KVM_GET_ONE_REG, ®);
@@ -1930,7 +1913,7 @@ void riscv_kvm_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
if (cpu->cfg.ext_zicboz &&
riscv_cpu_option_set(kvm_cboz_blocksize.name)) {
- reg.id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CONFIG,
+ reg.id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CONFIG,
kvm_cboz_blocksize.kvm_reg_id);
reg.addr = (uint64_t)&val;
ret = ioctl(kvmcpu.cpufd, KVM_GET_ONE_REG, ®);
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 4/9] target/riscv/kvm: turn kvm_riscv_reg_id_ulong() in a macro
2025-04-25 11:37 ` [PATCH v2 4/9] target/riscv/kvm: turn kvm_riscv_reg_id_ulong() in a macro Daniel Henrique Barboza
@ 2025-04-25 11:48 ` Andrew Jones
0 siblings, 0 replies; 20+ messages in thread
From: Andrew Jones @ 2025-04-25 11:48 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, liwei1518, zhiwei_liu,
palmer
s/in/into/ <<<$SUBJECT
On Fri, Apr 25, 2025 at 08:37:00AM -0300, Daniel Henrique Barboza wrote:
> We need the reg_id_ulong() helper to be a macro to be able to create a
> static array of KVMCPUConfig that will hold CSR information.
>
> Despite the amount of changes all of them are tedious/trivial:
>
> - replace instances of "kvm_riscv_reg_id_ulong" with
> "KVM_RISCV_REG_ID_ULONG";
>
> - RISCV_CORE_REG(), RISCV_CSR_REG(), RISCV_CONFIG_REG() and
> RISCV_VECTOR_CSR_REG() only receives one 'name' arg. Remove unneeded
> 'env' variables when applicable.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> target/riscv/kvm/kvm-cpu.c | 99 ++++++++++++++++----------------------
> 1 file changed, 41 insertions(+), 58 deletions(-)
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index c91ecdfe59..fd66bc1759 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -64,23 +64,11 @@ static bool cap_has_mp_state;
> #define KVM_RISCV_REG_ID_U64(type, idx) (KVM_REG_RISCV | KVM_REG_SIZE_U64 | \
> type | idx)
>
> -static uint64_t kvm_riscv_reg_id_ulong(CPURISCVState *env, uint64_t type,
> - uint64_t idx)
> -{
> - uint64_t id = KVM_REG_RISCV | type | idx;
> -
> - switch (riscv_cpu_mxl(env)) {
> - case MXL_RV32:
> - id |= KVM_REG_SIZE_U32;
> - break;
> - case MXL_RV64:
> - id |= KVM_REG_SIZE_U64;
> - break;
> - default:
> - g_assert_not_reached();
> - }
> - return id;
> -}
> +#if defined(TARGET_RISCV64)
> +#define KVM_RISCV_REG_ID_ULONG(type, idx) KVM_RISCV_REG_ID_U64(type, idx)
> +#else
> +#define KVM_RISCV_REG_ID_ULONG(type, idx) KVM_RISCV_REG_ID_U32(type, idx)
> +#endif
>
> static uint64_t kvm_encode_reg_size_id(uint64_t id, size_t size_b)
> {
> @@ -103,16 +91,16 @@ static uint64_t kvm_riscv_vector_reg_id(RISCVCPU *cpu,
> return kvm_encode_reg_size_id(id, size_b);
> }
>
> -#define RISCV_CORE_REG(env, name) \
> - kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, \
> +#define RISCV_CORE_REG(name) \
> + KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CORE, \
> KVM_REG_RISCV_CORE_REG(name))
>
> -#define RISCV_CSR_REG(env, name) \
> - kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CSR, \
> +#define RISCV_CSR_REG(name) \
> + KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CSR, \
> KVM_REG_RISCV_CSR_REG(name))
>
> -#define RISCV_CONFIG_REG(env, name) \
> - kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CONFIG, \
> +#define RISCV_CONFIG_REG(name) \
> + KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CONFIG, \
> KVM_REG_RISCV_CONFIG_REG(name))
>
> #define RISCV_TIMER_REG(name) KVM_RISCV_REG_ID_U64(KVM_REG_RISCV_TIMER, \
> @@ -122,13 +110,13 @@ static uint64_t kvm_riscv_vector_reg_id(RISCVCPU *cpu,
>
> #define RISCV_FP_D_REG(idx) KVM_RISCV_REG_ID_U64(KVM_REG_RISCV_FP_D, idx)
>
> -#define RISCV_VECTOR_CSR_REG(env, name) \
> - kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_VECTOR, \
> +#define RISCV_VECTOR_CSR_REG(name) \
> + KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_VECTOR, \
> KVM_REG_RISCV_VECTOR_CSR_REG(name))
>
> #define KVM_RISCV_GET_CSR(cs, env, csr, reg) \
> do { \
> - int _ret = kvm_get_one_reg(cs, RISCV_CSR_REG(env, csr), ®); \
> + int _ret = kvm_get_one_reg(cs, RISCV_CSR_REG(csr), ®); \
> if (_ret) { \
> return _ret; \
> } \
> @@ -136,7 +124,7 @@ static uint64_t kvm_riscv_vector_reg_id(RISCVCPU *cpu,
>
> #define KVM_RISCV_SET_CSR(cs, env, csr, reg) \
> do { \
> - int _ret = kvm_set_one_reg(cs, RISCV_CSR_REG(env, csr), ®); \
> + int _ret = kvm_set_one_reg(cs, RISCV_CSR_REG(csr), ®); \
> if (_ret) { \
> return _ret; \
> } \
> @@ -244,7 +232,7 @@ static void kvm_riscv_update_cpu_misa_ext(RISCVCPU *cpu, CPUState *cs)
>
> /* If we're here we're going to disable the MISA bit */
> reg = 0;
> - id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_ISA_EXT,
> + id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_ISA_EXT,
> misa_cfg->kvm_reg_id);
> ret = kvm_set_one_reg(cs, id, ®);
> if (ret != 0) {
> @@ -430,7 +418,6 @@ static KVMCPUConfig kvm_sbi_dbcn = {
>
> static void kvm_riscv_update_cpu_cfg_isa_ext(RISCVCPU *cpu, CPUState *cs)
> {
> - CPURISCVState *env = &cpu->env;
> uint64_t id, reg;
> int i, ret;
>
> @@ -441,7 +428,7 @@ static void kvm_riscv_update_cpu_cfg_isa_ext(RISCVCPU *cpu, CPUState *cs)
> continue;
> }
>
> - id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_ISA_EXT,
> + id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_ISA_EXT,
> multi_ext_cfg->kvm_reg_id);
> reg = kvm_cpu_cfg_get(cpu, multi_ext_cfg);
> ret = kvm_set_one_reg(cs, id, ®);
> @@ -566,14 +553,14 @@ static int kvm_riscv_get_regs_core(CPUState *cs)
> target_ulong reg;
> CPURISCVState *env = &RISCV_CPU(cs)->env;
>
> - ret = kvm_get_one_reg(cs, RISCV_CORE_REG(env, regs.pc), ®);
> + ret = kvm_get_one_reg(cs, RISCV_CORE_REG(regs.pc), ®);
> if (ret) {
> return ret;
> }
> env->pc = reg;
>
> for (i = 1; i < 32; i++) {
> - uint64_t id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, i);
> + uint64_t id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CORE, i);
> ret = kvm_get_one_reg(cs, id, ®);
> if (ret) {
> return ret;
> @@ -592,13 +579,13 @@ static int kvm_riscv_put_regs_core(CPUState *cs)
> CPURISCVState *env = &RISCV_CPU(cs)->env;
>
> reg = env->pc;
> - ret = kvm_set_one_reg(cs, RISCV_CORE_REG(env, regs.pc), ®);
> + ret = kvm_set_one_reg(cs, RISCV_CORE_REG(regs.pc), ®);
> if (ret) {
> return ret;
> }
>
> for (i = 1; i < 32; i++) {
> - uint64_t id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, i);
> + uint64_t id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CORE, i);
> reg = env->gpr[i];
> ret = kvm_set_one_reg(cs, id, ®);
> if (ret) {
> @@ -796,26 +783,26 @@ static int kvm_riscv_get_regs_vector(CPUState *cs)
> return 0;
> }
>
> - ret = kvm_get_one_reg(cs, RISCV_VECTOR_CSR_REG(env, vstart), ®);
> + ret = kvm_get_one_reg(cs, RISCV_VECTOR_CSR_REG(vstart), ®);
> if (ret) {
> return ret;
> }
> env->vstart = reg;
>
> - ret = kvm_get_one_reg(cs, RISCV_VECTOR_CSR_REG(env, vl), ®);
> + ret = kvm_get_one_reg(cs, RISCV_VECTOR_CSR_REG(vl), ®);
> if (ret) {
> return ret;
> }
> env->vl = reg;
>
> - ret = kvm_get_one_reg(cs, RISCV_VECTOR_CSR_REG(env, vtype), ®);
> + ret = kvm_get_one_reg(cs, RISCV_VECTOR_CSR_REG(vtype), ®);
> if (ret) {
> return ret;
> }
> env->vtype = reg;
>
> if (kvm_v_vlenb.supported) {
> - ret = kvm_get_one_reg(cs, RISCV_VECTOR_CSR_REG(env, vlenb), ®);
> + ret = kvm_get_one_reg(cs, RISCV_VECTOR_CSR_REG(vlenb), ®);
> if (ret) {
> return ret;
> }
> @@ -853,26 +840,26 @@ static int kvm_riscv_put_regs_vector(CPUState *cs)
> }
>
> reg = env->vstart;
> - ret = kvm_set_one_reg(cs, RISCV_VECTOR_CSR_REG(env, vstart), ®);
> + ret = kvm_set_one_reg(cs, RISCV_VECTOR_CSR_REG(vstart), ®);
> if (ret) {
> return ret;
> }
>
> reg = env->vl;
> - ret = kvm_set_one_reg(cs, RISCV_VECTOR_CSR_REG(env, vl), ®);
> + ret = kvm_set_one_reg(cs, RISCV_VECTOR_CSR_REG(vl), ®);
> if (ret) {
> return ret;
> }
>
> reg = env->vtype;
> - ret = kvm_set_one_reg(cs, RISCV_VECTOR_CSR_REG(env, vtype), ®);
> + ret = kvm_set_one_reg(cs, RISCV_VECTOR_CSR_REG(vtype), ®);
> if (ret) {
> return ret;
> }
>
> if (kvm_v_vlenb.supported) {
> reg = cpu->cfg.vlenb;
> - ret = kvm_set_one_reg(cs, RISCV_VECTOR_CSR_REG(env, vlenb), ®);
> + ret = kvm_set_one_reg(cs, RISCV_VECTOR_CSR_REG(vlenb), ®);
>
> for (int i = 0; i < 32; i++) {
> /*
> @@ -951,25 +938,24 @@ static void kvm_riscv_destroy_scratch_vcpu(KVMScratchCPU *scratch)
>
> static void kvm_riscv_init_machine_ids(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
> {
> - CPURISCVState *env = &cpu->env;
> struct kvm_one_reg reg;
> int ret;
>
> - reg.id = RISCV_CONFIG_REG(env, mvendorid);
> + reg.id = RISCV_CONFIG_REG(mvendorid);
> reg.addr = (uint64_t)&cpu->cfg.mvendorid;
> ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
> if (ret != 0) {
> error_report("Unable to retrieve mvendorid from host, error %d", ret);
> }
>
> - reg.id = RISCV_CONFIG_REG(env, marchid);
> + reg.id = RISCV_CONFIG_REG(marchid);
> reg.addr = (uint64_t)&cpu->cfg.marchid;
> ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
> if (ret != 0) {
> error_report("Unable to retrieve marchid from host, error %d", ret);
> }
>
> - reg.id = RISCV_CONFIG_REG(env, mimpid);
> + reg.id = RISCV_CONFIG_REG(mimpid);
> reg.addr = (uint64_t)&cpu->cfg.mimpid;
> ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
> if (ret != 0) {
> @@ -984,7 +970,7 @@ static void kvm_riscv_init_misa_ext_mask(RISCVCPU *cpu,
> struct kvm_one_reg reg;
> int ret;
>
> - reg.id = RISCV_CONFIG_REG(env, isa);
> + reg.id = RISCV_CONFIG_REG(isa);
> reg.addr = (uint64_t)&env->misa_ext_mask;
> ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
>
> @@ -1001,11 +987,10 @@ static void kvm_riscv_init_misa_ext_mask(RISCVCPU *cpu,
> static void kvm_riscv_read_cbomz_blksize(RISCVCPU *cpu, KVMScratchCPU *kvmcpu,
> KVMCPUConfig *cbomz_cfg)
> {
> - CPURISCVState *env = &cpu->env;
> struct kvm_one_reg reg;
> int ret;
>
> - reg.id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CONFIG,
> + reg.id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CONFIG,
> cbomz_cfg->kvm_reg_id);
> reg.addr = (uint64_t)kvmconfig_get_cfg_addr(cpu, cbomz_cfg);
> ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
> @@ -1019,7 +1004,6 @@ static void kvm_riscv_read_cbomz_blksize(RISCVCPU *cpu, KVMScratchCPU *kvmcpu,
> static void kvm_riscv_read_multiext_legacy(RISCVCPU *cpu,
> KVMScratchCPU *kvmcpu)
> {
> - CPURISCVState *env = &cpu->env;
> uint64_t val;
> int i, ret;
>
> @@ -1027,7 +1011,7 @@ static void kvm_riscv_read_multiext_legacy(RISCVCPU *cpu,
> KVMCPUConfig *multi_ext_cfg = &kvm_multi_ext_cfgs[i];
> struct kvm_one_reg reg;
>
> - reg.id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_ISA_EXT,
> + reg.id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_ISA_EXT,
> multi_ext_cfg->kvm_reg_id);
> reg.addr = (uint64_t)&val;
> ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
> @@ -1159,7 +1143,7 @@ static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
>
> for (i = 0; i < ARRAY_SIZE(kvm_multi_ext_cfgs); i++) {
> multi_ext_cfg = &kvm_multi_ext_cfgs[i];
> - reg_id = kvm_riscv_reg_id_ulong(&cpu->env, KVM_REG_RISCV_ISA_EXT,
> + reg_id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_ISA_EXT,
> multi_ext_cfg->kvm_reg_id);
> reg_search = bsearch(®_id, reglist->reg, reglist->n,
> sizeof(uint64_t), uint64_cmp);
> @@ -1338,12 +1322,11 @@ void kvm_arch_init_irq_routing(KVMState *s)
>
> static int kvm_vcpu_set_machine_ids(RISCVCPU *cpu, CPUState *cs)
> {
> - CPURISCVState *env = &cpu->env;
> target_ulong reg;
> uint64_t id;
> int ret;
>
> - id = RISCV_CONFIG_REG(env, mvendorid);
> + id = RISCV_CONFIG_REG(mvendorid);
> /*
> * cfg.mvendorid is an uint32 but a target_ulong will
> * be written. Assign it to a target_ulong var to avoid
> @@ -1355,13 +1338,13 @@ static int kvm_vcpu_set_machine_ids(RISCVCPU *cpu, CPUState *cs)
> return ret;
> }
>
> - id = RISCV_CONFIG_REG(env, marchid);
> + id = RISCV_CONFIG_REG(marchid);
> ret = kvm_set_one_reg(cs, id, &cpu->cfg.marchid);
> if (ret != 0) {
> return ret;
> }
>
> - id = RISCV_CONFIG_REG(env, mimpid);
> + id = RISCV_CONFIG_REG(mimpid);
> ret = kvm_set_one_reg(cs, id, &cpu->cfg.mimpid);
>
> return ret;
> @@ -1911,7 +1894,7 @@ void riscv_kvm_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
> if (cpu->cfg.ext_zicbom &&
> riscv_cpu_option_set(kvm_cbom_blocksize.name)) {
>
> - reg.id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CONFIG,
> + reg.id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CONFIG,
> kvm_cbom_blocksize.kvm_reg_id);
> reg.addr = (uint64_t)&val;
> ret = ioctl(kvmcpu.cpufd, KVM_GET_ONE_REG, ®);
> @@ -1930,7 +1913,7 @@ void riscv_kvm_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
> if (cpu->cfg.ext_zicboz &&
> riscv_cpu_option_set(kvm_cboz_blocksize.name)) {
>
> - reg.id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CONFIG,
> + reg.id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CONFIG,
> kvm_cboz_blocksize.kvm_reg_id);
> reg.addr = (uint64_t)&val;
> ret = ioctl(kvmcpu.cpufd, KVM_GET_ONE_REG, ®);
> --
> 2.49.0
>
Otherwise,
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 5/9] target/riscv/kvm: add kvm_csr_cfgs[]
2025-04-25 11:36 [PATCH v2 0/9] target/riscv/kvm: CSR related fixes Daniel Henrique Barboza
` (3 preceding siblings ...)
2025-04-25 11:37 ` [PATCH v2 4/9] target/riscv/kvm: turn kvm_riscv_reg_id_ulong() in a macro Daniel Henrique Barboza
@ 2025-04-25 11:37 ` Daniel Henrique Barboza
2025-04-25 11:58 ` Andrew Jones
2025-04-25 11:37 ` [PATCH v2 6/9] target/riscv/kvm: do not read unavailable CSRs Daniel Henrique Barboza
` (3 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Daniel Henrique Barboza @ 2025-04-25 11:37 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, palmer,
ajones, Daniel Henrique Barboza
At this moment we're not checking if the host has support for any
specific CSR before doing get/put regs. This will cause problems if the
host KVM doesn't support it (see [1] as an example).
We'll use the same approach done with the CPU extensions: read all known
KVM CSRs during init() to check for availability, then read/write them
if they are present. This will be made by either using get-reglist or by
directly reading the CSRs.
For now we'll just convert the CSRs to use a kvm_csr_cfg[] array,
reusing the same KVMCPUConfig abstraction we use for extensions, and use
the array in (get|put)_csr_regs() instead of manually listing them. A
lot of boilerplate will be added but at least we'll automate the get/put
procedure for CSRs, i.e. adding a new CSR in the future will be a matter
of adding it in kvm_csr_regs[] and everything else will be taken care
of.
Despite all the code changes no behavioral change is made.
[1] https://lore.kernel.org/qemu-riscv/CABJz62OfUDHYkQ0T3rGHStQprf1c7_E0qBLbLKhfv=+jb0SYAw@mail.gmail.com/
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
target/riscv/cpu.h | 1 +
target/riscv/kvm/kvm-cpu.c | 121 ++++++++++++++++++++++++++-----------
2 files changed, 86 insertions(+), 36 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 679f417336..f5a60d0c52 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -79,6 +79,7 @@ const char *riscv_get_misa_ext_name(uint32_t bit);
const char *riscv_get_misa_ext_description(uint32_t bit);
#define CPU_CFG_OFFSET(_prop) offsetof(struct RISCVCPUConfig, _prop)
+#define ENV_CSR_OFFSET(_csr) offsetof(CPURISCVState, _csr)
typedef struct riscv_cpu_profile {
struct riscv_cpu_profile *u_parent;
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index fd66bc1759..7cbe566e5f 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -114,22 +114,6 @@ static uint64_t kvm_riscv_vector_reg_id(RISCVCPU *cpu,
KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_VECTOR, \
KVM_REG_RISCV_VECTOR_CSR_REG(name))
-#define KVM_RISCV_GET_CSR(cs, env, csr, reg) \
- do { \
- int _ret = kvm_get_one_reg(cs, RISCV_CSR_REG(csr), ®); \
- if (_ret) { \
- return _ret; \
- } \
- } while (0)
-
-#define KVM_RISCV_SET_CSR(cs, env, csr, reg) \
- do { \
- int _ret = kvm_set_one_reg(cs, RISCV_CSR_REG(csr), ®); \
- if (_ret) { \
- return _ret; \
- } \
- } while (0)
-
#define KVM_RISCV_GET_TIMER(cs, name, reg) \
do { \
int ret = kvm_get_one_reg(cs, RISCV_TIMER_REG(name), ®); \
@@ -251,6 +235,53 @@ static void kvm_riscv_update_cpu_misa_ext(RISCVCPU *cpu, CPUState *cs)
}
}
+#define KVM_CSR_CFG(_name, _env_prop, reg_id) \
+ {.name = _name, .offset = ENV_CSR_OFFSET(_env_prop), \
+ .kvm_reg_id = reg_id}
+
+static KVMCPUConfig kvm_csr_cfgs[] = {
+ KVM_CSR_CFG("sstatus", mstatus, RISCV_CSR_REG(sstatus)),
+ KVM_CSR_CFG("sie", mie, RISCV_CSR_REG(sie)),
+ KVM_CSR_CFG("stvec", stvec, RISCV_CSR_REG(stvec)),
+ KVM_CSR_CFG("sscratch", sscratch, RISCV_CSR_REG(sscratch)),
+ KVM_CSR_CFG("sepc", sepc, RISCV_CSR_REG(sepc)),
+ KVM_CSR_CFG("scause", scause, RISCV_CSR_REG(scause)),
+ KVM_CSR_CFG("stval", stval, RISCV_CSR_REG(stval)),
+ KVM_CSR_CFG("sip", mip, RISCV_CSR_REG(sip)),
+ KVM_CSR_CFG("satp", satp, RISCV_CSR_REG(satp)),
+};
+
+static void *kvmconfig_get_env_addr(RISCVCPU *cpu, KVMCPUConfig *csr_cfg)
+{
+ return (void *)&cpu->env + csr_cfg->offset;
+}
+
+static uint64_t kvm_cpu_csr_get_u32(RISCVCPU *cpu, KVMCPUConfig *csr_cfg)
+{
+ uint32_t *val32 = kvmconfig_get_env_addr(cpu, csr_cfg);
+ return *val32;
+}
+
+static uint64_t kvm_cpu_csr_get_u64(RISCVCPU *cpu, KVMCPUConfig *csr_cfg)
+{
+ uint64_t *val64 = kvmconfig_get_env_addr(cpu, csr_cfg);
+ return *val64;
+}
+
+static void kvm_cpu_csr_set_u32(RISCVCPU *cpu, KVMCPUConfig *csr_cfg,
+ uint32_t val)
+{
+ uint32_t *val32 = kvmconfig_get_env_addr(cpu, csr_cfg);
+ *val32 = val;
+}
+
+static void kvm_cpu_csr_set_u64(RISCVCPU *cpu, KVMCPUConfig *csr_cfg,
+ uint64_t val)
+{
+ uint64_t *val64 = kvmconfig_get_env_addr(cpu, csr_cfg);
+ *val64 = val;
+}
+
#define KVM_EXT_CFG(_name, _prop, _reg_id) \
{.name = _name, .offset = CPU_CFG_OFFSET(_prop), \
.kvm_reg_id = _reg_id}
@@ -598,34 +629,52 @@ static int kvm_riscv_put_regs_core(CPUState *cs)
static int kvm_riscv_get_regs_csr(CPUState *cs)
{
- CPURISCVState *env = &RISCV_CPU(cs)->env;
+ RISCVCPU *cpu = RISCV_CPU(cs);
+ uint64_t reg;
+ int i, ret;
+
+ for (i = 0; i < ARRAY_SIZE(kvm_csr_cfgs); i++) {
+ KVMCPUConfig *csr_cfg = &kvm_csr_cfgs[i];
- KVM_RISCV_GET_CSR(cs, env, sstatus, env->mstatus);
- KVM_RISCV_GET_CSR(cs, env, sie, env->mie);
- KVM_RISCV_GET_CSR(cs, env, stvec, env->stvec);
- KVM_RISCV_GET_CSR(cs, env, sscratch, env->sscratch);
- KVM_RISCV_GET_CSR(cs, env, sepc, env->sepc);
- KVM_RISCV_GET_CSR(cs, env, scause, env->scause);
- KVM_RISCV_GET_CSR(cs, env, stval, env->stval);
- KVM_RISCV_GET_CSR(cs, env, sip, env->mip);
- KVM_RISCV_GET_CSR(cs, env, satp, env->satp);
+ ret = kvm_get_one_reg(cs, csr_cfg->kvm_reg_id, ®);
+ if (ret) {
+ return ret;
+ }
+
+ if (KVM_REG_SIZE(csr_cfg->kvm_reg_id) == sizeof(uint32_t)) {
+ kvm_cpu_csr_set_u32(cpu, csr_cfg, reg);
+ } else if (KVM_REG_SIZE(csr_cfg->kvm_reg_id) == sizeof(uint64_t)) {
+ kvm_cpu_csr_set_u64(cpu, csr_cfg, reg);
+ } else {
+ g_assert_not_reached();
+ }
+ }
return 0;
}
static int kvm_riscv_put_regs_csr(CPUState *cs)
{
- CPURISCVState *env = &RISCV_CPU(cs)->env;
+ RISCVCPU *cpu = RISCV_CPU(cs);
+ uint64_t reg;
+ int i, ret;
+
+ for (i = 0; i < ARRAY_SIZE(kvm_csr_cfgs); i++) {
+ KVMCPUConfig *csr_cfg = &kvm_csr_cfgs[i];
+
+ if (KVM_REG_SIZE(csr_cfg->kvm_reg_id) == sizeof(uint32_t)) {
+ reg = kvm_cpu_csr_get_u32(cpu, csr_cfg);
+ } else if (KVM_REG_SIZE(csr_cfg->kvm_reg_id) == sizeof(uint64_t)) {
+ reg = kvm_cpu_csr_get_u64(cpu, csr_cfg);
+ } else {
+ g_assert_not_reached();
+ }
- KVM_RISCV_SET_CSR(cs, env, sstatus, env->mstatus);
- KVM_RISCV_SET_CSR(cs, env, sie, env->mie);
- KVM_RISCV_SET_CSR(cs, env, stvec, env->stvec);
- KVM_RISCV_SET_CSR(cs, env, sscratch, env->sscratch);
- KVM_RISCV_SET_CSR(cs, env, sepc, env->sepc);
- KVM_RISCV_SET_CSR(cs, env, scause, env->scause);
- KVM_RISCV_SET_CSR(cs, env, stval, env->stval);
- KVM_RISCV_SET_CSR(cs, env, sip, env->mip);
- KVM_RISCV_SET_CSR(cs, env, satp, env->satp);
+ ret = kvm_set_one_reg(cs, csr_cfg->kvm_reg_id, ®);
+ if (ret) {
+ return ret;
+ }
+ }
return 0;
}
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 5/9] target/riscv/kvm: add kvm_csr_cfgs[]
2025-04-25 11:37 ` [PATCH v2 5/9] target/riscv/kvm: add kvm_csr_cfgs[] Daniel Henrique Barboza
@ 2025-04-25 11:58 ` Andrew Jones
0 siblings, 0 replies; 20+ messages in thread
From: Andrew Jones @ 2025-04-25 11:58 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, liwei1518, zhiwei_liu,
palmer
On Fri, Apr 25, 2025 at 08:37:01AM -0300, Daniel Henrique Barboza wrote:
> At this moment we're not checking if the host has support for any
> specific CSR before doing get/put regs. This will cause problems if the
> host KVM doesn't support it (see [1] as an example).
>
> We'll use the same approach done with the CPU extensions: read all known
> KVM CSRs during init() to check for availability, then read/write them
> if they are present. This will be made by either using get-reglist or by
> directly reading the CSRs.
>
> For now we'll just convert the CSRs to use a kvm_csr_cfg[] array,
> reusing the same KVMCPUConfig abstraction we use for extensions, and use
> the array in (get|put)_csr_regs() instead of manually listing them. A
> lot of boilerplate will be added but at least we'll automate the get/put
> procedure for CSRs, i.e. adding a new CSR in the future will be a matter
> of adding it in kvm_csr_regs[] and everything else will be taken care
> of.
>
> Despite all the code changes no behavioral change is made.
>
> [1] https://lore.kernel.org/qemu-riscv/CABJz62OfUDHYkQ0T3rGHStQprf1c7_E0qBLbLKhfv=+jb0SYAw@mail.gmail.com/
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> target/riscv/cpu.h | 1 +
> target/riscv/kvm/kvm-cpu.c | 121 ++++++++++++++++++++++++++-----------
> 2 files changed, 86 insertions(+), 36 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 679f417336..f5a60d0c52 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -79,6 +79,7 @@ const char *riscv_get_misa_ext_name(uint32_t bit);
> const char *riscv_get_misa_ext_description(uint32_t bit);
>
> #define CPU_CFG_OFFSET(_prop) offsetof(struct RISCVCPUConfig, _prop)
> +#define ENV_CSR_OFFSET(_csr) offsetof(CPURISCVState, _csr)
>
> typedef struct riscv_cpu_profile {
> struct riscv_cpu_profile *u_parent;
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index fd66bc1759..7cbe566e5f 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -114,22 +114,6 @@ static uint64_t kvm_riscv_vector_reg_id(RISCVCPU *cpu,
> KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_VECTOR, \
> KVM_REG_RISCV_VECTOR_CSR_REG(name))
>
> -#define KVM_RISCV_GET_CSR(cs, env, csr, reg) \
> - do { \
> - int _ret = kvm_get_one_reg(cs, RISCV_CSR_REG(csr), ®); \
> - if (_ret) { \
> - return _ret; \
> - } \
> - } while (0)
> -
> -#define KVM_RISCV_SET_CSR(cs, env, csr, reg) \
> - do { \
> - int _ret = kvm_set_one_reg(cs, RISCV_CSR_REG(csr), ®); \
> - if (_ret) { \
> - return _ret; \
> - } \
> - } while (0)
> -
> #define KVM_RISCV_GET_TIMER(cs, name, reg) \
> do { \
> int ret = kvm_get_one_reg(cs, RISCV_TIMER_REG(name), ®); \
> @@ -251,6 +235,53 @@ static void kvm_riscv_update_cpu_misa_ext(RISCVCPU *cpu, CPUState *cs)
> }
> }
>
> +#define KVM_CSR_CFG(_name, _env_prop, reg_id) \
> + {.name = _name, .offset = ENV_CSR_OFFSET(_env_prop), \
> + .kvm_reg_id = reg_id}
> +
> +static KVMCPUConfig kvm_csr_cfgs[] = {
> + KVM_CSR_CFG("sstatus", mstatus, RISCV_CSR_REG(sstatus)),
> + KVM_CSR_CFG("sie", mie, RISCV_CSR_REG(sie)),
> + KVM_CSR_CFG("stvec", stvec, RISCV_CSR_REG(stvec)),
> + KVM_CSR_CFG("sscratch", sscratch, RISCV_CSR_REG(sscratch)),
> + KVM_CSR_CFG("sepc", sepc, RISCV_CSR_REG(sepc)),
> + KVM_CSR_CFG("scause", scause, RISCV_CSR_REG(scause)),
> + KVM_CSR_CFG("stval", stval, RISCV_CSR_REG(stval)),
> + KVM_CSR_CFG("sip", mip, RISCV_CSR_REG(sip)),
> + KVM_CSR_CFG("satp", satp, RISCV_CSR_REG(satp)),
nit: could tabulate this
> +};
> +
> +static void *kvmconfig_get_env_addr(RISCVCPU *cpu, KVMCPUConfig *csr_cfg)
> +{
> + return (void *)&cpu->env + csr_cfg->offset;
> +}
> +
> +static uint64_t kvm_cpu_csr_get_u32(RISCVCPU *cpu, KVMCPUConfig *csr_cfg)
Despite the cover letter stating this was changed to return a uint32_t,
it's not.
> +{
> + uint32_t *val32 = kvmconfig_get_env_addr(cpu, csr_cfg);
> + return *val32;
> +}
> +
> +static uint64_t kvm_cpu_csr_get_u64(RISCVCPU *cpu, KVMCPUConfig *csr_cfg)
> +{
> + uint64_t *val64 = kvmconfig_get_env_addr(cpu, csr_cfg);
> + return *val64;
> +}
> +
> +static void kvm_cpu_csr_set_u32(RISCVCPU *cpu, KVMCPUConfig *csr_cfg,
> + uint32_t val)
> +{
> + uint32_t *val32 = kvmconfig_get_env_addr(cpu, csr_cfg);
> + *val32 = val;
> +}
> +
> +static void kvm_cpu_csr_set_u64(RISCVCPU *cpu, KVMCPUConfig *csr_cfg,
> + uint64_t val)
> +{
> + uint64_t *val64 = kvmconfig_get_env_addr(cpu, csr_cfg);
> + *val64 = val;
> +}
> +
> #define KVM_EXT_CFG(_name, _prop, _reg_id) \
> {.name = _name, .offset = CPU_CFG_OFFSET(_prop), \
> .kvm_reg_id = _reg_id}
> @@ -598,34 +629,52 @@ static int kvm_riscv_put_regs_core(CPUState *cs)
>
> static int kvm_riscv_get_regs_csr(CPUState *cs)
> {
> - CPURISCVState *env = &RISCV_CPU(cs)->env;
> + RISCVCPU *cpu = RISCV_CPU(cs);
> + uint64_t reg;
> + int i, ret;
> +
> + for (i = 0; i < ARRAY_SIZE(kvm_csr_cfgs); i++) {
> + KVMCPUConfig *csr_cfg = &kvm_csr_cfgs[i];
>
> - KVM_RISCV_GET_CSR(cs, env, sstatus, env->mstatus);
> - KVM_RISCV_GET_CSR(cs, env, sie, env->mie);
> - KVM_RISCV_GET_CSR(cs, env, stvec, env->stvec);
> - KVM_RISCV_GET_CSR(cs, env, sscratch, env->sscratch);
> - KVM_RISCV_GET_CSR(cs, env, sepc, env->sepc);
> - KVM_RISCV_GET_CSR(cs, env, scause, env->scause);
> - KVM_RISCV_GET_CSR(cs, env, stval, env->stval);
> - KVM_RISCV_GET_CSR(cs, env, sip, env->mip);
> - KVM_RISCV_GET_CSR(cs, env, satp, env->satp);
> + ret = kvm_get_one_reg(cs, csr_cfg->kvm_reg_id, ®);
> + if (ret) {
> + return ret;
> + }
> +
> + if (KVM_REG_SIZE(csr_cfg->kvm_reg_id) == sizeof(uint32_t)) {
> + kvm_cpu_csr_set_u32(cpu, csr_cfg, reg);
> + } else if (KVM_REG_SIZE(csr_cfg->kvm_reg_id) == sizeof(uint64_t)) {
> + kvm_cpu_csr_set_u64(cpu, csr_cfg, reg);
> + } else {
> + g_assert_not_reached();
> + }
> + }
>
> return 0;
> }
>
> static int kvm_riscv_put_regs_csr(CPUState *cs)
> {
> - CPURISCVState *env = &RISCV_CPU(cs)->env;
> + RISCVCPU *cpu = RISCV_CPU(cs);
> + uint64_t reg;
> + int i, ret;
> +
> + for (i = 0; i < ARRAY_SIZE(kvm_csr_cfgs); i++) {
> + KVMCPUConfig *csr_cfg = &kvm_csr_cfgs[i];
> +
> + if (KVM_REG_SIZE(csr_cfg->kvm_reg_id) == sizeof(uint32_t)) {
> + reg = kvm_cpu_csr_get_u32(cpu, csr_cfg);
> + } else if (KVM_REG_SIZE(csr_cfg->kvm_reg_id) == sizeof(uint64_t)) {
> + reg = kvm_cpu_csr_get_u64(cpu, csr_cfg);
> + } else {
> + g_assert_not_reached();
> + }
>
> - KVM_RISCV_SET_CSR(cs, env, sstatus, env->mstatus);
> - KVM_RISCV_SET_CSR(cs, env, sie, env->mie);
> - KVM_RISCV_SET_CSR(cs, env, stvec, env->stvec);
> - KVM_RISCV_SET_CSR(cs, env, sscratch, env->sscratch);
> - KVM_RISCV_SET_CSR(cs, env, sepc, env->sepc);
> - KVM_RISCV_SET_CSR(cs, env, scause, env->scause);
> - KVM_RISCV_SET_CSR(cs, env, stval, env->stval);
> - KVM_RISCV_SET_CSR(cs, env, sip, env->mip);
> - KVM_RISCV_SET_CSR(cs, env, satp, env->satp);
> + ret = kvm_set_one_reg(cs, csr_cfg->kvm_reg_id, ®);
> + if (ret) {
> + return ret;
> + }
> + }
>
> return 0;
> }
> --
> 2.49.0
>
Otherwise,
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 6/9] target/riscv/kvm: do not read unavailable CSRs
2025-04-25 11:36 [PATCH v2 0/9] target/riscv/kvm: CSR related fixes Daniel Henrique Barboza
` (4 preceding siblings ...)
2025-04-25 11:37 ` [PATCH v2 5/9] target/riscv/kvm: add kvm_csr_cfgs[] Daniel Henrique Barboza
@ 2025-04-25 11:37 ` Daniel Henrique Barboza
2025-04-25 12:02 ` Andrew Jones
2025-04-25 11:37 ` [PATCH v2 7/9] target/riscv/kvm: add senvcfg CSR Daniel Henrique Barboza
` (2 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Daniel Henrique Barboza @ 2025-04-25 11:37 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, palmer,
ajones, Daniel Henrique Barboza, Andrea Bolognani
[1] reports that commit 4db19d5b21 broke a KVM guest running kernel 6.6.
This happens because the kernel does not know 'senvcfg', making it
unable to boot because QEMU is reading/wriiting it without any checks.
After converting the CSRs to do "automated" get/put reg procedures in
the previous patch we can now scan for availability. Two functions are
created:
- kvm_riscv_read_csr_cfg_legacy() will check if the CSR exists by brute
forcing KVM_GET_ONE_REG in each one of them, interpreting an EINVAL
return as indication that the CSR isn't available. This will be use in
absence of KVM_GET_REG_LIST;
- kvm_riscv_read_csr_cfg() will use the existing result of get_reg_list
to check if the CSRs ids are present.
kvm_riscv_init_multiext_cfg() is now kvm_riscv_init_multiext_csr_cfg()
to reflect that the function is also dealing with CSRs.
[1] https://lore.kernel.org/qemu-riscv/CABJz62OfUDHYkQ0T3rGHStQprf1c7_E0qBLbLKhfv=+jb0SYAw@mail.gmail.com/
Fixes: 4db19d5b21 ("target/riscv/kvm: add missing KVM CSRs")
Reported-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
target/riscv/kvm/kvm-cpu.c | 62 ++++++++++++++++++++++++++++++++++++--
1 file changed, 59 insertions(+), 3 deletions(-)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 7cbe566e5f..f341085ba1 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -636,6 +636,10 @@ static int kvm_riscv_get_regs_csr(CPUState *cs)
for (i = 0; i < ARRAY_SIZE(kvm_csr_cfgs); i++) {
KVMCPUConfig *csr_cfg = &kvm_csr_cfgs[i];
+ if (!csr_cfg->supported) {
+ continue;
+ }
+
ret = kvm_get_one_reg(cs, csr_cfg->kvm_reg_id, ®);
if (ret) {
return ret;
@@ -662,6 +666,10 @@ static int kvm_riscv_put_regs_csr(CPUState *cs)
for (i = 0; i < ARRAY_SIZE(kvm_csr_cfgs); i++) {
KVMCPUConfig *csr_cfg = &kvm_csr_cfgs[i];
+ if (!csr_cfg->supported) {
+ continue;
+ }
+
if (KVM_REG_SIZE(csr_cfg->kvm_reg_id) == sizeof(uint32_t)) {
reg = kvm_cpu_csr_get_u32(cpu, csr_cfg);
} else if (KVM_REG_SIZE(csr_cfg->kvm_reg_id) == sizeof(uint64_t)) {
@@ -1090,6 +1098,32 @@ static void kvm_riscv_read_multiext_legacy(RISCVCPU *cpu,
}
}
+static void kvm_riscv_read_csr_cfg_legacy(KVMScratchCPU *kvmcpu)
+{
+ uint64_t val;
+ int i, ret;
+
+ for (i = 0; i < ARRAY_SIZE(kvm_csr_cfgs); i++) {
+ KVMCPUConfig *csr_cfg = &kvm_csr_cfgs[i];
+ struct kvm_one_reg reg;
+
+ reg.id = csr_cfg->kvm_reg_id;
+ reg.addr = (uint64_t)&val;
+ ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
+ if (ret != 0) {
+ if (errno == EINVAL) {
+ csr_cfg->supported = false;
+ } else {
+ error_report("Unable to read KVM CSR %s: %s",
+ csr_cfg->name, strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+ } else {
+ csr_cfg->supported = true;
+ }
+ }
+}
+
static int uint64_cmp(const void *a, const void *b)
{
uint64_t val1 = *(const uint64_t *)a;
@@ -1146,7 +1180,26 @@ static void kvm_riscv_read_vlenb(RISCVCPU *cpu, KVMScratchCPU *kvmcpu,
}
}
-static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
+static void kvm_riscv_read_csr_cfg(struct kvm_reg_list *reglist)
+{
+ struct kvm_reg_list *reg_search;
+ uint64_t reg_id;
+
+ for (int i = 0; i < ARRAY_SIZE(kvm_csr_cfgs); i++) {
+ KVMCPUConfig *csr_cfg = &kvm_csr_cfgs[i];
+
+ reg_id = csr_cfg->kvm_reg_id;
+ reg_search = bsearch(®_id, reglist->reg, reglist->n,
+ sizeof(uint64_t), uint64_cmp);
+ if (!reg_search) {
+ continue;
+ }
+
+ csr_cfg->supported = true;
+ }
+}
+
+static void kvm_riscv_init_cfg(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
{
g_autofree struct kvm_reg_list *reglist = NULL;
KVMCPUConfig *multi_ext_cfg;
@@ -1163,7 +1216,9 @@ static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
* (EINVAL). Use read_legacy() in this case.
*/
if (errno == EINVAL) {
- return kvm_riscv_read_multiext_legacy(cpu, kvmcpu);
+ kvm_riscv_read_multiext_legacy(cpu, kvmcpu);
+ kvm_riscv_read_csr_cfg_legacy(kvmcpu);
+ return;
} else if (errno != E2BIG) {
/*
* E2BIG is an expected error message for the API since we
@@ -1226,6 +1281,7 @@ static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
}
kvm_riscv_check_sbi_dbcn_support(cpu, reglist);
+ kvm_riscv_read_csr_cfg(reglist);
}
static void riscv_init_kvm_registers(Object *cpu_obj)
@@ -1239,7 +1295,7 @@ static void riscv_init_kvm_registers(Object *cpu_obj)
kvm_riscv_init_machine_ids(cpu, &kvmcpu);
kvm_riscv_init_misa_ext_mask(cpu, &kvmcpu);
- kvm_riscv_init_multiext_cfg(cpu, &kvmcpu);
+ kvm_riscv_init_cfg(cpu, &kvmcpu);
kvm_riscv_destroy_scratch_vcpu(&kvmcpu);
}
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 6/9] target/riscv/kvm: do not read unavailable CSRs
2025-04-25 11:37 ` [PATCH v2 6/9] target/riscv/kvm: do not read unavailable CSRs Daniel Henrique Barboza
@ 2025-04-25 12:02 ` Andrew Jones
0 siblings, 0 replies; 20+ messages in thread
From: Andrew Jones @ 2025-04-25 12:02 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, liwei1518, zhiwei_liu,
palmer, Andrea Bolognani
On Fri, Apr 25, 2025 at 08:37:02AM -0300, Daniel Henrique Barboza wrote:
> [1] reports that commit 4db19d5b21 broke a KVM guest running kernel 6.6.
> This happens because the kernel does not know 'senvcfg', making it
> unable to boot because QEMU is reading/wriiting it without any checks.
>
> After converting the CSRs to do "automated" get/put reg procedures in
> the previous patch we can now scan for availability. Two functions are
> created:
>
> - kvm_riscv_read_csr_cfg_legacy() will check if the CSR exists by brute
> forcing KVM_GET_ONE_REG in each one of them, interpreting an EINVAL
> return as indication that the CSR isn't available. This will be use in
> absence of KVM_GET_REG_LIST;
>
> - kvm_riscv_read_csr_cfg() will use the existing result of get_reg_list
> to check if the CSRs ids are present.
>
> kvm_riscv_init_multiext_cfg() is now kvm_riscv_init_multiext_csr_cfg()
^ kvm_riscv_init_cfg()
> to reflect that the function is also dealing with CSRs.
>
> [1] https://lore.kernel.org/qemu-riscv/CABJz62OfUDHYkQ0T3rGHStQprf1c7_E0qBLbLKhfv=+jb0SYAw@mail.gmail.com/
>
> Fixes: 4db19d5b21 ("target/riscv/kvm: add missing KVM CSRs")
> Reported-by: Andrea Bolognani <abologna@redhat.com>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> target/riscv/kvm/kvm-cpu.c | 62 ++++++++++++++++++++++++++++++++++++--
> 1 file changed, 59 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 7cbe566e5f..f341085ba1 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -636,6 +636,10 @@ static int kvm_riscv_get_regs_csr(CPUState *cs)
> for (i = 0; i < ARRAY_SIZE(kvm_csr_cfgs); i++) {
> KVMCPUConfig *csr_cfg = &kvm_csr_cfgs[i];
>
> + if (!csr_cfg->supported) {
> + continue;
> + }
> +
> ret = kvm_get_one_reg(cs, csr_cfg->kvm_reg_id, ®);
> if (ret) {
> return ret;
> @@ -662,6 +666,10 @@ static int kvm_riscv_put_regs_csr(CPUState *cs)
> for (i = 0; i < ARRAY_SIZE(kvm_csr_cfgs); i++) {
> KVMCPUConfig *csr_cfg = &kvm_csr_cfgs[i];
>
> + if (!csr_cfg->supported) {
> + continue;
> + }
> +
> if (KVM_REG_SIZE(csr_cfg->kvm_reg_id) == sizeof(uint32_t)) {
> reg = kvm_cpu_csr_get_u32(cpu, csr_cfg);
> } else if (KVM_REG_SIZE(csr_cfg->kvm_reg_id) == sizeof(uint64_t)) {
> @@ -1090,6 +1098,32 @@ static void kvm_riscv_read_multiext_legacy(RISCVCPU *cpu,
> }
> }
>
> +static void kvm_riscv_read_csr_cfg_legacy(KVMScratchCPU *kvmcpu)
> +{
> + uint64_t val;
> + int i, ret;
> +
> + for (i = 0; i < ARRAY_SIZE(kvm_csr_cfgs); i++) {
> + KVMCPUConfig *csr_cfg = &kvm_csr_cfgs[i];
> + struct kvm_one_reg reg;
> +
> + reg.id = csr_cfg->kvm_reg_id;
> + reg.addr = (uint64_t)&val;
> + ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
> + if (ret != 0) {
> + if (errno == EINVAL) {
> + csr_cfg->supported = false;
> + } else {
> + error_report("Unable to read KVM CSR %s: %s",
> + csr_cfg->name, strerror(errno));
> + exit(EXIT_FAILURE);
> + }
> + } else {
> + csr_cfg->supported = true;
> + }
> + }
> +}
> +
> static int uint64_cmp(const void *a, const void *b)
> {
> uint64_t val1 = *(const uint64_t *)a;
> @@ -1146,7 +1180,26 @@ static void kvm_riscv_read_vlenb(RISCVCPU *cpu, KVMScratchCPU *kvmcpu,
> }
> }
>
> -static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
> +static void kvm_riscv_read_csr_cfg(struct kvm_reg_list *reglist)
> +{
> + struct kvm_reg_list *reg_search;
> + uint64_t reg_id;
> +
> + for (int i = 0; i < ARRAY_SIZE(kvm_csr_cfgs); i++) {
> + KVMCPUConfig *csr_cfg = &kvm_csr_cfgs[i];
> +
> + reg_id = csr_cfg->kvm_reg_id;
> + reg_search = bsearch(®_id, reglist->reg, reglist->n,
> + sizeof(uint64_t), uint64_cmp);
> + if (!reg_search) {
> + continue;
> + }
> +
> + csr_cfg->supported = true;
> + }
> +}
> +
> +static void kvm_riscv_init_cfg(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
> {
> g_autofree struct kvm_reg_list *reglist = NULL;
> KVMCPUConfig *multi_ext_cfg;
> @@ -1163,7 +1216,9 @@ static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
> * (EINVAL). Use read_legacy() in this case.
> */
> if (errno == EINVAL) {
> - return kvm_riscv_read_multiext_legacy(cpu, kvmcpu);
> + kvm_riscv_read_multiext_legacy(cpu, kvmcpu);
> + kvm_riscv_read_csr_cfg_legacy(kvmcpu);
> + return;
> } else if (errno != E2BIG) {
> /*
> * E2BIG is an expected error message for the API since we
> @@ -1226,6 +1281,7 @@ static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
> }
>
> kvm_riscv_check_sbi_dbcn_support(cpu, reglist);
> + kvm_riscv_read_csr_cfg(reglist);
> }
>
> static void riscv_init_kvm_registers(Object *cpu_obj)
> @@ -1239,7 +1295,7 @@ static void riscv_init_kvm_registers(Object *cpu_obj)
>
> kvm_riscv_init_machine_ids(cpu, &kvmcpu);
> kvm_riscv_init_misa_ext_mask(cpu, &kvmcpu);
> - kvm_riscv_init_multiext_cfg(cpu, &kvmcpu);
> + kvm_riscv_init_cfg(cpu, &kvmcpu);
>
> kvm_riscv_destroy_scratch_vcpu(&kvmcpu);
> }
> --
> 2.49.0
>
Otherwise,
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 7/9] target/riscv/kvm: add senvcfg CSR
2025-04-25 11:36 [PATCH v2 0/9] target/riscv/kvm: CSR related fixes Daniel Henrique Barboza
` (5 preceding siblings ...)
2025-04-25 11:37 ` [PATCH v2 6/9] target/riscv/kvm: do not read unavailable CSRs Daniel Henrique Barboza
@ 2025-04-25 11:37 ` Daniel Henrique Barboza
2025-04-25 12:03 ` Andrew Jones
2025-04-25 11:37 ` [PATCH v2 8/9] target/riscv: widen (m|s)counteren to target_ulong Daniel Henrique Barboza
2025-04-25 11:37 ` [PATCH v2 9/9] target/riscv/kvm: add scounteren CSR Daniel Henrique Barboza
8 siblings, 1 reply; 20+ messages in thread
From: Daniel Henrique Barboza @ 2025-04-25 11:37 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, palmer,
ajones, Daniel Henrique Barboza
We're missing the senvcfg CSRs which is already present in the
KVM UAPI.
Reported-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
target/riscv/kvm/kvm-cpu.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index f341085ba1..e37fa38c07 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -249,6 +249,7 @@ static KVMCPUConfig kvm_csr_cfgs[] = {
KVM_CSR_CFG("stval", stval, RISCV_CSR_REG(stval)),
KVM_CSR_CFG("sip", mip, RISCV_CSR_REG(sip)),
KVM_CSR_CFG("satp", satp, RISCV_CSR_REG(satp)),
+ KVM_CSR_CFG("senvcfg", senvcfg, RISCV_CSR_REG(senvcfg)),
};
static void *kvmconfig_get_env_addr(RISCVCPU *cpu, KVMCPUConfig *csr_cfg)
@@ -698,6 +699,7 @@ static void kvm_riscv_reset_regs_csr(CPURISCVState *env)
env->stval = 0;
env->mip = 0;
env->satp = 0;
+ env->senvcfg = 0;
}
static int kvm_riscv_get_regs_fp(CPUState *cs)
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 7/9] target/riscv/kvm: add senvcfg CSR
2025-04-25 11:37 ` [PATCH v2 7/9] target/riscv/kvm: add senvcfg CSR Daniel Henrique Barboza
@ 2025-04-25 12:03 ` Andrew Jones
0 siblings, 0 replies; 20+ messages in thread
From: Andrew Jones @ 2025-04-25 12:03 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, liwei1518, zhiwei_liu,
palmer
On Fri, Apr 25, 2025 at 08:37:03AM -0300, Daniel Henrique Barboza wrote:
> We're missing the senvcfg CSRs which is already present in the
> KVM UAPI.
>
> Reported-by: Andrew Jones <ajones@ventanamicro.com>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> target/riscv/kvm/kvm-cpu.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index f341085ba1..e37fa38c07 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -249,6 +249,7 @@ static KVMCPUConfig kvm_csr_cfgs[] = {
> KVM_CSR_CFG("stval", stval, RISCV_CSR_REG(stval)),
> KVM_CSR_CFG("sip", mip, RISCV_CSR_REG(sip)),
> KVM_CSR_CFG("satp", satp, RISCV_CSR_REG(satp)),
> + KVM_CSR_CFG("senvcfg", senvcfg, RISCV_CSR_REG(senvcfg)),
> };
>
> static void *kvmconfig_get_env_addr(RISCVCPU *cpu, KVMCPUConfig *csr_cfg)
> @@ -698,6 +699,7 @@ static void kvm_riscv_reset_regs_csr(CPURISCVState *env)
> env->stval = 0;
> env->mip = 0;
> env->satp = 0;
> + env->senvcfg = 0;
> }
>
> static int kvm_riscv_get_regs_fp(CPUState *cs)
> --
> 2.49.0
>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 8/9] target/riscv: widen (m|s)counteren to target_ulong
2025-04-25 11:36 [PATCH v2 0/9] target/riscv/kvm: CSR related fixes Daniel Henrique Barboza
` (6 preceding siblings ...)
2025-04-25 11:37 ` [PATCH v2 7/9] target/riscv/kvm: add senvcfg CSR Daniel Henrique Barboza
@ 2025-04-25 11:37 ` Daniel Henrique Barboza
2025-04-25 12:11 ` Andrew Jones
2025-04-25 11:37 ` [PATCH v2 9/9] target/riscv/kvm: add scounteren CSR Daniel Henrique Barboza
8 siblings, 1 reply; 20+ messages in thread
From: Daniel Henrique Barboza @ 2025-04-25 11:37 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, palmer,
ajones, Daniel Henrique Barboza
We want to support scounteren as a KVM CSR. The KVM UAPI defines every
CSR size as target_ulong, and our env->scounteren is fixed at 32 bits.
The other existing cases where the property size does not match the KVM
reg size happens with uint64_t properties, like 'mstatus'. When running
a 32 bit CPU we'll write a 32 bit 'sstatus' KVM reg into the 64 bit
'mstatus' field. As long as we're consistent, i.e. we're always
reading/writing the same words, this is ok.
For scounteren, a KVM guest running in a 64 bit CPU will end up writing
a 64 bit reg in a 32 bit field. This will have all sort of funny side
effects in the KVM guest that we would rather avoid.
Increase scounteren to target_ulong to allow KVM to read/write the
scounteren CSR without any surprises. 'mcounteren' is being changed to
target_ulong for consistency.
Aside from bumping the version of the RISCVCPU vmstate no other
behavioral changes are expected.
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
target/riscv/cpu.h | 4 ++--
target/riscv/machine.c | 8 ++++----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index f5a60d0c52..0623c3187b 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -400,8 +400,8 @@ struct CPUArchState {
*/
bool two_stage_indirect_lookup;
- uint32_t scounteren;
- uint32_t mcounteren;
+ target_ulong scounteren;
+ target_ulong mcounteren;
uint32_t scountinhibit;
uint32_t mcountinhibit;
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index df2d5bad8d..4b11b203fb 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -401,8 +401,8 @@ static const VMStateDescription vmstate_ssp = {
const VMStateDescription vmstate_riscv_cpu = {
.name = "cpu",
- .version_id = 10,
- .minimum_version_id = 10,
+ .version_id = 11,
+ .minimum_version_id = 11,
.post_load = riscv_cpu_post_load,
.fields = (const VMStateField[]) {
VMSTATE_UINTTL_ARRAY(env.gpr, RISCVCPU, 32),
@@ -445,8 +445,8 @@ const VMStateDescription vmstate_riscv_cpu = {
VMSTATE_UINTTL(env.mtval, RISCVCPU),
VMSTATE_UINTTL(env.miselect, RISCVCPU),
VMSTATE_UINTTL(env.siselect, RISCVCPU),
- VMSTATE_UINT32(env.scounteren, RISCVCPU),
- VMSTATE_UINT32(env.mcounteren, RISCVCPU),
+ VMSTATE_UINTTL(env.scounteren, RISCVCPU),
+ VMSTATE_UINTTL(env.mcounteren, RISCVCPU),
VMSTATE_UINT32(env.scountinhibit, RISCVCPU),
VMSTATE_UINT32(env.mcountinhibit, RISCVCPU),
VMSTATE_STRUCT_ARRAY(env.pmu_ctrs, RISCVCPU, RV_MAX_MHPMCOUNTERS, 0,
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 8/9] target/riscv: widen (m|s)counteren to target_ulong
2025-04-25 11:37 ` [PATCH v2 8/9] target/riscv: widen (m|s)counteren to target_ulong Daniel Henrique Barboza
@ 2025-04-25 12:11 ` Andrew Jones
2025-04-25 13:10 ` Daniel Henrique Barboza
0 siblings, 1 reply; 20+ messages in thread
From: Andrew Jones @ 2025-04-25 12:11 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, liwei1518, zhiwei_liu,
palmer
On Fri, Apr 25, 2025 at 08:37:04AM -0300, Daniel Henrique Barboza wrote:
> We want to support scounteren as a KVM CSR. The KVM UAPI defines every
> CSR size as target_ulong, and our env->scounteren is fixed at 32 bits.
>
> The other existing cases where the property size does not match the KVM
> reg size happens with uint64_t properties, like 'mstatus'. When running
> a 32 bit CPU we'll write a 32 bit 'sstatus' KVM reg into the 64 bit
> 'mstatus' field. As long as we're consistent, i.e. we're always
> reading/writing the same words, this is ok.
>
> For scounteren, a KVM guest running in a 64 bit CPU will end up writing
> a 64 bit reg in a 32 bit field. This will have all sort of funny side
> effects in the KVM guest that we would rather avoid.
>
> Increase scounteren to target_ulong to allow KVM to read/write the
> scounteren CSR without any surprises. 'mcounteren' is being changed to
> target_ulong for consistency.
>
> Aside from bumping the version of the RISCVCPU vmstate no other
> behavioral changes are expected.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> target/riscv/cpu.h | 4 ++--
> target/riscv/machine.c | 8 ++++----
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index f5a60d0c52..0623c3187b 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -400,8 +400,8 @@ struct CPUArchState {
> */
> bool two_stage_indirect_lookup;
>
> - uint32_t scounteren;
> - uint32_t mcounteren;
> + target_ulong scounteren;
> + target_ulong mcounteren;
Let's leave mcounteren a u32 and write a comment above scounteren
explaining that it's supposed to be a u32 (as the spec says) but
we're using a ulong instead to support KVM's get/put due to how
it's defined in struct kvm_riscv_csr.
>
> uint32_t scountinhibit;
> uint32_t mcountinhibit;
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index df2d5bad8d..4b11b203fb 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -401,8 +401,8 @@ static const VMStateDescription vmstate_ssp = {
>
> const VMStateDescription vmstate_riscv_cpu = {
> .name = "cpu",
> - .version_id = 10,
> - .minimum_version_id = 10,
> + .version_id = 11,
> + .minimum_version_id = 11,
> .post_load = riscv_cpu_post_load,
> .fields = (const VMStateField[]) {
> VMSTATE_UINTTL_ARRAY(env.gpr, RISCVCPU, 32),
> @@ -445,8 +445,8 @@ const VMStateDescription vmstate_riscv_cpu = {
> VMSTATE_UINTTL(env.mtval, RISCVCPU),
> VMSTATE_UINTTL(env.miselect, RISCVCPU),
> VMSTATE_UINTTL(env.siselect, RISCVCPU),
> - VMSTATE_UINT32(env.scounteren, RISCVCPU),
> - VMSTATE_UINT32(env.mcounteren, RISCVCPU),
> + VMSTATE_UINTTL(env.scounteren, RISCVCPU),
> + VMSTATE_UINTTL(env.mcounteren, RISCVCPU),
Since we only expect the lower 32 bits to ever be written, then do we need
to make this change?
> VMSTATE_UINT32(env.scountinhibit, RISCVCPU),
> VMSTATE_UINT32(env.mcountinhibit, RISCVCPU),
> VMSTATE_STRUCT_ARRAY(env.pmu_ctrs, RISCVCPU, RV_MAX_MHPMCOUNTERS, 0,
> --
> 2.49.0
>
Thanks,
drew
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 8/9] target/riscv: widen (m|s)counteren to target_ulong
2025-04-25 12:11 ` Andrew Jones
@ 2025-04-25 13:10 ` Daniel Henrique Barboza
0 siblings, 0 replies; 20+ messages in thread
From: Daniel Henrique Barboza @ 2025-04-25 13:10 UTC (permalink / raw)
To: Andrew Jones
Cc: qemu-devel, qemu-riscv, alistair.francis, liwei1518, zhiwei_liu,
palmer
On 4/25/25 9:11 AM, Andrew Jones wrote:
> On Fri, Apr 25, 2025 at 08:37:04AM -0300, Daniel Henrique Barboza wrote:
>> We want to support scounteren as a KVM CSR. The KVM UAPI defines every
>> CSR size as target_ulong, and our env->scounteren is fixed at 32 bits.
>>
>> The other existing cases where the property size does not match the KVM
>> reg size happens with uint64_t properties, like 'mstatus'. When running
>> a 32 bit CPU we'll write a 32 bit 'sstatus' KVM reg into the 64 bit
>> 'mstatus' field. As long as we're consistent, i.e. we're always
>> reading/writing the same words, this is ok.
>>
>> For scounteren, a KVM guest running in a 64 bit CPU will end up writing
>> a 64 bit reg in a 32 bit field. This will have all sort of funny side
>> effects in the KVM guest that we would rather avoid.
>>
>> Increase scounteren to target_ulong to allow KVM to read/write the
>> scounteren CSR without any surprises. 'mcounteren' is being changed to
>> target_ulong for consistency.
>>
>> Aside from bumping the version of the RISCVCPU vmstate no other
>> behavioral changes are expected.
>>
>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>> ---
>> target/riscv/cpu.h | 4 ++--
>> target/riscv/machine.c | 8 ++++----
>> 2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index f5a60d0c52..0623c3187b 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -400,8 +400,8 @@ struct CPUArchState {
>> */
>> bool two_stage_indirect_lookup;
>>
>> - uint32_t scounteren;
>> - uint32_t mcounteren;
>> + target_ulong scounteren;
>> + target_ulong mcounteren;
>
> Let's leave mcounteren a u32 and write a comment above scounteren
> explaining that it's supposed to be a u32 (as the spec says) but
> we're using a ulong instead to support KVM's get/put due to how
> it's defined in struct kvm_riscv_csr.
Fair enough.
>
>>
>> uint32_t scountinhibit;
>> uint32_t mcountinhibit;
>> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
>> index df2d5bad8d..4b11b203fb 100644
>> --- a/target/riscv/machine.c
>> +++ b/target/riscv/machine.c
>> @@ -401,8 +401,8 @@ static const VMStateDescription vmstate_ssp = {
>>
>> const VMStateDescription vmstate_riscv_cpu = {
>> .name = "cpu",
>> - .version_id = 10,
>> - .minimum_version_id = 10,
>> + .version_id = 11,
>> + .minimum_version_id = 11,
>> .post_load = riscv_cpu_post_load,
>> .fields = (const VMStateField[]) {
>> VMSTATE_UINTTL_ARRAY(env.gpr, RISCVCPU, 32),
>> @@ -445,8 +445,8 @@ const VMStateDescription vmstate_riscv_cpu = {
>> VMSTATE_UINTTL(env.mtval, RISCVCPU),
>> VMSTATE_UINTTL(env.miselect, RISCVCPU),
>> VMSTATE_UINTTL(env.siselect, RISCVCPU),
>> - VMSTATE_UINT32(env.scounteren, RISCVCPU),
>> - VMSTATE_UINT32(env.mcounteren, RISCVCPU),
>> + VMSTATE_UINTTL(env.scounteren, RISCVCPU),
>> + VMSTATE_UINTTL(env.mcounteren, RISCVCPU),
>
> Since we only expect the lower 32 bits to ever be written, then do we need
> to make this change?
The VMSTATE_UINT32() macro checks for the size of the variable and will fail in compile
time if it's not an uint32_t:
In file included from /home/danielhb/work/qemu/include/qemu/osdep.h:53,
from ../target/riscv/machine.c:19:
/home/danielhb/work/qemu/include/qemu/compiler.h:65:35: error: invalid operands to binary - (have ‘uint32_t *’ {aka ‘unsigned int *’} and ‘target_ulong *’ {aka ‘long unsigned int *’})
65 | #define type_check(t1,t2) ((t1*)0 - (t2*)0)
| ^
/home/danielhb/work/qemu/include/migration/vmstate.h:270:6: note: in expansion of macro ‘type_check’
270 | type_check(_type, typeof_field(_state, _field)))
| ^~~~~~~~~~
/home/danielhb/work/qemu/include/migration/vmstate.h:321:21: note: in expansion of macro ‘vmstate_offset_value’
321 | .offset = vmstate_offset_value(_state, _field, _type), \
| ^~~~~~~~~~~~~~~~~~~~
/home/danielhb/work/qemu/include/migration/vmstate.h:854:5: note: in expansion of macro ‘VMSTATE_SINGLE_TEST’
854 | VMSTATE_SINGLE_TEST(_field, _state, NULL, _version, _info, _type)
| ^~~~~~~~~~~~~~~~~~~
/home/danielhb/work/qemu/include/migration/vmstate.h:902:5: note: in expansion of macro ‘VMSTATE_SINGLE’
902 | VMSTATE_SINGLE(_f, _s, _v, vmstate_info_uint32, uint32_t)
| ^~~~~~~~~~~~~~
/home/danielhb/work/qemu/include/migration/vmstate.h:939:5: note: in expansion of macro ‘VMSTATE_UINT32_V’
939 | VMSTATE_UINT32_V(_f, _s, 0)
| ^~~~~~~~~~~~~~~~
../target/riscv/machine.c:448:9: note: in expansion of macro ‘VMSTATE_UINT32’
448 | VMSTATE_UINT32(env.scounteren, RISCVCPU),
| ^~~~~~~~~~~~~~
Thanks,
Daniel
>
>> VMSTATE_UINT32(env.scountinhibit, RISCVCPU),
>> VMSTATE_UINT32(env.mcountinhibit, RISCVCPU),
>> VMSTATE_STRUCT_ARRAY(env.pmu_ctrs, RISCVCPU, RV_MAX_MHPMCOUNTERS, 0,
>> --
>> 2.49.0
>>
>
> Thanks,
> drew
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 9/9] target/riscv/kvm: add scounteren CSR
2025-04-25 11:36 [PATCH v2 0/9] target/riscv/kvm: CSR related fixes Daniel Henrique Barboza
` (7 preceding siblings ...)
2025-04-25 11:37 ` [PATCH v2 8/9] target/riscv: widen (m|s)counteren to target_ulong Daniel Henrique Barboza
@ 2025-04-25 11:37 ` Daniel Henrique Barboza
2025-04-25 12:12 ` Andrew Jones
8 siblings, 1 reply; 20+ messages in thread
From: Daniel Henrique Barboza @ 2025-04-25 11:37 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, palmer,
ajones, Daniel Henrique Barboza
Now that CPURISCVState::scounteren is a target_ulong, add support for
the scounteren KVM CSR.
Reported-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
target/riscv/kvm/kvm-cpu.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index e37fa38c07..5db63e78e2 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -250,6 +250,7 @@ static KVMCPUConfig kvm_csr_cfgs[] = {
KVM_CSR_CFG("sip", mip, RISCV_CSR_REG(sip)),
KVM_CSR_CFG("satp", satp, RISCV_CSR_REG(satp)),
KVM_CSR_CFG("senvcfg", senvcfg, RISCV_CSR_REG(senvcfg)),
+ KVM_CSR_CFG("scounteren", scounteren, RISCV_CSR_REG(scounteren)),
};
static void *kvmconfig_get_env_addr(RISCVCPU *cpu, KVMCPUConfig *csr_cfg)
@@ -700,6 +701,7 @@ static void kvm_riscv_reset_regs_csr(CPURISCVState *env)
env->mip = 0;
env->satp = 0;
env->senvcfg = 0;
+ env->scounteren = 0;
}
static int kvm_riscv_get_regs_fp(CPUState *cs)
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 9/9] target/riscv/kvm: add scounteren CSR
2025-04-25 11:37 ` [PATCH v2 9/9] target/riscv/kvm: add scounteren CSR Daniel Henrique Barboza
@ 2025-04-25 12:12 ` Andrew Jones
0 siblings, 0 replies; 20+ messages in thread
From: Andrew Jones @ 2025-04-25 12:12 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, liwei1518, zhiwei_liu,
palmer
On Fri, Apr 25, 2025 at 08:37:05AM -0300, Daniel Henrique Barboza wrote:
> Now that CPURISCVState::scounteren is a target_ulong, add support for
> the scounteren KVM CSR.
>
> Reported-by: Andrew Jones <ajones@ventanamicro.com>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> target/riscv/kvm/kvm-cpu.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index e37fa38c07..5db63e78e2 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -250,6 +250,7 @@ static KVMCPUConfig kvm_csr_cfgs[] = {
> KVM_CSR_CFG("sip", mip, RISCV_CSR_REG(sip)),
> KVM_CSR_CFG("satp", satp, RISCV_CSR_REG(satp)),
> KVM_CSR_CFG("senvcfg", senvcfg, RISCV_CSR_REG(senvcfg)),
> + KVM_CSR_CFG("scounteren", scounteren, RISCV_CSR_REG(scounteren)),
> };
>
> static void *kvmconfig_get_env_addr(RISCVCPU *cpu, KVMCPUConfig *csr_cfg)
> @@ -700,6 +701,7 @@ static void kvm_riscv_reset_regs_csr(CPURISCVState *env)
> env->mip = 0;
> env->satp = 0;
> env->senvcfg = 0;
> + env->scounteren = 0;
Let's keep these in struct kvm_riscv_csr order here and everywhere else
they get listed, like in kvm_csr_cfgs[]
> }
>
> static int kvm_riscv_get_regs_fp(CPUState *cs)
> --
> 2.49.0
>
Otherwise,
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply [flat|nested] 20+ messages in thread