[PATCH 0/4] Random kvmtool fixes

[PATCH 0/4] Random kvmtool fixes

[Marc Zyngier]

This small series addresses a number of issues that have been
pestering me for a while. Nothing major though:

The first two patches simply ensure that we can always use THP if the
have been enabled on the host. Third one fixes an annoying issue when
-tty is used.

The fourth patch allows me to use TAP interfaces *and* run kvmtool as
a non-priviledged user (tunctl is your BFF).

The whole series applies on top of kvmtool/next as of yesterday.

Thanks,

	M.

Marc Zyngier (4):
  kvmtool: ARM: force alignment of memory for THP
  kvmtool: ARM: pass MADV_HUGEPAGE to madvise
  kvmtool: Fix handling of POLLHUP when --tty is used
  kvmtool: allow the TAP interface to be specified on the command line

 tools/kvm/arm/kvm.c                | 10 ++++++----
 tools/kvm/include/kvm/virtio-net.h |  1 +
 tools/kvm/term.c                   |  4 +++-
 tools/kvm/virtio/net.c             | 21 ++++++++++++++-------
 4 files changed, 24 insertions(+), 12 deletions(-)

-- 
1.8.3.4

[PATCH 1/4] kvmtool: ARM: force alignment of memory for THP

[Marc Zyngier]

Use of THP requires that the VMA containing the guest memory is
2MB aligned. Unfortunately, nothing in kvmtool ensures that the
memory is actually aligned, making the use of THP very unlikely.

Just follow what we're already doing for virtio, and expand our
forced alignment to 2M.

* without this patch:
root@muffin-man:~# for i in $(seq 1 5); do ./hackbench 50 process 1000; done
Running with 50*40 (== 2000) tasks.
Time: 113.600
Running with 50*40 (== 2000) tasks.
Time: 108.650
Running with 50*40 (== 2000) tasks.
Time: 110.753
Running with 50*40 (== 2000) tasks.
Time: 116.992
Running with 50*40 (== 2000) tasks.
Time: 117.317

* with this patch:
root@muffin-man:~# for i in $(seq 1 5); do ./hackbench 50 process 1000; done
Running with 50*40 (== 2000) tasks.
Time: 97.613
Running with 50*40 (== 2000) tasks.
Time: 96.111
Running with 50*40 (== 2000) tasks.
Time: 97.090
Running with 50*40 (== 2000) tasks.
Time: 100.820
Running with 50*40 (== 2000) tasks.
Time: 100.298

Acked-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 tools/kvm/arm/kvm.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/kvm/arm/kvm.c b/tools/kvm/arm/kvm.c
index 008b7fe..d0d64ff 100644
--- a/tools/kvm/arm/kvm.c
+++ b/tools/kvm/arm/kvm.c
@@ -61,11 +61,13 @@ void kvm__arch_set_cmdline(char *cmdline, bool video)
 void kvm__arch_init(struct kvm *kvm, const char *hugetlbfs_path, u64 ram_size)
 {
 	/*
-	 * Allocate guest memory. We must align out buffer to 64K to
+	 * Allocate guest memory. We must align our buffer to 64K to
 	 * correlate with the maximum guest page size for virtio-mmio.
+	 * If using THP, then our minimal alignment becomes 2M.
+	 * 2M trumps 64K, so let's go with that.
 	 */
 	kvm->ram_size = min(ram_size, (u64)ARM_MAX_MEMORY(kvm));
-	kvm->arch.ram_alloc_size = kvm->ram_size + SZ_64K;
+	kvm->arch.ram_alloc_size = kvm->ram_size + SZ_2M;
 	kvm->arch.ram_alloc_start = mmap_anon_or_hugetlbfs(kvm, hugetlbfs_path,
 						kvm->arch.ram_alloc_size);
 
@@ -74,7 +76,7 @@ void kvm__arch_init(struct kvm *kvm, const char *hugetlbfs_path, u64 ram_size)
 		    kvm->arch.ram_alloc_size, errno);
 
 	kvm->ram_start = (void *)ALIGN((unsigned long)kvm->arch.ram_alloc_start,
-					SZ_64K);
+					SZ_2M);
 
 	madvise(kvm->arch.ram_alloc_start, kvm->arch.ram_alloc_size,
 		MADV_MERGEABLE);
-- 
1.8.3.4

[PATCH 2/4] kvmtool: ARM: pass MADV_HUGEPAGE to madvise

[Marc Zyngier]

If the host kernel is configured with CONFIG_TRANSPARENT_HUGEPAGE_MADVISE,
it is important to madvise(MADV_HUGEPAGE) the memory region.
Otherwise, the guest won't benefit from using THP.

Acked-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 tools/kvm/arm/kvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/kvm/arm/kvm.c b/tools/kvm/arm/kvm.c
index d0d64ff..58ad9fa 100644
--- a/tools/kvm/arm/kvm.c
+++ b/tools/kvm/arm/kvm.c
@@ -79,7 +79,7 @@ void kvm__arch_init(struct kvm *kvm, const char *hugetlbfs_path, u64 ram_size)
 					SZ_2M);
 
 	madvise(kvm->arch.ram_alloc_start, kvm->arch.ram_alloc_size,
-		MADV_MERGEABLE);
+		MADV_MERGEABLE | MADV_HUGEPAGE);
 
 	/* Initialise the virtual GIC. */
 	if (gic__init_irqchip(kvm))
-- 
1.8.3.4

[PATCH 3/4] kvmtool: Fix handling of POLLHUP when --tty is used

[Marc Zyngier]

