* [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* 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
* [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* 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
* [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* 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
* [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* 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
* [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 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