[RFC][PATCH] qemu-kvm: Introduce writeback scope for cpu_synchronize_state

[Jan Kiszka <jan.kiszka@siemens.com>]

This patch aims at addressing the mp_state writeback issue in a cleaner
fashion. By introducing additional information about the scope of the
scheduled vcpu state writeback, we can simply skin mp_state (and maybe
other specific states in the future) when updating the in-kernel state.

The writeback scope is defined when calling cpu_synchronize_state. It
accumulated, ie. once a full writeback was requested, this will stick
until it was performed.

This unbreaks --disable-kvm builds of qemu-kvm again.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

A corresponding upstream patch is ready to be posted as well, just
waiting for comments on the general direction from KVM POV.

 cpu-defs.h            |    2 +-
 exec.c                |    4 ++--
 gdbstub.c             |    8 ++++----
 hw/apic.c             |    5 ++---
 hw/pc.c               |    2 +-
 monitor.c             |    6 ++----
 qemu-kvm-ia64.c       |    2 ++
 qemu-kvm-x86.c        |    6 ++++--
 qemu-kvm.c            |   44 +++++++++++++++++++++++++-------------------
 qemu-kvm.h            |   13 ++++++-------
 target-i386/helper.c  |    2 +-
 target-i386/machine.c |    7 ++-----
 target-ppc/machine.c  |    4 ++--
 13 files changed, 54 insertions(+), 51 deletions(-)

diff --git a/cpu-defs.h b/cpu-defs.h
index cf502e9..b7cda81 100644
--- a/cpu-defs.h
+++ b/cpu-defs.h
@@ -142,7 +142,7 @@ struct KVMCPUState {
     pthread_t thread;
     int signalled;
     struct qemu_work_item *queued_work_first, *queued_work_last;
-    int regs_modified;
+    int writeback_scope;
 };
 
 #define CPU_TEMP_BUF_NLONGS 128
diff --git a/exec.c b/exec.c
index fcffb0f..290a565 100644
--- a/exec.c
+++ b/exec.c
@@ -529,14 +529,14 @@ static void cpu_common_pre_save(void *opaque)
 {
     CPUState *env = opaque;
 
-    cpu_synchronize_state(env);
+    cpu_synchronize_state(env, CPU_SYNC_RUNTIME);
 }
 
 static int cpu_common_pre_load(void *opaque)
 {
     CPUState *env = opaque;
 
-    cpu_synchronize_state(env);
+    cpu_synchronize_state(env, CPU_SYNC_RESET);
     return 0;
 }
 
diff --git a/gdbstub.c b/gdbstub.c
index ad7cdca..5a3e5ee 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1598,7 +1598,7 @@ static void gdb_breakpoint_remove_all(void)
 static void gdb_set_cpu_pc(GDBState *s, target_ulong pc)
 {
 #if defined(TARGET_I386)
-    cpu_synchronize_state(s->c_cpu);
+    cpu_synchronize_state(s->c_cpu, CPU_SYNC_RUNTIME);
     s->c_cpu->eip = pc;
 #elif defined (TARGET_PPC)
     s->c_cpu->nip = pc;
@@ -1785,7 +1785,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
         }
         break;
     case 'g':
-        cpu_synchronize_state(s->g_cpu);
+        cpu_synchronize_state(s->g_cpu, CPU_SYNC_RUNTIME);
         len = 0;
         for (addr = 0; addr < num_g_regs; addr++) {
             reg_size = gdb_read_register(s->g_cpu, mem_buf + len, addr);
@@ -1795,7 +1795,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
         put_packet(s, buf);
         break;
     case 'G':
-        cpu_synchronize_state(s->g_cpu);
+        cpu_synchronize_state(s->g_cpu, CPU_SYNC_RUNTIME);
         registers = mem_buf;
         len = strlen(p) / 2;
         hextomem((uint8_t *)registers, p, len);
@@ -1959,7 +1959,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
             thread = strtoull(p+16, (char **)&p, 16);
             env = find_cpu(thread);
             if (env != NULL) {
-                cpu_synchronize_state(env);
+                cpu_synchronize_state(env, CPU_SYNC_RUNTIME);
                 len = snprintf((char *)mem_buf, sizeof(mem_buf),
                                "CPU#%d [%s]", env->cpu_index,
                                env->halted ? "halted " : "running");
diff --git a/hw/apic.c b/hw/apic.c
index f7cb9d2..abebde3 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -488,7 +488,7 @@ void apic_init_reset(CPUState *env)
     if (!s)
         return;
 
-    cpu_synchronize_state(env);
+    cpu_synchronize_state(env, CPU_SYNC_RESET);
     s->tpr = 0;
     s->spurious_vec = 0xff;
     s->log_dest = 0;
@@ -512,7 +512,6 @@ void apic_init_reset(CPUState *env)
     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
 }
@@ -1070,7 +1069,7 @@ static void apic_reset(void *opaque)
     APICState *s = opaque;
     int bsp;
 
-    cpu_synchronize_state(s->cpu_env);
+    cpu_synchronize_state(s->cpu_env, CPU_SYNC_RESET);
 
     bsp = cpu_is_bsp(s->cpu_env);
     s->apicbase = 0xfee00000 |
diff --git a/hw/pc.c b/hw/pc.c
index 5d90f8c..23d4a8e 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1021,7 +1021,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_cpu_state.writeback_scope = CPU_SYNC_RESET;
     if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) {
         env->cpuid_apic_id = env->cpu_index;
         /* APIC reset callback resets cpu */
diff --git a/monitor.c b/monitor.c
index c683eb2..a476224 100644
--- a/monitor.c
+++ b/monitor.c
@@ -394,8 +394,7 @@ static CPUState *mon_get_cpu(void)
     if (!cur_mon->mon_cpu) {
         mon_set_cpu(0);
     }
-    cpu_synchronize_state(cur_mon->mon_cpu);
-    kvm_save_mpstate(cur_mon->mon_cpu);
+    cpu_synchronize_state(cur_mon->mon_cpu, CPU_SYNC_RUNTIME);
     return cur_mon->mon_cpu;
 }
 
@@ -486,8 +485,7 @@ static void do_info_cpus(Monitor *mon, QObject **ret_data)
         const char *answer;
         QDict *cpu = qdict_new();
 
-        cpu_synchronize_state(env);
-        kvm_save_mpstate(env);
+        cpu_synchronize_state(env, CPU_SYNC_RUNTIME);
 
         qdict_put(cpu, "CPU", qint_from_int(env->cpu_index));
         answer = (env == mon->mon_cpu) ? "yes" : "no";
diff --git a/qemu-kvm-ia64.c b/qemu-kvm-ia64.c
index a11fde8..fbf5dd6 100644
--- a/qemu-kvm-ia64.c
+++ b/qemu-kvm-ia64.c
@@ -103,6 +103,8 @@ void kvm_arch_save_mpstate(CPUState *env)
         env->mp_state = -1;
     else
         env->mp_state = mp_state.mp_state;
+    if (kvm_irqchip_in_kernel(kvm_context))
+        env->halted = (env->mp_state == KVM_MP_STATE_HALTED);
 #endif
 }
 
diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index 9df0d83..2a45189 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -1040,6 +1040,8 @@ void kvm_arch_save_mpstate(CPUState *env)
         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
@@ -1675,11 +1677,11 @@ uint32_t kvm_arch_get_supported_cpuid(CPUState *env, uint32_t function,
 void kvm_arch_process_irqchip_events(CPUState *env)
 {
     if (env->interrupt_request & CPU_INTERRUPT_INIT) {
-        kvm_cpu_synchronize_state(env);
+        kvm_cpu_synchronize_state(env, CPU_SYNC_RESET);
         do_cpu_init(env);
     }
     if (env->interrupt_request & CPU_INTERRUPT_SIPI) {
-        kvm_cpu_synchronize_state(env);
+        kvm_cpu_synchronize_state(env, CPU_SYNC_RESET);
         do_cpu_sipi(env);
     }
 }
diff --git a/qemu-kvm.c b/qemu-kvm.c
index 05caa1c..ca81a4a 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -906,9 +906,9 @@ int kvm_run(CPUState *env)
         run->request_interrupt_window = kvm_arch_try_push_interrupts(env);
 #endif
 
-    if (env->kvm_cpu_state.regs_modified) {
-        kvm_arch_put_registers(env);
-        env->kvm_cpu_state.regs_modified = 0;
+    if (env->kvm_cpu_state.writeback_scope) {
+        kvm_arch_put_registers(env, env->kvm_cpu_state.writeback_scope);
+        env->kvm_cpu_state.writeback_scope = 0;
     }
 
     r = pre_kvm_run(kvm, env);
@@ -1533,22 +1533,31 @@ static void on_vcpu(CPUState *env, void (*func)(void *data), void *data)
 
 void kvm_arch_get_registers(CPUState *env)
 {
-	kvm_arch_save_regs(env);
+    kvm_arch_save_regs(env);
+    kvm_arch_save_mpstate(env);
 }
 
-static void do_kvm_cpu_synchronize_state(void *_env)
+void kvm_arch_put_registers(CPUState *env, int scope)
 {
-    CPUState *env = _env;
-    if (!env->kvm_cpu_state.regs_modified) {
-        kvm_arch_get_registers(env);
-        env->kvm_cpu_state.regs_modified = 1;
+    kvm_load_registers(env);
+    if (scope & CPU_SYNC_RESET) {
+        kvm_load_mpstate(env);
     }
 }
 
-void kvm_cpu_synchronize_state(CPUState *env)
+static void kvm_do_synchronize_state(void *_env)
 {
-    if (!env->kvm_cpu_state.regs_modified)
-        on_vcpu(env, do_kvm_cpu_synchronize_state, env);
+    CPUState *env = _env;
+
+    kvm_arch_get_registers(env);
+}
+
+void kvm_cpu_synchronize_state(CPUState *env, int writeback_scope)
+{
+    if (!env->kvm_cpu_state.writeback_scope) {
+        on_vcpu(env, kvm_do_synchronize_state, env);
+        env->kvm_cpu_state.writeback_scope |= writeback_scope;
+    }
 }
 
 static void inject_interrupt(void *data)
@@ -1627,10 +1636,6 @@ 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)
@@ -2369,9 +2374,10 @@ 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);
-        cpu_single_env->kvm_cpu_state.regs_modified = 0;
+    if (cpu_single_env->kvm_cpu_state.writeback_scope) {
+        kvm_arch_put_registers(cpu_single_env,
+                               cpu_single_env->kvm_cpu_state.writeback_scope);
+        cpu_single_env->kvm_cpu_state.writeback_scope = 0;
     }
     dbg_data->err =
         kvm_set_guest_debug(cpu_single_env,
diff --git a/qemu-kvm.h b/qemu-kvm.h
index f897c7c..f791345 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -1098,18 +1098,17 @@ static inline int kvm_sync_vcpus(void)
 
 #ifndef QEMU_KVM_NO_CPU
 void kvm_arch_get_registers(CPUState *env);
+void kvm_arch_put_registers(CPUState *env, int scope);
 
-static inline void kvm_arch_put_registers(CPUState *env)
-{
-    kvm_load_registers(env);
-}
+void kvm_cpu_synchronize_state(CPUState *env, int writeback_scope);
 
-void kvm_cpu_synchronize_state(CPUState *env);
+#define CPU_SYNC_RUNTIME	(1 << 0)
+#define CPU_SYNC_RESET		(1 << 1)
 
-static inline void cpu_synchronize_state(CPUState *env)
+static inline void cpu_synchronize_state(CPUState *env, int writeback_scope)
 {
     if (kvm_enabled()) {
-        kvm_cpu_synchronize_state(env);
+        kvm_cpu_synchronize_state(env, writeback_scope);
     }
 }
 
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 4920708..8e2b15f 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -745,7 +745,7 @@ void cpu_dump_state(CPUState *env, FILE *f,
     char cc_op_name[32];
     static const char *seg_name[6] = { "ES", "CS", "SS", "DS", "FS", "GS" };
 
-    cpu_synchronize_state(env);
+    cpu_synchronize_state(env, CPU_SYNC_RUNTIME);
 
     eflags = env->eflags;
 #ifdef TARGET_X86_64
diff --git a/target-i386/machine.c b/target-i386/machine.c
index 2b88fea..23ae142 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -323,8 +323,7 @@ static void cpu_pre_save(void *opaque)
     CPUState *env = opaque;
     int i, bit;
 
-    cpu_synchronize_state(env);
-    kvm_save_mpstate(env);
+    cpu_synchronize_state(env, CPU_SYNC_RUNTIME);
 
     /* FPU */
     env->fpus_vmstate = (env->fpus & ~0x3800) | (env->fpstt & 0x7) << 11;
@@ -355,7 +354,7 @@ static int cpu_pre_load(void *opaque)
 {
     CPUState *env = opaque;
 
-    cpu_synchronize_state(env);
+    cpu_synchronize_state(env, CPU_SYNC_RESET);
     return 0;
 }
 
@@ -386,8 +385,6 @@ static int cpu_post_load(void *opaque, int version_id)
     }
 
     tlb_flush(env, 1);
-    kvm_load_mpstate(env);
-
     return 0;
 }
 
diff --git a/target-ppc/machine.c b/target-ppc/machine.c
index ead38e1..f190279 100644
--- a/target-ppc/machine.c
+++ b/target-ppc/machine.c
@@ -8,7 +8,7 @@ void cpu_save(QEMUFile *f, void *opaque)
     CPUState *env = (CPUState *)opaque;
     unsigned int i, j;
 
-    cpu_synchronize_state(env);
+    cpu_synchronize_state(env, CPU_SYNC_RUNTIME);
 
     for (i = 0; i < 32; i++)
         qemu_put_betls(f, &env->gpr[i]);
@@ -97,7 +97,7 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
     CPUState *env = (CPUState *)opaque;
     unsigned int i, j;
 
-    cpu_synchronize_state(env);
+    cpu_synchronize_state(env, CPU_SYNC_RESET);
 
     for (i = 0; i < 32; i++)
         qemu_get_betls(f, &env->gpr[i]);