| Message ID | 20200129200312.3200-3-frankja@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: s390: Add new reset vcpu API | expand |
On 29/01/2020 21.03, Janosch Frank wrote: > Add library access to more registers. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > .../testing/selftests/kvm/include/kvm_util.h | 6 +++ > tools/testing/selftests/kvm/lib/kvm_util.c | 48 +++++++++++++++++++ > 2 files changed, 54 insertions(+) > > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h > index 29cccaf96baf..ae0d14c2540a 100644 > --- a/tools/testing/selftests/kvm/include/kvm_util.h > +++ b/tools/testing/selftests/kvm/include/kvm_util.h > @@ -125,6 +125,12 @@ void vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, > struct kvm_sregs *sregs); > int _vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, > struct kvm_sregs *sregs); > +void vcpu_fpu_get(struct kvm_vm *vm, uint32_t vcpuid, > + struct kvm_fpu *fpu); > +void vcpu_fpu_set(struct kvm_vm *vm, uint32_t vcpuid, > + struct kvm_fpu *fpu); > +void vcpu_get_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg); > +void vcpu_set_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg); > #ifdef __KVM_HAVE_VCPU_EVENTS > void vcpu_events_get(struct kvm_vm *vm, uint32_t vcpuid, > struct kvm_vcpu_events *events); > diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c > index 41cf45416060..dae117728ec6 100644 > --- a/tools/testing/selftests/kvm/lib/kvm_util.c > +++ b/tools/testing/selftests/kvm/lib/kvm_util.c > @@ -1373,6 +1373,54 @@ int _vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_sregs *sregs) > return ioctl(vcpu->fd, KVM_SET_SREGS, sregs); > } > > +void vcpu_fpu_get(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_fpu *fpu) > +{ > + struct vcpu *vcpu = vcpu_find(vm, vcpuid); > + int ret; > + > + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); > + > + ret = ioctl(vcpu->fd, KVM_GET_FPU, fpu); > + TEST_ASSERT(ret == 0, "KVM_GET_FPU failed, rc: %i errno: %i", > + ret, errno); > +} > + > +void vcpu_fpu_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_fpu *fpu) > +{ > + struct vcpu *vcpu = vcpu_find(vm, vcpuid); > + int ret; > + > + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); > + > + ret = ioctl(vcpu->fd, KVM_SET_FPU, fpu); > + TEST_ASSERT(ret == 0, "KVM_SET_FPU failed, rc: %i errno: %i", > + ret, errno); > +} > + > +void vcpu_get_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg) > +{ > + struct vcpu *vcpu = vcpu_find(vm, vcpuid); > + int ret; > + > + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); > + > + ret = ioctl(vcpu->fd, KVM_GET_ONE_REG, reg); > + TEST_ASSERT(ret == 0, "KVM_GET_ONE_REG failed, rc: %i errno: %i", > + ret, errno); > +} > + > +void vcpu_set_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg) > +{ > + struct vcpu *vcpu = vcpu_find(vm, vcpuid); > + int ret; > + > + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); > + > + ret = ioctl(vcpu->fd, KVM_SET_ONE_REG, reg); > + TEST_ASSERT(ret == 0, "KVM_SET_ONE_REG failed, rc: %i errno: %i", > + ret, errno); > +} > + > /* > * VCPU Ioctl > * > Reviewed-by: Thomas Huth <thuth@redhat.com>
On Thu, Jan 30, 2020 at 11:36:21AM +0100, Thomas Huth wrote: > On 29/01/2020 21.03, Janosch Frank wrote: > > Add library access to more registers. > > > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > > --- > > .../testing/selftests/kvm/include/kvm_util.h | 6 +++ > > tools/testing/selftests/kvm/lib/kvm_util.c | 48 +++++++++++++++++++ > > 2 files changed, 54 insertions(+) > > > > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h > > index 29cccaf96baf..ae0d14c2540a 100644 > > --- a/tools/testing/selftests/kvm/include/kvm_util.h > > +++ b/tools/testing/selftests/kvm/include/kvm_util.h > > @@ -125,6 +125,12 @@ void vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, > > struct kvm_sregs *sregs); > > int _vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, > > struct kvm_sregs *sregs); > > +void vcpu_fpu_get(struct kvm_vm *vm, uint32_t vcpuid, > > + struct kvm_fpu *fpu); > > +void vcpu_fpu_set(struct kvm_vm *vm, uint32_t vcpuid, > > + struct kvm_fpu *fpu); > > +void vcpu_get_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg); > > +void vcpu_set_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg); > > #ifdef __KVM_HAVE_VCPU_EVENTS > > void vcpu_events_get(struct kvm_vm *vm, uint32_t vcpuid, > > struct kvm_vcpu_events *events); > > diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c > > index 41cf45416060..dae117728ec6 100644 > > --- a/tools/testing/selftests/kvm/lib/kvm_util.c > > +++ b/tools/testing/selftests/kvm/lib/kvm_util.c > > @@ -1373,6 +1373,54 @@ int _vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_sregs *sregs) > > return ioctl(vcpu->fd, KVM_SET_SREGS, sregs); > > } > > > > +void vcpu_fpu_get(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_fpu *fpu) > > +{ > > + struct vcpu *vcpu = vcpu_find(vm, vcpuid); > > + int ret; > > + > > + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); > > + > > + ret = ioctl(vcpu->fd, KVM_GET_FPU, fpu); > > + TEST_ASSERT(ret == 0, "KVM_GET_FPU failed, rc: %i errno: %i", > > + ret, errno); > > +} > > + > > +void vcpu_fpu_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_fpu *fpu) > > +{ > > + struct vcpu *vcpu = vcpu_find(vm, vcpuid); > > + int ret; > > + > > + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); > > + > > + ret = ioctl(vcpu->fd, KVM_SET_FPU, fpu); > > + TEST_ASSERT(ret == 0, "KVM_SET_FPU failed, rc: %i errno: %i", > > + ret, errno); > > +} > > + > > +void vcpu_get_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg) > > +{ > > + struct vcpu *vcpu = vcpu_find(vm, vcpuid); > > + int ret; > > + > > + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); > > + > > + ret = ioctl(vcpu->fd, KVM_GET_ONE_REG, reg); > > + TEST_ASSERT(ret == 0, "KVM_GET_ONE_REG failed, rc: %i errno: %i", > > + ret, errno); > > +} > > + > > +void vcpu_set_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg) > > +{ > > + struct vcpu *vcpu = vcpu_find(vm, vcpuid); > > + int ret; > > + > > + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); > > + > > + ret = ioctl(vcpu->fd, KVM_SET_ONE_REG, reg); > > + TEST_ASSERT(ret == 0, "KVM_SET_ONE_REG failed, rc: %i errno: %i", > > + ret, errno); > > +} > > + > > /* > > * VCPU Ioctl > > * > > > > Reviewed-by: Thomas Huth <thuth@redhat.com> > How about what's below instead. It should be equivalent. Thanks, drew diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h index 29cccaf96baf..d96a072e69bf 100644 --- a/tools/testing/selftests/kvm/include/kvm_util.h +++ b/tools/testing/selftests/kvm/include/kvm_util.h @@ -125,6 +125,31 @@ void vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_sregs *sregs); int _vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_sregs *sregs); + +static inline void +vcpu_fpu_get(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_fpu *fpu) +{ + vcpu_ioctl(vm, vcpuid, KVM_GET_FPU, fpu); +} + +static inline void +vcpu_fpu_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_fpu *fpu) +{ + vcpu_ioctl(vm, vcpuid, KVM_SET_FPU, fpu); +} + +static inline void +vcpu_get_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg) +{ + vcpu_ioctl(vm, vcpuid, KVM_GET_ONE_REG, reg); +} + +static inline void +vcpu_set_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg) +{ + vcpu_ioctl(vm, vcpuid, KVM_SET_ONE_REG, reg); +} + #ifdef __KVM_HAVE_VCPU_EVENTS void vcpu_events_get(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_vcpu_events *events);
On 1/30/20 2:55 PM, Andrew Jones wrote: > On Thu, Jan 30, 2020 at 11:36:21AM +0100, Thomas Huth wrote: >> On 29/01/2020 21.03, Janosch Frank wrote: >>> Add library access to more registers. >>> >>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >>> --- >>> .../testing/selftests/kvm/include/kvm_util.h | 6 +++ >>> tools/testing/selftests/kvm/lib/kvm_util.c | 48 +++++++++++++++++++ >>> 2 files changed, 54 insertions(+) >>> >>> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h >>> index 29cccaf96baf..ae0d14c2540a 100644 >>> --- a/tools/testing/selftests/kvm/include/kvm_util.h >>> +++ b/tools/testing/selftests/kvm/include/kvm_util.h >>> @@ -125,6 +125,12 @@ void vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, >>> struct kvm_sregs *sregs); >>> int _vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, >>> struct kvm_sregs *sregs); >>> +void vcpu_fpu_get(struct kvm_vm *vm, uint32_t vcpuid, >>> + struct kvm_fpu *fpu); >>> +void vcpu_fpu_set(struct kvm_vm *vm, uint32_t vcpuid, >>> + struct kvm_fpu *fpu); >>> +void vcpu_get_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg); >>> +void vcpu_set_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg); >>> #ifdef __KVM_HAVE_VCPU_EVENTS >>> void vcpu_events_get(struct kvm_vm *vm, uint32_t vcpuid, >>> struct kvm_vcpu_events *events); >>> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c >>> index 41cf45416060..dae117728ec6 100644 >>> --- a/tools/testing/selftests/kvm/lib/kvm_util.c >>> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c >>> @@ -1373,6 +1373,54 @@ int _vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_sregs *sregs) >>> return ioctl(vcpu->fd, KVM_SET_SREGS, sregs); >>> } >>> >>> +void vcpu_fpu_get(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_fpu *fpu) >>> +{ >>> + struct vcpu *vcpu = vcpu_find(vm, vcpuid); >>> + int ret; >>> + >>> + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); >>> + >>> + ret = ioctl(vcpu->fd, KVM_GET_FPU, fpu); >>> + TEST_ASSERT(ret == 0, "KVM_GET_FPU failed, rc: %i errno: %i", >>> + ret, errno); >>> +} >>> + >>> +void vcpu_fpu_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_fpu *fpu) >>> +{ >>> + struct vcpu *vcpu = vcpu_find(vm, vcpuid); >>> + int ret; >>> + >>> + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); >>> + >>> + ret = ioctl(vcpu->fd, KVM_SET_FPU, fpu); >>> + TEST_ASSERT(ret == 0, "KVM_SET_FPU failed, rc: %i errno: %i", >>> + ret, errno); >>> +} >>> + >>> +void vcpu_get_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg) >>> +{ >>> + struct vcpu *vcpu = vcpu_find(vm, vcpuid); >>> + int ret; >>> + >>> + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); >>> + >>> + ret = ioctl(vcpu->fd, KVM_GET_ONE_REG, reg); >>> + TEST_ASSERT(ret == 0, "KVM_GET_ONE_REG failed, rc: %i errno: %i", >>> + ret, errno); >>> +} >>> + >>> +void vcpu_set_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg) >>> +{ >>> + struct vcpu *vcpu = vcpu_find(vm, vcpuid); >>> + int ret; >>> + >>> + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); >>> + >>> + ret = ioctl(vcpu->fd, KVM_SET_ONE_REG, reg); >>> + TEST_ASSERT(ret == 0, "KVM_SET_ONE_REG failed, rc: %i errno: %i", >>> + ret, errno); >>> +} >>> + >>> /* >>> * VCPU Ioctl >>> * >>> >> >> Reviewed-by: Thomas Huth <thuth@redhat.com> >> > > How about what's below instead. It should be equivalent. With your proposed changes we loose a bit verbosity in the error messages. I need to think about which I like more. > > Thanks, > drew > > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h > index 29cccaf96baf..d96a072e69bf 100644 > --- a/tools/testing/selftests/kvm/include/kvm_util.h > +++ b/tools/testing/selftests/kvm/include/kvm_util.h > @@ -125,6 +125,31 @@ void vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, > struct kvm_sregs *sregs); > int _vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, > struct kvm_sregs *sregs); > + > +static inline void > +vcpu_fpu_get(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_fpu *fpu) > +{ > + vcpu_ioctl(vm, vcpuid, KVM_GET_FPU, fpu); > +} > + > +static inline void > +vcpu_fpu_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_fpu *fpu) > +{ > + vcpu_ioctl(vm, vcpuid, KVM_SET_FPU, fpu); > +} > + > +static inline void > +vcpu_get_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg) > +{ > + vcpu_ioctl(vm, vcpuid, KVM_GET_ONE_REG, reg); > +} > + > +static inline void > +vcpu_set_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg) > +{ > + vcpu_ioctl(vm, vcpuid, KVM_SET_ONE_REG, reg); > +} > + > #ifdef __KVM_HAVE_VCPU_EVENTS > void vcpu_events_get(struct kvm_vm *vm, uint32_t vcpuid, > struct kvm_vcpu_events *events); >
On Thu, Jan 30, 2020 at 03:10:55PM +0100, Janosch Frank wrote: > On 1/30/20 2:55 PM, Andrew Jones wrote: > > On Thu, Jan 30, 2020 at 11:36:21AM +0100, Thomas Huth wrote: > >> On 29/01/2020 21.03, Janosch Frank wrote: > >>> Add library access to more registers. > >>> > >>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > >>> --- > >>> .../testing/selftests/kvm/include/kvm_util.h | 6 +++ > >>> tools/testing/selftests/kvm/lib/kvm_util.c | 48 +++++++++++++++++++ > >>> 2 files changed, 54 insertions(+) > >>> > >>> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h > >>> index 29cccaf96baf..ae0d14c2540a 100644 > >>> --- a/tools/testing/selftests/kvm/include/kvm_util.h > >>> +++ b/tools/testing/selftests/kvm/include/kvm_util.h > >>> @@ -125,6 +125,12 @@ void vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, > >>> struct kvm_sregs *sregs); > >>> int _vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, > >>> struct kvm_sregs *sregs); > >>> +void vcpu_fpu_get(struct kvm_vm *vm, uint32_t vcpuid, > >>> + struct kvm_fpu *fpu); > >>> +void vcpu_fpu_set(struct kvm_vm *vm, uint32_t vcpuid, > >>> + struct kvm_fpu *fpu); nit: no need for the above line breaks. We don't even get to 80 char. > >>> +void vcpu_get_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg); > >>> +void vcpu_set_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg); > >>> #ifdef __KVM_HAVE_VCPU_EVENTS > >>> void vcpu_events_get(struct kvm_vm *vm, uint32_t vcpuid, > >>> struct kvm_vcpu_events *events); > >>> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c > >>> index 41cf45416060..dae117728ec6 100644 > >>> --- a/tools/testing/selftests/kvm/lib/kvm_util.c > >>> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c > >>> @@ -1373,6 +1373,54 @@ int _vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_sregs *sregs) > >>> return ioctl(vcpu->fd, KVM_SET_SREGS, sregs); > >>> } > >>> > >>> +void vcpu_fpu_get(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_fpu *fpu) > >>> +{ > >>> + struct vcpu *vcpu = vcpu_find(vm, vcpuid); > >>> + int ret; > >>> + > >>> + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); > >>> + > >>> + ret = ioctl(vcpu->fd, KVM_GET_FPU, fpu); > >>> + TEST_ASSERT(ret == 0, "KVM_GET_FPU failed, rc: %i errno: %i", > >>> + ret, errno); > >>> +} > >>> + > >>> +void vcpu_fpu_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_fpu *fpu) > >>> +{ > >>> + struct vcpu *vcpu = vcpu_find(vm, vcpuid); > >>> + int ret; > >>> + > >>> + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); > >>> + > >>> + ret = ioctl(vcpu->fd, KVM_SET_FPU, fpu); > >>> + TEST_ASSERT(ret == 0, "KVM_SET_FPU failed, rc: %i errno: %i", > >>> + ret, errno); > >>> +} > >>> + > >>> +void vcpu_get_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg) > >>> +{ > >>> + struct vcpu *vcpu = vcpu_find(vm, vcpuid); > >>> + int ret; > >>> + > >>> + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); > >>> + > >>> + ret = ioctl(vcpu->fd, KVM_GET_ONE_REG, reg); > >>> + TEST_ASSERT(ret == 0, "KVM_GET_ONE_REG failed, rc: %i errno: %i", > >>> + ret, errno); > >>> +} > >>> + > >>> +void vcpu_set_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg) > >>> +{ > >>> + struct vcpu *vcpu = vcpu_find(vm, vcpuid); > >>> + int ret; > >>> + > >>> + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); > >>> + > >>> + ret = ioctl(vcpu->fd, KVM_SET_ONE_REG, reg); > >>> + TEST_ASSERT(ret == 0, "KVM_SET_ONE_REG failed, rc: %i errno: %i", > >>> + ret, errno); > >>> +} > >>> + > >>> /* > >>> * VCPU Ioctl > >>> * > >>> > >> > >> Reviewed-by: Thomas Huth <thuth@redhat.com> > >> > > > > How about what's below instead. It should be equivalent. > > With your proposed changes we loose a bit verbosity in the error > messages. I need to think about which I like more. Looks like both error messages are missing something. The ones above are missing the string version of errno. The ones below are missing the string version of cmd. It's easy to add the string version of errno, which is an argument for keeping the functions above (but we could at least use _vcpu_ioctl to avoid duplicating the vcpu_find and vcpu!=NULL assert). Or, we could consider adding a kvm_ioctl_cmd_to_string() function, which might be nice for other ioctl wrappers now and in the future. It shouldn't be too bad to generate a string table from kvm.h, but of course we'd have to keep it maintained. Thanks, drew > > > > > Thanks, > > drew > > > > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h > > index 29cccaf96baf..d96a072e69bf 100644 > > --- a/tools/testing/selftests/kvm/include/kvm_util.h > > +++ b/tools/testing/selftests/kvm/include/kvm_util.h > > @@ -125,6 +125,31 @@ void vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, > > struct kvm_sregs *sregs); > > int _vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, > > struct kvm_sregs *sregs); > > + > > +static inline void > > +vcpu_fpu_get(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_fpu *fpu) > > +{ > > + vcpu_ioctl(vm, vcpuid, KVM_GET_FPU, fpu); > > +} > > + > > +static inline void > > +vcpu_fpu_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_fpu *fpu) > > +{ > > + vcpu_ioctl(vm, vcpuid, KVM_SET_FPU, fpu); > > +} > > + > > +static inline void > > +vcpu_get_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg) > > +{ > > + vcpu_ioctl(vm, vcpuid, KVM_GET_ONE_REG, reg); > > +} > > + > > +static inline void > > +vcpu_set_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg) > > +{ > > + vcpu_ioctl(vm, vcpuid, KVM_SET_ONE_REG, reg); > > +} > > + > > #ifdef __KVM_HAVE_VCPU_EVENTS > > void vcpu_events_get(struct kvm_vm *vm, uint32_t vcpuid, > > struct kvm_vcpu_events *events); > > > >
On 1/30/20 3:30 PM, Andrew Jones wrote: > On Thu, Jan 30, 2020 at 03:10:55PM +0100, Janosch Frank wrote: >> On 1/30/20 2:55 PM, Andrew Jones wrote: >>> On Thu, Jan 30, 2020 at 11:36:21AM +0100, Thomas Huth wrote: >>>> On 29/01/2020 21.03, Janosch Frank wrote: >>>>> Add library access to more registers. >>>>> >>>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >>>>> --- >>>>> .../testing/selftests/kvm/include/kvm_util.h | 6 +++ >>>>> tools/testing/selftests/kvm/lib/kvm_util.c | 48 +++++++++++++++++++ >>>>> 2 files changed, 54 insertions(+) >>>>> >>>>> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h >>>>> index 29cccaf96baf..ae0d14c2540a 100644 >>>>> --- a/tools/testing/selftests/kvm/include/kvm_util.h >>>>> +++ b/tools/testing/selftests/kvm/include/kvm_util.h >>>>> @@ -125,6 +125,12 @@ void vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, >>>>> struct kvm_sregs *sregs); >>>>> int _vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, >>>>> struct kvm_sregs *sregs); >>>>> +void vcpu_fpu_get(struct kvm_vm *vm, uint32_t vcpuid, >>>>> + struct kvm_fpu *fpu); >>>>> +void vcpu_fpu_set(struct kvm_vm *vm, uint32_t vcpuid, >>>>> + struct kvm_fpu *fpu); > > nit: no need for the above line breaks. We don't even get to 80 char. > >>>>> +void vcpu_get_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg); >>>>> +void vcpu_set_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg); >>>>> #ifdef __KVM_HAVE_VCPU_EVENTS >>>>> void vcpu_events_get(struct kvm_vm *vm, uint32_t vcpuid, >>>>> struct kvm_vcpu_events *events); >>>>> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c >>>>> index 41cf45416060..dae117728ec6 100644 >>>>> --- a/tools/testing/selftests/kvm/lib/kvm_util.c >>>>> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c >>>>> @@ -1373,6 +1373,54 @@ int _vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_sregs *sregs) >>>>> return ioctl(vcpu->fd, KVM_SET_SREGS, sregs); >>>>> } >>>>> >>>>> +void vcpu_fpu_get(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_fpu *fpu) >>>>> +{ >>>>> + struct vcpu *vcpu = vcpu_find(vm, vcpuid); >>>>> + int ret; >>>>> + >>>>> + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); >>>>> + >>>>> + ret = ioctl(vcpu->fd, KVM_GET_FPU, fpu); >>>>> + TEST_ASSERT(ret == 0, "KVM_GET_FPU failed, rc: %i errno: %i", >>>>> + ret, errno); >>>>> +} >>>>> + >>>>> +void vcpu_fpu_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_fpu *fpu) >>>>> +{ >>>>> + struct vcpu *vcpu = vcpu_find(vm, vcpuid); >>>>> + int ret; >>>>> + >>>>> + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); >>>>> + >>>>> + ret = ioctl(vcpu->fd, KVM_SET_FPU, fpu); >>>>> + TEST_ASSERT(ret == 0, "KVM_SET_FPU failed, rc: %i errno: %i", >>>>> + ret, errno); >>>>> +} >>>>> + >>>>> +void vcpu_get_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg) >>>>> +{ >>>>> + struct vcpu *vcpu = vcpu_find(vm, vcpuid); >>>>> + int ret; >>>>> + >>>>> + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); >>>>> + >>>>> + ret = ioctl(vcpu->fd, KVM_GET_ONE_REG, reg); >>>>> + TEST_ASSERT(ret == 0, "KVM_GET_ONE_REG failed, rc: %i errno: %i", >>>>> + ret, errno); >>>>> +} >>>>> + >>>>> +void vcpu_set_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg) >>>>> +{ >>>>> + struct vcpu *vcpu = vcpu_find(vm, vcpuid); >>>>> + int ret; >>>>> + >>>>> + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); >>>>> + >>>>> + ret = ioctl(vcpu->fd, KVM_SET_ONE_REG, reg); >>>>> + TEST_ASSERT(ret == 0, "KVM_SET_ONE_REG failed, rc: %i errno: %i", >>>>> + ret, errno); >>>>> +} >>>>> + >>>>> /* >>>>> * VCPU Ioctl >>>>> * >>>>> >>>> >>>> Reviewed-by: Thomas Huth <thuth@redhat.com> >>>> >>> >>> How about what's below instead. It should be equivalent. >> >> With your proposed changes we loose a bit verbosity in the error >> messages. I need to think about which I like more. > > Looks like both error messages are missing something. The ones above are > missing the string version of errno. The ones below are missing the string > version of cmd. It's easy to add the string version of errno, which is > an argument for keeping the functions above (but we could at least use > _vcpu_ioctl to avoid duplicating the vcpu_find and vcpu!=NULL assert). Will do > Or, we could consider adding a kvm_ioctl_cmd_to_string() function, > which might be nice for other ioctl wrappers now and in the future. > It shouldn't be too bad to generate a string table from kvm.h, but of > course we'd have to keep it maintained. I'm currently occupied with managing a lot of patches, so something like that is not very high on my todo list. > > Thanks, > drew > >> >>> >>> Thanks, >>> drew >>> >>> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h >>> index 29cccaf96baf..d96a072e69bf 100644 >>> --- a/tools/testing/selftests/kvm/include/kvm_util.h >>> +++ b/tools/testing/selftests/kvm/include/kvm_util.h >>> @@ -125,6 +125,31 @@ void vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, >>> struct kvm_sregs *sregs); >>> int _vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, >>> struct kvm_sregs *sregs); >>> + >>> +static inline void >>> +vcpu_fpu_get(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_fpu *fpu) >>> +{ >>> + vcpu_ioctl(vm, vcpuid, KVM_GET_FPU, fpu); >>> +} >>> + >>> +static inline void >>> +vcpu_fpu_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_fpu *fpu) >>> +{ >>> + vcpu_ioctl(vm, vcpuid, KVM_SET_FPU, fpu); >>> +} >>> + >>> +static inline void >>> +vcpu_get_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg) >>> +{ >>> + vcpu_ioctl(vm, vcpuid, KVM_GET_ONE_REG, reg); >>> +} >>> + >>> +static inline void >>> +vcpu_set_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg) >>> +{ >>> + vcpu_ioctl(vm, vcpuid, KVM_SET_ONE_REG, reg); >>> +} >>> + >>> #ifdef __KVM_HAVE_VCPU_EVENTS >>> void vcpu_events_get(struct kvm_vm *vm, uint32_t vcpuid, >>> struct kvm_vcpu_events *events); >>> >> >> > > >
On Thu, Jan 30, 2020 at 03:58:46PM +0100, Janosch Frank wrote: > On 1/30/20 3:30 PM, Andrew Jones wrote: > > On Thu, Jan 30, 2020 at 03:10:55PM +0100, Janosch Frank wrote: > >> On 1/30/20 2:55 PM, Andrew Jones wrote: > >>> On Thu, Jan 30, 2020 at 11:36:21AM +0100, Thomas Huth wrote: > >>>> On 29/01/2020 21.03, Janosch Frank wrote: > >>>>> Add library access to more registers. > >>>>> > >>>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > >>>>> --- > >>>>> .../testing/selftests/kvm/include/kvm_util.h | 6 +++ > >>>>> tools/testing/selftests/kvm/lib/kvm_util.c | 48 +++++++++++++++++++ > >>>>> 2 files changed, 54 insertions(+) > >>>>> > >>>>> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h > >>>>> index 29cccaf96baf..ae0d14c2540a 100644 > >>>>> --- a/tools/testing/selftests/kvm/include/kvm_util.h > >>>>> +++ b/tools/testing/selftests/kvm/include/kvm_util.h > >>>>> @@ -125,6 +125,12 @@ void vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, > >>>>> struct kvm_sregs *sregs); > >>>>> int _vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, > >>>>> struct kvm_sregs *sregs); > >>>>> +void vcpu_fpu_get(struct kvm_vm *vm, uint32_t vcpuid, > >>>>> + struct kvm_fpu *fpu); > >>>>> +void vcpu_fpu_set(struct kvm_vm *vm, uint32_t vcpuid, > >>>>> + struct kvm_fpu *fpu); > > > > nit: no need for the above line breaks. We don't even get to 80 char. > > > >>>>> +void vcpu_get_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg); > >>>>> +void vcpu_set_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg); > >>>>> #ifdef __KVM_HAVE_VCPU_EVENTS > >>>>> void vcpu_events_get(struct kvm_vm *vm, uint32_t vcpuid, > >>>>> struct kvm_vcpu_events *events); > >>>>> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c > >>>>> index 41cf45416060..dae117728ec6 100644 > >>>>> --- a/tools/testing/selftests/kvm/lib/kvm_util.c > >>>>> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c > >>>>> @@ -1373,6 +1373,54 @@ int _vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_sregs *sregs) > >>>>> return ioctl(vcpu->fd, KVM_SET_SREGS, sregs); > >>>>> } > >>>>> > >>>>> +void vcpu_fpu_get(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_fpu *fpu) > >>>>> +{ > >>>>> + struct vcpu *vcpu = vcpu_find(vm, vcpuid); > >>>>> + int ret; > >>>>> + > >>>>> + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); > >>>>> + > >>>>> + ret = ioctl(vcpu->fd, KVM_GET_FPU, fpu); > >>>>> + TEST_ASSERT(ret == 0, "KVM_GET_FPU failed, rc: %i errno: %i", > >>>>> + ret, errno); > >>>>> +} > >>>>> + > >>>>> +void vcpu_fpu_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_fpu *fpu) > >>>>> +{ > >>>>> + struct vcpu *vcpu = vcpu_find(vm, vcpuid); > >>>>> + int ret; > >>>>> + > >>>>> + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); > >>>>> + > >>>>> + ret = ioctl(vcpu->fd, KVM_SET_FPU, fpu); > >>>>> + TEST_ASSERT(ret == 0, "KVM_SET_FPU failed, rc: %i errno: %i", > >>>>> + ret, errno); > >>>>> +} > >>>>> + > >>>>> +void vcpu_get_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg) > >>>>> +{ > >>>>> + struct vcpu *vcpu = vcpu_find(vm, vcpuid); > >>>>> + int ret; > >>>>> + > >>>>> + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); > >>>>> + > >>>>> + ret = ioctl(vcpu->fd, KVM_GET_ONE_REG, reg); > >>>>> + TEST_ASSERT(ret == 0, "KVM_GET_ONE_REG failed, rc: %i errno: %i", > >>>>> + ret, errno); > >>>>> +} > >>>>> + > >>>>> +void vcpu_set_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg) > >>>>> +{ > >>>>> + struct vcpu *vcpu = vcpu_find(vm, vcpuid); > >>>>> + int ret; > >>>>> + > >>>>> + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); > >>>>> + > >>>>> + ret = ioctl(vcpu->fd, KVM_SET_ONE_REG, reg); > >>>>> + TEST_ASSERT(ret == 0, "KVM_SET_ONE_REG failed, rc: %i errno: %i", > >>>>> + ret, errno); > >>>>> +} > >>>>> + > >>>>> /* > >>>>> * VCPU Ioctl > >>>>> * > >>>>> > >>>> > >>>> Reviewed-by: Thomas Huth <thuth@redhat.com> > >>>> > >>> > >>> How about what's below instead. It should be equivalent. > >> > >> With your proposed changes we loose a bit verbosity in the error > >> messages. I need to think about which I like more. > > > > Looks like both error messages are missing something. The ones above are > > missing the string version of errno. The ones below are missing the string > > version of cmd. It's easy to add the string version of errno, which is > > an argument for keeping the functions above (but we could at least use > > _vcpu_ioctl to avoid duplicating the vcpu_find and vcpu!=NULL assert). > > Will do > > > Or, we could consider adding a kvm_ioctl_cmd_to_string() function, > > which might be nice for other ioctl wrappers now and in the future. > > It shouldn't be too bad to generate a string table from kvm.h, but of > > course we'd have to keep it maintained. > > I'm currently occupied with managing a lot of patches, so something like > that is not very high on my todo list. Yeah, no worries. We can go with a patch like this for now. I'll experiment with a table generator when I get a chance in order to see how ugly it gets. If it's too ugly I'll drop it too. Thanks, drew
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h index 29cccaf96baf..ae0d14c2540a 100644 --- a/tools/testing/selftests/kvm/include/kvm_util.h +++ b/tools/testing/selftests/kvm/include/kvm_util.h @@ -125,6 +125,12 @@ void vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_sregs *sregs); int _vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_sregs *sregs); +void vcpu_fpu_get(struct kvm_vm *vm, uint32_t vcpuid, + struct kvm_fpu *fpu); +void vcpu_fpu_set(struct kvm_vm *vm, uint32_t vcpuid, + struct kvm_fpu *fpu); +void vcpu_get_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg); +void vcpu_set_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg); #ifdef __KVM_HAVE_VCPU_EVENTS void vcpu_events_get(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_vcpu_events *events); diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c index 41cf45416060..dae117728ec6 100644 --- a/tools/testing/selftests/kvm/lib/kvm_util.c +++ b/tools/testing/selftests/kvm/lib/kvm_util.c @@ -1373,6 +1373,54 @@ int _vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_sregs *sregs) return ioctl(vcpu->fd, KVM_SET_SREGS, sregs); } +void vcpu_fpu_get(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_fpu *fpu) +{ + struct vcpu *vcpu = vcpu_find(vm, vcpuid); + int ret; + + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); + + ret = ioctl(vcpu->fd, KVM_GET_FPU, fpu); + TEST_ASSERT(ret == 0, "KVM_GET_FPU failed, rc: %i errno: %i", + ret, errno); +} + +void vcpu_fpu_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_fpu *fpu) +{ + struct vcpu *vcpu = vcpu_find(vm, vcpuid); + int ret; + + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); + + ret = ioctl(vcpu->fd, KVM_SET_FPU, fpu); + TEST_ASSERT(ret == 0, "KVM_SET_FPU failed, rc: %i errno: %i", + ret, errno); +} + +void vcpu_get_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg) +{ + struct vcpu *vcpu = vcpu_find(vm, vcpuid); + int ret; + + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); + + ret = ioctl(vcpu->fd, KVM_GET_ONE_REG, reg); + TEST_ASSERT(ret == 0, "KVM_GET_ONE_REG failed, rc: %i errno: %i", + ret, errno); +} + +void vcpu_set_reg(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_one_reg *reg) +{ + struct vcpu *vcpu = vcpu_find(vm, vcpuid); + int ret; + + TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid); + + ret = ioctl(vcpu->fd, KVM_SET_ONE_REG, reg); + TEST_ASSERT(ret == 0, "KVM_SET_ONE_REG failed, rc: %i errno: %i", + ret, errno); +} + /* * VCPU Ioctl *
Add library access to more registers. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- .../testing/selftests/kvm/include/kvm_util.h | 6 +++ tools/testing/selftests/kvm/lib/kvm_util.c | 48 +++++++++++++++++++ 2 files changed, 54 insertions(+)