Message ID | 20230718111404.23479-2-cohuck@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | arm/kvm: use kvm_{get,set}_one_reg | expand |
Hi Connie, On 7/18/23 21:14, Cornelia Huck wrote: > We can neaten the code by switching to the kvm_set_one_reg function. > > Signed-off-by: Cornelia Huck <cohuck@redhat.com> > --- > target/arm/kvm.c | 13 +++------ > target/arm/kvm64.c | 66 +++++++++++++--------------------------------- > 2 files changed, 21 insertions(+), 58 deletions(-) > Some wrong replacements to be fixed in kvm_arch_put_fpsimd() as below. Apart from that, LGTM: Reviewed-by: Gavin Shan <gshan@redhat.com> > diff --git a/target/arm/kvm.c b/target/arm/kvm.c > index b4c7654f4980..cdbffc3c6e0d 100644 > --- a/target/arm/kvm.c > +++ b/target/arm/kvm.c > @@ -561,7 +561,6 @@ bool write_list_to_kvmstate(ARMCPU *cpu, int level) > bool ok = true; > > for (i = 0; i < cpu->cpreg_array_len; i++) { > - struct kvm_one_reg r; > uint64_t regidx = cpu->cpreg_indexes[i]; > uint32_t v32; > int ret; > @@ -570,19 +569,17 @@ bool write_list_to_kvmstate(ARMCPU *cpu, int level) > continue; > } > > - r.id = regidx; > switch (regidx & KVM_REG_SIZE_MASK) { > case KVM_REG_SIZE_U32: > v32 = cpu->cpreg_values[i]; > - r.addr = (uintptr_t)&v32; > + ret = kvm_set_one_reg(cs, regidx, &v32); > break; > case KVM_REG_SIZE_U64: > - r.addr = (uintptr_t)(cpu->cpreg_values + i); > + ret = kvm_set_one_reg(cs, regidx, cpu->cpreg_values + i); > break; > default: > g_assert_not_reached(); > } > - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r); > if (ret) { > /* We might fail for "unknown register" and also for > * "you tried to set a register which is constant with > @@ -703,17 +700,13 @@ void kvm_arm_get_virtual_time(CPUState *cs) > void kvm_arm_put_virtual_time(CPUState *cs) > { > ARMCPU *cpu = ARM_CPU(cs); > - struct kvm_one_reg reg = { > - .id = KVM_REG_ARM_TIMER_CNT, > - .addr = (uintptr_t)&cpu->kvm_vtime, > - }; > int ret; > > if (!cpu->kvm_vtime_dirty) { > return; > } > > - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); > + ret = kvm_set_one_reg(cs, KVM_REG_ARM_TIMER_CNT, &cpu->kvm_vtime); > if (ret) { > error_report("Failed to set KVM_REG_ARM_TIMER_CNT"); > abort(); > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c > index 94bbd9661fd3..b4d02dff5381 100644 > --- a/target/arm/kvm64.c > +++ b/target/arm/kvm64.c > @@ -540,14 +540,10 @@ static int kvm_arm_sve_set_vls(CPUState *cs) > { > ARMCPU *cpu = ARM_CPU(cs); > uint64_t vls[KVM_ARM64_SVE_VLS_WORDS] = { cpu->sve_vq.map }; > - struct kvm_one_reg reg = { > - .id = KVM_REG_ARM64_SVE_VLS, > - .addr = (uint64_t)&vls[0], > - }; > > assert(cpu->sve_max_vq <= KVM_ARM64_SVE_VQ_MAX); > > - return kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); > + return kvm_set_one_reg(cs, KVM_REG_ARM64_SVE_VLS, &vls[0]); > } > > #define ARM_CPU_ID_MPIDR 3, 0, 0, 0, 5 > @@ -725,19 +721,17 @@ static void kvm_inject_arm_sea(CPUState *c) > static int kvm_arch_put_fpsimd(CPUState *cs) > { > CPUARMState *env = &ARM_CPU(cs)->env; > - struct kvm_one_reg reg; > int i, ret; > > for (i = 0; i < 32; i++) { > uint64_t *q = aa64_vfp_qreg(env, i); > #if HOST_BIG_ENDIAN > uint64_t fp_val[2] = { q[1], q[0] }; > - reg.addr = (uintptr_t)fp_val; > + ret = kvm_set_one_reg(cs, AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]), > + &fp_val); ^^^^^^^ s/&fp_val/fp_val > #else > - reg.addr = (uintptr_t)q; > + ret = kvm_set_one_reg(cs, AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]), &q); ^^^ s/&q/q > #endif > - reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]); > - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); > if (ret) { > return ret; > } > @@ -758,14 +752,11 @@ static int kvm_arch_put_sve(CPUState *cs) > CPUARMState *env = &cpu->env; > uint64_t tmp[ARM_MAX_VQ * 2]; > uint64_t *r; > - struct kvm_one_reg reg; > int n, ret; > > for (n = 0; n < KVM_ARM64_SVE_NUM_ZREGS; ++n) { > r = sve_bswap64(tmp, &env->vfp.zregs[n].d[0], cpu->sve_max_vq * 2); > - reg.addr = (uintptr_t)r; > - reg.id = KVM_REG_ARM64_SVE_ZREG(n, 0); > - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); > + ret = kvm_set_one_reg(cs, KVM_REG_ARM64_SVE_ZREG(n, 0), r); > if (ret) { > return ret; > } > @@ -774,9 +765,7 @@ static int kvm_arch_put_sve(CPUState *cs) > for (n = 0; n < KVM_ARM64_SVE_NUM_PREGS; ++n) { > r = sve_bswap64(tmp, r = &env->vfp.pregs[n].p[0], > DIV_ROUND_UP(cpu->sve_max_vq * 2, 8)); > - reg.addr = (uintptr_t)r; > - reg.id = KVM_REG_ARM64_SVE_PREG(n, 0); > - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); > + ret = kvm_set_one_reg(cs, KVM_REG_ARM64_SVE_PREG(n, 0), r); > if (ret) { > return ret; > } > @@ -784,9 +773,7 @@ static int kvm_arch_put_sve(CPUState *cs) > > r = sve_bswap64(tmp, &env->vfp.pregs[FFR_PRED_NUM].p[0], > DIV_ROUND_UP(cpu->sve_max_vq * 2, 8)); > - reg.addr = (uintptr_t)r; > - reg.id = KVM_REG_ARM64_SVE_FFR(0); > - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); > + ret = kvm_set_one_reg(cs, KVM_REG_ARM64_SVE_FFR(0), r); > if (ret) { > return ret; > } > @@ -796,7 +783,6 @@ static int kvm_arch_put_sve(CPUState *cs) > > int kvm_arch_put_registers(CPUState *cs, int level) > { > - struct kvm_one_reg reg; > uint64_t val; > uint32_t fpr; > int i, ret; > @@ -813,9 +799,8 @@ int kvm_arch_put_registers(CPUState *cs, int level) > } > > for (i = 0; i < 31; i++) { > - reg.id = AARCH64_CORE_REG(regs.regs[i]); > - reg.addr = (uintptr_t) &env->xregs[i]; > - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); > + ret = kvm_set_one_reg(cs, AARCH64_CORE_REG(regs.regs[i]), > + &env->xregs[i]); > if (ret) { > return ret; > } > @@ -826,16 +811,12 @@ int kvm_arch_put_registers(CPUState *cs, int level) > */ > aarch64_save_sp(env, 1); > > - reg.id = AARCH64_CORE_REG(regs.sp); > - reg.addr = (uintptr_t) &env->sp_el[0]; > - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); > + ret = kvm_set_one_reg(cs, AARCH64_CORE_REG(regs.sp), &env->sp_el[0]); > if (ret) { > return ret; > } > > - reg.id = AARCH64_CORE_REG(sp_el1); > - reg.addr = (uintptr_t) &env->sp_el[1]; > - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); > + ret = kvm_set_one_reg(cs, AARCH64_CORE_REG(sp_el1), &env->sp_el[1]); > if (ret) { > return ret; > } > @@ -846,23 +827,17 @@ int kvm_arch_put_registers(CPUState *cs, int level) > } else { > val = cpsr_read(env); > } > - reg.id = AARCH64_CORE_REG(regs.pstate); > - reg.addr = (uintptr_t) &val; > - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); > + ret = kvm_set_one_reg(cs, AARCH64_CORE_REG(regs.pstate), &val); > if (ret) { > return ret; > } > > - reg.id = AARCH64_CORE_REG(regs.pc); > - reg.addr = (uintptr_t) &env->pc; > - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); > + ret = kvm_set_one_reg(cs, AARCH64_CORE_REG(regs.pc), &env->pc); > if (ret) { > return ret; > } > > - reg.id = AARCH64_CORE_REG(elr_el1); > - reg.addr = (uintptr_t) &env->elr_el[1]; > - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); > + ret = kvm_set_one_reg(cs, AARCH64_CORE_REG(elr_el1), &env->elr_el[1]); > if (ret) { > return ret; > } > @@ -881,9 +856,8 @@ int kvm_arch_put_registers(CPUState *cs, int level) > > /* KVM 0-4 map to QEMU banks 1-5 */ > for (i = 0; i < KVM_NR_SPSR; i++) { > - reg.id = AARCH64_CORE_REG(spsr[i]); > - reg.addr = (uintptr_t) &env->banked_spsr[i + 1]; > - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); > + ret = kvm_set_one_reg(cs, AARCH64_CORE_REG(spsr[i]), > + &env->banked_spsr[i + 1]); > if (ret) { > return ret; > } > @@ -898,18 +872,14 @@ int kvm_arch_put_registers(CPUState *cs, int level) > return ret; > } > > - reg.addr = (uintptr_t)(&fpr); > fpr = vfp_get_fpsr(env); > - reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpsr); > - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); > + ret = kvm_set_one_reg(cs, AARCH64_SIMD_CTRL_REG(fp_regs.fpsr), &fpr); > if (ret) { > return ret; > } > > - reg.addr = (uintptr_t)(&fpr); > fpr = vfp_get_fpcr(env); > - reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpcr); > - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); > + ret = kvm_set_one_reg(cs, AARCH64_SIMD_CTRL_REG(fp_regs.fpcr), &fpr); > if (ret) { > return ret; > } Thanks, Gavin
On Mon, Jul 24 2023, Gavin Shan <gshan@redhat.com> wrote: > Hi Connie, > > On 7/18/23 21:14, Cornelia Huck wrote: >> We can neaten the code by switching to the kvm_set_one_reg function. >> >> Signed-off-by: Cornelia Huck <cohuck@redhat.com> >> --- >> target/arm/kvm.c | 13 +++------ >> target/arm/kvm64.c | 66 +++++++++++++--------------------------------- >> 2 files changed, 21 insertions(+), 58 deletions(-) >> > > Some wrong replacements to be fixed in kvm_arch_put_fpsimd() as below. > Apart from that, LGTM: > > Reviewed-by: Gavin Shan <gshan@redhat.com> > @@ -725,19 +721,17 @@ static void kvm_inject_arm_sea(CPUState *c) >> static int kvm_arch_put_fpsimd(CPUState *cs) >> { >> CPUARMState *env = &ARM_CPU(cs)->env; >> - struct kvm_one_reg reg; >> int i, ret; >> >> for (i = 0; i < 32; i++) { >> uint64_t *q = aa64_vfp_qreg(env, i); >> #if HOST_BIG_ENDIAN >> uint64_t fp_val[2] = { q[1], q[0] }; >> - reg.addr = (uintptr_t)fp_val; >> + ret = kvm_set_one_reg(cs, AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]), >> + &fp_val); > ^^^^^^^ > s/&fp_val/fp_val >> #else >> - reg.addr = (uintptr_t)q; >> + ret = kvm_set_one_reg(cs, AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]), &q); > ^^^ > s/&q/q > >> #endif Whoops, I thought I had double-checked these...
diff --git a/target/arm/kvm.c b/target/arm/kvm.c index b4c7654f4980..cdbffc3c6e0d 100644 --- a/target/arm/kvm.c +++ b/target/arm/kvm.c @@ -561,7 +561,6 @@ bool write_list_to_kvmstate(ARMCPU *cpu, int level) bool ok = true; for (i = 0; i < cpu->cpreg_array_len; i++) { - struct kvm_one_reg r; uint64_t regidx = cpu->cpreg_indexes[i]; uint32_t v32; int ret; @@ -570,19 +569,17 @@ bool write_list_to_kvmstate(ARMCPU *cpu, int level) continue; } - r.id = regidx; switch (regidx & KVM_REG_SIZE_MASK) { case KVM_REG_SIZE_U32: v32 = cpu->cpreg_values[i]; - r.addr = (uintptr_t)&v32; + ret = kvm_set_one_reg(cs, regidx, &v32); break; case KVM_REG_SIZE_U64: - r.addr = (uintptr_t)(cpu->cpreg_values + i); + ret = kvm_set_one_reg(cs, regidx, cpu->cpreg_values + i); break; default: g_assert_not_reached(); } - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r); if (ret) { /* We might fail for "unknown register" and also for * "you tried to set a register which is constant with @@ -703,17 +700,13 @@ void kvm_arm_get_virtual_time(CPUState *cs) void kvm_arm_put_virtual_time(CPUState *cs) { ARMCPU *cpu = ARM_CPU(cs); - struct kvm_one_reg reg = { - .id = KVM_REG_ARM_TIMER_CNT, - .addr = (uintptr_t)&cpu->kvm_vtime, - }; int ret; if (!cpu->kvm_vtime_dirty) { return; } - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); + ret = kvm_set_one_reg(cs, KVM_REG_ARM_TIMER_CNT, &cpu->kvm_vtime); if (ret) { error_report("Failed to set KVM_REG_ARM_TIMER_CNT"); abort(); diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c index 94bbd9661fd3..b4d02dff5381 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -540,14 +540,10 @@ static int kvm_arm_sve_set_vls(CPUState *cs) { ARMCPU *cpu = ARM_CPU(cs); uint64_t vls[KVM_ARM64_SVE_VLS_WORDS] = { cpu->sve_vq.map }; - struct kvm_one_reg reg = { - .id = KVM_REG_ARM64_SVE_VLS, - .addr = (uint64_t)&vls[0], - }; assert(cpu->sve_max_vq <= KVM_ARM64_SVE_VQ_MAX); - return kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); + return kvm_set_one_reg(cs, KVM_REG_ARM64_SVE_VLS, &vls[0]); } #define ARM_CPU_ID_MPIDR 3, 0, 0, 0, 5 @@ -725,19 +721,17 @@ static void kvm_inject_arm_sea(CPUState *c) static int kvm_arch_put_fpsimd(CPUState *cs) { CPUARMState *env = &ARM_CPU(cs)->env; - struct kvm_one_reg reg; int i, ret; for (i = 0; i < 32; i++) { uint64_t *q = aa64_vfp_qreg(env, i); #if HOST_BIG_ENDIAN uint64_t fp_val[2] = { q[1], q[0] }; - reg.addr = (uintptr_t)fp_val; + ret = kvm_set_one_reg(cs, AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]), + &fp_val); #else - reg.addr = (uintptr_t)q; + ret = kvm_set_one_reg(cs, AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]), &q); #endif - reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]); - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); if (ret) { return ret; } @@ -758,14 +752,11 @@ static int kvm_arch_put_sve(CPUState *cs) CPUARMState *env = &cpu->env; uint64_t tmp[ARM_MAX_VQ * 2]; uint64_t *r; - struct kvm_one_reg reg; int n, ret; for (n = 0; n < KVM_ARM64_SVE_NUM_ZREGS; ++n) { r = sve_bswap64(tmp, &env->vfp.zregs[n].d[0], cpu->sve_max_vq * 2); - reg.addr = (uintptr_t)r; - reg.id = KVM_REG_ARM64_SVE_ZREG(n, 0); - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); + ret = kvm_set_one_reg(cs, KVM_REG_ARM64_SVE_ZREG(n, 0), r); if (ret) { return ret; } @@ -774,9 +765,7 @@ static int kvm_arch_put_sve(CPUState *cs) for (n = 0; n < KVM_ARM64_SVE_NUM_PREGS; ++n) { r = sve_bswap64(tmp, r = &env->vfp.pregs[n].p[0], DIV_ROUND_UP(cpu->sve_max_vq * 2, 8)); - reg.addr = (uintptr_t)r; - reg.id = KVM_REG_ARM64_SVE_PREG(n, 0); - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); + ret = kvm_set_one_reg(cs, KVM_REG_ARM64_SVE_PREG(n, 0), r); if (ret) { return ret; } @@ -784,9 +773,7 @@ static int kvm_arch_put_sve(CPUState *cs) r = sve_bswap64(tmp, &env->vfp.pregs[FFR_PRED_NUM].p[0], DIV_ROUND_UP(cpu->sve_max_vq * 2, 8)); - reg.addr = (uintptr_t)r; - reg.id = KVM_REG_ARM64_SVE_FFR(0); - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); + ret = kvm_set_one_reg(cs, KVM_REG_ARM64_SVE_FFR(0), r); if (ret) { return ret; } @@ -796,7 +783,6 @@ static int kvm_arch_put_sve(CPUState *cs) int kvm_arch_put_registers(CPUState *cs, int level) { - struct kvm_one_reg reg; uint64_t val; uint32_t fpr; int i, ret; @@ -813,9 +799,8 @@ int kvm_arch_put_registers(CPUState *cs, int level) } for (i = 0; i < 31; i++) { - reg.id = AARCH64_CORE_REG(regs.regs[i]); - reg.addr = (uintptr_t) &env->xregs[i]; - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); + ret = kvm_set_one_reg(cs, AARCH64_CORE_REG(regs.regs[i]), + &env->xregs[i]); if (ret) { return ret; } @@ -826,16 +811,12 @@ int kvm_arch_put_registers(CPUState *cs, int level) */ aarch64_save_sp(env, 1); - reg.id = AARCH64_CORE_REG(regs.sp); - reg.addr = (uintptr_t) &env->sp_el[0]; - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); + ret = kvm_set_one_reg(cs, AARCH64_CORE_REG(regs.sp), &env->sp_el[0]); if (ret) { return ret; } - reg.id = AARCH64_CORE_REG(sp_el1); - reg.addr = (uintptr_t) &env->sp_el[1]; - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); + ret = kvm_set_one_reg(cs, AARCH64_CORE_REG(sp_el1), &env->sp_el[1]); if (ret) { return ret; } @@ -846,23 +827,17 @@ int kvm_arch_put_registers(CPUState *cs, int level) } else { val = cpsr_read(env); } - reg.id = AARCH64_CORE_REG(regs.pstate); - reg.addr = (uintptr_t) &val; - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); + ret = kvm_set_one_reg(cs, AARCH64_CORE_REG(regs.pstate), &val); if (ret) { return ret; } - reg.id = AARCH64_CORE_REG(regs.pc); - reg.addr = (uintptr_t) &env->pc; - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); + ret = kvm_set_one_reg(cs, AARCH64_CORE_REG(regs.pc), &env->pc); if (ret) { return ret; } - reg.id = AARCH64_CORE_REG(elr_el1); - reg.addr = (uintptr_t) &env->elr_el[1]; - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); + ret = kvm_set_one_reg(cs, AARCH64_CORE_REG(elr_el1), &env->elr_el[1]); if (ret) { return ret; } @@ -881,9 +856,8 @@ int kvm_arch_put_registers(CPUState *cs, int level) /* KVM 0-4 map to QEMU banks 1-5 */ for (i = 0; i < KVM_NR_SPSR; i++) { - reg.id = AARCH64_CORE_REG(spsr[i]); - reg.addr = (uintptr_t) &env->banked_spsr[i + 1]; - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); + ret = kvm_set_one_reg(cs, AARCH64_CORE_REG(spsr[i]), + &env->banked_spsr[i + 1]); if (ret) { return ret; } @@ -898,18 +872,14 @@ int kvm_arch_put_registers(CPUState *cs, int level) return ret; } - reg.addr = (uintptr_t)(&fpr); fpr = vfp_get_fpsr(env); - reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpsr); - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); + ret = kvm_set_one_reg(cs, AARCH64_SIMD_CTRL_REG(fp_regs.fpsr), &fpr); if (ret) { return ret; } - reg.addr = (uintptr_t)(&fpr); fpr = vfp_get_fpcr(env); - reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpcr); - ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); + ret = kvm_set_one_reg(cs, AARCH64_SIMD_CTRL_REG(fp_regs.fpcr), &fpr); if (ret) { return ret; }
We can neaten the code by switching to the kvm_set_one_reg function. Signed-off-by: Cornelia Huck <cohuck@redhat.com> --- target/arm/kvm.c | 13 +++------ target/arm/kvm64.c | 66 +++++++++++++--------------------------------- 2 files changed, 21 insertions(+), 58 deletions(-)