* [PATCH v2 0/3] arm/kvm: use kvm_{get,set}_one_reg
@ 2023-10-10 14:24 Cornelia Huck
2023-10-10 14:24 ` [PATCH v2 1/3] arm/kvm: convert to kvm_set_one_reg Cornelia Huck
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Cornelia Huck @ 2023-10-10 14:24 UTC (permalink / raw)
To: Peter Maydell, Paolo Bonzini
Cc: Gavin Shan, qemu-arm, qemu-devel, kvm, Cornelia Huck
I sent this cleanup first... in mid-July (ugh). But better late than never, I guess.
From v1:
- fix buglets (thanks Gavin)
- add patch 3 on top
The kvm_{get,set}_one_reg functions have been around for a very long
time, and using them instead of open-coding the ioctl invocations
saves lines of code, and gives us a tracepoint as well. They cannot
be used by invocations of the ioctl not acting on a CPUState, but
that still leaves a lot of conversions in the target/arm code.
target/mips and target/ppc also have some potential for conversions,
but as I cannot test either (and they are both in 'Odd fixes' anyway),
I left them alone.
Survives some testing on a Mt. Snow.
Cornelia Huck (3):
arm/kvm: convert to kvm_set_one_reg
arm/kvm: convert to kvm_get_one_reg
arm/kvm: convert to read_sys_reg64
target/arm/kvm.c | 28 +++-------
target/arm/kvm64.c | 129 ++++++++++++---------------------------------
2 files changed, 40 insertions(+), 117 deletions(-)
--
2.41.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/3] arm/kvm: convert to kvm_set_one_reg
2023-10-10 14:24 [PATCH v2 0/3] arm/kvm: use kvm_{get,set}_one_reg Cornelia Huck
@ 2023-10-10 14:24 ` Cornelia Huck
2023-10-11 2:44 ` Philippe Mathieu-Daudé
2023-10-10 14:24 ` [PATCH v2 2/3] arm/kvm: convert to kvm_get_one_reg Cornelia Huck
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Cornelia Huck @ 2023-10-10 14:24 UTC (permalink / raw)
To: Peter Maydell, Paolo Bonzini
Cc: Gavin Shan, qemu-arm, qemu-devel, kvm, Cornelia Huck
We can neaten the code by switching to the kvm_set_one_reg function.
Reviewed-by: Gavin Shan <gshan@redhat.com>
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(-)
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index b66b936a9586..1a8084c4601c 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -589,7 +589,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;
@@ -598,19 +597,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
@@ -731,17 +728,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 5e95c496bb9f..047b269a7918 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
@@ -726,19 +722,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;
}
@@ -759,14 +753,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;
}
@@ -775,9 +766,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;
}
@@ -785,9 +774,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;
}
@@ -797,7 +784,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;
@@ -814,9 +800,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;
}
@@ -827,16 +812,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;
}
@@ -847,23 +828,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;
}
@@ -882,9 +857,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;
}
@@ -899,18 +873,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;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/3] arm/kvm: convert to kvm_get_one_reg
2023-10-10 14:24 [PATCH v2 0/3] arm/kvm: use kvm_{get,set}_one_reg Cornelia Huck
2023-10-10 14:24 ` [PATCH v2 1/3] arm/kvm: convert to kvm_set_one_reg Cornelia Huck
@ 2023-10-10 14:24 ` Cornelia Huck
2023-10-11 2:44 ` Philippe Mathieu-Daudé
2023-10-10 14:24 ` [PATCH v2 3/3] arm/kvm: convert to read_sys_reg64 Cornelia Huck
2023-10-17 12:15 ` [PATCH v2 0/3] arm/kvm: use kvm_{get,set}_one_reg Peter Maydell
3 siblings, 1 reply; 10+ messages in thread
From: Cornelia Huck @ 2023-10-10 14:24 UTC (permalink / raw)
To: Peter Maydell, Paolo Bonzini
Cc: Gavin Shan, qemu-arm, qemu-devel, kvm, Cornelia Huck
We can neaten the code by switching the callers that work on a
CPUstate to the kvm_get_one_reg function.
Reviewed-by: Gavin Shan <gshan@redhat.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
target/arm/kvm.c | 15 +++---------
target/arm/kvm64.c | 57 ++++++++++++----------------------------------
2 files changed, 18 insertions(+), 54 deletions(-)
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 1a8084c4601c..7903e2ddde1b 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -553,24 +553,19 @@ bool write_kvmstate_to_list(ARMCPU *cpu)
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;
- r.id = regidx;
-
switch (regidx & KVM_REG_SIZE_MASK) {
case KVM_REG_SIZE_U32:
- r.addr = (uintptr_t)&v32;
- ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
+ ret = kvm_get_one_reg(cs, regidx, &v32);
if (!ret) {
cpu->cpreg_values[i] = v32;
}
break;
case KVM_REG_SIZE_U64:
- r.addr = (uintptr_t)(cpu->cpreg_values + i);
- ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
+ ret = kvm_get_one_reg(cs, regidx, cpu->cpreg_values + i);
break;
default:
g_assert_not_reached();
@@ -706,17 +701,13 @@ int kvm_arm_sync_mpstate_to_qemu(ARMCPU *cpu)
void kvm_arm_get_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_GET_ONE_REG, ®);
+ ret = kvm_get_one_reg(cs, KVM_REG_ARM_TIMER_CNT, &cpu->kvm_vtime);
if (ret) {
error_report("Failed to get KVM_REG_ARM_TIMER_CNT");
abort();
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 047b269a7918..558c0b88dd69 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -909,14 +909,11 @@ int kvm_arch_put_registers(CPUState *cs, int level)
static int kvm_arch_get_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);
- reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
- reg.addr = (uintptr_t)q;
- ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
+ ret = kvm_get_one_reg(cs, AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]), q);
if (ret) {
return ret;
} else {
@@ -940,15 +937,12 @@ static int kvm_arch_get_sve(CPUState *cs)
{
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = &cpu->env;
- struct kvm_one_reg reg;
uint64_t *r;
int n, ret;
for (n = 0; n < KVM_ARM64_SVE_NUM_ZREGS; ++n) {
r = &env->vfp.zregs[n].d[0];
- reg.addr = (uintptr_t)r;
- reg.id = KVM_REG_ARM64_SVE_ZREG(n, 0);
- ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
+ ret = kvm_get_one_reg(cs, KVM_REG_ARM64_SVE_ZREG(n, 0), r);
if (ret) {
return ret;
}
@@ -957,9 +951,7 @@ static int kvm_arch_get_sve(CPUState *cs)
for (n = 0; n < KVM_ARM64_SVE_NUM_PREGS; ++n) {
r = &env->vfp.pregs[n].p[0];
- reg.addr = (uintptr_t)r;
- reg.id = KVM_REG_ARM64_SVE_PREG(n, 0);
- ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
+ ret = kvm_get_one_reg(cs, KVM_REG_ARM64_SVE_PREG(n, 0), r);
if (ret) {
return ret;
}
@@ -967,9 +959,7 @@ static int kvm_arch_get_sve(CPUState *cs)
}
r = &env->vfp.pregs[FFR_PRED_NUM].p[0];
- reg.addr = (uintptr_t)r;
- reg.id = KVM_REG_ARM64_SVE_FFR(0);
- ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
+ ret = kvm_get_one_reg(cs, KVM_REG_ARM64_SVE_FFR(0), r);
if (ret) {
return ret;
}
@@ -980,7 +970,6 @@ static int kvm_arch_get_sve(CPUState *cs)
int kvm_arch_get_registers(CPUState *cs)
{
- struct kvm_one_reg reg;
uint64_t val;
unsigned int el;
uint32_t fpr;
@@ -990,31 +979,24 @@ int kvm_arch_get_registers(CPUState *cs)
CPUARMState *env = &cpu->env;
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_GET_ONE_REG, ®);
+ ret = kvm_get_one_reg(cs, AARCH64_CORE_REG(regs.regs[i]),
+ &env->xregs[i]);
if (ret) {
return ret;
}
}
- reg.id = AARCH64_CORE_REG(regs.sp);
- reg.addr = (uintptr_t) &env->sp_el[0];
- ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
+ ret = kvm_get_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_GET_ONE_REG, ®);
+ ret = kvm_get_one_reg(cs, AARCH64_CORE_REG(sp_el1), &env->sp_el[1]);
if (ret) {
return ret;
}
- reg.id = AARCH64_CORE_REG(regs.pstate);
- reg.addr = (uintptr_t) &val;
- ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
+ ret = kvm_get_one_reg(cs, AARCH64_CORE_REG(regs.pstate), &val);
if (ret) {
return ret;
}
@@ -1031,9 +1013,7 @@ int kvm_arch_get_registers(CPUState *cs)
*/
aarch64_restore_sp(env, 1);
- reg.id = AARCH64_CORE_REG(regs.pc);
- reg.addr = (uintptr_t) &env->pc;
- ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
+ ret = kvm_get_one_reg(cs, AARCH64_CORE_REG(regs.pc), &env->pc);
if (ret) {
return ret;
}
@@ -1047,9 +1027,7 @@ int kvm_arch_get_registers(CPUState *cs)
aarch64_sync_64_to_32(env);
}
- reg.id = AARCH64_CORE_REG(elr_el1);
- reg.addr = (uintptr_t) &env->elr_el[1];
- ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
+ ret = kvm_get_one_reg(cs, AARCH64_CORE_REG(elr_el1), &env->elr_el[1]);
if (ret) {
return ret;
}
@@ -1059,9 +1037,8 @@ int kvm_arch_get_registers(CPUState *cs)
* KVM SPSRs 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_GET_ONE_REG, ®);
+ ret = kvm_get_one_reg(cs, AARCH64_CORE_REG(spsr[i]),
+ &env->banked_spsr[i + 1]);
if (ret) {
return ret;
}
@@ -1082,17 +1059,13 @@ int kvm_arch_get_registers(CPUState *cs)
return ret;
}
- reg.addr = (uintptr_t)(&fpr);
- reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpsr);
- ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
+ ret = kvm_get_one_reg(cs, AARCH64_SIMD_CTRL_REG(fp_regs.fpsr), &fpr);
if (ret) {
return ret;
}
vfp_set_fpsr(env, fpr);
- reg.addr = (uintptr_t)(&fpr);
- reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpcr);
- ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
+ ret = kvm_get_one_reg(cs, AARCH64_SIMD_CTRL_REG(fp_regs.fpcr), &fpr);
if (ret) {
return ret;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 3/3] arm/kvm: convert to read_sys_reg64
2023-10-10 14:24 [PATCH v2 0/3] arm/kvm: use kvm_{get,set}_one_reg Cornelia Huck
2023-10-10 14:24 ` [PATCH v2 1/3] arm/kvm: convert to kvm_set_one_reg Cornelia Huck
2023-10-10 14:24 ` [PATCH v2 2/3] arm/kvm: convert to kvm_get_one_reg Cornelia Huck
@ 2023-10-10 14:24 ` Cornelia Huck
2023-10-11 0:02 ` Gavin Shan
2023-10-17 12:09 ` Peter Maydell
2023-10-17 12:15 ` [PATCH v2 0/3] arm/kvm: use kvm_{get,set}_one_reg Peter Maydell
3 siblings, 2 replies; 10+ messages in thread
From: Cornelia Huck @ 2023-10-10 14:24 UTC (permalink / raw)
To: Peter Maydell, Paolo Bonzini
Cc: Gavin Shan, qemu-arm, qemu-devel, kvm, Cornelia Huck
We can use read_sys_reg64 to get the SVE_VLS register instead of
calling GET_ONE_REG directly.
Suggested-by: Gavin Shan <gshan@redhat.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
target/arm/kvm64.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 558c0b88dd69..d40c89a84752 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -500,10 +500,6 @@ uint32_t kvm_arm_sve_get_vls(CPUState *cs)
.target = -1,
.features[0] = (1 << KVM_ARM_VCPU_SVE),
};
- struct kvm_one_reg reg = {
- .id = KVM_REG_ARM64_SVE_VLS,
- .addr = (uint64_t)&vls[0],
- };
int fdarray[3], ret;
probed = true;
@@ -512,7 +508,7 @@ uint32_t kvm_arm_sve_get_vls(CPUState *cs)
error_report("failed to create scratch VCPU with SVE enabled");
abort();
}
- ret = ioctl(fdarray[2], KVM_GET_ONE_REG, ®);
+ ret = read_sys_reg64(fdarray[2], &vls[0], KVM_REG_ARM64_SVE_VLS);
kvm_arm_destroy_scratch_host_vcpu(fdarray);
if (ret) {
error_report("failed to get KVM_REG_ARM64_SVE_VLS: %s",
--
2.41.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] arm/kvm: convert to read_sys_reg64
2023-10-10 14:24 ` [PATCH v2 3/3] arm/kvm: convert to read_sys_reg64 Cornelia Huck
@ 2023-10-11 0:02 ` Gavin Shan
2023-10-17 12:09 ` Peter Maydell
1 sibling, 0 replies; 10+ messages in thread
From: Gavin Shan @ 2023-10-11 0:02 UTC (permalink / raw)
To: Cornelia Huck, Peter Maydell, Paolo Bonzini; +Cc: qemu-arm, qemu-devel, kvm
On 10/11/23 00:24, Cornelia Huck wrote:
> We can use read_sys_reg64 to get the SVE_VLS register instead of
> calling GET_ONE_REG directly.
>
> Suggested-by: Gavin Shan <gshan@redhat.com>
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
> target/arm/kvm64.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
Reviewed-by: Gavin Shan <gshan@redhat.com>
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index 558c0b88dd69..d40c89a84752 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -500,10 +500,6 @@ uint32_t kvm_arm_sve_get_vls(CPUState *cs)
> .target = -1,
> .features[0] = (1 << KVM_ARM_VCPU_SVE),
> };
> - struct kvm_one_reg reg = {
> - .id = KVM_REG_ARM64_SVE_VLS,
> - .addr = (uint64_t)&vls[0],
> - };
> int fdarray[3], ret;
>
> probed = true;
> @@ -512,7 +508,7 @@ uint32_t kvm_arm_sve_get_vls(CPUState *cs)
> error_report("failed to create scratch VCPU with SVE enabled");
> abort();
> }
> - ret = ioctl(fdarray[2], KVM_GET_ONE_REG, ®);
> + ret = read_sys_reg64(fdarray[2], &vls[0], KVM_REG_ARM64_SVE_VLS);
> kvm_arm_destroy_scratch_host_vcpu(fdarray);
> if (ret) {
> error_report("failed to get KVM_REG_ARM64_SVE_VLS: %s",
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] arm/kvm: convert to kvm_set_one_reg
2023-10-10 14:24 ` [PATCH v2 1/3] arm/kvm: convert to kvm_set_one_reg Cornelia Huck
@ 2023-10-11 2:44 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-11 2:44 UTC (permalink / raw)
To: Cornelia Huck, Peter Maydell, Paolo Bonzini
Cc: Gavin Shan, qemu-arm, qemu-devel, kvm
On 10/10/23 16:24, Cornelia Huck wrote:
> We can neaten the code by switching to the kvm_set_one_reg function.
>
> Reviewed-by: Gavin Shan <gshan@redhat.com>
> 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(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/3] arm/kvm: convert to kvm_get_one_reg
2023-10-10 14:24 ` [PATCH v2 2/3] arm/kvm: convert to kvm_get_one_reg Cornelia Huck
@ 2023-10-11 2:44 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-11 2:44 UTC (permalink / raw)
To: Cornelia Huck, Peter Maydell, Paolo Bonzini
Cc: Gavin Shan, qemu-arm, qemu-devel, kvm
On 10/10/23 16:24, Cornelia Huck wrote:
> We can neaten the code by switching the callers that work on a
> CPUstate to the kvm_get_one_reg function.
>
> Reviewed-by: Gavin Shan <gshan@redhat.com>
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
> target/arm/kvm.c | 15 +++---------
> target/arm/kvm64.c | 57 ++++++++++++----------------------------------
> 2 files changed, 18 insertions(+), 54 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] arm/kvm: convert to read_sys_reg64
2023-10-10 14:24 ` [PATCH v2 3/3] arm/kvm: convert to read_sys_reg64 Cornelia Huck
2023-10-11 0:02 ` Gavin Shan
@ 2023-10-17 12:09 ` Peter Maydell
2023-10-17 15:28 ` Cornelia Huck
1 sibling, 1 reply; 10+ messages in thread
From: Peter Maydell @ 2023-10-17 12:09 UTC (permalink / raw)
To: Cornelia Huck; +Cc: Paolo Bonzini, Gavin Shan, qemu-arm, qemu-devel, kvm
On Tue, 10 Oct 2023 at 15:25, Cornelia Huck <cohuck@redhat.com> wrote:
>
> We can use read_sys_reg64 to get the SVE_VLS register instead of
> calling GET_ONE_REG directly.
>
> Suggested-by: Gavin Shan <gshan@redhat.com>
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
> target/arm/kvm64.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index 558c0b88dd69..d40c89a84752 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -500,10 +500,6 @@ uint32_t kvm_arm_sve_get_vls(CPUState *cs)
> .target = -1,
> .features[0] = (1 << KVM_ARM_VCPU_SVE),
> };
> - struct kvm_one_reg reg = {
> - .id = KVM_REG_ARM64_SVE_VLS,
> - .addr = (uint64_t)&vls[0],
> - };
> int fdarray[3], ret;
>
> probed = true;
> @@ -512,7 +508,7 @@ uint32_t kvm_arm_sve_get_vls(CPUState *cs)
> error_report("failed to create scratch VCPU with SVE enabled");
> abort();
> }
> - ret = ioctl(fdarray[2], KVM_GET_ONE_REG, ®);
> + ret = read_sys_reg64(fdarray[2], &vls[0], KVM_REG_ARM64_SVE_VLS);
> kvm_arm_destroy_scratch_host_vcpu(fdarray);
> if (ret) {
> error_report("failed to get KVM_REG_ARM64_SVE_VLS: %s",
read_sys_reg64() asserts that the register you're trying to
read is 64 bits, but KVM_REG_ARM64_SVE_VLS is not, it's 512 bits:
#define KVM_REG_ARM64_SVE_VLS (KVM_REG_ARM64 | KVM_REG_ARM64_SVE | \
KVM_REG_SIZE_U512 | 0xffff)
So this change would trip the assert on a host where SVE
is supported and enabled.
thanks
-- PMM
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 0/3] arm/kvm: use kvm_{get,set}_one_reg
2023-10-10 14:24 [PATCH v2 0/3] arm/kvm: use kvm_{get,set}_one_reg Cornelia Huck
` (2 preceding siblings ...)
2023-10-10 14:24 ` [PATCH v2 3/3] arm/kvm: convert to read_sys_reg64 Cornelia Huck
@ 2023-10-17 12:15 ` Peter Maydell
3 siblings, 0 replies; 10+ messages in thread
From: Peter Maydell @ 2023-10-17 12:15 UTC (permalink / raw)
To: Cornelia Huck; +Cc: Paolo Bonzini, Gavin Shan, qemu-arm, qemu-devel, kvm
On Tue, 10 Oct 2023 at 15:25, Cornelia Huck <cohuck@redhat.com> wrote:
>
> I sent this cleanup first... in mid-July (ugh). But better late than never, I guess.
>
> From v1:
> - fix buglets (thanks Gavin)
> - add patch 3 on top
>
> The kvm_{get,set}_one_reg functions have been around for a very long
> time, and using them instead of open-coding the ioctl invocations
> saves lines of code, and gives us a tracepoint as well. They cannot
> be used by invocations of the ioctl not acting on a CPUState, but
> that still leaves a lot of conversions in the target/arm code.
I've applied patches 1 and 2 to target-arm.next.
thanks
-- PMM
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] arm/kvm: convert to read_sys_reg64
2023-10-17 12:09 ` Peter Maydell
@ 2023-10-17 15:28 ` Cornelia Huck
0 siblings, 0 replies; 10+ messages in thread
From: Cornelia Huck @ 2023-10-17 15:28 UTC (permalink / raw)
To: Peter Maydell; +Cc: Paolo Bonzini, Gavin Shan, qemu-arm, qemu-devel, kvm
On Tue, Oct 17 2023, Peter Maydell <peter.maydell@linaro.org> wrote:
> On Tue, 10 Oct 2023 at 15:25, Cornelia Huck <cohuck@redhat.com> wrote:
>>
>> We can use read_sys_reg64 to get the SVE_VLS register instead of
>> calling GET_ONE_REG directly.
>>
>> Suggested-by: Gavin Shan <gshan@redhat.com>
>> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
>> ---
>> target/arm/kvm64.c | 6 +-----
>> 1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
>> index 558c0b88dd69..d40c89a84752 100644
>> --- a/target/arm/kvm64.c
>> +++ b/target/arm/kvm64.c
>> @@ -500,10 +500,6 @@ uint32_t kvm_arm_sve_get_vls(CPUState *cs)
>> .target = -1,
>> .features[0] = (1 << KVM_ARM_VCPU_SVE),
>> };
>> - struct kvm_one_reg reg = {
>> - .id = KVM_REG_ARM64_SVE_VLS,
>> - .addr = (uint64_t)&vls[0],
>> - };
>> int fdarray[3], ret;
>>
>> probed = true;
>> @@ -512,7 +508,7 @@ uint32_t kvm_arm_sve_get_vls(CPUState *cs)
>> error_report("failed to create scratch VCPU with SVE enabled");
>> abort();
>> }
>> - ret = ioctl(fdarray[2], KVM_GET_ONE_REG, ®);
>> + ret = read_sys_reg64(fdarray[2], &vls[0], KVM_REG_ARM64_SVE_VLS);
>> kvm_arm_destroy_scratch_host_vcpu(fdarray);
>> if (ret) {
>> error_report("failed to get KVM_REG_ARM64_SVE_VLS: %s",
>
> read_sys_reg64() asserts that the register you're trying to
> read is 64 bits, but KVM_REG_ARM64_SVE_VLS is not, it's 512 bits:
>
> #define KVM_REG_ARM64_SVE_VLS (KVM_REG_ARM64 | KVM_REG_ARM64_SVE | \
> KVM_REG_SIZE_U512 | 0xffff)
>
> So this change would trip the assert on a host where SVE
> is supported and enabled.
Whoops, it seems that I misread this. (And my test environment didn't
have that enabled...)
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-10-17 15:28 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-10 14:24 [PATCH v2 0/3] arm/kvm: use kvm_{get,set}_one_reg Cornelia Huck
2023-10-10 14:24 ` [PATCH v2 1/3] arm/kvm: convert to kvm_set_one_reg Cornelia Huck
2023-10-11 2:44 ` Philippe Mathieu-Daudé
2023-10-10 14:24 ` [PATCH v2 2/3] arm/kvm: convert to kvm_get_one_reg Cornelia Huck
2023-10-11 2:44 ` Philippe Mathieu-Daudé
2023-10-10 14:24 ` [PATCH v2 3/3] arm/kvm: convert to read_sys_reg64 Cornelia Huck
2023-10-11 0:02 ` Gavin Shan
2023-10-17 12:09 ` Peter Maydell
2023-10-17 15:28 ` Cornelia Huck
2023-10-17 12:15 ` [PATCH v2 0/3] arm/kvm: use kvm_{get,set}_one_reg Peter Maydell
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).