[PATCH 00/11] kvm tools: Misc patches (mips support)

[PATCH 00/11] kvm tools: Misc patches (mips support)

[Andreas Herrmann]

Hi,

These patches contain changes that I am currently using on top of
git://github.com/penberg/linux-kvm.git (as of v3.13-rc1-1427-gd9147fb)
to run lkvm on MIPS.

The core is David's work for mips support and laoding elf binaries.

I rebased his stuff, rearranged patches somewhat and split out general
(non-mips-specific) modifications.

I used lkvm to test mips-paravirtualized guest on a host running KVM
with MIPS-VZ (on octeon3). (Patches for this are not upstream yet but
are in the works.)

Comments are welcome.


Thanks,

Andreas

[PATCH 01/11] kvm tools: Print message on failure of KVM_CREATE_VM

[Andreas Herrmann]

From: David Daney <david.daney@cavium.com>

Signed-off-by: David Daney <david.daney@cavium.com>
Signed-off-by: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
---
 tools/kvm/kvm.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c
index d7d2e84..7bd20d3 100644
--- a/tools/kvm/kvm.c
+++ b/tools/kvm/kvm.c
@@ -286,6 +286,7 @@ int kvm__init(struct kvm *kvm)
 
 	kvm->vm_fd = ioctl(kvm->sys_fd, KVM_CREATE_VM, 0);
 	if (kvm->vm_fd < 0) {
+		pr_err("KVM_CREATE_VM ioctl");
 		ret = kvm->vm_fd;
 		goto err_sys_fd;
 	}
-- 
1.7.9.5

[PATCH 02/11] kvm tools: Fix print format warnings

[Andreas Herrmann]

This should fix following warnings

 builtin-stat.c:93:3: warning: format '%llu' expects argument of type 'long long unsigned int', but argument 2 has type '__u64' [-Wformat]
 builtin-run.c:188:4: warning: format '%Lu' expects argument of type 'long long unsigned int', but argument 3 has type '__u64' [-Wformat]
 builtin-run.c:554:3: warning: format '%llu' expects argument of type 'long long unsigned int', but argument 2 has type 'u64' [-Wformat]
 builtin-run.c:554:3: warning: format '%llu' expects argument of type 'long long unsigned int', but argument 3 has type 'u64' [-Wformat]
 builtin-run.c:645:3: warning: format '%Lu' expects argument of type 'long long unsigned int', but argument 4 has type 'u64' [-Wformat]
 disk/core.c:330:4: warning: format '%llu' expects argument of type 'long long unsigned int', but argument 4 has type '__dev_t' [-Wformat]
 disk/core.c:330:4: warning: format '%llu' expects argument of type 'long long unsigned int', but argument 5 has type '__dev_t' [-Wformat]
 disk/core.c:330:4: warning: format '%llu' expects argument of type 'long long unsigned int', but argument 6 has type '__ino64_t' [-Wformat]
 mmio.c:134:5: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 4 has type 'u64' [-Wformat]
 util/util.c:101:7: warning: format '%lld' expects argument of type 'long long int', but argument 3 has type 'u64' [-Wformat]
 util/util.c:113:7: warning: format '%lld' expects argument of type 'long long int', but argument 2 has type 'u64' [-Wformat]
 hw/pci-shmem.c:339:3: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 2 has type 'u64' [-Wformat]
 hw/pci-shmem.c:340:3: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 2 has type 'u64' [-Wformat]

as observed when compiling on mips64.

Signed-off-by: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
---
 tools/kvm/builtin-run.c  |   12 ++++++++----
 tools/kvm/builtin-stat.c |    2 +-
 tools/kvm/disk/core.c    |    4 +++-
 tools/kvm/hw/pci-shmem.c |    5 +++--
 tools/kvm/mmio.c         |    5 +++--
 tools/kvm/util/util.c    |    4 ++--
 6 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index da95d71..1ee75ad 100644
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
@@ -184,8 +184,8 @@ panic_kvm:
 		current_kvm_cpu->kvm_run->exit_reason,
 		kvm_exit_reasons[current_kvm_cpu->kvm_run->exit_reason]);
 	if (current_kvm_cpu->kvm_run->exit_reason == KVM_EXIT_UNKNOWN)
-		fprintf(stderr, "KVM exit code: 0x%Lu\n",
-			current_kvm_cpu->kvm_run->hw.hardware_exit_reason);
+		fprintf(stderr, "KVM exit code: 0x%llu\n",
+			(unsigned long long)current_kvm_cpu->kvm_run->hw.hardware_exit_reason);
 
 	kvm_cpu__set_debug_fd(STDOUT_FILENO);
 	kvm_cpu__show_registers(current_kvm_cpu);
@@ -551,7 +551,9 @@ static struct kvm *kvm_cmd_run_init(int argc, const char **argv)
 		kvm->cfg.ram_size = get_ram_size(kvm->cfg.nrcpus);
 
 	if (kvm->cfg.ram_size > host_ram_size())
-		pr_warning("Guest memory size %lluMB exceeds host physical RAM size %lluMB", kvm->cfg.ram_size, host_ram_size());
+		pr_warning("Guest memory size %lluMB exceeds host physical RAM size %lluMB",
+			(unsigned long long)kvm->cfg.ram_size,
+			(unsigned long long)host_ram_size());
 
 	kvm->cfg.ram_size <<= MB_SHIFT;
 
@@ -639,7 +641,9 @@ static struct kvm *kvm_cmd_run_init(int argc, const char **argv)
 	kvm->cfg.real_cmdline = real_cmdline;
 
 	printf("  # %s run -k %s -m %Lu -c %d --name %s\n", KVM_BINARY_NAME,
-		kvm->cfg.kernel_filename, kvm->cfg.ram_size / 1024 / 1024, kvm->cfg.nrcpus, kvm->cfg.guest_name);
+		kvm->cfg.kernel_filename,
+		(unsigned long long)kvm->cfg.ram_size / 1024 / 1024,
+		kvm->cfg.nrcpus, kvm->cfg.guest_name);
 
 	if (init_list__init(kvm) < 0)
 		die ("Initialisation failed");
diff --git a/tools/kvm/builtin-stat.c b/tools/kvm/builtin-stat.c
index ffd72e8..5d6407e 100644
--- a/tools/kvm/builtin-stat.c
+++ b/tools/kvm/builtin-stat.c
@@ -90,7 +90,7 @@ static int do_memstat(const char *name, int sock)
 			printf("The total amount of memory available (in bytes):");
 			break;
 		}
-		printf("%llu\n", stats[i].val);
+		printf("%llu\n", (unsigned long long)stats[i].val);
 	}
 	printf("\n");
 
diff --git a/tools/kvm/disk/core.c b/tools/kvm/disk/core.c
index 4e9bda0..309e16c 100644
--- a/tools/kvm/disk/core.c
+++ b/tools/kvm/disk/core.c
@@ -327,7 +327,9 @@ ssize_t disk_image__get_serial(struct disk_image *disk, void *buffer, ssize_t *l
 		return r;
 
 	*len = snprintf(buffer, *len, "%llu%llu%llu",
-			(u64)st.st_dev, (u64)st.st_rdev, (u64)st.st_ino);
+			(unsigned long long)st.st_dev,
+			(unsigned long long)st.st_rdev,
+			(unsigned long long)st.st_ino);
 	return *len;
 }
 
diff --git a/tools/kvm/hw/pci-shmem.c b/tools/kvm/hw/pci-shmem.c
index 34de747..d769e432 100644
--- a/tools/kvm/hw/pci-shmem.c
+++ b/tools/kvm/hw/pci-shmem.c
@@ -336,8 +336,9 @@ int shmem_parser(const struct option *opt, const char *arg, int unset)
 		strcpy(handle, default_handle);
 	}
 	if (verbose) {
-		pr_info("shmem: phys_addr = %llx", phys_addr);
-		pr_info("shmem: size      = %llx", size);
+		pr_info("shmem: phys_addr = %llx",
+			(unsigned long long)phys_addr);
+		pr_info("shmem: size      = %llx", (unsigned long long)size);
 		pr_info("shmem: handle    = %s", handle);
 		pr_info("shmem: create    = %d", create);
 	}
diff --git a/tools/kvm/mmio.c b/tools/kvm/mmio.c
index 5d65d28..786c3eb 100644
--- a/tools/kvm/mmio.c
+++ b/tools/kvm/mmio.c
@@ -130,8 +130,9 @@ bool kvm__emulate_mmio(struct kvm *kvm, u64 phys_addr, u8 *data, u32 len, u8 is_
 		mmio->mmio_fn(phys_addr, data, len, is_write, mmio->ptr);
 	else {
 		if (kvm->cfg.mmio_debug)
-			fprintf(stderr, "Warning: Ignoring MMIO %s at %016llx (length %u)\n",
-				to_direction(is_write), phys_addr, len);
+			fprintf(stderr,	"Warning: Ignoring MMIO %s at %016llx (length %u)\n",
+				to_direction(is_write),
+				(unsigned long long)phys_addr, len);
 	}
 	br_read_unlock();
 
diff --git a/tools/kvm/util/util.c b/tools/kvm/util/util.c
index c11a15a..1877105 100644
--- a/tools/kvm/util/util.c
+++ b/tools/kvm/util/util.c
@@ -98,7 +98,7 @@ void *mmap_hugetlbfs(struct kvm *kvm, const char *htlbfs_path, u64 size)
 	blk_size = (unsigned long)sfs.f_bsize;
 	if (sfs.f_bsize == 0 || blk_size > size) {
 		die("Can't use hugetlbfs pagesize %ld for mem size %lld\n",
-		    blk_size, size);
+			blk_size, (unsigned long long)size);
 	}
 
 	kvm->ram_pagesize = blk_size;
@@ -110,7 +110,7 @@ void *mmap_hugetlbfs(struct kvm *kvm, const char *htlbfs_path, u64 size)
 	unlink(mpath);
 	if (ftruncate(fd, size) < 0)
 		die("Can't ftruncate for mem mapping size %lld\n",
-		    size);
+			(unsigned long long)size);
 	addr = mmap(NULL, size, PROT_RW, MAP_PRIVATE, fd, 0);
 	close(fd);
 
-- 
1.7.9.5

[PATCH 03/11] kvm tools: Move definition of TERM_MAX_DEVS to header

[Andreas Herrmann]

In order to use it in other C files (in addition to term.c).

Signed-off-by: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
---
 tools/kvm/include/kvm/term.h |    2 ++
 tools/kvm/term.c             |    1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/kvm/include/kvm/term.h b/tools/kvm/include/kvm/term.h
index 5f63457..dc9882e 100644
--- a/tools/kvm/include/kvm/term.h
+++ b/tools/kvm/include/kvm/term.h
@@ -10,6 +10,8 @@
 #define CONSOLE_VIRTIO	2
 #define CONSOLE_HV	3
 
+#define TERM_MAX_DEVS	4
+
 int term_putc_iov(struct iovec *iov, int iovcnt, int term);
 int term_getc_iov(struct kvm *kvm, struct iovec *iov, int iovcnt, int term);
 int term_putc(char *addr, int cnt, int term);
diff --git a/tools/kvm/term.c b/tools/kvm/term.c
index 214f5e2..3de410b 100644
--- a/tools/kvm/term.c
+++ b/tools/kvm/term.c
@@ -16,7 +16,6 @@
 
 #define TERM_FD_IN      0
 #define TERM_FD_OUT     1
-#define TERM_MAX_DEVS	4
 
 static struct termios	orig_term;
 
-- 
1.7.9.5

[PATCH 04/11] kvm tools: Allow to load ELF binary

[Andreas Herrmann]

Signed-off-by: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
---
 tools/kvm/include/kvm/kvm.h |    1 +
 tools/kvm/kvm.c             |   11 +++++++++++
 2 files changed, 12 insertions(+)

