[PATCH 1/6] kvm tools: Return error status in lkvm list

[PATCH 1/6] kvm tools: Return error status in lkvm list

[Michael Ellerman]

Currently list always returns 0, even if there was an error. Instead
have it accumulate any errors and return that.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 tools/kvm/builtin-list.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/kvm/builtin-list.c b/tools/kvm/builtin-list.c
index 9299f17..c35be93 100644
--- a/tools/kvm/builtin-list.c
+++ b/tools/kvm/builtin-list.c
@@ -123,7 +123,7 @@ static void parse_setup_options(int argc, const char **argv)
 
 int kvm_cmd_list(int argc, const char **argv, const char *prefix)
 {
-	int r;
+	int status, r;
 
 	parse_setup_options(argc, argv);
 
@@ -133,17 +133,23 @@ int kvm_cmd_list(int argc, const char **argv, const char *prefix)
 	printf("%6s %-20s %s\n", "PID", "NAME", "STATE");
 	printf("------------------------------------\n");
 
+	status = 0;
+
 	if (run) {
 		r = kvm_list_running_instances();
 		if (r < 0)
 			perror("Error listing instances");
+
+		status |= r;
 	}
 
 	if (rootfs) {
 		r = kvm_list_rootfs();
 		if (r < 0)
 			perror("Error listing rootfs");
+
+		status |= r;
 	}
 
-	return 0;
+	return status;
 }
-- 
1.7.10.4

[PATCH 2/6] kvm tools: More error handling in the ipc code

[Michael Ellerman]

Add perror() calls to a couple of exit paths, to ease debugging.

There are also two places where we print "Failed starting IPC thread",
but one is really an epoll failure, so make that obvious.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 tools/kvm/kvm-ipc.c |   17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/tools/kvm/kvm-ipc.c b/tools/kvm/kvm-ipc.c
index bdcc0d1..7897519 100644
--- a/tools/kvm/kvm-ipc.c
+++ b/tools/kvm/kvm-ipc.c
@@ -49,18 +49,25 @@ static int kvm__create_socket(struct kvm *kvm)
 	}
 
 	s = socket(AF_UNIX, SOCK_STREAM, 0);
-	if (s < 0)
+	if (s < 0) {
+		perror("socket");
 		return s;
+	}
+
 	local.sun_family = AF_UNIX;
 	strlcpy(local.sun_path, full_name, sizeof(local.sun_path));
 	len = strlen(local.sun_path) + sizeof(local.sun_family);
 	r = bind(s, (struct sockaddr *)&local, len);
-	if (r < 0)
+	if (r < 0) {
+		perror("bind");
 		goto fail;
+	}
 
 	r = listen(s, 5);
-	if (r < 0)
+	if (r < 0) {
+		perror("listen");
 		goto fail;
+	}
 
 	return s;
 
@@ -430,6 +437,7 @@ int kvm_ipc__init(struct kvm *kvm)
 
 	epoll_fd = epoll_create(KVM_IPC_MAX_MSGS);
 	if (epoll_fd < 0) {
+		perror("epoll_create");
 		ret = epoll_fd;
 		goto err;
 	}
@@ -437,13 +445,14 @@ int kvm_ipc__init(struct kvm *kvm)
 	ev.events = EPOLLIN | EPOLLET;
 	ev.data.fd = sock;
 	if (epoll_ctl(epoll_fd, EPOLL_CTL_ADD, sock, &ev) < 0) {
-		pr_err("Failed starting IPC thread");
+		pr_err("Failed adding socket to epoll");
 		ret = -EFAULT;
 		goto err_epoll;
 	}
 
 	stop_fd = eventfd(0, 0);
 	if (stop_fd < 0) {
+		perror("eventfd");
 		ret = stop_fd;
 		goto err_epoll;
 	}
-- 
1.7.10.4

[PATCH 3/6] kvm tools: Rework stdio/stdout handling to support redirection

[Michael Ellerman]

Currently if you redirect the output from "lkvm run" to a file then
term_init() will fail, because it can't call the terminal ioctls.

So check if stdin and stdout are ttys, if either is not then skip the
rest of the terminal setup. Redirecting one but not the other is a
little odd, but does work.

Note that we skip registering the cleanup routines, so we don't need to
modify them.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 tools/kvm/term.c |   15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/tools/kvm/term.c b/tools/kvm/term.c
index 4413450..fa85e4a 100644
--- a/tools/kvm/term.c
+++ b/tools/kvm/term.c
@@ -140,6 +140,15 @@ int term_init(struct kvm *kvm)
 	struct termios term;
 	int i, r;
 
