[patch] pass opague CPUState through libkvm instead of int vcpu

[patch] pass opague CPUState through libkvm instead of int vcpu

[Jes Sorensen]

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

Avi Kivity wrote:
> Jes Sorensen wrote:
>>> That's a little ugly -- passing two arguments which describe the 
>>> vcpu.  How about passing env to kvm_create_vcpu() instead?
>>
>> I am all in favor of doing this, but in order to do so, we have to
>> teach libkvm about CPUState - are you ok with that?
> 
> No, libkvm is independent of qemu.
> 
> We can make kvm_create_vcpu(kvm_context_t kvm, int vcpu, void *env), and 
> pass 'env' as the opaque to all the callbacks, instead of the vcpu number.

Hi,

I pulled out the part of the patch that does the opague 'env' pointer.
It should be pretty straight forward - the vcpu_info patch will go on
top of this, but I'll work on that tomorrow.

Let me know what you think - this is about as small I can make this
patch :-)

Tested on ia64.

Cheers,
Jes

[-- Attachment #2: 0009-qemu-kvm-opaque-cpustate.patch --]
[-- Type: text/plain, Size: 9724 bytes --]

Change code to pass around opague pointer to CPUState through libkvm,
avoiding conversion from CPUState to int vcpu and back in the callbacks
into qemu.

Signed-off-by: Jes Sorensen <jes@sgi.com>

 ---
 libkvm/kvm-common.h     |    8 ++++----
 libkvm/libkvm.c         |   28 ++++++++++++++--------------
 libkvm/libkvm.h         |   10 +++++-----
 qemu/qemu-kvm-ia64.c    |    4 ++--
 qemu/qemu-kvm-powerpc.c |    5 ++---
 qemu/qemu-kvm-x86.c     |   11 +++++------
 qemu/qemu-kvm.c         |   21 ++++++++++++---------
 qemu/qemu-kvm.h         |    4 ++--
 8 files changed, 46 insertions(+), 45 deletions(-)

Index: kvm-userspace.git/libkvm/kvm-common.h
===================================================================
--- kvm-userspace.git.orig/libkvm/kvm-common.h
+++ kvm-userspace.git/libkvm/kvm-common.h
@@ -84,11 +84,11 @@
 void kvm_show_code(kvm_context_t kvm, int vcpu);
 
 int handle_halt(kvm_context_t kvm, int vcpu);
-int handle_shutdown(kvm_context_t kvm, int vcpu);
-void post_kvm_run(kvm_context_t kvm, int vcpu);
-int pre_kvm_run(kvm_context_t kvm, int 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_context_t kvm, int vcpu);
+int handle_debug(kvm_context_t kvm, void *env);
 int try_push_interrupts(kvm_context_t kvm);
 
 #endif
Index: kvm-userspace.git/libkvm/libkvm.c
===================================================================
--- kvm-userspace.git.orig/libkvm/libkvm.c
+++ kvm-userspace.git/libkvm/libkvm.c
@@ -738,9 +738,9 @@
 	return 0;
 }
 
-int handle_debug(kvm_context_t kvm, int vcpu)
+int handle_debug(kvm_context_t kvm, void *env)
 {
-	return kvm->callbacks->debug(kvm->opaque, vcpu);
+	return kvm->callbacks->debug(kvm->opaque, env);
 }
 
 int kvm_get_regs(kvm_context_t kvm, int vcpu, struct kvm_regs *regs)
@@ -822,9 +822,9 @@
 	return kvm->callbacks->halt(kvm->opaque, vcpu);
 }
 
-int handle_shutdown(kvm_context_t kvm, int vcpu)
+int handle_shutdown(kvm_context_t kvm, void *env)
 {
-	return kvm->callbacks->shutdown(kvm->opaque, vcpu);
+	return kvm->callbacks->shutdown(kvm->opaque, env);
 }
 
 int try_push_interrupts(kvm_context_t kvm)