The --tty option allows the redirection of a console (serial or virtio)
to a pseudo-terminal. As long as the slave port of this pseudo-terminal
is not opened by another process, a poll() call on the master port will
return POLLHUP in the .event field.

This confuses the virtio console code, as term_readable() returns
a positive value, indicating that something is available, while the
call to term_getc_iov will fail.

The fix is to check for the presence of the POLLIN flag in the .event
field. Note that this is only a partial fix, as kvmtool will still
consume vast amounts of CPU resource by spinning like crazy until
the slave port is actually opened.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 tools/kvm/term.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/kvm/term.c b/tools/kvm/term.c
index 5c3e543..214f5e2 100644
--- a/tools/kvm/term.c
+++ b/tools/kvm/term.c
@@ -89,8 +89,10 @@ bool term_readable(int term)
 		.events	= POLLIN,
 		.revents = 0,
 	};
+	int err;
 
-	return poll(&pollfd, 1, 0) > 0;
+	err = poll(&pollfd, 1, 0);
+	return (err > 0 && (pollfd.revents & POLLIN));
 }
 
 static void *term_poll_thread_loop(void *param)
-- 
1.8.3.4

[PATCH 4/4] kvmtool: allow the TAP interface to be specified on the command line

[Marc Zyngier]

In order to overcome the fact that a TAP interface can only be created
by root, allow the use of an interface that has already been created,
configured, made persistent and owned by a specific user/group (such
as done with tunctl).

In this case, any kind of configuration can be skipped (IP, up and
running mode), and the TAP is assumed to be ready for use.

This is done by introducing the "tapif" option, as used here:
	--network trans=mmio,mode=tap,tapif=blah

where "blah" is a TAP interface.

This allow the creation/configuration of the interface to be controlled
by root, and lkvm to be run as a normal user.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 tools/kvm/include/kvm/virtio-net.h |  1 +
 tools/kvm/virtio/net.c             | 21 ++++++++++++++-------
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/tools/kvm/include/kvm/virtio-net.h b/tools/kvm/include/kvm/virtio-net.h
index 0f4d1e5..f435cc3 100644
--- a/tools/kvm/include/kvm/virtio-net.h
+++ b/tools/kvm/include/kvm/virtio-net.h
@@ -10,6 +10,7 @@ struct virtio_net_params {
 	const char *host_ip;
 	const char *script;
 	const char *trans;
+	const char *tapif;
 	char guest_mac[6];
 	char host_mac[6];
 	struct kvm *kvm;
diff --git a/tools/kvm/virtio/net.c b/tools/kvm/virtio/net.c
index dbb4431..82dbb88 100644
--- a/tools/kvm/virtio/net.c
+++ b/tools/kvm/virtio/net.c
@@ -257,6 +257,7 @@ static bool virtio_net__tap_init(struct net_dev *ndev)
 	struct sockaddr_in sin = {0};
 	struct ifreq ifr;
 	const struct virtio_net_params *params = ndev->params;
+	bool skipconf = !!params->tapif;
 
 	/* Did the user already gave us the FD? */
 	if (params->fd) {
@@ -272,6 +273,8 @@ static bool virtio_net__tap_init(struct net_dev *ndev)
 
 	memset(&ifr, 0, sizeof(ifr));
 	ifr.ifr_flags = IFF_TAP | IFF_NO_PI | IFF_VNET_HDR;
+	if (params->tapif)
+		strncpy(ifr.ifr_name, params->tapif, sizeof(ifr.ifr_name));
 	if (ioctl(ndev->tap_fd, TUNSETIFF, &ifr) < 0) {
 		pr_warning("Config tap device error. Are you root?");
 		goto fail;
@@ -308,7 +311,7 @@ static bool virtio_net__tap_init(struct net_dev *ndev)
 				goto fail;
 			}
 		}
-	} else {
+	} else if (!skipconf) {
 		memset(&ifr, 0, sizeof(ifr));
 		strncpy(ifr.ifr_name, ndev->tap_name, sizeof(ndev->tap_name));
 		sin.sin_addr.s_addr = inet_addr(params->host_ip);
@@ -320,12 +323,14 @@ static bool virtio_net__tap_init(struct net_dev *ndev)
 		}
 	}
 
-	memset(&ifr, 0, sizeof(ifr));
-	strncpy(ifr.ifr_name, ndev->tap_name, sizeof(ndev->tap_name));
-	ioctl(sock, SIOCGIFFLAGS, &ifr);
-	ifr.ifr_flags |= IFF_UP | IFF_RUNNING;
-	if (ioctl(sock, SIOCSIFFLAGS, &ifr) < 0)
-		pr_warning("Could not bring tap device up");
+	if (!skipconf) {
+		memset(&ifr, 0, sizeof(ifr));
+		strncpy(ifr.ifr_name, ndev->tap_name, sizeof(ndev->tap_name));
+		ioctl(sock, SIOCGIFFLAGS, &ifr);
+		ifr.ifr_flags |= IFF_UP | IFF_RUNNING;
+		if (ioctl(sock, SIOCSIFFLAGS, &ifr) < 0)
+			pr_warning("Could not bring tap device up");
+	}
 
 	close(sock);
 
@@ -650,6 +655,8 @@ static int set_net_param(struct kvm *kvm, struct virtio_net_params *p,
 		p->host_ip = strdup(val);
 	} else if (strcmp(param, "trans") == 0) {
 		p->trans = strdup(val);
+	} else if (strcmp(param, "tapif") == 0) {
+		p->tapif = strdup(val);
 	} else if (strcmp(param, "vhost") == 0) {
 		p->vhost = atoi(val);
 	} else if (strcmp(param, "fd") == 0) {
-- 
1.8.3.4