[PATCH v4 4/4] tests: check KVM ioctl commands decoding

Masatake YAMATO yamato at redhat.com
Thu Dec 7 15:15:37 UTC 2017


On Thu, 7 Dec 2017 17:18:50 +0300, "Dmitry V. Levin" <ldv at altlinux.org> wrote:
> On Thu, Dec 07, 2017 at 11:14:34PM +0900, Masatake YAMATO wrote:
>> On Thu, 7 Dec 2017 17:04:05 +0300, "Dmitry V. Levin" <ldv at altlinux.org> wrote:
>> > On Mon, Dec 04, 2017 at 10:08:17PM +0900, Masatake YAMATO wrote:
>> >> * tests/Makefile.am (check_PROGRAMS): Add ioctl_kvm.
>> >> (DECODER_TESTS): Add ioctl_kvm.test.
>> >> (EXTRA_DIST): Add ioctl_kvm.expected.
>> >> * tests/ioctl_kvm.c: New test target file.
>> >>   Taken from https://lwn.net/Articles/658512/.
>> >> * tests/ioctl_kvm.expected: New expected file.
>> >> * tests/ioctl_kvm.test: New test driver.
>> >> 
>> >> Changes in v2:
>> >> * Skip the test case if kvm.h is not available.
>> >> * Skip the test case if opening /dev/kvm is failed.
>> >> * Include sys/typtes.h first.
>> >> 
>> >>   All items are suggested by ldv.
>> >> 
>> >> Highlights in v3:
>> >> * Build the new test case when __x86_64__ is defined.
>> >> * Don't use macros/functions in err.h.
>> >>   Use macros/functions in tests.h instead.
>> >> * Remove redundant inclusion of header files.
>> >> 
>> >>   All above items are suggested by ldv.
>> >> 
>> >> * Add .test and .expected file to Makefile.am.
>> >> 
>> >> No Change in v4.
>> > 
>> > Yet the test doesn't quite fit the test suite, so I took the liberty
>> > to tweak it here and there, please have a look.
>> > 
>> > I also renamed it to make room for more formal KVM_* ioctl tests like
>> > those we already have (tests/ioctl_*.c) for other ioctl decoders.
>> > 
>> > * tests/ioctl_kvm_run.c: New file.
>> > * tests/ioctl_kvm_run.test: New test.
>> > * tests/Makefile.am (DECODER_TESTS): Add ioctl_kvm_run.test.
>> > * tests/pure_executables.list: Add ioctl_kvm_run.
>> > * tests/.gitignore: Likewise.
>> > 
>> > Co-authored-by: Dmitry V. Levin <ldv at altlinux.org>
>> > ---
>> >  tests/.gitignore            |   1 +
>> >  tests/Makefile.am           |   1 +
>> >  tests/ioctl_kvm_run.c       | 220 ++++++++++++++++++++++++++++++++++++++++++++
>> >  tests/ioctl_kvm_run.test    |  11 +++
>> >  tests/pure_executables.list |   1 +
>> >  5 files changed, 234 insertions(+)
>> >  create mode 100644 tests/ioctl_kvm_run.c
>> >  create mode 100755 tests/ioctl_kvm_run.test
>> > 
>> > diff --git a/tests/.gitignore b/tests/.gitignore
>> > index 93c980f..7838a5b 100644
>> > --- a/tests/.gitignore
>> > +++ b/tests/.gitignore
>> > @@ -128,6 +128,7 @@ ioctl_dm
>> >  ioctl_dm-v
>> >  ioctl_evdev
>> >  ioctl_evdev-v
>> > +ioctl_kvm_run
>> >  ioctl_loop
>> >  ioctl_loop-nv
>> >  ioctl_loop-v
>> > diff --git a/tests/Makefile.am b/tests/Makefile.am
>> > index 7bc2872..22a39d7 100644
>> > --- a/tests/Makefile.am
>> > +++ b/tests/Makefile.am
>> > @@ -206,6 +206,7 @@ DECODER_TESTS = \
>> >  	ioctl.test \
>> >  	ioctl_dm-v.test \
>> >  	ioctl_dm.test \
>> > +	ioctl_kvm_run.test \
>> >  	ioctl_loop-nv.test \
>> >  	ioctl_nsfs.test \
>> >  	ioctl_sock_gifconf.test \
>> > diff --git a/tests/ioctl_kvm_run.c b/tests/ioctl_kvm_run.c
>> > new file mode 100644
>> > index 0000000..90fffba
>> > --- /dev/null
>> > +++ b/tests/ioctl_kvm_run.c
>> > @@ -0,0 +1,220 @@
>> > +/*
>> > + * Check decoding of KVM_* commands of ioctl syscall using /dev/kvm API.
>> > + * Based on kvmtest.c from https://lwn.net/Articles/658512/
>> > + *
>> > + * kvmtest.c author: Josh Triplett <josh at joshtriplett.org>
>> > + * Copyright (c) 2015 Intel Corporation
>> > + * Copyright (c) 2017 The strace developers.
>> > + *
>> > + * Permission is hereby granted, free of charge, to any person obtaining a copy
>> > + * of this software and associated documentation files (the "Software"), to
>> > + * deal in the Software without restriction, including without limitation the
>> > + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
>> > + * sell copies of the Software, and to permit persons to whom the Software is
>> > + * furnished to do so, subject to the following conditions:
>> > + *
>> > + * The above copyright notice and this permission notice shall be included in
>> > + * all copies or substantial portions of the Software.
>> > + *
>> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
>> > + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>> > + * IN THE SOFTWARE.
>> > + */
>> > +
>> > +#include "tests.h"
>> > +
>> > +#if defined HAVE_LINUX_KVM_H				\
>> > + && defined HAVE_STRUCT_KVM_REGS			\
>> > + && defined HAVE_STRUCT_KVM_SREGS			\
>> > + && defined HAVE_STRUCT_KVM_USERSPACE_MEMORY_REGION	\
>> > + &&(defined __x86_64__ || defined __i386__)
>> > +
>> > +# include <fcntl.h>
>> > +# include <stdint.h>
>> > +# include <stdio.h>
>> > +# include <stdlib.h>
>> > +# include <string.h>
>> > +# include <sys/ioctl.h>
>> > +# include <sys/mman.h>
>> > +# include <linux/kvm.h>
>> > +
>> > +static int
>> > +kvm_ioctl(int fd, unsigned long cmd, const char *cmd_str, void *arg)
>> > +{
>> > +	int rc = ioctl(fd, cmd, arg);
>> > +	if (rc < 0)
>> > +		perror_msg_and_skip("%s", cmd_str);
>> > +	return rc;
>> > +}
>> > +
>> > +#define KVM_IOCTL(fd_, cmd_, arg_)	\
>> > +	kvm_ioctl((fd_), (cmd_), #cmd_, (arg_))
>> > +
>> > +static const char dev[] = "/dev/kvm";
>> > +static const char vm_dev[] = "anon_inode:kvm-vm";
>> > +static const char vcpu_dev[] = "anon_inode:kvm-vcpu";
>> > +static size_t page_size;
>> > +
>> > +static void
>> > +code(void)
>> > +{
>> > +	__asm__("mov $0xd80003f8, %edx; mov $'\n', %al; out %al, (%dx); hlt");
>> > +}
>> > +
>> > +static void
>> > +run_kvm(const int vcpu_fd, struct kvm_run *const run, const size_t mmap_size,
>> > +	void *const mem)
>> > +{
>> > +	/* Initialize CS to point at 0, via a read-modify-write of sregs. */
>> > +	struct kvm_sregs sregs;
>> > +	KVM_IOCTL(vcpu_fd, KVM_GET_SREGS, &sregs);
>> > +	printf("ioctl(%d<%s>, KVM_GET_SREGS, {cs={base=%#jx, limit=%u, selector=%u"
>> > +	       ", type=%u, present=%u, dpl=%u, db=%u, s=%u, l=%u, g=%u, avl=%u}"
>> > +	       ", ...}) = 0\n", vcpu_fd, vcpu_dev, (uintmax_t) sregs.cs.base,
>> > +	       sregs.cs.limit, sregs.cs.selector, sregs.cs.type,
>> > +	       sregs.cs.present, sregs.cs.dpl, sregs.cs.db, sregs.cs.s,
>> > +	       sregs.cs.l, sregs.cs.g, sregs.cs.avl);
>> > +
>> > +	sregs.cs.base = 0;
>> > +	sregs.cs.selector = 0;
>> > +	KVM_IOCTL(vcpu_fd, KVM_SET_SREGS, &sregs);
>> > +	printf("ioctl(%d<%s>, KVM_SET_SREGS, {cs={base=%#jx, limit=%u"
>> > +	       ", selector=%u, type=%u, present=%u, dpl=%u, db=%u, s=%u"
>> > +	       ", l=%u, g=%u, avl=%u}, ...}) = 0\n",
>> > +	       vcpu_fd, vcpu_dev, (uintmax_t) sregs.cs.base,
>> > +	       sregs.cs.limit, sregs.cs.selector, sregs.cs.type,
>> > +	       sregs.cs.present, sregs.cs.dpl, sregs.cs.db, sregs.cs.s,
>> > +	       sregs.cs.l, sregs.cs.g, sregs.cs.avl);
>> > +
>> > +	/*
>> > +	 * Initialize registers: instruction pointer for our code, addends,
>> > +	 * and initial flags required by x86 architecture.
>> > +	 */
>> > +	struct kvm_regs regs = {
>> > +		.rip = page_size,
>> > +		.rax = 2,
>> > +		.rbx = 2,
>> > +		.rflags = 0x2,
>> > +	};
>> > +	KVM_IOCTL(vcpu_fd, KVM_SET_REGS, &regs);
>> > +	printf("ioctl(%d<%s>, KVM_SET_REGS, {rax=%#jx, ..."
>> > +	       ", rsp=%#jx, rbp=%#jx, ..., rip=%#jx, rflags=%#jx}) = 0\n",
>> > +	       vcpu_fd, vcpu_dev, (uintmax_t) regs.rax,
>> > +	       (uintmax_t) regs.rsp, (uintmax_t) regs.rbp,
>> > +	       (uintmax_t) regs.rip, (uintmax_t) regs.rflags);
>> > +
>> > +	/* Copy the code till the end of page */
>> > +	size_t code_size = page_size - ((uintptr_t) code & (page_size - 1));
>> > +	if (code_size < 16)
>> > +		code_size = 16;
>> > +	memcpy(mem, code, code_size);
>> > +
>> > +	const char *p = "\n";
>> > +
>> > +	/* Repeatedly run code and handle VM exits. */
>> > +	for (;;) {
>> > +		KVM_IOCTL(vcpu_fd, KVM_RUN, NULL);
>> > +		printf("ioctl(%d<%s>, KVM_RUN, 0) = 0\n", vcpu_fd, vcpu_dev);
>> > +
>> > +		switch (run->exit_reason) {
>> > +		case KVM_EXIT_HLT:
>> > +			if (p)
>> > +				error_msg_and_fail("premature KVM_EXIT_HLT");
>> > +			return;
>> > +		case KVM_EXIT_IO:
>> > +			if (run->io.direction == KVM_EXIT_IO_OUT
>> > +			    && run->io.size == 1
>> > +			    && run->io.port == 0x03f8
>> > +			    && run->io.count == 1
>> > +			    && run->io.data_offset < mmap_size
>> > +			    && p && *p == ((char *) run)[run->io.data_offset])
>> > +				p = NULL;
>> > +			else
>> > +				error_msg_and_fail("unhandled KVM_EXIT_IO");
>> > +			break;
>> > +		default:
>> > +			error_msg_and_fail("exit_reason = %#x",
>> > +					   run->exit_reason);
>> > +		}
>> > +	}
>> > +}
>> > +
>> > +int
>> > +main(void)
>> > +{
>> > +	skip_if_unavailable("/proc/self/fd/");
>> > +
>> > +	int kvm = open(dev, O_RDWR);
>> > +	if (kvm < 0)
>> > +		perror_msg_and_skip("open: %s", dev);
>> > +
>> > +	/* Make sure we have the stable version of the API */
>> > +	int ret = KVM_IOCTL(kvm, KVM_GET_API_VERSION, 0);
>> > +	if (ret != KVM_API_VERSION)
>> > +		error_msg_and_skip("KVM_GET_API_VERSION returned %d"
>> > +				   ", KVM_API_VERSION is %d",
>> > +				   kvm, KVM_API_VERSION);
>> > +	printf("ioctl(%d<%s>, KVM_GET_API_VERSION, 0) = %d\n",
>> > +	       kvm, dev, ret);
>> > +
>> > +	int vm_fd = KVM_IOCTL(kvm, KVM_CREATE_VM, 0);
>> > +	printf("ioctl(%d<%s>, KVM_CREATE_VM, 0) = %d<%s>\n",
>> > +	       kvm, dev, vm_fd, vm_dev);
>> > +
>> > +	/* Allocate one aligned page of guest memory to hold the code. */
>> > +	page_size = get_page_size();
>> > +	void *const mem = mmap(NULL, page_size, PROT_READ | PROT_WRITE,
>> > +				  MAP_SHARED | MAP_ANONYMOUS, -1, 0);
>> > +	if (mem == MAP_FAILED)
>> > +		perror_msg_and_fail("mmap page");
>> > +
>> > +	/* Map it to the second page frame (to avoid the real-mode IDT at 0). */
>> > +	struct kvm_userspace_memory_region region = {
>> > +		.slot = 0,
>> > +		.guest_phys_addr = page_size,
>> > +		.memory_size = page_size,
>> > +		.userspace_addr = (uintptr_t) mem,
>> > +	};
>> > +	KVM_IOCTL(vm_fd, KVM_SET_USER_MEMORY_REGION, &region);
>> > +	printf("ioctl(%d<%s>, KVM_SET_USER_MEMORY_REGION"
>> > +	       ", {slot=0, flags=0, guest_phys_addr=%#lx, memory_size=%lu"
>> > +	       ", userspace_addr=%p) = 0\n", vm_fd, vm_dev,
>> 
>> Please, look at [PATCH v5 1/4] and [PATCH v5 4/4]. I found I missed
>> printing '}' at the end. I added the last '}' in the above patch.
> 
> Yes, I missed it, too.  Thanks, fixed locally.
> 
>> > +	       (unsigned long) page_size, (unsigned long) page_size, mem);
>> > +
>> > +	int vcpu_fd = KVM_IOCTL(vm_fd, KVM_CREATE_VCPU, NULL);
>> > +	printf("ioctl(%d<%s>, KVM_CREATE_VCPU, 0) = %d<%s>\n",
>> > +	       vm_fd, vm_dev, vcpu_fd, vcpu_dev);
>> > +
>> > +	/* Map the shared kvm_run structure and following data. */
>> > +	ret = KVM_IOCTL(kvm, KVM_GET_VCPU_MMAP_SIZE, NULL);
>> > +	struct kvm_run *run;
>> > +	if (ret < (int) sizeof(*run))
>> > +		error_msg_and_fail("KVM_GET_VCPU_MMAP_SIZE returned %d < %d",
>> > +				   ret, (int) sizeof(*run));
>> > +	printf("ioctl(%d<%s>, KVM_GET_VCPU_MMAP_SIZE, 0) = %d\n",
>> > +	       kvm, dev, ret);
>> > +
>> > +	const size_t mmap_size = (ret + page_size - 1) & -page_size;
>> > +	run = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE,
>> > +		   MAP_SHARED, vcpu_fd, 0);
>> > +	if (run == MAP_FAILED)
>> > +		perror_msg_and_fail("mmap vcpu");
>> > +
>> > +	run_kvm(vcpu_fd, run, mmap_size, mem);
>> > +
>> > +	puts("+++ exited with 0 +++");
>> > +	return 0;
>> > +}
>> > +
>> > +#else /* !HAVE_LINUX_KVM_H */
>> > +
>> > +SKIP_MAIN_UNDEFINED("HAVE_LINUX_KVM_H && HAVE_STRUCT_KVM_REGS && "
>> > +		    "HAVE_STRUCT_KVM_SREGS && "
>> > +		    "HAVE_STRUCT_KVM_USERSPACE_MEMORY_REGION && "
>> > +		    "(__x86_64__ || __i386__)")
>> > +
>> > +#endif
>> > diff --git a/tests/ioctl_kvm_run.test b/tests/ioctl_kvm_run.test
>> > new file mode 100755
>> > index 0000000..a23cbeb
>> > --- /dev/null
>> > +++ b/tests/ioctl_kvm_run.test
>> > @@ -0,0 +1,11 @@
>> > +#!/bin/sh
>> > +
>> > +# Check decoding of KVM_* ioctl commands.
>> > +
>> > +. "${srcdir=.}/init.sh"
>> > +
>> > +check_prog grep
>> > +run_prog > /dev/null
>> > +run_strace -a36 -y -eioctl $args > "$EXP"
>> > +grep -v '^ioctl([012],' < "$LOG" > "$OUT"
>> > +match_diff "$OUT" "$EXP"
>> > diff --git a/tests/pure_executables.list b/tests/pure_executables.list
>> > index 7ab52ef..0d1964d 100755
>> > --- a/tests/pure_executables.list
>> > +++ b/tests/pure_executables.list
>> > @@ -103,6 +103,7 @@ ioctl
>> >  ioctl_block
>> >  ioctl_dm
>> >  ioctl_evdev
>> > +ioctl_kvm_run
>> >  ioctl_loop
>> >  ioctl_mtd
>> >  ioctl_rtc
> 
> What about the patch itself?  Are you OK with the changes?

I read this through. Other than assembly language code in code(),
it looks good for me.

I don't understand what is done in code().
I compiled .c file having only code() and run objdump -d:
What I got is

    0000000000000000 <code>:
       0:	55                   	push   %rbp
       1:	48 89 e5             	mov    %rsp,%rbp
       4:	ba f8 03 00 d8       	mov    $0xd80003f8,%edx
       9:	b0 0a                	mov    $0xa,%al
       b:	ee                   	out    %al,(%dx)
       c:	f4                   	hlt    
       d:	90                   	nop
       e:	5d                   	pop    %rbp
       f:	c3                   	retq

I find no add operator in the output. So I guess what this
code does is different fron what the original code does:

	const uint8_t code[] = {
		0xba, 0xf8, 0x03, /* mov $0x3f8, %dx */
		0x00, 0xd8,       /* add %bl, %al */
		0x04, '0',        /* add $'0', %al */

The code executed by kvm is not important here. However,
I wondher what the intent is.

Masatake YAMATO

> 
> -- 
> ldv





More information about the Strace-devel mailing list