Message ID | 20231208183835.2411523-2-dbarboza@ventanamicro.com |
---|---|
State | New |
Headers | show |
Series | target/riscv/kvm: fix KVM reg id sizes | expand |
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
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; }
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(-)