[PATCH V3 0/2] kvm tools: Prepare kvmtool for another architecture

[PATCH V3 0/2] kvm tools: Prepare kvmtool for another architecture

[Matt Evans]

The last remaining patches from the preparation series, with changes:
- Map from hugetlbfs does plain statfs (without odd error checking),
  checks result of ftruncate()
- Remove typo whereby kvm_cpu__emulate_mmio() calls self.....

Cheers,


Matt

Matt Evans (2):
  kvm tools: Add ability to map guest RAM from hugetlbfs
  kvm tools: Create arch-specific kvm_cpu__emulate_{mm}io()

 tools/kvm/builtin-run.c                  |    4 ++-
 tools/kvm/include/kvm/kvm.h              |    4 +-
 tools/kvm/include/kvm/util.h             |    4 +++
 tools/kvm/kvm-cpu.c                      |   34 +++++++++++++-------------
 tools/kvm/kvm.c                          |    4 +-
 tools/kvm/util.c                         |   38 ++++++++++++++++++++++++++++++
 tools/kvm/x86/include/kvm/kvm-cpu-arch.h |   17 ++++++++++++-
 tools/kvm/x86/kvm.c                      |   20 +++++++++++++--
 8 files changed, 99 insertions(+), 26 deletions(-)

[PATCH V3 1/2] kvm tools: Add ability to map guest RAM from hugetlbfs

[Matt Evans]

Add a --hugetlbfs commandline option to give a path to hugetlbfs-map guest
memory (down in kvm__arch_init()).  For x86, guest memory is a normal
ANON mmap() if this option is not provided, otherwise a hugetlbfs mmap.

This maps directly from a hugetlbfs temp file rather than using something
like MADV_HUGEPAGES so that, if the user asks for hugepages, we definitely
are using hugepages.  (This is particularly useful for architectures that
don't yet support KVM without hugepages, so we definitely need to use
them for the whole of guest RAM.)

Signed-off-by: Matt Evans <matt@ozlabs.org>
---
 tools/kvm/builtin-run.c      |    4 +++-
 tools/kvm/include/kvm/kvm.h  |    4 ++--
 tools/kvm/include/kvm/util.h |    4 ++++
 tools/kvm/kvm.c              |    4 ++--
 tools/kvm/util.c             |   38 ++++++++++++++++++++++++++++++++++++++
 tools/kvm/x86/kvm.c          |   20 +++++++++++++++++---
 6 files changed, 66 insertions(+), 8 deletions(-)

diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index 4411c9e..ab05f8c 100644
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
@@ -85,6 +85,7 @@ static const char *host_mac;
 static const char *script;
 static const char *guest_name;
 static const char *sandbox;
+static const char *hugetlbfs_path;
 static struct virtio_net_params *net_params;
 static bool single_step;
 static bool readonly_image[MAX_DISK_IMAGES];
@@ -437,6 +438,7 @@ static const struct option options[] = {
 		     tty_parser),
 	OPT_STRING('\0', "sandbox", &sandbox, "script",
 			"Run this script when booting into custom rootfs"),
+	OPT_STRING('\0', "hugetlbfs", &hugetlbfs_path, "path", "Hugetlbfs path"),
 
 	OPT_GROUP("Kernel options:"),
 	OPT_STRING('k', "kernel", &kernel_filename, "kernel",
@@ -924,7 +926,7 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
 		guest_name = default_name;
 	}
 
-	kvm = kvm__init(dev, ram_size, guest_name);
+	kvm = kvm__init(dev, hugetlbfs_path, ram_size, guest_name);
 
 	kvm->single_step = single_step;
 
diff --git a/tools/kvm/include/kvm/kvm.h b/tools/kvm/include/kvm/kvm.h
index 5fe6e75..7159952 100644
--- a/tools/kvm/include/kvm/kvm.h
+++ b/tools/kvm/include/kvm/kvm.h
@@ -30,7 +30,7 @@ struct kvm_ext {
 void kvm__set_dir(const char *fmt, ...);
 const char *kvm__get_dir(void);
 
-struct kvm *kvm__init(const char *kvm_dev, u64 ram_size, const char *name);
+struct kvm *kvm__init(const char *kvm_dev, const char *hugetlbfs_path, u64 ram_size, const char *name);
 int kvm__recommended_cpus(struct kvm *kvm);
 int kvm__max_cpus(struct kvm *kvm);
 void kvm__init_ram(struct kvm *kvm);
@@ -54,7 +54,7 @@ int kvm__enumerate_instances(int (*callback)(const char *name, int pid));
 void kvm__remove_socket(const char *name);
 
 void kvm__arch_set_cmdline(char *cmdline, bool video);
-void kvm__arch_init(struct kvm *kvm, const char *kvm_dev, u64 ram_size, const char *name);
+void kvm__arch_init(struct kvm *kvm, const char *kvm_dev, const char *hugetlbfs_path, u64 ram_size, const char *name);
 void kvm__arch_setup_firmware(struct kvm *kvm);
 bool kvm__arch_cpu_supports_vm(void);
 void kvm__arch_periodic_poll(struct kvm *kvm);
diff --git a/tools/kvm/include/kvm/util.h b/tools/kvm/include/kvm/util.h
index dc2e0b9..1f6fbbd 100644
--- a/tools/kvm/include/kvm/util.h
+++ b/tools/kvm/include/kvm/util.h
@@ -20,6 +20,7 @@
 #include <limits.h>
 #include <sys/param.h>
 #include <sys/types.h>
+#include <linux/types.h>
 
 #ifdef __GNUC__
 #define NORETURN __attribute__((__noreturn__))
@@ -75,4 +76,7 @@ static inline void msleep(unsigned int msecs)
 {
 	usleep(MSECS_TO_USECS(msecs));
 }
+
+void *mmap_hugetlbfs(const char *htlbfs_path, u64 size);
+
 #endif /* KVM__UTIL_H */
diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c
index c54f886..35ca2c5 100644
--- a/tools/kvm/kvm.c
+++ b/tools/kvm/kvm.c
@@ -306,7 +306,7 @@ int kvm__max_cpus(struct kvm *kvm)
 	return ret;
 }
 
