[PATCH 00/10] Clean up vcpu context structure

[PATCH 00/10] Clean up vcpu context structure

[Glauber Costa]

This series aims at cleanin up vcpu_context structure. I am not removing yet
the fd field, because it is used in the ioctls, and I want to do it separadedly.

But after this series, this structure exists only as a way to hold the file descriptor,
and is, much cleaner, and much closer to upstream qemu than before.

[PATCH 01/10] use a more upstream friendly version of irqchip-in-kernel test

[Glauber Costa]

Upstream now has tests for irqchip_in_kernel. It differs from our signature, as it does
not take any parameter. For consistency, convert our usage. Also, use a field in KVMState
to store it, so we can have the exact same function as qemu upstream does.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 exec.c         |    2 +-
 hw/apic.c      |   10 +++++-----
 hw/ioapic.c    |    6 +++---
 hw/msix.c      |   12 ++++++------
 hw/pc.c        |    2 +-
 hw/pci.c       |    2 +-
 kvm-all.c      |    2 ++
 qemu-kvm-x86.c |    8 ++++----
 qemu-kvm.c     |   10 +++-------
 qemu-kvm.h     |   13 ++++---------
 10 files changed, 30 insertions(+), 37 deletions(-)

diff --git a/exec.c b/exec.c
index 24956cf..fcffb0f 100644
--- a/exec.c
+++ b/exec.c
@@ -1570,7 +1570,7 @@ void cpu_interrupt(CPUState *env, int mask)
 
     old_mask = env->interrupt_request;
     env->interrupt_request |= mask;
-    if (kvm_enabled() && !qemu_kvm_irqchip_in_kernel())
+    if (kvm_enabled() && !kvm_irqchip_in_kernel())
 	kvm_update_interrupt_request(env);
 
 #ifndef CONFIG_USER_ONLY
diff --git a/hw/apic.c b/hw/apic.c
index 2952675..b8fe529 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -301,7 +301,7 @@ void cpu_set_apic_base(CPUState *env, uint64_t val)
 #endif
     if (!s)
         return;
-    if (kvm_enabled() && qemu_kvm_irqchip_in_kernel())
+    if (kvm_enabled() && kvm_irqchip_in_kernel())
         s->apicbase = val;
     else
         s->apicbase = (val & 0xfffff000) |
@@ -509,7 +509,7 @@ void apic_init_reset(CPUState *env)
 
     env->halted = !(s->apicbase & MSR_IA32_APICBASE_BSP);
 #ifdef KVM_CAP_MP_STATE
-    if (kvm_enabled() && qemu_kvm_irqchip_in_kernel()) {
+    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
         env->mp_state
             = env->halted ? KVM_MP_STATE_UNINITIALIZED : KVM_MP_STATE_RUNNABLE;
     }
@@ -961,7 +961,7 @@ static void kvm_kernel_lapic_load_from_user(APICState *s)
 void qemu_kvm_load_lapic(CPUState *env)
 {
 #ifdef KVM_CAP_IRQCHIP
-    if (kvm_enabled() && kvm_vcpu_inited(env) && qemu_kvm_irqchip_in_kernel()) {
+    if (kvm_enabled() && kvm_vcpu_inited(env) && kvm_irqchip_in_kernel()) {
         kvm_kernel_lapic_load_from_user(env->apic_state);
     }
 #endif
@@ -972,7 +972,7 @@ static void apic_pre_save(void *opaque)
 #ifdef KVM_CAP_IRQCHIP
     APICState *s = (void *)opaque;
 
-    if (kvm_enabled() && qemu_kvm_irqchip_in_kernel()) {
+    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
         kvm_kernel_lapic_save_to_user(s);
     }
 #endif
@@ -983,7 +983,7 @@ static int apic_post_load(void *opaque, int version_id)
 #ifdef KVM_CAP_IRQCHIP
     APICState *s = opaque;
 
-    if (kvm_enabled() && qemu_kvm_irqchip_in_kernel()) {
+    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
         kvm_kernel_lapic_load_from_user(s);
     }
 #endif
diff --git a/hw/ioapic.c b/hw/ioapic.c
index cd62395..a66325d 100644
--- a/hw/ioapic.c
+++ b/hw/ioapic.c
@@ -244,7 +244,7 @@ static void ioapic_pre_save(void *opaque)
 {
     IOAPICState *s = (void *)opaque;
  
-    if (kvm_enabled() && qemu_kvm_irqchip_in_kernel()) {
+    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
         kvm_kernel_ioapic_save_to_user(s);
     }
 }
@@ -263,7 +263,7 @@ static int ioapic_post_load(void *opaque, int version_id)
 {
     IOAPICState *s = opaque;
 
-    if (kvm_enabled() && qemu_kvm_irqchip_in_kernel()) {
+    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
         kvm_kernel_ioapic_load_from_user(s);
     }
     return 0;
@@ -297,7 +297,7 @@ static void ioapic_reset(void *opaque)
     for(i = 0; i < IOAPIC_NUM_PINS; i++)
         s->ioredtbl[i] = 1 << 16; /* mask LVT */
 #ifdef KVM_CAP_IRQCHIP
-    if (kvm_enabled() && qemu_kvm_irqchip_in_kernel()) {
+    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
         kvm_kernel_ioapic_load_from_user(s);
     }
 #endif
diff --git a/hw/msix.c b/hw/msix.c
index b68fb5f..c1e5eb8 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -221,7 +221,7 @@ static int msix_add_config(struct PCIDevice *pdev, unsigned short nentries,
 static void msix_free_irq_entries(PCIDevice *dev)
 {
     int vector;
-    if (kvm_enabled() && qemu_kvm_irqchip_in_kernel()) {
+    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
         kvm_msix_free(dev);
     }
 
@@ -298,7 +298,7 @@ static void msix_mmio_writel(void *opaque, target_phys_addr_t addr,
     int vector = offset / MSIX_ENTRY_SIZE;
     int was_masked = msix_is_masked(dev, vector);
     memcpy(dev->msix_table_page + offset, &val, 4);
-    if (kvm_enabled() && qemu_kvm_irqchip_in_kernel()) {
+    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
         kvm_msix_update(dev, vector, was_masked, msix_is_masked(dev, vector));
     }
     if (!msix_is_masked(dev, vector) && msix_is_pending(dev, vector)) {
@@ -354,7 +354,7 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries,
         return -EINVAL;
 
 #ifdef KVM_CAP_IRQCHIP
-    if (kvm_enabled() && qemu_kvm_irqchip_in_kernel()) {
+    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
         dev->msix_irq_entries = qemu_malloc(nentries *
                                             sizeof *dev->msix_irq_entries);
     }
@@ -478,7 +478,7 @@ void msix_notify(PCIDevice *dev, unsigned vector)
     }
 
 #ifdef KVM_CAP_IRQCHIP
-    if (kvm_enabled() && qemu_kvm_irqchip_in_kernel()) {
+    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
         kvm_set_irq(dev->msix_irq_entries[vector].gsi, 1, NULL);
         return;
     }
@@ -516,7 +516,7 @@ int msix_vector_use(PCIDevice *dev, unsigned vector)
     if (dev->msix_entry_used[vector]) {
         return 0;
     }
-    if (kvm_enabled() && qemu_kvm_irqchip_in_kernel()) {
+    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
         ret = kvm_msix_add(dev, vector);
         if (ret) {
             return ret;
@@ -531,7 +531,7 @@ void msix_vector_unuse(PCIDevice *dev, unsigned vector)
 {
     if (vector < dev->msix_entries_nr && dev->msix_entry_used[vector]) {
         --dev->msix_entry_used[vector];
-        if (kvm_enabled() && qemu_kvm_irqchip_in_kernel()) {
+        if (kvm_enabled() && kvm_irqchip_in_kernel()) {
             kvm_msix_del(dev, vector);
         }
     }
diff --git a/hw/pc.c b/hw/pc.c
index f504f0b..9f53c97 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1288,7 +1288,7 @@ static void pc_init1(ram_addr_t ram_size,
 
     cpu_irq = qemu_allocate_irqs(pic_irq_request, NULL, 1);
 #ifdef KVM_CAP_IRQCHIP
-    if (kvm_enabled() && qemu_kvm_irqchip_in_kernel()) {
+    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
         isa_irq_state = qemu_mallocz(sizeof(*isa_irq_state));
         isa_irq = i8259 = kvm_i8259_init(cpu_irq[0]);
     } else
diff --git a/hw/pci.c b/hw/pci.c
index 4472910..5f6bae0 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -679,7 +679,7 @@ void pci_default_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int l)
     }
 
 #ifdef CONFIG_KVM_DEVICE_ASSIGNMENT
-    if (kvm_enabled() && qemu_kvm_irqchip_in_kernel() &&
+    if (kvm_enabled() && kvm_irqchip_in_kernel() &&
         addr >= PIIX_CONFIG_IRQ_ROUTE &&
 	addr < PIIX_CONFIG_IRQ_ROUTE + 4)
         assigned_dev_update_irqs();
diff --git a/kvm-all.c b/kvm-all.c
index b2651df..1356aa8 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -155,12 +155,14 @@ static void kvm_reset_vcpu(void *opaque)
         abort();
     }
 }
+#endif
 
 int kvm_irqchip_in_kernel(void)
 {
     return kvm_state->irqchip_in_kernel;
 }
 
+#ifdef KVM_UPSTREAM
 int kvm_pit_in_kernel(void)
 {
     return kvm_state->pit_in_kernel;
diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index acb1b91..a44ae67 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -284,7 +284,7 @@ int kvm_destroy_memory_alias(kvm_context_t kvm, uint64_t phys_start)
 int kvm_get_lapic(kvm_vcpu_context_t vcpu, struct kvm_lapic_state *s)
 {
 	int r;
-	if (!kvm_irqchip_in_kernel(vcpu->kvm))
+	if (!kvm_irqchip_in_kernel())
 		return 0;
 	r = ioctl(vcpu->fd, KVM_GET_LAPIC, s);
 	if (r == -1) {
@@ -297,7 +297,7 @@ int kvm_get_lapic(kvm_vcpu_context_t vcpu, struct kvm_lapic_state *s)
 int kvm_set_lapic(kvm_vcpu_context_t vcpu, struct kvm_lapic_state *s)
 {
 	int r;
-	if (!kvm_irqchip_in_kernel(vcpu->kvm))
+	if (!kvm_irqchip_in_kernel())
 		return 0;
 	r = ioctl(vcpu->fd, KVM_SET_LAPIC, s);
 	if (r == -1) {
@@ -1376,7 +1376,7 @@ int kvm_arch_halt(void *opaque, kvm_vcpu_context_t vcpu)
 
 void kvm_arch_pre_kvm_run(void *opaque, CPUState *env)
 {
-    if (!kvm_irqchip_in_kernel(kvm_context))
+    if (!kvm_irqchip_in_kernel())
 	kvm_set_cr8(env->kvm_cpu_state.vcpu_ctx, cpu_get_apic_tpr(env));
 }
 
@@ -1440,7 +1440,7 @@ void kvm_arch_cpu_reset(CPUState *env)
 {
     kvm_arch_load_regs(env);
     if (!cpu_is_bsp(env)) {
-	if (kvm_irqchip_in_kernel(kvm_context)) {
+	if (kvm_irqchip_in_kernel()) {
 #ifdef KVM_CAP_MP_STATE
 	    kvm_reset_mpstate(env->kvm_cpu_state.vcpu_ctx);
 #endif
diff --git a/qemu-kvm.c b/qemu-kvm.c
index a4a90ed..6bda694 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -558,6 +558,7 @@ void kvm_create_irqchip(kvm_context_t kvm)
         }
     }
 #endif
+    kvm_state->irqchip_in_kernel = kvm->irqchip_in_kernel;
 }
 
 int kvm_create(kvm_context_t kvm, unsigned long phys_mem_bytes, void **vm_mem)
@@ -1109,11 +1110,6 @@ int kvm_set_signal_mask(kvm_vcpu_context_t vcpu, const sigset_t *sigset)
     return r;
 }
 
-int kvm_irqchip_in_kernel(kvm_context_t kvm)
-{
-    return kvm->irqchip_in_kernel;
-}
-
 int kvm_pit_in_kernel(kvm_context_t kvm)
 {
     return kvm->pit_in_kernel;
@@ -1610,7 +1606,7 @@ void kvm_arch_get_registers(CPUState *env)
 	kvm_arch_save_regs(env);
 	kvm_arch_save_mpstate(env);
 #ifdef KVM_CAP_MP_STATE
-	if (kvm_irqchip_in_kernel(kvm_context))
+	if (kvm_irqchip_in_kernel())
 		env->halted = (env->mp_state == KVM_MP_STATE_HALTED);
 #endif
 }
@@ -1967,7 +1963,7 @@ static int kvm_main_loop_cpu(CPUState *env)
 
     while (1) {
         int run_cpu = !is_cpu_stopped(env);
-        if (run_cpu && !kvm_irqchip_in_kernel(kvm_context)) {
+        if (run_cpu && !kvm_irqchip_in_kernel()) {
             process_irqchip_events(env);
             run_cpu = !env->halted;
         }
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 05b025f..e957e96 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -553,13 +553,6 @@ int kvm_dirty_pages_log_enable_all(kvm_context_t kvm);
  */
 int kvm_dirty_pages_log_reset(kvm_context_t kvm);
 
-/*!
- * \brief Query whether in kernel irqchip is used
- *
- * \param kvm Pointer to the current kvm_context
- */
-int kvm_irqchip_in_kernel(kvm_context_t kvm);
-
 #ifdef KVM_CAP_IRQCHIP
 /*!
  * \brief Dump in kernel IRQCHIP contents
@@ -1122,7 +1115,6 @@ int handle_tpr_access(void *opaque, kvm_vcpu_context_t vcpu, uint64_t rip,
 int kvm_has_sync_mmu(void);
 
 #define kvm_enabled() (kvm_allowed)
-#define qemu_kvm_irqchip_in_kernel() kvm_irqchip_in_kernel(kvm_context)
 #define qemu_kvm_pit_in_kernel() kvm_pit_in_kernel(kvm_context)
 #define qemu_kvm_has_gsi_routing() kvm_has_gsi_routing(kvm_context)
 #ifdef TARGET_I386
@@ -1134,7 +1126,6 @@ void kvm_load_tsc(CPUState *env);
 #define kvm_has_sync_mmu() (0)
 #define kvm_enabled() (0)
 #define kvm_nested 0
-#define qemu_kvm_irqchip_in_kernel() (0)
 #define qemu_kvm_pit_in_kernel() (0)
 #define qemu_kvm_has_gsi_routing() (0)
 #ifndef QEMU_KVM_NO_CPU
@@ -1210,6 +1201,8 @@ static inline int kvm_set_migration_log(int enable)
     return kvm_physical_memory_set_dirty_tracking(enable);
 }
 
+
+int kvm_irqchip_in_kernel(void);
 #ifdef CONFIG_KVM
 
 typedef struct KVMSlot {
@@ -1232,6 +1225,8 @@ typedef struct KVMState {
 #ifdef KVM_CAP_SET_GUEST_DEBUG
     struct kvm_sw_breakpoint_head kvm_sw_breakpoints;
 #endif
+    int irqchip_in_kernel;
+
     struct kvm_context kvm_context;
 } KVMState;
 
-- 
1.6.2.5

[PATCH 02/10] drop kvm_mmio_read and write

[Glauber Costa]

they are just a tiny wrapper around qemu memory functions. Drop' em

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 qemu-kvm-x86.c |    7 +------
 qemu-kvm.c     |   24 ++++--------------------
 2 files changed, 5 insertions(+), 26 deletions(-)

diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index a44ae67..e8ec59d 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -364,7 +364,6 @@ void kvm_show_code(kvm_vcpu_context_t vcpu)
 	unsigned char code;
 	char code_str[SHOW_CODE_LEN * 3 + 1];
 	unsigned long rip;
-	kvm_context_t kvm = vcpu->kvm;
 
 	r = ioctl(fd, KVM_GET_SREGS, &sregs);
 	if (r == -1) {
@@ -384,11 +383,7 @@ void kvm_show_code(kvm_vcpu_context_t vcpu)
 	for (n = -back_offset; n < SHOW_CODE_LEN-back_offset; ++n) {
 		if (n == 0)
 			strcat(code_str, " -->");
-		r = kvm_mmio_read(kvm->opaque, rip + n, &code, 1);
-		if (r < 0) {
-			strcat(code_str, " xx");
-			continue;
-		}
+		cpu_physical_memory_rw(rip + n, &code, 1, 1);
 		sprintf(code_str + strlen(code_str), " %02x", code);
 	}
 	fprintf(stderr, "code:%s\n", code_str);
diff --git a/qemu-kvm.c b/qemu-kvm.c
index 6bda694..4ad6856 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -111,18 +111,6 @@ static int kvm_debug(void *opaque, void *data,
 }
 #endif
 
-int kvm_mmio_read(void *opaque, uint64_t addr, uint8_t *data, int len)
-{
-    cpu_physical_memory_rw(addr, data, len, 0);
-    return 0;
-}
-
-int kvm_mmio_write(void *opaque, uint64_t addr, uint8_t *data, int len)
-{
-    cpu_physical_memory_rw(addr, data, len, 1);
-    return 0;
-}
-
 static int handle_unhandled(uint64_t reason)
 {
     fprintf(stderr, "kvm: unhandled exit %" PRIx64 "\n", reason);
@@ -891,7 +879,6 @@ int kvm_set_mpstate(kvm_vcpu_context_t vcpu, struct kvm_mp_state *mp_state)
 static int handle_mmio(kvm_vcpu_context_t vcpu)
 {
     unsigned long addr = vcpu->run->mmio.phys_addr;
-    kvm_context_t kvm = vcpu->kvm;
     struct kvm_run *kvm_run = vcpu->run;
     void *data = kvm_run->mmio.data;
 
@@ -899,10 +886,8 @@ static int handle_mmio(kvm_vcpu_context_t vcpu)
     if ((addr > 0xa0000 - 4 && addr <= 0xa0000) && kvm_run->mmio.len == 3)
         return 0;
 
-    if (kvm_run->mmio.is_write)
-        return kvm_mmio_write(kvm->opaque, addr, data, kvm_run->mmio.len);
-    else
-        return kvm_mmio_read(kvm->opaque, addr, data, kvm_run->mmio.len);
+    cpu_physical_memory_rw(addr, data, kvm_run->mmio.len, kvm_run->mmio.is_write);
+    return 0;
 }
 
 int handle_io_window(kvm_context_t kvm)
@@ -994,10 +979,9 @@ int kvm_run(kvm_vcpu_context_t vcpu, void *env)
         struct kvm_coalesced_mmio_ring *ring =
             (void *) run + kvm_state->coalesced_mmio * PAGE_SIZE;
         while (ring->first != ring->last) {
-            kvm_mmio_write(kvm->opaque,
-                           ring->coalesced_mmio[ring->first].phys_addr,
+            cpu_physical_memory_rw(ring->coalesced_mmio[ring->first].phys_addr,
                            &ring->coalesced_mmio[ring->first].data[0],
-                           ring->coalesced_mmio[ring->first].len);
+                           ring->coalesced_mmio[ring->first].len, 1);
             smp_wmb();
             ring->first = (ring->first + 1) % KVM_COALESCED_MMIO_MAX;
         }
-- 
1.6.2.5

[PATCH 03/10] remove unneded opaque.

[Glauber Costa]

kvm_debug() and kvm_arch_halt() both take an opaque field for no reason,
since it is totally unused. kvm_halt() itself is just a wrapper around
kvm_arch_halt(), and is removed.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 qemu-kvm-ia64.c |    2 +-
 qemu-kvm-x86.c  |    2 +-
 qemu-kvm.c      |   13 +++----------
 qemu-kvm.h      |    2 +-
 4 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/qemu-kvm-ia64.c b/qemu-kvm-ia64.c
index d26c2a9..a11fde8 100644
--- a/qemu-kvm-ia64.c
+++ b/qemu-kvm-ia64.c
@@ -30,7 +30,7 @@ int kvm_arch_init_vcpu(CPUState *cenv)
     return 0;
 }
 
-int kvm_arch_halt(void *opaque, kvm_vcpu_context_t vcpu)
+int kvm_arch_halt(kvm_vcpu_context_t vcpu)
 {
     CPUState *env = cpu_single_env;
     env->hflags |= HF_HALTED_MASK;
diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index e8ec59d..7546fcb 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -1357,7 +1357,7 @@ int kvm_arch_init_vcpu(CPUState *cenv)
     return 0;
 }
 
-int kvm_arch_halt(void *opaque, kvm_vcpu_context_t vcpu)
+int kvm_arch_halt(kvm_vcpu_context_t vcpu)
 {
     CPUState *env = cpu_single_env;
 
diff --git a/qemu-kvm.c b/qemu-kvm.c
index 4ad6856..b92a68e 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -97,11 +97,10 @@ int kvm_abi = EXPECTED_KVM_API_VERSION;
 int kvm_page_size;
 
 #ifdef KVM_CAP_SET_GUEST_DEBUG
-static int kvm_debug(void *opaque, void *data,
+static int kvm_debug(CPUState *env,
                      struct kvm_debug_exit_arch *arch_info)
 {
     int handle = kvm_arch_debug(arch_info);
-    CPUState *env = data;
 
     if (handle) {
         kvm_debug_cpu_requested = env;
@@ -816,9 +815,8 @@ int handle_debug(kvm_vcpu_context_t vcpu, void *env)
 {
 #ifdef KVM_CAP_SET_GUEST_DEBUG
     struct kvm_run *run = vcpu->run;
-    kvm_context_t kvm = vcpu->kvm;
 
-    return kvm_debug(kvm->opaque, env, &run->debug.arch);
+    return kvm_debug(env, &run->debug.arch);
 #else
     return 0;
 #endif
@@ -895,11 +893,6 @@ int handle_io_window(kvm_context_t kvm)
     return 1;
 }
 
-int handle_halt(kvm_vcpu_context_t vcpu)
-{
-    return kvm_arch_halt(vcpu->kvm->opaque, vcpu);
-}
-
 int handle_shutdown(kvm_context_t kvm, CPUState *env)
 {
     /* stop the current vcpu from going back to guest mode */
@@ -1019,7 +1012,7 @@ int kvm_run(kvm_vcpu_context_t vcpu, void *env)
             r = handle_mmio(vcpu);
             break;
         case KVM_EXIT_HLT:
-            r = handle_halt(vcpu);
+            r = kvm_arch_halt(vcpu);
             break;
         case KVM_EXIT_IRQ_WINDOW_OPEN:
             break;
diff --git a/qemu-kvm.h b/qemu-kvm.h
index e957e96..e51dd2c 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -1109,7 +1109,7 @@ struct ioperm_data {
 };
 
 void qemu_kvm_cpu_stop(CPUState *env);
-int kvm_arch_halt(void *opaque, kvm_vcpu_context_t vcpu);
+int kvm_arch_halt(kvm_vcpu_context_t vcpu);
 int handle_tpr_access(void *opaque, kvm_vcpu_context_t vcpu, uint64_t rip,
                       int is_write);
 int kvm_has_sync_mmu(void);
-- 
1.6.2.5

[PATCH 04/10] remove kvm_context from vpcu structure

[Glauber Costa]

It is not used anywhere, so just blow it away.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 qemu-kvm.c |    4 +---
 qemu-kvm.h |    1 -
 2 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index b92a68e..5c7376d 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -441,9 +441,7 @@ kvm_vcpu_context_t kvm_create_vcpu(CPUState *env, int id)
     long mmap_size;
     int r;
     kvm_vcpu_context_t vcpu_ctx = qemu_malloc(sizeof(struct kvm_vcpu_context));
-    kvm_context_t kvm = kvm_context;
 
-    vcpu_ctx->kvm = kvm;
     vcpu_ctx->id = id;
 
     r = kvm_vm_ioctl(kvm_state, KVM_CREATE_VCPU, id);
@@ -938,8 +936,8 @@ int kvm_run(kvm_vcpu_context_t vcpu, void *env)
     int r;
     int fd = vcpu->fd;
     struct kvm_run *run = vcpu->run;
-    kvm_context_t kvm = vcpu->kvm;
     CPUState *_env = env;
+    kvm_context_t kvm = &_env->kvm_state->kvm_context;
 
   again:
     push_nmi(kvm);
diff --git a/qemu-kvm.h b/qemu-kvm.h
index e51dd2c..4dfd5ea 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -79,7 +79,6 @@ struct kvm_context {
 struct kvm_vcpu_context {
     int fd;
     struct kvm_run *run;
-    struct kvm_context *kvm;
     uint32_t id;
 };
 
-- 
1.6.2.5

[PATCH 05/10] Use kvm_run inside CPUState.

[Glauber Costa]

There is no need to replicate kvm_run inside vcpu context, since there is
already a field dedicated to that in CPUState. For now, keep both on, so
function can be converted slowly, since a lot of function headers will
be changed to take an env parameter, instead of a vcpu one.

This patch also converts functions that are static to this file as a first step.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 qemu-kvm.c |   27 +++++++++++++++------------
 qemu-kvm.h |    1 -
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index 5c7376d..a7eda2d 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -459,13 +459,16 @@ kvm_vcpu_context_t kvm_create_vcpu(CPUState *env, int id)
         fprintf(stderr, "get vcpu mmap size: %m\n");
         goto err_fd;
     }
-    vcpu_ctx->run =
+    env->kvm_run =
         mmap(NULL, mmap_size, PROT_READ | PROT_WRITE, MAP_SHARED, vcpu_ctx->fd,
              0);
-    if (vcpu_ctx->run == MAP_FAILED) {
+    if (env->kvm_run == MAP_FAILED) {
         fprintf(stderr, "mmap vcpu area: %m\n");
         goto err_fd;
     }
+
+    vcpu_ctx->run = env->kvm_run;
+
     return vcpu_ctx;
   err_fd:
     close(vcpu_ctx->fd);
@@ -757,9 +760,9 @@ int kvm_set_irqchip(kvm_context_t kvm, struct kvm_irqchip *chip)
 
 #endif
 
-static int handle_io(kvm_vcpu_context_t vcpu)
+static int handle_io(CPUState *env)
 {
-    struct kvm_run *run = vcpu->run;
+    struct kvm_run *run = env->kvm_run;
     uint16_t addr = run->io.port;
     int i;
     void *p = (void *) run + run->io.data_offset;
@@ -809,10 +812,10 @@ static int handle_io(kvm_vcpu_context_t vcpu)
     return 0;
 }
 
-int handle_debug(kvm_vcpu_context_t vcpu, void *env)
+static int handle_debug(CPUState *env)
 {
 #ifdef KVM_CAP_SET_GUEST_DEBUG
-    struct kvm_run *run = vcpu->run;
+    struct kvm_run *run = env->kvm_run;
 
     return kvm_debug(env, &run->debug.arch);
 #else
@@ -872,10 +875,10 @@ int kvm_set_mpstate(kvm_vcpu_context_t vcpu, struct kvm_mp_state *mp_state)
 }
 #endif
 
-static int handle_mmio(kvm_vcpu_context_t vcpu)
+static int handle_mmio(CPUState *env)
 {
-    unsigned long addr = vcpu->run->mmio.phys_addr;
-    struct kvm_run *kvm_run = vcpu->run;
+    unsigned long addr = env->kvm_run->mmio.phys_addr;
+    struct kvm_run *kvm_run = env->kvm_run;
     void *data = kvm_run->mmio.data;
 
     /* hack: Red Hat 7.1 generates these weird accesses. */
@@ -1001,13 +1004,13 @@ int kvm_run(kvm_vcpu_context_t vcpu, void *env)
             abort();
             break;
         case KVM_EXIT_IO:
-            r = handle_io(vcpu);
+            r = handle_io(env);
             break;
         case KVM_EXIT_DEBUG:
-            r = handle_debug(vcpu, env);
+            r = handle_debug(env);
             break;
         case KVM_EXIT_MMIO:
-            r = handle_mmio(vcpu);
+            r = handle_mmio(env);
             break;
         case KVM_EXIT_HLT:
             r = kvm_arch_halt(vcpu);
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 4dfd5ea..3bc483e 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -106,7 +106,6 @@ int handle_shutdown(kvm_context_t kvm, CPUState *env);
 void post_kvm_run(kvm_context_t kvm, CPUState *env);
 int pre_kvm_run(kvm_context_t kvm, CPUState *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__)
-- 
1.6.2.5

[PATCH 06/10] make some functions static

[Glauber Costa]

Some functions that uses kvm_run are not static, but should be.
Make them static, and make them get a CPUState parameter in the process.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 qemu-kvm-x86.c |   18 +++++++++---------
 qemu-kvm.h     |   33 ---------------------------------
 2 files changed, 9 insertions(+), 42 deletions(-)

diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index 7546fcb..10bd530 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -538,19 +538,19 @@ void kvm_show_regs(kvm_vcpu_context_t vcpu)
 		sregs.efer);
 }
 
-uint64_t kvm_get_apic_base(kvm_vcpu_context_t vcpu)
+static uint64_t kvm_get_apic_base(CPUState *env)
 {
-	return vcpu->run->apic_base;
+	return env->kvm_run->apic_base;
 }
 
-void kvm_set_cr8(kvm_vcpu_context_t vcpu, uint64_t cr8)
+static void kvm_set_cr8(CPUState *env, uint64_t cr8)
 {
-	vcpu->run->cr8 = cr8;
+	env->kvm_run->cr8 = cr8;
 }
 
-__u64 kvm_get_cr8(kvm_vcpu_context_t vcpu)
+static __u64 kvm_get_cr8(CPUState *env)
 {
-	return vcpu->run->cr8;
+	return env->kvm_run->cr8;
 }
 
 int kvm_setup_cpuid(kvm_vcpu_context_t vcpu, int nent,
@@ -1372,7 +1372,7 @@ int kvm_arch_halt(kvm_vcpu_context_t vcpu)
 void kvm_arch_pre_kvm_run(void *opaque, CPUState *env)
 {
     if (!kvm_irqchip_in_kernel())
-	kvm_set_cr8(env->kvm_cpu_state.vcpu_ctx, cpu_get_apic_tpr(env));
+	kvm_set_cr8(env, cpu_get_apic_tpr(env));
 }
 
 void kvm_arch_post_kvm_run(void *opaque, CPUState *env)
@@ -1382,8 +1382,8 @@ void kvm_arch_post_kvm_run(void *opaque, CPUState *env)
     env->eflags = kvm_get_interrupt_flag(env->kvm_cpu_state.vcpu_ctx)
 	? env->eflags | IF_MASK : env->eflags & ~IF_MASK;
 
-    cpu_set_apic_tpr(env, kvm_get_cr8(env->kvm_cpu_state.vcpu_ctx));
-    cpu_set_apic_base(env, kvm_get_apic_base(env->kvm_cpu_state.vcpu_ctx));
+    cpu_set_apic_tpr(env, kvm_get_cr8(env));
+    cpu_set_apic_base(env, kvm_get_apic_base(env));
 }
 
 int kvm_arch_has_work(CPUState *env)
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 3bc483e..1f23fe4 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -219,17 +219,6 @@ int kvm_run(kvm_vcpu_context_t vcpu, void *env);
 int kvm_get_interrupt_flag(kvm_vcpu_context_t vcpu);
 
 /*!
- * \brief Get the value of the APIC_BASE msr as of last exit to userspace
- *
- * This gets the APIC_BASE msr as it was on the last exit to userspace.
- *
- * \param kvm Pointer to the current kvm_context
- * \param vcpu Which virtual CPU should get dumped
- * \return APIC_BASE msr contents
- */
-uint64_t kvm_get_apic_base(kvm_vcpu_context_t vcpu);
-
-/*!
  * \brief Check if a vcpu is ready for interrupt injection
  *
  * This checks if vcpu interrupts are not masked by mov ss or sti.
@@ -422,28 +411,6 @@ int kvm_set_shadow_pages(kvm_context_t kvm, unsigned int nrshadow_pages);
  */
 int kvm_get_shadow_pages(kvm_context_t kvm, unsigned int *nrshadow_pages);
 
-/*!
- * \brief Set up cr8 for next time the vcpu is executed
- *
- * This is a fast setter for cr8, which will be applied when the
- * vcpu next enters guest mode.
- *
- * \param kvm Pointer to the current kvm_context
- * \param vcpu Which virtual CPU should get dumped
- * \param cr8 next cr8 value
- */
-void kvm_set_cr8(kvm_vcpu_context_t vcpu, uint64_t cr8);
-
-/*!
- * \brief Get cr8 for sync tpr in qemu apic emulation
- *
- * This is a getter for cr8, which used to sync with the tpr in qemu
- * apic emualtion.
- *
- * \param kvm Pointer to the current kvm_context
- * \param vcpu Which virtual CPU should get dumped
- */
-__u64 kvm_get_cr8(kvm_vcpu_context_t vcpu);
 #endif
 
 /*!
-- 
1.6.2.5

[PATCH 07/10] use env as parameter for functions that access kvm_run

[Glauber Costa]

Convert header of public functions that access kvm_run, so they can
use CPUState version instead.

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

diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index 10bd530..c5455d7 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -1379,7 +1379,7 @@ void kvm_arch_post_kvm_run(void *opaque, CPUState *env)
 {
     cpu_single_env = env;
 
-    env->eflags = kvm_get_interrupt_flag(env->kvm_cpu_state.vcpu_ctx)
+    env->eflags = kvm_get_interrupt_flag(env)
 	? env->eflags | IF_MASK : env->eflags & ~IF_MASK;
 
     cpu_set_apic_tpr(env, kvm_get_cr8(env));
@@ -1400,7 +1400,7 @@ int kvm_arch_try_push_interrupts(void *opaque)
     CPUState *env = cpu_single_env;
     int r, irq;
 
-    if (kvm_is_ready_for_interrupt_injection(env->kvm_cpu_state.vcpu_ctx) &&
+    if (kvm_is_ready_for_interrupt_injection(env) &&
         (env->interrupt_request & CPU_INTERRUPT_HARD) &&
         (env->eflags & IF_MASK)) {
             env->interrupt_request &= ~CPU_INTERRUPT_HARD;
diff --git a/qemu-kvm.c b/qemu-kvm.c
index a7eda2d..bd1c0b5 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -924,14 +924,14 @@ int pre_kvm_run(kvm_context_t kvm, CPUState *env)
     return 0;
 }
 
-int kvm_get_interrupt_flag(kvm_vcpu_context_t vcpu)
+int kvm_get_interrupt_flag(CPUState *env)
 {
-    return vcpu->run->if_flag;
+    return env->kvm_run->if_flag;
 }
 
-int kvm_is_ready_for_interrupt_injection(kvm_vcpu_context_t vcpu)
+int kvm_is_ready_for_interrupt_injection(CPUState *env)
 {
-    return vcpu->run->ready_for_interrupt_injection;
+    return env->kvm_run->ready_for_interrupt_injection;
 }
 
 int kvm_run(kvm_vcpu_context_t vcpu, void *env)
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 1f23fe4..28940dd 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -216,7 +216,7 @@ int kvm_run(kvm_vcpu_context_t vcpu, void *env);
  * \param vcpu Which virtual CPU should get dumped
  * \return interrupt flag value (0 or 1)
  */
-int kvm_get_interrupt_flag(kvm_vcpu_context_t vcpu);
+int kvm_get_interrupt_flag(CPUState *env);
 
 /*!
  * \brief Check if a vcpu is ready for interrupt injection
@@ -227,7 +227,7 @@ int kvm_get_interrupt_flag(kvm_vcpu_context_t vcpu);
  * \param vcpu Which virtual CPU should get dumped
  * \return boolean indicating interrupt injection readiness
  */
-int kvm_is_ready_for_interrupt_injection(kvm_vcpu_context_t vcpu);
+int kvm_is_ready_for_interrupt_injection(CPUState *env);
 
 /*!
  * \brief Read VCPU registers
-- 
1.6.2.5

[PATCH 08/10] use env in kvm_arch_run

[Glauber Costa]

kvm_arch_run() takes a vcpu context parameter, but CPUState should
do. Convert it, as well as the functions that it end up calling

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 qemu-kvm-x86.c |   12 ++++++------
 qemu-kvm.c     |    2 +-
 qemu-kvm.h     |    3 ++-
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index c5455d7..fffcfd8 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -162,10 +162,10 @@ int kvm_arch_create(kvm_context_t kvm, unsigned long phys_mem_bytes,
 
 #ifdef KVM_EXIT_TPR_ACCESS
 
-static int kvm_handle_tpr_access(kvm_vcpu_context_t vcpu)
+static int kvm_handle_tpr_access(CPUState *env)
 {
-	struct kvm_run *run = vcpu->run;
-	kvm_tpr_access_report(cpu_single_env,
+	struct kvm_run *run = env->kvm_run;
+	kvm_tpr_access_report(env,
                          run->tpr_access.rip,
                          run->tpr_access.is_write);
     return 0;
@@ -190,10 +190,10 @@ int kvm_enable_vapic(kvm_vcpu_context_t vcpu, uint64_t vapic)
 
 #endif
 
-int kvm_arch_run(kvm_vcpu_context_t vcpu)
+int kvm_arch_run(CPUState *env)
 {
 	int r = 0;
-	struct kvm_run *run = vcpu->run;
+	struct kvm_run *run = env->kvm_run;
 
 
 	switch (run->exit_reason) {
@@ -203,7 +203,7 @@ int kvm_arch_run(kvm_vcpu_context_t vcpu)
 #endif
 #ifdef KVM_EXIT_TPR_ACCESS
 		case KVM_EXIT_TPR_ACCESS:
-			r = kvm_handle_tpr_access(vcpu);
+			r = kvm_handle_tpr_access(env);
 			break;
 #endif
 		default:
diff --git a/qemu-kvm.c b/qemu-kvm.c
index bd1c0b5..fb50bb4 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -1037,7 +1037,7 @@ int kvm_run(kvm_vcpu_context_t vcpu, void *env)
 	    abort();
 	    break;
         default:
-            if (kvm_arch_run(vcpu)) {
+            if (kvm_arch_run(env)) {
                 fprintf(stderr, "unhandled vm exit: 0x%x\n", run->exit_reason);
                 kvm_show_regs(vcpu);
                 abort();
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 28940dd..adb2970 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -93,7 +93,8 @@ int kvm_alloc_userspace_memory(kvm_context_t kvm, unsigned long memory,
 
 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);
+
+int kvm_arch_run(CPUState *env);
 
 
 void kvm_show_code(kvm_vcpu_context_t vcpu);
-- 
1.6.2.5

[PATCH 09/10] remove kvm_run from vcpu_context

[Glauber Costa]

There are no more users of kvm_run inside vcpu context. Remove it, and leave the one
inside CPUState for the job.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 qemu-kvm.c |    4 +---
 qemu-kvm.h |    1 -
 2 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index fb50bb4..8257dd3 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -467,8 +467,6 @@ kvm_vcpu_context_t kvm_create_vcpu(CPUState *env, int id)
         goto err_fd;
     }
 
-    vcpu_ctx->run = env->kvm_run;
-
     return vcpu_ctx;
   err_fd:
     close(vcpu_ctx->fd);
@@ -938,9 +936,9 @@ int kvm_run(kvm_vcpu_context_t vcpu, void *env)
 {
     int r;
     int fd = vcpu->fd;
-    struct kvm_run *run = vcpu->run;
     CPUState *_env = env;
     kvm_context_t kvm = &_env->kvm_state->kvm_context;
+    struct kvm_run *run = _env->kvm_run;
 
   again:
     push_nmi(kvm);
diff --git a/qemu-kvm.h b/qemu-kvm.h
index adb2970..f051889 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -78,7 +78,6 @@ struct kvm_context {
 
 struct kvm_vcpu_context {
     int fd;
-    struct kvm_run *run;
     uint32_t id;
 };
 
-- 
1.6.2.5

[PATCH 10/10] remove id field from vcpu context structure

[Glauber Costa]

qemu already stores id information in a couple of places.
No need to do it here too.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 qemu-kvm.c |    2 --
 qemu-kvm.h |    1 -
 2 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index 8257dd3..06c1226 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -442,8 +442,6 @@ kvm_vcpu_context_t kvm_create_vcpu(CPUState *env, int id)
     int r;
     kvm_vcpu_context_t vcpu_ctx = qemu_malloc(sizeof(struct kvm_vcpu_context));
 
-    vcpu_ctx->id = id;
-
     r = kvm_vm_ioctl(kvm_state, KVM_CREATE_VCPU, id);
     if (r < 0) {
         fprintf(stderr, "kvm_create_vcpu: %m\n");
diff --git a/qemu-kvm.h b/qemu-kvm.h
index f051889..d07aa86 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -78,7 +78,6 @@ struct kvm_context {
 
 struct kvm_vcpu_context {
     int fd;
-    uint32_t id;
 };
 
 typedef struct kvm_context *kvm_context_t;
-- 
1.6.2.5

Re: [PATCH 00/10] Clean up vcpu context structure

[Avi Kivity]

On 10/09/2009 08:03 PM, Glauber Costa wrote:
> This series aims at cleanin up vcpu_context structure. I am not removing yet
> the fd field, because it is used in the ioctls, and I want to do it separadedly.
>
> But after this series, this structure exists only as a way to hold the file descriptor,
> and is, much cleaner, and much closer to upstream qemu than before.
>    

I agree.  Looks good!


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

Re: [PATCH 00/10] Clean up vcpu context structure

[Marcelo Tosatti]

On Fri, Oct 09, 2009 at 03:03:08PM -0300, Glauber Costa wrote:
> This series aims at cleanin up vcpu_context structure. I am not removing yet
> the fd field, because it is used in the ioctls, and I want to do it separadedly.
> 
> But after this series, this structure exists only as a way to hold the file descriptor,
> and is, much cleaner, and much closer to upstream qemu than before.

Applied, thanks.