Re: [Qemu-devel] [PULL 13/21] apic_common: vapic_paddr synchronization fix

[Pavel Dovgaluk]

> -----Original Message-----
> From: Paolo Bonzini [mailto:pbonz...@redhat.com]
> Sent: Friday, September 19, 2014 5:42 PM
> To: Pavel Dovgaluk; qemu-devel@nongnu.org
> Subject: Re: [PULL 13/21] apic_common: vapic_paddr synchronization fix
> 
> Il 19/09/2014 14:50, Pavel Dovgaluk ha scritto:
> > vapic_paddr depends on cpu_number. cpu_number cannot be retrieved when 
> > do_vapic_enable
> executes.
> > Thus we cannot reconstruct vapic_paddr in that function.
> 
> cpu_number will always be zero, because do_vapic_enable is only executed
> for smp_cpus == 1.

Right. Here it is:

diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index ee95963..8a19699 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -59,6 +59,7 @@ typedef struct VAPICROMState {
     GuestROMState rom_state;
     size_t rom_size;
     bool rom_mapped_writable;
+    VMChangeStateEntry *vmsentry;
 } VAPICROMState;
 
 #define TYPE_VAPIC "kvmvapic"
@@ -731,14 +732,41 @@ static void do_vapic_enable(void *data)
     VAPICROMState *s = data;
     X86CPU *cpu = X86_CPU(first_cpu);
 
-    vapic_enable(s, cpu);
+    static const uint8_t enabled = 1;
+    cpu_physical_memory_write(s->vapic_paddr + offsetof(VAPICState, enabled),
+                              &enabled, sizeof(enabled));
+    apic_enable_vapic(cpu->apic_state, s->vapic_paddr);
+    s->state = VAPIC_ACTIVE;
 }
 
-static int vapic_post_load(void *opaque, int version_id)
+static void kvmvapic_vm_state_change(void *opaque, int running,
+                                    RunState state)
 {
     VAPICROMState *s = opaque;
     uint8_t *zero;
 
+    if (!running) {
+        return;
+    }
+
+    if (s->state == VAPIC_ACTIVE) {
+        if (smp_cpus == 1) {
+            run_on_cpu(first_cpu, do_vapic_enable, s);
+        } else {
+            zero = g_malloc0(s->rom_state.vapic_size);
+            cpu_physical_memory_write(s->vapic_paddr, zero,
+                                      s->rom_state.vapic_size);
+            g_free(zero);
+        }
+    }
+
+    qemu_del_vm_change_state_handler(s->vmsentry);
+}
+
+static int vapic_post_load(void *opaque, int version_id)
+{
+    VAPICROMState *s = opaque;
+
     /*
      * The old implementation of qemu-kvm did not provide the state
      * VAPIC_STANDBY. Reconstruct it.
@@ -752,17 +780,8 @@ static int vapic_post_load(void *opaque, int version_id)
             return -1;
         }
     }
-    if (s->state == VAPIC_ACTIVE) {
-        if (smp_cpus == 1) {
-            run_on_cpu(first_cpu, do_vapic_enable, s);
-        } else {
-            zero = g_malloc0(s->rom_state.vapic_size);
-            cpu_physical_memory_write(s->vapic_paddr, zero,
-                                      s->rom_state.vapic_size);
-            g_free(zero);
-        }
-    }
 
+    s->vmsentry = qemu_add_vm_change_state_handler(kvmvapic_vm_state_change, 
s);
     return 0;
 }