[Qemu-devel] [PATCH 00/21] qemu-kvm: Hook cleanups and extended use of upstream code

[Qemu-devel] [PATCH 00/21] qemu-kvm: Hook cleanups and extended use of upstream code

[Jan Kiszka]

Let's start with the overall stats:

 31 files changed, 274 insertions(+), 822 deletions(-)

So this series drops far more than 500 lines of redundant code, moving
qemu-kvm yet a bit closer to upstream.

The other highlight is the simplification of synchronization between
in-kernel and user space VCPU states. This area used to call a lot of
problems in the past because it was tricky to get things right,
specifically during the multi-threaded startup. The new approach pushes
all the sync work around reset and vmsave/load into generic code, not
only removing the burden from developers of, say, in-kernel APIC
support, but also dropping most of our kvm-specific hooks, especially in
the qemu-kvm tree.

While I tested this on various VMs around, and things look good so far,
I wouldn't be surprised if there are some regressions remaining,
specifically in the non-x86 parts that I wasn't able to test or even
build. Please have a careful look!

Regarding the organization of the series: Patches prefixed with "KVM:"
are for upstream, unmodified or with only minor adjustments. But I have
a separate series against uq/master here that just needs final polishing
and can then be rolled out as well.

You can pull this series from

	git://git.kiszka.org/qemu-kvm.git queues/vcpu-state

There are two more items on my to-do list, yet with medium prio:
 o switch kvm_arch_save/load_regs and sub-functions to upstream code
 o drop qemu-kvm's slot management in favor of upstream's implementation

Jan Kiszka (21):
  qemu-kvm: Drop vmport changes
  KVM: Make vmport KVM-compatible
  qemu-kvm: Clean up register access API
  KVM: x86: Fix up misreported CPU features
  qemu-kvm: Use upstream kvm_enabled and cpu_synchronize_state
  qemu-kvm: Use upstream kvm_setup_guest_memory
  qemu-kvm: Use some more upstream prototypes
  qemu-kvm: Use upstream kvm_arch_get_supported_cpuid
  qemu-kvm: Use upstream kvm_pit_in_kernel
  KVM: Move and rename regs_modified
  KVM: Rework of guest debug state writing
  qemu-kvm: Use upstream kvm_vcpu_dirty
  qemu-kvm: Use upstream guest debug code
  qemu-kvm: Rework VCPU state writeback API
  qemu-kvm: Clean up mpstate synchronization
  KVM: x86: Restrict writeback of VCPU state
  qemu-kvm: Use VCPU event state for reset and vmsave/load
  qemu-kvm: Cleanup/fix TSC and PV clock writeback
  qemu-kvm: Clean up KVM's APIC hooks
  qemu-kvm: Move kvm_set_boot_cpu_id
  qemu-kvm: Bring qemu_init_vcpu back home

 cpu-defs.h            |    2 +-
 exec.c                |   17 --
 hw/apic.c             |   47 +-----
 hw/i8254.c            |    6 +-
 hw/i8259.c            |    2 +-
 hw/ioapic.c           |    2 +-
 hw/msix.c             |    3 +-
 hw/pc.c               |   13 +--
 hw/pcspk.c            |    4 +-
 hw/piix_pci.c         |    2 +-
 hw/ppc_newworld.c     |    3 -
 hw/ppc_oldworld.c     |    3 -
 hw/s390-virtio.c      |    1 -
 hw/vmport.c           |   14 +--
 kvm-all.c             |   51 +++---
 kvm.h                 |   35 +++--
 qemu-kvm-ia64.c       |    6 +-
 qemu-kvm-x86.c        |  415 +++++++++----------------------------------------
 qemu-kvm.c            |  159 +++----------------
 qemu-kvm.h            |  158 +------------------
 savevm.c              |    4 +
 sysemu.h              |    4 +
 target-i386/cpu.h     |    9 +-
 target-i386/helper.c  |    2 +
 target-i386/kvm.c     |   61 +++++--
 target-i386/machine.c |   27 ----
 target-ia64/machine.c |    5 +-
 target-ppc/kvm.c      |    2 +-
 target-ppc/machine.c  |    4 -
 target-s390x/kvm.c    |    3 +-
 vl.c                  |   32 ++++-
 31 files changed, 274 insertions(+), 822 deletions(-)

[Qemu-devel] [PATCH 01/21] qemu-kvm: Drop vmport changes

[Jan Kiszka]

This attempt to make vmport KVM compatible is half-broken and is
scheduled to be replaced by proper upstream support.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/vmport.c |   13 +------------
 1 files changed, 1 insertions(+), 12 deletions(-)

diff --git a/hw/vmport.c b/hw/vmport.c
index 648861b..884af3f 100644
--- a/hw/vmport.c
+++ b/hw/vmport.c
@@ -21,12 +21,10 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
-
 #include "hw.h"
 #include "isa.h"
 #include "pc.h"
 #include "sysemu.h"
-#include "qemu-kvm.h"
 
 //#define VMPORT_DEBUG
 
@@ -59,10 +57,6 @@ static uint32_t vmport_ioport_read(void *opaque, uint32_t addr)
     CPUState *env = cpu_single_env;
     unsigned char command;
     uint32_t eax;
-    uint32_t ret;
-
-    if (kvm_enabled())
-	kvm_save_registers(env);
 
     eax = env->regs[R_EAX];
     if (eax != VMPORT_MAGIC)
@@ -79,12 +73,7 @@ static uint32_t vmport_ioport_read(void *opaque, uint32_t addr)
         return eax;
     }
 
-    ret = s->func[command](s->opaque[command], addr);
-
-    if (kvm_enabled())
-	kvm_load_registers(env);
-
-    return ret;
+    return s->func[command](s->opaque[command], addr);
 }
 
 static void vmport_ioport_write(void *opaque, uint32_t addr, uint32_t val)
-- 
1.6.0.2

[Qemu-devel] [PATCH 02/21] KVM: Make vmport KVM-compatible

[Jan Kiszka]

The vmport "device" accesses the VCPU registers, so it requires proper
cpu_synchronize_state. Add it to vmport_ioport_read, which also
synchronizes vmport_ioport_write.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/vmport.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/vmport.c b/hw/vmport.c
index 884af3f..6c9d7c9 100644
--- a/hw/vmport.c
+++ b/hw/vmport.c
@@ -25,6 +25,7 @@
 #include "isa.h"
 #include "pc.h"
 #include "sysemu.h"
+#include "kvm.h"
 
 //#define VMPORT_DEBUG
 
@@ -58,6 +59,8 @@ static uint32_t vmport_ioport_read(void *opaque, uint32_t addr)
     unsigned char command;
     uint32_t eax;
 
+    cpu_synchronize_state(env);
+
     eax = env->regs[R_EAX];
     if (eax != VMPORT_MAGIC)
         return eax;
-- 
1.6.0.2

[Qemu-devel] [PATCH 03/21] qemu-kvm: Clean up register access API

[Jan Kiszka]

qemu-kvm's functios for accessing the VCPU registers are
kvm_arch_load/save_regs. Use them directly instead of going through
various wrappers. Specifically, we do not need on_vcpu wrapping as all
users either already run in the related thread or call while the vm is
stopped.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 qemu-kvm.c            |   37 +++----------------------------------
 qemu-kvm.h            |   11 -----------
 target-ia64/machine.c |    4 ++--
 3 files changed, 5 insertions(+), 47 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index a305907..97c098c 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -862,7 +862,7 @@ int pre_kvm_run(kvm_context_t kvm, CPUState *env)
     kvm_arch_pre_run(env, env->kvm_run);
 
     if (env->kvm_cpu_state.regs_modified) {
-        kvm_arch_put_registers(env);
+        kvm_arch_load_regs(env);
         env->kvm_cpu_state.regs_modified = 0;
     }
 
@@ -1532,16 +1532,11 @@ static void on_vcpu(CPUState *env, void (*func)(void *data), void *data)
         qemu_cond_wait(&qemu_work_cond);
 }
 
-void kvm_arch_get_registers(CPUState *env)
-{
-	kvm_arch_save_regs(env);
-}
-
 static void do_kvm_cpu_synchronize_state(void *_env)
 {
     CPUState *env = _env;
     if (!env->kvm_cpu_state.regs_modified) {
-        kvm_arch_get_registers(env);
+        kvm_arch_save_regs(env);
         env->kvm_cpu_state.regs_modified = 1;
     }
 }
@@ -1584,32 +1579,6 @@ void kvm_update_interrupt_request(CPUState *env)
     }
 }
 
-static void kvm_do_load_registers(void *_env)
-{
-    CPUState *env = _env;
-
-    kvm_arch_load_regs(env);
-}
-
-void kvm_load_registers(CPUState *env)
-{
-    if (kvm_enabled() && qemu_system_ready)
-        on_vcpu(env, kvm_do_load_registers, env);
-}
-
-static void kvm_do_save_registers(void *_env)
-{
-    CPUState *env = _env;
-
-    kvm_arch_save_regs(env);
-}
-
-void kvm_save_registers(CPUState *env)
-{
-    if (kvm_enabled())
-        on_vcpu(env, kvm_do_save_registers, env);
-}
-
 static void kvm_do_load_mpstate(void *_env)
 {
     CPUState *env = _env;
@@ -2379,7 +2348,7 @@ static void kvm_invoke_set_guest_debug(void *data)
     struct kvm_set_guest_debug_data *dbg_data = data;
 
     if (cpu_single_env->kvm_cpu_state.regs_modified) {
-        kvm_arch_put_registers(cpu_single_env);
+        kvm_arch_save_regs(cpu_single_env);
         cpu_single_env->kvm_cpu_state.regs_modified = 0;
     }
     dbg_data->err =
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 6b3e5a1..1354227 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -902,8 +902,6 @@ int kvm_main_loop(void);
 int kvm_init_ap(void);
 #ifndef QEMU_KVM_NO_CPU
 int kvm_vcpu_inited(CPUState *env);
-void kvm_load_registers(CPUState *env);
-void kvm_save_registers(CPUState *env);
 void kvm_load_mpstate(CPUState *env);
 void kvm_save_mpstate(CPUState *env);
 int kvm_cpu_exec(CPUState *env);
@@ -1068,8 +1066,6 @@ void kvm_load_tsc(CPUState *env);
 #ifdef TARGET_I386
 #define qemu_kvm_has_pit_state2() (0)
 #endif
-#define kvm_load_registers(env) do {} while(0)
-#define kvm_save_registers(env) do {} while(0)
 #define kvm_save_mpstate(env)   do {} while(0)
 #define qemu_kvm_cpu_stop(env) do {} while(0)
 static inline void kvm_init_vcpu(CPUState *env)
@@ -1098,13 +1094,6 @@ static inline int kvm_sync_vcpus(void)
 }
 
 #ifndef QEMU_KVM_NO_CPU
-void kvm_arch_get_registers(CPUState *env);
-
-static inline void kvm_arch_put_registers(CPUState *env)
-{
-    kvm_load_registers(env);
-}
-
 void kvm_cpu_synchronize_state(CPUState *env);
 
 static inline void cpu_synchronize_state(CPUState *env)
diff --git a/target-ia64/machine.c b/target-ia64/machine.c
index 70ef379..7d29575 100644
--- a/target-ia64/machine.c
+++ b/target-ia64/machine.c
@@ -9,7 +9,7 @@ void cpu_save(QEMUFile *f, void *opaque)
     CPUState *env = opaque;
 
     if (kvm_enabled()) {
-        kvm_save_registers(env);
+        kvm_arch_save_regs(env);
         kvm_arch_save_mpstate(env);
     }
 }
@@ -19,7 +19,7 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
     CPUState *env = opaque;
 
     if (kvm_enabled()) {
-        kvm_load_registers(env);
+        kvm_arch_load_regs(env);
         kvm_arch_load_mpstate(env);
     }
     return 0;
-- 
1.6.0.2

[Qemu-devel] [PATCH 04/21] KVM: x86: Fix up misreported CPU features

[Jan Kiszka]

>From qemu-kvm: Kernels before 2.6.30 misreported some essential CPU
features via KVM_GET_SUPPORTED_CPUID. Fix them up.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 target-i386/kvm.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 504f501..9fb96b5 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -101,12 +101,18 @@ uint32_t kvm_arch_get_supported_cpuid(CPUState *env, uint32_t function, int reg)
                 break;
             case R_EDX:
                 ret = cpuid->entries[i].edx;
-                if (function == 0x80000001) {
+                switch (function) {
+                case 1:
+                    /* KVM before 2.6.30 misreports the following features */
+                    ret |= CPUID_MTRR | CPUID_PAT | CPUID_MCE | CPUID_MCA;
+                    break;
+                case 0x80000001:
                     /* On Intel, kvm returns cpuid according to the Intel spec,
                      * so add missing bits according to the AMD spec:
                      */
                     cpuid_1_edx = kvm_arch_get_supported_cpuid(env, 1, R_EDX);
                     ret |= cpuid_1_edx & 0xdfeff7ff;
+                    break;
                 }
                 break;
             }
-- 
1.6.0.2

[Qemu-devel] [PATCH 05/21] qemu-kvm: Use upstream kvm_enabled and cpu_synchronize_state

[Jan Kiszka]

They are identical, no need for private copies. This requires replacing
qemu-kvm.h includes with kvm.h, a good thing anyway, and reveals that
there is no need for QEMU_KVM_NO_CPU protection.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/i8254.c    |    2 +-
 hw/i8259.c    |    2 +-
 hw/ioapic.c   |    2 +-
 hw/msix.c     |    3 +--
 hw/pc.c       |    2 +-
 hw/piix_pci.c |    2 +-
 kvm.h         |    7 +++----
 qemu-kvm.h    |   41 -----------------------------------------
 vl.c          |    3 ++-
 9 files changed, 11 insertions(+), 53 deletions(-)

diff --git a/hw/i8254.c b/hw/i8254.c
index c4f8d2e..db9e94a 100644
--- a/hw/i8254.c
+++ b/hw/i8254.c
@@ -25,7 +25,7 @@
 #include "pc.h"
 #include "isa.h"
 #include "qemu-timer.h"
-#include "qemu-kvm.h"
+#include "kvm.h"
 #include "i8254.h"
 
 //#define DEBUG_PIT
diff --git a/hw/i8259.c b/hw/i8259.c
index 7a484c0..b64c6fb 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -27,7 +27,7 @@
 #include "monitor.h"
 #include "qemu-timer.h"
 
-#include "qemu-kvm.h"
+#include "kvm.h"
 
 /* debug PIC */
 //#define DEBUG_PIC
diff --git a/hw/ioapic.c b/hw/ioapic.c
index a66325d..0adb0ac 100644
--- a/hw/ioapic.c
+++ b/hw/ioapic.c
@@ -26,7 +26,7 @@
 #include "qemu-timer.h"
 #include "host-utils.h"
 
-#include "qemu-kvm.h"
+#include "kvm.h"
 
 //#define DEBUG_IOAPIC
 
diff --git a/hw/msix.c b/hw/msix.c
index 87f125b..faee0b2 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -14,8 +14,7 @@
 #include "hw.h"
 #include "msix.h"
 #include "pci.h"
-#define QEMU_KVM_NO_CPU
-#include "qemu-kvm.h"
+#include "kvm.h"
 
 /* Declaration from linux/pci_regs.h */
 #define  PCI_CAP_ID_MSIX 0x11 /* MSI-X */
diff --git a/hw/pc.c b/hw/pc.c
index 97e16ce..dac373e 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -47,7 +47,7 @@
 #include "multiboot.h"
 #include "device-assignment.h"
 
-#include "qemu-kvm.h"
+#include "kvm.h"
 
 /* output Bochs bios info messages */
 //#define DEBUG_BIOS
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 155587b..170f858 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -28,7 +28,7 @@
 #include "pci_host.h"
 #include "isa.h"
 #include "sysbus.h"
-#include "qemu-kvm.h"
+#include "kvm.h"
 
 /*
  * I440FX chipset data sheet.
diff --git a/kvm.h b/kvm.h
index 9fa4e25..d0f4bbe 100644
--- a/kvm.h
+++ b/kvm.h
@@ -18,8 +18,6 @@
 #include "qemu-queue.h"
 #include "qemu-kvm.h"
 
-#ifdef KVM_UPSTREAM
-
 #ifdef CONFIG_KVM
 extern int kvm_allowed;
 
@@ -28,6 +26,7 @@ extern int kvm_allowed;
 #define kvm_enabled() (0)
 #endif
 
+#ifdef KVM_UPSTREAM
 struct kvm_run;
 
 /* external API */
@@ -138,6 +137,8 @@ int kvm_check_extension(KVMState *s, unsigned int extension);
 
 uint32_t kvm_arch_get_supported_cpuid(CPUState *env, uint32_t function,
                                       int reg);
+#endif
+
 void kvm_cpu_synchronize_state(CPUState *env);
 
 /* generic hooks - to be moved/refactored once there are more users */
@@ -150,5 +151,3 @@ static inline void cpu_synchronize_state(CPUState *env)
 }
 
 #endif
-
-#endif
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 1354227..d838bca 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -8,9 +8,7 @@
 #ifndef THE_ORIGINAL_AND_TRUE_QEMU_KVM_H
 #define THE_ORIGINAL_AND_TRUE_QEMU_KVM_H
 
-#ifndef QEMU_KVM_NO_CPU
 #include "cpu.h"
-#endif
 
 #include <signal.h>
 #include <stdlib.h>
@@ -94,8 +92,6 @@ void kvm_show_code(CPUState *env);
 
 int handle_halt(CPUState *env);
 
-#ifndef QEMU_KVM_NO_CPU
-
 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);
@@ -113,8 +109,6 @@ struct kvm_x86_mce;
 int kvm_set_mce(CPUState *env, struct kvm_x86_mce *mce);
 #endif
 
-#endif
-
 /*!
  * \brief Create new KVM context
  *
@@ -880,8 +874,6 @@ static inline int kvm_init(int smp_cpus)
     return 0;
 }
 
-#ifndef QEMU_KVM_NO_CPU
-
 static inline void kvm_inject_x86_mce(CPUState *cenv, int bank,
                                       uint64_t status, uint64_t mcg_status,
                                       uint64_t addr, uint64_t misc,
@@ -891,16 +883,11 @@ static inline void kvm_inject_x86_mce(CPUState *cenv, int bank,
         abort();
 }
 
-#endif
-
-extern int kvm_allowed;
-
 #endif                          /* !CONFIG_KVM */
 
 
 int kvm_main_loop(void);
 int kvm_init_ap(void);
-#ifndef QEMU_KVM_NO_CPU
 int kvm_vcpu_inited(CPUState *env);
 void kvm_load_mpstate(CPUState *env);
 void kvm_save_mpstate(CPUState *env);
@@ -914,7 +901,6 @@ int kvm_update_guest_debug(CPUState *env, unsigned long reinject_trap);
 void kvm_apic_init(CPUState *env);
 /* called from vcpu initialization */
 void qemu_kvm_load_lapic(CPUState *env);
-#endif
 
 void kvm_hpet_enable_kpit(void);
 void kvm_hpet_disable_kpit(void);
@@ -923,13 +909,11 @@ int kvm_set_irq(int irq, int level, int *status);
 int kvm_physical_memory_set_dirty_tracking(int enable);
 int kvm_update_dirty_pages_log(void);
 
-#ifndef QEMU_KVM_NO_CPU
 void qemu_kvm_call_with_env(void (*func)(void *), void *data, CPUState *env);
 void qemu_kvm_cpuid_on_env(CPUState *env);
 void kvm_inject_interrupt(CPUState *env, int mask);
 void kvm_update_after_sipi(CPUState *env);
 void kvm_update_interrupt_request(CPUState *env);
-#endif
 void kvm_set_phys_mem(target_phys_addr_t start_addr, ram_addr_t size,
                       ram_addr_t phys_offset);
 void *kvm_cpu_create_phys_mem(target_phys_addr_t start_addr, unsigned long size,
@@ -946,7 +930,6 @@ int kvm_qemu_destroy_memory_alias(uint64_t phys_start);
 
 int kvm_arch_qemu_create_context(void);
 
-#ifndef QEMU_KVM_NO_CPU
 void kvm_arch_save_regs(CPUState *env);
 void kvm_arch_load_regs(CPUState *env);
 void kvm_arch_load_mpstate(CPUState *env);
@@ -988,19 +971,15 @@ int kvm_arch_remove_hw_breakpoint(target_ulong addr, target_ulong len,
 void kvm_arch_remove_all_hw_breakpoints(void);
 void kvm_arch_update_guest_debug(CPUState *env, struct kvm_guest_debug *dbg);
 
-#endif
-
 void qemu_kvm_aio_wait_start(void);
 void qemu_kvm_aio_wait(void);
 void qemu_kvm_aio_wait_end(void);
 
 void qemu_kvm_notify_work(void);
 
-#ifndef QEMU_KVM_NO_CPU
 void kvm_tpr_opt_setup(void);
 void kvm_tpr_access_report(CPUState *env, uint64_t rip, int is_write);
 void kvm_tpr_vcpu_start(CPUState *env);
-#endif
 
 int qemu_kvm_get_dirty_pages(unsigned long phys_addr, void *buf);
 int kvm_coalesce_mmio_region(target_phys_addr_t start, ram_addr_t size);
@@ -1021,14 +1000,11 @@ void kvm_arch_do_ioperm(void *_data);
 #endif
 
 #define ALIGN(x, y)  (((x)+(y)-1) & ~((y)-1))
-#ifndef QEMU_KVM_NO_CPU
 #define BITMAP_SIZE(m) (ALIGN(((m)>>TARGET_PAGE_BITS), HOST_LONG_BITS) / 8)
-#endif
 
 #ifdef CONFIG_KVM
 #include "qemu-queue.h"
 
-extern int kvm_allowed;
 extern int kvm_irqchip;
 extern int kvm_pit;
 extern int kvm_pit_reinject;
@@ -1048,7 +1024,6 @@ int handle_tpr_access(void *opaque, CPUState *env, uint64_t rip,
                       int is_write);
 int kvm_has_sync_mmu(void);
 
-#define kvm_enabled() (kvm_allowed)
 #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
@@ -1058,11 +1033,9 @@ void kvm_init_vcpu(CPUState *env);
 void kvm_load_tsc(CPUState *env);
 #else
 #define kvm_has_sync_mmu() (0)
-#define kvm_enabled() (0)
 #define kvm_nested 0
 #define qemu_kvm_pit_in_kernel() (0)
 #define qemu_kvm_has_gsi_routing() (0)
-#ifndef QEMU_KVM_NO_CPU
 #ifdef TARGET_I386
 #define qemu_kvm_has_pit_state2() (0)
 #endif
@@ -1076,7 +1049,6 @@ static inline void kvm_load_tsc(CPUState *env)
 {
 }
 #endif
-#endif
 
 void kvm_mutex_unlock(void);
 void kvm_mutex_lock(void);
@@ -1093,22 +1065,9 @@ static inline int kvm_sync_vcpus(void)
     return 0;
 }
 
-#ifndef QEMU_KVM_NO_CPU
-void kvm_cpu_synchronize_state(CPUState *env);
-
-static inline void cpu_synchronize_state(CPUState *env)
-{
-    if (kvm_enabled()) {
-        kvm_cpu_synchronize_state(env);
-    }
-}
-
 uint32_t kvm_arch_get_supported_cpuid(CPUState *env, uint32_t function,
                                       int reg);
 
-
-#endif
-
 static inline int kvm_set_migration_log(int enable)
 {
     return kvm_physical_memory_set_dirty_tracking(enable);
diff --git a/vl.c b/vl.c
index 4ef6a78..25af448 100644
--- a/vl.c
+++ b/vl.c
@@ -157,7 +157,6 @@ int main(int argc, char **argv)
 #include "qemu-option.h"
 #include "qemu-config.h"
 #include "qemu-objects.h"
-#include "qemu-kvm.h"
 #include "hw/device-assignment.h"
 
 #include "disas.h"
@@ -5799,8 +5798,10 @@ int main(int argc, char **argv, char **envp)
             fprintf(stderr, "failed to initialize KVM\n");
             exit(1);
 #endif
+#ifdef CONFIG_KVM
             fprintf(stderr, "Could not initialize KVM, will disable KVM support\n");
             kvm_allowed = 0;
+#endif
         }
     }
 
-- 
1.6.0.2

[Qemu-devel] [PATCH 06/21] qemu-kvm: Use upstream kvm_setup_guest_memory

[Jan Kiszka]

Nothing missing in upstream kvm_setup_guest_memory, it is even more
careful about error handling.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kvm-all.c  |    3 ---
 kvm.h      |    3 +--
 qemu-kvm.c |   15 ---------------
 qemu-kvm.h |    1 -
 4 files changed, 1 insertions(+), 21 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 0423fff..e7fa605 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -886,7 +886,6 @@ int kvm_has_vcpu_events(void)
     return kvm_state->vcpu_events;
 }
 
-#ifdef KVM_UPSTREAM
 void kvm_setup_guest_memory(void *start, size_t size)
 {
     if (!kvm_has_sync_mmu()) {
@@ -905,8 +904,6 @@ void kvm_setup_guest_memory(void *start, size_t size)
     }
 }
 
-#endif /* KVM_UPSTREAM */
-
 #ifdef KVM_CAP_SET_GUEST_DEBUG
 
 #ifdef KVM_UPSTREAM
diff --git a/kvm.h b/kvm.h
index d0f4bbe..05ee540 100644
--- a/kvm.h
+++ b/kvm.h
@@ -54,10 +54,9 @@ int kvm_has_vcpu_events(void);
 int kvm_put_vcpu_events(CPUState *env);
 int kvm_get_vcpu_events(CPUState *env);
 
-#ifdef KVM_UPSTREAM
-
 void kvm_setup_guest_memory(void *start, size_t size);
 
+#ifdef KVM_UPSTREAM
 int kvm_coalesce_mmio_region(target_phys_addr_t start, ram_addr_t size);
 int kvm_uncoalesce_mmio_region(target_phys_addr_t start, ram_addr_t size);
 
diff --git a/qemu-kvm.c b/qemu-kvm.c
index 97c098c..76f056c 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -2321,21 +2321,6 @@ void kvm_set_phys_mem(target_phys_addr_t start_addr, ram_addr_t size,
     return;
 }
 