@@ -837,14 +837,14 @@
 	return kvm->callbacks->try_push_nmi(kvm->opaque);
 }
 
-void post_kvm_run(kvm_context_t kvm, int vcpu)
+void post_kvm_run(kvm_context_t kvm, void *env)
 {
-	kvm->callbacks->post_kvm_run(kvm->opaque, vcpu);
+	kvm->callbacks->post_kvm_run(kvm->opaque, env);
 }
 
-int pre_kvm_run(kvm_context_t kvm, int vcpu)
+int pre_kvm_run(kvm_context_t kvm, void *env)
 {
-	return kvm->callbacks->pre_kvm_run(kvm->opaque, vcpu);
+	return kvm->callbacks->pre_kvm_run(kvm->opaque, env);
 }
 
 int kvm_get_interrupt_flag(kvm_context_t kvm, int vcpu)
@@ -872,7 +872,7 @@
 #endif
 }
 
-int kvm_run(kvm_context_t kvm, int vcpu)
+int kvm_run(kvm_context_t kvm, int vcpu, void *env)
 {
 	int r;
 	int fd = kvm->vcpu_fd[vcpu];
@@ -886,19 +886,19 @@
 	if (!kvm->irqchip_in_kernel)
 		run->request_interrupt_window = try_push_interrupts(kvm);
 #endif
-	r = pre_kvm_run(kvm, vcpu);
+	r = pre_kvm_run(kvm, env);
 	if (r)
 	    return r;
 	r = ioctl(fd, KVM_RUN, 0);
 
 	if (r == -1 && errno != EINTR && errno != EAGAIN) {
 		r = -errno;
-		post_kvm_run(kvm, vcpu);
+		post_kvm_run(kvm, env);
 		fprintf(stderr, "kvm_run: %s\n", strerror(-r));
 		return r;
 	}
 
-	post_kvm_run(kvm, vcpu);
+	post_kvm_run(kvm, env);
 
 #if defined(KVM_CAP_COALESCED_MMIO)
 	if (kvm->coalesced_mmio) {
@@ -948,7 +948,7 @@
 			r = handle_io(kvm, run, vcpu);
 			break;
 		case KVM_EXIT_DEBUG:
-			r = handle_debug(kvm, vcpu);
+			r = handle_debug(kvm, env);
 			break;
 		case KVM_EXIT_MMIO:
 			r = handle_mmio(kvm, run);
@@ -962,7 +962,7 @@
 #endif
 			break;
 		case KVM_EXIT_SHUTDOWN:
-			r = handle_shutdown(kvm, vcpu);
+			r = handle_shutdown(kvm, env);
 			break;
 #if defined(__s390__)
 		case KVM_EXIT_S390_SIEIC:
Index: kvm-userspace.git/libkvm/libkvm.h
===================================================================
--- kvm-userspace.git.orig/libkvm/libkvm.h
+++ kvm-userspace.git/libkvm/libkvm.h
@@ -55,7 +55,7 @@
 	/// generic memory writes to unmapped memory (For MMIO devices)
     int (*mmio_write)(void *opaque, uint64_t addr, uint8_t *data,
 					int len);
-    int (*debug)(void *opaque, int vcpu);
+    int (*debug)(void *opaque, void *env);
 	/*!
 	 * \brief Called when the VCPU issues an 'hlt' instruction.
 	 *
@@ -63,12 +63,12 @@
 	 * on the host CPU.
 	 */
     int (*halt)(void *opaque, int vcpu);
-    int (*shutdown)(void *opaque, int vcpu);
+    int (*shutdown)(void *opaque, void *env);
     int (*io_window)(void *opaque);
     int (*try_push_interrupts)(void *opaque);
     int (*try_push_nmi)(void *opaque);
-    void (*post_kvm_run)(void *opaque, int vcpu);
-    int (*pre_kvm_run)(void *opaque, int vcpu);
+    void (*post_kvm_run)(void *opaque, void *env);
+    int (*pre_kvm_run)(void *opaque, void *env);
     int (*tpr_access)(void *opaque, int vcpu, uint64_t rip, int is_write);
 #if defined(__powerpc__)
     int (*powerpc_dcr_read)(int vcpu, uint32_t dcrn, uint32_t *data);
@@ -181,7 +181,7 @@
  * return except for when an error has occured, or when you have sent it
  * an EINTR signal.
  */
-int kvm_run(kvm_context_t kvm, int vcpu);
+int kvm_run(kvm_context_t kvm, int vcpu, void *env);
 
 /*!
  * \brief Get interrupt flag from on last exit to userspace
Index: kvm-userspace.git/qemu/qemu-kvm-ia64.c
===================================================================
--- kvm-userspace.git.orig/qemu/qemu-kvm-ia64.c
+++ kvm-userspace.git/qemu/qemu-kvm-ia64.c
@@ -39,11 +39,11 @@
     return 1;
 }
 
-void kvm_arch_pre_kvm_run(void *opaque, int vcpu)
+void kvm_arch_pre_kvm_run(void *opaque, CPUState *env)
 {
 }
 
-void kvm_arch_post_kvm_run(void *opaque, int vcpu)
+void kvm_arch_post_kvm_run(void *opaque, CPUState *env)
 {
 }
 
Index: kvm-userspace.git/qemu/qemu-kvm-powerpc.c
===================================================================
--- kvm-userspace.git.orig/qemu/qemu-kvm-powerpc.c
+++ kvm-userspace.git/qemu/qemu-kvm-powerpc.c
@@ -142,14 +142,13 @@
     return 1;
 }
 
-void kvm_arch_pre_kvm_run(void *opaque, int vcpu)
+void kvm_arch_pre_kvm_run(void *opaque, CPUState *env)
 {
 	return;
 }
 
-void kvm_arch_post_kvm_run(void *opaque, int vcpu)
+void kvm_arch_post_kvm_run(void *opaque, CPUState *env)
 {
-    CPUState *env = qemu_kvm_cpu_env(vcpu);
     cpu_single_env = env;
 }
 
Index: kvm-userspace.git/qemu/qemu-kvm-x86.c
===================================================================
--- kvm-userspace.git.orig/qemu/qemu-kvm-x86.c
+++ kvm-userspace.git/qemu/qemu-kvm-x86.c
@@ -619,17 +619,16 @@
     return 1;
 }
 
-void kvm_arch_pre_kvm_run(void *opaque, int vcpu)
+void kvm_arch_pre_kvm_run(void *opaque, CPUState *env)
 {
-    CPUState *env = cpu_single_env;
-
     if (!kvm_irqchip_in_kernel(kvm_context))
-	kvm_set_cr8(kvm_context, vcpu, cpu_get_apic_tpr(env));
+	kvm_set_cr8(kvm_context, env->cpu_index, cpu_get_apic_tpr(env));
 }
 
-void kvm_arch_post_kvm_run(void *opaque, int vcpu)
+void kvm_arch_post_kvm_run(void *opaque, CPUState *env)
 {
-    CPUState *env = qemu_kvm_cpu_env(vcpu);
+    int vcpu = env->cpu_index;
+
     cpu_single_env = env;
 
     env->eflags = kvm_get_interrupt_flag(kvm_context, vcpu)
Index: kvm-userspace.git/qemu/qemu-kvm.c
===================================================================
--- kvm-userspace.git.orig/qemu/qemu-kvm.c
+++ kvm-userspace.git/qemu/qemu-kvm.c
@@ -181,18 +181,19 @@
     return kvm_arch_try_push_nmi(opaque);
 }
 
-static void post_kvm_run(void *opaque, int vcpu)
+static void post_kvm_run(void *opaque, void *data)
 {
+    CPUState *env = (CPUState *)data;
 
     pthread_mutex_lock(&qemu_mutex);
-    kvm_arch_post_kvm_run(opaque, vcpu);
+    kvm_arch_post_kvm_run(opaque, env);
 }
 
-static int pre_kvm_run(void *opaque, int vcpu)
+static int pre_kvm_run(void *opaque, void *data)
 {
-    CPUState *env = qemu_kvm_cpu_env(vcpu);
+    CPUState *env = (CPUState *)data;
 
-    kvm_arch_pre_kvm_run(opaque, vcpu);
+    kvm_arch_pre_kvm_run(opaque, env);
 
     if (env->interrupt_request & CPU_INTERRUPT_EXIT)
 	return 1;
@@ -230,7 +231,7 @@
 {
     int r;
 
-    r = kvm_run(kvm_context, env->cpu_index);
+    r = kvm_run(kvm_context, env->cpu_index, env);
     if (r < 0) {
         printf("kvm_run returned %d\n", r);
         exit(1);
@@ -635,10 +636,12 @@
     return 0;
 }
 
-static int kvm_debug(void *opaque, int vcpu)
+static int kvm_debug(void *opaque, void *data)
 {
+    struct CPUState *env = (struct CPUState *)data;
+
     kvm_debug_stop_requested = 1;
-    vcpu_info[vcpu].stopped = 1;
+    vcpu_info[env->cpu_index].stopped = 1;
     return 1;
 }
 
@@ -732,7 +735,7 @@
     return kvm_arch_halt(opaque, vcpu);
 }
 
-static int kvm_shutdown(void *opaque, int vcpu)
+static int kvm_shutdown(void *opaque, void *data)
 {
     /* stop the current vcpu from going back to guest mode */
     vcpu_info[cpu_single_env->cpu_index].stopped = 1;
Index: kvm-userspace.git/qemu/qemu-kvm.h
===================================================================
--- kvm-userspace.git.orig/qemu/qemu-kvm.h
+++ kvm-userspace.git/qemu/qemu-kvm.h
@@ -62,8 +62,8 @@
 void kvm_arch_load_regs(CPUState *env);
 int kvm_arch_qemu_init_env(CPUState *cenv);
 int kvm_arch_halt(void *opaque, int vcpu);
-void kvm_arch_pre_kvm_run(void *opaque, int vcpu);
-void kvm_arch_post_kvm_run(void *opaque, int vcpu);
+void kvm_arch_pre_kvm_run(void *opaque, CPUState *env);
+void kvm_arch_post_kvm_run(void *opaque, CPUState *env);
 int kvm_arch_has_work(CPUState *env);
 int kvm_arch_try_push_interrupts(void *opaque);
 int kvm_arch_try_push_nmi(void *opaque);

Re: [patch] pass opague CPUState through libkvm instead of int vcpu

[Avi Kivity]

Jes Sorensen wrote:
> I pulled out the part of the patch that does the opague 'env' pointer.
> It should be pretty straight forward - the vcpu_info patch will go on
> top of this, but I'll work on that tomorrow.
>
> Let me know what you think - this is about as small I can make this
> patch :-)

It's good; applied, thanks.

There's a slight redundancy in that some callbacks are passed two 
opaques (per-cpu and per-vm), but it won't kill anyone.

(btw, this could have been done much more simply: declare a thread local 
variable to hold the current env)

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

Re: [patch] pass opague CPUState through libkvm instead of int vcpu

[Jes Sorensen]

Avi Kivity wrote:
> Jes Sorensen wrote:
> It's good; applied, thanks.
> 
> There's a slight redundancy in that some callbacks are passed two 
> opaques (per-cpu and per-vm), but it won't kill anyone.
> 
> (btw, this could have been done much more simply: declare a thread local 
> variable to hold the current env)

The thread local current env is coming in the vcpu_info merge patch :-)

Cheers,
Jes