* [PATCH for-9.0 0/5] target/riscv/kvm: fix KVM reg id sizes
@ 2023-12-08 18:38 Daniel Henrique Barboza
2023-12-08 18:38 ` [PATCH for-9.0 1/5] target/riscv/kvm: change KVM_REG_RISCV_FP_F to u32 Daniel Henrique Barboza
` (5 more replies)
0 siblings, 6 replies; 12+ messages in thread
From: Daniel Henrique Barboza @ 2023-12-08 18:38 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, ajones, Daniel Henrique Barboza
Hi,
While working in a follow-up for the Vector KVM regs, where we would
read 'vlenb' and then all other vregs [1], Drew noticed that we're using
kvm_riscv_reg_id() in registers that are u32 and u64.
The helper is returning ulong regs for all cases, meaning that we return
the wrong size for u32 regs when running 6 in 64 bits (case of FP_F
regs) and u64 regs when running in 32 bits (case of FP_D and timer
regs).
It's best to do a quick bug fix round in the KVM reg IDs before adding
more IDs, so here we are.
This is marked as 9.0 because the fixes aren't critical enough to
postpone 8.2. Michael, patches 1, 2 and 3 are good candidates for
8.2-stable.
Alistair, in case these are accepted I'll re-send "[PATCH for-9.0 0/4]
target/riscv: add RVV CSRs" based on these fixes to avoid conflicts in
the tree.
[1] this is dependent on kernel side changes that are being discussed
here: https://lore.kernel.org/kvm/20231205174509.2238870-1-dbarboza@ventanamicro.com/
("[PATCH v3 0/3] RISC-V, KVM: add 'vlenb' and vector CSRs to get-reg-list")
Daniel Henrique Barboza (5):
target/riscv/kvm: change KVM_REG_RISCV_FP_F to u32
target/riscv/kvm: change KVM_REG_RISCV_FP_D to u64
target/riscv/kvm: change timer regs size to u64
target/riscv/kvm: add RISCV_CONFIG_REG()
target/riscv/kvm: rename riscv_reg_id() to riscv_reg_id_ulong()
target/riscv/kvm/kvm-cpu.c | 109 ++++++++++++++++++++-----------------
1 file changed, 59 insertions(+), 50 deletions(-)
--
2.41.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH for-9.0 1/5] target/riscv/kvm: change KVM_REG_RISCV_FP_F to u32
2023-12-08 18:38 [PATCH for-9.0 0/5] target/riscv/kvm: fix KVM reg id sizes Daniel Henrique Barboza
@ 2023-12-08 18:38 ` Daniel Henrique Barboza
2023-12-15 13:08 ` Andrew Jones
2023-12-08 18:38 ` [PATCH for-9.0 2/5] target/riscv/kvm: change KVM_REG_RISCV_FP_D to u64 Daniel Henrique Barboza
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Daniel Henrique Barboza @ 2023-12-08 18:38 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, ajones, Daniel Henrique Barboza
KVM_REG_RISCV_FP_F regs have u32 size according to the API, but by using
kvm_riscv_reg_id() in RISCV_FP_F_REG() we're returning u64 sizes when
running with TARGET_RISCV64. The most likely reason why no one noticed
this is because we're not implementing kvm_cpu_synchronize_state() in
RISC-V yet.
Create a new helper that returns a KVM ID with u32 size and use it in
RISCV_FP_F_REG().
Reported-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
target/riscv/kvm/kvm-cpu.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 45b6cf1cfa..9bfbc4ac98 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -72,6 +72,11 @@ static uint64_t kvm_riscv_reg_id(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;
+}
+
#define RISCV_CORE_REG(env, name) kvm_riscv_reg_id(env, KVM_REG_RISCV_CORE, \
KVM_REG_RISCV_CORE_REG(name))
@@ -81,7 +86,7 @@ static uint64_t kvm_riscv_reg_id(CPURISCVState *env, uint64_t type,
#define RISCV_TIMER_REG(env, name) kvm_riscv_reg_id(env, KVM_REG_RISCV_TIMER, \
KVM_REG_RISCV_TIMER_REG(name))
-#define RISCV_FP_F_REG(env, idx) kvm_riscv_reg_id(env, 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(env, idx) kvm_riscv_reg_id(env, KVM_REG_RISCV_FP_D, idx)
@@ -586,7 +591,7 @@ static int kvm_riscv_get_regs_fp(CPUState *cs)
if (riscv_has_ext(env, RVF)) {
uint32_t reg;
for (i = 0; i < 32; i++) {
- ret = kvm_get_one_reg(cs, RISCV_FP_F_REG(env, i), ®);
+ ret = kvm_get_one_reg(cs, RISCV_FP_F_REG(i), ®);
if (ret) {
return ret;
}
@@ -620,7 +625,7 @@ static int kvm_riscv_put_regs_fp(CPUState *cs)
uint32_t reg;
for (i = 0; i < 32; i++) {
reg = env->fpr[i];
- ret = kvm_set_one_reg(cs, RISCV_FP_F_REG(env, i), ®);
+ ret = kvm_set_one_reg(cs, RISCV_FP_F_REG(i), ®);
if (ret) {
return ret;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH for-9.0 2/5] target/riscv/kvm: change KVM_REG_RISCV_FP_D to u64
2023-12-08 18:38 [PATCH for-9.0 0/5] target/riscv/kvm: fix KVM reg id sizes Daniel Henrique Barboza
2023-12-08 18:38 ` [PATCH for-9.0 1/5] target/riscv/kvm: change KVM_REG_RISCV_FP_F to u32 Daniel Henrique Barboza
@ 2023-12-08 18:38 ` Daniel Henrique Barboza
2023-12-15 13:11 ` Andrew Jones
2023-12-08 18:38 ` [PATCH for-9.0 3/5] target/riscv/kvm: change timer regs size " Daniel Henrique Barboza
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Daniel Henrique Barboza @ 2023-12-08 18:38 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, ajones, Daniel Henrique Barboza
KVM_REG_RISCV_FP_D regs are always u64 size. Using kvm_riscv_reg_id() in
RISCV_FP_D_REG() ends up encoding the wrong size if we're running with
TARGET_RISCV32.
Create a new helper that returns a KVM ID with u64 size and use it with
RISCV_FP_D_REG().
Reported-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
target/riscv/kvm/kvm-cpu.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 9bfbc4ac98..34ed82ebe5 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -77,6 +77,11 @@ 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;
+}
+
#define RISCV_CORE_REG(env, name) kvm_riscv_reg_id(env, KVM_REG_RISCV_CORE, \
KVM_REG_RISCV_CORE_REG(name))
@@ -88,7 +93,7 @@ static uint64_t kvm_riscv_reg_id_u32(uint64_t type, uint64_t idx)
#define RISCV_FP_F_REG(idx) kvm_riscv_reg_id_u32(KVM_REG_RISCV_FP_F, idx)
-#define RISCV_FP_D_REG(env, idx) kvm_riscv_reg_id(env, KVM_REG_RISCV_FP_D, idx)
+#define RISCV_FP_D_REG(idx) kvm_riscv_reg_id_u64(KVM_REG_RISCV_FP_D, idx)
#define KVM_RISCV_GET_CSR(cs, env, csr, reg) \
do { \
@@ -579,7 +584,7 @@ static int kvm_riscv_get_regs_fp(CPUState *cs)
if (riscv_has_ext(env, RVD)) {
uint64_t reg;
for (i = 0; i < 32; i++) {
- ret = kvm_get_one_reg(cs, RISCV_FP_D_REG(env, i), ®);
+ ret = kvm_get_one_reg(cs, RISCV_FP_D_REG(i), ®);
if (ret) {
return ret;
}
@@ -613,7 +618,7 @@ static int kvm_riscv_put_regs_fp(CPUState *cs)
uint64_t reg;
for (i = 0; i < 32; i++) {
reg = env->fpr[i];
- ret = kvm_set_one_reg(cs, RISCV_FP_D_REG(env, i), ®);
+ ret = kvm_set_one_reg(cs, RISCV_FP_D_REG(i), ®);
if (ret) {
return ret;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH for-9.0 3/5] target/riscv/kvm: change timer regs size to u64
2023-12-08 18:38 [PATCH for-9.0 0/5] target/riscv/kvm: fix KVM reg id sizes Daniel Henrique Barboza
2023-12-08 18:38 ` [PATCH for-9.0 1/5] target/riscv/kvm: change KVM_REG_RISCV_FP_F to u32 Daniel Henrique Barboza
2023-12-08 18:38 ` [PATCH for-9.0 2/5] target/riscv/kvm: change KVM_REG_RISCV_FP_D to u64 Daniel Henrique Barboza
@ 2023-12-08 18:38 ` Daniel Henrique Barboza
2023-12-15 13:14 ` Andrew Jones
2023-12-08 18:38 ` [PATCH for-9.0 4/5] target/riscv/kvm: add RISCV_CONFIG_REG() Daniel Henrique Barboza
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Daniel Henrique Barboza @ 2023-12-08 18:38 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, ajones, Daniel Henrique Barboza
KVM_REG_RISCV_TIMER regs are always u64 according to the KVM API, but at
this moment we'll return u32 regs if we're running a RISCV32 target.
Use the kvm_riscv_reg_id_u64() helper in RISCV_TIMER_REG() to fix it.
Reported-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
target/riscv/kvm/kvm-cpu.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 34ed82ebe5..476e5d4b3d 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -88,7 +88,7 @@ static uint64_t kvm_riscv_reg_id_u64(uint64_t type, uint64_t idx)
#define RISCV_CSR_REG(env, name) kvm_riscv_reg_id(env, KVM_REG_RISCV_CSR, \
KVM_REG_RISCV_CSR_REG(name))
-#define RISCV_TIMER_REG(env, name) kvm_riscv_reg_id(env, 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)
@@ -111,17 +111,17 @@ static uint64_t kvm_riscv_reg_id_u64(uint64_t type, uint64_t idx)
} \
} while (0)
-#define KVM_RISCV_GET_TIMER(cs, env, name, reg) \
+#define KVM_RISCV_GET_TIMER(cs, name, reg) \
do { \
- int ret = kvm_get_one_reg(cs, RISCV_TIMER_REG(env, name), ®); \
+ int ret = kvm_get_one_reg(cs, RISCV_TIMER_REG(name), ®); \
if (ret) { \
abort(); \
} \
} while (0)
-#define KVM_RISCV_SET_TIMER(cs, env, name, reg) \
+#define KVM_RISCV_SET_TIMER(cs, name, reg) \
do { \
- int ret = kvm_set_one_reg(cs, RISCV_TIMER_REG(env, name), ®); \
+ int ret = kvm_set_one_reg(cs, RISCV_TIMER_REG(name), ®); \
if (ret) { \
abort(); \
} \
@@ -649,10 +649,10 @@ static void kvm_riscv_get_regs_timer(CPUState *cs)
return;
}
- KVM_RISCV_GET_TIMER(cs, env, time, env->kvm_timer_time);
- KVM_RISCV_GET_TIMER(cs, env, compare, env->kvm_timer_compare);
- KVM_RISCV_GET_TIMER(cs, env, state, env->kvm_timer_state);
- KVM_RISCV_GET_TIMER(cs, env, frequency, env->kvm_timer_frequency);
+ KVM_RISCV_GET_TIMER(cs, time, env->kvm_timer_time);
+ KVM_RISCV_GET_TIMER(cs, compare, env->kvm_timer_compare);
+ KVM_RISCV_GET_TIMER(cs, state, env->kvm_timer_state);
+ KVM_RISCV_GET_TIMER(cs, frequency, env->kvm_timer_frequency);
env->kvm_timer_dirty = true;
}
@@ -666,8 +666,8 @@ static void kvm_riscv_put_regs_timer(CPUState *cs)
return;
}
- KVM_RISCV_SET_TIMER(cs, env, time, env->kvm_timer_time);
- KVM_RISCV_SET_TIMER(cs, env, compare, env->kvm_timer_compare);
+ KVM_RISCV_SET_TIMER(cs, time, env->kvm_timer_time);
+ KVM_RISCV_SET_TIMER(cs, compare, env->kvm_timer_compare);
/*
* To set register of RISCV_TIMER_REG(state) will occur a error from KVM
@@ -676,7 +676,7 @@ static void kvm_riscv_put_regs_timer(CPUState *cs)
* TODO If KVM changes, adapt here.
*/
if (env->kvm_timer_state) {
- KVM_RISCV_SET_TIMER(cs, env, state, env->kvm_timer_state);
+ KVM_RISCV_SET_TIMER(cs, state, env->kvm_timer_state);
}
/*
@@ -685,7 +685,7 @@ static void kvm_riscv_put_regs_timer(CPUState *cs)
* during the migration.
*/
if (migration_is_running(migrate_get_current()->state)) {
- KVM_RISCV_GET_TIMER(cs, env, frequency, reg);
+ KVM_RISCV_GET_TIMER(cs, frequency, reg);
if (reg != env->kvm_timer_frequency) {
error_report("Dst Hosts timer frequency != Src Hosts");
}
--
2.41.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH for-9.0 4/5] target/riscv/kvm: add RISCV_CONFIG_REG()
2023-12-08 18:38 [PATCH for-9.0 0/5] target/riscv/kvm: fix KVM reg id sizes Daniel Henrique Barboza
` (2 preceding siblings ...)
2023-12-08 18:38 ` [PATCH for-9.0 3/5] target/riscv/kvm: change timer regs size " Daniel Henrique Barboza
@ 2023-12-08 18:38 ` Daniel Henrique Barboza
2023-12-15 13:18 ` Andrew Jones
2023-12-08 18:38 ` [PATCH for-9.0 5/5] target/riscv/kvm: rename riscv_reg_id() to riscv_reg_id_ulong() Daniel Henrique Barboza
2023-12-18 0:09 ` [PATCH for-9.0 0/5] target/riscv/kvm: fix KVM reg id sizes Alistair Francis
5 siblings, 1 reply; 12+ messages in thread
From: Daniel Henrique Barboza @ 2023-12-08 18:38 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, ajones, Daniel Henrique Barboza
Create a RISCV_CONFIG_REG() macro, similar to what other regs use, to
hide away some of the boilerplate.
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
target/riscv/kvm/kvm-cpu.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 476e5d4b3d..11797338ec 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -88,6 +88,10 @@ static uint64_t kvm_riscv_reg_id_u64(uint64_t type, uint64_t idx)
#define RISCV_CSR_REG(env, name) kvm_riscv_reg_id(env, KVM_REG_RISCV_CSR, \
KVM_REG_RISCV_CSR_REG(name))
+#define RISCV_CONFIG_REG(env, name) \
+ kvm_riscv_reg_id(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, \
KVM_REG_RISCV_TIMER_REG(name))
@@ -756,24 +760,21 @@ static void kvm_riscv_init_machine_ids(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
struct kvm_one_reg reg;
int ret;
- reg.id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG,
- KVM_REG_RISCV_CONFIG_REG(mvendorid));
+ reg.id = RISCV_CONFIG_REG(env, 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 = kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG,
- KVM_REG_RISCV_CONFIG_REG(marchid));
+ reg.id = RISCV_CONFIG_REG(env, 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 = kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG,
- KVM_REG_RISCV_CONFIG_REG(mimpid));
+ reg.id = RISCV_CONFIG_REG(env, mimpid);
reg.addr = (uint64_t)&cpu->cfg.mimpid;
ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
if (ret != 0) {
@@ -788,8 +789,7 @@ static void kvm_riscv_init_misa_ext_mask(RISCVCPU *cpu,
struct kvm_one_reg reg;
int ret;
- reg.id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG,
- KVM_REG_RISCV_CONFIG_REG(isa));
+ reg.id = RISCV_CONFIG_REG(env, isa);
reg.addr = (uint64_t)&env->misa_ext_mask;
ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
@@ -1094,8 +1094,7 @@ static int kvm_vcpu_set_machine_ids(RISCVCPU *cpu, CPUState *cs)
uint64_t id;
int ret;
- id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG,
- KVM_REG_RISCV_CONFIG_REG(mvendorid));
+ id = RISCV_CONFIG_REG(env, mvendorid);
/*
* cfg.mvendorid is an uint32 but a target_ulong will
* be written. Assign it to a target_ulong var to avoid
@@ -1107,15 +1106,13 @@ static int kvm_vcpu_set_machine_ids(RISCVCPU *cpu, CPUState *cs)
return ret;
}
- id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG,
- KVM_REG_RISCV_CONFIG_REG(marchid));
+ id = RISCV_CONFIG_REG(env, marchid);
ret = kvm_set_one_reg(cs, id, &cpu->cfg.marchid);
if (ret != 0) {
return ret;
}
- id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG,
- KVM_REG_RISCV_CONFIG_REG(mimpid));
+ id = RISCV_CONFIG_REG(env, mimpid);
ret = kvm_set_one_reg(cs, id, &cpu->cfg.mimpid);
return ret;
--
2.41.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH for-9.0 5/5] target/riscv/kvm: rename riscv_reg_id() to riscv_reg_id_ulong()
2023-12-08 18:38 [PATCH for-9.0 0/5] target/riscv/kvm: fix KVM reg id sizes Daniel Henrique Barboza
` (3 preceding siblings ...)
2023-12-08 18:38 ` [PATCH for-9.0 4/5] target/riscv/kvm: add RISCV_CONFIG_REG() Daniel Henrique Barboza
@ 2023-12-08 18:38 ` Daniel Henrique Barboza
2023-12-15 13:19 ` Andrew Jones
2023-12-18 0:09 ` [PATCH for-9.0 0/5] target/riscv/kvm: fix KVM reg id sizes Alistair Francis
5 siblings, 1 reply; 12+ messages in thread
From: Daniel Henrique Barboza @ 2023-12-08 18:38 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, ajones, Daniel Henrique Barboza
kvm_riscv_reg_id() returns an id encoded with an ulong size, i.e. an u32
size when running TARGET_RISCV32 and u64 when running TARGET_RISCV64.
Rename it to kvm_riscv_reg_id_ulong() to enhance code readability. It'll
be in line with the existing kvm_riscv_reg_id_<size>() helpers.
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
target/riscv/kvm/kvm-cpu.c | 40 ++++++++++++++++++++------------------
1 file changed, 21 insertions(+), 19 deletions(-)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 11797338ec..62a1e51f0a 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -54,7 +54,7 @@ void riscv_kvm_aplic_request(void *opaque, int irq, int level)
static bool cap_has_mp_state;
-static uint64_t kvm_riscv_reg_id(CPURISCVState *env, uint64_t type,
+static uint64_t kvm_riscv_reg_id_ulong(CPURISCVState *env, uint64_t type,
uint64_t idx)
{
uint64_t id = KVM_REG_RISCV | type | idx;
@@ -82,15 +82,17 @@ static uint64_t kvm_riscv_reg_id_u64(uint64_t type, uint64_t idx)
return KVM_REG_RISCV | KVM_REG_SIZE_U64 | type | idx;
}
-#define RISCV_CORE_REG(env, name) kvm_riscv_reg_id(env, KVM_REG_RISCV_CORE, \
- KVM_REG_RISCV_CORE_REG(name))
+#define RISCV_CORE_REG(env, name) \
+ kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, \
+ KVM_REG_RISCV_CORE_REG(name))
-#define RISCV_CSR_REG(env, name) kvm_riscv_reg_id(env, KVM_REG_RISCV_CSR, \
- KVM_REG_RISCV_CSR_REG(name))
+#define RISCV_CSR_REG(env, name) \
+ kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CSR, \
+ KVM_REG_RISCV_CSR_REG(name))
#define RISCV_CONFIG_REG(env, name) \
- kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG, \
- KVM_REG_RISCV_CONFIG_REG(name))
+ 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, \
KVM_REG_RISCV_TIMER_REG(name))
@@ -216,8 +218,8 @@ 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(env, KVM_REG_RISCV_ISA_EXT,
- misa_cfg->kvm_reg_id);
+ id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_ISA_EXT,
+ misa_cfg->kvm_reg_id);
ret = kvm_set_one_reg(cs, id, ®);
if (ret != 0) {
/*
@@ -378,8 +380,8 @@ static void kvm_riscv_update_cpu_cfg_isa_ext(RISCVCPU *cpu, CPUState *cs)
continue;
}
- id = kvm_riscv_reg_id(env, KVM_REG_RISCV_ISA_EXT,
- multi_ext_cfg->kvm_reg_id);
+ id = kvm_riscv_reg_id_ulong(env, 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, ®);
if (ret != 0) {
@@ -509,7 +511,7 @@ static int kvm_riscv_get_regs_core(CPUState *cs)
env->pc = reg;
for (i = 1; i < 32; i++) {
- uint64_t id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CORE, i);
+ uint64_t id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, i);
ret = kvm_get_one_reg(cs, id, ®);
if (ret) {
return ret;
@@ -534,7 +536,7 @@ static int kvm_riscv_put_regs_core(CPUState *cs)
}
for (i = 1; i < 32; i++) {
- uint64_t id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CORE, i);
+ uint64_t id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, i);
reg = env->gpr[i];
ret = kvm_set_one_reg(cs, id, ®);
if (ret) {
@@ -810,8 +812,8 @@ static void kvm_riscv_read_cbomz_blksize(RISCVCPU *cpu, KVMScratchCPU *kvmcpu,
struct kvm_one_reg reg;
int ret;
- reg.id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG,
- cbomz_cfg->kvm_reg_id);
+ reg.id = kvm_riscv_reg_id_ulong(env, 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, ®);
if (ret != 0) {
@@ -832,8 +834,8 @@ 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(env, KVM_REG_RISCV_ISA_EXT,
- multi_ext_cfg->kvm_reg_id);
+ reg.id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_ISA_EXT,
+ multi_ext_cfg->kvm_reg_id);
reg.addr = (uint64_t)&val;
ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
if (ret != 0) {
@@ -925,8 +927,8 @@ 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(&cpu->env, KVM_REG_RISCV_ISA_EXT,
- multi_ext_cfg->kvm_reg_id);
+ reg_id = kvm_riscv_reg_id_ulong(&cpu->env, KVM_REG_RISCV_ISA_EXT,
+ multi_ext_cfg->kvm_reg_id);
reg_search = bsearch(®_id, reglist->reg, reglist->n,
sizeof(uint64_t), uint64_cmp);
if (!reg_search) {
--
2.41.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH for-9.0 1/5] target/riscv/kvm: change KVM_REG_RISCV_FP_F to u32
2023-12-08 18:38 ` [PATCH for-9.0 1/5] target/riscv/kvm: change KVM_REG_RISCV_FP_F to u32 Daniel Henrique Barboza
@ 2023-12-15 13:08 ` Andrew Jones
0 siblings, 0 replies; 12+ messages in thread
From: Andrew Jones @ 2023-12-15 13:08 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518,
zhiwei_liu, palmer
On Fri, Dec 08, 2023 at 03:38:31PM -0300, Daniel Henrique Barboza wrote:
> KVM_REG_RISCV_FP_F regs have u32 size according to the API, but by using
> kvm_riscv_reg_id() in RISCV_FP_F_REG() we're returning u64 sizes when
> running with TARGET_RISCV64. The most likely reason why no one noticed
> this is because we're not implementing kvm_cpu_synchronize_state() in
> RISC-V yet.
>
> Create a new helper that returns a KVM ID with u32 size and use it in
> RISCV_FP_F_REG().
>
> Reported-by: Andrew Jones <ajones@ventanamicro.com>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> target/riscv/kvm/kvm-cpu.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 45b6cf1cfa..9bfbc4ac98 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -72,6 +72,11 @@ static uint64_t kvm_riscv_reg_id(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;
> +}
> +
> #define RISCV_CORE_REG(env, name) kvm_riscv_reg_id(env, KVM_REG_RISCV_CORE, \
> KVM_REG_RISCV_CORE_REG(name))
>
> @@ -81,7 +86,7 @@ static uint64_t kvm_riscv_reg_id(CPURISCVState *env, uint64_t type,
> #define RISCV_TIMER_REG(env, name) kvm_riscv_reg_id(env, KVM_REG_RISCV_TIMER, \
> KVM_REG_RISCV_TIMER_REG(name))
>
> -#define RISCV_FP_F_REG(env, idx) kvm_riscv_reg_id(env, 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(env, idx) kvm_riscv_reg_id(env, KVM_REG_RISCV_FP_D, idx)
>
> @@ -586,7 +591,7 @@ static int kvm_riscv_get_regs_fp(CPUState *cs)
> if (riscv_has_ext(env, RVF)) {
> uint32_t reg;
> for (i = 0; i < 32; i++) {
> - ret = kvm_get_one_reg(cs, RISCV_FP_F_REG(env, i), ®);
> + ret = kvm_get_one_reg(cs, RISCV_FP_F_REG(i), ®);
> if (ret) {
> return ret;
> }
> @@ -620,7 +625,7 @@ static int kvm_riscv_put_regs_fp(CPUState *cs)
> uint32_t reg;
> for (i = 0; i < 32; i++) {
> reg = env->fpr[i];
> - ret = kvm_set_one_reg(cs, RISCV_FP_F_REG(env, i), ®);
> + ret = kvm_set_one_reg(cs, RISCV_FP_F_REG(i), ®);
> if (ret) {
> return ret;
> }
> --
> 2.41.0
>
This patch looks good, so
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
But... I don't see where we save/restore __riscv_f_ext_state.fcsr
Thanks,
drew
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH for-9.0 2/5] target/riscv/kvm: change KVM_REG_RISCV_FP_D to u64
2023-12-08 18:38 ` [PATCH for-9.0 2/5] target/riscv/kvm: change KVM_REG_RISCV_FP_D to u64 Daniel Henrique Barboza
@ 2023-12-15 13:11 ` Andrew Jones
0 siblings, 0 replies; 12+ messages in thread
From: Andrew Jones @ 2023-12-15 13:11 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518,
zhiwei_liu, palmer
On Fri, Dec 08, 2023 at 03:38:32PM -0300, Daniel Henrique Barboza wrote:
> KVM_REG_RISCV_FP_D regs are always u64 size. Using kvm_riscv_reg_id() in
> RISCV_FP_D_REG() ends up encoding the wrong size if we're running with
> TARGET_RISCV32.
>
> Create a new helper that returns a KVM ID with u64 size and use it with
> RISCV_FP_D_REG().
>
> Reported-by: Andrew Jones <ajones@ventanamicro.com>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> target/riscv/kvm/kvm-cpu.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 9bfbc4ac98..34ed82ebe5 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -77,6 +77,11 @@ 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;
> +}
> +
> #define RISCV_CORE_REG(env, name) kvm_riscv_reg_id(env, KVM_REG_RISCV_CORE, \
> KVM_REG_RISCV_CORE_REG(name))
>
> @@ -88,7 +93,7 @@ static uint64_t kvm_riscv_reg_id_u32(uint64_t type, uint64_t idx)
>
> #define RISCV_FP_F_REG(idx) kvm_riscv_reg_id_u32(KVM_REG_RISCV_FP_F, idx)
>
> -#define RISCV_FP_D_REG(env, idx) kvm_riscv_reg_id(env, KVM_REG_RISCV_FP_D, idx)
> +#define RISCV_FP_D_REG(idx) kvm_riscv_reg_id_u64(KVM_REG_RISCV_FP_D, idx)
>
> #define KVM_RISCV_GET_CSR(cs, env, csr, reg) \
> do { \
> @@ -579,7 +584,7 @@ static int kvm_riscv_get_regs_fp(CPUState *cs)
> if (riscv_has_ext(env, RVD)) {
> uint64_t reg;
> for (i = 0; i < 32; i++) {
> - ret = kvm_get_one_reg(cs, RISCV_FP_D_REG(env, i), ®);
> + ret = kvm_get_one_reg(cs, RISCV_FP_D_REG(i), ®);
> if (ret) {
> return ret;
> }
> @@ -613,7 +618,7 @@ static int kvm_riscv_put_regs_fp(CPUState *cs)
> uint64_t reg;
> for (i = 0; i < 32; i++) {
> reg = env->fpr[i];
> - ret = kvm_set_one_reg(cs, RISCV_FP_D_REG(env, i), ®);
> + ret = kvm_set_one_reg(cs, RISCV_FP_D_REG(i), ®);
> if (ret) {
> return ret;
> }
> --
> 2.41.0
>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
afaict, we're also missing fcsr here. And watch out for D's fcsr, it's
32-bit, even though the rest of the registers are 64-bit.
Thanks,
drew
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH for-9.0 3/5] target/riscv/kvm: change timer regs size to u64
2023-12-08 18:38 ` [PATCH for-9.0 3/5] target/riscv/kvm: change timer regs size " Daniel Henrique Barboza
@ 2023-12-15 13:14 ` Andrew Jones
0 siblings, 0 replies; 12+ messages in thread
From: Andrew Jones @ 2023-12-15 13:14 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518,
zhiwei_liu, palmer
On Fri, Dec 08, 2023 at 03:38:33PM -0300, Daniel Henrique Barboza wrote:
> KVM_REG_RISCV_TIMER regs are always u64 according to the KVM API, but at
> this moment we'll return u32 regs if we're running a RISCV32 target.
>
> Use the kvm_riscv_reg_id_u64() helper in RISCV_TIMER_REG() to fix it.
>
> Reported-by: Andrew Jones <ajones@ventanamicro.com>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> target/riscv/kvm/kvm-cpu.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 34ed82ebe5..476e5d4b3d 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -88,7 +88,7 @@ static uint64_t kvm_riscv_reg_id_u64(uint64_t type, uint64_t idx)
> #define RISCV_CSR_REG(env, name) kvm_riscv_reg_id(env, KVM_REG_RISCV_CSR, \
> KVM_REG_RISCV_CSR_REG(name))
>
> -#define RISCV_TIMER_REG(env, name) kvm_riscv_reg_id(env, 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)
> @@ -111,17 +111,17 @@ static uint64_t kvm_riscv_reg_id_u64(uint64_t type, uint64_t idx)
> } \
> } while (0)
>
> -#define KVM_RISCV_GET_TIMER(cs, env, name, reg) \
> +#define KVM_RISCV_GET_TIMER(cs, name, reg) \
> do { \
> - int ret = kvm_get_one_reg(cs, RISCV_TIMER_REG(env, name), ®); \
> + int ret = kvm_get_one_reg(cs, RISCV_TIMER_REG(name), ®); \
> if (ret) { \
> abort(); \
> } \
> } while (0)
>
> -#define KVM_RISCV_SET_TIMER(cs, env, name, reg) \
> +#define KVM_RISCV_SET_TIMER(cs, name, reg) \
> do { \
> - int ret = kvm_set_one_reg(cs, RISCV_TIMER_REG(env, name), ®); \
> + int ret = kvm_set_one_reg(cs, RISCV_TIMER_REG(name), ®); \
> if (ret) { \
> abort(); \
> } \
> @@ -649,10 +649,10 @@ static void kvm_riscv_get_regs_timer(CPUState *cs)
> return;
> }
>
> - KVM_RISCV_GET_TIMER(cs, env, time, env->kvm_timer_time);
> - KVM_RISCV_GET_TIMER(cs, env, compare, env->kvm_timer_compare);
> - KVM_RISCV_GET_TIMER(cs, env, state, env->kvm_timer_state);
> - KVM_RISCV_GET_TIMER(cs, env, frequency, env->kvm_timer_frequency);
> + KVM_RISCV_GET_TIMER(cs, time, env->kvm_timer_time);
> + KVM_RISCV_GET_TIMER(cs, compare, env->kvm_timer_compare);
> + KVM_RISCV_GET_TIMER(cs, state, env->kvm_timer_state);
> + KVM_RISCV_GET_TIMER(cs, frequency, env->kvm_timer_frequency);
>
> env->kvm_timer_dirty = true;
> }
> @@ -666,8 +666,8 @@ static void kvm_riscv_put_regs_timer(CPUState *cs)
> return;
> }
>
> - KVM_RISCV_SET_TIMER(cs, env, time, env->kvm_timer_time);
> - KVM_RISCV_SET_TIMER(cs, env, compare, env->kvm_timer_compare);
> + KVM_RISCV_SET_TIMER(cs, time, env->kvm_timer_time);
> + KVM_RISCV_SET_TIMER(cs, compare, env->kvm_timer_compare);
>
> /*
> * To set register of RISCV_TIMER_REG(state) will occur a error from KVM
> @@ -676,7 +676,7 @@ static void kvm_riscv_put_regs_timer(CPUState *cs)
> * TODO If KVM changes, adapt here.
> */
> if (env->kvm_timer_state) {
> - KVM_RISCV_SET_TIMER(cs, env, state, env->kvm_timer_state);
> + KVM_RISCV_SET_TIMER(cs, state, env->kvm_timer_state);
> }
>
> /*
> @@ -685,7 +685,7 @@ static void kvm_riscv_put_regs_timer(CPUState *cs)
> * during the migration.
> */
> if (migration_is_running(migrate_get_current()->state)) {
> - KVM_RISCV_GET_TIMER(cs, env, frequency, reg);
> + KVM_RISCV_GET_TIMER(cs, frequency, reg);
> if (reg != env->kvm_timer_frequency) {
> error_report("Dst Hosts timer frequency != Src Hosts");
> }
> --
> 2.41.0
>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH for-9.0 4/5] target/riscv/kvm: add RISCV_CONFIG_REG()
2023-12-08 18:38 ` [PATCH for-9.0 4/5] target/riscv/kvm: add RISCV_CONFIG_REG() Daniel Henrique Barboza
@ 2023-12-15 13:18 ` Andrew Jones
0 siblings, 0 replies; 12+ messages in thread
From: Andrew Jones @ 2023-12-15 13:18 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518,
zhiwei_liu, palmer
On Fri, Dec 08, 2023 at 03:38:34PM -0300, Daniel Henrique Barboza wrote:
> Create a RISCV_CONFIG_REG() macro, similar to what other regs use, to
> hide away some of the boilerplate.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> target/riscv/kvm/kvm-cpu.c | 25 +++++++++++--------------
> 1 file changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 476e5d4b3d..11797338ec 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -88,6 +88,10 @@ static uint64_t kvm_riscv_reg_id_u64(uint64_t type, uint64_t idx)
> #define RISCV_CSR_REG(env, name) kvm_riscv_reg_id(env, KVM_REG_RISCV_CSR, \
> KVM_REG_RISCV_CSR_REG(name))
>
> +#define RISCV_CONFIG_REG(env, name) \
> + kvm_riscv_reg_id(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, \
> KVM_REG_RISCV_TIMER_REG(name))
>
> @@ -756,24 +760,21 @@ static void kvm_riscv_init_machine_ids(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
> struct kvm_one_reg reg;
> int ret;
>
> - reg.id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG,
> - KVM_REG_RISCV_CONFIG_REG(mvendorid));
> + reg.id = RISCV_CONFIG_REG(env, 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 = kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG,
> - KVM_REG_RISCV_CONFIG_REG(marchid));
> + reg.id = RISCV_CONFIG_REG(env, 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 = kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG,
> - KVM_REG_RISCV_CONFIG_REG(mimpid));
> + reg.id = RISCV_CONFIG_REG(env, mimpid);
> reg.addr = (uint64_t)&cpu->cfg.mimpid;
> ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
> if (ret != 0) {
> @@ -788,8 +789,7 @@ static void kvm_riscv_init_misa_ext_mask(RISCVCPU *cpu,
> struct kvm_one_reg reg;
> int ret;
>
> - reg.id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG,
> - KVM_REG_RISCV_CONFIG_REG(isa));
> + reg.id = RISCV_CONFIG_REG(env, isa);
> reg.addr = (uint64_t)&env->misa_ext_mask;
> ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
>
> @@ -1094,8 +1094,7 @@ static int kvm_vcpu_set_machine_ids(RISCVCPU *cpu, CPUState *cs)
> uint64_t id;
> int ret;
>
> - id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG,
> - KVM_REG_RISCV_CONFIG_REG(mvendorid));
> + id = RISCV_CONFIG_REG(env, mvendorid);
> /*
> * cfg.mvendorid is an uint32 but a target_ulong will
> * be written. Assign it to a target_ulong var to avoid
> @@ -1107,15 +1106,13 @@ static int kvm_vcpu_set_machine_ids(RISCVCPU *cpu, CPUState *cs)
> return ret;
> }
>
> - id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG,
> - KVM_REG_RISCV_CONFIG_REG(marchid));
> + id = RISCV_CONFIG_REG(env, marchid);
> ret = kvm_set_one_reg(cs, id, &cpu->cfg.marchid);
> if (ret != 0) {
> return ret;
> }
>
> - id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG,
> - KVM_REG_RISCV_CONFIG_REG(mimpid));
> + id = RISCV_CONFIG_REG(env, mimpid);
> ret = kvm_set_one_reg(cs, id, &cpu->cfg.mimpid);
>
> return ret;
> --
> 2.41.0
>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH for-9.0 5/5] target/riscv/kvm: rename riscv_reg_id() to riscv_reg_id_ulong()
2023-12-08 18:38 ` [PATCH for-9.0 5/5] target/riscv/kvm: rename riscv_reg_id() to riscv_reg_id_ulong() Daniel Henrique Barboza
@ 2023-12-15 13:19 ` Andrew Jones
0 siblings, 0 replies; 12+ messages in thread
From: Andrew Jones @ 2023-12-15 13:19 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518,
zhiwei_liu, palmer
On Fri, Dec 08, 2023 at 03:38:35PM -0300, Daniel Henrique Barboza wrote:
> kvm_riscv_reg_id() returns an id encoded with an ulong size, i.e. an u32
> size when running TARGET_RISCV32 and u64 when running TARGET_RISCV64.
>
> Rename it to kvm_riscv_reg_id_ulong() to enhance code readability. It'll
> be in line with the existing kvm_riscv_reg_id_<size>() helpers.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> target/riscv/kvm/kvm-cpu.c | 40 ++++++++++++++++++++------------------
> 1 file changed, 21 insertions(+), 19 deletions(-)
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 11797338ec..62a1e51f0a 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -54,7 +54,7 @@ void riscv_kvm_aplic_request(void *opaque, int irq, int level)
>
> static bool cap_has_mp_state;
>
> -static uint64_t kvm_riscv_reg_id(CPURISCVState *env, uint64_t type,
> +static uint64_t kvm_riscv_reg_id_ulong(CPURISCVState *env, uint64_t type,
> uint64_t idx)
> {
> uint64_t id = KVM_REG_RISCV | type | idx;
> @@ -82,15 +82,17 @@ static uint64_t kvm_riscv_reg_id_u64(uint64_t type, uint64_t idx)
> return KVM_REG_RISCV | KVM_REG_SIZE_U64 | type | idx;
> }
>
> -#define RISCV_CORE_REG(env, name) kvm_riscv_reg_id(env, KVM_REG_RISCV_CORE, \
> - KVM_REG_RISCV_CORE_REG(name))
> +#define RISCV_CORE_REG(env, name) \
> + kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, \
> + KVM_REG_RISCV_CORE_REG(name))
>
> -#define RISCV_CSR_REG(env, name) kvm_riscv_reg_id(env, KVM_REG_RISCV_CSR, \
> - KVM_REG_RISCV_CSR_REG(name))
> +#define RISCV_CSR_REG(env, name) \
> + kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CSR, \
> + KVM_REG_RISCV_CSR_REG(name))
>
> #define RISCV_CONFIG_REG(env, name) \
> - kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG, \
> - KVM_REG_RISCV_CONFIG_REG(name))
> + 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, \
> KVM_REG_RISCV_TIMER_REG(name))
> @@ -216,8 +218,8 @@ 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(env, KVM_REG_RISCV_ISA_EXT,
> - misa_cfg->kvm_reg_id);
> + id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_ISA_EXT,
> + misa_cfg->kvm_reg_id);
> ret = kvm_set_one_reg(cs, id, ®);
> if (ret != 0) {
> /*
> @@ -378,8 +380,8 @@ static void kvm_riscv_update_cpu_cfg_isa_ext(RISCVCPU *cpu, CPUState *cs)
> continue;
> }
>
> - id = kvm_riscv_reg_id(env, KVM_REG_RISCV_ISA_EXT,
> - multi_ext_cfg->kvm_reg_id);
> + id = kvm_riscv_reg_id_ulong(env, 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, ®);
> if (ret != 0) {
> @@ -509,7 +511,7 @@ static int kvm_riscv_get_regs_core(CPUState *cs)
> env->pc = reg;
>
> for (i = 1; i < 32; i++) {
> - uint64_t id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CORE, i);
> + uint64_t id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, i);
> ret = kvm_get_one_reg(cs, id, ®);
> if (ret) {
> return ret;
> @@ -534,7 +536,7 @@ static int kvm_riscv_put_regs_core(CPUState *cs)
> }
>
> for (i = 1; i < 32; i++) {
> - uint64_t id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CORE, i);
> + uint64_t id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, i);
> reg = env->gpr[i];
> ret = kvm_set_one_reg(cs, id, ®);
> if (ret) {
> @@ -810,8 +812,8 @@ static void kvm_riscv_read_cbomz_blksize(RISCVCPU *cpu, KVMScratchCPU *kvmcpu,
> struct kvm_one_reg reg;
> int ret;
>
> - reg.id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG,
> - cbomz_cfg->kvm_reg_id);
> + reg.id = kvm_riscv_reg_id_ulong(env, 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, ®);
> if (ret != 0) {
> @@ -832,8 +834,8 @@ 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(env, KVM_REG_RISCV_ISA_EXT,
> - multi_ext_cfg->kvm_reg_id);
> + reg.id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_ISA_EXT,
> + multi_ext_cfg->kvm_reg_id);
> reg.addr = (uint64_t)&val;
> ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
> if (ret != 0) {
> @@ -925,8 +927,8 @@ 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(&cpu->env, KVM_REG_RISCV_ISA_EXT,
> - multi_ext_cfg->kvm_reg_id);
> + reg_id = kvm_riscv_reg_id_ulong(&cpu->env, KVM_REG_RISCV_ISA_EXT,
> + multi_ext_cfg->kvm_reg_id);
> reg_search = bsearch(®_id, reglist->reg, reglist->n,
> sizeof(uint64_t), uint64_cmp);
> if (!reg_search) {
> --
> 2.41.0
>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH for-9.0 0/5] target/riscv/kvm: fix KVM reg id sizes
2023-12-08 18:38 [PATCH for-9.0 0/5] target/riscv/kvm: fix KVM reg id sizes Daniel Henrique Barboza
` (4 preceding siblings ...)
2023-12-08 18:38 ` [PATCH for-9.0 5/5] target/riscv/kvm: rename riscv_reg_id() to riscv_reg_id_ulong() Daniel Henrique Barboza
@ 2023-12-18 0:09 ` Alistair Francis
5 siblings, 0 replies; 12+ messages in thread
From: Alistair Francis @ 2023-12-18 0:09 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518,
zhiwei_liu, palmer, ajones
On Sat, Dec 9, 2023 at 4:40 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> Hi,
>
> While working in a follow-up for the Vector KVM regs, where we would
> read 'vlenb' and then all other vregs [1], Drew noticed that we're using
> kvm_riscv_reg_id() in registers that are u32 and u64.
>
> The helper is returning ulong regs for all cases, meaning that we return
> the wrong size for u32 regs when running 6 in 64 bits (case of FP_F
> regs) and u64 regs when running in 32 bits (case of FP_D and timer
> regs).
>
> It's best to do a quick bug fix round in the KVM reg IDs before adding
> more IDs, so here we are.
>
> This is marked as 9.0 because the fixes aren't critical enough to
> postpone 8.2. Michael, patches 1, 2 and 3 are good candidates for
> 8.2-stable.
>
> Alistair, in case these are accepted I'll re-send "[PATCH for-9.0 0/4]
> target/riscv: add RVV CSRs" based on these fixes to avoid conflicts in
> the tree.
>
>
> [1] this is dependent on kernel side changes that are being discussed
> here: https://lore.kernel.org/kvm/20231205174509.2238870-1-dbarboza@ventanamicro.com/
> ("[PATCH v3 0/3] RISC-V, KVM: add 'vlenb' and vector CSRs to get-reg-list")
>
>
> Daniel Henrique Barboza (5):
> target/riscv/kvm: change KVM_REG_RISCV_FP_F to u32
> target/riscv/kvm: change KVM_REG_RISCV_FP_D to u64
> target/riscv/kvm: change timer regs size to u64
> target/riscv/kvm: add RISCV_CONFIG_REG()
> target/riscv/kvm: rename riscv_reg_id() to riscv_reg_id_ulong()
Thanks!
Applied to riscv-to-apply.next
Alistair
>
> target/riscv/kvm/kvm-cpu.c | 109 ++++++++++++++++++++-----------------
> 1 file changed, 59 insertions(+), 50 deletions(-)
>
> --
> 2.41.0
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-12-18 0:11 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-08 18:38 [PATCH for-9.0 0/5] target/riscv/kvm: fix KVM reg id sizes Daniel Henrique Barboza
2023-12-08 18:38 ` [PATCH for-9.0 1/5] target/riscv/kvm: change KVM_REG_RISCV_FP_F to u32 Daniel Henrique Barboza
2023-12-15 13:08 ` Andrew Jones
2023-12-08 18:38 ` [PATCH for-9.0 2/5] target/riscv/kvm: change KVM_REG_RISCV_FP_D to u64 Daniel Henrique Barboza
2023-12-15 13:11 ` Andrew Jones
2023-12-08 18:38 ` [PATCH for-9.0 3/5] target/riscv/kvm: change timer regs size " Daniel Henrique Barboza
2023-12-15 13:14 ` Andrew Jones
2023-12-08 18:38 ` [PATCH for-9.0 4/5] target/riscv/kvm: add RISCV_CONFIG_REG() Daniel Henrique Barboza
2023-12-15 13:18 ` Andrew Jones
2023-12-08 18:38 ` [PATCH for-9.0 5/5] target/riscv/kvm: rename riscv_reg_id() to riscv_reg_id_ulong() Daniel Henrique Barboza
2023-12-15 13:19 ` Andrew Jones
2023-12-18 0:09 ` [PATCH for-9.0 0/5] target/riscv/kvm: fix KVM reg id sizes Alistair Francis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).