[PATCH 2/2] kvm: add tests for KVM_SET_CPUID2 and KVM_GET_SUPPORTED_CPUID ioctl

[Dmitry V. Levin]

On Sat, Jun 23, 2018 at 01:10:14AM +0100, Pierre Marsais wrote:
> * tests/ioctl_kvm_run.c (main): Add tests for cpuid related ioctl

The change adds checks for three ioctls, so the commit message should
either name all of them or neither of them.

I suggest the following wording:

tests: check decoding of kvm ioctls related to struct kvm_cpuid2 

> Signed-off-by: Pierre Marsais <pierre.marsais at lse.epita.fr>
> ---
>  tests/ioctl_kvm_run.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/tests/ioctl_kvm_run.c b/tests/ioctl_kvm_run.c
> index e1bef579..55319fd5 100644
> --- a/tests/ioctl_kvm_run.c
> +++ b/tests/ioctl_kvm_run.c
> @@ -253,6 +253,34 @@ main(void)
>  	if (run == MAP_FAILED)
>  		perror_msg_and_fail("mmap vcpu");
>  
> +	/*
> +	 * cpuid_nent should be KVM_MAX_CPUID_ENTRIES, however it is not defined
> +	 * in public headers.
> +	 */

In cases like this we usually write

#ifndef KVM_MAX_CPUID_ENTRIES
# define KVM_MAX_CPUID_ENTRIES 80
#endif

> +	size_t cpuid_nent = 80;
> +	struct kvm_cpuid2 *cpuid = malloc(sizeof(*cpuid) +
> +					  cpuid_nent * sizeof(*cpuid->entries));

What if malloc() fails?

I suggest using tail_alloc instead (see tests/tests.h and many examples of
its use in tests/).

> +	cpuid->nent = cpuid_nent;
> +
> +	KVM_IOCTL(kvm, KVM_GET_SUPPORTED_CPUID, cpuid);
> +	printf("ioctl(%d<%s>, KVM_GET_SUPPORTED_CPUID, {nent=%d, entries=[",
> +	       kvm, dev, cpuid->nent);
> +	if (cpuid->nent)
> +		printf("...");
> +	printf("]}) = 0\n");

What if KVM_GET_SUPPORTED_CPUID fails?

Note that the most interesting part of kvm_cpuid2 decoder
(print_kvm_cpuid_entry) is not covered by this test.

> +
> +	struct kvm_cpuid2 cpuid_tmp = { .nent = 0 };
> +	KVM_IOCTL(vcpu_fd, KVM_SET_CPUID2, &cpuid_tmp);
> +	printf("ioctl(%d<%s>, KVM_SET_CPUID2, {nent=%d, entries=[]}) = 0\n",
> +	       vcpu_fd, vcpu_dev, cpuid_tmp.nent);
> +	KVM_IOCTL(vcpu_fd, KVM_SET_CPUID2, cpuid);
> +	printf("ioctl(%d<%s>, KVM_SET_CPUID2, {nent=%d, entries=[",
> +	       vcpu_fd, vcpu_dev, cpuid->nent);
> +	if (cpuid->nent)
> +		printf("...");
> +	printf("]}) = 0\n");

Note that umove_or_printaddr() != 0 case of kvm_ioctl_decode_cpuid2 is not
covered by this test.


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20180626/04e61980/attachment.bin>