+	for (i = 0; i < 4; i++)
+		if (term_fds[i][TERM_FD_IN] == 0) {
+			term_fds[i][TERM_FD_IN] = STDIN_FILENO;
+			term_fds[i][TERM_FD_OUT] = STDOUT_FILENO;
+		}
+
+	if (!isatty(STDIN_FILENO) || !isatty(STDOUT_FILENO))
+		return 0;
+
 	r = tcgetattr(STDIN_FILENO, &orig_term);
 	if (r < 0) {
 		pr_warning("unable to save initial standard input settings");
@@ -151,12 +160,6 @@ int term_init(struct kvm *kvm)
 	term.c_lflag &= ~(ICANON | ECHO | ISIG);
 	tcsetattr(STDIN_FILENO, TCSANOW, &term);
 
-	for (i = 0; i < 4; i++)
-		if (term_fds[i][TERM_FD_IN] == 0) {
-			term_fds[i][TERM_FD_IN] = STDIN_FILENO;
-			term_fds[i][TERM_FD_OUT] = STDOUT_FILENO;
-		}
-
 	signal(SIGTERM, term_sig_cleanup);
 	atexit(term_cleanup);
 
-- 
1.7.10.4

[PATCH 4/6] kvm tools: powerpc: Fix buglet in xics_init() handling of nrcpus

[Michael Ellerman]

In xics_init() we set the maximum server to kvm->nrcpus, and then set
the nr_servers using maximum server + 1.

That is off by one, in the harmless direction.

Simplify it to just set nr_servers = kvm->nrcpus.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 tools/kvm/powerpc/xics.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/tools/kvm/powerpc/xics.c b/tools/kvm/powerpc/xics.c
index d4b5caa..cf64a08 100644
--- a/tools/kvm/powerpc/xics.c
+++ b/tools/kvm/powerpc/xics.c
@@ -445,16 +445,13 @@ static void rtas_int_on(struct kvm_cpu *vcpu, uint32_t token,
 
 static int xics_init(struct kvm *kvm)
 {
-	int max_server_num;
 	unsigned int i;
 	struct icp_state *icp;
 	struct ics_state *ics;
 	int j;
 
-	max_server_num = kvm->nrcpus;
-
 	icp = malloc(sizeof(*icp));
-	icp->nr_servers = max_server_num + 1;
+	icp->nr_servers = kvm->nrcpus;
 	icp->ss = malloc(icp->nr_servers * sizeof(struct icp_server_state));
 
 	for (i = 0; i < icp->nr_servers; i++) {
-- 
1.7.10.4

[PATCH 5/6] kvm tools: powerpc: Add cpu info entry for POWER8

[Michael Ellerman]

We should hard-code less of this stuff, but for now this works.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 tools/kvm/powerpc/cpu_info.c |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/tools/kvm/powerpc/cpu_info.c b/tools/kvm/powerpc/cpu_info.c
index 11ca14e..a9dfe39 100644
--- a/tools/kvm/powerpc/cpu_info.c
+++ b/tools/kvm/powerpc/cpu_info.c
@@ -35,6 +35,20 @@ static struct cpu_info cpu_power7_info = {
 	},
 };
 
+/* POWER8 */
+
+static struct cpu_info cpu_power8_info = {
+	.name = "POWER8",
+	.tb_freq = 512000000,
+	.d_bsize = 128,
+	.i_bsize = 128,
+	.flags = CPUINFO_FLAG_DFP | CPUINFO_FLAG_VSX | CPUINFO_FLAG_VMX,
+	.mmu_info = {
+		.flags = KVM_PPC_PAGE_SIZES_REAL | KVM_PPC_1T_SEGMENTS,
+		.slb_size = 32,
+	},
+};
+
 /* PPC970/G5 */
 
 static struct cpu_info cpu_970_info = {
@@ -52,6 +66,7 @@ static struct pvr_info host_pvr_info[] = {
 	{ 0xffffffff, 0x0f000003, &cpu_power7_info },
 	{ 0xffff0000, 0x003f0000, &cpu_power7_info },
 	{ 0xffff0000, 0x004a0000, &cpu_power7_info },
+	{ 0xffff0000, 0x004b0000, &cpu_power8_info },
 	{ 0xffff0000, 0x00390000, &cpu_970_info },
 	{ 0xffff0000, 0x003c0000, &cpu_970_info },
         { 0xffff0000, 0x00440000, &cpu_970_info },
-- 
1.7.10.4

[PATCH 6/6] kvm tools: powerpc: Only emit TB freq if it's non-zero

[Michael Ellerman]

The kernel can handle a missing timebase-frequency property much better
than one that claims zero.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 tools/kvm/powerpc/kvm.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/kvm/powerpc/kvm.c b/tools/kvm/powerpc/kvm.c
index dc9f89d..b4b9f82 100644
--- a/tools/kvm/powerpc/kvm.c
+++ b/tools/kvm/powerpc/kvm.c
@@ -389,7 +389,9 @@ static int setup_fdt(struct kvm *kvm)
 		_FDT(fdt_property_cell(fdt, "dcache-block-size", cpu_info->d_bsize));
 		_FDT(fdt_property_cell(fdt, "icache-block-size", cpu_info->i_bsize));
 
-		_FDT(fdt_property_cell(fdt, "timebase-frequency", cpu_info->tb_freq));
+		if (cpu_info->tb_freq)
+			_FDT(fdt_property_cell(fdt, "timebase-frequency", cpu_info->tb_freq));
+
 		/* Lies, but safeish lies! */
 		_FDT(fdt_property_cell(fdt, "clock-frequency", 0xddbab200));
 
-- 
1.7.10.4

Re: [PATCH 1/6] kvm tools: Return error status in lkvm list

[Pekka Enberg]

Applied all patches, thanks a lot Michael!