[PATCH 1/2] kvm: decode the argument of KVM_SET_CPUID2 and KVM_GET_*_CPUID ioctl command

[Dmitry V. Levin]

On Sat, Jun 23, 2018 at 01:10:13AM +0100, Pierre Marsais wrote:
> * configure.ac (AC_CHECK_TYPES): Add struct kvm_cpuid2

According to the GNU ChangeLog style, these sentences are terminated with
dot.

> * kvm.c (print_kvm_cpuid_entry): New function
> (kvm_ioctl_decode_cpuid2): New function

Several similar changes within the same can be grouped.
HAVE_STRUCT_KVM_CPUID2 condition shouldn't be omitted.

For example,
* kvm.c [HAVE_STRUCT_KVM_CPUID2]: Include "xlat/kvm_cpuid_flags.h".
[HAVE_STRUCT_KVM_CPUID2] (print_kvm_cpuid_entry, kvm_ioctl_decode_cpuid2):
New functions.

> (kvm_ioctl): Use kvm_ioctl_decode_cpuid2

(kvm_ioctl) [HAVE_STRUCT_KVM_CPUID2]:  Usekvm_ioctl_decode_cpuid2.

> * xlat/kvm_cpuid_flags.in: New file
> 
> Signed-off-by: Pierre Marsais <pierre.marsais at lse.epita.fr>
> ---
>  configure.ac            |  1 +
>  kvm.c                   | 60 +++++++++++++++++++++++++++++++++++++++++
>  xlat/kvm_cpuid_flags.in |  3 +++
>  3 files changed, 64 insertions(+)
>  create mode 100644 xlat/kvm_cpuid_flags.in
> 
> diff --git a/configure.ac b/configure.ac
> index 9dec1b23..c1eeb578 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -575,6 +575,7 @@ AC_CHECK_TYPES(m4_normalize([
>  ]),,, [#include <linux/ptp_clock.h>])
>  
>  AC_CHECK_TYPES(m4_normalize([
> +	struct kvm_cpuid2,
>  	struct kvm_regs,
>  	struct kvm_sregs,
>  	struct kvm_userspace_memory_region
> diff --git a/kvm.c b/kvm.c
> index 86fd9e50..d9f1fff7 100644
> --- a/kvm.c
> +++ b/kvm.c
> @@ -85,6 +85,59 @@ kvm_ioctl_decode_regs(struct tcb *const tcp, const unsigned int code,
>  }
>  # endif /* HAVE_STRUCT_KVM_REGS */
>  
> +# ifdef HAVE_STRUCT_KVM_CPUID2
> +#  include "xlat/kvm_cpuid_flags.h"
> +static bool
> +print_kvm_cpuid_entry(struct tcb *const tcp,
> +		      void* elem_buf, size_t elem_size, void* data)
> +{
> +	const struct kvm_cpuid_entry2 *entry = elem_buf;
> +	PRINT_FIELD_X("{", *entry, function);
> +	PRINT_FIELD_X(", ", *entry, index);
> +	PRINT_FIELD_FLAGS(", ", *entry, flags, kvm_cpuid_flags,
> +			  "KVM_CPUID_FLAG_???");
> +	PRINT_FIELD_X(", ", *entry, eax);
> +	PRINT_FIELD_X(", ", *entry, ebx);
> +	PRINT_FIELD_X(", ", *entry, ecx);
> +	PRINT_FIELD_X(", ", *entry, edx);
> +	tprints("}");
> +
> +	return 1;

return true;

> +}
> +
> +static int
> +kvm_ioctl_decode_cpuid2(struct tcb *const tcp, const unsigned int code,
> +			const kernel_ulong_t arg)
> +{
> +	struct kvm_cpuid2 cpuid;
> +
> +	if ((code == KVM_GET_EMULATED_CPUID ||
> +	     code == KVM_GET_SUPPORTED_CPUID) && entering(tcp))
> +		return 0;
> +
> +	tprints(", ");
> +	if (!umove_or_printaddr(tcp, arg, &cpuid)) {
> +		struct kvm_cpuid_entry2 entry;

I'd move the definition of "entry" ...

> +		PRINT_FIELD_U("{", cpuid, nent);
> +
> +		tprints(", entries=");
> +		if (abbrev(tcp)) {
> +			tprints("[");
> +			if (cpuid.nent)
> +				tprints("...");
> +			tprints("]");
> +
> +		} else

... here.

> +			print_array(tcp, arg + sizeof(cpuid), cpuid.nent,
> +				    &entry, sizeof(entry), tfetch_mem,
> +				    print_kvm_cpuid_entry, NULL);
> +		tprints("}");
> +	}
> +
> +	return RVAL_IOCTL_DECODED;
> +}
> +# endif /* HAVE_STRUCT_KVM_CPUID2 */
> +
>  # ifdef HAVE_STRUCT_KVM_SREGS
>  static int
>  kvm_ioctl_decode_sregs(struct tcb *const tcp, const unsigned int code,
> @@ -127,6 +180,13 @@ kvm_ioctl(struct tcb *const tcp, const unsigned int code, const kernel_ulong_t a
>  		return kvm_ioctl_decode_sregs(tcp, code, arg);
>  # endif
>  
> +# ifdef HAVE_STRUCT_KVM_CPUID2
> +       case KVM_SET_CPUID2:
> +       case KVM_GET_SUPPORTED_CPUID:
> +       case KVM_GET_EMULATED_CPUID:
> +               return kvm_ioctl_decode_cpuid2(tcp, code, arg);
> +# endif
> +
>  	case KVM_CREATE_VM:
>  		return RVAL_DECODED | RVAL_FD;
>  	case KVM_RUN:
> diff --git a/xlat/kvm_cpuid_flags.in b/xlat/kvm_cpuid_flags.in
> new file mode 100644
> index 00000000..d80ce734
> --- /dev/null
> +++ b/xlat/kvm_cpuid_flags.in
> @@ -0,0 +1,3 @@
> +KVM_CPUID_FLAG_SIGNIFCANT_INDEX
> +KVM_CPUID_FLAG_STATEFUL_FUNC
> +KVM_CPUID_FLAG_STATE_READ_NEXT
> -- 
> 2.17.1

Besides these minor issues, the change looks fine.


-- 
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/29d05b53/attachment.bin>