-struct kvm *kvm__init(const char *kvm_dev, u64 ram_size, const char *name)
+struct kvm *kvm__init(const char *kvm_dev, const char *hugetlbfs_path, u64 ram_size, const char *name)
 {
 	struct kvm *kvm;
 	int ret;
@@ -339,7 +339,7 @@ struct kvm *kvm__init(const char *kvm_dev, u64 ram_size, const char *name)
 	if (kvm__check_extensions(kvm))
 		die("A required KVM extention is not supported by OS");
 
-	kvm__arch_init(kvm, kvm_dev, ram_size, name);
+	kvm__arch_init(kvm, kvm_dev, hugetlbfs_path, ram_size, name);
 
 	kvm->name = name;
 
diff --git a/tools/kvm/util.c b/tools/kvm/util.c
index 4efbce9..f0a4bf7 100644
--- a/tools/kvm/util.c
+++ b/tools/kvm/util.c
@@ -4,6 +4,11 @@
 
 #include "kvm/util.h"
 
+#include <linux/magic.h>	/* For HUGETLBFS_MAGIC */
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <sys/statfs.h>
+
 static void report(const char *prefix, const char *err, va_list params)
 {
 	char msg[1024];
@@ -99,3 +104,36 @@ size_t strlcat(char *dest, const char *src, size_t count)
 
 	return res;
 }
+
+void *mmap_hugetlbfs(const char *htlbfs_path, u64 size)
+{
+	char mpath[PATH_MAX];
+	int fd;
+	struct statfs sfs;
+	void *addr;
+
+	if (statfs(htlbfs_path, &sfs) < 0)
+		die("Can't stat %s\n", htlbfs_path);
+
+	if (sfs.f_type != HUGETLBFS_MAGIC) {
+		die("%s is not hugetlbfs!\n", htlbfs_path);
+	}
+
+	if (sfs.f_bsize == 0 || (unsigned long)sfs.f_bsize > size) {
+		die("Can't use hugetlbfs pagesize %ld for mem size %lld\n",
+		    sfs.f_bsize, size);
+	}
+
+	snprintf(mpath, PATH_MAX, "%s/kvmtoolXXXXXX", htlbfs_path);
+	fd = mkstemp(mpath);
+	if (fd < 0)
+		die("Can't open %s for hugetlbfs map\n", mpath);
+	unlink(mpath);
+	if (ftruncate(fd, size) < 0)
+		die("Can't ftruncate for mem mapping size %lld\n",
+		    size);
+	addr = mmap(NULL, size, PROT_RW, MAP_PRIVATE, fd, 0);
+	close(fd);
+
+	return addr;
+}
diff --git a/tools/kvm/x86/kvm.c b/tools/kvm/x86/kvm.c
index da4a6b6..bc52ef3 100644
--- a/tools/kvm/x86/kvm.c
+++ b/tools/kvm/x86/kvm.c
@@ -130,8 +130,22 @@ void kvm__arch_set_cmdline(char *cmdline, bool video)
 		strcat(cmdline, " console=ttyS0 earlyprintk=serial i8042.noaux=1");
 }
 
+/* This function wraps the decision between hugetlbfs map (if requested) or normal mmap */
+static void *mmap_anon_or_hugetlbfs(const char *hugetlbfs_path, u64 size)
+{
+	if (hugetlbfs_path) {
+		/*
+		 * We don't /need/ to map guest RAM from hugetlbfs, but we do so
+		 * if the user specifies a hugetlbfs path.
+		 */
+		return mmap_hugetlbfs(hugetlbfs_path, size);
+	} else {
+		return mmap(NULL, size, PROT_RW, MAP_ANON_NORESERVE, -1, 0);
+	}
+}
+
 /* Architecture-specific KVM init */
