[Xen-changelog] [qemu-xen-unstable] piix4acpi, xen, hotplug: Fix race with ACPI AML code and hotplug.

[patchbot@xxxxxxx]

commit f62079cd7de6ec37f48dfc80fb5906f49fecd6f6
Author:     Konrad Rzeszutek Wilk <konrad@xxxxxxxxxx>
AuthorDate: Tue May 14 18:48:49 2013 +0100
Commit:     Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
CommitDate: Tue May 21 11:49:40 2013 +0100

    piix4acpi, xen, hotplug: Fix race with ACPI AML code and hotplug.
    
    This is a race so the amount varies but on a 4PCPU box
    I seem to get only ~14 out of 16 vCPUs I want to online.
    
    The issue at hand is that QEMU xenstore.c hotplug code changes
    the vCPU array and triggers an ACPI SCI for each vCPU
    online/offline change. That means we modify the array of vCPUs
    as the guests ACPI AML code is reading it - resulting in
    the guest reading the data only once and not changing the
    CPU states appropiately.
    
    The fix is to seperate the vCPU array changes from the ACPI SCI
    notification. The code now will enumerate all of the vCPUs
    and change the vCPU array if there is a need for a change.
    If a change did occur then only _one_ ACPI SCI pulse is sent
    to the guest. The vCPU array at that point has the online/offline
    modified to what the user wanted to have.
    
    Specifically, if a user provided this command:
     xl vcpu-set latest 16
    
    (guest config has vcpus=1, maxvcpus=32) QEMU and the guest
    (in this case Linux) would do:
    
    QEMU:                                           Guest OS:
    -xenstore_process_vcpu_set_event
     -> Gets an XenBus notification for CPU1
     -> Updates the gpe_state.cpus_state bitfield.
            -> Pulses the ACPI SCI
                                                    - ACPI SCI kicks in
    
     -> Gets an XenBus notification for CPU2
     -> Updates the gpe_state.cpus_state bitfield.
            -> Pulses the ACPI SCI
    
     -> Gets an XenBus notification for CPU3
     -> Updates the gpe_state.cpus_state bitfield.
            -> Pulses the ACPI SCI
       ...
                                                     - Method(PRST) invoked
    
     -> Gets an XenBus notification for CPU12
     -> Updates the gpe_state.cpus_state bitfield.
            -> Pulses the ACPI SCI
                                                      - reads AF00 for CPU state
                                                        [gets 0xff]
                                                      - reads AF02 [gets 0x7f]
    
     -> Gets an XenBus notification for CPU13
     -> Updates the gpe_state.cpus_state bitfield.
            -> Pulses the ACPI SCI
    
            .. until VCPU 16
                                                     - Method PRST updates
                                                       PR01 through 13 FLG
                                                       entry.
                                                     - PR01->PR13 _MAD
                                                       invoked.
    
                                                     - Brings up 13 CPUs.
    
    While QEMU updates the rest of the cpus_state bitfields the ACPI AML
    only does the CPU hotplug on those it had read.
    
    Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
    Acked-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
    Acked-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx> (for 4.3 release)
---
 xenstore.c |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/xenstore.c b/xenstore.c
index c861d36..b0d6f77 100644
--- a/xenstore.c
+++ b/xenstore.c
@@ -22,6 +22,7 @@
 #include "pci.h"
 #include "qemu-timer.h"
 #include "qemu-xen.h"
+#include "xen_backend.h"
 
 struct xs_handle *xsh = NULL;
 static char *media_filename[MAX_DRIVES+1];
@@ -999,25 +1000,24 @@ void xenstore_record_dm_state(const char *state)
 {
     xenstore_record_dm("state", state);
 }
-
-static void xenstore_process_vcpu_set_event(char **vec)
+static int xenstore_process_one_vcpu_set_event(char *node)
 {
     char *act = NULL;
-    char *vcpustr, *node = vec[XS_WATCH_PATH];
+    char *vcpustr;
     unsigned int vcpu, len;
     int changed = -EINVAL;
 
     vcpustr = strstr(node, "cpu/");
     if (!vcpustr) {
         fprintf(stderr, "vcpu-set: watch node error.\n");
-        return;
+        return changed;
     }
     sscanf(vcpustr, "cpu/%u", &vcpu);
 
     act = xs_read(xsh, XBT_NULL, node, &len);
     if (!act) {
         fprintf(stderr, "vcpu-set: no command yet.\n");
-        return;
+        return changed;
     }
 
     if (!strncmp(act, "online", len))
@@ -1028,8 +1028,61 @@ static void xenstore_process_vcpu_set_event(char **vec)
         fprintf(stderr, "vcpu-set: command error.\n");
 
     free(act);
-    if (changed > 0)
+    return changed;
+}
+static void xenstore_process_vcpu_set_event(char **vec)
+{
+    int changed = 0, rc, i, num = 0;
+    char *vcpu, **dir;
+    char *path = vec[XS_WATCH_PATH];
+
+    /*
+     * Process the event right away in case the loop below fails
+     * to get to vCPU that is in the event.
+     */
+    rc = xenstore_process_one_vcpu_set_event(path);
+    if (rc > 0)
+        changed = 1;
+    /*
+     * We get: /local/domain/<domid>/cpu/<vcpu>/availability or
+     * (at init) /local/domain/<domid>/cpu [ignore it] and need to
+     * iterate over /local/domain/<domid>/cpu/ directory.
+     */
+    vcpu = strstr(path, "cpu/");
+    if (!vcpu) {
+        fprintf(stderr,"[%s]: %s has no CPU!\n", __func__, path);
+        return;
+    }
+    /* Eliminate '/availability' */
+    vcpu[3] = '\0';
+    dir = xs_directory(xsh, XBT_NULL, path, &num);
+
+    if (!dir) {
+        fprintf(stderr, "[%s]: directory %s has no dirs!\n", __func__, path);
+        return;
+    }
+    if (num != vcpus)
+        fprintf(stderr, "[%s]: %d (number of vcpu entries) != %d (maxvcpus)! "\
+                "Continuing on..\n", __func__, num, vcpus);
+
+    for (i = 0; i < num; i++) {
+        char attr[XEN_BUFSIZE];
+
+        /* Construct "/local/domain/<domid>/cpu" (path) with <vcpu> (attr),
+         * and "availability" with '/' sprinkled around. */
+        snprintf(attr, XEN_BUFSIZE, "%s/%s/%s", path, dir[i],  "availability");
+        rc = xenstore_process_one_vcpu_set_event(attr);
+
+        if (rc > 0)
+            changed = 1;
+        if (rc < 0) /* say xs_read failed */
+            break;
+    }
+    free (dir);
+    if (changed > 0) {
+        fprintf(stderr, "Notifying OS about CPU hotplug changes.\n");
         qemu_cpu_notify();
+    }
     return;
 }
 
--
generated by git-patchbot for /home/xen/git/qemu-xen-unstable.git

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog