[PATCH 0/4] Start using KVMState

[PATCH 0/4] Start using KVMState

[Glauber Costa]

Guys,

This is to address the feedback I received in the last attempt.

I'm now attempting to move kvm_context inside of KVMState
Patch #3 should be applied to qemu. I'll relay it to Anthony
if we reach consensus on it here.
Patch #4 is an example use of KVMState, to show how I intend to proceed.

Patches 1 and 2 are cleanups that can be applied separatedly if we want to.

[PATCH 1/4] get rid of libkvm-common.h

[Glauber Costa]

This file has no purpose in life in new scheme of things. Move
its contents to libkvm-all.h. There are some name clashes
in qemu-kvm.c. Those function names are then changed.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 kvm.h                |    1 +
 libkvm-all.h         |   82 ++++++++++++++++++++++++++++++++++++++++-
 libkvm-common.h      |  100 --------------------------------------------------
 qemu-kvm-x86.c       |    2 +
 qemu-kvm.c           |   12 +++---
 target-i386/libkvm.h |    2 +-
 target-ia64/libkvm.h |    2 +-
 target-ppc/libkvm.h  |    2 +-
 8 files changed, 92 insertions(+), 111 deletions(-)
 delete mode 100644 libkvm-common.h

diff --git a/kvm.h b/kvm.h
index 0c3e7b1..e1c6eba 100644
--- a/kvm.h
+++ b/kvm.h
@@ -16,6 +16,7 @@
 
 #include "config.h"
 #include "sys-queue.h"
+#include "libkvm-all.h"
 
 #ifdef KVM_UPSTREAM
 
diff --git a/libkvm-all.h b/libkvm-all.h
index 2b18c00..03b98df 100644
--- a/libkvm-all.h
+++ b/libkvm-all.h
@@ -19,12 +19,90 @@
 
 #include <signal.h>
 
-struct kvm_context;
-struct kvm_vcpu_context;
+/* FIXME: share this number with kvm */
+/* FIXME: or dynamically alloc/realloc regions */
+#ifdef __s390__
+#define KVM_MAX_NUM_MEM_REGIONS 1u
+#define MAX_VCPUS 64
+#define LIBKVM_S390_ORIGIN (0UL)
+#elif defined(__ia64__)
+#define KVM_MAX_NUM_MEM_REGIONS 32u
+#define MAX_VCPUS 256
+#else
+#define KVM_MAX_NUM_MEM_REGIONS 32u
+#define MAX_VCPUS 16
+#endif
+
+/* kvm abi verison variable */
+extern int kvm_abi;
+
+/**
+ * \brief The KVM context
+ *
+ * The verbose KVM context
+ */
+
+struct kvm_context {
+	/// Filedescriptor to /dev/kvm
+	int fd;
+	int vm_fd;
+	/// Callbacks that KVM uses to emulate various unvirtualizable functionality
+	struct kvm_callbacks *callbacks;
+	void *opaque;
+	/// is dirty pages logging enabled for all regions or not
+	int dirty_pages_log_all;
+	/// do not create in-kernel irqchip if set
+	int no_irqchip_creation;
+	/// in-kernel irqchip status
+	int irqchip_in_kernel;
+	/// ioctl to use to inject interrupts
+	int irqchip_inject_ioctl;
+	/// do not create in-kernel pit if set
+	int no_pit_creation;
+	/// in-kernel pit status
+	int pit_in_kernel;
+	/// in-kernel coalesced mmio
+	int coalesced_mmio;
+#ifdef KVM_CAP_IRQ_ROUTING
+	struct kvm_irq_routing *irq_routes;
+	int nr_allocated_irq_routes;
+#endif
+	void *used_gsi_bitmap;
+	int max_gsi;
+};
+
+struct kvm_vcpu_context
+{
+	int fd;
+	struct kvm_run *run;
+	struct kvm_context *kvm;
+	uint32_t id;
+};
 
 typedef struct kvm_context *kvm_context_t;
 typedef struct kvm_vcpu_context *kvm_vcpu_context_t;
 
+int kvm_alloc_kernel_memory(kvm_context_t kvm, unsigned long memory,
+								void **vm_mem);
+int kvm_alloc_userspace_memory(kvm_context_t kvm, unsigned long memory,
+								void **vm_mem);
+
+int kvm_arch_create(kvm_context_t kvm, unsigned long phys_mem_bytes,
+                        void **vm_mem);
+int kvm_arch_run(kvm_vcpu_context_t vcpu);
+
+
+void kvm_show_code(kvm_vcpu_context_t vcpu);
+
+int handle_halt(kvm_vcpu_context_t vcpu);
+int handle_shutdown(kvm_context_t kvm, void *env);
+void post_kvm_run(kvm_context_t kvm, void *env);
+int pre_kvm_run(kvm_context_t kvm, void *env);
+int handle_io_window(kvm_context_t kvm);
+int handle_debug(kvm_vcpu_context_t vcpu, void *env);
+int try_push_interrupts(kvm_context_t kvm);
+
+
 #if defined(__x86_64__) || defined(__i386__)
 struct kvm_msr_list *kvm_get_msr_list(kvm_context_t);
 int kvm_get_msrs(kvm_vcpu_context_t, struct kvm_msr_entry *msrs, int n);
diff --git a/libkvm-common.h b/libkvm-common.h
deleted file mode 100644
index dc5b667..0000000
--- a/libkvm-common.h
+++ /dev/null
@@ -1,100 +0,0 @@
-/*
- * This header is for functions & variables that will ONLY be
- * used inside libkvm.
- *
- * derived from libkvm.c
- *
- * Copyright (C) 2006 Qumranet, Inc.
- *
- * Authors:
- *	Avi Kivity   <avi@qumranet.com>
- *	Yaniv Kamay  <yaniv@qumranet.com>
- *
- *   This work is licensed under the GNU LGPL license, version 2.
- */
-
-#ifndef KVM_COMMON_H
-#define KVM_COMMON_H
-
-/* FIXME: share this number with kvm */
-/* FIXME: or dynamically alloc/realloc regions */
-#ifdef __s390__
-#define KVM_MAX_NUM_MEM_REGIONS 1u
-#define MAX_VCPUS 64
-#define LIBKVM_S390_ORIGIN (0UL)
-#elif defined(__ia64__)
-#define KVM_MAX_NUM_MEM_REGIONS 32u
-#define MAX_VCPUS 256
-#else
-#define KVM_MAX_NUM_MEM_REGIONS 32u
-#define MAX_VCPUS 16
-#endif
-
-
-/* kvm abi verison variable */
-extern int kvm_abi;
-
-/**
- * \brief The KVM context
- *
- * The verbose KVM context
- */
-
-struct kvm_context {
-	/// Filedescriptor to /dev/kvm
-	int fd;
-	int vm_fd;
-	/// Callbacks that KVM uses to emulate various unvirtualizable functionality
-	struct kvm_callbacks *callbacks;
-	void *opaque;
-	/// is dirty pages logging enabled for all regions or not
-	int dirty_pages_log_all;
-	/// do not create in-kernel irqchip if set
-	int no_irqchip_creation;
-	/// in-kernel irqchip status
-	int irqchip_in_kernel;
-	/// ioctl to use to inject interrupts
-	int irqchip_inject_ioctl;
-	/// do not create in-kernel pit if set
-	int no_pit_creation;
-	/// in-kernel pit status
-	int pit_in_kernel;
-	/// in-kernel coalesced mmio
-	int coalesced_mmio;
-#ifdef KVM_CAP_IRQ_ROUTING
-	struct kvm_irq_routing *irq_routes;
-	int nr_allocated_irq_routes;
-#endif
-	void *used_gsi_bitmap;
-	int max_gsi;
-};
-
-struct kvm_vcpu_context
-{
-	int fd;
-	struct kvm_run *run;
-	struct kvm_context *kvm;
-	uint32_t id;
-};
-
-int kvm_alloc_kernel_memory(kvm_context_t kvm, unsigned long memory,
-								void **vm_mem);
-int kvm_alloc_userspace_memory(kvm_context_t kvm, unsigned long memory,
-								void **vm_mem);
-
-int kvm_arch_create(kvm_context_t kvm, unsigned long phys_mem_bytes,
-                        void **vm_mem);
-int kvm_arch_run(kvm_vcpu_context_t vcpu);
-
-
-void kvm_show_code(kvm_vcpu_context_t vcpu);
-
-int handle_halt(kvm_vcpu_context_t vcpu);
-int handle_shutdown(kvm_context_t kvm, void *env);
-void post_kvm_run(kvm_context_t kvm, void *env);
-int pre_kvm_run(kvm_context_t kvm, void *env);
-int handle_io_window(kvm_context_t kvm);
-int handle_debug(kvm_vcpu_context_t vcpu, void *env);
-int try_push_interrupts(kvm_context_t kvm);
-
-#endif
diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index 5526d8f..729d600 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -20,6 +20,8 @@
 #include <sys/utsname.h>
 #include <linux/kvm_para.h>
 
+#include "kvm.h"
+
 #define MSR_IA32_TSC		0x10
 
 static struct kvm_msr_list *kvm_msr_list;
diff --git a/qemu-kvm.c b/qemu-kvm.c
index 7acc0ef..2aeb17c 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -149,12 +149,12 @@ void kvm_apic_init(CPUState *env)
 
 #include <signal.h>
 
-static int try_push_interrupts(void *opaque)
+static int kvm_try_push_interrupts(void *opaque)
 {
     return kvm_arch_try_push_interrupts(opaque);
 }
 
-static void post_kvm_run(void *opaque, void *data)
+static void kvm_post_run(void *opaque, void *data)
 {
     CPUState *env = (CPUState *)data;
 
@@ -162,7 +162,7 @@ static void post_kvm_run(void *opaque, void *data)
     kvm_arch_post_kvm_run(opaque, env);
 }
 
-static int pre_kvm_run(void *opaque, void *data)
+static int kvm_pre_run(void *opaque, void *data)
 {
     CPUState *env = (CPUState *)data;
 
@@ -761,12 +761,12 @@ static struct kvm_callbacks qemu_kvm_ops = {
     .halt  = kvm_halt,
     .shutdown = kvm_shutdown,
     .io_window = kvm_io_window,
-    .try_push_interrupts = try_push_interrupts,
+    .try_push_interrupts = kvm_try_push_interrupts,
 #ifdef KVM_CAP_USER_NMI
     .push_nmi = kvm_arch_push_nmi,
 #endif
-    .post_kvm_run = post_kvm_run,
-    .pre_kvm_run = pre_kvm_run,
+    .post_kvm_run = kvm_post_run,
+    .pre_kvm_run = kvm_pre_run,
 #ifdef TARGET_I386
     .tpr_access = handle_tpr_access,
 #endif
diff --git a/target-i386/libkvm.h b/target-i386/libkvm.h
index 081e010..f3fc6dc 100644
--- a/target-i386/libkvm.h
+++ b/target-i386/libkvm.h
@@ -18,7 +18,7 @@
 #ifndef KVM_X86_H
 #define KVM_X86_H
 
-#include "libkvm-common.h"
+#include "libkvm-all.h"
 
 #define PAGE_SIZE 4096ul
 #define PAGE_MASK (~(PAGE_SIZE - 1))
diff --git a/target-ia64/libkvm.h b/target-ia64/libkvm.h
index f084420..417f7f1 100644
--- a/target-ia64/libkvm.h
+++ b/target-ia64/libkvm.h
@@ -18,7 +18,7 @@
 #ifndef KVM_IA64_H
 #define KVM_IA64_H
 
-#include "libkvm-common.h"
+#include "libkvm-all.h"
 
 extern int kvm_page_size;
 
diff --git a/target-ppc/libkvm.h b/target-ppc/libkvm.h
index 95c314d..80b6b06 100644
--- a/target-ppc/libkvm.h
+++ b/target-ppc/libkvm.h
@@ -19,7 +19,7 @@
 #ifndef KVM_POWERPC_H
 #define KVM_POWERPC_H
 
-#include "libkvm-common.h"
+#include "libkvm-all.h"
 
 extern int kvm_page_size;
 
-- 
1.5.6.6

[PATCH 2/4] pull qemu headers into libkvm

[Glauber Costa]

Those headers define qemu specific things like ram_addr_t.
This will allow us to start using them in libkvm.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 libkvm-all.c         |    2 +-
 libkvm-all.h         |    1 +
 target-i386/libkvm.c |    2 ++
 3 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/libkvm-all.c b/libkvm-all.c
index dd56498..45679fb 100644
--- a/libkvm-all.c
+++ b/libkvm-all.c
@@ -26,6 +26,7 @@
 #error libkvm: userspace and kernel version mismatch
 #endif
 
+#include "sysemu.h"
 #include <unistd.h>
 #include <fcntl.h>
 #include <stdio.h>
@@ -47,7 +48,6 @@
 #define DPRINTF(fmt, args...) do {} while (0)
 #endif
 
-#define MIN(x,y) ((x) < (y) ? (x) : (y))
 #define ALIGN(x, y) (((x)+(y)-1) & ~((y)-1))
 
 int kvm_abi = EXPECTED_KVM_API_VERSION;
diff --git a/libkvm-all.h b/libkvm-all.h
index 03b98df..d647ef1 100644
--- a/libkvm-all.h
+++ b/libkvm-all.h
@@ -82,6 +82,7 @@ struct kvm_vcpu_context
 typedef struct kvm_context *kvm_context_t;
 typedef struct kvm_vcpu_context *kvm_vcpu_context_t;
 
+#include "kvm.h"
 int kvm_alloc_kernel_memory(kvm_context_t kvm, unsigned long memory,
 								void **vm_mem);
 int kvm_alloc_userspace_memory(kvm_context_t kvm, unsigned long memory,
diff --git a/target-i386/libkvm.c b/target-i386/libkvm.c
index f88102e..0f4e009 100644
--- a/target-i386/libkvm.c
+++ b/target-i386/libkvm.c
@@ -1,3 +1,5 @@
+#include "sysemu.h"
+
 #include "libkvm-all.h"
 #include "libkvm.h"
 #include <errno.h>
-- 
1.5.6.6

[PATCH 3/4] Move KVMState to common header

[Glauber Costa]

This patch should be applied to main qemu, but I'll
first post it here for appreciation. In this patch,
we move KVMState definition to kvm.h header. With this
done, we can also use its definition in our files, until
there is no more such thing as "our" files. This is too
selfish anyway.

Later on, we'll move our internal state inside it.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 kvm-all.c  |   21 ---------------------
 kvm.h      |   40 +++++++++++++++++++++++++++++++---------
 qemu-kvm.h |    8 --------
 3 files changed, 31 insertions(+), 38 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index c89e3b1..d60126c 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -39,32 +39,11 @@
     do { } while (0)
 #endif
 
-typedef struct KVMSlot
-{
-    target_phys_addr_t start_addr;
-    ram_addr_t memory_size;
-    ram_addr_t phys_offset;
-    int slot;
-    int flags;
-} KVMSlot;
 
 typedef struct kvm_dirty_log KVMDirtyLog;
 
 int kvm_allowed = 0;
 
-struct KVMState
-{
-    KVMSlot slots[32];
-    int fd;
-    int vmfd;
-    int coalesced_mmio;
-    int broken_set_mem_region;
-    int migration_log;
-#ifdef KVM_CAP_SET_GUEST_DEBUG
-    struct kvm_sw_breakpoint_head kvm_sw_breakpoints;
-#endif
-};
-
 static KVMState *kvm_state;
 
 static KVMSlot *kvm_alloc_slot(KVMState *s)
diff --git a/kvm.h b/kvm.h
index e1c6eba..553c03e 100644
--- a/kvm.h
+++ b/kvm.h
@@ -18,6 +18,37 @@
 #include "sys-queue.h"
 #include "libkvm-all.h"
 
+typedef struct KVMSlot
+{
+    target_phys_addr_t start_addr;
+    ram_addr_t memory_size;
+    ram_addr_t phys_offset;
+    int slot;
+    int flags;
+} KVMSlot;
+
+struct kvm_sw_breakpoint {
+    target_ulong pc;
+    target_ulong saved_insn;
+    int use_count;
+    TAILQ_ENTRY(kvm_sw_breakpoint) entry;
+};
+
+TAILQ_HEAD(kvm_sw_breakpoint_head, kvm_sw_breakpoint);
+
+struct KVMState
+{
+    KVMSlot slots[32];
+    int fd;
+    int vmfd;
+    int coalesced_mmio;
+    int broken_set_mem_region;
+    int migration_log;
+#ifdef KVM_CAP_SET_GUEST_DEBUG
+    struct kvm_sw_breakpoint_head kvm_sw_breakpoints;
+#endif
+};
+
 #ifdef KVM_UPSTREAM
 
 #ifdef CONFIG_KVM
@@ -97,15 +128,6 @@ int kvm_arch_init_vcpu(CPUState *env);
 struct kvm_guest_debug;
 struct kvm_debug_exit_arch;
 
-struct kvm_sw_breakpoint {
-    target_ulong pc;
-    target_ulong saved_insn;
-    int use_count;
-    TAILQ_ENTRY(kvm_sw_breakpoint) entry;
-};
-
-TAILQ_HEAD(kvm_sw_breakpoint_head, kvm_sw_breakpoint);
-
 int kvm_arch_debug(struct kvm_debug_exit_arch *arch_info);
 
 struct kvm_sw_breakpoint *kvm_find_sw_breakpoint(CPUState *env,
diff --git a/qemu-kvm.h b/qemu-kvm.h
index fa40542..88bbba4 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -81,14 +81,6 @@ void kvm_arch_cpu_reset(CPUState *env);
 struct kvm_guest_debug;
 struct kvm_debug_exit_arch;
 
-struct kvm_sw_breakpoint {
-    target_ulong pc;
-    target_ulong saved_insn;
-    int use_count;
-    TAILQ_ENTRY(kvm_sw_breakpoint) entry;
-};
-TAILQ_HEAD(kvm_sw_breakpoint_head, kvm_sw_breakpoint);
-
 extern struct kvm_sw_breakpoint_head kvm_sw_breakpoints;
 
 int kvm_arch_debug(struct kvm_debug_exit_arch *arch_info);
-- 
1.5.6.6

[PATCH 4/4] Use KVMState as main state container

[Glauber Costa]

Put kvm_context inside KVMState. We can then start using
KVMState where we need to, to mid-term, start sharing
code with qemu mainline.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 kvm.h        |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
 libkvm-all.c |   19 ++++++++++---------
 libkvm-all.h |   47 +----------------------------------------------
 qemu-kvm.c   |    9 +++++----
 4 files changed, 68 insertions(+), 62 deletions(-)

diff --git a/kvm.h b/kvm.h
index 553c03e..32af545 100644
--- a/kvm.h
+++ b/kvm.h
@@ -16,7 +16,53 @@
 
 #include "config.h"
 #include "sys-queue.h"
-#include "libkvm-all.h"
+
+/**
+ * \brief The KVM context
+ *
+ * The verbose KVM context
+ */
+
+struct kvm_context {
+	/// Filedescriptor to /dev/kvm
+	int fd;
+	int vm_fd;
+	/// Callbacks that KVM uses to emulate various unvirtualizable functionality
+	struct kvm_callbacks *callbacks;
+	void *opaque;
+	/// is dirty pages logging enabled for all regions or not
+	int dirty_pages_log_all;
+	/// do not create in-kernel irqchip if set
+	int no_irqchip_creation;
+	/// in-kernel irqchip status
+	int irqchip_in_kernel;
+	/// ioctl to use to inject interrupts
+	int irqchip_inject_ioctl;
+	/// do not create in-kernel pit if set
+	int no_pit_creation;
+	/// in-kernel pit status
+	int pit_in_kernel;
+	/// in-kernel coalesced mmio
+	int coalesced_mmio;
+#ifdef KVM_CAP_IRQ_ROUTING
+	struct kvm_irq_routing *irq_routes;
+	int nr_allocated_irq_routes;
+#endif
+	void *used_gsi_bitmap;
+	int max_gsi;
+};
+
+struct kvm_vcpu_context
+{
+	int fd;
+	struct kvm_run *run;
+	struct kvm_context *kvm;
+	uint32_t id;
+};
+
+typedef struct kvm_context *kvm_context_t;
+typedef struct kvm_vcpu_context *kvm_vcpu_context_t;
+
 
 typedef struct KVMSlot
 {
@@ -47,8 +93,13 @@ struct KVMState
 #ifdef KVM_CAP_SET_GUEST_DEBUG
     struct kvm_sw_breakpoint_head kvm_sw_breakpoints;
 #endif
+#ifdef USE_KVM
+    struct kvm_context kvm_context;
+#endif
 };
 
+typedef struct KVMState KVMState;
+
 #ifdef KVM_UPSTREAM
 
 #ifdef CONFIG_KVM
@@ -97,8 +148,6 @@ int kvm_update_guest_debug(CPUState *env, unsigned long reinject_trap);
 
 /* internal API */
 
-struct KVMState;
-typedef struct KVMState KVMState;
 
 int kvm_ioctl(KVMState *s, int type, ...);
 
diff --git a/libkvm-all.c b/libkvm-all.c
index 45679fb..32fdf96 100644
--- a/libkvm-all.c
+++ b/libkvm-all.c
@@ -288,11 +288,12 @@ int kvm_dirty_pages_log_reset(kvm_context_t kvm)
 }
 
 
-kvm_context_t kvm_init(struct kvm_callbacks *callbacks,
+KVMState *kvm_init(struct kvm_callbacks *callbacks,
 		       void *opaque)
 {
 	int fd;
 	kvm_context_t kvm;
+    KVMState *s;
 	int r, gsi_count;
 
 	fd = open("/dev/kvm", O_RDWR);
@@ -319,10 +320,9 @@ kvm_context_t kvm_init(struct kvm_callbacks *callbacks,
 	}
 	kvm_abi = r;
 	kvm_page_size = getpagesize();
-	kvm = malloc(sizeof(*kvm));
-	if (kvm == NULL)
-		goto out_close;
-	memset(kvm, 0, sizeof(*kvm));
+	s = qemu_malloc(sizeof(*s));
+	memset(s, 0, sizeof(*s));
+    kvm = &s->kvm_context;
 	kvm->fd = fd;
 	kvm->vm_fd = -1;
 	kvm->callbacks = callbacks;
@@ -348,10 +348,11 @@ kvm_context_t kvm_init(struct kvm_callbacks *callbacks,
 			set_gsi(kvm, i);
 	}
 
-	return kvm;
- out_close:
-	close(fd);
-	return NULL;
+	return s;
+
+out_close:
+    close(fd);
+    return NULL;
 }
 
 void kvm_finalize(kvm_context_t kvm)
diff --git a/libkvm-all.h b/libkvm-all.h
index d647ef1..251dede 100644
--- a/libkvm-all.h
+++ b/libkvm-all.h
@@ -36,51 +36,6 @@
 /* kvm abi verison variable */
 extern int kvm_abi;
 
-/**
- * \brief The KVM context
- *
- * The verbose KVM context
- */
-
-struct kvm_context {
-	/// Filedescriptor to /dev/kvm
-	int fd;
-	int vm_fd;
-	/// Callbacks that KVM uses to emulate various unvirtualizable functionality
-	struct kvm_callbacks *callbacks;
-	void *opaque;
-	/// is dirty pages logging enabled for all regions or not
-	int dirty_pages_log_all;
-	/// do not create in-kernel irqchip if set
-	int no_irqchip_creation;
-	/// in-kernel irqchip status
-	int irqchip_in_kernel;
-	/// ioctl to use to inject interrupts
-	int irqchip_inject_ioctl;
-	/// do not create in-kernel pit if set
-	int no_pit_creation;
-	/// in-kernel pit status
-	int pit_in_kernel;
-	/// in-kernel coalesced mmio
-	int coalesced_mmio;
-#ifdef KVM_CAP_IRQ_ROUTING
-	struct kvm_irq_routing *irq_routes;
-	int nr_allocated_irq_routes;
-#endif
-	void *used_gsi_bitmap;
-	int max_gsi;
-};
-
-struct kvm_vcpu_context
-{
-	int fd;
-	struct kvm_run *run;
-	struct kvm_context *kvm;
-	uint32_t id;
-};
-
-typedef struct kvm_context *kvm_context_t;
-typedef struct kvm_vcpu_context *kvm_vcpu_context_t;
 
 #include "kvm.h"
 int kvm_alloc_kernel_memory(kvm_context_t kvm, unsigned long memory,
@@ -181,7 +136,7 @@ struct kvm_callbacks {
  * \param opaque Not used
  * \return NULL on failure
  */
-kvm_context_t kvm_init(struct kvm_callbacks *callbacks,
+KVMState *kvm_init(struct kvm_callbacks *callbacks,
 		       void *opaque);
 
 /*!
diff --git a/qemu-kvm.c b/qemu-kvm.c
index 2aeb17c..b441970 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -33,6 +33,8 @@ int kvm_irqchip = 1;
 int kvm_pit = 1;
 int kvm_pit_reinject = 1;
 int kvm_nested = 0;
+KVMState *kvm_state;
+
 kvm_context_t kvm_context;
 
 pthread_mutex_t qemu_mutex = PTHREAD_MUTEX_INITIALIZER;
@@ -780,10 +782,9 @@ static struct kvm_callbacks qemu_kvm_ops = {
 int kvm_qemu_init()
 {
     /* Try to initialize kvm */
-    kvm_context = kvm_init(&qemu_kvm_ops, cpu_single_env);
-    if (!kvm_context) {
-      	return -1;
-    }
+    kvm_state = kvm_init(&qemu_kvm_ops, cpu_single_env);
+    kvm_context = &kvm_state->kvm_context;
+
     pthread_mutex_lock(&qemu_mutex);
 
     return 0;
-- 
1.5.6.6

Re: [PATCH 3/4] Move KVMState to common header

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1205 bytes --]

Glauber Costa wrote:
> This patch should be applied to main qemu, but I'll
> first post it here for appreciation. In this patch,
> we move KVMState definition to kvm.h header. With this
> done, we can also use its definition in our files, until
> there is no more such thing as "our" files. This is too
> selfish anyway.
> 
> Later on, we'll move our internal state inside it.

Well, in upstream no one outside kvm-all.c needs to (and likely should
be allowed to) access fields from struct KVMState & KVMSlot directly.
That avoids misuse outside the KVM layer and enforces KVM arch code to
properly call into the generic layer.

But I see the problem for qemu-kvm's transition time, so let's try to
find an intermediate solution until its code layout is aligned (I don't
see any blockers for this). Suggestion: Replicate the relevant
structures into a new, temporary header. If upstream may extend its
original structures, this should from now on have happened *first*
inside qemu-kvm, so no inconsistency can arise unless downstream messed
it up already. At some point (hopefully not too far away), no user of
that header will remain and we will be able to drop it again.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [PATCH 3/4] Move KVMState to common header

[Glauber Costa]

On Mon, Jun 08, 2009 at 09:54:34PM +0200, Jan Kiszka wrote:
> Glauber Costa wrote:
> > This patch should be applied to main qemu, but I'll
> > first post it here for appreciation. In this patch,
> > we move KVMState definition to kvm.h header. With this
> > done, we can also use its definition in our files, until
> > there is no more such thing as "our" files. This is too
> > selfish anyway.
> > 
> > Later on, we'll move our internal state inside it.
> 
> Well, in upstream no one outside kvm-all.c needs to (and likely should
> be allowed to) access fields from struct KVMState & KVMSlot directly.
> That avoids misuse outside the KVM layer and enforces KVM arch code to
> properly call into the generic layer.
> 
> But I see the problem for qemu-kvm's transition time, so let's try to
> find an intermediate solution until its code layout is aligned (I don't
> see any blockers for this). Suggestion: Replicate the relevant
> structures into a new, temporary header. If upstream may extend its
> original structures, this should from now on have happened *first*
> inside qemu-kvm, so no inconsistency can arise unless downstream messed
> it up already. At some point (hopefully not too far away), no user of
> that header will remain and we will be able to drop it again.
I'm fine with whatever anthony wants.

> 
> Jan
> 

Re: [PATCH 3/4] Move KVMState to common header

[Anthony Liguori]

Glauber Costa wrote:
> On Mon, Jun 08, 2009 at 09:54:34PM +0200, Jan Kiszka wrote:
>   
>> Glauber Costa wrote:
>>     
>>> This patch should be applied to main qemu, but I'll
>>> first post it here for appreciation. In this patch,
>>> we move KVMState definition to kvm.h header. With this
>>> done, we can also use its definition in our files, until
>>> there is no more such thing as "our" files. This is too
>>> selfish anyway.
>>>
>>> Later on, we'll move our internal state inside it.
>>>       
>> Well, in upstream no one outside kvm-all.c needs to (and likely should
>> be allowed to) access fields from struct KVMState & KVMSlot directly.
>> That avoids misuse outside the KVM layer and enforces KVM arch code to
>> properly call into the generic layer.
>>
>> But I see the problem for qemu-kvm's transition time, so let's try to
>> find an intermediate solution until its code layout is aligned (I don't
>> see any blockers for this). Suggestion: Replicate the relevant
>> structures into a new, temporary header. If upstream may extend its
>> original structures, this should from now on have happened *first*
>> inside qemu-kvm, so no inconsistency can arise unless downstream messed
>> it up already. At some point (hopefully not too far away), no user of
>> that header will remain and we will be able to drop it again.
>>     
> I'm fine with whatever anthony wants.
>   

I'm fine with either solution as they are both temporary measures...

Regards,

Anthony Liguori

Re: [PATCH 0/4] Start using KVMState

[Avi Kivity]

On 06/08/2009 10:10 PM, Glauber Costa wrote:
> Guys,
>
> This is to address the feedback I received in the last attempt.
>    

Does this fix the tpr patching regression?  What was the problem?

It always helps to explicitly list changes and add a version number to 
patchsets, to help maintainers with limited memory capacity.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 0/4] Start using KVMState

[Glauber Costa]

On Thu, Jun 18, 2009 at 12:15:12PM +0300, Avi Kivity wrote:
> On 06/08/2009 10:10 PM, Glauber Costa wrote:
>> Guys,
>>
>> This is to address the feedback I received in the last attempt.
>>    
>
> Does this fix the tpr patching regression?  What was the problem?
>
> It always helps to explicitly list changes and add a version number to  
> patchsets, to help maintainers with limited memory capacity.
No, forget about these one. The one you should apply is the one that
moves libkvm into qemu-kvm for i386. Subj is [PATCH] get rid of target-i386/libkvm.c

There is no tpr patching regression, since you said you did not push my previous
series (that removes the callbacks). It (get rid of ...) will just prevent it from 
happening whenever I send it again.