diff --git a/tools/kvm/include/kvm/kvm.h b/tools/kvm/include/kvm/kvm.h
index d05b936..3643fe3 100644
--- a/tools/kvm/include/kvm/kvm.h
+++ b/tools/kvm/include/kvm/kvm.h
@@ -109,6 +109,7 @@ void *guest_flat_to_host(struct kvm *kvm, u64 offset);
 u64 host_to_guest_flat(struct kvm *kvm, void *ptr);
 
 int load_flat_binary(struct kvm *kvm, int fd_kernel, int fd_initrd, const char *kernel_cmdline);
+int load_elf_binary(struct kvm *kvm, int fd_kernel, int fd_initrd, const char *kernel_cmdline);
 bool load_bzimage(struct kvm *kvm, int fd_kernel, int fd_initrd, const char *kernel_cmdline);
 
 /*
diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c
index 7bd20d3..cfc0693 100644
--- a/tools/kvm/kvm.c
+++ b/tools/kvm/kvm.c
@@ -349,6 +349,12 @@ static bool initrd_check(int fd)
 		!memcmp(id, CPIO_MAGIC, 4);
 }
 
+int __attribute__((__weak__)) load_elf_binary(struct kvm *kvm, int fd_kernel,
+				int fd_initrd, const char *kernel_cmdline)
+{
+	return false;
+}
+
 bool kvm__load_kernel(struct kvm *kvm, const char *kernel_filename,
 		const char *initrd_filename, const char *kernel_cmdline)
 {
@@ -375,6 +381,11 @@ bool kvm__load_kernel(struct kvm *kvm, const char *kernel_filename,
 
 	pr_warning("%s is not a bzImage. Trying to load it as a flat binary...", kernel_filename);
 
+	ret = load_elf_binary(kvm, fd_kernel, fd_initrd, kernel_cmdline);
+
+	if (ret)
+		goto found_kernel;
+
 	ret = load_flat_binary(kvm, fd_kernel, fd_initrd, kernel_cmdline);
 
 	if (ret)
-- 
1.7.9.5

[PATCH 05/11] kvm tools, mips: Add MIPS support

[Andreas Herrmann]

From: David Daney <david.daney@cavium.com>

So far this was tested with host running KVM using MIPS-VZ (on Cavium
Octeon3). A paravirtualized mips kernel was used for the guest.

[andreas.herrmann:
   * Renamed kvm__arch_periodic_poll to kvm__arch_read_term
     because of commit fa817d892508b6d3a90f478dbeedbe5583b14da7
     (kvm tools: remove periodic tick in favour of a polling thread)
   * Added ioport__map_irq skeleton to fix build problem.
   * Rely on TERM_MAX_DEVS instead of using other macros
   * Adaptions for MMIO support
   * Set coalesc offset
   * Fixed compile warnings]

Signed-off-by: David Daney <david.daney@cavium.com>
Signed-off-by: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
---
 tools/kvm/mips/include/kvm/barrier.h         |   20 +++
 tools/kvm/mips/include/kvm/kvm-arch.h        |   33 +++++
 tools/kvm/mips/include/kvm/kvm-config-arch.h |    7 +
 tools/kvm/mips/include/kvm/kvm-cpu-arch.h    |   42 ++++++
 tools/kvm/mips/irq.c                         |   10 ++
 tools/kvm/mips/kvm-cpu.c                     |  201 ++++++++++++++++++++++++++
 tools/kvm/mips/kvm.c                         |  134 +++++++++++++++++
 7 files changed, 447 insertions(+)
 create mode 100644 tools/kvm/mips/include/kvm/barrier.h
 create mode 100644 tools/kvm/mips/include/kvm/kvm-arch.h
 create mode 100644 tools/kvm/mips/include/kvm/kvm-config-arch.h
 create mode 100644 tools/kvm/mips/include/kvm/kvm-cpu-arch.h
 create mode 100644 tools/kvm/mips/irq.c
 create mode 100644 tools/kvm/mips/kvm-cpu.c
 create mode 100644 tools/kvm/mips/kvm.c

diff --git a/tools/kvm/mips/include/kvm/barrier.h b/tools/kvm/mips/include/kvm/barrier.h
new file mode 100644
index 0000000..45bfa72
--- /dev/null
+++ b/tools/kvm/mips/include/kvm/barrier.h
@@ -0,0 +1,20 @@
+#ifndef _KVM_BARRIER_H_
+#define _KVM_BARRIER_H_
+
+#define barrier() asm volatile("": : :"memory")
+
+#define mb()	asm volatile (".set push\n\t.set mips2\n\tsync\n\t.set pop": : :"memory")
+#define rmb() mb()
+#define wmb() mb()
+
+#ifdef CONFIG_SMP
+#define smp_mb()	mb()
+#define smp_rmb()	rmb()
+#define smp_wmb()	wmb()
+#else
+#define smp_mb()	barrier()
+#define smp_rmb()	barrier()
+#define smp_wmb()	barrier()
+#endif
+
+#endif /* _KVM_BARRIER_H_ */
diff --git a/tools/kvm/mips/include/kvm/kvm-arch.h b/tools/kvm/mips/include/kvm/kvm-arch.h
new file mode 100644
index 0000000..4a8407b
--- /dev/null
+++ b/tools/kvm/mips/include/kvm/kvm-arch.h
@@ -0,0 +1,33 @@
+#ifndef KVM__KVM_ARCH_H
+#define KVM__KVM_ARCH_H
+
+#define KVM_MMIO_START		0x10000000
+#define KVM_PCI_CFG_AREA	KVM_MMIO_START
+#define KVM_PCI_MMIO_AREA	(KVM_MMIO_START + 0x1000000)
+#define KVM_VIRTIO_MMIO_AREA	(KVM_MMIO_START + 0x2000000)
+
+/*
+ * Just for reference. This and the above corresponds to what's used
+ * in mipsvz_page_fault() in kvm_mipsvz.c of the host kernel.
+ */
+#define KVM_MIPS_IOPORT_AREA	0x1e000000
+#define KVM_MIPS_IOPORT_SIZE	0x00010000
+#define KVM_MIPS_IRQCHIP_AREA	0x1e010000
+#define KVM_MIPS_IRQCHIP_SIZE	0x00010000
+
+#define KVM_IRQ_OFFSET		1
+
+#define VIRTIO_DEFAULT_TRANS(kvm)	VIRTIO_PCI
+
+#include <stdbool.h>
+
+#include "linux/types.h"
+
+struct kvm_arch {
+	u64 entry_point;
+	u64 argc;
+	u64 argv;
+	bool is64bit;
+};
+
+#endif /* KVM__KVM_ARCH_H */
diff --git a/tools/kvm/mips/include/kvm/kvm-config-arch.h b/tools/kvm/mips/include/kvm/kvm-config-arch.h
new file mode 100644
index 0000000..8a28f9d
--- /dev/null
+++ b/tools/kvm/mips/include/kvm/kvm-config-arch.h
@@ -0,0 +1,7 @@
+#ifndef KVM__KVM_CONFIG_ARCH_H
+#define KVM__KVM_CONFIG_ARCH_H
+
+struct kvm_config_arch {
+};
+
+#endif /* KVM__MIPS_KVM_CONFIG_ARCH_H */
diff --git a/tools/kvm/mips/include/kvm/kvm-cpu-arch.h b/tools/kvm/mips/include/kvm/kvm-cpu-arch.h
new file mode 100644
index 0000000..d160837
--- /dev/null
+++ b/tools/kvm/mips/include/kvm/kvm-cpu-arch.h
@@ -0,0 +1,42 @@
+#ifndef KVM__KVM_CPU_ARCH_H
+#define KVM__KVM_CPU_ARCH_H
+
+#include <linux/kvm.h>	/* for struct kvm_regs */
+#include "kvm/kvm.h"	/* for kvm__emulate_{mm}io() */
+#include <pthread.h>
+
+struct kvm;
+
+struct kvm_cpu {
+	pthread_t	thread;		/* VCPU thread */
+
+	unsigned long	cpu_id;
+
+	struct kvm	*kvm;		/* parent KVM */
+	int		vcpu_fd;	/* For VCPU ioctls() */
+	struct kvm_run	*kvm_run;
+
+	struct kvm_regs	regs;
+
+	u8		is_running;
+	u8		paused;
+	u8		needs_nmi;
+
+	struct kvm_coalesced_mmio_ring *ring;
+};
+
+/*
+ * As these are such simple wrappers, let's have them in the header so they'll
+ * be cheaper to call:
+ */
+static inline bool kvm_cpu__emulate_io(struct kvm *kvm, u16 port, void *data, int direction, int size, u32 count)
+{
+	return kvm__emulate_io(kvm, port, data, direction, size, count);
+}
+
+static inline bool kvm_cpu__emulate_mmio(struct kvm *kvm, u64 phys_addr, u8 *data, u32 len, u8 is_write)
+{
+	return kvm__emulate_mmio(kvm, phys_addr, data, len, is_write);
+}
+
+#endif /* KVM__KVM_CPU_ARCH_H */
diff --git a/tools/kvm/mips/irq.c b/tools/kvm/mips/irq.c
new file mode 100644
index 0000000..c1ff6bb
--- /dev/null
+++ b/tools/kvm/mips/irq.c
@@ -0,0 +1,10 @@
+#include "kvm/irq.h"
+#include "kvm/kvm.h"
+
+#include <stdlib.h>
+
+int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg)
+{
+	pr_warning("irq__add_msix_route");
+	return 1;
+}
diff --git a/tools/kvm/mips/kvm-cpu.c b/tools/kvm/mips/kvm-cpu.c
new file mode 100644
index 0000000..0b44d50
--- /dev/null
+++ b/tools/kvm/mips/kvm-cpu.c
@@ -0,0 +1,201 @@
+#include "kvm/kvm-cpu.h"
+#include "kvm/term.h"
+
+#include <stdlib.h>
+
+static int debug_fd;
+
+void kvm_cpu__set_debug_fd(int fd)
+{
+	debug_fd = fd;
+}
+
+int kvm_cpu__get_debug_fd(void)
+{
+	return debug_fd;
+}
+
+void kvm_cpu__delete(struct kvm_cpu *vcpu)
+{
+	free(vcpu);
+}
+
+static struct kvm_cpu *kvm_cpu__new(struct kvm *kvm)
+{
+	struct kvm_cpu *vcpu;
+
+	vcpu = calloc(1, sizeof(*vcpu));
+	if (!vcpu)
+		return NULL;
+
+	vcpu->kvm = kvm;
+
+	return vcpu;
+}
+
+struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id)
+{
+	struct kvm_cpu *vcpu;
+	int mmap_size;
+	int coalesced_offset;
+
+	vcpu = kvm_cpu__new(kvm);
+	if (!vcpu)
+		return NULL;
+
+	vcpu->cpu_id = cpu_id;
+
+	vcpu->vcpu_fd = ioctl(vcpu->kvm->vm_fd, KVM_CREATE_VCPU, cpu_id);
+	if (vcpu->vcpu_fd < 0)
+		die_perror("KVM_CREATE_VCPU ioctl");
+
+	mmap_size = ioctl(vcpu->kvm->sys_fd, KVM_GET_VCPU_MMAP_SIZE, 0);
+	if (mmap_size < 0)
+		die_perror("KVM_GET_VCPU_MMAP_SIZE ioctl");
+
+	vcpu->kvm_run = mmap(NULL, mmap_size, PROT_RW, MAP_SHARED, vcpu->vcpu_fd, 0);
+	if (vcpu->kvm_run == MAP_FAILED)
+		die("unable to mmap vcpu fd");
+
+	vcpu->is_running = true;
+
+	coalesced_offset = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_COALESCED_MMIO);
+	if (coalesced_offset)
+		vcpu->ring = (void *)vcpu->kvm_run + (coalesced_offset * PAGE_SIZE);
+
+	return vcpu;
+}
+
+static void kvm_cpu__setup_regs(struct kvm_cpu *vcpu)
+{
+	uint32_t v;
+	struct kvm_one_reg one_reg;
+
+	memset(&vcpu->regs, 0, sizeof(vcpu->regs));
+	vcpu->regs.pc = vcpu->kvm->arch.entry_point;
+	vcpu->regs.gpr[4] = vcpu->kvm->arch.argc;
+	vcpu->regs.gpr[5] = vcpu->kvm->arch.argv;
+
+	if (ioctl(vcpu->vcpu_fd, KVM_SET_REGS, &vcpu->regs) < 0)
+		die_perror("KVM_SET_REGS failed");
+
+
+	one_reg.id = KVM_REG_MIPS | KVM_REG_SIZE_U32 | (0x10000 + 8 * 12 + 0); /* Status */
+	one_reg.addr = (unsigned long)(uint32_t *)&v;
+	v = 6;
+
+	if (ioctl(vcpu->vcpu_fd, KVM_SET_ONE_REG, &one_reg) < 0)
+		die_perror("KVM_SET_ONE_REG failed");
+}
+
+/**
+ * kvm_cpu__reset_vcpu - reset virtual CPU to a known state
+ */
+void kvm_cpu__reset_vcpu(struct kvm_cpu *vcpu)
+{
+	kvm_cpu__setup_regs(vcpu);
+}
+
+static bool kvm_cpu__hypercall_write_cons(struct kvm_cpu *vcpu)
+{
+	int term = (int)vcpu->kvm_run->hypercall.args[0];
+	u64 addr = vcpu->kvm_run->hypercall.args[1];
+	int len = (int)vcpu->kvm_run->hypercall.args[2];
+	char *host_addr;
+
+	if (term < 0 || term >= TERM_MAX_DEVS) {
+		pr_warning("hypercall_write_cons term out of range <%d>", term);
+		return false;
+	}
+	if (len <= 0) {
+		pr_warning("hypercall_write_cons len out of range <%d>", len);
+		return false;
+	}
+
+	if ((addr & 0xffffffffc0000000ull) == 0xffffffff80000000ull)
+		addr &= 0x1ffffffful; /* Convert KSEG{0,1} to physical. */
+	if ((addr & 0xc000000000000000ull) == 0x8000000000000000ull)
+		addr &= 0x07ffffffffffffffull; /* Convert XKPHYS to pysical */
+
+	host_addr = guest_flat_to_host(vcpu->kvm, addr);
+	if (!host_addr) {
+		pr_warning("hypercall_write_cons unmapped physaddr %llx", (unsigned long long)addr);
+		return false;
+	}
+
+	term_putc(host_addr, len, term);
+
+	return true;
+}
+
+bool kvm_cpu__handle_exit(struct kvm_cpu *vcpu)
+{
+	switch(vcpu->kvm_run->exit_reason) {
+	case KVM_EXIT_HYPERCALL:
+		if (vcpu->kvm_run->hypercall.nr == 0) {
+			return kvm_cpu__hypercall_write_cons(vcpu);
+		} else {
+			pr_warning("KVM_EXIT_HYPERCALL unrecognized call %llu",
+				   (unsigned long long)vcpu->kvm_run->hypercall.nr);
+			return false;
+		}
+	case KVM_EXIT_EXCEPTION:
+	case KVM_EXIT_INTERNAL_ERROR:
+		return false;
+	default:
+		break;
+	}
+	return false;
+}
+
+void kvm_cpu__arch_nmi(struct kvm_cpu *cpu)
+{
+}
+
+void kvm_cpu__show_registers(struct kvm_cpu *vcpu)
+{
+	struct kvm_regs regs;
+
+	if (ioctl(vcpu->vcpu_fd, KVM_GET_REGS, &regs) < 0)
+		die("KVM_GET_REGS failed");
+	dprintf(debug_fd, "\n Registers:\n");
+	dprintf(debug_fd,   " ----------\n");
+	dprintf(debug_fd, "$0   : %016lx %016lx %016lx %016lx\n",
+		(unsigned long)regs.gpr[0], (unsigned long)regs.gpr[1],
+		(unsigned long)regs.gpr[2], (unsigned long)regs.gpr[3]);
+	dprintf(debug_fd, "$4   : %016lx %016lx %016lx %016lx\n",
+		(unsigned long)regs.gpr[4], (unsigned long)regs.gpr[5],
+		(unsigned long)regs.gpr[6], (unsigned long)regs.gpr[7]);
+	dprintf(debug_fd, "$8   : %016lx %016lx %016lx %016lx\n",
+		(unsigned long)regs.gpr[8], (unsigned long)regs.gpr[9],
+		(unsigned long)regs.gpr[10], (unsigned long)regs.gpr[11]);
+	dprintf(debug_fd, "$12  : %016lx %016lx %016lx %016lx\n",
+		(unsigned long)regs.gpr[12], (unsigned long)regs.gpr[13],
+		(unsigned long)regs.gpr[14], (unsigned long)regs.gpr[15]);
+	dprintf(debug_fd, "$16  : %016lx %016lx %016lx %016lx\n",
+		(unsigned long)regs.gpr[16], (unsigned long)regs.gpr[17],
+		(unsigned long)regs.gpr[18], (unsigned long)regs.gpr[19]);
+	dprintf(debug_fd, "$20  : %016lx %016lx %016lx %016lx\n",
+		(unsigned long)regs.gpr[20], (unsigned long)regs.gpr[21],
+		(unsigned long)regs.gpr[22], (unsigned long)regs.gpr[23]);
+	dprintf(debug_fd, "$24  : %016lx %016lx %016lx %016lx\n",
+		(unsigned long)regs.gpr[24], (unsigned long)regs.gpr[25],
+		(unsigned long)regs.gpr[26], (unsigned long)regs.gpr[27]);
+	dprintf(debug_fd, "$28  : %016lx %016lx %016lx %016lx\n",
+		(unsigned long)regs.gpr[28], (unsigned long)regs.gpr[29],
+		(unsigned long)regs.gpr[30], (unsigned long)regs.gpr[31]);
+
+	dprintf(debug_fd, "hi   : %016lx\n", (unsigned long)regs.hi);
+	dprintf(debug_fd, "lo   : %016lx\n", (unsigned long)regs.lo);
+	dprintf(debug_fd, "epc  : %016lx\n", (unsigned long)regs.pc);
+
+	dprintf(debug_fd, "\n");
+}
+
+void kvm_cpu__show_code(struct kvm_cpu *vcpu)
+{
+}
+
+void kvm_cpu__show_page_tables(struct kvm_cpu *vcpu)
+{
+}
diff --git a/tools/kvm/mips/kvm.c b/tools/kvm/mips/kvm.c
new file mode 100644
index 0000000..2b73785
--- /dev/null
+++ b/tools/kvm/mips/kvm.c
@@ -0,0 +1,134 @@
+#include "kvm/kvm.h"
+#include "kvm/ioport.h"
+#include "kvm/virtio-console.h"
+
+#include <linux/kvm.h>
+
+#include <ctype.h>
+#include <unistd.h>
+
+struct kvm_ext kvm_req_ext[] = {
+	{ 0, 0 }
+};
+
+void kvm__arch_read_term(struct kvm *kvm)
+{
+	virtio_console__inject_interrupt(kvm);
+}
+
+void kvm__init_ram(struct kvm *kvm)
+{
+	u64	phys_start, phys_size;
+	void	*host_mem;
+
+	phys_start = 0;
+	phys_size  = kvm->ram_size;
+	host_mem   = kvm->ram_start;
+
+	kvm__register_mem(kvm, phys_start, phys_size, host_mem);
+}
+
+void kvm__arch_delete_ram(struct kvm *kvm)
+{
+	munmap(kvm->ram_start, kvm->ram_size);
+}
+
+void kvm__arch_set_cmdline(char *cmdline, bool video)
+{
+
+}
+
+/* Architecture-specific KVM init */
+void kvm__arch_init(struct kvm *kvm, const char *hugetlbfs_path, u64 ram_size)
+{
+	int ret;
+
+	kvm->ram_start = mmap_anon_or_hugetlbfs(kvm, hugetlbfs_path, ram_size);
+	kvm->ram_size = ram_size;
+
+	if (kvm->ram_start == MAP_FAILED)
+		die("out of memory");
+
+	madvise(kvm->ram_start, kvm->ram_size, MADV_MERGEABLE);
+
+	ret = ioctl(kvm->vm_fd, KVM_CREATE_IRQCHIP);
+	if (ret < 0)
+		die_perror("KVM_CREATE_IRQCHIP ioctl");
+}
+
+void kvm__irq_line(struct kvm *kvm, int irq, int level)
+{
+	struct kvm_irq_level irq_level;
+	int ret;
+
+	irq_level.irq = irq;
+	irq_level.level = level ? 1 : 0;
+
+	ret = ioctl(kvm->vm_fd, KVM_IRQ_LINE, &irq_level);
+	if (ret < 0)
+		die_perror("KVM_IRQ_LINE ioctl");
+}
+
+void kvm__irq_trigger(struct kvm *kvm, int irq)
+{
+	struct kvm_irq_level irq_level;
+	int ret;
+
+	irq_level.irq = irq;
+	irq_level.level = 1;
+
+	ret = ioctl(kvm->vm_fd, KVM_IRQ_LINE, &irq_level);
+	if (ret < 0)
+		die_perror("KVM_IRQ_LINE ioctl");
+}
+
+void ioport__setup_arch(struct kvm *kvm)
+{
+}
+
+bool kvm__arch_cpu_supports_vm(void)
+{
+	return true;
+}
+bool kvm__load_firmware(struct kvm *kvm, const char *firmware_filename)
+{
+	return false;
+}
+int kvm__arch_setup_firmware(struct kvm *kvm)
+{
+	return 0;
+}
+
+/* Load at the 1M point. */
+#define KERNEL_LOAD_ADDR 0x1000000
+int load_flat_binary(struct kvm *kvm, int fd_kernel, int fd_initrd, const char *kernel_cmdline)
+{
+	void *p;
+	void *k_start;
+	int nr;
+
+	if (lseek(fd_kernel, 0, SEEK_SET) < 0)
+		die_perror("lseek");
+
+	p = k_start = guest_flat_to_host(kvm, KERNEL_LOAD_ADDR);
+
+	while ((nr = read(fd_kernel, p, 65536)) > 0)
+		p += nr;
+
+	kvm->arch.is64bit = true;
+	kvm->arch.entry_point = 0xffffffff81000000ull;
+
+	pr_info("Loaded kernel to 0x%x (%ld bytes)", KERNEL_LOAD_ADDR, (long int)(p - k_start));
+
+	return true;
+}
+
+bool load_bzimage(struct kvm *kvm, int fd_kernel, int fd_initrd,
+		  const char *kernel_cmdline)
+{
+	return false;
+}
+
+void ioport__map_irq(u8 *irq)
+{
+}
-- 
1.7.9.5

Re: [PATCH 05/11] kvm tools, mips: Add MIPS support

[James Hogan]

Hi Andreas,

On 06/05/14 16:51, Andreas Herrmann wrote:
> From: David Daney <david.daney@cavium.com>
> 
> So far this was tested with host running KVM using MIPS-VZ (on Cavium
> Octeon3). A paravirtualized mips kernel was used for the guest.
> 
> [andreas.herrmann:
>    * Renamed kvm__arch_periodic_poll to kvm__arch_read_term
>      because of commit fa817d892508b6d3a90f478dbeedbe5583b14da7
>      (kvm tools: remove periodic tick in favour of a polling thread)
>    * Added ioport__map_irq skeleton to fix build problem.
>    * Rely on TERM_MAX_DEVS instead of using other macros
>    * Adaptions for MMIO support
>    * Set coalesc offset
>    * Fixed compile warnings]
> 
> Signed-off-by: David Daney <david.daney@cavium.com>
> Signed-off-by: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>


> +static bool kvm_cpu__hypercall_write_cons(struct kvm_cpu *vcpu)
> +{
> +	int term = (int)vcpu->kvm_run->hypercall.args[0];
> +	u64 addr = vcpu->kvm_run->hypercall.args[1];
> +	int len = (int)vcpu->kvm_run->hypercall.args[2];
> +	char *host_addr;
> +
> +	if (term < 0 || term >= TERM_MAX_DEVS) {
> +		pr_warning("hypercall_write_cons term out of range <%d>", term);
> +		return false;
> +	}
> +	if (len <= 0) {
> +		pr_warning("hypercall_write_cons len out of range <%d>", len);
> +		return false;
> +	}
> +
> +	if ((addr & 0xffffffffc0000000ull) == 0xffffffff80000000ull)
> +		addr &= 0x1ffffffful; /* Convert KSEG{0,1} to physical. */
> +	if ((addr & 0xc000000000000000ull) == 0x8000000000000000ull)
> +		addr &= 0x07ffffffffffffffull; /* Convert XKPHYS to pysical */
> +
> +	host_addr = guest_flat_to_host(vcpu->kvm, addr);
> +	if (!host_addr) {
> +		pr_warning("hypercall_write_cons unmapped physaddr %llx", (unsigned long long)addr);
> +		return false;
> +	}
> +
> +	term_putc(host_addr, len, term);

Does len need to be range checked?

> +void kvm_cpu__show_registers(struct kvm_cpu *vcpu)
> +{
> +	struct kvm_regs regs;
> +
> +	if (ioctl(vcpu->vcpu_fd, KVM_GET_REGS, &regs) < 0)
> +		die("KVM_GET_REGS failed");
> +	dprintf(debug_fd, "\n Registers:\n");
> +	dprintf(debug_fd,   " ----------\n");
> +	dprintf(debug_fd, "$0   : %016lx %016lx %016lx %016lx\n",
> +		(unsigned long)regs.gpr[0], (unsigned long)regs.gpr[1],
> +		(unsigned long)regs.gpr[2], (unsigned long)regs.gpr[3]);

Presumably there's nothing stopping a 32-bit userland from creating a
64-bit guest? If that's the case should this all use unsigned long longs?

> +	dprintf(debug_fd, "$4   : %016lx %016lx %016lx %016lx\n",
> +		(unsigned long)regs.gpr[4], (unsigned long)regs.gpr[5],
> +		(unsigned long)regs.gpr[6], (unsigned long)regs.gpr[7]);
> +	dprintf(debug_fd, "$8   : %016lx %016lx %016lx %016lx\n",
> +		(unsigned long)regs.gpr[8], (unsigned long)regs.gpr[9],
> +		(unsigned long)regs.gpr[10], (unsigned long)regs.gpr[11]);
> +	dprintf(debug_fd, "$12  : %016lx %016lx %016lx %016lx\n",
> +		(unsigned long)regs.gpr[12], (unsigned long)regs.gpr[13],
> +		(unsigned long)regs.gpr[14], (unsigned long)regs.gpr[15]);
> +	dprintf(debug_fd, "$16  : %016lx %016lx %016lx %016lx\n",
> +		(unsigned long)regs.gpr[16], (unsigned long)regs.gpr[17],
> +		(unsigned long)regs.gpr[18], (unsigned long)regs.gpr[19]);
> +	dprintf(debug_fd, "$20  : %016lx %016lx %016lx %016lx\n",
> +		(unsigned long)regs.gpr[20], (unsigned long)regs.gpr[21],
> +		(unsigned long)regs.gpr[22], (unsigned long)regs.gpr[23]);
> +	dprintf(debug_fd, "$24  : %016lx %016lx %016lx %016lx\n",
> +		(unsigned long)regs.gpr[24], (unsigned long)regs.gpr[25],
> +		(unsigned long)regs.gpr[26], (unsigned long)regs.gpr[27]);
> +	dprintf(debug_fd, "$28  : %016lx %016lx %016lx %016lx\n",
> +		(unsigned long)regs.gpr[28], (unsigned long)regs.gpr[29],
> +		(unsigned long)regs.gpr[30], (unsigned long)regs.gpr[31]);
> +
> +	dprintf(debug_fd, "hi   : %016lx\n", (unsigned long)regs.hi);
> +	dprintf(debug_fd, "lo   : %016lx\n", (unsigned long)regs.lo);
> +	dprintf(debug_fd, "epc  : %016lx\n", (unsigned long)regs.pc);
> +
> +	dprintf(debug_fd, "\n");
> +}

Cheers
James

Re: [PATCH 05/11] kvm tools, mips: Add MIPS support

[Andreas Herrmann]

On Fri, May 09, 2014 at 10:15:29PM +0100, James Hogan wrote:
> Hi Andreas,
> 
> On 06/05/14 16:51, Andreas Herrmann wrote:
> > From: David Daney <david.daney@cavium.com>
> > 
> > So far this was tested with host running KVM using MIPS-VZ (on Cavium
> > Octeon3). A paravirtualized mips kernel was used for the guest.
> > 
> > [andreas.herrmann:
> >    * Renamed kvm__arch_periodic_poll to kvm__arch_read_term
> >      because of commit fa817d892508b6d3a90f478dbeedbe5583b14da7
> >      (kvm tools: remove periodic tick in favour of a polling thread)
> >    * Added ioport__map_irq skeleton to fix build problem.
> >    * Rely on TERM_MAX_DEVS instead of using other macros
> >    * Adaptions for MMIO support
> >    * Set coalesc offset
> >    * Fixed compile warnings]
> > 
> > Signed-off-by: David Daney <david.daney@cavium.com>
> > Signed-off-by: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
> 
> 
> > +static bool kvm_cpu__hypercall_write_cons(struct kvm_cpu *vcpu)
> > +{
> > +	int term = (int)vcpu->kvm_run->hypercall.args[0];
> > +	u64 addr = vcpu->kvm_run->hypercall.args[1];
> > +	int len = (int)vcpu->kvm_run->hypercall.args[2];
> > +	char *host_addr;
> > +
> > +	if (term < 0 || term >= TERM_MAX_DEVS) {
> > +		pr_warning("hypercall_write_cons term out of range <%d>", term);
> > +		return false;
> > +	}
> > +	if (len <= 0) {
> > +		pr_warning("hypercall_write_cons len out of range <%d>", len);
> > +		return false;
> > +	}
> > +
> > +	if ((addr & 0xffffffffc0000000ull) == 0xffffffff80000000ull)
> > +		addr &= 0x1ffffffful; /* Convert KSEG{0,1} to physical. */
> > +	if ((addr & 0xc000000000000000ull) == 0x8000000000000000ull)
> > +		addr &= 0x07ffffffffffffffull; /* Convert XKPHYS to pysical */
> > +
> > +	host_addr = guest_flat_to_host(vcpu->kvm, addr);
> > +	if (!host_addr) {
> > +		pr_warning("hypercall_write_cons unmapped physaddr %llx", (unsigned long long)addr);
> > +		return false;
> > +	}
> > +
> > +	term_putc(host_addr, len, term);
> 
> Does len need to be range checked?

len <= 0 is checked above.
I don't think an upper boundery check is required.
term_putc (using write) should be able to handle it.
No?

> > +void kvm_cpu__show_registers(struct kvm_cpu *vcpu)
> > +{
> > +	struct kvm_regs regs;
> > +
> > +	if (ioctl(vcpu->vcpu_fd, KVM_GET_REGS, &regs) < 0)
> > +		die("KVM_GET_REGS failed");
> > +	dprintf(debug_fd, "\n Registers:\n");
> > +	dprintf(debug_fd,   " ----------\n");
> > +	dprintf(debug_fd, "$0   : %016lx %016lx %016lx %016lx\n",
> > +		(unsigned long)regs.gpr[0], (unsigned long)regs.gpr[1],
> > +		(unsigned long)regs.gpr[2], (unsigned long)regs.gpr[3]);
> 
> Presumably there's nothing stopping a 32-bit userland from creating a
> 64-bit guest?

Yes, that can be run.

> If that's the case should this all use unsigned long longs?

... and yes it creates wrong register dump.

Will fix this.

> > +	dprintf(debug_fd, "$4   : %016lx %016lx %016lx %016lx\n",
> > +		(unsigned long)regs.gpr[4], (unsigned long)regs.gpr[5],
> > +		(unsigned long)regs.gpr[6], (unsigned long)regs.gpr[7]);
> > +	dprintf(debug_fd, "$8   : %016lx %016lx %016lx %016lx\n",
> > +		(unsigned long)regs.gpr[8], (unsigned long)regs.gpr[9],
> > +		(unsigned long)regs.gpr[10], (unsigned long)regs.gpr[11]);
> > +	dprintf(debug_fd, "$12  : %016lx %016lx %016lx %016lx\n",
> > +		(unsigned long)regs.gpr[12], (unsigned long)regs.gpr[13],
> > +		(unsigned long)regs.gpr[14], (unsigned long)regs.gpr[15]);
> > +	dprintf(debug_fd, "$16  : %016lx %016lx %016lx %016lx\n",
> > +		(unsigned long)regs.gpr[16], (unsigned long)regs.gpr[17],
> > +		(unsigned long)regs.gpr[18], (unsigned long)regs.gpr[19]);
> > +	dprintf(debug_fd, "$20  : %016lx %016lx %016lx %016lx\n",
> > +		(unsigned long)regs.gpr[20], (unsigned long)regs.gpr[21],
> > +		(unsigned long)regs.gpr[22], (unsigned long)regs.gpr[23]);
> > +	dprintf(debug_fd, "$24  : %016lx %016lx %016lx %016lx\n",
> > +		(unsigned long)regs.gpr[24], (unsigned long)regs.gpr[25],
> > +		(unsigned long)regs.gpr[26], (unsigned long)regs.gpr[27]);
> > +	dprintf(debug_fd, "$28  : %016lx %016lx %016lx %016lx\n",
> > +		(unsigned long)regs.gpr[28], (unsigned long)regs.gpr[29],
> > +		(unsigned long)regs.gpr[30], (unsigned long)regs.gpr[31]);
> > +
> > +	dprintf(debug_fd, "hi   : %016lx\n", (unsigned long)regs.hi);
> > +	dprintf(debug_fd, "lo   : %016lx\n", (unsigned long)regs.lo);
> > +	dprintf(debug_fd, "epc  : %016lx\n", (unsigned long)regs.pc);
> > +
> > +	dprintf(debug_fd, "\n");
> > +}
> 
> Cheers
> James

Thanks,
Andreas

Re: [PATCH 05/11] kvm tools, mips: Add MIPS support

[James Hogan]

Hi Andreas,

On 12/05/14 14:01, Andreas Herrmann wrote:
> On Fri, May 09, 2014 at 10:15:29PM +0100, James Hogan wrote:
>> On 06/05/14 16:51, Andreas Herrmann wrote:
>>> +static bool kvm_cpu__hypercall_write_cons(struct kvm_cpu *vcpu)
>>> +{
>>> +	int term = (int)vcpu->kvm_run->hypercall.args[0];
>>> +	u64 addr = vcpu->kvm_run->hypercall.args[1];
>>> +	int len = (int)vcpu->kvm_run->hypercall.args[2];
>>> +	char *host_addr;
>>> +
>>> +	if (term < 0 || term >= TERM_MAX_DEVS) {
>>> +		pr_warning("hypercall_write_cons term out of range <%d>", term);
>>> +		return false;
>>> +	}
>>> +	if (len <= 0) {
>>> +		pr_warning("hypercall_write_cons len out of range <%d>", len);
>>> +		return false;
>>> +	}
>>> +
>>> +	if ((addr & 0xffffffffc0000000ull) == 0xffffffff80000000ull)
>>> +		addr &= 0x1ffffffful; /* Convert KSEG{0,1} to physical. */
>>> +	if ((addr & 0xc000000000000000ull) == 0x8000000000000000ull)
>>> +		addr &= 0x07ffffffffffffffull; /* Convert XKPHYS to pysical */
>>> +
>>> +	host_addr = guest_flat_to_host(vcpu->kvm, addr);
>>> +	if (!host_addr) {
>>> +		pr_warning("hypercall_write_cons unmapped physaddr %llx", (unsigned long long)addr);
>>> +		return false;
>>> +	}
>>> +
>>> +	term_putc(host_addr, len, term);
>>
>> Does len need to be range checked?
> 
> len <= 0 is checked above.
> I don't think an upper boundery check is required.
> term_putc (using write) should be able to handle it.
> No?

Well it looks to me from my naive look at the code (my experience with
tools/kvm/ is pretty much just reading some of the code after looking at
this patchset) like the guest could provide a very large positive len
argument and overflow the host_addr of the memory bank, possibly reading
into other userspace memory which would then get written to the console.
Yes, if it's unmapped the kernel will detect it so it's not so bad (no
seg faults). I guess it all depends how any memory that is passed to
kvm__register_mem was allocated. mmap_anon_or_hugetlbfs may use mmap
which leaves the possibility open of another virtual mapping being
created immediately after it.

AFAICT the best way to avoid that is probably to somehow extend
guest_flat_to_host to provide the address limit too so the provided
length can be checked/clipped, or maybe call it for the end address too
to check the full range is valid and belongs to the same mapping,
although that's a bit more of a hack and technically isn't watertight!

Maybe I'm being paranoid though :)

Cheers
James

Re: [PATCH 05/11] kvm tools, mips: Add MIPS support

[Andreas Herrmann]

On Mon, May 12, 2014 at 03:09:58PM +0100, James Hogan wrote:
> Hi Andreas,
> 
> On 12/05/14 14:01, Andreas Herrmann wrote:
> > On Fri, May 09, 2014 at 10:15:29PM +0100, James Hogan wrote:
> >> On 06/05/14 16:51, Andreas Herrmann wrote:
> >>> +static bool kvm_cpu__hypercall_write_cons(struct kvm_cpu *vcpu)
> >>> +{
> >>> +	int term = (int)vcpu->kvm_run->hypercall.args[0];
> >>> +	u64 addr = vcpu->kvm_run->hypercall.args[1];
> >>> +	int len = (int)vcpu->kvm_run->hypercall.args[2];
> >>> +	char *host_addr;
> >>> +
> >>> +	if (term < 0 || term >= TERM_MAX_DEVS) {
> >>> +		pr_warning("hypercall_write_cons term out of range <%d>", term);
> >>> +		return false;
> >>> +	}
> >>> +	if (len <= 0) {
> >>> +		pr_warning("hypercall_write_cons len out of range <%d>", len);
> >>> +		return false;
> >>> +	}
> >>> +
> >>> +	if ((addr & 0xffffffffc0000000ull) == 0xffffffff80000000ull)
> >>> +		addr &= 0x1ffffffful; /* Convert KSEG{0,1} to physical. */
> >>> +	if ((addr & 0xc000000000000000ull) == 0x8000000000000000ull)
> >>> +		addr &= 0x07ffffffffffffffull; /* Convert XKPHYS to pysical */
> >>> +
> >>> +	host_addr = guest_flat_to_host(vcpu->kvm, addr);
> >>> +	if (!host_addr) {
> >>> +		pr_warning("hypercall_write_cons unmapped physaddr %llx", (unsigned long long)addr);
> >>> +		return false;
> >>> +	}
> >>> +
> >>> +	term_putc(host_addr, len, term);
> >>
> >> Does len need to be range checked?
> > 
> > len <= 0 is checked above.
> > I don't think an upper boundery check is required.
> > term_putc (using write) should be able to handle it.
> > No?
> 
> Well it looks to me from my naive look at the code (my experience with
> tools/kvm/ is pretty much just reading some of the code after looking at
> this patchset) like the guest could provide a very large positive len
> argument and overflow the host_addr of the memory bank, possibly reading
> into other userspace memory which would then get written to the console.
> Yes, if it's unmapped the kernel will detect it so it's not so bad (no
> seg faults). I guess it all depends how any memory that is passed to
> kvm__register_mem was allocated. mmap_anon_or_hugetlbfs may use mmap
> which leaves the possibility open of another virtual mapping being
> created immediately after it.
> 
> AFAICT the best way to avoid that is probably to somehow extend
> guest_flat_to_host to provide the address limit too so the provided
> length can be checked/clipped, or maybe call it for the end address too
> to check the full range is valid and belongs to the same mapping,
> although that's a bit more of a hack and technically isn't watertight!
> 
> Maybe I'm being paranoid though :)

I aggree that also the upper bound should be checked.

I think extending the len check with something like

 "|| !host_ptr_in_ram(vcpu->kvm,host_addr + len)"

should do it.


Thanks,
Andreas

[PATCH 06/11] kvm tools, mips: Enable build of mips support

[Andreas Herrmann]

From: David Daney <david.daney@cavium.com>

Signed-off-by: David Daney <david.daney@cavium.com>
Signed-off-by: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
---
 tools/kvm/Makefile |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/kvm/Makefile b/tools/kvm/Makefile
index b872651..91286ad 100644
--- a/tools/kvm/Makefile
+++ b/tools/kvm/Makefile
@@ -105,7 +105,7 @@ OBJS	+= virtio/mmio.o
 
 # Translate uname -m into ARCH string
 ARCH ?= $(shell uname -m | sed -e s/i.86/i386/ -e s/ppc.*/powerpc/ \
-	  -e s/armv7.*/arm/ -e s/aarch64.*/arm64/)
+	  -e s/armv7.*/arm/ -e s/aarch64.*/arm64/ -e s/mips64/mips/)
 
 ifeq ($(ARCH),i386)
 	ARCH         := x86
@@ -184,6 +184,15 @@ ifeq ($(ARCH), arm64)
 	ARCH_WANT_LIBFDT := y
 endif
 
+ifeq ($(ARCH),mips)
+	DEFINES		+= -DCONFIG_MIPS
+	ARCH_INCLUDE	:= mips/include
+	CFLAGS		+= -I../../arch/mips/include/asm/mach-cavium-octeon
+	CFLAGS		+= -I../../arch/mips/include/asm/mach-generic
+	OBJS		+= mips/kvm.o
+	OBJS		+= mips/kvm-cpu.o
+	OBJS		+= mips/irq.o
+endif
 ###
 
 ifeq (,$(ARCH_INCLUDE))
-- 
1.7.9.5

Re: [PATCH 06/11] kvm tools, mips: Enable build of mips support

[James Hogan]

Hi Andreas,

On 06/05/14 16:51, Andreas Herrmann wrote:
> From: David Daney <david.daney@cavium.com>
> 
> Signed-off-by: David Daney <david.daney@cavium.com>
> Signed-off-by: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
> ---
>  tools/kvm/Makefile |   11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/kvm/Makefile b/tools/kvm/Makefile
> index b872651..91286ad 100644
> --- a/tools/kvm/Makefile
> +++ b/tools/kvm/Makefile
> @@ -105,7 +105,7 @@ OBJS	+= virtio/mmio.o
>  
>  # Translate uname -m into ARCH string
>  ARCH ?= $(shell uname -m | sed -e s/i.86/i386/ -e s/ppc.*/powerpc/ \
> -	  -e s/armv7.*/arm/ -e s/aarch64.*/arm64/)
> +	  -e s/armv7.*/arm/ -e s/aarch64.*/arm64/ -e s/mips64/mips/)
>  
>  ifeq ($(ARCH),i386)
>  	ARCH         := x86
> @@ -184,6 +184,15 @@ ifeq ($(ARCH), arm64)
>  	ARCH_WANT_LIBFDT := y
>  endif
>  
> +ifeq ($(ARCH),mips)
> +	DEFINES		+= -DCONFIG_MIPS
> +	ARCH_INCLUDE	:= mips/include
> +	CFLAGS		+= -I../../arch/mips/include/asm/mach-cavium-octeon
> +	CFLAGS		+= -I../../arch/mips/include/asm/mach-generic

I can't see any obvious includes from these two directories in the
previous patch. Are there any?

> +	OBJS		+= mips/kvm.o
> +	OBJS		+= mips/kvm-cpu.o
> +	OBJS		+= mips/irq.o
> +endif
>  ###
>  
>  ifeq (,$(ARCH_INCLUDE))
> 

Cheers
James

Re: [PATCH 06/11] kvm tools, mips: Enable build of mips support

[Andreas Herrmann]

On Fri, May 09, 2014 at 10:22:15PM +0100, James Hogan wrote:
> Hi Andreas,
> 
> On 06/05/14 16:51, Andreas Herrmann wrote:
> > From: David Daney <david.daney@cavium.com>
> > 
> > Signed-off-by: David Daney <david.daney@cavium.com>
> > Signed-off-by: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
> > ---
> >  tools/kvm/Makefile |   11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/kvm/Makefile b/tools/kvm/Makefile
> > index b872651..91286ad 100644
> > --- a/tools/kvm/Makefile
> > +++ b/tools/kvm/Makefile
> > @@ -105,7 +105,7 @@ OBJS	+= virtio/mmio.o
> >  
> >  # Translate uname -m into ARCH string
> >  ARCH ?= $(shell uname -m | sed -e s/i.86/i386/ -e s/ppc.*/powerpc/ \
> > -	  -e s/armv7.*/arm/ -e s/aarch64.*/arm64/)
> > +	  -e s/armv7.*/arm/ -e s/aarch64.*/arm64/ -e s/mips64/mips/)
> >  
> >  ifeq ($(ARCH),i386)
> >  	ARCH         := x86
> > @@ -184,6 +184,15 @@ ifeq ($(ARCH), arm64)
> >  	ARCH_WANT_LIBFDT := y
> >  endif
> >  
> > +ifeq ($(ARCH),mips)
> > +	DEFINES		+= -DCONFIG_MIPS
> > +	ARCH_INCLUDE	:= mips/include
> > +	CFLAGS		+= -I../../arch/mips/include/asm/mach-cavium-octeon
> > +	CFLAGS		+= -I../../arch/mips/include/asm/mach-generic
> 
> I can't see any obvious includes from these two directories in the
> previous patch. Are there any?

No, there aren't.
Both lines should be removed.

> > +	OBJS		+= mips/kvm.o
> > +	OBJS		+= mips/kvm-cpu.o
> > +	OBJS		+= mips/irq.o
> > +endif
> >  ###
> >  
> >  ifeq (,$(ARCH_INCLUDE))
> > 
> 
> Cheers
> James


Thanks,
Andreas

[PATCH 07/11] kvm tools: Provide per arch macro to specify type for KVM_CREATE_VM

[Andreas Herrmann]

This is is usually 0 for most archs. On mips we have two types.
TE (type 0) and MIPS-VZ (type 1). Default to 1 on mips.

Signed-off-by: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
---
 tools/kvm/arm/include/arm-common/kvm-arch.h |    2 ++
 tools/kvm/kvm.c                             |    2 +-
 tools/kvm/mips/include/kvm/kvm-arch.h       |    2 ++
 tools/kvm/powerpc/include/kvm/kvm-arch.h    |    2 ++
 tools/kvm/x86/include/kvm/kvm-arch.h        |    2 ++
 5 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/kvm/arm/include/arm-common/kvm-arch.h b/tools/kvm/arm/include/arm-common/kvm-arch.h
index b6c4bf8..a552163 100644
--- a/tools/kvm/arm/include/arm-common/kvm-arch.h
+++ b/tools/kvm/arm/include/arm-common/kvm-arch.h
@@ -32,6 +32,8 @@
 
 #define KVM_IRQ_OFFSET		GIC_SPI_IRQ_BASE
 
+#define KVM_VM_TYPE		0
+
 #define VIRTIO_DEFAULT_TRANS(kvm)	\
 	((kvm)->cfg.arch.virtio_trans_pci ? VIRTIO_PCI : VIRTIO_MMIO)
 
diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c
index cfc0693..278b915 100644
--- a/tools/kvm/kvm.c
+++ b/tools/kvm/kvm.c
@@ -284,7 +284,7 @@ int kvm__init(struct kvm *kvm)
 		goto err_sys_fd;
 	}
 
-	kvm->vm_fd = ioctl(kvm->sys_fd, KVM_CREATE_VM, 0);
+	kvm->vm_fd = ioctl(kvm->sys_fd, KVM_CREATE_VM, KVM_VM_TYPE);
 	if (kvm->vm_fd < 0) {
 		pr_err("KVM_CREATE_VM ioctl");
 		ret = kvm->vm_fd;
diff --git a/tools/kvm/mips/include/kvm/kvm-arch.h b/tools/kvm/mips/include/kvm/kvm-arch.h
index 4a8407b..0210f0b 100644
--- a/tools/kvm/mips/include/kvm/kvm-arch.h
+++ b/tools/kvm/mips/include/kvm/kvm-arch.h
@@ -17,6 +17,8 @@
 
 #define KVM_IRQ_OFFSET		1
 
+#define KVM_VM_TYPE		1
+
 #define VIRTIO_DEFAULT_TRANS(kvm)	VIRTIO_PCI
 
 #include <stdbool.h>
diff --git a/tools/kvm/powerpc/include/kvm/kvm-arch.h b/tools/kvm/powerpc/include/kvm/kvm-arch.h
index f8627a2..fdd518f 100644
--- a/tools/kvm/powerpc/include/kvm/kvm-arch.h
+++ b/tools/kvm/powerpc/include/kvm/kvm-arch.h
@@ -44,6 +44,8 @@
 
 #define KVM_IRQ_OFFSET			16
 
+#define KVM_VM_TYPE			0
+
 #define VIRTIO_DEFAULT_TRANS(kvm)	VIRTIO_PCI
 
 struct spapr_phb;
diff --git a/tools/kvm/x86/include/kvm/kvm-arch.h b/tools/kvm/x86/include/kvm/kvm-arch.h
index a9f23b8..673bdf1 100644
--- a/tools/kvm/x86/include/kvm/kvm-arch.h
+++ b/tools/kvm/x86/include/kvm/kvm-arch.h
@@ -27,6 +27,8 @@
 
 #define KVM_IRQ_OFFSET		5
 
+#define KVM_VM_TYPE		0
+
 #define VIRTIO_DEFAULT_TRANS(kvm)	VIRTIO_PCI
 
 struct kvm_arch {
-- 
1.7.9.5

Re: [PATCH 07/11] kvm tools: Provide per arch macro to specify type for KVM_CREATE_VM

[James Hogan]

Hi Andreas,

On 06/05/14 16:51, Andreas Herrmann wrote:
> This is is usually 0 for most archs. On mips we have two types.
> TE (type 0) and MIPS-VZ (type 1). Default to 1 on mips.
> 
> Signed-off-by: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
> ---
>  tools/kvm/arm/include/arm-common/kvm-arch.h |    2 ++
>  tools/kvm/kvm.c                             |    2 +-
>  tools/kvm/mips/include/kvm/kvm-arch.h       |    2 ++
>  tools/kvm/powerpc/include/kvm/kvm-arch.h    |    2 ++
>  tools/kvm/x86/include/kvm/kvm-arch.h        |    2 ++
>  5 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/kvm/arm/include/arm-common/kvm-arch.h b/tools/kvm/arm/include/arm-common/kvm-arch.h
> index b6c4bf8..a552163 100644
> --- a/tools/kvm/arm/include/arm-common/kvm-arch.h
> +++ b/tools/kvm/arm/include/arm-common/kvm-arch.h
> @@ -32,6 +32,8 @@
>  
>  #define KVM_IRQ_OFFSET		GIC_SPI_IRQ_BASE
>  
> +#define KVM_VM_TYPE		0
> +
>  #define VIRTIO_DEFAULT_TRANS(kvm)	\
>  	((kvm)->cfg.arch.virtio_trans_pci ? VIRTIO_PCI : VIRTIO_MMIO)
>  
> diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c
> index cfc0693..278b915 100644
> --- a/tools/kvm/kvm.c
> +++ b/tools/kvm/kvm.c
> @@ -284,7 +284,7 @@ int kvm__init(struct kvm *kvm)
>  		goto err_sys_fd;
>  	}
>  
> -	kvm->vm_fd = ioctl(kvm->sys_fd, KVM_CREATE_VM, 0);
> +	kvm->vm_fd = ioctl(kvm->sys_fd, KVM_CREATE_VM, KVM_VM_TYPE);
>  	if (kvm->vm_fd < 0) {
>  		pr_err("KVM_CREATE_VM ioctl");
>  		ret = kvm->vm_fd;
> diff --git a/tools/kvm/mips/include/kvm/kvm-arch.h b/tools/kvm/mips/include/kvm/kvm-arch.h
> index 4a8407b..0210f0b 100644
> --- a/tools/kvm/mips/include/kvm/kvm-arch.h
> +++ b/tools/kvm/mips/include/kvm/kvm-arch.h
> @@ -17,6 +17,8 @@
>  
>  #define KVM_IRQ_OFFSET		1
>  
> +#define KVM_VM_TYPE		1

A comment or define to clarify this wouldn't hurt.

Cheers
James

Re: [PATCH 07/11] kvm tools: Provide per arch macro to specify type for KVM_CREATE_VM

[Andreas Herrmann]

On Fri, May 09, 2014 at 10:34:30PM +0100, James Hogan wrote:
> Hi Andreas,
> 
> On 06/05/14 16:51, Andreas Herrmann wrote:
> > This is is usually 0 for most archs. On mips we have two types.
> > TE (type 0) and MIPS-VZ (type 1). Default to 1 on mips.
> > 
> > Signed-off-by: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
> > ---
> >  tools/kvm/arm/include/arm-common/kvm-arch.h |    2 ++
> >  tools/kvm/kvm.c                             |    2 +-
> >  tools/kvm/mips/include/kvm/kvm-arch.h       |    2 ++
> >  tools/kvm/powerpc/include/kvm/kvm-arch.h    |    2 ++
> >  tools/kvm/x86/include/kvm/kvm-arch.h        |    2 ++
> >  5 files changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/kvm/arm/include/arm-common/kvm-arch.h b/tools/kvm/arm/include/arm-common/kvm-arch.h
> > index b6c4bf8..a552163 100644
> > --- a/tools/kvm/arm/include/arm-common/kvm-arch.h
> > +++ b/tools/kvm/arm/include/arm-common/kvm-arch.h
> > @@ -32,6 +32,8 @@
> >  
> >  #define KVM_IRQ_OFFSET		GIC_SPI_IRQ_BASE
> >  
> > +#define KVM_VM_TYPE		0
> > +
> >  #define VIRTIO_DEFAULT_TRANS(kvm)	\
> >  	((kvm)->cfg.arch.virtio_trans_pci ? VIRTIO_PCI : VIRTIO_MMIO)
> >  
> > diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c
> > index cfc0693..278b915 100644
> > --- a/tools/kvm/kvm.c
> > +++ b/tools/kvm/kvm.c
> > @@ -284,7 +284,7 @@ int kvm__init(struct kvm *kvm)
> >  		goto err_sys_fd;
> >  	}
> >  
> > -	kvm->vm_fd = ioctl(kvm->sys_fd, KVM_CREATE_VM, 0);
> > +	kvm->vm_fd = ioctl(kvm->sys_fd, KVM_CREATE_VM, KVM_VM_TYPE);
> >  	if (kvm->vm_fd < 0) {
> >  		pr_err("KVM_CREATE_VM ioctl");
> >  		ret = kvm->vm_fd;
> > diff --git a/tools/kvm/mips/include/kvm/kvm-arch.h b/tools/kvm/mips/include/kvm/kvm-arch.h
> > index 4a8407b..0210f0b 100644
> > --- a/tools/kvm/mips/include/kvm/kvm-arch.h
> > +++ b/tools/kvm/mips/include/kvm/kvm-arch.h
> > @@ -17,6 +17,8 @@
> >  
> >  #define KVM_IRQ_OFFSET		1
> >  
> > +#define KVM_VM_TYPE		1
> 
> A comment or define to clarify this wouldn't hurt.

Ok.

Thanks,
Andreas

[PATCH 08/11] kvm tools: Handle virtio/pci I/O space as little endian.

[Andreas Herrmann]

From: David Daney <david.daney@cavium.com>

It doesn't work on big endian hosts as is.

Signed-off-by: David Daney <david.daney@cavium.com>
Signed-off-by: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
---
 tools/kvm/pci.c        |   16 +++++++++++++---
 tools/kvm/virtio/pci.c |    6 +++---
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/tools/kvm/pci.c b/tools/kvm/pci.c
index c2da152..e4857b9 100644
--- a/tools/kvm/pci.c
+++ b/tools/kvm/pci.c
@@ -10,7 +10,7 @@
 
 #define PCI_BAR_OFFSET(b)		(offsetof(struct pci_device_header, bar[b]))
 
-static union pci_config_address		pci_config_address;
+static u32 pci_config_address_bits;
 
 /* This is within our PCI gap - in an unused area.
  * Note this is a PCI *bus address*, is used to assign BARs etc.!
@@ -49,7 +49,7 @@ static void *pci_config_address_ptr(u16 port)
 	void *base;
 
 	offset	= port - PCI_CONFIG_ADDRESS;
-	base	= &pci_config_address;
+	base	= &pci_config_address_bits;
 
 	return base + offset;
 }
@@ -79,6 +79,10 @@ static struct ioport_operations pci_config_address_ops = {
 
 static bool pci_device_exists(u8 bus_number, u8 device_number, u8 function_number)
 {
+	union pci_config_address pci_config_address;
+
+	pci_config_address.w = ioport__read32(&pci_config_address_bits);
+
 	if (pci_config_address.bus_number != bus_number)
 		return false;
 
@@ -90,6 +94,9 @@ static bool pci_device_exists(u8 bus_number, u8 device_number, u8 function_numbe
 
 static bool pci_config_data_out(struct ioport *ioport, struct kvm *kvm, u16 port, void *data, int size)
 {
+	union pci_config_address pci_config_address;
+
+	pci_config_address.w = ioport__read32(&pci_config_address_bits);
 	/*
 	 * If someone accesses PCI configuration space offsets that are not
 	 * aligned to 4 bytes, it uses ioports to signify that.
@@ -103,6 +110,9 @@ static bool pci_config_data_out(struct ioport *ioport, struct kvm *kvm, u16 port
 
 static bool pci_config_data_in(struct ioport *ioport, struct kvm *kvm, u16 port, void *data, int size)
 {
+	union pci_config_address pci_config_address;
+
+	pci_config_address.w = ioport__read32(&pci_config_address_bits);
 	/*
 	 * If someone accesses PCI configuration space offsets that are not
 	 * aligned to 4 bytes, it uses ioports to signify that.
@@ -133,7 +143,7 @@ void pci__config_wr(struct kvm *kvm, union pci_config_address addr, void *data,
 			void *p = device__find_dev(DEVICE_BUS_PCI, dev_num)->data;
 			struct pci_device_header *hdr = p;
 			u8 bar = (offset - PCI_BAR_OFFSET(0)) / (sizeof(u32));
-			u32 sz = PCI_IO_SIZE;
+			u32 sz = cpu_to_le32(PCI_IO_SIZE);
 
 			if (bar < 6 && hdr->bar_size[bar])
 				sz = hdr->bar_size[bar];
diff --git a/tools/kvm/virtio/pci.c b/tools/kvm/virtio/pci.c
index 665d492..f0ae8d4 100644
--- a/tools/kvm/virtio/pci.c
+++ b/tools/kvm/virtio/pci.c
@@ -376,9 +376,9 @@ int virtio_pci__init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
 							| PCI_BASE_ADDRESS_SPACE_MEMORY),
 		.status			= cpu_to_le16(PCI_STATUS_CAP_LIST),
 		.capabilities		= (void *)&vpci->pci_hdr.msix - (void *)&vpci->pci_hdr,
-		.bar_size[0]		= IOPORT_SIZE,
-		.bar_size[1]		= IOPORT_SIZE,
-		.bar_size[2]		= PCI_IO_SIZE * 2,
+		.bar_size[0]		= cpu_to_le32(IOPORT_SIZE),
+		.bar_size[1]		= cpu_to_le32(IOPORT_SIZE),
+		.bar_size[2]		= cpu_to_le32(PCI_IO_SIZE*2),
 	};
 
 	vpci->dev_hdr = (struct device_header) {
-- 
1.7.9.5

[PATCH 09/11] kvm tools, mips: Add support for loading elf binaries

[Andreas Herrmann]

From: David Daney <david.daney@cavium.com>

[andreas.herrmann:
       * Fixed compile warnings]

Signed-off-by: David Daney <david.daney@cavium.com>
Signed-off-by: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
---
 tools/kvm/mips/kvm.c |  200 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 200 insertions(+)

diff --git a/tools/kvm/mips/kvm.c b/tools/kvm/mips/kvm.c
index 2b73785..09192c8 100644
--- a/tools/kvm/mips/kvm.c
+++ b/tools/kvm/mips/kvm.c
@@ -6,6 +6,7 @@
 
 #include <ctype.h>
 #include <unistd.h>
+#include <elf.h>
 
 struct kvm_ext kvm_req_ext[] = {
 	{ 0, 0 }
@@ -99,6 +100,43 @@ int kvm__arch_setup_firmware(struct kvm *kvm)
 	return 0;
 }
 
+static void kvm__mips_install_cmdline(struct kvm *kvm)
+{
+	char *p = kvm->ram_start;
+	u64 cmdline_offset = 0x2000;
+	u64 argv_start = 0x3000;
+	u64 argv_offset = argv_start;
+	u64 argc = 0;
+
+	sprintf(p + cmdline_offset, "mem=0x%llx@0 ",
+		 (unsigned long long)kvm->ram_size);
+
+	strcat(p + cmdline_offset, kvm->cfg.real_cmdline); /* maximum size is 2K */
+
+	while (p[cmdline_offset]) {
+		if (!isspace(p[cmdline_offset])) {
+			if (kvm->arch.is64bit) {
+				*(u64 *)(p + argv_offset) = 0xffffffff80000000ull + cmdline_offset;
+				argv_offset += sizeof(u64);
+			} else {
+				*(u32 *)(p + argv_offset) = 0x80000000u + cmdline_offset;
+				argv_offset += sizeof(u32);
+			}
+			argc++;
+			while(p[cmdline_offset] && !isspace(p[cmdline_offset]))
+				cmdline_offset++;
+			continue;
+		}
+		/* Must be a space character skip over these*/
+		while(p[cmdline_offset] && isspace(p[cmdline_offset])) {
+			p[cmdline_offset] = 0;
+			cmdline_offset++;
+		}
+	}
+	kvm->arch.argc = argc;
+	kvm->arch.argv = 0xffffffff80000000ull + argv_start;
+}
+
 /* Load at the 1M point. */
 #define KERNEL_LOAD_ADDR 0x1000000
 int load_flat_binary(struct kvm *kvm, int fd_kernel, int fd_initrd, const char *kernel_cmdline)
@@ -123,6 +161,168 @@ int load_flat_binary(struct kvm *kvm, int fd_kernel, int fd_initrd, const char *
 	return true;
 }
 
+struct kvm__arch_elf_info {
+	u64 load_addr;
+	u64 entry_point;
+	size_t len;
+	size_t offset;
+};
+
+static bool kvm__arch_get_elf_64_info(Elf64_Ehdr *ehdr, int fd_kernel,
+				      struct kvm__arch_elf_info *ei)
+{
+	int i;
+	size_t nr;
+	Elf64_Phdr phdr;
+
+	if (ehdr->e_phentsize != sizeof(phdr)) {
+		pr_info("Incompatible ELF PHENTSIZE %d", ehdr->e_phentsize);
+		return false;
+	}
+
+	ei->entry_point = ehdr->e_entry;
+
+	if (lseek(fd_kernel, ehdr->e_phoff, SEEK_SET) < 0)
+		die_perror("lseek");
+
+	phdr.p_type = PT_NULL;
+	for (i = 0; i < ehdr->e_phnum; i++) {
+		nr = read(fd_kernel, &phdr, sizeof(phdr));
+		if (nr != sizeof(phdr)) {
+			pr_info("Couldn't read %d bytes for ELF PHDR.", (int)sizeof(phdr));
+			return false;
+		}
+		if (phdr.p_type == PT_LOAD)
+			break;
+	}
+	if (phdr.p_type != PT_LOAD) {
+		pr_info("No PT_LOAD Program Header found.");
+		return false;
+	}
+
+	ei->load_addr = phdr.p_paddr;
+
+	if ((ei->load_addr & 0xffffffffc0000000ull) == 0xffffffff80000000ull)
+		ei->load_addr &= 0x1ffffffful; /* Convert KSEG{0,1} to physical. */
+	if ((ei->load_addr & 0xc000000000000000ull) == 0x8000000000000000ull)
+		ei->load_addr &= 0x07ffffffffffffffull; /* Convert XKPHYS to pysical */
+
+
+	ei->len = phdr.p_filesz;
+	ei->offset = phdr.p_offset;
+
+	return true;
+}
+
+static bool kvm__arch_get_elf_32_info(Elf32_Ehdr *ehdr, int fd_kernel,
+				      struct kvm__arch_elf_info *ei)
+{
+	int i;
+	size_t nr;
+	Elf32_Phdr phdr;
+
+	if (ehdr->e_phentsize != sizeof(phdr)) {
+		pr_info("Incompatible ELF PHENTSIZE %d", ehdr->e_phentsize);
+		return false;
+	}
+
+	ei->entry_point = (s64)((s32)ehdr->e_entry);
+
+	if (lseek(fd_kernel, ehdr->e_phoff, SEEK_SET) < 0)
+		die_perror("lseek");
+
+	phdr.p_type = PT_NULL;
+	for (i = 0; i < ehdr->e_phnum; i++) {
+		nr = read(fd_kernel, &phdr, sizeof(phdr));
+		if (nr != sizeof(phdr)) {
+			pr_info("Couldn't read %d bytes for ELF PHDR.", (int)sizeof(phdr));
+			return false;
+		}
+		if (phdr.p_type == PT_LOAD)
+			break;
+	}
+	if (phdr.p_type != PT_LOAD) {
+		pr_info("No PT_LOAD Program Header found.");
+		return false;
+	}
+
+	ei->load_addr = (s64)((s32)phdr.p_paddr);
+
+	if ((ei->load_addr & 0xffffffffc0000000ull) == 0xffffffff80000000ull)
+		ei->load_addr &= 0x1fffffffull; /* Convert KSEG{0,1} to physical. */
+
+	ei->len = phdr.p_filesz;
+	ei->offset = phdr.p_offset;
+
+	return true;
+}
+
+int load_elf_binary(struct kvm *kvm, int fd_kernel, int fd_initrd, const char *kernel_cmdline)
+{
+	union {
+		Elf64_Ehdr ehdr;
+		Elf32_Ehdr ehdr32;
+	} eh;
+
+	size_t nr;
+	char *p;
+	struct kvm__arch_elf_info ei;
+
+	if (lseek(fd_kernel, 0, SEEK_SET) < 0)
+		die_perror("lseek");
+
+	nr = read(fd_kernel, &eh, sizeof(eh));
+	if (nr != sizeof(eh)) {
+		pr_info("Couldn't read %d bytes for ELF header.", (int)sizeof(eh));
+		return false;
+	}
+
+	if (eh.ehdr.e_ident[EI_MAG0] != ELFMAG0 ||
+	    eh.ehdr.e_ident[EI_MAG1] != ELFMAG1 ||
+	    eh.ehdr.e_ident[EI_MAG2] != ELFMAG2 ||
+	    eh.ehdr.e_ident[EI_MAG3] != ELFMAG3 ||
+	    (eh.ehdr.e_ident[EI_CLASS] != ELFCLASS64 && eh.ehdr.e_ident[EI_CLASS] != ELFCLASS32) ||
+	    eh.ehdr.e_ident[EI_VERSION] != EV_CURRENT) {
+		pr_info("Incompatible ELF header.");
+		return false;
+	}
+	if (eh.ehdr.e_type != ET_EXEC || eh.ehdr.e_machine != EM_MIPS) {
+		pr_info("Incompatible ELF not MIPS EXEC.");
+		return false;
+	}
+
+	if (eh.ehdr.e_ident[EI_CLASS] == ELFCLASS64) {
+		if (!kvm__arch_get_elf_64_info(&eh.ehdr, fd_kernel, &ei))
+			return false;
+		kvm->arch.is64bit = true;
+	} else {
+		if (!kvm__arch_get_elf_32_info(&eh.ehdr32, fd_kernel, &ei))
+			return false;
+		kvm->arch.is64bit = false;
+	}
+
+	kvm->arch.entry_point = ei.entry_point;
+
+	if (lseek(fd_kernel, ei.offset, SEEK_SET) < 0)
+		die_perror("lseek");
+
+	p = guest_flat_to_host(kvm, ei.load_addr);
+
+	pr_info("ELF Loading 0x%lx bytes from 0x%llx to 0x%llx",
+		(unsigned long)ei.len, (unsigned long long)ei.offset, (unsigned long long)ei.load_addr);
+	do {
+		nr = read(fd_kernel, p, ei.len);
+		if (nr < 0)
+			die_perror("read");
+		p += nr;
+		ei.len -= nr;
+	} while (ei.len);
+
+	kvm__mips_install_cmdline(kvm);
+
+	return true;
+}
+
 bool load_bzimage(struct kvm *kvm, int fd_kernel, int fd_initrd,
 		  const char *kernel_cmdline)
 {
-- 
1.7.9.5

[PATCH 10/11] kvm tools: Introduce weak (default) load_bzimage function

[Andreas Herrmann]

... to get rid of its function definition from archs that don't
support it.

Signed-off-by: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
---
 tools/kvm/arm/fdt.c     |    7 -------
 tools/kvm/kvm.c         |    6 ++++++
 tools/kvm/mips/kvm.c    |    6 ------
 tools/kvm/powerpc/kvm.c |    7 -------
 4 files changed, 6 insertions(+), 20 deletions(-)

diff --git a/tools/kvm/arm/fdt.c b/tools/kvm/arm/fdt.c
index 30cd75a..186a718 100644
--- a/tools/kvm/arm/fdt.c
+++ b/tools/kvm/arm/fdt.c
@@ -276,10 +276,3 @@ int load_flat_binary(struct kvm *kvm, int fd_kernel, int fd_initrd,
 
 	return true;
 }
-
-bool load_bzimage(struct kvm *kvm, int fd_kernel, int fd_initrd,
-		  const char *kernel_cmdline)
-{
-	/* To b or not to b? That is the zImage. */
-	return false;
-}
diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c
index 278b915..e1b9f6c 100644
--- a/tools/kvm/kvm.c
+++ b/tools/kvm/kvm.c
@@ -355,6 +355,12 @@ int __attribute__((__weak__)) load_elf_binary(struct kvm *kvm, int fd_kernel,
 	return false;
 }
 
+bool __attribute__((__weak__)) load_bzimage(struct kvm *kvm, int fd_kernel,
+				int fd_initrd, const char *kernel_cmdline)
+{
+	return false;
+}
+
 bool kvm__load_kernel(struct kvm *kvm, const char *kernel_filename,
 		const char *initrd_filename, const char *kernel_cmdline)
 {
diff --git a/tools/kvm/mips/kvm.c b/tools/kvm/mips/kvm.c
index 09192c8..fc0428b 100644
--- a/tools/kvm/mips/kvm.c
+++ b/tools/kvm/mips/kvm.c
@@ -323,12 +323,6 @@ int load_elf_binary(struct kvm *kvm, int fd_kernel, int fd_initrd, const char *k
 	return true;
 }
 
-bool load_bzimage(struct kvm *kvm, int fd_kernel, int fd_initrd,
-		  const char *kernel_cmdline)
-{
-	return false;
-}
-
 void ioport__map_irq(u8 *irq)
 {
 }
diff --git a/tools/kvm/powerpc/kvm.c b/tools/kvm/powerpc/kvm.c
index c1712cf..2b03a12 100644
--- a/tools/kvm/powerpc/kvm.c
+++ b/tools/kvm/powerpc/kvm.c
@@ -204,13 +204,6 @@ int load_flat_binary(struct kvm *kvm, int fd_kernel, int fd_initrd, const char *
 	return true;
 }
 
-bool load_bzimage(struct kvm *kvm, int fd_kernel, int fd_initrd,
-		  const char *kernel_cmdline)
-{
-	/* We don't support bzImages. */
-	return false;
-}
-
 struct fdt_prop {
 	void *value;
 	int size;
-- 
1.7.9.5

Re: [PATCH 10/11] kvm tools: Introduce weak (default) load_bzimage function

[James Hogan]

Hi Andreas,

On 06/05/14 16:51, Andreas Herrmann wrote:
> ... to get rid of its function definition from archs that don't
> support it.

Maybe it makes sense to put this patch before the main mips one so that
the function doesn't have to be added for mips in the first place

Cheers
James

> 
> Signed-off-by: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
> ---
>  tools/kvm/arm/fdt.c     |    7 -------
>  tools/kvm/kvm.c         |    6 ++++++
>  tools/kvm/mips/kvm.c    |    6 ------
>  tools/kvm/powerpc/kvm.c |    7 -------
>  4 files changed, 6 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/kvm/arm/fdt.c b/tools/kvm/arm/fdt.c
> index 30cd75a..186a718 100644
> --- a/tools/kvm/arm/fdt.c
> +++ b/tools/kvm/arm/fdt.c
> @@ -276,10 +276,3 @@ int load_flat_binary(struct kvm *kvm, int fd_kernel, int fd_initrd,
>  
>  	return true;
>  }
> -
> -bool load_bzimage(struct kvm *kvm, int fd_kernel, int fd_initrd,
> -		  const char *kernel_cmdline)
> -{
> -	/* To b or not to b? That is the zImage. */
> -	return false;
> -}
> diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c
> index 278b915..e1b9f6c 100644
> --- a/tools/kvm/kvm.c
> +++ b/tools/kvm/kvm.c
> @@ -355,6 +355,12 @@ int __attribute__((__weak__)) load_elf_binary(struct kvm *kvm, int fd_kernel,
>  	return false;
>  }
>  
> +bool __attribute__((__weak__)) load_bzimage(struct kvm *kvm, int fd_kernel,
> +				int fd_initrd, const char *kernel_cmdline)
> +{
> +	return false;
> +}
> +
>  bool kvm__load_kernel(struct kvm *kvm, const char *kernel_filename,
>  		const char *initrd_filename, const char *kernel_cmdline)
>  {
> diff --git a/tools/kvm/mips/kvm.c b/tools/kvm/mips/kvm.c
> index 09192c8..fc0428b 100644
> --- a/tools/kvm/mips/kvm.c
> +++ b/tools/kvm/mips/kvm.c
> @@ -323,12 +323,6 @@ int load_elf_binary(struct kvm *kvm, int fd_kernel, int fd_initrd, const char *k
>  	return true;
>  }
>  
> -bool load_bzimage(struct kvm *kvm, int fd_kernel, int fd_initrd,
> -		  const char *kernel_cmdline)
> -{
> -	return false;
> -}
> -
>  void ioport__map_irq(u8 *irq)
>  {
>  }
> diff --git a/tools/kvm/powerpc/kvm.c b/tools/kvm/powerpc/kvm.c
> index c1712cf..2b03a12 100644
> --- a/tools/kvm/powerpc/kvm.c
> +++ b/tools/kvm/powerpc/kvm.c
> @@ -204,13 +204,6 @@ int load_flat_binary(struct kvm *kvm, int fd_kernel, int fd_initrd, const char *
>  	return true;
>  }
>  
> -bool load_bzimage(struct kvm *kvm, int fd_kernel, int fd_initrd,
> -		  const char *kernel_cmdline)
> -{
> -	/* We don't support bzImages. */
> -	return false;
> -}
> -
>  struct fdt_prop {
>  	void *value;
>  	int size;
> 

Re: [PATCH 10/11] kvm tools: Introduce weak (default) load_bzimage function

[Andreas Herrmann]

On Fri, May 09, 2014 at 10:44:24PM +0100, James Hogan wrote:
> Hi Andreas,
> 
> On 06/05/14 16:51, Andreas Herrmann wrote:
> > ... to get rid of its function definition from archs that don't
> > support it.
> 
> Maybe it makes sense to put this patch before the main mips one so that
> the function doesn't have to be added for mips in the first place

Yes, that makes sense.

Thanks,
Andreas

> Cheers
> James
> 
> > 
> > Signed-off-by: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
> > ---
> >  tools/kvm/arm/fdt.c     |    7 -------
> >  tools/kvm/kvm.c         |    6 ++++++
> >  tools/kvm/mips/kvm.c    |    6 ------
> >  tools/kvm/powerpc/kvm.c |    7 -------
> >  4 files changed, 6 insertions(+), 20 deletions(-)
> > 
> > diff --git a/tools/kvm/arm/fdt.c b/tools/kvm/arm/fdt.c
> > index 30cd75a..186a718 100644
> > --- a/tools/kvm/arm/fdt.c
> > +++ b/tools/kvm/arm/fdt.c
> > @@ -276,10 +276,3 @@ int load_flat_binary(struct kvm *kvm, int fd_kernel, int fd_initrd,
> >  
> >  	return true;
> >  }
> > -
> > -bool load_bzimage(struct kvm *kvm, int fd_kernel, int fd_initrd,
> > -		  const char *kernel_cmdline)
> > -{
> > -	/* To b or not to b? That is the zImage. */
> > -	return false;
> > -}
> > diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c
> > index 278b915..e1b9f6c 100644
> > --- a/tools/kvm/kvm.c
> > +++ b/tools/kvm/kvm.c
> > @@ -355,6 +355,12 @@ int __attribute__((__weak__)) load_elf_binary(struct kvm *kvm, int fd_kernel,
> >  	return false;
> >  }
> >  
> > +bool __attribute__((__weak__)) load_bzimage(struct kvm *kvm, int fd_kernel,
> > +				int fd_initrd, const char *kernel_cmdline)
> > +{
> > +	return false;
> > +}
> > +
> >  bool kvm__load_kernel(struct kvm *kvm, const char *kernel_filename,
> >  		const char *initrd_filename, const char *kernel_cmdline)
> >  {
> > diff --git a/tools/kvm/mips/kvm.c b/tools/kvm/mips/kvm.c
> > index 09192c8..fc0428b 100644
> > --- a/tools/kvm/mips/kvm.c
> > +++ b/tools/kvm/mips/kvm.c
> > @@ -323,12 +323,6 @@ int load_elf_binary(struct kvm *kvm, int fd_kernel, int fd_initrd, const char *k
> >  	return true;
> >  }
> >  
> > -bool load_bzimage(struct kvm *kvm, int fd_kernel, int fd_initrd,
> > -		  const char *kernel_cmdline)
> > -{
> > -	return false;
> > -}
> > -
> >  void ioport__map_irq(u8 *irq)
> >  {
> >  }
> > diff --git a/tools/kvm/powerpc/kvm.c b/tools/kvm/powerpc/kvm.c
> > index c1712cf..2b03a12 100644
> > --- a/tools/kvm/powerpc/kvm.c
> > +++ b/tools/kvm/powerpc/kvm.c
> > @@ -204,13 +204,6 @@ int load_flat_binary(struct kvm *kvm, int fd_kernel, int fd_initrd, const char *
> >  	return true;
> >  }
> >  
> > -bool load_bzimage(struct kvm *kvm, int fd_kernel, int fd_initrd,
> > -		  const char *kernel_cmdline)
> > -{
> > -	/* We don't support bzImages. */
> > -	return false;
> > -}
> > -
> >  struct fdt_prop {
> >  	void *value;
> >  	int size;
> > 

[PATCH 11/11] kvm tools: Modify term_putc to write more than one char

[Andreas Herrmann]

From: David Daney <david.daney@cavium.com>

It is a performance enhancement. When running in a simulator, each
system call to write a character takes a lot of time.  Batching them
up decreases the overhead (in the root kernel) of each virtio console
write.

Signed-off-by: David Daney <david.daney@cavium.com>
Signed-off-by: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
---
 tools/kvm/term.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/kvm/term.c b/tools/kvm/term.c
index 3de410b..b153eed 100644
--- a/tools/kvm/term.c
+++ b/tools/kvm/term.c
@@ -52,11 +52,14 @@ int term_getc(struct kvm *kvm, int term)
 int term_putc(char *addr, int cnt, int term)
 {
 	int ret;
+	int num_remaining = cnt;
 
-	while (cnt--) {
-		ret = write(term_fds[term][TERM_FD_OUT], addr++, 1);
+	while (num_remaining) {
+		ret = write(term_fds[term][TERM_FD_OUT], addr, num_remaining);
 		if (ret < 0)
 			return 0;
+		num_remaining -= ret;
+		addr += ret;
 	}
 
 	return cnt;
-- 
1.7.9.5

Re: [PATCH 11/11] kvm tools: Modify term_putc to write more than one char

[Sergei Shtylyov]

Hello.

On 06-05-2014 19:51, Andreas Herrmann wrote:

> From: David Daney <david.daney@cavium.com>

> It is a performance enhancement. When running in a simulator, each
> system call to write a character takes a lot of time.  Batching them
> up decreases the overhead (in the root kernel) of each virtio console
> write.

> Signed-off-by: David Daney <david.daney@cavium.com>
> Signed-off-by: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
> ---
>   tools/kvm/term.c |    7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)

> diff --git a/tools/kvm/term.c b/tools/kvm/term.c
> index 3de410b..b153eed 100644
> --- a/tools/kvm/term.c
> +++ b/tools/kvm/term.c
> @@ -52,11 +52,14 @@ int term_getc(struct kvm *kvm, int term)
>   int term_putc(char *addr, int cnt, int term)
>   {
>   	int ret;
> +	int num_remaining = cnt;
>
> -	while (cnt--) {
> -		ret = write(term_fds[term][TERM_FD_OUT], addr++, 1);
> +	while (num_remaining) {
> +		ret = write(term_fds[term][TERM_FD_OUT], addr, num_remaining);
>   		if (ret < 0)
>   			return 0;

    Perhaps 'return cnt - num_remaining' instead?

> +		num_remaining -= ret;
> +		addr += ret;
>   	}
>
>   	return cnt;

WBR, Sergei

Re: [PATCH 11/11] kvm tools: Modify term_putc to write more than one char

[Andreas Herrmann]

On Wed, May 07, 2014 at 07:58:30PM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 06-05-2014 19:51, Andreas Herrmann wrote:
> 
> >From: David Daney <david.daney@cavium.com>
> 
> >It is a performance enhancement. When running in a simulator, each
> >system call to write a character takes a lot of time.  Batching them
> >up decreases the overhead (in the root kernel) of each virtio console
> >write.
> 
> >Signed-off-by: David Daney <david.daney@cavium.com>
> >Signed-off-by: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
> >---
> >  tools/kvm/term.c |    7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> >diff --git a/tools/kvm/term.c b/tools/kvm/term.c
> >index 3de410b..b153eed 100644
> >--- a/tools/kvm/term.c
> >+++ b/tools/kvm/term.c
> >@@ -52,11 +52,14 @@ int term_getc(struct kvm *kvm, int term)
> >  int term_putc(char *addr, int cnt, int term)
> >  {
> >  	int ret;
> >+	int num_remaining = cnt;
> >
> >-	while (cnt--) {
> >-		ret = write(term_fds[term][TERM_FD_OUT], addr++, 1);
> >+	while (num_remaining) {
> >+		ret = write(term_fds[term][TERM_FD_OUT], addr, num_remaining);
> >  		if (ret < 0)
> >  			return 0;
> 
>    Perhaps 'return cnt - num_remaining' instead?

Although all current callers of this function are not checking the
return value I aggree that this change would be nice to have.

I wouldn't make this change within this patch though.
(I'll add a separate patch to modify the return value.)

> >+		num_remaining -= ret;
> >+		addr += ret;
> >  	}
> >
> >  	return cnt;
> 
> WBR, Sergei


Thanks,
Andreas