-void kvm__arch_init(struct kvm *kvm, const char *kvm_dev, u64 ram_size, const char *name)
+void kvm__arch_init(struct kvm *kvm, const char *kvm_dev, const char *hugetlbfs_path, u64 ram_size, const char *name)
 {
 	struct kvm_pit_config pit_config = { .flags = 0, };
 	int ret;
@@ -147,9 +161,9 @@ void kvm__arch_init(struct kvm *kvm, const char *kvm_dev, u64 ram_size, const ch
 	kvm->ram_size		= ram_size;
 
 	if (kvm->ram_size < KVM_32BIT_GAP_START) {
-		kvm->ram_start = mmap(NULL, ram_size, PROT_RW, MAP_ANON_NORESERVE, -1, 0);
+		kvm->ram_start = mmap_anon_or_hugetlbfs(hugetlbfs_path, ram_size);
 	} else {
-		kvm->ram_start = mmap(NULL, ram_size + KVM_32BIT_GAP_SIZE, PROT_RW, MAP_ANON_NORESERVE, -1, 0);
+		kvm->ram_start = mmap_anon_or_hugetlbfs(hugetlbfs_path, ram_size + KVM_32BIT_GAP_SIZE);
 		if (kvm->ram_start != MAP_FAILED) {
 			/*
 			 * We mprotect the gap (see kvm__init_ram() for details) PROT_NONE so that
-- 
1.7.0.4

Re: [PATCH V3 1/2] kvm tools: Add ability to map guest RAM from hugetlbfs

[David Evensky]

On an x86 32bit system (and using the 32bit CodeSourcery toolchain on a x86_64 system) I get:

evensky@machine:~/.../linux-kvm/tools/kvm$ make
  CC       util/util.o
util/util.c: In function 'mmap_hugetlbfs':
util/util.c:93:17: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
util/util.c:99:7: error: format '%ld' expects argument of type 'long int', but argument 2 has type 'int' [-Werror=format]
cc1: all warnings being treated as errors

make: *** [util/util.o] Error 1

Thanks,
\dae

On Tue, Dec 13, 2011 at 05:21:46PM +1100, Matt Evans wrote:
> Add a --hugetlbfs commandline option to give a path to hugetlbfs-map guest
> memory (down in kvm__arch_init()).  For x86, guest memory is a normal
> ANON mmap() if this option is not provided, otherwise a hugetlbfs mmap.
> 
> This maps directly from a hugetlbfs temp file rather than using something
> like MADV_HUGEPAGES so that, if the user asks for hugepages, we definitely
> are using hugepages.  (This is particularly useful for architectures that
> don't yet support KVM without hugepages, so we definitely need to use
> them for the whole of guest RAM.)
> 
> Signed-off-by: Matt Evans <matt@ozlabs.org>
> ---
>  tools/kvm/builtin-run.c      |    4 +++-
>  tools/kvm/include/kvm/kvm.h  |    4 ++--
>  tools/kvm/include/kvm/util.h |    4 ++++
>  tools/kvm/kvm.c              |    4 ++--
>  tools/kvm/util.c             |   38 ++++++++++++++++++++++++++++++++++++++
>  tools/kvm/x86/kvm.c          |   20 +++++++++++++++++---
>  6 files changed, 66 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
> index 4411c9e..ab05f8c 100644
> --- a/tools/kvm/builtin-run.c
> +++ b/tools/kvm/builtin-run.c
> @@ -85,6 +85,7 @@ static const char *host_mac;
>  static const char *script;
>  static const char *guest_name;
>  static const char *sandbox;
> +static const char *hugetlbfs_path;
>  static struct virtio_net_params *net_params;
>  static bool single_step;
>  static bool readonly_image[MAX_DISK_IMAGES];
> @@ -437,6 +438,7 @@ static const struct option options[] = {
>  		     tty_parser),
>  	OPT_STRING('\0', "sandbox", &sandbox, "script",
>  			"Run this script when booting into custom rootfs"),
> +	OPT_STRING('\0', "hugetlbfs", &hugetlbfs_path, "path", "Hugetlbfs path"),
>  
>  	OPT_GROUP("Kernel options:"),
>  	OPT_STRING('k', "kernel", &kernel_filename, "kernel",
> @@ -924,7 +926,7 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
>  		guest_name = default_name;
>  	}
>  
> -	kvm = kvm__init(dev, ram_size, guest_name);
> +	kvm = kvm__init(dev, hugetlbfs_path, ram_size, guest_name);
>  
>  	kvm->single_step = single_step;
>  
> diff --git a/tools/kvm/include/kvm/kvm.h b/tools/kvm/include/kvm/kvm.h
> index 5fe6e75..7159952 100644
> --- a/tools/kvm/include/kvm/kvm.h
> +++ b/tools/kvm/include/kvm/kvm.h
> @@ -30,7 +30,7 @@ struct kvm_ext {
>  void kvm__set_dir(const char *fmt, ...);
>  const char *kvm__get_dir(void);
>  
> -struct kvm *kvm__init(const char *kvm_dev, u64 ram_size, const char *name);
> +struct kvm *kvm__init(const char *kvm_dev, const char *hugetlbfs_path, u64 ram_size, const char *name);
>  int kvm__recommended_cpus(struct kvm *kvm);
>  int kvm__max_cpus(struct kvm *kvm);
>  void kvm__init_ram(struct kvm *kvm);
> @@ -54,7 +54,7 @@ int kvm__enumerate_instances(int (*callback)(const char *name, int pid));
>  void kvm__remove_socket(const char *name);
>  
>  void kvm__arch_set_cmdline(char *cmdline, bool video);
> -void kvm__arch_init(struct kvm *kvm, const char *kvm_dev, u64 ram_size, const char *name);
> +void kvm__arch_init(struct kvm *kvm, const char *kvm_dev, const char *hugetlbfs_path, u64 ram_size, const char *name);
>  void kvm__arch_setup_firmware(struct kvm *kvm);
>  bool kvm__arch_cpu_supports_vm(void);
>  void kvm__arch_periodic_poll(struct kvm *kvm);
> diff --git a/tools/kvm/include/kvm/util.h b/tools/kvm/include/kvm/util.h
> index dc2e0b9..1f6fbbd 100644
> --- a/tools/kvm/include/kvm/util.h
> +++ b/tools/kvm/include/kvm/util.h
> @@ -20,6 +20,7 @@
>  #include <limits.h>
>  #include <sys/param.h>
>  #include <sys/types.h>
> +#include <linux/types.h>
>  
>  #ifdef __GNUC__
>  #define NORETURN __attribute__((__noreturn__))
> @@ -75,4 +76,7 @@ static inline void msleep(unsigned int msecs)
>  {
>  	usleep(MSECS_TO_USECS(msecs));
>  }
> +
> +void *mmap_hugetlbfs(const char *htlbfs_path, u64 size);
> +
>  #endif /* KVM__UTIL_H */
> diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c
> index c54f886..35ca2c5 100644
> --- a/tools/kvm/kvm.c
> +++ b/tools/kvm/kvm.c
> @@ -306,7 +306,7 @@ int kvm__max_cpus(struct kvm *kvm)
>  	return ret;
>  }
>  
> -struct kvm *kvm__init(const char *kvm_dev, u64 ram_size, const char *name)
> +struct kvm *kvm__init(const char *kvm_dev, const char *hugetlbfs_path, u64 ram_size, const char *name)
>  {
>  	struct kvm *kvm;
>  	int ret;
> @@ -339,7 +339,7 @@ struct kvm *kvm__init(const char *kvm_dev, u64 ram_size, const char *name)
>  	if (kvm__check_extensions(kvm))
>  		die("A required KVM extention is not supported by OS");
>  
> -	kvm__arch_init(kvm, kvm_dev, ram_size, name);
> +	kvm__arch_init(kvm, kvm_dev, hugetlbfs_path, ram_size, name);
>  
>  	kvm->name = name;
>  
> diff --git a/tools/kvm/util.c b/tools/kvm/util.c
> index 4efbce9..f0a4bf7 100644
> --- a/tools/kvm/util.c
> +++ b/tools/kvm/util.c
> @@ -4,6 +4,11 @@
>  
>  #include "kvm/util.h"
>  
> +#include <linux/magic.h>	/* For HUGETLBFS_MAGIC */
> +#include <sys/mman.h>
> +#include <sys/stat.h>
> +#include <sys/statfs.h>
> +
>  static void report(const char *prefix, const char *err, va_list params)
>  {
>  	char msg[1024];
> @@ -99,3 +104,36 @@ size_t strlcat(char *dest, const char *src, size_t count)
>  
>  	return res;
>  }
> +
> +void *mmap_hugetlbfs(const char *htlbfs_path, u64 size)
> +{
> +	char mpath[PATH_MAX];
> +	int fd;
> +	struct statfs sfs;
> +	void *addr;
> +
> +	if (statfs(htlbfs_path, &sfs) < 0)
> +		die("Can't stat %s\n", htlbfs_path);
> +
> +	if (sfs.f_type != HUGETLBFS_MAGIC) {
> +		die("%s is not hugetlbfs!\n", htlbfs_path);
> +	}
> +
> +	if (sfs.f_bsize == 0 || (unsigned long)sfs.f_bsize > size) {
> +		die("Can't use hugetlbfs pagesize %ld for mem size %lld\n",
> +		    sfs.f_bsize, size);
> +	}
> +
> +	snprintf(mpath, PATH_MAX, "%s/kvmtoolXXXXXX", htlbfs_path);
> +	fd = mkstemp(mpath);
> +	if (fd < 0)
> +		die("Can't open %s for hugetlbfs map\n", mpath);
> +	unlink(mpath);
> +	if (ftruncate(fd, size) < 0)
> +		die("Can't ftruncate for mem mapping size %lld\n",
> +		    size);
> +	addr = mmap(NULL, size, PROT_RW, MAP_PRIVATE, fd, 0);
> +	close(fd);
> +
> +	return addr;
> +}
> diff --git a/tools/kvm/x86/kvm.c b/tools/kvm/x86/kvm.c
> index da4a6b6..bc52ef3 100644
> --- a/tools/kvm/x86/kvm.c
> +++ b/tools/kvm/x86/kvm.c
> @@ -130,8 +130,22 @@ void kvm__arch_set_cmdline(char *cmdline, bool video)
>  		strcat(cmdline, " console=ttyS0 earlyprintk=serial i8042.noaux=1");
>  }
>  
> +/* This function wraps the decision between hugetlbfs map (if requested) or normal mmap */
> +static void *mmap_anon_or_hugetlbfs(const char *hugetlbfs_path, u64 size)
> +{
> +	if (hugetlbfs_path) {
> +		/*
> +		 * We don't /need/ to map guest RAM from hugetlbfs, but we do so
> +		 * if the user specifies a hugetlbfs path.
> +		 */
> +		return mmap_hugetlbfs(hugetlbfs_path, size);
> +	} else {
> +		return mmap(NULL, size, PROT_RW, MAP_ANON_NORESERVE, -1, 0);
> +	}
> +}
> +
>  /* Architecture-specific KVM init */
> -void kvm__arch_init(struct kvm *kvm, const char *kvm_dev, u64 ram_size, const char *name)
> +void kvm__arch_init(struct kvm *kvm, const char *kvm_dev, const char *hugetlbfs_path, u64 ram_size, const char *name)
>  {
>  	struct kvm_pit_config pit_config = { .flags = 0, };
>  	int ret;
> @@ -147,9 +161,9 @@ void kvm__arch_init(struct kvm *kvm, const char *kvm_dev, u64 ram_size, const ch
>  	kvm->ram_size		= ram_size;
>  
>  	if (kvm->ram_size < KVM_32BIT_GAP_START) {
> -		kvm->ram_start = mmap(NULL, ram_size, PROT_RW, MAP_ANON_NORESERVE, -1, 0);
> +		kvm->ram_start = mmap_anon_or_hugetlbfs(hugetlbfs_path, ram_size);
>  	} else {
> -		kvm->ram_start = mmap(NULL, ram_size + KVM_32BIT_GAP_SIZE, PROT_RW, MAP_ANON_NORESERVE, -1, 0);
> +		kvm->ram_start = mmap_anon_or_hugetlbfs(hugetlbfs_path, ram_size + KVM_32BIT_GAP_SIZE);
>  		if (kvm->ram_start != MAP_FAILED) {
>  			/*
>  			 * We mprotect the gap (see kvm__init_ram() for details) PROT_NONE so that
> -- 
> 1.7.0.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 1/2] kvm tools: Add ability to map guest RAM from hugetlbfs

[Matt Evans]

On 14/12/11 11:03, David Evensky wrote:
> 
> 
> On an x86 32bit system (and using the 32bit CodeSourcery toolchain on a x86_64 system) I get:
> 
> evensky@machine:~/.../linux-kvm/tools/kvm$ make
>   CC       util/util.o
> util/util.c: In function 'mmap_hugetlbfs':
> util/util.c:93:17: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
> util/util.c:99:7: error: format '%ld' expects argument of type 'long int', but argument 2 has type 'int' [-Werror=format]
> cc1: all warnings being treated as errors
> 
> make: *** [util/util.o] Error 1

Hi David,


Argh!  I didn't catch this as it compiles fine on my x86_64 box (with 64bit
toolchain) >:( So, struct statfs's f_type and f_blocks are __SWORD_TYPE which is
either an int or a long (if on 64bit).... so identical to if it were simply a
long, nice.

Pekka, a re-jiggle fix attached.


Thanks,


Matt


---
From: Matt Evans <matt@ozlabs.org>
Date: Wed, 14 Dec 2011 12:10:03 +1100
Subject: [PATCH] kvm tools: Fix build of util.c on 32bit machines

commit 378ee7e6dd301347c6bf2c740cb1fb40174bcb8b broke the -Werror build
on 32bit targets due to some variable typing in struct statfs.

Fixes the build.

Signed-off-by: Matt Evans <matt@ozlabs.org>
---
 tools/kvm/util/util.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/kvm/util/util.c b/tools/kvm/util/util.c
index ad5418e..00f7315 100644
--- a/tools/kvm/util/util.c
+++ b/tools/kvm/util/util.c
@@ -86,17 +86,19 @@ void *mmap_hugetlbfs(const char *htlbfs_path, u64 size)
 	int fd;
 	struct statfs sfs;
 	void *addr;
+	unsigned long blk_size;
 
 	if (statfs(htlbfs_path, &sfs) < 0)
 		die("Can't stat %s\n", htlbfs_path);
 
-	if (sfs.f_type != HUGETLBFS_MAGIC) {
+	if ((unsigned int)sfs.f_type != HUGETLBFS_MAGIC) {
 		die("%s is not hugetlbfs!\n", htlbfs_path);
 	}
 
-	if (sfs.f_bsize == 0 || (unsigned long)sfs.f_bsize > 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",
-		    sfs.f_bsize, size);
+		    blk_size, size);
 	}
 
 	snprintf(mpath, PATH_MAX, "%s/kvmtoolXXXXXX", htlbfs_path);
-- 
1.7.0.4

[PATCH V3 2/2] kvm tools: Create arch-specific kvm_cpu__emulate_{mm}io()

[Matt Evans]

Different architectures will deal with MMIO exits differently.  For example,
KVM_EXIT_IO is x86-specific, and I/O cycles are often synthesised by steering
into windows in PCI bridges on other architectures.

This patch calls arch-specific kvm_cpu__emulate_io() and kvm_cpu__emulate_mmio()
from the main runloop's IO and MMIO exit handlers.  For x86, these directly
call kvm__emulate_io() and kvm__emulate_mmio() but other architectures will
perform some address munging before passing on the call.

Signed-off-by: Matt Evans <matt@ozlabs.org>
---
 tools/kvm/kvm-cpu.c                      |   34 +++++++++++++++---------------
 tools/kvm/x86/include/kvm/kvm-cpu-arch.h |   17 ++++++++++++++-
 2 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/tools/kvm/kvm-cpu.c b/tools/kvm/kvm-cpu.c
index 8ec4efa..b7ae3d3 100644
--- a/tools/kvm/kvm-cpu.c
+++ b/tools/kvm/kvm-cpu.c
@@ -52,11 +52,11 @@ static void kvm_cpu__handle_coalesced_mmio(struct kvm_cpu *cpu)
 		while (cpu->ring->first != cpu->ring->last) {
 			struct kvm_coalesced_mmio *m;
 			m = &cpu->ring->coalesced_mmio[cpu->ring->first];
-			kvm__emulate_mmio(cpu->kvm,
-					m->phys_addr,
-					m->data,
-					m->len,
-					1);
+			kvm_cpu__emulate_mmio(cpu->kvm,
+					      m->phys_addr,
+					      m->data,
+					      m->len,
+					      1);
 			cpu->ring->first = (cpu->ring->first + 1) % KVM_COALESCED_MMIO_MAX;
 		}
 	}
@@ -111,13 +111,13 @@ int kvm_cpu__start(struct kvm_cpu *cpu)
 		case KVM_EXIT_IO: {
 			bool ret;
 
-			ret = kvm__emulate_io(cpu->kvm,
-					cpu->kvm_run->io.port,
-					(u8 *)cpu->kvm_run +
-					cpu->kvm_run->io.data_offset,
-					cpu->kvm_run->io.direction,
-					cpu->kvm_run->io.size,
-					cpu->kvm_run->io.count);
+			ret = kvm_cpu__emulate_io(cpu->kvm,
+						  cpu->kvm_run->io.port,
+						  (u8 *)cpu->kvm_run +
+						  cpu->kvm_run->io.data_offset,
+						  cpu->kvm_run->io.direction,
+						  cpu->kvm_run->io.size,
+						  cpu->kvm_run->io.count);
 
 			if (!ret)
 				goto panic_kvm;
@@ -126,11 +126,11 @@ int kvm_cpu__start(struct kvm_cpu *cpu)
 		case KVM_EXIT_MMIO: {
 			bool ret;
 
-			ret = kvm__emulate_mmio(cpu->kvm,
-					cpu->kvm_run->mmio.phys_addr,
-					cpu->kvm_run->mmio.data,
-					cpu->kvm_run->mmio.len,
-					cpu->kvm_run->mmio.is_write);
+			ret = kvm_cpu__emulate_mmio(cpu->kvm,
+						    cpu->kvm_run->mmio.phys_addr,
+						    cpu->kvm_run->mmio.data,
+						    cpu->kvm_run->mmio.len,
+						    cpu->kvm_run->mmio.is_write);
 
 			if (!ret)
 				goto panic_kvm;
diff --git a/tools/kvm/x86/include/kvm/kvm-cpu-arch.h b/tools/kvm/x86/include/kvm/kvm-cpu-arch.h
index 822d966..198efe6 100644
--- a/tools/kvm/x86/include/kvm/kvm-cpu-arch.h
+++ b/tools/kvm/x86/include/kvm/kvm-cpu-arch.h
@@ -4,7 +4,8 @@
 /* Architecture-specific kvm_cpu definitions. */
 
 #include <linux/kvm.h>	/* for struct kvm_regs */
-
+#include "kvm/kvm.h"	/* for kvm__emulate_{mm}io() */
+#include <stdbool.h>
 #include <pthread.h>
 
 struct kvm;
@@ -31,4 +32,18 @@ struct kvm_cpu {
 	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 */
-- 
1.7.0.4

Re: [PATCH V3 2/2] kvm tools: Create arch-specific kvm_cpu__emulate_{mm}io()

[Alexander Graf]

On 13.12.2011, at 07:21, Matt Evans wrote:

> Different architectures will deal with MMIO exits differently.  For example,
> KVM_EXIT_IO is x86-specific, and I/O cycles are often synthesised by steering
> into windows in PCI bridges on other architectures.
> 
> This patch calls arch-specific kvm_cpu__emulate_io() and kvm_cpu__emulate_mmio()
> from the main runloop's IO and MMIO exit handlers.  For x86, these directly
> call kvm__emulate_io() and kvm__emulate_mmio() but other architectures will
> perform some address munging before passing on the call.

Why do you need address munging? PIO is simply not there and MMIO always goes to the physical address the CPU sees, so I don't see what you want to munge. The way the memory bus is attached to the CPU should certainly not be modeled differently for PPC and x86.


Alex

Re: [PATCH V3 2/2] kvm tools: Create arch-specific kvm_cpu__emulate_{mm}io()

[Matt Evans]

On 23/12/2011, at 11:58 PM, Alexander Graf wrote:

> 
> On 13.12.2011, at 07:21, Matt Evans wrote:
> 
>> Different architectures will deal with MMIO exits differently.  For example,
>> KVM_EXIT_IO is x86-specific, and I/O cycles are often synthesised by steering
>> into windows in PCI bridges on other architectures.
>> 
>> This patch calls arch-specific kvm_cpu__emulate_io() and kvm_cpu__emulate_mmio()
>> from the main runloop's IO and MMIO exit handlers.  For x86, these directly
>> call kvm__emulate_io() and kvm__emulate_mmio() but other architectures will
>> perform some address munging before passing on the call.
> 
> Why do you need address munging? PIO is simply not there and MMIO always goes to the physical address the CPU sees, so I don't see what you want to munge. The way the memory bus is attached to the CPU should certainly not be modeled differently for PPC and x86.

PIO not there?  PIO is used heavily in kvmtool.  So, I made a window in a similar way to how a real PHB has PIO-window-in-MMIO.

PCI BARs are currently 32-bit.  I don't want to limit the guest RAM to <4G nor puncture holes in it just to make it look like x86... PCI bus addresses == CPU addresses is a bit of an x86ism.  So, I just used another PHB window to offset 32bit PCI MMIO up somewhere else.  We can then use all 4G of PCI MMIO space without putting that at addr 0 and RAM starting >4G.  (And then, exception vectors where?)

The PCI/BARs/MMIO code could really support 64bit addresses though that's a bit of an orthogonal bit of work.  Why should PPC have an MMIO hole in the middle of RAM?


Cheers!


Matt

Re: [PATCH V3 2/2] kvm tools: Create arch-specific kvm_cpu__emulate_{mm}io()

[Alexander Graf]

On 23.12.2011, at 14:26, Matt Evans wrote:

> 
> On 23/12/2011, at 11:58 PM, Alexander Graf wrote:
> 
>> 
>> On 13.12.2011, at 07:21, Matt Evans wrote:
>> 
>>> Different architectures will deal with MMIO exits differently.  For example,
>>> KVM_EXIT_IO is x86-specific, and I/O cycles are often synthesised by steering
>>> into windows in PCI bridges on other architectures.
>>> 
>>> This patch calls arch-specific kvm_cpu__emulate_io() and kvm_cpu__emulate_mmio()
>>> from the main runloop's IO and MMIO exit handlers.  For x86, these directly
>>> call kvm__emulate_io() and kvm__emulate_mmio() but other architectures will
>>> perform some address munging before passing on the call.
>> 
>> Why do you need address munging? PIO is simply not there and MMIO always goes to the physical address the CPU sees, so I don't see what you want to munge. The way the memory bus is attached to the CPU should certainly not be modeled differently for PPC and x86.
> 
> PIO not there?  PIO is used heavily in kvmtool.  So, I made a window in a similar way to how a real PHB has PIO-window-in-MMIO.
> 
> PCI BARs are currently 32-bit.  I don't want to limit the guest RAM to <4G nor puncture holes in it just to make it look like x86... PCI bus addresses == CPU addresses is a bit of an x86ism.  So, I just used another PHB window to offset 32bit PCI MMIO up somewhere else.  We can then use all 4G of PCI MMIO space without putting that at addr 0 and RAM starting >4G.  (And then, exception vectors where?)
> 
> The PCI/BARs/MMIO code could really support 64bit addresses though that's a bit of an orthogonal bit of work.  Why should PPC have an MMIO hole in the middle of RAM?

I fully agree with what you're saying, but the layering seems off. If the CPU gets an MMIO request, it gets that on a physical address from the view of the CPU. Why would you want to have manual munging there to get to whatever window you have? Just map the MMIO regions to the higher addresses and expose whatever different representation you have to the device, not to the CPU layer.

As for PIO, you won't get PIO requests from the CPU, so you can ignore those. All events you get from the CPU are MMIO.


Alex

Re: [PATCH V3 2/2] kvm tools: Create arch-specific kvm_cpu__emulate_{mm}io()

[Matt Evans]

Hey Alex,

On 24/12/11 00:39, Alexander Graf wrote:
> 
> On 23.12.2011, at 14:26, Matt Evans wrote:
> 
>>
>> On 23/12/2011, at 11:58 PM, Alexander Graf wrote:
>>
>>>
>>> On 13.12.2011, at 07:21, Matt Evans wrote:
>>>
>>>> Different architectures will deal with MMIO exits differently.  For example,
>>>> KVM_EXIT_IO is x86-specific, and I/O cycles are often synthesised by steering
>>>> into windows in PCI bridges on other architectures.
>>>>
>>>> This patch calls arch-specific kvm_cpu__emulate_io() and kvm_cpu__emulate_mmio()
>>>> from the main runloop's IO and MMIO exit handlers.  For x86, these directly
>>>> call kvm__emulate_io() and kvm__emulate_mmio() but other architectures will
>>>> perform some address munging before passing on the call.
>>>
>>> Why do you need address munging? PIO is simply not there and MMIO always goes to the physical address the CPU sees, so I don't see what you want to munge. The way the memory bus is attached to the CPU should certainly not be modeled differently for PPC and x86.
>>
>> PIO not there?  PIO is used heavily in kvmtool.  So, I made a window in a similar way to how a real PHB has PIO-window-in-MMIO.
>>
>> PCI BARs are currently 32-bit.  I don't want to limit the guest RAM to <4G
>> nor puncture holes in it just to make it look like x86... PCI bus addresses
>> == CPU addresses is a bit of an x86ism.  So, I just used another PHB window
>> to offset 32bit PCI MMIO up somewhere else.  We can then use all 4G of PCI
>> MMIO space without putting that at addr 0 and RAM starting >4G.  (And then,
>> exception vectors where?)
>>
>> The PCI/BARs/MMIO code could really support 64bit addresses though that's a
>> bit of an orthogonal bit of work.  Why should PPC have an MMIO hole in the
>> middle of RAM?
> 

Sooo.. call it post-holiday bliss but I don't understand what you're saying
here. :)

> I fully agree with what you're saying, but the layering seems off. If the CPU
> gets an MMIO request, it gets that on a physical address from the view of the
  ^^^^ produces?

> CPU. Why would you want to have manual munging there to get to whatever window
> you have? Just map the MMIO regions to the higher addresses and expose
> whatever different representation you have to the device, not to the CPU
> layer.

What do you mean here by "map" and representation?  The only way I can parse
this is as though you're describing PCI devices seeing PCI bus addresses which
CPU MMIOs are converted to by the window offset, i.e. what already exists
i.e. what you're disagreeing with :-)

Sorry.. please explain some more.  Is your suggestion to make CPU phys addresses
and PCI bus addresses 1:1?  (Hole in RAM..)


Thanks!


Matt

Re: [PATCH V3 2/2] kvm tools: Create arch-specific kvm_cpu__emulate_{mm}io()

[Alexander Graf]

On 06.01.2012, at 06:32, Matt Evans wrote:

> Hey Alex,
> 
> On 24/12/11 00:39, Alexander Graf wrote:
>> 
>> On 23.12.2011, at 14:26, Matt Evans wrote:
>> 
>>> 
>>> On 23/12/2011, at 11:58 PM, Alexander Graf wrote:
>>> 
>>>> 
>>>> On 13.12.2011, at 07:21, Matt Evans wrote:
>>>> 
>>>>> Different architectures will deal with MMIO exits differently.  For example,
>>>>> KVM_EXIT_IO is x86-specific, and I/O cycles are often synthesised by steering
>>>>> into windows in PCI bridges on other architectures.
>>>>> 
>>>>> This patch calls arch-specific kvm_cpu__emulate_io() and kvm_cpu__emulate_mmio()
>>>>> from the main runloop's IO and MMIO exit handlers.  For x86, these directly
>>>>> call kvm__emulate_io() and kvm__emulate_mmio() but other architectures will
>>>>> perform some address munging before passing on the call.
>>>> 
>>>> Why do you need address munging? PIO is simply not there and MMIO always goes to the physical address the CPU sees, so I don't see what you want to munge. The way the memory bus is attached to the CPU should certainly not be modeled differently for PPC and x86.
>>> 
>>> PIO not there?  PIO is used heavily in kvmtool.  So, I made a window in a similar way to how a real PHB has PIO-window-in-MMIO.
>>> 
>>> PCI BARs are currently 32-bit.  I don't want to limit the guest RAM to <4G
>>> nor puncture holes in it just to make it look like x86... PCI bus addresses
>>> == CPU addresses is a bit of an x86ism.  So, I just used another PHB window
>>> to offset 32bit PCI MMIO up somewhere else.  We can then use all 4G of PCI
>>> MMIO space without putting that at addr 0 and RAM starting >4G.  (And then,
>>> exception vectors where?)
>>> 
>>> The PCI/BARs/MMIO code could really support 64bit addresses though that's a
>>> bit of an orthogonal bit of work.  Why should PPC have an MMIO hole in the
>>> middle of RAM?
>> 
> 
> Sooo.. call it post-holiday bliss but I don't understand what you're saying
> here. :)
> 
>> I fully agree with what you're saying, but the layering seems off. If the CPU
>> gets an MMIO request, it gets that on a physical address from the view of the
>  ^^^^ produces?
> 
>> CPU. Why would you want to have manual munging there to get to whatever window
>> you have? Just map the MMIO regions to the higher addresses and expose
>> whatever different representation you have to the device, not to the CPU
>> layer.
> 
> What do you mean here by "map" and representation?  The only way I can parse
> this is as though you're describing PCI devices seeing PCI bus addresses which
> CPU MMIOs are converted to by the window offset, i.e. what already exists
> i.e. what you're disagreeing with :-)

Yup :). There's no reason you only have a single PCI bus. Or only PCI in general too. Having a single offset there is a tremendous oversimplification :)


Alex