-int kvm_setup_guest_memory(void *area, unsigned long size)
-{
-    int ret = 0;
-
-#ifdef MADV_DONTFORK
-    if (kvm_enabled() && !kvm_has_sync_mmu())
-        ret = madvise(area, size, MADV_DONTFORK);
-#endif
-
-    if (ret)
-        perror("madvise");
-
-    return ret;
-}
-
 #ifdef KVM_CAP_SET_GUEST_DEBUG
 
 struct kvm_set_guest_debug_data {
diff --git a/qemu-kvm.h b/qemu-kvm.h
index d838bca..0664c1d 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -923,7 +923,6 @@ void kvm_cpu_destroy_phys_mem(target_phys_addr_t start_addr,
                               unsigned long size);
 void kvm_qemu_log_memory(target_phys_addr_t start, target_phys_addr_t size,
                          int log);
-int kvm_setup_guest_memory(void *area, unsigned long size);
 int kvm_qemu_create_memory_alias(uint64_t phys_start, uint64_t len,
                                  uint64_t target_phys);
 int kvm_qemu_destroy_memory_alias(uint64_t phys_start);
-- 
1.6.0.2

[Qemu-devel] [PATCH 07/21] qemu-kvm: Use some more upstream prototypes

[Jan Kiszka]

Drop our private typedef of KVMState and use more identical upstream
prototypes.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kvm.h      |   10 +++++++---
 qemu-kvm.c |    4 +++-
 qemu-kvm.h |   24 +++---------------------
 3 files changed, 13 insertions(+), 25 deletions(-)

diff --git a/kvm.h b/kvm.h
index 05ee540..b5ed744 100644
--- a/kvm.h
+++ b/kvm.h
@@ -32,11 +32,13 @@ struct kvm_run;
 /* external API */
 
 int kvm_init(int smp_cpus);
+#endif /* KVM_UPSTREAM */
 
 int kvm_init_vcpu(CPUState *env);
 
 int kvm_cpu_exec(CPUState *env);
 
+#ifdef KVM_UPSTREAM
 void kvm_set_phys_mem(target_phys_addr_t start_addr,
                       ram_addr_t size,
                       ram_addr_t phys_offset);
@@ -47,19 +49,19 @@ int kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr,
 int kvm_log_start(target_phys_addr_t phys_addr, ram_addr_t size);
 int kvm_log_stop(target_phys_addr_t phys_addr, ram_addr_t size);
 int kvm_set_migration_log(int enable);
+#endif /* KVM_UPSTREAM */
 
 int kvm_has_sync_mmu(void);
-#endif /* KVM_UPSTREAM */
 int kvm_has_vcpu_events(void);
 int kvm_put_vcpu_events(CPUState *env);
 int kvm_get_vcpu_events(CPUState *env);
 
 void kvm_setup_guest_memory(void *start, size_t size);
 
-#ifdef KVM_UPSTREAM
 int kvm_coalesce_mmio_region(target_phys_addr_t start, ram_addr_t size);
 int kvm_uncoalesce_mmio_region(target_phys_addr_t start, ram_addr_t size);
 
+#ifdef KVM_UPSTREAM
 int kvm_insert_breakpoint(CPUState *current_env, target_ulong addr,
                           target_ulong len, int type);
 int kvm_remove_breakpoint(CPUState *current_env, target_ulong addr,
@@ -69,6 +71,7 @@ int kvm_update_guest_debug(CPUState *env, unsigned long reinject_trap);
 
 int kvm_pit_in_kernel(void);
 int kvm_irqchip_in_kernel(void);
+#endif /* KVM_UPSTREAM */
 
 /* internal API */
 
@@ -97,7 +100,6 @@ int kvm_arch_init(KVMState *s, int smp_cpus);
 
 int kvm_arch_init_vcpu(CPUState *env);
 
-#endif
 void kvm_arch_reset_vcpu(CPUState *env);
 #ifdef KVM_UPSTREAM
 
@@ -131,9 +133,11 @@ int kvm_arch_remove_hw_breakpoint(target_ulong addr,
 void kvm_arch_remove_all_hw_breakpoints(void);
 
 void kvm_arch_update_guest_debug(CPUState *env, struct kvm_guest_debug *dbg);
+#endif
 
 int kvm_check_extension(KVMState *s, unsigned int extension);
 
+#ifdef KVM_UPSTREAM
 uint32_t kvm_arch_get_supported_cpuid(CPUState *env, uint32_t function,
                                       int reg);
 #endif
diff --git a/qemu-kvm.c b/qemu-kvm.c
index 76f056c..12442a7 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -1909,12 +1909,14 @@ static void *ap_main_loop(void *_env)
     return NULL;
 }
 
-void kvm_init_vcpu(CPUState *env)
+int kvm_init_vcpu(CPUState *env)
 {
     pthread_create(&env->kvm_cpu_state.thread, NULL, ap_main_loop, env);
 
     while (env->created == 0)
         qemu_cond_wait(&qemu_vcpu_cond);
+
+    return 0;
 }
 
 int kvm_vcpu_inited(CPUState *env)
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 0664c1d..150017d 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -891,7 +891,6 @@ int kvm_init_ap(void);
 int kvm_vcpu_inited(CPUState *env);
 void kvm_load_mpstate(CPUState *env);
 void kvm_save_mpstate(CPUState *env);
-int kvm_cpu_exec(CPUState *env);
 int kvm_insert_breakpoint(CPUState * current_env, target_ulong addr,
                           target_ulong len, int type);
 int kvm_remove_breakpoint(CPUState * current_env, target_ulong addr,
@@ -933,9 +932,6 @@ void kvm_arch_save_regs(CPUState *env);
 void kvm_arch_load_regs(CPUState *env);
 void kvm_arch_load_mpstate(CPUState *env);
 void kvm_arch_save_mpstate(CPUState *env);
-int kvm_arch_init_vcpu(CPUState *cenv);
-int kvm_arch_pre_run(CPUState *env, struct kvm_run *run);
-int kvm_arch_post_run(CPUState *env, struct kvm_run *run);
 int kvm_arch_has_work(CPUState *env);
 void kvm_arch_process_irqchip_events(CPUState *env);
 int kvm_arch_try_push_interrupts(void *opaque);
@@ -981,8 +977,6 @@ void kvm_tpr_access_report(CPUState *env, uint64_t rip, int is_write);
 void kvm_tpr_vcpu_start(CPUState *env);
 
 int qemu_kvm_get_dirty_pages(unsigned long phys_addr, void *buf);
-int kvm_coalesce_mmio_region(target_phys_addr_t start, ram_addr_t size);
-int kvm_uncoalesce_mmio_region(target_phys_addr_t start, ram_addr_t size);
 
 int kvm_arch_init_irq_routing(void);
 
@@ -1021,17 +1015,14 @@ void qemu_kvm_cpu_stop(CPUState *env);
 int kvm_arch_halt(CPUState *env);
 int handle_tpr_access(void *opaque, CPUState *env, uint64_t rip,
                       int is_write);
-int kvm_has_sync_mmu(void);
 
 #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
 #define qemu_kvm_has_pit_state2() kvm_has_pit_state2(kvm_context)
 #endif
-void kvm_init_vcpu(CPUState *env);
 void kvm_load_tsc(CPUState *env);
 #else
-#define kvm_has_sync_mmu() (0)
 #define kvm_nested 0
 #define qemu_kvm_pit_in_kernel() (0)
 #define qemu_kvm_has_gsi_routing() (0)
@@ -1040,10 +1031,6 @@ void kvm_load_tsc(CPUState *env);
 #endif
 #define kvm_save_mpstate(env)   do {} while(0)
 #define qemu_kvm_cpu_stop(env) do {} while(0)
-static inline void kvm_init_vcpu(CPUState *env)
-{
-}
-
 static inline void kvm_load_tsc(CPUState *env)
 {
 }
@@ -1086,7 +1073,7 @@ typedef struct KVMSlot {
 
 typedef struct kvm_dirty_log KVMDirtyLog;
 
-typedef struct KVMState {
+struct KVMState {
     KVMSlot slots[32];
     int fd;
     int vmfd;
@@ -1100,14 +1087,9 @@ typedef struct KVMState {
     int irqchip_in_kernel;
 
     struct kvm_context kvm_context;
-} KVMState;
-
-extern KVMState *kvm_state;
+};
 
-int kvm_ioctl(KVMState *s, int type, ...);
-int kvm_vm_ioctl(KVMState *s, int type, ...);
-int kvm_vcpu_ioctl(CPUState *env, int type, ...);
-int kvm_check_extension(KVMState *s, unsigned int ext);
+extern struct KVMState *kvm_state;
 
 int kvm_tpr_enable_vapic(CPUState *env);
 
-- 
1.6.0.2

[Qemu-devel] [PATCH 08/21] qemu-kvm: Use upstream kvm_arch_get_supported_cpuid

[Jan Kiszka]

It is idential to our version now, so drop the copy.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kvm.h             |    3 -
 qemu-kvm-x86.c    |  106 -----------------------------------------------------
 qemu-kvm.h        |    5 --
 target-i386/kvm.c |    4 +-
 4 files changed, 2 insertions(+), 116 deletions(-)

diff --git a/kvm.h b/kvm.h
index b5ed744..189a5d4 100644
--- a/kvm.h
+++ b/kvm.h
@@ -137,11 +137,8 @@ void kvm_arch_update_guest_debug(CPUState *env, struct kvm_guest_debug *dbg);
 
 int kvm_check_extension(KVMState *s, unsigned int extension);
 
-#ifdef KVM_UPSTREAM
 uint32_t kvm_arch_get_supported_cpuid(CPUState *env, uint32_t function,
                                       int reg);
-#endif
-
 void kvm_cpu_synchronize_state(CPUState *env);
 
 /* generic hooks - to be moved/refactored once there are more users */
diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index 7f820a4..0457a6e 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -627,106 +627,6 @@ int kvm_disable_tpr_access_reporting(CPUState *env)
 
 #endif
 
-#ifdef KVM_CAP_EXT_CPUID
-
-static struct kvm_cpuid2 *try_get_cpuid(kvm_context_t kvm, int max)
-{
-	struct kvm_cpuid2 *cpuid;
-	int r, size;
-
-	size = sizeof(*cpuid) + max * sizeof(*cpuid->entries);
-	cpuid = qemu_malloc(size);
-	cpuid->nent = max;
-	r = kvm_ioctl(kvm_state, KVM_GET_SUPPORTED_CPUID, cpuid);
-	if (r == 0 && cpuid->nent >= max)
-		r = -E2BIG;
-	if (r < 0) {
-		if (r == -E2BIG) {
-			free(cpuid);
-			return NULL;
-		} else {
-			fprintf(stderr, "KVM_GET_SUPPORTED_CPUID failed: %s\n",
-				strerror(-r));
-			exit(1);
-		}
-	}
-	return cpuid;
-}
-
-#define R_EAX 0
-#define R_ECX 1
-#define R_EDX 2
-#define R_EBX 3
-#define R_ESP 4
-#define R_EBP 5
-#define R_ESI 6
-#define R_EDI 7
-
-uint32_t kvm_get_supported_cpuid(kvm_context_t kvm, uint32_t function, int reg)
-{
-	struct kvm_cpuid2 *cpuid;
-	int i, max;
-	uint32_t ret = 0;
-	uint32_t cpuid_1_edx;
-
-	if (!kvm_check_extension(kvm_state, KVM_CAP_EXT_CPUID)) {
-		return -1U;
-	}
-
-	max = 1;
-	while ((cpuid = try_get_cpuid(kvm, max)) == NULL) {
-		max *= 2;
-	}
-
-	for (i = 0; i < cpuid->nent; ++i) {
-		if (cpuid->entries[i].function == function) {
-			switch (reg) {
-			case R_EAX:
-				ret = cpuid->entries[i].eax;
-				break;
-			case R_EBX:
-				ret = cpuid->entries[i].ebx;
-				break;
-			case R_ECX:
-				ret = cpuid->entries[i].ecx;
-				break;
-			case R_EDX:
-				ret = cpuid->entries[i].edx;
-                                if (function == 1) {
-                                    /* kvm misreports the following features
-                                     */
-                                    ret |= 1 << 12; /* MTRR */
-                                    ret |= 1 << 16; /* PAT */
-                                    ret |= 1 << 7;  /* MCE */
-                                    ret |= 1 << 14; /* MCA */
-                                }
-
-				/* On Intel, kvm returns cpuid according to
-				 * the Intel spec, so add missing bits
-				 * according to the AMD spec:
-				 */
-				if (function == 0x80000001) {
-					cpuid_1_edx = kvm_get_supported_cpuid(kvm, 1, R_EDX);
-					ret |= cpuid_1_edx & 0xdfeff7ff;
-				}
-				break;
-			}
-		}
-	}
-
-	free(cpuid);
-
-	return ret;
-}
-
-#else
-
-uint32_t kvm_get_supported_cpuid(kvm_context_t kvm, uint32_t function, int reg)
-{
-	return -1U;
-}
-
-#endif
 int kvm_qemu_create_memory_alias(uint64_t phys_start,
                                  uint64_t len,
                                  uint64_t target_phys)
@@ -1686,12 +1586,6 @@ int kvm_arch_init_irq_routing(void)
     return 0;
 }
 
-uint32_t kvm_arch_get_supported_cpuid(CPUState *env, uint32_t function,
-                                      int reg)
-{
-    return kvm_get_supported_cpuid(kvm_context, function, reg);
-}
-
 void kvm_arch_process_irqchip_events(CPUState *env)
 {
     if (env->interrupt_request & CPU_INTERRUPT_INIT) {
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 150017d..7b75fdd 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -859,8 +859,6 @@ int kvm_assign_set_msix_entry(kvm_context_t kvm,
                               struct kvm_assigned_msix_entry *entry);
 #endif
 
-uint32_t kvm_get_supported_cpuid(kvm_context_t kvm, uint32_t function, int reg);
-
 #else                           /* !CONFIG_KVM */
 
 typedef struct kvm_context *kvm_context_t;
@@ -1051,9 +1049,6 @@ static inline int kvm_sync_vcpus(void)
     return 0;
 }
 
-uint32_t kvm_arch_get_supported_cpuid(CPUState *env, uint32_t function,
-                                      int reg);
-
 static inline int kvm_set_migration_log(int enable)
 {
     return kvm_physical_memory_set_dirty_tracking(enable);
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 9fb96b5..8743f32 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -25,8 +25,6 @@
 #include "gdbstub.h"
 #include "host-utils.h"
 
-#ifdef KVM_UPSTREAM
-
 #ifdef CONFIG_KVM_PARA
 #include <linux/kvm_para.h>
 #endif
@@ -133,6 +131,8 @@ uint32_t kvm_arch_get_supported_cpuid(CPUState *env, uint32_t function, int reg)
 
 #endif
 
+#ifdef KVM_UPSTREAM
+
 static void kvm_trim_features(uint32_t *features, uint32_t supported)
 {
     int i;
-- 
1.6.0.2

[Qemu-devel] [PATCH 09/21] qemu-kvm: Use upstream kvm_pit_in_kernel

[Jan Kiszka]

Drop private version in favor of recently added upstream service and
track it state directly in KVMState.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/i8254.c     |    4 ++--
 hw/pc.c        |    2 +-
 hw/pcspk.c     |    4 ++--
 kvm-all.c      |    2 +-
 kvm.h          |    2 +-
 qemu-kvm-x86.c |   12 ++++++------
 qemu-kvm.c     |    5 -----
 qemu-kvm.h     |   13 +------------
 8 files changed, 14 insertions(+), 30 deletions(-)

diff --git a/hw/i8254.c b/hw/i8254.c
index db9e94a..1add08e 100644
--- a/hw/i8254.c
+++ b/hw/i8254.c
@@ -491,7 +491,7 @@ void hpet_disable_pit(void)
 {
     PITChannelState *s = &pit_state.channels[0];
 
-    if (kvm_enabled() && qemu_kvm_pit_in_kernel()) {
+    if (kvm_enabled() && kvm_pit_in_kernel()) {
         if (qemu_kvm_has_pit_state2()) {
             kvm_hpet_disable_kpit();
         } else {
@@ -515,7 +515,7 @@ void hpet_enable_pit(void)
     PITState *pit = &pit_state;
     PITChannelState *s = &pit->channels[0];
 
-    if (kvm_enabled() && qemu_kvm_pit_in_kernel()) {
+    if (kvm_enabled() && kvm_pit_in_kernel()) {
         if (qemu_kvm_has_pit_state2()) {
             kvm_hpet_enable_kpit();
         } else {
diff --git a/hw/pc.c b/hw/pc.c
index dac373e..7a7dfa7 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -951,7 +951,7 @@ static void pc_init1(ram_addr_t ram_size,
         ioapic_irq_hack = isa_irq;
     }
 #ifdef CONFIG_KVM_PIT
-    if (kvm_enabled() && qemu_kvm_pit_in_kernel())
+    if (kvm_enabled() && kvm_pit_in_kernel())
 	pit = kvm_pit_init(0x40, isa_reserve_irq(0));
     else
 #endif
diff --git a/hw/pcspk.c b/hw/pcspk.c
index 128836b..fb5f763 100644
--- a/hw/pcspk.c
+++ b/hw/pcspk.c
@@ -56,7 +56,7 @@ static void kvm_get_pit_ch2(PITState *pit,
 {
     struct kvm_pit_state pit_state;
 
-    if (kvm_enabled() && qemu_kvm_pit_in_kernel()) {
+    if (kvm_enabled() && kvm_pit_in_kernel()) {
         kvm_get_pit(kvm_context, &pit_state);
         pit->channels[2].mode = pit_state.channels[2].mode;
         pit->channels[2].count = pit_state.channels[2].count;
@@ -71,7 +71,7 @@ static void kvm_get_pit_ch2(PITState *pit,
 static void kvm_set_pit_ch2(PITState *pit,
                             struct kvm_pit_state *inkernel_state)
 {
-    if (kvm_enabled() && qemu_kvm_pit_in_kernel()) {
+    if (kvm_enabled() && kvm_pit_in_kernel()) {
         inkernel_state->channels[2].mode = pit->channels[2].mode;
         inkernel_state->channels[2].count = pit->channels[2].count;
         inkernel_state->channels[2].count_load_time =
diff --git a/kvm-all.c b/kvm-all.c
index e7fa605..6cbca97 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -164,13 +164,13 @@ 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;
 }
 
 
+#ifdef KVM_UPSTREAM
 int kvm_init_vcpu(CPUState *env)
 {
     KVMState *s = kvm_state;
diff --git a/kvm.h b/kvm.h
index 189a5d4..253b45d 100644
--- a/kvm.h
+++ b/kvm.h
@@ -68,10 +68,10 @@ int kvm_remove_breakpoint(CPUState *current_env, target_ulong addr,
                           target_ulong len, int type);
 void kvm_remove_all_breakpoints(CPUState *current_env);
 int kvm_update_guest_debug(CPUState *env, unsigned long reinject_trap);
+#endif /* KVM_UPSTREAM */
 
 int kvm_pit_in_kernel(void);
 int kvm_irqchip_in_kernel(void);
-#endif /* KVM_UPSTREAM */
 
 /* internal API */
 
diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index 0457a6e..074b510 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -119,13 +119,13 @@ static int kvm_create_pit(kvm_context_t kvm)
 #ifdef KVM_CAP_PIT
 	int r;
 
-	kvm->pit_in_kernel = 0;
+	kvm_state->pit_in_kernel = 0;
 	if (!kvm->no_pit_creation) {
 		r = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION, KVM_CAP_PIT);
 		if (r > 0) {
 			r = kvm_vm_ioctl(kvm_state, KVM_CREATE_PIT);
 			if (r >= 0)
-				kvm->pit_in_kernel = 1;
+				kvm_state->pit_in_kernel = 1;
 			else {
 				fprintf(stderr, "Create kernel PIC irqchip failed\n");
 				return r;
@@ -311,14 +311,14 @@ int kvm_set_lapic(CPUState *env, struct kvm_lapic_state *s)
 
 int kvm_get_pit(kvm_context_t kvm, struct kvm_pit_state *s)
 {
-	if (!kvm->pit_in_kernel)
+	if (!kvm_pit_in_kernel())
 		return 0;
 	return kvm_vm_ioctl(kvm_state, KVM_GET_PIT, s);
 }
 
 int kvm_set_pit(kvm_context_t kvm, struct kvm_pit_state *s)
 {
-	if (!kvm->pit_in_kernel)
+	if (!kvm_pit_in_kernel())
 		return 0;
 	return kvm_vm_ioctl(kvm_state, KVM_SET_PIT, s);
 }
@@ -326,14 +326,14 @@ int kvm_set_pit(kvm_context_t kvm, struct kvm_pit_state *s)
 #ifdef KVM_CAP_PIT_STATE2
 int kvm_get_pit2(kvm_context_t kvm, struct kvm_pit_state2 *ps2)
 {
-	if (!kvm->pit_in_kernel)
+	if (!kvm_pit_in_kernel())
 		return 0;
 	return kvm_vm_ioctl(kvm_state, KVM_GET_PIT2, ps2);
 }
 
 int kvm_set_pit2(kvm_context_t kvm, struct kvm_pit_state2 *ps2)
 {
-	if (!kvm->pit_in_kernel)
+	if (!kvm_pit_in_kernel())
 		return 0;
 	return kvm_vm_ioctl(kvm_state, KVM_SET_PIT2, ps2);
 }
diff --git a/qemu-kvm.c b/qemu-kvm.c
index 12442a7..3ad0ec7 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -1040,11 +1040,6 @@ int kvm_set_signal_mask(CPUState *env, const sigset_t *sigset)
     return r;
 }
 
-int kvm_pit_in_kernel(kvm_context_t kvm)
-{
-    return kvm->pit_in_kernel;
-}
-
 int kvm_inject_nmi(CPUState *env)
 {
 #ifdef KVM_CAP_USER_NMI
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 7b75fdd..4c687ce 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -64,8 +64,6 @@ struct kvm_context {
     int irqchip_inject_ioctl;
     /// do not create in-kernel pit if set
     int no_pit_creation;
-    /// in-kernel pit status
-    int pit_in_kernel;
 #ifdef KVM_CAP_IRQ_ROUTING
     struct kvm_irq_routing *irq_routes;
     int nr_allocated_irq_routes;
@@ -563,13 +561,6 @@ void kvm_inject_x86_mce(CPUState *cenv, int bank, uint64_t status,
                         int abort_on_error);
 
 /*!
- * \brief Query wheather in kernel pit is used
- *
- *  \param kvm Pointer to the current kvm_context
- */
-int kvm_pit_in_kernel(kvm_context_t kvm);
-
-/*!
  * \brief Initialize coalesced MMIO
  *
  * Check for coalesced MMIO capability and store in context
@@ -1014,7 +1005,6 @@ int kvm_arch_halt(CPUState *env);
 int handle_tpr_access(void *opaque, CPUState *env, uint64_t rip,
                       int is_write);
 
-#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
 #define qemu_kvm_has_pit_state2() kvm_has_pit_state2(kvm_context)
@@ -1022,7 +1012,6 @@ int handle_tpr_access(void *opaque, CPUState *env, uint64_t rip,
 void kvm_load_tsc(CPUState *env);
 #else
 #define kvm_nested 0
-#define qemu_kvm_pit_in_kernel() (0)
 #define qemu_kvm_has_gsi_routing() (0)
 #ifdef TARGET_I386
 #define qemu_kvm_has_pit_state2() (0)
@@ -1055,7 +1044,6 @@ static inline int kvm_set_migration_log(int enable)
 }
 
 
-int kvm_irqchip_in_kernel(void);
 #ifdef CONFIG_KVM
 
 typedef struct KVMSlot {
@@ -1080,6 +1068,7 @@ struct KVMState {
     QTAILQ_HEAD(, kvm_sw_breakpoint) kvm_sw_breakpoints;
 #endif
     int irqchip_in_kernel;
+    int pit_in_kernel;
 
     struct kvm_context kvm_context;
 };
-- 
1.6.0.2

[Qemu-devel] [PATCH 10/21] KVM: Move and rename regs_modified

[Jan Kiszka]

Touching the user space representation of KVM's VCPU state is -
naturally - a per-VCPU thing. So move the dirty flag into KVM_CPU_COMMON
and rename it at this chance to reflect its true meaning.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpu-defs.h |    1 +
 kvm-all.c  |   12 ++++++------
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/cpu-defs.h b/cpu-defs.h
index cf502e9..49a9e8d 100644
--- a/cpu-defs.h
+++ b/cpu-defs.h
@@ -208,6 +208,7 @@ struct KVMCPUState {
     struct KVMState *kvm_state;                                         \
     struct kvm_run *kvm_run;                                            \
     int kvm_fd;                                                         \
+    int kvm_vcpu_dirty;                                                 \
     uint32_t stop;   /* Stop request */                                 \
     uint32_t stopped; /* Artificially stopped */                        \
     struct KVMCPUState kvm_cpu_state;
diff --git a/kvm-all.c b/kvm-all.c
index 6cbca97..3516f01 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -573,9 +573,9 @@ static void kvm_run_coalesced_mmio(CPUState *env, struct kvm_run *run)
 
 void kvm_cpu_synchronize_state(CPUState *env)
 {
-    if (!env->kvm_state->regs_modified) {
+    if (!env->kvm_vcpu_dirty) {
         kvm_arch_get_registers(env);
-        env->kvm_state->regs_modified = 1;
+        env->kvm_vcpu_dirty = 1;
     }
 }
 
@@ -593,9 +593,9 @@ int kvm_cpu_exec(CPUState *env)
             break;
         }
 
-        if (env->kvm_state->regs_modified) {
+        if (env->kvm_vcpu_dirty) {
             kvm_arch_put_registers(env);
-            env->kvm_state->regs_modified = 0;
+            env->kvm_vcpu_dirty = 0;
         }
 
         kvm_arch_pre_run(env, run);
@@ -951,9 +951,9 @@ static void kvm_invoke_set_guest_debug(void *data)
     struct kvm_set_guest_debug_data *dbg_data = data;
     CPUState *env = dbg_data->env;
 
-    if (env->kvm_state->regs_modified) {
+    if (env->kvm_vcpu_dirty) {
         kvm_arch_put_registers(env);
-        env->kvm_state->regs_modified = 0;
+        env->kvm_vcpu_dirty = 0;
     }
     dbg_data->err = kvm_vcpu_ioctl(env, KVM_SET_GUEST_DEBUG, &dbg_data->dbg);
 }
-- 
1.6.0.2

[Qemu-devel] [PATCH 11/21] KVM: Rework of guest debug state writing

[Jan Kiszka]

So far we synchronized any dirty VCPU state back into the kernel before
updating the guest debug state. This was a tribute to a deficite in x86
kernels before 2.6.33. But as this is an arch-dependent issue, it is
better handle in the x86 part of KVM and remove the writeback point for
generic code.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kvm-all.c         |   12 ++++--------
 target-i386/cpu.h |    9 ++++++++-
 target-i386/kvm.c |   11 +++++++++++
 3 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 3516f01..9c921cc 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -951,10 +951,6 @@ static void kvm_invoke_set_guest_debug(void *data)
     struct kvm_set_guest_debug_data *dbg_data = data;
     CPUState *env = dbg_data->env;
 
-    if (env->kvm_vcpu_dirty) {
-        kvm_arch_put_registers(env);
-        env->kvm_vcpu_dirty = 0;
-    }
     dbg_data->err = kvm_vcpu_ioctl(env, KVM_SET_GUEST_DEBUG, &dbg_data->dbg);
 }
 
@@ -962,12 +958,12 @@ int kvm_update_guest_debug(CPUState *env, unsigned long reinject_trap)
 {
     struct kvm_set_guest_debug_data data;
 
-    data.dbg.control = 0;
-    if (env->singlestep_enabled)
-        data.dbg.control = KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP;
+    data.dbg.control = reinject_trap;
 
+    if (env->singlestep_enabled) {
+        data.dbg.control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP;
+    }
     kvm_arch_update_guest_debug(env, &data.dbg);
-    data.dbg.control |= reinject_trap;
     data.env = env;
 
     on_vcpu(env, kvm_invoke_set_guest_debug, &data);
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 7d0bbd0..7787fb1 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -21,6 +21,10 @@
 
 #include "config.h"
 
+#ifdef CONFIG_KVM
+#include <linux/kvm.h>  /* for kvm_guest_debug */
+#endif
+
 #ifdef TARGET_X86_64
 #define TARGET_LONG_BITS 64
 #else
@@ -718,7 +722,10 @@ typedef struct CPUX86State {
     uint8_t has_error_code;
     uint32_t sipi_vector;
     uint32_t cpuid_kvm_features;
-    
+#if defined(CONFIG_KVM) && defined(KVM_CAP_SET_GUEST_DEBUG)
+    struct kvm_guest_debug kvm_guest_debug;
+#endif
+
     /* in order to simplify APIC support, we leave this pointer to the
        user */
     struct APICState *apic_state;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 8743f32..5ac12a8 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -865,6 +865,15 @@ int kvm_arch_put_registers(CPUState *env)
     if (ret < 0)
         return ret;
 
+    /*
+     * Kernels before 2.6.33 overwrote flags.TF injected via SET_GUEST_DEBUG
+     * while updating GP regs. Work around this by updating the debug state
+     * once again.
+     */
+    ret = kvm_vcpu_ioctl(env, KVM_SET_GUEST_DEBUG, &env->kvm_guest_debug);
+    if (ret < 0)
+        return ret;
+
     ret = kvm_put_fpu(env);
     if (ret < 0)
         return ret;
@@ -1163,6 +1172,8 @@ void kvm_arch_update_guest_debug(CPUState *env, struct kvm_guest_debug *dbg)
                 (len_code[hw_breakpoint[n].len] << (18 + n*4));
         }
     }
+    /* Keep a copy for the writeback workaround in kvm_arch_put_registers */
+    memcpy(&env->kvm_guest_debug, dbg, sizeof(env->kvm_guest_debug));
 }
 #endif /* KVM_CAP_SET_GUEST_DEBUG */
 #endif
-- 
1.6.0.2

[Qemu-devel] [PATCH 12/21] qemu-kvm: Use upstream kvm_vcpu_dirty

[Jan Kiszka]

Drop regs_modified in favor of upstream's equivalent and clean up
kvm_cpu_synchronize_state at this chance.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpu-defs.h |    1 -
 hw/pc.c    |    2 +-
 qemu-kvm.c |   18 +++++++++---------
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/cpu-defs.h b/cpu-defs.h
index 49a9e8d..c57d8df 100644
--- a/cpu-defs.h
+++ b/cpu-defs.h
@@ -142,7 +142,6 @@ struct KVMCPUState {
     pthread_t thread;
     int signalled;
     struct qemu_work_item *queued_work_first, *queued_work_last;
-    int regs_modified;
 };
 
 #define CPU_TEMP_BUF_NLONGS 128
diff --git a/hw/pc.c b/hw/pc.c
index 7a7dfa7..af6ea8b 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -744,7 +744,7 @@ CPUState *pc_new_cpu(const char *cpu_model)
         fprintf(stderr, "Unable to find x86 CPU definition\n");
         exit(1);
     }
-    env->kvm_cpu_state.regs_modified = 1;
+    env->kvm_vcpu_dirty = 1;
     if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) {
         env->cpuid_apic_id = env->cpu_index;
         /* APIC reset callback resets cpu */
diff --git a/qemu-kvm.c b/qemu-kvm.c
index 3ad0ec7..c04f805 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -861,9 +861,9 @@ int pre_kvm_run(kvm_context_t kvm, CPUState *env)
 {
     kvm_arch_pre_run(env, env->kvm_run);
 
-    if (env->kvm_cpu_state.regs_modified) {
+    if (env->kvm_vcpu_dirty) {
         kvm_arch_load_regs(env);
-        env->kvm_cpu_state.regs_modified = 0;
+        env->kvm_vcpu_dirty = 0;
     }
 
     pthread_mutex_unlock(&qemu_mutex);
@@ -1530,16 +1530,16 @@ static void on_vcpu(CPUState *env, void (*func)(void *data), void *data)
 static void do_kvm_cpu_synchronize_state(void *_env)
 {
     CPUState *env = _env;
-    if (!env->kvm_cpu_state.regs_modified) {
-        kvm_arch_save_regs(env);
-        env->kvm_cpu_state.regs_modified = 1;
-    }
+
+    kvm_arch_save_regs(env);
 }
 
 void kvm_cpu_synchronize_state(CPUState *env)
 {
-    if (!env->kvm_cpu_state.regs_modified)
+    if (!env->kvm_vcpu_dirty) {
         on_vcpu(env, do_kvm_cpu_synchronize_state, env);
+        env->kvm_vcpu_dirty = 1;
+    }
 }
 
 static void inject_interrupt(void *data)
@@ -2329,9 +2329,9 @@ static void kvm_invoke_set_guest_debug(void *data)
 {
     struct kvm_set_guest_debug_data *dbg_data = data;
 
-    if (cpu_single_env->kvm_cpu_state.regs_modified) {
+    if (cpu_single_env->kvm_vcpu_dirty) {
         kvm_arch_save_regs(cpu_single_env);
-        cpu_single_env->kvm_cpu_state.regs_modified = 0;
+        cpu_single_env->kvm_vcpu_dirty = 0;
     }
     dbg_data->err =
         kvm_set_guest_debug(cpu_single_env,
-- 
1.6.0.2

[Qemu-devel] [PATCH 13/21] qemu-kvm: Use upstream guest debug code

[Jan Kiszka]

Code was absolute identical except for previous cleanup in upstream.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kvm-all.c         |    7 +-
 kvm.h             |    4 -
 qemu-kvm-x86.c    |  178 ++--------------------------------------------------
 qemu-kvm.c        |   44 -------------
 qemu-kvm.h        |   37 -----------
 target-i386/kvm.c |    2 +-
 6 files changed, 11 insertions(+), 261 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 9c921cc..f3cfa2c 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -919,7 +919,9 @@ static void on_vcpu(CPUState *env, void (*func)(void *data), void *data)
     func(data);
 #endif
 }
-#endif /* KVM_UPSTREAM */
+#else /* !KVM_UPSTREAM */
+static void on_vcpu(CPUState *env, void (*func)(void *data), void *data);
+#endif /* !KVM_UPSTREAM */
 
 struct kvm_sw_breakpoint *kvm_find_sw_breakpoint(CPUState *env,
                                                  target_ulong pc)
@@ -938,8 +940,6 @@ int kvm_sw_breakpoints_active(CPUState *env)
     return !QTAILQ_EMPTY(&env->kvm_state->kvm_sw_breakpoints);
 }
 
-#ifdef KVM_UPSTREAM
-
 struct kvm_set_guest_debug_data {
     struct kvm_guest_debug dbg;
     CPUState *env;
@@ -969,7 +969,6 @@ int kvm_update_guest_debug(CPUState *env, unsigned long reinject_trap)
     on_vcpu(env, kvm_invoke_set_guest_debug, &data);
     return data.err;
 }
-#endif
 
 int kvm_insert_breakpoint(CPUState *current_env, target_ulong addr,
                           target_ulong len, int type)
diff --git a/kvm.h b/kvm.h
index 253b45d..740fd1a 100644
--- a/kvm.h
+++ b/kvm.h
@@ -61,14 +61,12 @@ void kvm_setup_guest_memory(void *start, size_t size);
 int kvm_coalesce_mmio_region(target_phys_addr_t start, ram_addr_t size);
 int kvm_uncoalesce_mmio_region(target_phys_addr_t start, ram_addr_t size);
 
-#ifdef KVM_UPSTREAM
 int kvm_insert_breakpoint(CPUState *current_env, target_ulong addr,
                           target_ulong len, int type);
 int kvm_remove_breakpoint(CPUState *current_env, target_ulong addr,
                           target_ulong len, int type);
 void kvm_remove_all_breakpoints(CPUState *current_env);
 int kvm_update_guest_debug(CPUState *env, unsigned long reinject_trap);
-#endif /* KVM_UPSTREAM */
 
 int kvm_pit_in_kernel(void);
 int kvm_irqchip_in_kernel(void);
@@ -101,7 +99,6 @@ int kvm_arch_init(KVMState *s, int smp_cpus);
 int kvm_arch_init_vcpu(CPUState *env);
 
 void kvm_arch_reset_vcpu(CPUState *env);
-#ifdef KVM_UPSTREAM
 
 struct kvm_guest_debug;
 struct kvm_debug_exit_arch;
@@ -133,7 +130,6 @@ int kvm_arch_remove_hw_breakpoint(target_ulong addr,
 void kvm_arch_remove_all_hw_breakpoints(void);
 
 void kvm_arch_update_guest_debug(CPUState *env, struct kvm_guest_debug *dbg);
-#endif
 
 int kvm_check_extension(KVMState *s, unsigned int extension);
 
diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index 074b510..834e9c1 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -835,6 +835,13 @@ void kvm_arch_load_regs(CPUState *env)
 
     kvm_set_regs(env, &regs);
 
+    /*
+     * Kernels before 2.6.33 overwrote flags.TF injected via SET_GUEST_DEBUG
+     * while updating GP regs. Work around this by updating the debug state
+     * once again.
+     */
+    kvm_vcpu_ioctl(env, KVM_SET_GUEST_DEBUG, &env->kvm_guest_debug);
+
     memset(&fpu, 0, sizeof fpu);
     fpu.fsw = env->fpus & ~(7 << 11);
     fpu.fsw |= (env->fpstt & 7) << 11;
@@ -1372,177 +1379,6 @@ void kvm_arch_cpu_reset(CPUState *env)
     }
 }
 
-int kvm_arch_insert_sw_breakpoint(CPUState *env, struct kvm_sw_breakpoint *bp)
-{
-    uint8_t int3 = 0xcc;
-
-    if (cpu_memory_rw_debug(env, bp->pc, (uint8_t *)&bp->saved_insn, 1, 0) ||
-        cpu_memory_rw_debug(env, bp->pc, &int3, 1, 1))
-        return -EINVAL;
-    return 0;
-}
-
-int kvm_arch_remove_sw_breakpoint(CPUState *env, struct kvm_sw_breakpoint *bp)
-{
-    uint8_t int3;
-
-    if (cpu_memory_rw_debug(env, bp->pc, &int3, 1, 0) || int3 != 0xcc ||
-        cpu_memory_rw_debug(env, bp->pc, (uint8_t *)&bp->saved_insn, 1, 1))
-        return -EINVAL;
-    return 0;
-}
-
-#ifdef KVM_CAP_SET_GUEST_DEBUG
-static struct {
-    target_ulong addr;
-    int len;
-    int type;
-} hw_breakpoint[4];
-
-static int nb_hw_breakpoint;
-
-static int find_hw_breakpoint(target_ulong addr, int len, int type)
-{
-    int n;
-
-    for (n = 0; n < nb_hw_breakpoint; n++)
-	if (hw_breakpoint[n].addr == addr && hw_breakpoint[n].type == type &&
-	    (hw_breakpoint[n].len == len || len == -1))
-	    return n;
-    return -1;
-}
-
-int kvm_arch_insert_hw_breakpoint(target_ulong addr,
-                                  target_ulong len, int type)
-{
-    switch (type) {
-    case GDB_BREAKPOINT_HW:
-	len = 1;
-	break;
-    case GDB_WATCHPOINT_WRITE:
-    case GDB_WATCHPOINT_ACCESS:
-	switch (len) {
-	case 1:
-	    break;
-	case 2:
-	case 4:
-	case 8:
-	    if (addr & (len - 1))
-		return -EINVAL;
-	    break;
-	default:
-	    return -EINVAL;
-	}
-	break;
-    default:
-	return -ENOSYS;
-    }
-
-    if (nb_hw_breakpoint == 4)
-        return -ENOBUFS;
-
-    if (find_hw_breakpoint(addr, len, type) >= 0)
-        return -EEXIST;
-
-    hw_breakpoint[nb_hw_breakpoint].addr = addr;
-    hw_breakpoint[nb_hw_breakpoint].len = len;
-    hw_breakpoint[nb_hw_breakpoint].type = type;
-    nb_hw_breakpoint++;
-
-    return 0;
-}
-
-int kvm_arch_remove_hw_breakpoint(target_ulong addr,
-                                  target_ulong len, int type)
-{
-    int n;
-
-    n = find_hw_breakpoint(addr, (type == GDB_BREAKPOINT_HW) ? 1 : len, type);
-    if (n < 0)
-        return -ENOENT;
-
-    nb_hw_breakpoint--;
-    hw_breakpoint[n] = hw_breakpoint[nb_hw_breakpoint];
-
-    return 0;
-}
-
-void kvm_arch_remove_all_hw_breakpoints(void)
-{
-    nb_hw_breakpoint = 0;
-}
-
-static CPUWatchpoint hw_watchpoint;
-
-int kvm_arch_debug(struct kvm_debug_exit_arch *arch_info)
-{
-    int handle = 0;
-    int n;
-
-    if (arch_info->exception == 1) {
-	if (arch_info->dr6 & (1 << 14)) {
-	    if (cpu_single_env->singlestep_enabled)
-		handle = 1;
-	} else {
-	    for (n = 0; n < 4; n++)
-		if (arch_info->dr6 & (1 << n))
-		    switch ((arch_info->dr7 >> (16 + n*4)) & 0x3) {
-		    case 0x0:
-			handle = 1;
-			break;
-		    case 0x1:
-			handle = 1;
-			cpu_single_env->watchpoint_hit = &hw_watchpoint;
-			hw_watchpoint.vaddr = hw_breakpoint[n].addr;
-			hw_watchpoint.flags = BP_MEM_WRITE;
-			break;
-		    case 0x3:
-			handle = 1;
-			cpu_single_env->watchpoint_hit = &hw_watchpoint;
-			hw_watchpoint.vaddr = hw_breakpoint[n].addr;
-			hw_watchpoint.flags = BP_MEM_ACCESS;
-			break;
-		    }
-	}
-    } else if (kvm_find_sw_breakpoint(cpu_single_env, arch_info->pc))
-	handle = 1;
-
-    if (!handle)
-	kvm_update_guest_debug(cpu_single_env,
-			(arch_info->exception == 1) ?
-			KVM_GUESTDBG_INJECT_DB : KVM_GUESTDBG_INJECT_BP);
-
-    return handle;
-}
-
-void kvm_arch_update_guest_debug(CPUState *env, struct kvm_guest_debug *dbg)
-{
-    const uint8_t type_code[] = {
-	[GDB_BREAKPOINT_HW] = 0x0,
-	[GDB_WATCHPOINT_WRITE] = 0x1,
-	[GDB_WATCHPOINT_ACCESS] = 0x3
-    };
-    const uint8_t len_code[] = {
-	[1] = 0x0, [2] = 0x1, [4] = 0x3, [8] = 0x2
-    };
-    int n;
-
-    if (kvm_sw_breakpoints_active(env))
-	dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP;
-
-    if (nb_hw_breakpoint > 0) {
-	dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_HW_BP;
-	dbg->arch.debugreg[7] = 0x0600;
-	for (n = 0; n < nb_hw_breakpoint; n++) {
-	    dbg->arch.debugreg[n] = hw_breakpoint[n].addr;
-	    dbg->arch.debugreg[7] |= (2 << (n * 2)) |
-		(type_code[hw_breakpoint[n].type] << (16 + n*4)) |
-		(len_code[hw_breakpoint[n].len] << (18 + n*4));
-	}
-    }
-}
-#endif
-
 #ifdef CONFIG_KVM_DEVICE_ASSIGNMENT
 void kvm_arch_do_ioperm(void *_data)
 {
diff --git a/qemu-kvm.c b/qemu-kvm.c
index c04f805..a220353 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -1016,13 +1016,6 @@ int kvm_inject_irq(CPUState *env, unsigned irq)
     return kvm_vcpu_ioctl(env, KVM_INTERRUPT, &intr);
 }
 
-#ifdef KVM_CAP_SET_GUEST_DEBUG
-int kvm_set_guest_debug(CPUState *env, struct kvm_guest_debug *dbg)
-{
-    return kvm_vcpu_ioctl(env, KVM_SET_GUEST_DEBUG, dbg);
-}
-#endif
-
 int kvm_set_signal_mask(CPUState *env, const sigset_t *sigset)
 {
     struct kvm_signal_mask *sigmask;
@@ -2318,43 +2311,6 @@ void kvm_set_phys_mem(target_phys_addr_t start_addr, ram_addr_t size,
     return;
 }
 
-#ifdef KVM_CAP_SET_GUEST_DEBUG
-
-struct kvm_set_guest_debug_data {
-    struct kvm_guest_debug dbg;
-    int err;
-};
-
-static void kvm_invoke_set_guest_debug(void *data)
-{
-    struct kvm_set_guest_debug_data *dbg_data = data;
-
-    if (cpu_single_env->kvm_vcpu_dirty) {
-        kvm_arch_save_regs(cpu_single_env);
-        cpu_single_env->kvm_vcpu_dirty = 0;
-    }
-    dbg_data->err =
-        kvm_set_guest_debug(cpu_single_env,
-                            &dbg_data->dbg);
-}
-
-int kvm_update_guest_debug(CPUState *env, unsigned long reinject_trap)
-{
-    struct kvm_set_guest_debug_data data;
-
-    data.dbg.control = 0;
-    if (env->singlestep_enabled)
-        data.dbg.control = KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP;
-
-    kvm_arch_update_guest_debug(env, &data.dbg);
-    data.dbg.control |= reinject_trap;
-
-    on_vcpu(env, kvm_invoke_set_guest_debug, &data);
-    return data.err;
-}
-
-#endif
-
 /*
  * dirty pages logging
  */
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 4c687ce..e8c3a58 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -323,10 +323,6 @@ static inline int kvm_reset_mpstate(CPUState *env)
  */
 int kvm_inject_irq(CPUState *env, unsigned irq);
 
-#ifdef KVM_CAP_SET_GUEST_DEBUG
-int kvm_set_guest_debug(CPUState *env, struct kvm_guest_debug *dbg);
-#endif
-
 #if defined(__i386__) || defined(__x86_64__)
 /*!
  * \brief Setup a vcpu's cpuid instruction emulation
@@ -880,12 +876,6 @@ int kvm_init_ap(void);
 int kvm_vcpu_inited(CPUState *env);
 void kvm_load_mpstate(CPUState *env);
 void kvm_save_mpstate(CPUState *env);
-int kvm_insert_breakpoint(CPUState * current_env, target_ulong addr,
-                          target_ulong len, int type);
-int kvm_remove_breakpoint(CPUState * current_env, target_ulong addr,
-                          target_ulong len, int type);
-void kvm_remove_all_breakpoints(CPUState * current_env);
-int kvm_update_guest_debug(CPUState *env, unsigned long reinject_trap);
 void kvm_apic_init(CPUState *env);
 /* called from vcpu initialization */
 void qemu_kvm_load_lapic(CPUState *env);
@@ -928,33 +918,6 @@ void kvm_arch_push_nmi(void *opaque);
 void kvm_arch_cpu_reset(CPUState *env);
 int kvm_set_boot_cpu_id(uint32_t id);
 
-struct kvm_guest_debug;
-struct kvm_debug_exit_arch;
-
-struct kvm_sw_breakpoint {
-    target_ulong pc;
-    target_ulong saved_insn;
-    int use_count;
-    QTAILQ_ENTRY(kvm_sw_breakpoint) entry;
-};
-
-QTAILQ_HEAD(kvm_sw_breakpoint_head, kvm_sw_breakpoint);
-
-int kvm_arch_debug(struct kvm_debug_exit_arch *arch_info);
-int kvm_sw_breakpoints_active(CPUState *env);
-struct kvm_sw_breakpoint *kvm_find_sw_breakpoint(CPUState *env,
-                                                 target_ulong pc);
-int kvm_arch_insert_sw_breakpoint(CPUState * current_env,
-                                  struct kvm_sw_breakpoint *bp);
-int kvm_arch_remove_sw_breakpoint(CPUState * current_env,
-                                  struct kvm_sw_breakpoint *bp);
-int kvm_arch_insert_hw_breakpoint(target_ulong addr, target_ulong len,
-                                  int type);
-int kvm_arch_remove_hw_breakpoint(target_ulong addr, target_ulong len,
-                                  int type);
-void kvm_arch_remove_all_hw_breakpoints(void);
-void kvm_arch_update_guest_debug(CPUState *env, struct kvm_guest_debug *dbg);
-
 void qemu_kvm_aio_wait_start(void);
 void qemu_kvm_aio_wait(void);
 void qemu_kvm_aio_wait_end(void);
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 5ac12a8..e1ae15d 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1003,6 +1003,7 @@ int kvm_arch_handle_exit(CPUState *env, struct kvm_run *run)
 
     return ret;
 }
+#endif
 
 #ifdef KVM_CAP_SET_GUEST_DEBUG
 int kvm_arch_insert_sw_breakpoint(CPUState *env, struct kvm_sw_breakpoint *bp)
@@ -1176,6 +1177,5 @@ void kvm_arch_update_guest_debug(CPUState *env, struct kvm_guest_debug *dbg)
     memcpy(&env->kvm_guest_debug, dbg, sizeof(env->kvm_guest_debug));
 }
 #endif /* KVM_CAP_SET_GUEST_DEBUG */
-#endif
 
 #include "qemu-kvm-x86.c"
-- 
1.6.0.2

[Qemu-devel] [PATCH 14/21] qemu-kvm: Rework VCPU state writeback API

[Jan Kiszka]

This grand cleanup drops all reset and vmsave/load related
synchronization points in favor of four(!) generic hooks:

- cpu_synchronize_all_states in qemu_savevm_state_complete
  (initial sync from kernel before vmsave)
- cpu_synchronize_all_post_init in qemu_loadvm_state
  (writeback after vmload)
- cpu_synchronize_all_post_init in main after machine init
- cpu_synchronize_all_post_reset in qemu_system_reset
  (writeback after system reset)

These writeback points + the existing one of VCPU exec after
cpu_synchronize_state map on three levels of writeback:

- KVM_PUT_ASYNC_STATE (during runtime, other VCPUs continue to run)
- KVM_PUT_RESET_STATE (on synchronous system reset, all VCPUs stopped)
- KVM_PUT_FULL_STATE  (on init or vmload, all VCPUs stopped as well)

This level is passed to the arch-specific VCPU state writing function
that will decide which concrete substates need to be written. That way,
no writer of load, save or reset functions that interact with in-kernel
KVM states will ever have to worry about synchronization again. That
also means that a lot of reasons for races, segfaults and deadlocks are
eliminated.

cpu_synchronize_state remains untouched, just as Anthony suggested. We
continue to need it before reading or writing of VCPU states that are
also tracked by in-kernel KVM subsystems.

Consequently, this patch removes many cpu_synchronize_state calls that
are now redundant, just like remaining explicit register syncs. It does
not touch qemu-kvm's special hooks for mpstate, vcpu_events, or tsc
loading. They will be cleaned up by individual patches.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 exec.c                |   17 -----------------
 hw/apic.c             |    3 ---
 hw/pc.c               |    1 -
 hw/ppc_newworld.c     |    3 ---
 hw/ppc_oldworld.c     |    3 ---
 hw/s390-virtio.c      |    1 -
 kvm-all.c             |   19 +++++++++++++------
 kvm.h                 |   22 +++++++++++++++++++++-
 qemu-kvm-ia64.c       |    2 +-
 qemu-kvm-x86.c        |    3 +--
 qemu-kvm.c            |   16 +++++++++++++---
 qemu-kvm.h            |    2 +-
 savevm.c              |    4 ++++
 sysemu.h              |    4 ++++
 target-i386/kvm.c     |    2 +-
 target-i386/machine.c |   10 ----------
 target-ia64/machine.c |    2 --
 target-ppc/kvm.c      |    2 +-
 target-ppc/machine.c  |    4 ----
 target-s390x/kvm.c    |    3 +--
 vl.c                  |   29 +++++++++++++++++++++++++++++
 21 files changed, 90 insertions(+), 62 deletions(-)

diff --git a/exec.c b/exec.c
index ade09cb..7b35e0f 100644
--- a/exec.c
+++ b/exec.c
@@ -529,21 +529,6 @@ void cpu_exec_init_all(unsigned long tb_size)
 
 #if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY)
 
-static void cpu_common_pre_save(void *opaque)
-{
-    CPUState *env = opaque;
-
-    cpu_synchronize_state(env);
-}
-
-static int cpu_common_pre_load(void *opaque)
-{
-    CPUState *env = opaque;
-
-    cpu_synchronize_state(env);
-    return 0;
-}
-
 static int cpu_common_post_load(void *opaque, int version_id)
 {
     CPUState *env = opaque;
@@ -561,8 +546,6 @@ static const VMStateDescription vmstate_cpu_common = {
     .version_id = 1,
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
-    .pre_save = cpu_common_pre_save,
-    .pre_load = cpu_common_pre_load,
     .post_load = cpu_common_post_load,
     .fields      = (VMStateField []) {
         VMSTATE_UINT32(halted, CPUState),
diff --git a/hw/apic.c b/hw/apic.c
index ae805dc..3e03e10 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -488,7 +488,6 @@ void apic_init_reset(CPUState *env)
     if (!s)
         return;
 
-    cpu_synchronize_state(env);
     s->tpr = 0;
     s->spurious_vec = 0xff;
     s->log_dest = 0;
@@ -1070,8 +1069,6 @@ static void apic_reset(void *opaque)
     APICState *s = opaque;
     int bsp;
 
-    cpu_synchronize_state(s->cpu_env);
-
     bsp = cpu_is_bsp(s->cpu_env);
     s->apicbase = 0xfee00000 |
         (bsp ? MSR_IA32_APICBASE_BSP : 0) | MSR_IA32_APICBASE_ENABLE;
diff --git a/hw/pc.c b/hw/pc.c
index af6ea8b..6c15a9f 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -744,7 +744,6 @@ CPUState *pc_new_cpu(const char *cpu_model)
         fprintf(stderr, "Unable to find x86 CPU definition\n");
         exit(1);
     }
-    env->kvm_vcpu_dirty = 1;
     if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) {
         env->cpuid_apic_id = env->cpu_index;
         /* APIC reset callback resets cpu */
diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c
index a4c714a..9e288bd 100644
--- a/hw/ppc_newworld.c
+++ b/hw/ppc_newworld.c
@@ -139,9 +139,6 @@ static void ppc_core99_init (ram_addr_t ram_size,
         envs[i] = env;
     }
 
-    /* Make sure all register sets take effect */
-    cpu_synchronize_state(env);
-
     /* allocate RAM */
     ram_offset = qemu_ram_alloc(ram_size);
     cpu_register_physical_memory(0, ram_size, ram_offset);
diff --git a/hw/ppc_oldworld.c b/hw/ppc_oldworld.c
index 7ccc6a1..1aa05ed 100644
--- a/hw/ppc_oldworld.c
+++ b/hw/ppc_oldworld.c
@@ -164,9 +164,6 @@ static void ppc_heathrow_init (ram_addr_t ram_size,
         envs[i] = env;
     }
 
-    /* Make sure all register sets take effect */
-    cpu_synchronize_state(env);
-
     /* allocate RAM */
     if (ram_size > (2047 << 20)) {
         fprintf(stderr,
diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
index 3582728..ad3386f 100644
--- a/hw/s390-virtio.c
+++ b/hw/s390-virtio.c
@@ -185,7 +185,6 @@ static void s390_init(ram_addr_t ram_size,
             exit(1);
         }
 
-        cpu_synchronize_state(env);
         env->psw.addr = KERN_IMAGE_START;
         env->psw.mask = 0x0000000180000000ULL;
     }
diff --git a/kvm-all.c b/kvm-all.c
index f3cfa2c..8fc89c1 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -152,10 +152,6 @@ static void kvm_reset_vcpu(void *opaque)
     CPUState *env = opaque;
 
     kvm_arch_reset_vcpu(env);
-    if (kvm_arch_put_registers(env)) {
-        fprintf(stderr, "Fatal: kvm vcpu reset failed\n");
-        abort();
-    }
 }
 #endif
 
@@ -206,7 +202,6 @@ int kvm_init_vcpu(CPUState *env)
     if (ret == 0) {
         qemu_register_reset(kvm_reset_vcpu, env);
         kvm_arch_reset_vcpu(env);
-        ret = kvm_arch_put_registers(env);
     }
 err:
     return ret;
@@ -579,6 +574,18 @@ void kvm_cpu_synchronize_state(CPUState *env)
     }
 }
 
+void kvm_cpu_synchronize_post_reset(CPUState *env)
+{
+    kvm_arch_put_registers(env, KVM_PUT_RESET_STATE);
+    env->kvm_vcpu_dirty = 0;
+}
+
+void kvm_cpu_synchronize_post_init(CPUState *env)
+{
+    kvm_arch_put_registers(env, KVM_PUT_FULL_STATE);
+    env->kvm_vcpu_dirty = 0;
+}
+
 int kvm_cpu_exec(CPUState *env)
 {
     struct kvm_run *run = env->kvm_run;
@@ -594,7 +601,7 @@ int kvm_cpu_exec(CPUState *env)
         }
 
         if (env->kvm_vcpu_dirty) {
-            kvm_arch_put_registers(env);
+            kvm_arch_put_registers(env, KVM_PUT_ASYNC_STATE);
             env->kvm_vcpu_dirty = 0;
         }
 
diff --git a/kvm.h b/kvm.h
index 740fd1a..ee8b3f6 100644
--- a/kvm.h
+++ b/kvm.h
@@ -92,7 +92,11 @@ int kvm_arch_pre_run(CPUState *env, struct kvm_run *run);
 
 int kvm_arch_get_registers(CPUState *env);
 
-int kvm_arch_put_registers(CPUState *env);
+#define KVM_PUT_ASYNC_STATE     1
+#define KVM_PUT_RESET_STATE     2
+#define KVM_PUT_FULL_STATE      3
+
+int kvm_arch_put_registers(CPUState *env, int level);
 
 int kvm_arch_init(KVMState *s, int smp_cpus);
 
@@ -136,6 +140,8 @@ int kvm_check_extension(KVMState *s, unsigned int extension);
 uint32_t kvm_arch_get_supported_cpuid(CPUState *env, uint32_t function,
                                       int reg);
 void kvm_cpu_synchronize_state(CPUState *env);
+void kvm_cpu_synchronize_post_reset(CPUState *env);
+void kvm_cpu_synchronize_post_init(CPUState *env);
 
 /* generic hooks - to be moved/refactored once there are more users */
 
@@ -146,4 +152,18 @@ static inline void cpu_synchronize_state(CPUState *env)
     }
 }
 
+static inline void cpu_synchronize_post_reset(CPUState *env)
+{
+    if (kvm_enabled()) {
+        kvm_cpu_synchronize_post_reset(env);
+    }
+}
+
+static inline void cpu_synchronize_post_init(CPUState *env)
+{
+    if (kvm_enabled()) {
+        kvm_cpu_synchronize_post_init(env);
+    }
+}
+
 #endif
diff --git a/qemu-kvm-ia64.c b/qemu-kvm-ia64.c
index a11fde8..fc8110e 100644
--- a/qemu-kvm-ia64.c
+++ b/qemu-kvm-ia64.c
@@ -16,7 +16,7 @@ int kvm_arch_qemu_create_context(void)
     return 0;
 }
 
-void kvm_arch_load_regs(CPUState *env)
+void kvm_arch_load_regs(CPUState *env, int level)
 {
 }
 
diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index 834e9c1..63cd095 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -803,7 +803,7 @@ static void get_seg(SegmentCache *lhs, const struct kvm_segment *rhs)
 	| (rhs->avl * DESC_AVL_MASK);
 }
 
-void kvm_arch_load_regs(CPUState *env)
+void kvm_arch_load_regs(CPUState *env, int level)
 {
     struct kvm_regs regs;
     struct kvm_fpu fpu;
@@ -1365,7 +1365,6 @@ void kvm_arch_push_nmi(void *opaque)
 void kvm_arch_cpu_reset(CPUState *env)
 {
     kvm_arch_reset_vcpu(env);
-    kvm_arch_load_regs(env);
     kvm_put_vcpu_events(env);
     if (!cpu_is_bsp(env)) {
 	if (kvm_irqchip_in_kernel()) {
diff --git a/qemu-kvm.c b/qemu-kvm.c
index a220353..53030f1 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -862,7 +862,7 @@ int pre_kvm_run(kvm_context_t kvm, CPUState *env)
     kvm_arch_pre_run(env, env->kvm_run);
 
     if (env->kvm_vcpu_dirty) {
-        kvm_arch_load_regs(env);
+        kvm_arch_load_regs(env, KVM_PUT_ASYNC_STATE);
         env->kvm_vcpu_dirty = 0;
     }
 
@@ -1535,6 +1535,18 @@ void kvm_cpu_synchronize_state(CPUState *env)
     }
 }
 
+void kvm_cpu_synchronize_post_reset(CPUState *env)
+{
+    kvm_arch_load_regs(env, KVM_PUT_RESET_STATE);
+    env->kvm_vcpu_dirty = 0;
+}
+
+void kvm_cpu_synchronize_post_init(CPUState *env)
+{
+    kvm_arch_load_regs(env, KVM_PUT_FULL_STATE);
+    env->kvm_vcpu_dirty = 0;
+}
+
 static void inject_interrupt(void *data)
 {
     cpu_interrupt(current_env, (long) data);
@@ -1880,8 +1892,6 @@ static void *ap_main_loop(void *_env)
 
     kvm_arch_init_vcpu(env);
 
-    kvm_arch_load_regs(env);
-
     /* signal VCPU creation */
     current_env->created = 1;
     pthread_cond_signal(&qemu_vcpu_cond);
diff --git a/qemu-kvm.h b/qemu-kvm.h
index e8c3a58..6d785a0 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -908,7 +908,7 @@ int kvm_qemu_destroy_memory_alias(uint64_t phys_start);
 int kvm_arch_qemu_create_context(void);
 
 void kvm_arch_save_regs(CPUState *env);
-void kvm_arch_load_regs(CPUState *env);
+void kvm_arch_load_regs(CPUState *env, int level);
 void kvm_arch_load_mpstate(CPUState *env);
 void kvm_arch_save_mpstate(CPUState *env);
 int kvm_arch_has_work(CPUState *env);
diff --git a/savevm.c b/savevm.c
index 2fd3de6..f1e675c 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1368,6 +1368,8 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f)
 {
     SaveStateEntry *se;
 
+    cpu_synchronize_all_states();
+
     QTAILQ_FOREACH(se, &savevm_handlers, entry) {
         if (se->save_live_state == NULL)
             continue;
@@ -1568,6 +1570,8 @@ int qemu_loadvm_state(QEMUFile *f)
         }
     }
 
+    cpu_synchronize_all_post_init();
+
     ret = 0;
 
 out:
diff --git a/sysemu.h b/sysemu.h
index ff97786..f92b94a 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -59,6 +59,10 @@ int load_vmstate(Monitor *mon, const char *name);
 void do_delvm(Monitor *mon, const QDict *qdict);
 void do_info_snapshots(Monitor *mon);
 
+void cpu_synchronize_all_states(void);
+void cpu_synchronize_all_post_reset(void);
+void cpu_synchronize_all_post_init(void);
+
 void qemu_announce_self(void);
 
 void main_loop_wait(int timeout);
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index e1ae15d..4a0c8bb 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -857,7 +857,7 @@ int kvm_get_vcpu_events(CPUState *env)
 }
 
 #ifdef KVM_UPSTREAM
-int kvm_arch_put_registers(CPUState *env)
+int kvm_arch_put_registers(CPUState *env, int level)
 {
     int ret;
 
diff --git a/target-i386/machine.c b/target-i386/machine.c
index 0b8a33a..bebde82 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -322,7 +322,6 @@ static void cpu_pre_save(void *opaque)
     CPUState *env = opaque;
     int i;
 
-    cpu_synchronize_state(env);
     if (kvm_enabled()) {
         kvm_save_mpstate(env);
         kvm_get_vcpu_events(env);
@@ -342,14 +341,6 @@ static void cpu_pre_save(void *opaque)
 #endif
 }
 
-static int cpu_pre_load(void *opaque)
-{
-    CPUState *env = opaque;
-
-    cpu_synchronize_state(env);
-    return 0;
-}
-
 static int cpu_post_load(void *opaque, int version_id)
 {
     CPUState *env = opaque;
@@ -389,7 +380,6 @@ static const VMStateDescription vmstate_cpu = {
     .minimum_version_id = 3,
     .minimum_version_id_old = 3,
     .pre_save = cpu_pre_save,
-    .pre_load = cpu_pre_load,
     .post_load = cpu_post_load,
     .fields      = (VMStateField []) {
         VMSTATE_UINTTL_ARRAY(regs, CPUState, CPU_NB_REGS),
diff --git a/target-ia64/machine.c b/target-ia64/machine.c
index 7d29575..fdbeeef 100644
--- a/target-ia64/machine.c
+++ b/target-ia64/machine.c
@@ -9,7 +9,6 @@ void cpu_save(QEMUFile *f, void *opaque)
     CPUState *env = opaque;
 
     if (kvm_enabled()) {
-        kvm_arch_save_regs(env);
         kvm_arch_save_mpstate(env);
     }
 }
@@ -19,7 +18,6 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
     CPUState *env = opaque;
 
     if (kvm_enabled()) {
-        kvm_arch_load_regs(env);
         kvm_arch_load_mpstate(env);
     }
     return 0;
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 0424a78..debe441 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -57,7 +57,7 @@ void kvm_arch_reset_vcpu(CPUState *env)
 {
 }
 
-int kvm_arch_put_registers(CPUState *env)
+int kvm_arch_put_registers(CPUState *env, int level)
 {
     struct kvm_regs regs;
     int ret;
diff --git a/target-ppc/machine.c b/target-ppc/machine.c
index ead38e1..81d3f72 100644
--- a/target-ppc/machine.c
+++ b/target-ppc/machine.c
@@ -8,8 +8,6 @@ void cpu_save(QEMUFile *f, void *opaque)
     CPUState *env = (CPUState *)opaque;
     unsigned int i, j;
 
-    cpu_synchronize_state(env);
-
     for (i = 0; i < 32; i++)
         qemu_put_betls(f, &env->gpr[i]);
 #if !defined(TARGET_PPC64)
@@ -97,8 +95,6 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
     CPUState *env = (CPUState *)opaque;
     unsigned int i, j;
 
-    cpu_synchronize_state(env);
-
     for (i = 0; i < 32; i++)
         qemu_get_betls(f, &env->gpr[i]);
 #if !defined(TARGET_PPC64)
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 0992563..59b8a17 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -91,7 +91,7 @@ void kvm_arch_reset_vcpu(CPUState *env)
     /* FIXME: add code to reset vcpu. */
 }
 
-int kvm_arch_put_registers(CPUState *env)
+int kvm_arch_put_registers(CPUState *env, int level)
 {
     struct kvm_regs regs;
     int ret;
@@ -296,7 +296,6 @@ static int handle_hypercall(CPUState *env, struct kvm_run *run)
 
     cpu_synchronize_state(env);
     r = s390_virtio_hypercall(env);
-    kvm_arch_put_registers(env);
 
     return r;
 }
diff --git a/vl.c b/vl.c
index 25af448..74f2a6f 100644
--- a/vl.c
+++ b/vl.c
@@ -3047,6 +3047,33 @@ static void nographic_update(void *opaque)
     qemu_mod_timer(nographic_timer, interval + qemu_get_clock(rt_clock));
 }
 
+void cpu_synchronize_all_states(void)
+{
+    CPUState *cpu;
+
+    for (cpu = first_cpu; cpu; cpu = cpu->next_cpu) {
+        cpu_synchronize_state(cpu);
+    }
+}
+
+void cpu_synchronize_all_post_reset(void)
+{
+    CPUState *cpu;
+
+    for (cpu = first_cpu; cpu; cpu = cpu->next_cpu) {
+        cpu_synchronize_post_reset(cpu);
+    }
+}
+
+void cpu_synchronize_all_post_init(void)
+{
+    CPUState *cpu;
+
+    for (cpu = first_cpu; cpu; cpu = cpu->next_cpu) {
+        cpu_synchronize_post_init(cpu);
+    }
+}
+
 struct vm_change_state_entry {
     VMChangeStateHandler *cb;
     void *opaque;
@@ -3195,6 +3222,7 @@ void qemu_system_reset(void)
     QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
         re->func(re->opaque);
     }
+    cpu_synchronize_all_post_reset();
 }
 
 void qemu_system_reset_request(void)
@@ -5956,6 +5984,7 @@ int main(int argc, char **argv, char **envp)
     machine->init(ram_size, boot_devices,
                   kernel_filename, kernel_cmdline, initrd_filename, cpu_model);
 
+    cpu_synchronize_all_post_init();
 
 #ifndef _WIN32
     /* must be after terminal init, SDL library changes signal handlers */
-- 
1.6.0.2

[Qemu-devel] [PATCH 15/21] qemu-kvm: Clean up mpstate synchronization

[Jan Kiszka]

Push mpstate reading/writing into kvm_arch_load/save_regs and, on x86,
properly synchronize with halted in the accessor functions.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/apic.c             |    7 ----
 qemu-kvm-ia64.c       |    4 ++-
 qemu-kvm-x86.c        |   88 +++++++++++++++++++++++++++---------------------
 qemu-kvm.c            |   30 -----------------
 qemu-kvm.h            |   15 --------
 target-i386/machine.c |    6 ---
 target-ia64/machine.c |    3 ++
 7 files changed, 55 insertions(+), 98 deletions(-)

diff --git a/hw/apic.c b/hw/apic.c
index 3e03e10..092c61e 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -507,13 +507,6 @@ void apic_init_reset(CPUState *env)
     s->wait_for_sipi = 1;
 
     env->halted = !(s->apicbase & MSR_IA32_APICBASE_BSP);
-#ifdef KVM_CAP_MP_STATE
-    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
-        env->mp_state
-            = env->halted ? KVM_MP_STATE_UNINITIALIZED : KVM_MP_STATE_RUNNABLE;
-        kvm_load_mpstate(env);
-    }
-#endif
 }
 
 static void apic_startup(APICState *s, int vector_num)
diff --git a/qemu-kvm-ia64.c b/qemu-kvm-ia64.c
index fc8110e..39bcbeb 100644
--- a/qemu-kvm-ia64.c
+++ b/qemu-kvm-ia64.c
@@ -124,7 +124,9 @@ void kvm_arch_cpu_reset(CPUState *env)
 {
     if (kvm_irqchip_in_kernel(kvm_context)) {
 #ifdef KVM_CAP_MP_STATE
-	kvm_reset_mpstate(env->kvm_cpu_state.vcpu_ctx);
+        struct kvm_mp_state mp_state = {.mp_state = KVM_MP_STATE_UNINITIALIZED
+        };
+        kvm_set_mpstate(env, &mp_state);
 #endif
     } else {
 	env->interrupt_request &= ~CPU_INTERRUPT_HARD;
diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index 63cd095..6b5895f 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -754,6 +754,48 @@ static int get_msr_entry(struct kvm_msr_entry *entry, CPUState *env)
         return 0;
 }
 
+static void kvm_arch_save_mpstate(CPUState *env)
+{
+#ifdef KVM_CAP_MP_STATE
+    int r;
+    struct kvm_mp_state mp_state;
+
+    r = kvm_get_mpstate(env, &mp_state);
+    if (r < 0) {
+        env->mp_state = -1;
+    } else {
+        env->mp_state = mp_state.mp_state;
+        if (kvm_irqchip_in_kernel()) {
+            env->halted = (env->mp_state == KVM_MP_STATE_HALTED);
+        }
+    }
+#else
+    env->mp_state = -1;
+#endif
+}
+
+static void kvm_arch_load_mpstate(CPUState *env)
+{
+#ifdef KVM_CAP_MP_STATE
+    struct kvm_mp_state mp_state;
+
+    /*
+     * -1 indicates that the host did not support GET_MP_STATE ioctl,
+     *  so don't touch it.
+     */
+    if (env->mp_state != -1) {
+        if (kvm_irqchip_in_kernel()) {
+            env->mp_state = env->halted ? KVM_MP_STATE_UNINITIALIZED :
+                                          KVM_MP_STATE_RUNNABLE;
+            /* Avoid deadlock: no user space IRQ will ever clear it. */
+            env->halted = 0;
+        }
+        mp_state.mp_state = env->mp_state;
+        kvm_set_mpstate(env, &mp_state);
+    }
+#endif
+}
+
 static void set_v8086_seg(struct kvm_segment *lhs, const SegmentCache *rhs)
 {
     lhs->selector = rhs->selector;
@@ -926,6 +968,10 @@ void kvm_arch_load_regs(CPUState *env, int level)
     rc = kvm_set_msrs(env, msrs, n);
     if (rc == -1)
         perror("kvm_set_msrs FAILED");
+
+    if (level >= KVM_PUT_RESET_STATE) {
+        kvm_arch_load_mpstate(env);
+    }
 }
 
 void kvm_load_tsc(CPUState *env)
@@ -940,36 +986,6 @@ void kvm_load_tsc(CPUState *env)
         perror("kvm_set_tsc FAILED.\n");
 }
 
-void kvm_arch_save_mpstate(CPUState *env)
-{
-#ifdef KVM_CAP_MP_STATE
-    int r;
-    struct kvm_mp_state mp_state;
-
-    r = kvm_get_mpstate(env, &mp_state);
-    if (r < 0)
-        env->mp_state = -1;
-    else
-        env->mp_state = mp_state.mp_state;
-#else
-    env->mp_state = -1;
-#endif
-}
-
-void kvm_arch_load_mpstate(CPUState *env)
-{
-#ifdef KVM_CAP_MP_STATE
-    struct kvm_mp_state mp_state = { .mp_state = env->mp_state };
-
-    /*
-     * -1 indicates that the host did not support GET_MP_STATE ioctl,
-     *  so don't touch it.
-     */
-    if (env->mp_state != -1)
-        kvm_set_mpstate(env, &mp_state);
-#endif
-}
-
 void kvm_arch_save_regs(CPUState *env)
 {
     struct kvm_regs regs;
@@ -1366,15 +1382,9 @@ void kvm_arch_cpu_reset(CPUState *env)
 {
     kvm_arch_reset_vcpu(env);
     kvm_put_vcpu_events(env);
-    if (!cpu_is_bsp(env)) {
-	if (kvm_irqchip_in_kernel()) {
-#ifdef KVM_CAP_MP_STATE
-	    kvm_reset_mpstate(env);
-#endif
-	} else {
-	    env->interrupt_request &= ~CPU_INTERRUPT_HARD;
-	    env->halted = 1;
-	}
+    if (!cpu_is_bsp(env) && !kvm_irqchip_in_kernel()) {
+        env->interrupt_request &= ~CPU_INTERRUPT_HARD;
+        env->halted = 1;
     }
 }
 
diff --git a/qemu-kvm.c b/qemu-kvm.c
index 53030f1..efa6a29 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -1579,36 +1579,6 @@ void kvm_update_interrupt_request(CPUState *env)
     }
 }
 
-static void kvm_do_load_mpstate(void *_env)
-{
-    CPUState *env = _env;
-
-    kvm_arch_load_mpstate(env);
-}
-
-void kvm_load_mpstate(CPUState *env)
-{
-    if (kvm_enabled() && qemu_system_ready && kvm_vcpu_inited(env))
-        on_vcpu(env, kvm_do_load_mpstate, env);
-}
-
-static void kvm_do_save_mpstate(void *_env)
-{
-    CPUState *env = _env;
-
-    kvm_arch_save_mpstate(env);
-#ifdef KVM_CAP_MP_STATE
-    if (kvm_irqchip_in_kernel())
-        env->halted = (env->mp_state == KVM_MP_STATE_HALTED);
-#endif
-}
-
-void kvm_save_mpstate(CPUState *env)
-{
-    if (kvm_enabled())
-        on_vcpu(env, kvm_do_save_mpstate, env);
-}
-
 int kvm_cpu_exec(CPUState *env)
 {
     int r;
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 6d785a0..aa7bcce 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -299,16 +299,6 @@ int kvm_get_mpstate(CPUState *env, struct kvm_mp_state *mp_state);
  *
  */
 int kvm_set_mpstate(CPUState *env, struct kvm_mp_state *mp_state);
-/*!
- *  * \brief Reset VCPU MP state
- *
- */
-static inline int kvm_reset_mpstate(CPUState *env)
-{
-    struct kvm_mp_state mp_state = {.mp_state = KVM_MP_STATE_UNINITIALIZED
-    };
-    return kvm_set_mpstate(env, &mp_state);
-}
 #endif
 
 /*!
@@ -874,8 +864,6 @@ static inline void kvm_inject_x86_mce(CPUState *cenv, int bank,
 int kvm_main_loop(void);
 int kvm_init_ap(void);
 int kvm_vcpu_inited(CPUState *env);
-void kvm_load_mpstate(CPUState *env);
-void kvm_save_mpstate(CPUState *env);
 void kvm_apic_init(CPUState *env);
 /* called from vcpu initialization */
 void qemu_kvm_load_lapic(CPUState *env);
@@ -909,8 +897,6 @@ int kvm_arch_qemu_create_context(void);
 
 void kvm_arch_save_regs(CPUState *env);
 void kvm_arch_load_regs(CPUState *env, int level);
-void kvm_arch_load_mpstate(CPUState *env);
-void kvm_arch_save_mpstate(CPUState *env);
 int kvm_arch_has_work(CPUState *env);
 void kvm_arch_process_irqchip_events(CPUState *env);
 int kvm_arch_try_push_interrupts(void *opaque);
@@ -979,7 +965,6 @@ void kvm_load_tsc(CPUState *env);
 #ifdef TARGET_I386
 #define qemu_kvm_has_pit_state2() (0)
 #endif
-#define kvm_save_mpstate(env)   do {} while(0)
 #define qemu_kvm_cpu_stop(env) do {} while(0)
 static inline void kvm_load_tsc(CPUState *env)
 {
diff --git a/target-i386/machine.c b/target-i386/machine.c
index bebde82..61e6a87 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -323,7 +323,6 @@ static void cpu_pre_save(void *opaque)
     int i;
 
     if (kvm_enabled()) {
-        kvm_save_mpstate(env);
         kvm_get_vcpu_events(env);
     }
 
@@ -362,12 +361,7 @@ static int cpu_post_load(void *opaque, int version_id)
     tlb_flush(env, 1);
 
     if (kvm_enabled()) {
-        /* when in-kernel irqchip is used, env->halted causes deadlock
-           because no userspace IRQs will ever clear this flag */
-        env->halted = 0;
-
         kvm_load_tsc(env);
-        kvm_load_mpstate(env);
         kvm_put_vcpu_events(env);
     }
 
diff --git a/target-ia64/machine.c b/target-ia64/machine.c
index fdbeeef..8cf5bdd 100644
--- a/target-ia64/machine.c
+++ b/target-ia64/machine.c
@@ -4,6 +4,9 @@
 #include "exec-all.h"
 #include "qemu-kvm.h"
 
+void kvm_arch_save_mpstate(CPUState *env);
+void kvm_arch_load_mpstate(CPUState *env);
+
 void cpu_save(QEMUFile *f, void *opaque)
 {
     CPUState *env = opaque;
-- 
1.6.0.2

[Qemu-devel] [PATCH 16/21] KVM: x86: Restrict writeback of VCPU state

[Jan Kiszka]

Do not write nmi_pending, sipi_vector, and mpstate unless we at least go
through a reset. And TSC as well as KVM wallclocks should only be
written on full sync, otherwise we risk to drop some time on during
state read-modify-write.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kvm.h                 |    2 +-
 qemu-kvm-x86.c        |    2 +-
 target-i386/kvm.c     |   32 ++++++++++++++++++++------------
 target-i386/machine.c |    2 +-
 4 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/kvm.h b/kvm.h
index ee8b3f6..e4005d8 100644
--- a/kvm.h
+++ b/kvm.h
@@ -53,7 +53,7 @@ int kvm_set_migration_log(int enable);
 
 int kvm_has_sync_mmu(void);
 int kvm_has_vcpu_events(void);
-int kvm_put_vcpu_events(CPUState *env);
+int kvm_put_vcpu_events(CPUState *env, int level);
 int kvm_get_vcpu_events(CPUState *env);
 
 void kvm_setup_guest_memory(void *start, size_t size);
diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index 6b5895f..21476db 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -1381,7 +1381,7 @@ void kvm_arch_push_nmi(void *opaque)
 void kvm_arch_cpu_reset(CPUState *env)
 {
     kvm_arch_reset_vcpu(env);
-    kvm_put_vcpu_events(env);
+    kvm_put_vcpu_events(env, KVM_PUT_RESET_STATE);
     if (!cpu_is_bsp(env) && !kvm_irqchip_in_kernel()) {
         env->interrupt_request &= ~CPU_INTERRUPT_HARD;
         env->halted = 1;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 4a0c8bb..fefd5a5 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -544,7 +544,7 @@ static void kvm_msr_entry_set(struct kvm_msr_entry *entry,
     entry->data = value;
 }
 
-static int kvm_put_msrs(CPUState *env)
+static int kvm_put_msrs(CPUState *env, int level)
 {
     struct {
         struct kvm_msrs info;
@@ -558,7 +558,6 @@ static int kvm_put_msrs(CPUState *env)
     kvm_msr_entry_set(&msrs[n++], MSR_IA32_SYSENTER_EIP, env->sysenter_eip);
     if (kvm_has_msr_star(env))
 	kvm_msr_entry_set(&msrs[n++], MSR_STAR, env->star);
-    kvm_msr_entry_set(&msrs[n++], MSR_IA32_TSC, env->tsc);
     kvm_msr_entry_set(&msrs[n++], MSR_VM_HSAVE_PA, env->vm_hsave);
 #ifdef TARGET_X86_64
     /* FIXME if lm capable */
@@ -567,8 +566,12 @@ static int kvm_put_msrs(CPUState *env)
     kvm_msr_entry_set(&msrs[n++], MSR_FMASK, env->fmask);
     kvm_msr_entry_set(&msrs[n++], MSR_LSTAR, env->lstar);
 #endif
-    kvm_msr_entry_set(&msrs[n++], MSR_KVM_SYSTEM_TIME,  env->system_time_msr);
-    kvm_msr_entry_set(&msrs[n++], MSR_KVM_WALL_CLOCK,  env->wall_clock_msr);
+    if (level == KVM_PUT_FULL_STATE) {
+        kvm_msr_entry_set(&msrs[n++], MSR_IA32_TSC, env->tsc);
+        kvm_msr_entry_set(&msrs[n++], MSR_KVM_SYSTEM_TIME,
+                          env->system_time_msr);
+        kvm_msr_entry_set(&msrs[n++], MSR_KVM_WALL_CLOCK, env->wall_clock_msr);
+    }
 
     msr_data.info.nmsrs = n;
 
@@ -786,7 +789,7 @@ static int kvm_get_mp_state(CPUState *env)
 }
 #endif
 
-int kvm_put_vcpu_events(CPUState *env)
+int kvm_put_vcpu_events(CPUState *env, int level)
 {
 #ifdef KVM_CAP_VCPU_EVENTS
     struct kvm_vcpu_events events;
@@ -810,8 +813,11 @@ int kvm_put_vcpu_events(CPUState *env)
 
     events.sipi_vector = env->sipi_vector;
 
-    events.flags =
-        KVM_VCPUEVENT_VALID_NMI_PENDING | KVM_VCPUEVENT_VALID_SIPI_VECTOR;
+    events.flags = 0;
+    if (level >= KVM_PUT_RESET_STATE) {
+        events.flags |=
+            KVM_VCPUEVENT_VALID_NMI_PENDING | KVM_VCPUEVENT_VALID_SIPI_VECTOR;
+    }
 
     return kvm_vcpu_ioctl(env, KVM_SET_VCPU_EVENTS, &events);
 #else
@@ -882,15 +888,17 @@ int kvm_arch_put_registers(CPUState *env, int level)
     if (ret < 0)
         return ret;
 
-    ret = kvm_put_msrs(env);
+    ret = kvm_put_msrs(env, level);
     if (ret < 0)
         return ret;
 
-    ret = kvm_put_mp_state(env);
-    if (ret < 0)
-        return ret;
+    if (level >= KVM_PUT_RESET_STATE) {
+        ret = kvm_put_mp_state(env);
+        if (ret < 0)
+            return ret;
+    }
 
-    ret = kvm_put_vcpu_events(env);
+    ret = kvm_put_vcpu_events(env, level);
     if (ret < 0)
         return ret;
 
diff --git a/target-i386/machine.c b/target-i386/machine.c
index 61e6a87..6fca559 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -362,7 +362,7 @@ static int cpu_post_load(void *opaque, int version_id)
 
     if (kvm_enabled()) {
         kvm_load_tsc(env);
-        kvm_put_vcpu_events(env);
+        kvm_put_vcpu_events(env, KVM_PUT_FULL_STATE);
     }
 
     return 0;
-- 
1.6.0.2

[Qemu-devel] [PATCH 17/21] qemu-kvm: Use VCPU event state for reset and vmsave/load

[Jan Kiszka]

Push reading/writing of vcpu_events into kvm_arch_load/save_regs to
avoid KVM-specific hooks in generic code.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kvm.h                 |    2 --
 qemu-kvm-x86.c        |    6 ++++--
 target-i386/kvm.c     |    4 ++--
 target-i386/machine.c |    6 ------
 4 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/kvm.h b/kvm.h
index e4005d8..686ee33 100644
--- a/kvm.h
+++ b/kvm.h
@@ -53,8 +53,6 @@ int kvm_set_migration_log(int enable);
 
 int kvm_has_sync_mmu(void);
 int kvm_has_vcpu_events(void);
-int kvm_put_vcpu_events(CPUState *env, int level);
-int kvm_get_vcpu_events(CPUState *env);
 
 void kvm_setup_guest_memory(void *start, size_t size);
 
diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index 21476db..f484149 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -972,6 +972,8 @@ void kvm_arch_load_regs(CPUState *env, int level)
     if (level >= KVM_PUT_RESET_STATE) {
         kvm_arch_load_mpstate(env);
     }
+
+    kvm_put_vcpu_events(env, level);
 }
 
 void kvm_load_tsc(CPUState *env)
@@ -1141,6 +1143,7 @@ void kvm_arch_save_regs(CPUState *env)
         }
     }
     kvm_arch_save_mpstate(env);
+    kvm_get_vcpu_events(env);
 }
 
 static void do_cpuid_ent(struct kvm_cpuid_entry2 *e, uint32_t function,
@@ -1215,7 +1218,7 @@ int kvm_arch_init_vcpu(CPUState *cenv)
 
     qemu_kvm_load_lapic(cenv);
 
-    cenv->interrupt_injected = -1;
+    kvm_arch_reset_vcpu(cenv);
 
 #ifdef KVM_CPUID_SIGNATURE
     /* Paravirtualization CPUIDs */
@@ -1381,7 +1384,6 @@ void kvm_arch_push_nmi(void *opaque)
 void kvm_arch_cpu_reset(CPUState *env)
 {
     kvm_arch_reset_vcpu(env);
-    kvm_put_vcpu_events(env, KVM_PUT_RESET_STATE);
     if (!cpu_is_bsp(env) && !kvm_irqchip_in_kernel()) {
         env->interrupt_request &= ~CPU_INTERRUPT_HARD;
         env->halted = 1;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index fefd5a5..9bd2952 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -789,7 +789,7 @@ static int kvm_get_mp_state(CPUState *env)
 }
 #endif
 
-int kvm_put_vcpu_events(CPUState *env, int level)
+static int kvm_put_vcpu_events(CPUState *env, int level)
 {
 #ifdef KVM_CAP_VCPU_EVENTS
     struct kvm_vcpu_events events;
@@ -825,7 +825,7 @@ int kvm_put_vcpu_events(CPUState *env, int level)
 #endif
 }
 
-int kvm_get_vcpu_events(CPUState *env)
+static int kvm_get_vcpu_events(CPUState *env)
 {
 #ifdef KVM_CAP_VCPU_EVENTS
     struct kvm_vcpu_events events;
diff --git a/target-i386/machine.c b/target-i386/machine.c
index 6fca559..bcc315b 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -5,7 +5,6 @@
 
 #include "exec-all.h"
 #include "kvm.h"
-#include "qemu-kvm.h"
 
 static const VMStateDescription vmstate_segment = {
     .name = "segment",
@@ -322,10 +321,6 @@ static void cpu_pre_save(void *opaque)
     CPUState *env = opaque;
     int i;
 
-    if (kvm_enabled()) {
-        kvm_get_vcpu_events(env);
-    }
-
     /* FPU */
     env->fpus_vmstate = (env->fpus & ~0x3800) | (env->fpstt & 0x7) << 11;
     env->fptag_vmstate = 0;
@@ -362,7 +357,6 @@ static int cpu_post_load(void *opaque, int version_id)
 
     if (kvm_enabled()) {
         kvm_load_tsc(env);
-        kvm_put_vcpu_events(env, KVM_PUT_FULL_STATE);
     }
 
     return 0;
-- 
1.6.0.2

[Qemu-devel] [PATCH 18/21] qemu-kvm: Cleanup/fix TSC and PV clock writeback

[Jan Kiszka]

Drop kvm_load_tsc in favor of level-dependent writeback in
kvm_arch_load_regs. KVM's PV clock MSRs fall in the same category and
should therefore only be written back on full sync.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 qemu-kvm-x86.c        |   19 +++++--------------
 qemu-kvm.h            |    4 ----
 target-i386/machine.c |    5 -----
 3 files changed, 5 insertions(+), 23 deletions(-)

diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index f484149..4b78570 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -962,8 +962,11 @@ void kvm_arch_load_regs(CPUState *env, int level)
         set_msr_entry(&msrs[n++], MSR_LSTAR  ,           env->lstar);
     }
 #endif
-    set_msr_entry(&msrs[n++], MSR_KVM_SYSTEM_TIME,  env->system_time_msr);
-    set_msr_entry(&msrs[n++], MSR_KVM_WALL_CLOCK,  env->wall_clock_msr);
+    if (level == KVM_PUT_FULL_STATE) {
+        set_msr_entry(&msrs[n++], MSR_IA32_TSC, env->tsc);
+        set_msr_entry(&msrs[n++], MSR_KVM_SYSTEM_TIME, env->system_time_msr);
+        set_msr_entry(&msrs[n++], MSR_KVM_WALL_CLOCK, env->wall_clock_msr);
+    }
 
     rc = kvm_set_msrs(env, msrs, n);
     if (rc == -1)
@@ -976,18 +979,6 @@ void kvm_arch_load_regs(CPUState *env, int level)
     kvm_put_vcpu_events(env, level);
 }
 
-void kvm_load_tsc(CPUState *env)
-{
-    int rc;
-    struct kvm_msr_entry msr;
-
-    set_msr_entry(&msr, MSR_IA32_TSC, env->tsc);
-
-    rc = kvm_set_msrs(env, &msr, 1);
-    if (rc == -1)
-        perror("kvm_set_tsc FAILED.\n");
-}
-
 void kvm_arch_save_regs(CPUState *env)
 {
     struct kvm_regs regs;
diff --git a/qemu-kvm.h b/qemu-kvm.h
index aa7bcce..2af206c 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -958,7 +958,6 @@ int handle_tpr_access(void *opaque, CPUState *env, uint64_t rip,
 #ifdef TARGET_I386
 #define qemu_kvm_has_pit_state2() kvm_has_pit_state2(kvm_context)
 #endif
-void kvm_load_tsc(CPUState *env);
 #else
 #define kvm_nested 0
 #define qemu_kvm_has_gsi_routing() (0)
@@ -966,9 +965,6 @@ void kvm_load_tsc(CPUState *env);
 #define qemu_kvm_has_pit_state2() (0)
 #endif
 #define qemu_kvm_cpu_stop(env) do {} while(0)
-static inline void kvm_load_tsc(CPUState *env)
-{
-}
 #endif
 
 void kvm_mutex_unlock(void);
diff --git a/target-i386/machine.c b/target-i386/machine.c
index bcc315b..b547e2a 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -354,11 +354,6 @@ static int cpu_post_load(void *opaque, int version_id)
         hw_breakpoint_insert(env, i);
 
     tlb_flush(env, 1);
-
-    if (kvm_enabled()) {
-        kvm_load_tsc(env);
-    }
-
     return 0;
 }
 
-- 
1.6.0.2

[Qemu-devel] [PATCH 19/21] qemu-kvm: Clean up KVM's APIC hooks

[Jan Kiszka]

The APIC is part of the VCPU state, so trigger its readout and writeback
from kvm_arch_save/load_regs. Thanks to the transparent sync on reset
and vmsave/load, we can also drop explicit sync code, reducing the diff
to upstream.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/apic.c      |   37 +++++--------------------------------
 qemu-kvm-x86.c |    4 ++--
 qemu-kvm.h     |    5 ++---
 3 files changed, 9 insertions(+), 37 deletions(-)

diff --git a/hw/apic.c b/hw/apic.c
index 092c61e..d8c4f7c 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -24,8 +24,6 @@
 #include "host-utils.h"
 #include "kvm.h"
 
-#include "qemu-kvm.h"
-
 //#define DEBUG_APIC
 
 /* APIC Local Vector Table */
@@ -951,36 +949,22 @@ static void kvm_kernel_lapic_load_from_user(APICState *s)
 
 #endif
 
-void qemu_kvm_load_lapic(CPUState *env)
+void kvm_load_lapic(CPUState *env)
 {
 #ifdef KVM_CAP_IRQCHIP
-    if (kvm_enabled() && kvm_vcpu_inited(env) && kvm_irqchip_in_kernel()) {
-        kvm_kernel_lapic_load_from_user(env->apic_state);
-    }
-#endif
-}
-
-static void apic_pre_save(void *opaque)
-{
-#ifdef KVM_CAP_IRQCHIP
-    APICState *s = (void *)opaque;
-
     if (kvm_enabled() && kvm_irqchip_in_kernel()) {
-        kvm_kernel_lapic_save_to_user(s);
+        kvm_kernel_lapic_load_from_user(env->apic_state);
     }
 #endif
 }
 
-static int apic_post_load(void *opaque, int version_id)
+void kvm_save_lapic(CPUState *env)
 {
 #ifdef KVM_CAP_IRQCHIP
-    APICState *s = opaque;
-
     if (kvm_enabled() && kvm_irqchip_in_kernel()) {
-        kvm_kernel_lapic_load_from_user(s);
+        kvm_kernel_lapic_save_to_user(env->apic_state);
     }
 #endif
-    return 0;
 }
 
 /* This function is only used for old state version 1 and 2 */
@@ -1019,9 +1003,6 @@ static int apic_load_old(QEMUFile *f, void *opaque, int version_id)
 
     if (version_id >= 2)
         qemu_get_timer(f, s->timer);
-
-    qemu_kvm_load_lapic(s->cpu_env);
-
     return 0;
 }
 
@@ -1052,9 +1033,7 @@ static const VMStateDescription vmstate_apic = {
         VMSTATE_INT64(next_time, APICState),
         VMSTATE_TIMER(timer, APICState),
         VMSTATE_END_OF_LIST()
-    },
-    .pre_save = apic_pre_save,
-    .post_load = apic_post_load,
+    }
 };
 
 static void apic_reset(void *opaque)
@@ -1077,7 +1056,6 @@ static void apic_reset(void *opaque)
          */
         s->lvt[APIC_LVT_LINT0] = 0x700;
     }
-    qemu_kvm_load_lapic(s->cpu_env);
 }
 
 static CPUReadMemoryFunc * const apic_mem_read[3] = {
@@ -1121,11 +1099,6 @@ int apic_init(CPUState *env)
     vmstate_register(s->idx, &vmstate_apic, s);
     qemu_register_reset(apic_reset, s);
 
-    /* apic_reset must be called before the vcpu threads are initialized and load
-     * registers, in qemu-kvm.
-     */
-    apic_reset(s);
-
     local_apics[s->idx] = s;
     return 0;
 }
diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index 4b78570..9de018e 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -974,6 +974,7 @@ void kvm_arch_load_regs(CPUState *env, int level)
 
     if (level >= KVM_PUT_RESET_STATE) {
         kvm_arch_load_mpstate(env);
+        kvm_load_lapic(env);
     }
 
     kvm_put_vcpu_events(env, level);
@@ -1134,6 +1135,7 @@ void kvm_arch_save_regs(CPUState *env)
         }
     }
     kvm_arch_save_mpstate(env);
+    kvm_save_lapic(env);
     kvm_get_vcpu_events(env);
 }
 
@@ -1207,8 +1209,6 @@ int kvm_arch_init_vcpu(CPUState *cenv)
     CPUState copy;
     uint32_t i, j, limit;
 
-    qemu_kvm_load_lapic(cenv);
-
     kvm_arch_reset_vcpu(cenv);
 
 #ifdef KVM_CPUID_SIGNATURE
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 2af206c..fea23a4 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -864,9 +864,8 @@ static inline void kvm_inject_x86_mce(CPUState *cenv, int bank,
 int kvm_main_loop(void);
 int kvm_init_ap(void);
 int kvm_vcpu_inited(CPUState *env);
-void kvm_apic_init(CPUState *env);
-/* called from vcpu initialization */
-void qemu_kvm_load_lapic(CPUState *env);
+void kvm_save_lapic(CPUState *env);
+void kvm_load_lapic(CPUState *env);
 
 void kvm_hpet_enable_kpit(void);
 void kvm_hpet_disable_kpit(void);
-- 
1.6.0.2

[Qemu-devel] [PATCH 20/21] qemu-kvm: Move kvm_set_boot_cpu_id

[Jan Kiszka]

Setting the boot CPU ID is arch-specific KVM stuff. So push it where it
belongs to.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/pc.c        |    3 ---
 qemu-kvm-x86.c |    3 ++-
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 6c15a9f..3df6195 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -803,9 +803,6 @@ static void pc_init1(ram_addr_t ram_size,
 #endif
     }
 
-    if (kvm_enabled()) {
-        kvm_set_boot_cpu_id(0);
-    }
     for (i = 0; i < smp_cpus; i++) {
         env = pc_new_cpu(cpu_model);
     }
diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index 9de018e..0f34451 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -695,7 +695,8 @@ int kvm_arch_qemu_create_context(void)
     if (kvm_check_extension(kvm_state, KVM_CAP_ADJUST_CLOCK))
         vmstate_register(0, &vmstate_kvmclock, &kvmclock_data);
 #endif
-    return 0;
+
+    return kvm_set_boot_cpu_id(0);
 }
 
 static void set_msr_entry(struct kvm_msr_entry *entry, uint32_t index,
-- 
1.6.0.2

[Qemu-devel] [PATCH 21/21] qemu-kvm: Bring qemu_init_vcpu back home

[Jan Kiszka]

There is no need for the this hack anymore, initialization is now robust
against reordering as it doesn't try to write the VCPU state on its own.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/pc.c              |    5 -----
 target-i386/helper.c |    2 ++
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 3df6195..cd0746c 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -751,11 +751,6 @@ CPUState *pc_new_cpu(const char *cpu_model)
     } else {
         qemu_register_reset((QEMUResetHandler*)cpu_reset, env);
     }
-
-    /* kvm needs this to run after the apic is initialized. Otherwise,
-     * it can access invalid state and crash.
-     */
-    qemu_init_vcpu(env);
     return env;
 }
 
diff --git a/target-i386/helper.c b/target-i386/helper.c
index f9d63f6..f83e8cc 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1953,6 +1953,8 @@ CPUX86State *cpu_x86_init(const char *cpu_model)
     }
     mce_init(env);
 
+    qemu_init_vcpu(env);
+
     return env;
 }
 
-- 
1.6.0.2

[Qemu-devel] Re: [PATCH 00/21] qemu-kvm: Hook cleanups and extended use of upstream code

[Alexander Graf]

On 02.02.2010, at 09:18, Jan Kiszka wrote:

> Let's start with the overall stats:
> 
> 31 files changed, 274 insertions(+), 822 deletions(-)
> 
> So this series drops far more than 500 lines of redundant code, moving
> qemu-kvm yet a bit closer to upstream.
> 
> The other highlight is the simplification of synchronization between
> in-kernel and user space VCPU states. This area used to call a lot of
> problems in the past because it was tricky to get things right,
> specifically during the multi-threaded startup. The new approach pushes
> all the sync work around reset and vmsave/load into generic code, not
> only removing the burden from developers of, say, in-kernel APIC
> support, but also dropping most of our kvm-specific hooks, especially in
> the qemu-kvm tree.
> 
> While I tested this on various VMs around, and things look good so far,
> I wouldn't be surprised if there are some regressions remaining,
> specifically in the non-x86 parts that I wasn't able to test or even
> build. Please have a careful look!

The good news on that part is that apart from IA64, all other archs are broken in qemu-kvm anyways, but work on upstream qemu. So moving towards upstream definitely helps here.

Alex

[Qemu-devel] Re: [PATCH 03/21] qemu-kvm: Clean up register access API

[Gleb Natapov]

On Tue, Feb 02, 2010 at 09:18:49AM +0100, Jan Kiszka wrote:
> qemu-kvm's functios for accessing the VCPU registers are
> kvm_arch_load/save_regs. Use them directly instead of going through
> various wrappers. Specifically, we do not need on_vcpu wrapping as all
> users either already run in the related thread or call while the vm is
> stopped.
> 
Can we put check for that into those functions just to be sure.
Something like:
	assert(!vm_stopped && env->thread_id != pthread_id())

> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  qemu-kvm.c            |   37 +++----------------------------------
>  qemu-kvm.h            |   11 -----------
>  target-ia64/machine.c |    4 ++--
>  3 files changed, 5 insertions(+), 47 deletions(-)
> 
> diff --git a/qemu-kvm.c b/qemu-kvm.c
> index a305907..97c098c 100644
> --- a/qemu-kvm.c
> +++ b/qemu-kvm.c
> @@ -862,7 +862,7 @@ int pre_kvm_run(kvm_context_t kvm, CPUState *env)
>      kvm_arch_pre_run(env, env->kvm_run);
>  
>      if (env->kvm_cpu_state.regs_modified) {
> -        kvm_arch_put_registers(env);
> +        kvm_arch_load_regs(env);
>          env->kvm_cpu_state.regs_modified = 0;
>      }
>  
> @@ -1532,16 +1532,11 @@ static void on_vcpu(CPUState *env, void (*func)(void *data), void *data)
>          qemu_cond_wait(&qemu_work_cond);
>  }
>  
> -void kvm_arch_get_registers(CPUState *env)
> -{
> -	kvm_arch_save_regs(env);
> -}
> -
>  static void do_kvm_cpu_synchronize_state(void *_env)
>  {
>      CPUState *env = _env;
>      if (!env->kvm_cpu_state.regs_modified) {
> -        kvm_arch_get_registers(env);
> +        kvm_arch_save_regs(env);
>          env->kvm_cpu_state.regs_modified = 1;
>      }
>  }
> @@ -1584,32 +1579,6 @@ void kvm_update_interrupt_request(CPUState *env)
>      }
>  }
>  
> -static void kvm_do_load_registers(void *_env)
> -{
> -    CPUState *env = _env;
> -
> -    kvm_arch_load_regs(env);
> -}
> -
> -void kvm_load_registers(CPUState *env)
> -{
> -    if (kvm_enabled() && qemu_system_ready)
> -        on_vcpu(env, kvm_do_load_registers, env);
> -}
> -
> -static void kvm_do_save_registers(void *_env)
> -{
> -    CPUState *env = _env;
> -
> -    kvm_arch_save_regs(env);
> -}
> -
> -void kvm_save_registers(CPUState *env)
> -{
> -    if (kvm_enabled())
> -        on_vcpu(env, kvm_do_save_registers, env);
> -}
> -
>  static void kvm_do_load_mpstate(void *_env)
>  {
>      CPUState *env = _env;
> @@ -2379,7 +2348,7 @@ static void kvm_invoke_set_guest_debug(void *data)
>      struct kvm_set_guest_debug_data *dbg_data = data;
>  
>      if (cpu_single_env->kvm_cpu_state.regs_modified) {
> -        kvm_arch_put_registers(cpu_single_env);
> +        kvm_arch_save_regs(cpu_single_env);
>          cpu_single_env->kvm_cpu_state.regs_modified = 0;
>      }
>      dbg_data->err =
> diff --git a/qemu-kvm.h b/qemu-kvm.h
> index 6b3e5a1..1354227 100644
> --- a/qemu-kvm.h
> +++ b/qemu-kvm.h
> @@ -902,8 +902,6 @@ int kvm_main_loop(void);
>  int kvm_init_ap(void);
>  #ifndef QEMU_KVM_NO_CPU
>  int kvm_vcpu_inited(CPUState *env);
> -void kvm_load_registers(CPUState *env);
> -void kvm_save_registers(CPUState *env);
>  void kvm_load_mpstate(CPUState *env);
>  void kvm_save_mpstate(CPUState *env);
>  int kvm_cpu_exec(CPUState *env);
> @@ -1068,8 +1066,6 @@ void kvm_load_tsc(CPUState *env);
>  #ifdef TARGET_I386
>  #define qemu_kvm_has_pit_state2() (0)
>  #endif
> -#define kvm_load_registers(env) do {} while(0)
> -#define kvm_save_registers(env) do {} while(0)
>  #define kvm_save_mpstate(env)   do {} while(0)
>  #define qemu_kvm_cpu_stop(env) do {} while(0)
>  static inline void kvm_init_vcpu(CPUState *env)
> @@ -1098,13 +1094,6 @@ static inline int kvm_sync_vcpus(void)
>  }
>  
>  #ifndef QEMU_KVM_NO_CPU
> -void kvm_arch_get_registers(CPUState *env);
> -
> -static inline void kvm_arch_put_registers(CPUState *env)
> -{
> -    kvm_load_registers(env);
> -}
> -
>  void kvm_cpu_synchronize_state(CPUState *env);
>  
>  static inline void cpu_synchronize_state(CPUState *env)
> diff --git a/target-ia64/machine.c b/target-ia64/machine.c
> index 70ef379..7d29575 100644
> --- a/target-ia64/machine.c
> +++ b/target-ia64/machine.c
> @@ -9,7 +9,7 @@ void cpu_save(QEMUFile *f, void *opaque)
>      CPUState *env = opaque;
>  
>      if (kvm_enabled()) {
> -        kvm_save_registers(env);
> +        kvm_arch_save_regs(env);
>          kvm_arch_save_mpstate(env);
>      }
>  }
> @@ -19,7 +19,7 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
>      CPUState *env = opaque;
>  
>      if (kvm_enabled()) {
> -        kvm_load_registers(env);
> +        kvm_arch_load_regs(env);
>          kvm_arch_load_mpstate(env);
>      }
>      return 0;
> -- 
> 1.6.0.2
> 
> --
> 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

--
			Gleb.

[Qemu-devel] Re: [PATCH 03/21] qemu-kvm: Clean up register access API

[Jan Kiszka]

Gleb Natapov wrote:
> On Tue, Feb 02, 2010 at 09:18:49AM +0100, Jan Kiszka wrote:
>> qemu-kvm's functios for accessing the VCPU registers are
>> kvm_arch_load/save_regs. Use them directly instead of going through
>> various wrappers. Specifically, we do not need on_vcpu wrapping as all
>> users either already run in the related thread or call while the vm is
>> stopped.
>>
> Can we put check for that into those functions just to be sure.
> Something like:
> 	assert(!vm_stopped && env->thread_id != pthread_id())
> 

Good idea. Will add this to a potential v2 or send an add-on patch. We
just need something else than vm_stopped (for reset, only the vcpu
threads are stopped, not the vm), probably env->stopped in qemu-kvm.

>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  qemu-kvm.c            |   37 +++----------------------------------
>>  qemu-kvm.h            |   11 -----------
>>  target-ia64/machine.c |    4 ++--
>>  3 files changed, 5 insertions(+), 47 deletions(-)
>>
>> diff --git a/qemu-kvm.c b/qemu-kvm.c
>> index a305907..97c098c 100644
>> --- a/qemu-kvm.c
>> +++ b/qemu-kvm.c
>> @@ -862,7 +862,7 @@ int pre_kvm_run(kvm_context_t kvm, CPUState *env)
>>      kvm_arch_pre_run(env, env->kvm_run);
>>  
>>      if (env->kvm_cpu_state.regs_modified) {
>> -        kvm_arch_put_registers(env);
>> +        kvm_arch_load_regs(env);
>>          env->kvm_cpu_state.regs_modified = 0;
>>      }
>>  
>> @@ -1532,16 +1532,11 @@ static void on_vcpu(CPUState *env, void (*func)(void *data), void *data)
>>          qemu_cond_wait(&qemu_work_cond);
>>  }
>>  
>> -void kvm_arch_get_registers(CPUState *env)
>> -{
>> -	kvm_arch_save_regs(env);
>> -}
>> -
>>  static void do_kvm_cpu_synchronize_state(void *_env)
>>  {
>>      CPUState *env = _env;
>>      if (!env->kvm_cpu_state.regs_modified) {
>> -        kvm_arch_get_registers(env);
>> +        kvm_arch_save_regs(env);
>>          env->kvm_cpu_state.regs_modified = 1;
>>      }
>>  }
>> @@ -1584,32 +1579,6 @@ void kvm_update_interrupt_request(CPUState *env)
>>      }
>>  }
>>  
>> -static void kvm_do_load_registers(void *_env)
>> -{
>> -    CPUState *env = _env;
>> -
>> -    kvm_arch_load_regs(env);
>> -}
>> -
>> -void kvm_load_registers(CPUState *env)
>> -{
>> -    if (kvm_enabled() && qemu_system_ready)
>> -        on_vcpu(env, kvm_do_load_registers, env);
>> -}
>> -
>> -static void kvm_do_save_registers(void *_env)
>> -{
>> -    CPUState *env = _env;
>> -
>> -    kvm_arch_save_regs(env);
>> -}
>> -
>> -void kvm_save_registers(CPUState *env)
>> -{
>> -    if (kvm_enabled())
>> -        on_vcpu(env, kvm_do_save_registers, env);
>> -}
>> -
>>  static void kvm_do_load_mpstate(void *_env)
>>  {
>>      CPUState *env = _env;
>> @@ -2379,7 +2348,7 @@ static void kvm_invoke_set_guest_debug(void *data)
>>      struct kvm_set_guest_debug_data *dbg_data = data;
>>  
>>      if (cpu_single_env->kvm_cpu_state.regs_modified) {
>> -        kvm_arch_put_registers(cpu_single_env);
>> +        kvm_arch_save_regs(cpu_single_env);
>>          cpu_single_env->kvm_cpu_state.regs_modified = 0;
>>      }
>>      dbg_data->err =
>> diff --git a/qemu-kvm.h b/qemu-kvm.h
>> index 6b3e5a1..1354227 100644
>> --- a/qemu-kvm.h
>> +++ b/qemu-kvm.h
>> @@ -902,8 +902,6 @@ int kvm_main_loop(void);
>>  int kvm_init_ap(void);
>>  #ifndef QEMU_KVM_NO_CPU
>>  int kvm_vcpu_inited(CPUState *env);
>> -void kvm_load_registers(CPUState *env);
>> -void kvm_save_registers(CPUState *env);
>>  void kvm_load_mpstate(CPUState *env);
>>  void kvm_save_mpstate(CPUState *env);
>>  int kvm_cpu_exec(CPUState *env);
>> @@ -1068,8 +1066,6 @@ void kvm_load_tsc(CPUState *env);
>>  #ifdef TARGET_I386
>>  #define qemu_kvm_has_pit_state2() (0)
>>  #endif
>> -#define kvm_load_registers(env) do {} while(0)
>> -#define kvm_save_registers(env) do {} while(0)
>>  #define kvm_save_mpstate(env)   do {} while(0)
>>  #define qemu_kvm_cpu_stop(env) do {} while(0)
>>  static inline void kvm_init_vcpu(CPUState *env)
>> @@ -1098,13 +1094,6 @@ static inline int kvm_sync_vcpus(void)
>>  }
>>  
>>  #ifndef QEMU_KVM_NO_CPU
>> -void kvm_arch_get_registers(CPUState *env);
>> -
>> -static inline void kvm_arch_put_registers(CPUState *env)
>> -{
>> -    kvm_load_registers(env);
>> -}
>> -
>>  void kvm_cpu_synchronize_state(CPUState *env);
>>  
>>  static inline void cpu_synchronize_state(CPUState *env)
>> diff --git a/target-ia64/machine.c b/target-ia64/machine.c
>> index 70ef379..7d29575 100644
>> --- a/target-ia64/machine.c
>> +++ b/target-ia64/machine.c
>> @@ -9,7 +9,7 @@ void cpu_save(QEMUFile *f, void *opaque)
>>      CPUState *env = opaque;
>>  
>>      if (kvm_enabled()) {
>> -        kvm_save_registers(env);
>> +        kvm_arch_save_regs(env);
>>          kvm_arch_save_mpstate(env);
>>      }
>>  }
>> @@ -19,7 +19,7 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
>>      CPUState *env = opaque;
>>  
>>      if (kvm_enabled()) {
>> -        kvm_load_registers(env);
>> +        kvm_arch_load_regs(env);
>>          kvm_arch_load_mpstate(env);
>>      }
>>      return 0;
>> -- 
>> 1.6.0.2
>>
>> --
>> 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
> 
> --
> 			Gleb.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [PATCH 00/21] qemu-kvm: Hook cleanups and extended use of upstream code

[Jan Kiszka]

Alexander Graf wrote:
> On 02.02.2010, at 09:18, Jan Kiszka wrote:
> 
>> Let's start with the overall stats:
>>
>> 31 files changed, 274 insertions(+), 822 deletions(-)
>>
>> So this series drops far more than 500 lines of redundant code, moving
>> qemu-kvm yet a bit closer to upstream.
>>
>> The other highlight is the simplification of synchronization between
>> in-kernel and user space VCPU states. This area used to call a lot of
>> problems in the past because it was tricky to get things right,
>> specifically during the multi-threaded startup. The new approach pushes
>> all the sync work around reset and vmsave/load into generic code, not
>> only removing the burden from developers of, say, in-kernel APIC
>> support, but also dropping most of our kvm-specific hooks, especially in
>> the qemu-kvm tree.
>>
>> While I tested this on various VMs around, and things look good so far,
>> I wouldn't be surprised if there are some regressions remaining,
>> specifically in the non-x86 parts that I wasn't able to test or even
>> build. Please have a careful look!
> 
> The good news on that part is that apart from IA64, all other archs are broken in qemu-kvm anyways, but work on upstream qemu. So moving towards upstream definitely helps here.
> 

OK, then you probably want my corresponding uq/master series in order to
test. Will try to roll them out ASAP.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [PATCH 15/21] qemu-kvm: Clean up mpstate synchronization

[Gleb Natapov]

On Tue, Feb 02, 2010 at 09:19:01AM +0100, Jan Kiszka wrote:
> Push mpstate reading/writing into kvm_arch_load/save_regs and, on x86,
> properly synchronize with halted in the accessor functions.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  hw/apic.c             |    7 ----
>  qemu-kvm-ia64.c       |    4 ++-
>  qemu-kvm-x86.c        |   88 +++++++++++++++++++++++++++---------------------
>  qemu-kvm.c            |   30 -----------------
>  qemu-kvm.h            |   15 --------
>  target-i386/machine.c |    6 ---
>  target-ia64/machine.c |    3 ++
>  7 files changed, 55 insertions(+), 98 deletions(-)
> 
> diff --git a/hw/apic.c b/hw/apic.c
> index 3e03e10..092c61e 100644
> --- a/hw/apic.c
> +++ b/hw/apic.c
> @@ -507,13 +507,6 @@ void apic_init_reset(CPUState *env)
>      s->wait_for_sipi = 1;
>  
>      env->halted = !(s->apicbase & MSR_IA32_APICBASE_BSP);
> -#ifdef KVM_CAP_MP_STATE
> -    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
> -        env->mp_state
> -            = env->halted ? KVM_MP_STATE_UNINITIALIZED : KVM_MP_STATE_RUNNABLE;
> -        kvm_load_mpstate(env);
> -    }
> -#endif
>  }
>  
>  static void apic_startup(APICState *s, int vector_num)
> diff --git a/qemu-kvm-ia64.c b/qemu-kvm-ia64.c
> index fc8110e..39bcbeb 100644
> --- a/qemu-kvm-ia64.c
> +++ b/qemu-kvm-ia64.c
> @@ -124,7 +124,9 @@ void kvm_arch_cpu_reset(CPUState *env)
>  {
>      if (kvm_irqchip_in_kernel(kvm_context)) {
>  #ifdef KVM_CAP_MP_STATE
> -	kvm_reset_mpstate(env->kvm_cpu_state.vcpu_ctx);
> +        struct kvm_mp_state mp_state = {.mp_state = KVM_MP_STATE_UNINITIALIZED
> +        };
> +        kvm_set_mpstate(env, &mp_state);
>  #endif
>      } else {
>  	env->interrupt_request &= ~CPU_INTERRUPT_HARD;
> diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
> index 63cd095..6b5895f 100644
> --- a/qemu-kvm-x86.c
> +++ b/qemu-kvm-x86.c
> @@ -754,6 +754,48 @@ static int get_msr_entry(struct kvm_msr_entry *entry, CPUState *env)
>          return 0;
>  }
>  
> +static void kvm_arch_save_mpstate(CPUState *env)
> +{
> +#ifdef KVM_CAP_MP_STATE
> +    int r;
> +    struct kvm_mp_state mp_state;
> +
> +    r = kvm_get_mpstate(env, &mp_state);
> +    if (r < 0) {
> +        env->mp_state = -1;
> +    } else {
> +        env->mp_state = mp_state.mp_state;
> +        if (kvm_irqchip_in_kernel()) {
> +            env->halted = (env->mp_state == KVM_MP_STATE_HALTED);
> +        }
> +    }
> +#else
> +    env->mp_state = -1;
> +#endif
> +}
> +
> +static void kvm_arch_load_mpstate(CPUState *env)
> +{
> +#ifdef KVM_CAP_MP_STATE
> +    struct kvm_mp_state mp_state;
> +
> +    /*
> +     * -1 indicates that the host did not support GET_MP_STATE ioctl,
> +     *  so don't touch it.
> +     */
> +    if (env->mp_state != -1) {
> +        if (kvm_irqchip_in_kernel()) {
> +            env->mp_state = env->halted ? KVM_MP_STATE_UNINITIALIZED :
> +                                          KVM_MP_STATE_RUNNABLE;
When irqchip is in kernel env->halted doesn't contain any relevant
information, so this is incorrect. Actually env->halted is updated only
to show correct cpu state during "info cpus".

> +            /* Avoid deadlock: no user space IRQ will ever clear it. */
And this comment explains why looking at env->halt when irqchip is in
kernel is wrong :)

> +            env->halted = 0;
> +        }
> +        mp_state.mp_state = env->mp_state;
> +        kvm_set_mpstate(env, &mp_state);
> +    }
> +#endif
> +}
> +
>  static void set_v8086_seg(struct kvm_segment *lhs, const SegmentCache *rhs)
>  {
>      lhs->selector = rhs->selector;
> @@ -926,6 +968,10 @@ void kvm_arch_load_regs(CPUState *env, int level)
>      rc = kvm_set_msrs(env, msrs, n);
>      if (rc == -1)
>          perror("kvm_set_msrs FAILED");
> +
> +    if (level >= KVM_PUT_RESET_STATE) {
> +        kvm_arch_load_mpstate(env);
> +    }
>  }
>  
>  void kvm_load_tsc(CPUState *env)
> @@ -940,36 +986,6 @@ void kvm_load_tsc(CPUState *env)
>          perror("kvm_set_tsc FAILED.\n");
>  }
>  
> -void kvm_arch_save_mpstate(CPUState *env)
> -{
> -#ifdef KVM_CAP_MP_STATE
> -    int r;
> -    struct kvm_mp_state mp_state;
> -
> -    r = kvm_get_mpstate(env, &mp_state);
> -    if (r < 0)
> -        env->mp_state = -1;
> -    else
> -        env->mp_state = mp_state.mp_state;
> -#else
> -    env->mp_state = -1;
> -#endif
> -}
> -
> -void kvm_arch_load_mpstate(CPUState *env)
> -{
> -#ifdef KVM_CAP_MP_STATE
> -    struct kvm_mp_state mp_state = { .mp_state = env->mp_state };
> -
> -    /*
> -     * -1 indicates that the host did not support GET_MP_STATE ioctl,
> -     *  so don't touch it.
> -     */
> -    if (env->mp_state != -1)
> -        kvm_set_mpstate(env, &mp_state);
> -#endif
> -}
> -
>  void kvm_arch_save_regs(CPUState *env)
>  {
>      struct kvm_regs regs;
> @@ -1366,15 +1382,9 @@ void kvm_arch_cpu_reset(CPUState *env)
>  {
>      kvm_arch_reset_vcpu(env);
>      kvm_put_vcpu_events(env);
> -    if (!cpu_is_bsp(env)) {
> -	if (kvm_irqchip_in_kernel()) {
> -#ifdef KVM_CAP_MP_STATE
> -	    kvm_reset_mpstate(env);
> -#endif
> -	} else {
> -	    env->interrupt_request &= ~CPU_INTERRUPT_HARD;
> -	    env->halted = 1;
> -	}
> +    if (!cpu_is_bsp(env) && !kvm_irqchip_in_kernel()) {
> +        env->interrupt_request &= ~CPU_INTERRUPT_HARD;
> +        env->halted = 1;
>      }
>  }
>  
> diff --git a/qemu-kvm.c b/qemu-kvm.c
> index 53030f1..efa6a29 100644
> --- a/qemu-kvm.c
> +++ b/qemu-kvm.c
> @@ -1579,36 +1579,6 @@ void kvm_update_interrupt_request(CPUState *env)
>      }
>  }
>  
> -static void kvm_do_load_mpstate(void *_env)
> -{
> -    CPUState *env = _env;
> -
> -    kvm_arch_load_mpstate(env);
> -}
> -
> -void kvm_load_mpstate(CPUState *env)
> -{
> -    if (kvm_enabled() && qemu_system_ready && kvm_vcpu_inited(env))
> -        on_vcpu(env, kvm_do_load_mpstate, env);
> -}
> -
> -static void kvm_do_save_mpstate(void *_env)
> -{
> -    CPUState *env = _env;
> -
> -    kvm_arch_save_mpstate(env);
> -#ifdef KVM_CAP_MP_STATE
> -    if (kvm_irqchip_in_kernel())
> -        env->halted = (env->mp_state == KVM_MP_STATE_HALTED);
> -#endif
> -}
> -
> -void kvm_save_mpstate(CPUState *env)
> -{
> -    if (kvm_enabled())
> -        on_vcpu(env, kvm_do_save_mpstate, env);
> -}
> -
>  int kvm_cpu_exec(CPUState *env)
>  {
>      int r;
> diff --git a/qemu-kvm.h b/qemu-kvm.h
> index 6d785a0..aa7bcce 100644
> --- a/qemu-kvm.h
> +++ b/qemu-kvm.h
> @@ -299,16 +299,6 @@ int kvm_get_mpstate(CPUState *env, struct kvm_mp_state *mp_state);
>   *
>   */
>  int kvm_set_mpstate(CPUState *env, struct kvm_mp_state *mp_state);
> -/*!
> - *  * \brief Reset VCPU MP state
> - *
> - */
> -static inline int kvm_reset_mpstate(CPUState *env)
> -{
> -    struct kvm_mp_state mp_state = {.mp_state = KVM_MP_STATE_UNINITIALIZED
> -    };
> -    return kvm_set_mpstate(env, &mp_state);
> -}
>  #endif
>  
>  /*!
> @@ -874,8 +864,6 @@ static inline void kvm_inject_x86_mce(CPUState *cenv, int bank,
>  int kvm_main_loop(void);
>  int kvm_init_ap(void);
>  int kvm_vcpu_inited(CPUState *env);
> -void kvm_load_mpstate(CPUState *env);
> -void kvm_save_mpstate(CPUState *env);
>  void kvm_apic_init(CPUState *env);
>  /* called from vcpu initialization */
>  void qemu_kvm_load_lapic(CPUState *env);
> @@ -909,8 +897,6 @@ int kvm_arch_qemu_create_context(void);
>  
>  void kvm_arch_save_regs(CPUState *env);
>  void kvm_arch_load_regs(CPUState *env, int level);
> -void kvm_arch_load_mpstate(CPUState *env);
> -void kvm_arch_save_mpstate(CPUState *env);
>  int kvm_arch_has_work(CPUState *env);
>  void kvm_arch_process_irqchip_events(CPUState *env);
>  int kvm_arch_try_push_interrupts(void *opaque);
> @@ -979,7 +965,6 @@ void kvm_load_tsc(CPUState *env);
>  #ifdef TARGET_I386
>  #define qemu_kvm_has_pit_state2() (0)
>  #endif
> -#define kvm_save_mpstate(env)   do {} while(0)
>  #define qemu_kvm_cpu_stop(env) do {} while(0)
>  static inline void kvm_load_tsc(CPUState *env)
>  {
> diff --git a/target-i386/machine.c b/target-i386/machine.c
> index bebde82..61e6a87 100644
> --- a/target-i386/machine.c
> +++ b/target-i386/machine.c
> @@ -323,7 +323,6 @@ static void cpu_pre_save(void *opaque)
>      int i;
>  
>      if (kvm_enabled()) {
> -        kvm_save_mpstate(env);
>          kvm_get_vcpu_events(env);
>      }
>  
> @@ -362,12 +361,7 @@ static int cpu_post_load(void *opaque, int version_id)
>      tlb_flush(env, 1);
>  
>      if (kvm_enabled()) {
> -        /* when in-kernel irqchip is used, env->halted causes deadlock
> -           because no userspace IRQs will ever clear this flag */
> -        env->halted = 0;
> -
>          kvm_load_tsc(env);
> -        kvm_load_mpstate(env);
>          kvm_put_vcpu_events(env);
>      }
>  
> diff --git a/target-ia64/machine.c b/target-ia64/machine.c
> index fdbeeef..8cf5bdd 100644
> --- a/target-ia64/machine.c
> +++ b/target-ia64/machine.c
> @@ -4,6 +4,9 @@
>  #include "exec-all.h"
>  #include "qemu-kvm.h"
>  
> +void kvm_arch_save_mpstate(CPUState *env);
> +void kvm_arch_load_mpstate(CPUState *env);
> +
>  void cpu_save(QEMUFile *f, void *opaque)
>  {
>      CPUState *env = opaque;
> -- 
> 1.6.0.2
> 
> --
> 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

--
			Gleb.

[Qemu-devel] Re: [PATCH 15/21] qemu-kvm: Clean up mpstate synchronization

[Jan Kiszka]

Gleb Natapov wrote:
> On Tue, Feb 02, 2010 at 09:19:01AM +0100, Jan Kiszka wrote:
>> Push mpstate reading/writing into kvm_arch_load/save_regs and, on x86,
>> properly synchronize with halted in the accessor functions.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  hw/apic.c             |    7 ----
>>  qemu-kvm-ia64.c       |    4 ++-
>>  qemu-kvm-x86.c        |   88 +++++++++++++++++++++++++++---------------------
>>  qemu-kvm.c            |   30 -----------------
>>  qemu-kvm.h            |   15 --------
>>  target-i386/machine.c |    6 ---
>>  target-ia64/machine.c |    3 ++
>>  7 files changed, 55 insertions(+), 98 deletions(-)
>>
>> diff --git a/hw/apic.c b/hw/apic.c
>> index 3e03e10..092c61e 100644
>> --- a/hw/apic.c
>> +++ b/hw/apic.c
>> @@ -507,13 +507,6 @@ void apic_init_reset(CPUState *env)
>>      s->wait_for_sipi = 1;
>>  
>>      env->halted = !(s->apicbase & MSR_IA32_APICBASE_BSP);
>> -#ifdef KVM_CAP_MP_STATE
>> -    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
>> -        env->mp_state
>> -            = env->halted ? KVM_MP_STATE_UNINITIALIZED : KVM_MP_STATE_RUNNABLE;
>> -        kvm_load_mpstate(env);
>> -    }
>> -#endif
>>  }
>>  
>>  static void apic_startup(APICState *s, int vector_num)
>> diff --git a/qemu-kvm-ia64.c b/qemu-kvm-ia64.c
>> index fc8110e..39bcbeb 100644
>> --- a/qemu-kvm-ia64.c
>> +++ b/qemu-kvm-ia64.c
>> @@ -124,7 +124,9 @@ void kvm_arch_cpu_reset(CPUState *env)
>>  {
>>      if (kvm_irqchip_in_kernel(kvm_context)) {
>>  #ifdef KVM_CAP_MP_STATE
>> -	kvm_reset_mpstate(env->kvm_cpu_state.vcpu_ctx);
>> +        struct kvm_mp_state mp_state = {.mp_state = KVM_MP_STATE_UNINITIALIZED
>> +        };
>> +        kvm_set_mpstate(env, &mp_state);
>>  #endif
>>      } else {
>>  	env->interrupt_request &= ~CPU_INTERRUPT_HARD;
>> diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
>> index 63cd095..6b5895f 100644
>> --- a/qemu-kvm-x86.c
>> +++ b/qemu-kvm-x86.c
>> @@ -754,6 +754,48 @@ static int get_msr_entry(struct kvm_msr_entry *entry, CPUState *env)
>>          return 0;
>>  }
>>  
>> +static void kvm_arch_save_mpstate(CPUState *env)
>> +{
>> +#ifdef KVM_CAP_MP_STATE
>> +    int r;
>> +    struct kvm_mp_state mp_state;
>> +
>> +    r = kvm_get_mpstate(env, &mp_state);
>> +    if (r < 0) {
>> +        env->mp_state = -1;
>> +    } else {
>> +        env->mp_state = mp_state.mp_state;
>> +        if (kvm_irqchip_in_kernel()) {
>> +            env->halted = (env->mp_state == KVM_MP_STATE_HALTED);
>> +        }
>> +    }
>> +#else
>> +    env->mp_state = -1;
>> +#endif
>> +}
>> +
>> +static void kvm_arch_load_mpstate(CPUState *env)
>> +{
>> +#ifdef KVM_CAP_MP_STATE
>> +    struct kvm_mp_state mp_state;
>> +
>> +    /*
>> +     * -1 indicates that the host did not support GET_MP_STATE ioctl,
>> +     *  so don't touch it.
>> +     */
>> +    if (env->mp_state != -1) {
>> +        if (kvm_irqchip_in_kernel()) {
>> +            env->mp_state = env->halted ? KVM_MP_STATE_UNINITIALIZED :
>> +                                          KVM_MP_STATE_RUNNABLE;
> When irqchip is in kernel env->halted doesn't contain any relevant
> information, so this is incorrect. Actually env->halted is updated only
> to show correct cpu state during "info cpus".

OK, copied from apic_init_reset, see above. So that hunk was probably at
least useless, and now it's harmfull. Will drop this and only sync from
mp_state -> halted.

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [PATCH 15/21] qemu-kvm: Clean up mpstate synchronization

[Gleb Natapov]

On Tue, Feb 02, 2010 at 01:31:50PM +0100, Jan Kiszka wrote:
> Gleb Natapov wrote:
> > On Tue, Feb 02, 2010 at 09:19:01AM +0100, Jan Kiszka wrote:
> >> Push mpstate reading/writing into kvm_arch_load/save_regs and, on x86,
> >> properly synchronize with halted in the accessor functions.
> >>
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >> ---
> >>  hw/apic.c             |    7 ----
> >>  qemu-kvm-ia64.c       |    4 ++-
> >>  qemu-kvm-x86.c        |   88 +++++++++++++++++++++++++++---------------------
> >>  qemu-kvm.c            |   30 -----------------
> >>  qemu-kvm.h            |   15 --------
> >>  target-i386/machine.c |    6 ---
> >>  target-ia64/machine.c |    3 ++
> >>  7 files changed, 55 insertions(+), 98 deletions(-)
> >>
> >> diff --git a/hw/apic.c b/hw/apic.c
> >> index 3e03e10..092c61e 100644
> >> --- a/hw/apic.c
> >> +++ b/hw/apic.c
> >> @@ -507,13 +507,6 @@ void apic_init_reset(CPUState *env)
> >>      s->wait_for_sipi = 1;
> >>  
> >>      env->halted = !(s->apicbase & MSR_IA32_APICBASE_BSP);
> >> -#ifdef KVM_CAP_MP_STATE
> >> -    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
> >> -        env->mp_state
> >> -            = env->halted ? KVM_MP_STATE_UNINITIALIZED : KVM_MP_STATE_RUNNABLE;
> >> -        kvm_load_mpstate(env);
> >> -    }
> >> -#endif
> >>  }
> >>  
> >>  static void apic_startup(APICState *s, int vector_num)
> >> diff --git a/qemu-kvm-ia64.c b/qemu-kvm-ia64.c
> >> index fc8110e..39bcbeb 100644
> >> --- a/qemu-kvm-ia64.c
> >> +++ b/qemu-kvm-ia64.c
> >> @@ -124,7 +124,9 @@ void kvm_arch_cpu_reset(CPUState *env)
> >>  {
> >>      if (kvm_irqchip_in_kernel(kvm_context)) {
> >>  #ifdef KVM_CAP_MP_STATE
> >> -	kvm_reset_mpstate(env->kvm_cpu_state.vcpu_ctx);
> >> +        struct kvm_mp_state mp_state = {.mp_state = KVM_MP_STATE_UNINITIALIZED
> >> +        };
> >> +        kvm_set_mpstate(env, &mp_state);
> >>  #endif
> >>      } else {
> >>  	env->interrupt_request &= ~CPU_INTERRUPT_HARD;
> >> diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
> >> index 63cd095..6b5895f 100644
> >> --- a/qemu-kvm-x86.c
> >> +++ b/qemu-kvm-x86.c
> >> @@ -754,6 +754,48 @@ static int get_msr_entry(struct kvm_msr_entry *entry, CPUState *env)
> >>          return 0;
> >>  }
> >>  
> >> +static void kvm_arch_save_mpstate(CPUState *env)
> >> +{
> >> +#ifdef KVM_CAP_MP_STATE
> >> +    int r;
> >> +    struct kvm_mp_state mp_state;
> >> +
> >> +    r = kvm_get_mpstate(env, &mp_state);
> >> +    if (r < 0) {
> >> +        env->mp_state = -1;
> >> +    } else {
> >> +        env->mp_state = mp_state.mp_state;
> >> +        if (kvm_irqchip_in_kernel()) {
> >> +            env->halted = (env->mp_state == KVM_MP_STATE_HALTED);
> >> +        }
> >> +    }
> >> +#else
> >> +    env->mp_state = -1;
> >> +#endif
> >> +}
> >> +
> >> +static void kvm_arch_load_mpstate(CPUState *env)
> >> +{
> >> +#ifdef KVM_CAP_MP_STATE
> >> +    struct kvm_mp_state mp_state;
> >> +
> >> +    /*
> >> +     * -1 indicates that the host did not support GET_MP_STATE ioctl,
> >> +     *  so don't touch it.
> >> +     */
> >> +    if (env->mp_state != -1) {
> >> +        if (kvm_irqchip_in_kernel()) {
> >> +            env->mp_state = env->halted ? KVM_MP_STATE_UNINITIALIZED :
> >> +                                          KVM_MP_STATE_RUNNABLE;
> > When irqchip is in kernel env->halted doesn't contain any relevant
> > information, so this is incorrect. Actually env->halted is updated only
> > to show correct cpu state during "info cpus".
> 
> OK, copied from apic_init_reset, see above. So that hunk was probably at
> least useless, and now it's harmfull. Will drop this and only sync from
> mp_state -> halted.
> 
It was not useless in apic_init_reset it was a shortcut for:
env->mp_state = !(s->apicbase & MSR_IA32_APICBASE_BSP) ? KVM_MP_STATE_UNINITIALIZED : KVM_MP_STATE_RUNNABLE;

On reset BSP VCPU should set env->mp_state to KVM_MP_STATE_RUNNABLE and
all others to KVM_MP_STATE_UNINITIALIZED.

--
			Gleb.

[Qemu-devel] Re: [PATCH 15/21] qemu-kvm: Clean up mpstate synchronization

[Jan Kiszka]

Gleb Natapov wrote:
> On Tue, Feb 02, 2010 at 01:31:50PM +0100, Jan Kiszka wrote:
>> Gleb Natapov wrote:
>>> On Tue, Feb 02, 2010 at 09:19:01AM +0100, Jan Kiszka wrote:
>>>> Push mpstate reading/writing into kvm_arch_load/save_regs and, on x86,
>>>> properly synchronize with halted in the accessor functions.
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>> ---
>>>>  hw/apic.c             |    7 ----
>>>>  qemu-kvm-ia64.c       |    4 ++-
>>>>  qemu-kvm-x86.c        |   88 +++++++++++++++++++++++++++---------------------
>>>>  qemu-kvm.c            |   30 -----------------
>>>>  qemu-kvm.h            |   15 --------
>>>>  target-i386/machine.c |    6 ---
>>>>  target-ia64/machine.c |    3 ++
>>>>  7 files changed, 55 insertions(+), 98 deletions(-)
>>>>
>>>> diff --git a/hw/apic.c b/hw/apic.c
>>>> index 3e03e10..092c61e 100644
>>>> --- a/hw/apic.c
>>>> +++ b/hw/apic.c
>>>> @@ -507,13 +507,6 @@ void apic_init_reset(CPUState *env)
>>>>      s->wait_for_sipi = 1;
>>>>  
>>>>      env->halted = !(s->apicbase & MSR_IA32_APICBASE_BSP);
>>>> -#ifdef KVM_CAP_MP_STATE
>>>> -    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
>>>> -        env->mp_state
>>>> -            = env->halted ? KVM_MP_STATE_UNINITIALIZED : KVM_MP_STATE_RUNNABLE;
>>>> -        kvm_load_mpstate(env);
>>>> -    }
>>>> -#endif
>>>>  }
>>>>  
>>>>  static void apic_startup(APICState *s, int vector_num)
>>>> diff --git a/qemu-kvm-ia64.c b/qemu-kvm-ia64.c
>>>> index fc8110e..39bcbeb 100644
>>>> --- a/qemu-kvm-ia64.c
>>>> +++ b/qemu-kvm-ia64.c
>>>> @@ -124,7 +124,9 @@ void kvm_arch_cpu_reset(CPUState *env)
>>>>  {
>>>>      if (kvm_irqchip_in_kernel(kvm_context)) {
>>>>  #ifdef KVM_CAP_MP_STATE
>>>> -	kvm_reset_mpstate(env->kvm_cpu_state.vcpu_ctx);
>>>> +        struct kvm_mp_state mp_state = {.mp_state = KVM_MP_STATE_UNINITIALIZED
>>>> +        };
>>>> +        kvm_set_mpstate(env, &mp_state);
>>>>  #endif
>>>>      } else {
>>>>  	env->interrupt_request &= ~CPU_INTERRUPT_HARD;
>>>> diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
>>>> index 63cd095..6b5895f 100644
>>>> --- a/qemu-kvm-x86.c
>>>> +++ b/qemu-kvm-x86.c
>>>> @@ -754,6 +754,48 @@ static int get_msr_entry(struct kvm_msr_entry *entry, CPUState *env)
>>>>          return 0;
>>>>  }
>>>>  
>>>> +static void kvm_arch_save_mpstate(CPUState *env)
>>>> +{
>>>> +#ifdef KVM_CAP_MP_STATE
>>>> +    int r;
>>>> +    struct kvm_mp_state mp_state;
>>>> +
>>>> +    r = kvm_get_mpstate(env, &mp_state);
>>>> +    if (r < 0) {
>>>> +        env->mp_state = -1;
>>>> +    } else {
>>>> +        env->mp_state = mp_state.mp_state;
>>>> +        if (kvm_irqchip_in_kernel()) {
>>>> +            env->halted = (env->mp_state == KVM_MP_STATE_HALTED);
>>>> +        }
>>>> +    }
>>>> +#else
>>>> +    env->mp_state = -1;
>>>> +#endif
>>>> +}
>>>> +
>>>> +static void kvm_arch_load_mpstate(CPUState *env)
>>>> +{
>>>> +#ifdef KVM_CAP_MP_STATE
>>>> +    struct kvm_mp_state mp_state;
>>>> +
>>>> +    /*
>>>> +     * -1 indicates that the host did not support GET_MP_STATE ioctl,
>>>> +     *  so don't touch it.
>>>> +     */
>>>> +    if (env->mp_state != -1) {
>>>> +        if (kvm_irqchip_in_kernel()) {
>>>> +            env->mp_state = env->halted ? KVM_MP_STATE_UNINITIALIZED :
>>>> +                                          KVM_MP_STATE_RUNNABLE;
>>> When irqchip is in kernel env->halted doesn't contain any relevant
>>> information, so this is incorrect. Actually env->halted is updated only
>>> to show correct cpu state during "info cpus".
>> OK, copied from apic_init_reset, see above. So that hunk was probably at
>> least useless, and now it's harmfull. Will drop this and only sync from
>> mp_state -> halted.
>>
> It was not useless in apic_init_reset it was a shortcut for:
> env->mp_state = !(s->apicbase & MSR_IA32_APICBASE_BSP) ? KVM_MP_STATE_UNINITIALIZED : KVM_MP_STATE_RUNNABLE;
> 
> On reset BSP VCPU should set env->mp_state to KVM_MP_STATE_RUNNABLE and
> all others to KVM_MP_STATE_UNINITIALIZED.

OK, belongs to kvm vpcu init code then - less encrypted.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [PATCH 20/21] qemu-kvm: Move kvm_set_boot_cpu_id

[Gleb Natapov]

On Tue, Feb 02, 2010 at 09:19:06AM +0100, Jan Kiszka wrote:
> Setting the boot CPU ID is arch-specific KVM stuff. So push it where it
> belongs to.
> 
pc_init1 is also arch-specific, no? TCG should also be able to
have BSP apic_id != 0.

> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  hw/pc.c        |    3 ---
>  qemu-kvm-x86.c |    3 ++-
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/pc.c b/hw/pc.c
> index 6c15a9f..3df6195 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -803,9 +803,6 @@ static void pc_init1(ram_addr_t ram_size,
>  #endif
>      }
>  
> -    if (kvm_enabled()) {
> -        kvm_set_boot_cpu_id(0);
> -    }
>      for (i = 0; i < smp_cpus; i++) {
>          env = pc_new_cpu(cpu_model);
>      }
> diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
> index 9de018e..0f34451 100644
> --- a/qemu-kvm-x86.c
> +++ b/qemu-kvm-x86.c
> @@ -695,7 +695,8 @@ int kvm_arch_qemu_create_context(void)
>      if (kvm_check_extension(kvm_state, KVM_CAP_ADJUST_CLOCK))
>          vmstate_register(0, &vmstate_kvmclock, &kvmclock_data);
>  #endif
> -    return 0;
> +
> +    return kvm_set_boot_cpu_id(0);
>  }
>  
>  static void set_msr_entry(struct kvm_msr_entry *entry, uint32_t index,
> -- 
> 1.6.0.2
> 
> --
> 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

--
			Gleb.

[Qemu-devel] Re: [PATCH 20/21] qemu-kvm: Move kvm_set_boot_cpu_id

[Jan Kiszka]

Gleb Natapov wrote:
> On Tue, Feb 02, 2010 at 09:19:06AM +0100, Jan Kiszka wrote:
>> Setting the boot CPU ID is arch-specific KVM stuff. So push it where it
>> belongs to.
>>
> pc_init1 is also arch-specific, no? TCG should also be able to
> have BSP apic_id != 0.

But not kvm-specific.

I don't understand your second remark. Can you help me how TCG is
affected by kvm_set_boot_cpu_id?

> 
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  hw/pc.c        |    3 ---
>>  qemu-kvm-x86.c |    3 ++-
>>  2 files changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/pc.c b/hw/pc.c
>> index 6c15a9f..3df6195 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -803,9 +803,6 @@ static void pc_init1(ram_addr_t ram_size,
>>  #endif
>>      }
>>  
>> -    if (kvm_enabled()) {
>> -        kvm_set_boot_cpu_id(0);
>> -    }
>>      for (i = 0; i < smp_cpus; i++) {
>>          env = pc_new_cpu(cpu_model);
>>      }
>> diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
>> index 9de018e..0f34451 100644
>> --- a/qemu-kvm-x86.c
>> +++ b/qemu-kvm-x86.c
>> @@ -695,7 +695,8 @@ int kvm_arch_qemu_create_context(void)
>>      if (kvm_check_extension(kvm_state, KVM_CAP_ADJUST_CLOCK))
>>          vmstate_register(0, &vmstate_kvmclock, &kvmclock_data);
>>  #endif
>> -    return 0;
>> +
>> +    return kvm_set_boot_cpu_id(0);
>>  }
>>  
>>  static void set_msr_entry(struct kvm_msr_entry *entry, uint32_t index,
>> -- 
>> 1.6.0.2
>>
>> --
>> 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
> 
> --
> 			Gleb.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [PATCH 20/21] qemu-kvm: Move kvm_set_boot_cpu_id

[Gleb Natapov]

On Tue, Feb 02, 2010 at 03:20:02PM +0100, Jan Kiszka wrote:
> Gleb Natapov wrote:
> > On Tue, Feb 02, 2010 at 09:19:06AM +0100, Jan Kiszka wrote:
> >> Setting the boot CPU ID is arch-specific KVM stuff. So push it where it
> >> belongs to.
> >>
> > pc_init1 is also arch-specific, no? TCG should also be able to
> > have BSP apic_id != 0.
> 
> But not kvm-specific.
> 
> I don't understand your second remark. Can you help me how TCG is
> affected by kvm_set_boot_cpu_id?
> 
It is not affected right now. It assumes that apic ID of BSP cpu is 0,
but this limitation does not exists on real HW. So when QEMU will be fixed
and it will be possible to configure what CPU is BSP this will be the
pace to do it.

> > 
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >> ---
> >>  hw/pc.c        |    3 ---
> >>  qemu-kvm-x86.c |    3 ++-
> >>  2 files changed, 2 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/hw/pc.c b/hw/pc.c
> >> index 6c15a9f..3df6195 100644
> >> --- a/hw/pc.c
> >> +++ b/hw/pc.c
> >> @@ -803,9 +803,6 @@ static void pc_init1(ram_addr_t ram_size,
> >>  #endif
> >>      }
> >>  
> >> -    if (kvm_enabled()) {
> >> -        kvm_set_boot_cpu_id(0);
> >> -    }
> >>      for (i = 0; i < smp_cpus; i++) {
> >>          env = pc_new_cpu(cpu_model);
> >>      }
> >> diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
> >> index 9de018e..0f34451 100644
> >> --- a/qemu-kvm-x86.c
> >> +++ b/qemu-kvm-x86.c
> >> @@ -695,7 +695,8 @@ int kvm_arch_qemu_create_context(void)
> >>      if (kvm_check_extension(kvm_state, KVM_CAP_ADJUST_CLOCK))
> >>          vmstate_register(0, &vmstate_kvmclock, &kvmclock_data);
> >>  #endif
> >> -    return 0;
> >> +
> >> +    return kvm_set_boot_cpu_id(0);
> >>  }
> >>  
> >>  static void set_msr_entry(struct kvm_msr_entry *entry, uint32_t index,
> >> -- 
> >> 1.6.0.2
> >>
> >> --
> >> 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
> > 
> > --
> > 			Gleb.
> 
> Jan
> 
> -- 
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux

--
			Gleb.

[Qemu-devel] Re: [PATCH 20/21] qemu-kvm: Move kvm_set_boot_cpu_id

[Jan Kiszka]

Gleb Natapov wrote:
> On Tue, Feb 02, 2010 at 03:20:02PM +0100, Jan Kiszka wrote:
>> Gleb Natapov wrote:
>>> On Tue, Feb 02, 2010 at 09:19:06AM +0100, Jan Kiszka wrote:
>>>> Setting the boot CPU ID is arch-specific KVM stuff. So push it where it
>>>> belongs to.
>>>>
>>> pc_init1 is also arch-specific, no? TCG should also be able to
>>> have BSP apic_id != 0.
>> But not kvm-specific.
>>
>> I don't understand your second remark. Can you help me how TCG is
>> affected by kvm_set_boot_cpu_id?
>>
> It is not affected right now. It assumes that apic ID of BSP cpu is 0,
> but this limitation does not exists on real HW. So when QEMU will be fixed
> and it will be possible to configure what CPU is BSP this will be the
> pace to do it.

That day pc_init1 (or whatever x86 part) will set the bsp number
somewhere in env or apicstate, and we will transfer that afterwards to kvm.

The point is that kvm_* belongs into kvm[-all].c as far as possible. And
in this case it is possible.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux