Re: [PATCH v8 2/4] selftests: KVM: Add fpu and one reg set/get library functions

[Janosch Frank <frankja@linux.ibm.com>]

[-- Attachment #1.1: Type: text/plain, Size: 5100 bytes --]

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);
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]