[Qemu-devel] [PATCH v2 0/6] Fix migration issues with arbitrary cpu-hot(un)plug

[Qemu-devel] [PATCH v2 0/6] Fix migration issues with arbitrary cpu-hot(un)plug

[Igor Mammedov]

Changes from v1:
  - be conservative, drop QTAIL_*() macros hunks and do list element
    check/cleanup localy in cpu_exec_exit()
  - fix conflict caused by above
  - update Reviewed-bys fom v1
  - drop spapr patches as they will be a bit different and depend
    on not yet applied to master patch:
     'spapr: disintricate core-id from DT semantics'

Series fixes migration issues caused by unstable cpu_index which depended
on order cpus were created/destroyed. It follows David's idea to make
cpu_index assignable by selected boards if board supports cpu-hotplug
with device_add and needs stable cpu_index/'migration id' but leaves
behaviour of the same as before for users that don't care about
cpu-hot(un)plug making changes low-risk.

tested with:
  SRC -snapshot -enable-kvm -smp 1,maxcpus=3 -m 256M guest.img -monitor stdio \
       -device qemu64-x86_64-cpu,id=cpudel,apic-id=1 \
       -device qemu64-x86_64-cpu,apic-id=2 
  (qemu) device_del cpudel
  (qemu) stop
  (qemu) migrate "exec:gzip -c > STATEFILE.gz"
  
  DST -snapshot -enable-kvm -smp 1,maxcpus=3 -m 256M guest.img -monitor stdio \
      -device qemu64-x86_64-cpu,apic-id=2 \
      -incoming "exec: gzip -c -d STATEFILE.gz"

git tree to test with:
     https://github.com/imammedo/qemu cpu-index-stable-v2
 to view
     https://github.com/imammedo/qemu/commits/cpu-index-stable-v2

CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Peter Crosthwaite <crosthwaite.peter@gmail.com>
CC: Richard Henderson <rth@twiddle.net>
CC: Eduardo Habkost <ehabkost@redhat.com>
CC: "Michael S. Tsirkin" <mst@redhat.com>
CC: David Gibson <david@gibson.dropbear.id.au>
CC: Alexander Graf <agraf@suse.de>
CC: Riku Voipio <riku.voipio@iki.fi>
CC: Bharata B Rao <bharata@linux.vnet.ibm.com>
CC: qemu-ppc@nongnu.org


Igor Mammedov (6):
  exec: reduce CONFIG_USER_ONLY ifdeffenery
  exec: don't use cpu_index to detect if cpu_exec_init()'s been called
    for cpu
  exec: set cpu_index only if it's not been explictly set
  qdev: fix object reference leak in case device.realize() fails
  pc: init CPUState->cpu_index with index in possible_cpus[]
  Revert "pc: Enforce adding CPUs contiguously and removing them in
    opposite order"

 bsd-user/qemu.h         |  2 --
 include/exec/exec-all.h | 12 +++++++++
 include/qom/cpu.h       |  2 ++
 linux-user/qemu.h       |  2 --
 exec.c                  | 66 +++++++++----------------------------------------
 hw/core/qdev.c          |  8 +++++-
 hw/i386/pc.c            | 38 +++-------------------------
 qom/cpu.c               |  2 +-
 8 files changed, 38 insertions(+), 94 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH v2 1/6] exec: reduce CONFIG_USER_ONLY ifdeffenery

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 bsd-user/qemu.h         |  2 --
 include/exec/exec-all.h | 12 ++++++++++++
 linux-user/qemu.h       |  2 --
 exec.c                  | 17 +++--------------
 4 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index 6ccc544..2b2b918 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -209,8 +209,6 @@ abi_long target_mremap(abi_ulong old_addr, abi_ulong old_size,
                        abi_ulong new_addr);
 int target_msync(abi_ulong start, abi_ulong len, int flags);
 extern unsigned long last_brk;
-void cpu_list_lock(void);
-void cpu_list_unlock(void);
 #if defined(CONFIG_USE_NPTL)
 void mmap_fork_start(void);
 void mmap_fork_end(int child);
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index acda7b6..d008296 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -56,6 +56,18 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
                               target_ulong pc, target_ulong cs_base,
                               uint32_t flags,
                               int cflags);
+#if defined(CONFIG_USER_ONLY)
+void cpu_list_lock(void);
+void cpu_list_unlock(void);
+#else
+static inline void cpu_list_unlock(void)
+{
+}
+static inline void cpu_list_lock(void)
+{
+}
+#endif
+
 void cpu_exec_init(CPUState *cpu, Error **errp);
 void QEMU_NORETURN cpu_loop_exit(CPUState *cpu);
 void QEMU_NORETURN cpu_loop_exit_restore(CPUState *cpu, uintptr_t pc);
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index cdf23a7..bef465d 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -419,8 +419,6 @@ int target_msync(abi_ulong start, abi_ulong len, int flags);
 extern unsigned long last_brk;
 extern abi_ulong mmap_next_start;
 abi_ulong mmap_find_vma(abi_ulong, abi_ulong);
-void cpu_list_lock(void);
-void cpu_list_unlock(void);
 void mmap_fork_start(void);
 void mmap_fork_end(int child);
 
diff --git a/exec.c b/exec.c
index 60cf46a..2f57c62 100644
--- a/exec.c
+++ b/exec.c
@@ -642,23 +642,17 @@ void cpu_exec_exit(CPUState *cpu)
 {
     CPUClass *cc = CPU_GET_CLASS(cpu);
 
-#if defined(CONFIG_USER_ONLY)
     cpu_list_lock();
-#endif
     if (cpu->cpu_index == -1) {
         /* cpu_index was never allocated by this @cpu or was already freed. */
-#if defined(CONFIG_USER_ONLY)
         cpu_list_unlock();
-#endif
         return;
     }
 
     QTAILQ_REMOVE(&cpus, cpu, node);
     cpu_release_index(cpu);
     cpu->cpu_index = -1;
-#if defined(CONFIG_USER_ONLY)
     cpu_list_unlock();
-#endif
 
     if (cc->vmsd != NULL) {
         vmstate_unregister(NULL, cc->vmsd, cpu);
@@ -670,7 +664,7 @@ void cpu_exec_exit(CPUState *cpu)
 
 void cpu_exec_init(CPUState *cpu, Error **errp)
 {
-    CPUClass *cc = CPU_GET_CLASS(cpu);
+    CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
     Error *local_err = NULL;
 
     cpu->as = NULL;
@@ -694,22 +688,17 @@ void cpu_exec_init(CPUState *cpu, Error **errp)
     object_ref(OBJECT(cpu->memory));
 #endif
 
-#if defined(CONFIG_USER_ONLY)
     cpu_list_lock();
-#endif
     cpu->cpu_index = cpu_get_free_index(&local_err);
     if (local_err) {
         error_propagate(errp, local_err);
-#if defined(CONFIG_USER_ONLY)
         cpu_list_unlock();
-#endif
         return;
     }
     QTAILQ_INSERT_TAIL(&cpus, cpu, node);
-#if defined(CONFIG_USER_ONLY)
-    (void) cc;
     cpu_list_unlock();
-#else
+
+#ifndef CONFIG_USER_ONLY
     if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
         vmstate_register(NULL, cpu->cpu_index, &vmstate_cpu_common, cpu);
     }
-- 
2.7.4

[Qemu-devel] [PATCH v2 2/6] exec: don't use cpu_index to detect if cpu_exec_init()'s been called for cpu

[Igor Mammedov]

Instead use QTAIL's tqe_prev field to detect if cpu's been
placed in list by cpu_exec_init() which is always set if
QTAIL element is in list.

Fixes SIGSEGV on failure path in case cpu_index is assigned
by board and cpu.relalize() fails before cpu_exec_init() is called.

In follow up patches, cpu_index will be assigned by boards that
support cpu hot(un)plug and need stable cpu_index that doesn't
depend on order cpus are created/removed.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reported-by: David Gibson <david@gibson.dropbear.id.au>
---
v2:
  replace setting tqe_prev to NULL in generic QTAIL_REMOVE
  with a check and cleanup in cpu_exec_exit() to be on safe side.
  And delay QTAIL_REMOVE() change to 2.8.
---
 exec.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/exec.c b/exec.c
index 2f57c62..ae45a70 100644
--- a/exec.c
+++ b/exec.c
@@ -643,13 +643,14 @@ void cpu_exec_exit(CPUState *cpu)
     CPUClass *cc = CPU_GET_CLASS(cpu);
 
     cpu_list_lock();
-    if (cpu->cpu_index == -1) {
-        /* cpu_index was never allocated by this @cpu or was already freed. */
+    if (cpu->node.tqe_prev == NULL) {
+        /* there is nothing to undo since cpu_exec_init() hasn't been called */
         cpu_list_unlock();
         return;
     }
 
     QTAILQ_REMOVE(&cpus, cpu, node);
+    cpu->node.tqe_prev = NULL;
     cpu_release_index(cpu);
     cpu->cpu_index = -1;
     cpu_list_unlock();
-- 
2.7.4

[Qemu-devel] [PATCH v2 3/6] exec: set cpu_index only if it's not been explictly set

[Igor Mammedov]

it keeps the legacy behavior for all users that doesn't care
about stable cpu_index value, but would allow boards that
would support device_add/device_del to set stable cpu_index
that won't depend on order in which cpus are created/destroyed.

While at that simplify cpu_get_free_index() as cpu_index
generated by USER_ONLY and softmmu variants is the same
since none of the users support cpu-remove so far, except
of not yet released spapr/x86 device_add/delr, which
will be altered by follow up patches to set stable
cpu_index manually.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 include/qom/cpu.h |  2 ++
 exec.c            | 44 ++++++--------------------------------------
 qom/cpu.c         |  2 +-
 3 files changed, 9 insertions(+), 39 deletions(-)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index cbcd64c..ce0c406 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -883,4 +883,6 @@ extern const struct VMStateDescription vmstate_cpu_common;
     .offset = 0,                                                            \
 }
 
+#define UNASSIGNED_CPU_INDEX -1
+
 #endif
diff --git a/exec.c b/exec.c
index ae45a70..50e3ee2 100644
--- a/exec.c
+++ b/exec.c
@@ -598,30 +598,7 @@ AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)
 }
 #endif
 
-#ifndef CONFIG_USER_ONLY
-static DECLARE_BITMAP(cpu_index_map, MAX_CPUMASK_BITS);
-
-static int cpu_get_free_index(Error **errp)
-{
-    int cpu = find_first_zero_bit(cpu_index_map, MAX_CPUMASK_BITS);
-
-    if (cpu >= MAX_CPUMASK_BITS) {
-        error_setg(errp, "Trying to use more CPUs than max of %d",
-                   MAX_CPUMASK_BITS);
-        return -1;
-    }
-
-    bitmap_set(cpu_index_map, cpu, 1);
-    return cpu;
-}
-
-static void cpu_release_index(CPUState *cpu)
-{
-    bitmap_clear(cpu_index_map, cpu->cpu_index, 1);
-}
-#else
-
-static int cpu_get_free_index(Error **errp)
+static int cpu_get_free_index(void)
 {
     CPUState *some_cpu;
     int cpu_index = 0;
@@ -632,12 +609,6 @@ static int cpu_get_free_index(Error **errp)
     return cpu_index;
 }
 
-static void cpu_release_index(CPUState *cpu)
-{
-    return;
-}
-#endif
-
 void cpu_exec_exit(CPUState *cpu)
 {
     CPUClass *cc = CPU_GET_CLASS(cpu);
@@ -651,8 +622,7 @@ void cpu_exec_exit(CPUState *cpu)
 
     QTAILQ_REMOVE(&cpus, cpu, node);
     cpu->node.tqe_prev = NULL;
-    cpu_release_index(cpu);
-    cpu->cpu_index = -1;
+    cpu->cpu_index = UNASSIGNED_CPU_INDEX;
     cpu_list_unlock();
 
     if (cc->vmsd != NULL) {
@@ -666,7 +636,7 @@ void cpu_exec_exit(CPUState *cpu)
 void cpu_exec_init(CPUState *cpu, Error **errp)
 {
     CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
-    Error *local_err = NULL;
+    Error *local_err ATTRIBUTE_UNUSED = NULL;
 
     cpu->as = NULL;
     cpu->num_ases = 0;
@@ -690,11 +660,9 @@ void cpu_exec_init(CPUState *cpu, Error **errp)
 #endif
 
     cpu_list_lock();
-    cpu->cpu_index = cpu_get_free_index(&local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        cpu_list_unlock();
-        return;
+    if (cpu->cpu_index == UNASSIGNED_CPU_INDEX) {
+        cpu->cpu_index = cpu_get_free_index();
+        assert(cpu->cpu_index != UNASSIGNED_CPU_INDEX);
     }
     QTAILQ_INSERT_TAIL(&cpus, cpu, node);
     cpu_list_unlock();
diff --git a/qom/cpu.c b/qom/cpu.c
index 42b5631..2553247 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -340,7 +340,7 @@ static void cpu_common_initfn(Object *obj)
     CPUState *cpu = CPU(obj);
     CPUClass *cc = CPU_GET_CLASS(obj);
 
-    cpu->cpu_index = -1;
+    cpu->cpu_index = UNASSIGNED_CPU_INDEX;
     cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs;
     qemu_mutex_init(&cpu->work_mutex);
     QTAILQ_INIT(&cpu->breakpoints);
-- 
2.7.4

[Qemu-devel] [PATCH v2 4/6] qdev: fix object reference leak in case device.realize() fails

[Igor Mammedov]

If device doesn't have parent assined before its realize
is called, device_set_realized() will implicitly set parent
to '/machine/unattached'.

However device_set_realized() may fail after that point at
several other points leaving not realized object dangling
in '/machine/unattached' and as result caller of

  obj = object_new()
    obj->ref == 1
  object_property_set_bool(obj,..., true, "realized",...)
    obj->ref == 2
  if (fail)
      object_unref(obj);
      obj->ref == 1

will get object leak instead of expected object destruction.

Fix it by making device_set_realized() to cleanup after itself
in case of failure.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/core/qdev.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 6680089..ee4a083 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -885,6 +885,8 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
     HotplugHandler *hotplug_ctrl;
     BusState *bus;
     Error *local_err = NULL;
+    bool unattached_parent = false;
+    static int unattached_count;
 
     if (dev->hotplugged && !dc->hotpluggable) {
         error_setg(errp, QERR_DEVICE_NO_HOTPLUG, object_get_typename(obj));
@@ -893,12 +895,12 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
 
     if (value && !dev->realized) {
         if (!obj->parent) {
-            static int unattached_count;
             gchar *name = g_strdup_printf("device[%d]", unattached_count++);
 
             object_property_add_child(container_get(qdev_get_machine(),
                                                     "/unattached"),
                                       name, obj, &error_abort);
+            unattached_parent = true;
             g_free(name);
         }
 
@@ -987,6 +989,10 @@ post_realize_fail:
 
 fail:
     error_propagate(errp, local_err);
+    if (unattached_parent) {
+        object_unparent(OBJECT(dev));
+        unattached_count--;
+    }
 }
 
 static bool device_get_hotpluggable(Object *obj, Error **errp)
-- 
2.7.4

[Qemu-devel] [PATCH v2 5/6] pc: init CPUState->cpu_index with index in possible_cpus[]

[Igor Mammedov]

It will enshure that cpu_index for a given cpu stays the same
regardless of the order cpus has been created/deleted.

No compat code is needed as for initial cpus index in
possible_cpus[] matches cpu_index that's been auto-allocated
in cpu_exec_init().

Tha same applies for hotplug with cpu-add command if cpus are
added sequentially in increasing order as 'id' matches cpu_index.

If cpu-add had been used for creating out-of-order cpus,
that created unmigratable instance since it were not possible
to start target with the same cpu_index using old way
of migrating instance with hotplugged cpus:

* source QEMU with CLI (-smp 1,maxcpus=3 and cpu-add id=2)
  following set of cpu_index is allocated [0, 1] with
  apics set [0, 2] respectivelly
* target QEMU is started with CLI -smp 2,maxcpus=3
  resulting in set of cpu_index [0, 1] but with
  set of apics [0, 1] wich doesn't match source.

So we don't need compat code in this case as it's never worked
and newelly added device_add support would use stable cpu_index
set by machine to begin with, so it won't have above limitation
and source QEMU could be migrated to destination regardless
of the order cpus were created.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/pc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 9e3c70f..d6f0347 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1875,6 +1875,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
                             DeviceState *dev, Error **errp)
 {
     int idx;
+    CPUState *cs;
     CPUArchId *cpu_slot;
     X86CPUTopoInfo topo;
     X86CPU *cpu = X86_CPU(dev);
@@ -1975,6 +1976,9 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
         return;
     }
     cpu->thread_id = topo.smt_id;
+
+    cs = CPU(cpu);
+    cs->cpu_index = idx;
 }
 
 static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
-- 
2.7.4

[Qemu-devel] [PATCH v2 6/6] Revert "pc: Enforce adding CPUs contiguously and removing them in opposite order"

[Igor Mammedov]

This reverts commit 4da7faaeb0c7dd3f7f233165d336c878f78fd1eb.

However since commit:
  pc: init CPUState->cpu_index with index in possible_cpus[]
cpu_index is stable regardless of the order cpus were created
and QEMU instance stays migratable always so limitation added
by 4da7faaeb could be safely removed.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/pc.c | 34 ----------------------------------
 1 file changed, 34 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index d6f0347..47593b7 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1818,23 +1818,6 @@ static void pc_cpu_unplug_request_cb(HotplugHandler *hotplug_dev,
         goto out;
     }
 
-    if (idx < pcms->possible_cpus->len - 1 &&
-        pcms->possible_cpus->cpus[idx + 1].cpu != NULL) {
-        X86CPU *cpu;
-
-        for (idx = pcms->possible_cpus->len - 1;
-             pcms->possible_cpus->cpus[idx].cpu == NULL; idx--) {
-            ;;
-        }
-
-        cpu = X86_CPU(pcms->possible_cpus->cpus[idx].cpu);
-        error_setg(&local_err, "CPU [socket-id: %u, core-id: %u,"
-                   " thread-id: %u] should be removed first",
-                   cpu->socket_id, cpu->core_id, cpu->thread_id);
-        goto out;
-
-    }
-
     hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev);
     hhc->unplug_request(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err);
 
@@ -1932,23 +1915,6 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
         return;
     }
 
-    if (idx != 0 && pcms->possible_cpus->cpus[idx - 1].cpu == NULL) {
-        PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
-
-        for (idx = 1; pcms->possible_cpus->cpus[idx].cpu != NULL; idx++) {
-            ;;
-        }
-
-        x86_topo_ids_from_apicid(pcms->possible_cpus->cpus[idx].arch_id,
-                                 smp_cores, smp_threads, &topo);
-
-        if (!pcmc->legacy_cpu_hotplug) {
-            error_setg(errp, "CPU [socket: %u, core: %u, thread: %u] should be"
-                       " added first", topo.pkg_id, topo.core_id, topo.smt_id);
-            return;
-        }
-    }
-
     /* if 'address' properties socket-id/core-id/thread-id are not set, set them
      * so that query_hotpluggable_cpus would show correct values
      */
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 2/6] exec: don't use cpu_index to detect if cpu_exec_init()'s been called for cpu

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1887 bytes --]

On Mon, Jul 25, 2016 at 11:59:20AM +0200, Igor Mammedov wrote:
> Instead use QTAIL's tqe_prev field to detect if cpu's been
> placed in list by cpu_exec_init() which is always set if
> QTAIL element is in list.
> 
> Fixes SIGSEGV on failure path in case cpu_index is assigned
> by board and cpu.relalize() fails before cpu_exec_init() is called.
> 
> In follow up patches, cpu_index will be assigned by boards that
> support cpu hot(un)plug and need stable cpu_index that doesn't
> depend on order cpus are created/removed.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Reported-by: David Gibson <david@gibson.dropbear.id.au>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> v2:
>   replace setting tqe_prev to NULL in generic QTAIL_REMOVE
>   with a check and cleanup in cpu_exec_exit() to be on safe side.
>   And delay QTAIL_REMOVE() change to 2.8.
> ---
>  exec.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 2f57c62..ae45a70 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -643,13 +643,14 @@ void cpu_exec_exit(CPUState *cpu)
>      CPUClass *cc = CPU_GET_CLASS(cpu);
>  
>      cpu_list_lock();
> -    if (cpu->cpu_index == -1) {
> -        /* cpu_index was never allocated by this @cpu or was already freed. */
> +    if (cpu->node.tqe_prev == NULL) {
> +        /* there is nothing to undo since cpu_exec_init() hasn't been called */
>          cpu_list_unlock();
>          return;
>      }
>  
>      QTAILQ_REMOVE(&cpus, cpu, node);
> +    cpu->node.tqe_prev = NULL;
>      cpu_release_index(cpu);
>      cpu->cpu_index = -1;
>      cpu_list_unlock();

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH v2 0/6] Fix migration issues with arbitrary cpu-hot(un)plug

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3157 bytes --]

On Mon, Jul 25, 2016 at 11:59:18AM +0200, Igor Mammedov wrote:
> Changes from v1:
>   - be conservative, drop QTAIL_*() macros hunks and do list element
>     check/cleanup localy in cpu_exec_exit()
>   - fix conflict caused by above
>   - update Reviewed-bys fom v1
>   - drop spapr patches as they will be a bit different and depend
>     on not yet applied to master patch:
>      'spapr: disintricate core-id from DT semantics'
> 
> Series fixes migration issues caused by unstable cpu_index which depended
> on order cpus were created/destroyed. It follows David's idea to make
> cpu_index assignable by selected boards if board supports cpu-hotplug
> with device_add and needs stable cpu_index/'migration id' but leaves
> behaviour of the same as before for users that don't care about
> cpu-hot(un)plug making changes low-risk.

Looks good.  I'd like to see this merged ASAP, and then I can sort out
the ppc / spapr changes we need based on it.

> tested with:
>   SRC -snapshot -enable-kvm -smp 1,maxcpus=3 -m 256M guest.img -monitor stdio \
>        -device qemu64-x86_64-cpu,id=cpudel,apic-id=1 \
>        -device qemu64-x86_64-cpu,apic-id=2 
>   (qemu) device_del cpudel
>   (qemu) stop
>   (qemu) migrate "exec:gzip -c > STATEFILE.gz"
>   
>   DST -snapshot -enable-kvm -smp 1,maxcpus=3 -m 256M guest.img -monitor stdio \
>       -device qemu64-x86_64-cpu,apic-id=2 \
>       -incoming "exec: gzip -c -d STATEFILE.gz"
> 
> git tree to test with:
>      https://github.com/imammedo/qemu cpu-index-stable-v2
>  to view
>      https://github.com/imammedo/qemu/commits/cpu-index-stable-v2
> 
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> CC: Richard Henderson <rth@twiddle.net>
> CC: Eduardo Habkost <ehabkost@redhat.com>
> CC: "Michael S. Tsirkin" <mst@redhat.com>
> CC: David Gibson <david@gibson.dropbear.id.au>
> CC: Alexander Graf <agraf@suse.de>
> CC: Riku Voipio <riku.voipio@iki.fi>
> CC: Bharata B Rao <bharata@linux.vnet.ibm.com>
> CC: qemu-ppc@nongnu.org
> 
> 
> Igor Mammedov (6):
>   exec: reduce CONFIG_USER_ONLY ifdeffenery
>   exec: don't use cpu_index to detect if cpu_exec_init()'s been called
>     for cpu
>   exec: set cpu_index only if it's not been explictly set
>   qdev: fix object reference leak in case device.realize() fails
>   pc: init CPUState->cpu_index with index in possible_cpus[]
>   Revert "pc: Enforce adding CPUs contiguously and removing them in
>     opposite order"
> 
>  bsd-user/qemu.h         |  2 --
>  include/exec/exec-all.h | 12 +++++++++
>  include/qom/cpu.h       |  2 ++
>  linux-user/qemu.h       |  2 --
>  exec.c                  | 66 +++++++++----------------------------------------
>  hw/core/qdev.c          |  8 +++++-
>  hw/i386/pc.c            | 38 +++-------------------------
>  qom/cpu.c               |  2 +-
>  8 files changed, 38 insertions(+), 94 deletions(-)
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH v2 0/6] Fix migration issues with arbitrary cpu-hot(un)plug

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2052 bytes --]

On Mon, Jul 25, 2016 at 11:59:18AM +0200, Igor Mammedov wrote:
> Changes from v1:
>   - be conservative, drop QTAIL_*() macros hunks and do list element
>     check/cleanup localy in cpu_exec_exit()
>   - fix conflict caused by above
>   - update Reviewed-bys fom v1
>   - drop spapr patches as they will be a bit different and depend
>     on not yet applied to master patch:
>      'spapr: disintricate core-id from DT semantics'
> 
> Series fixes migration issues caused by unstable cpu_index which depended
> on order cpus were created/destroyed. It follows David's idea to make
> cpu_index assignable by selected boards if board supports cpu-hotplug
> with device_add and needs stable cpu_index/'migration id' but leaves
> behaviour of the same as before for users that don't care about
> cpu-hot(un)plug making changes low-risk.
> 
> tested with:
>   SRC -snapshot -enable-kvm -smp 1,maxcpus=3 -m 256M guest.img -monitor stdio \
>        -device qemu64-x86_64-cpu,id=cpudel,apic-id=1 \
>        -device qemu64-x86_64-cpu,apic-id=2 
>   (qemu) device_del cpudel
>   (qemu) stop
>   (qemu) migrate "exec:gzip -c > STATEFILE.gz"
>   
>   DST -snapshot -enable-kvm -smp 1,maxcpus=3 -m 256M guest.img -monitor stdio \
>       -device qemu64-x86_64-cpu,apic-id=2 \
>       -incoming "exec: gzip -c -d STATEFILE.gz"
> 
> git tree to test with:
>      https://github.com/imammedo/qemu cpu-index-stable-v2
>  to view
>      https://github.com/imammedo/qemu/commits/cpu-index-stable-v2

Eduardo,

Igor said he thought these would probably go in via your tree.  Do you
have any kind of ETA for this?

I've put these into my ppc-for-2.7 tree, not because I intend to push
them from there, but so I can do the ppc specific fixups on top of
them.  I'm hoping these will disappear in a rebase before my next pull
request.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH v2 0/6] Fix migration issues with arbitrary cpu-hot(un)plug

[Michael S. Tsirkin]

On Mon, Jul 25, 2016 at 11:59:18AM +0200, Igor Mammedov wrote:
> Changes from v1:
>   - be conservative, drop QTAIL_*() macros hunks and do list element
>     check/cleanup localy in cpu_exec_exit()
>   - fix conflict caused by above
>   - update Reviewed-bys fom v1
>   - drop spapr patches as they will be a bit different and depend
>     on not yet applied to master patch:
>      'spapr: disintricate core-id from DT semantics'
> 
> Series fixes migration issues caused by unstable cpu_index which depended
> on order cpus were created/destroyed. It follows David's idea to make
> cpu_index assignable by selected boards if board supports cpu-hotplug
> with device_add and needs stable cpu_index/'migration id' but leaves
> behaviour of the same as before for users that don't care about
> cpu-hot(un)plug making changes low-risk.
> 
> tested with:
>   SRC -snapshot -enable-kvm -smp 1,maxcpus=3 -m 256M guest.img -monitor stdio \
>        -device qemu64-x86_64-cpu,id=cpudel,apic-id=1 \
>        -device qemu64-x86_64-cpu,apic-id=2 
>   (qemu) device_del cpudel
>   (qemu) stop
>   (qemu) migrate "exec:gzip -c > STATEFILE.gz"
>   
>   DST -snapshot -enable-kvm -smp 1,maxcpus=3 -m 256M guest.img -monitor stdio \
>       -device qemu64-x86_64-cpu,apic-id=2 \
>       -incoming "exec: gzip -c -d STATEFILE.gz"
> 
> git tree to test with:
>      https://github.com/imammedo/qemu cpu-index-stable-v2
>  to view
>      https://github.com/imammedo/qemu/commits/cpu-index-stable-v2
> 
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> CC: Richard Henderson <rth@twiddle.net>
> CC: Eduardo Habkost <ehabkost@redhat.com>
> CC: "Michael S. Tsirkin" <mst@redhat.com>
> CC: David Gibson <david@gibson.dropbear.id.au>
> CC: Alexander Graf <agraf@suse.de>
> CC: Riku Voipio <riku.voipio@iki.fi>
> CC: Bharata B Rao <bharata@linux.vnet.ibm.com>
> CC: qemu-ppc@nongnu.org
> 

For PC bits:

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>



> Igor Mammedov (6):
>   exec: reduce CONFIG_USER_ONLY ifdeffenery
>   exec: don't use cpu_index to detect if cpu_exec_init()'s been called
>     for cpu
>   exec: set cpu_index only if it's not been explictly set
>   qdev: fix object reference leak in case device.realize() fails
>   pc: init CPUState->cpu_index with index in possible_cpus[]
>   Revert "pc: Enforce adding CPUs contiguously and removing them in
>     opposite order"
> 
>  bsd-user/qemu.h         |  2 --
>  include/exec/exec-all.h | 12 +++++++++
>  include/qom/cpu.h       |  2 ++
>  linux-user/qemu.h       |  2 --
>  exec.c                  | 66 +++++++++----------------------------------------
>  hw/core/qdev.c          |  8 +++++-
>  hw/i386/pc.c            | 38 +++-------------------------
>  qom/cpu.c               |  2 +-
>  8 files changed, 38 insertions(+), 94 deletions(-)
> 
> -- 
> 2.7.4

Re: [Qemu-devel] [PATCH v2 0/6] Fix migration issues with arbitrary cpu-hot(un)plug

[Eduardo Habkost]

On Tue, Jul 26, 2016 at 01:16:59PM +1000, David Gibson wrote:
> On Mon, Jul 25, 2016 at 11:59:18AM +0200, Igor Mammedov wrote:
> > Changes from v1:
> >   - be conservative, drop QTAIL_*() macros hunks and do list element
> >     check/cleanup localy in cpu_exec_exit()
> >   - fix conflict caused by above
> >   - update Reviewed-bys fom v1
> >   - drop spapr patches as they will be a bit different and depend
> >     on not yet applied to master patch:
> >      'spapr: disintricate core-id from DT semantics'
> > 
> > Series fixes migration issues caused by unstable cpu_index which depended
> > on order cpus were created/destroyed. It follows David's idea to make
> > cpu_index assignable by selected boards if board supports cpu-hotplug
> > with device_add and needs stable cpu_index/'migration id' but leaves
> > behaviour of the same as before for users that don't care about
> > cpu-hot(un)plug making changes low-risk.
> > 
> > tested with:
> >   SRC -snapshot -enable-kvm -smp 1,maxcpus=3 -m 256M guest.img -monitor stdio \
> >        -device qemu64-x86_64-cpu,id=cpudel,apic-id=1 \
> >        -device qemu64-x86_64-cpu,apic-id=2 
> >   (qemu) device_del cpudel
> >   (qemu) stop
> >   (qemu) migrate "exec:gzip -c > STATEFILE.gz"
> >   
> >   DST -snapshot -enable-kvm -smp 1,maxcpus=3 -m 256M guest.img -monitor stdio \
> >       -device qemu64-x86_64-cpu,apic-id=2 \
> >       -incoming "exec: gzip -c -d STATEFILE.gz"
> > 
> > git tree to test with:
> >      https://github.com/imammedo/qemu cpu-index-stable-v2
> >  to view
> >      https://github.com/imammedo/qemu/commits/cpu-index-stable-v2
> 
> Eduardo,
> 
> Igor said he thought these would probably go in via your tree.  Do you
> have any kind of ETA for this?

I will merge it today and send a pull request today or tomorrow.

> 
> I've put these into my ppc-for-2.7 tree, not because I intend to push
> them from there, but so I can do the ppc specific fixups on top of
> them.  I'm hoping these will disappear in a rebase before my next pull
> request.

No problem. Thanks!

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 3/6] exec: set cpu_index only if it's not been explictly set

[Eduardo Habkost]

On Mon, Jul 25, 2016 at 11:59:21AM +0200, Igor Mammedov wrote:
> it keeps the legacy behavior for all users that doesn't care
> about stable cpu_index value, but would allow boards that
> would support device_add/device_del to set stable cpu_index
> that won't depend on order in which cpus are created/destroyed.
> 
> While at that simplify cpu_get_free_index() as cpu_index
> generated by USER_ONLY and softmmu variants is the same
> since none of the users support cpu-remove so far, except
> of not yet released spapr/x86 device_add/delr, which
> will be altered by follow up patches to set stable
> cpu_index manually.

So, cpu_get_free_index() behavior is exactly the same because
cpu-remove is either unsupported, or only supported for the last
CPU. But I worry that this will easily break if anybody starts
implementing CPU removal in other machines without setting
cpu_index explicitly in the board code. Then we can make
cpu_get_free_index() generate a duplicate cpu_index.

I wonder if there any way we can add an assert() somewhere to
ensure no machine will ever allow CPU removal while not
initializing cpu_index explicitly.

(This shouldn't hold this patch, it's just a suggestion for a
possible follow-up patch).

Additional comment:

> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
[...]
> -static int cpu_get_free_index(Error **errp)
> -{
> -    int cpu = find_first_zero_bit(cpu_index_map, MAX_CPUMASK_BITS);
> -
> -    if (cpu >= MAX_CPUMASK_BITS) {
> -        error_setg(errp, "Trying to use more CPUs than max of %d",
> -                   MAX_CPUMASK_BITS);
> -        return -1;
> -    }

We are now relying on the rest of the QEMU code to make sure
cpu_index will be always < MAX_CPUMASK_BITS. In this case, I
suggest we add an assert() below:

[...]
> -    cpu->cpu_index = cpu_get_free_index(&local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        cpu_list_unlock();
> -        return;
> +    if (cpu->cpu_index == UNASSIGNED_CPU_INDEX) {
> +        cpu->cpu_index = cpu_get_free_index();
> +        assert(cpu->cpu_index != UNASSIGNED_CPU_INDEX);
>      }

Here:
  assert(cpu->cpu_index <= MAX_CPUMASK_BITS)


Both comments can be addressed in a follow-up patch, so:

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 0/6] Fix migration issues with arbitrary cpu-hot(un)plug

[Eduardo Habkost]

On Mon, Jul 25, 2016 at 11:59:18AM +0200, Igor Mammedov wrote:
[...]
> Igor Mammedov (6):
>   exec: reduce CONFIG_USER_ONLY ifdeffenery
>   exec: don't use cpu_index to detect if cpu_exec_init()'s been called
>     for cpu
>   exec: set cpu_index only if it's not been explictly set
>   qdev: fix object reference leak in case device.realize() fails
>   pc: init CPUState->cpu_index with index in possible_cpus[]
>   Revert "pc: Enforce adding CPUs contiguously and removing them in
>     opposite order"

Applied to x86-next. Thanks!

Subject line of patch 2/6 was changed to:
  "exec: Don't use cpu_index to detect if cpu_exec_init()'s been called"
to make it shorter.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 3/6] exec: set cpu_index only if it's not been explictly set

[Igor Mammedov]

On Tue, 26 Jul 2016 15:28:13 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Mon, Jul 25, 2016 at 11:59:21AM +0200, Igor Mammedov wrote:
> > it keeps the legacy behavior for all users that doesn't care
> > about stable cpu_index value, but would allow boards that
> > would support device_add/device_del to set stable cpu_index
> > that won't depend on order in which cpus are created/destroyed.
> > 
> > While at that simplify cpu_get_free_index() as cpu_index
> > generated by USER_ONLY and softmmu variants is the same
> > since none of the users support cpu-remove so far, except
> > of not yet released spapr/x86 device_add/delr, which
> > will be altered by follow up patches to set stable
> > cpu_index manually.  
> 
> So, cpu_get_free_index() behavior is exactly the same because
> cpu-remove is either unsupported, or only supported for the last
> CPU. But I worry that this will easily break if anybody starts
> implementing CPU removal in other machines without setting
> cpu_index explicitly in the board code. Then we can make
> cpu_get_free_index() generate a duplicate cpu_index.
> 
> I wonder if there any way we can add an assert() somewhere to
> ensure no machine will ever allow CPU removal while not
> initializing cpu_index explicitly.
> 
> (This shouldn't hold this patch, it's just a suggestion for a
> possible follow-up patch).
I'll try to post it shortly on top of this patch

> 
> Additional comment:
> 
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au>  
> [...]
> > -static int cpu_get_free_index(Error **errp)
> > -{
> > -    int cpu = find_first_zero_bit(cpu_index_map, MAX_CPUMASK_BITS);
> > -
> > -    if (cpu >= MAX_CPUMASK_BITS) {
> > -        error_setg(errp, "Trying to use more CPUs than max of %d",
> > -                   MAX_CPUMASK_BITS);
> > -        return -1;
> > -    }  
> 
> We are now relying on the rest of the QEMU code to make sure
> cpu_index will be always < MAX_CPUMASK_BITS. In this case, I
> suggest we add an assert() below:
> 
> [...]
> > -    cpu->cpu_index = cpu_get_free_index(&local_err);
> > -    if (local_err) {
> > -        error_propagate(errp, local_err);
> > -        cpu_list_unlock();
> > -        return;
> > +    if (cpu->cpu_index == UNASSIGNED_CPU_INDEX) {
> > +        cpu->cpu_index = cpu_get_free_index();
> > +        assert(cpu->cpu_index != UNASSIGNED_CPU_INDEX);
> >      }  
> 
> Here:
>   assert(cpu->cpu_index <= MAX_CPUMASK_BITS)
I'd rather get rid of MAX_CPUMASK_BITS but I haven't looked at
how hard it will be yet.

> 
> 
> Both comments can be addressed in a follow-up patch, so:
> 
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> 

[Qemu-devel] [PATCH v2 7/6] exec: ensure the only one cpu_index allocation method is used

[Igor Mammedov]

Make sure that cpu_index auto allocation isn't used in
combination with manual cpu_index assignment. And
dissallow out of order cpu removal if auto allocation
is in use.

Target that wishes to support out of order unplug should
switch to manual cpu_index assignment. Following patch
could be used as an example:
 (pc: init CPUState->cpu_index with index in possible_cpus[]))

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 exec.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/exec.c b/exec.c
index 8e8416b..e0d8f18 100644
--- a/exec.c
+++ b/exec.c
@@ -598,11 +598,14 @@ AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)
 }
 #endif
 
+static bool cpu_index_auto_assigned;
+
 static int cpu_get_free_index(void)
 {
     CPUState *some_cpu;
     int cpu_index = 0;
 
+    cpu_index_auto_assigned = true;
     CPU_FOREACH(some_cpu) {
         cpu_index++;
     }
@@ -620,6 +623,8 @@ void cpu_exec_exit(CPUState *cpu)
         return;
     }
 
+    assert(!(cpu_index_auto_assigned && cpu != QTAILQ_LAST(&cpus, CPUTailQ)));
+
     QTAILQ_REMOVE(&cpus, cpu, node);
     cpu->cpu_index = UNASSIGNED_CPU_INDEX;
     cpu_list_unlock();
@@ -662,6 +667,8 @@ void cpu_exec_init(CPUState *cpu, Error **errp)
     if (cpu->cpu_index == UNASSIGNED_CPU_INDEX) {
         cpu->cpu_index = cpu_get_free_index();
         assert(cpu->cpu_index != UNASSIGNED_CPU_INDEX);
+    } else {
+        assert(!cpu_index_auto_assigned);
     }
     QTAILQ_INSERT_TAIL(&cpus, cpu, node);
     cpu_list_unlock();
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 7/6] exec: ensure the only one cpu_index allocation method is used

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1993 bytes --]

On Wed, Jul 27, 2016 at 11:24:54AM +0200, Igor Mammedov wrote:
> Make sure that cpu_index auto allocation isn't used in
> combination with manual cpu_index assignment. And
> dissallow out of order cpu removal if auto allocation
> is in use.
> 
> Target that wishes to support out of order unplug should
> switch to manual cpu_index assignment. Following patch
> could be used as an example:
>  (pc: init CPUState->cpu_index with index in possible_cpus[]))
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  exec.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/exec.c b/exec.c
> index 8e8416b..e0d8f18 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -598,11 +598,14 @@ AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)
>  }
>  #endif
>  
> +static bool cpu_index_auto_assigned;
> +
>  static int cpu_get_free_index(void)
>  {
>      CPUState *some_cpu;
>      int cpu_index = 0;
>  
> +    cpu_index_auto_assigned = true;
>      CPU_FOREACH(some_cpu) {
>          cpu_index++;
>      }
> @@ -620,6 +623,8 @@ void cpu_exec_exit(CPUState *cpu)
>          return;
>      }
>  
> +    assert(!(cpu_index_auto_assigned && cpu != QTAILQ_LAST(&cpus, CPUTailQ)));
> +
>      QTAILQ_REMOVE(&cpus, cpu, node);
>      cpu->cpu_index = UNASSIGNED_CPU_INDEX;
>      cpu_list_unlock();
> @@ -662,6 +667,8 @@ void cpu_exec_init(CPUState *cpu, Error **errp)
>      if (cpu->cpu_index == UNASSIGNED_CPU_INDEX) {
>          cpu->cpu_index = cpu_get_free_index();
>          assert(cpu->cpu_index != UNASSIGNED_CPU_INDEX);
> +    } else {
> +        assert(!cpu_index_auto_assigned);
>      }
>      QTAILQ_INSERT_TAIL(&cpus, cpu, node);
>      cpu_list_unlock();

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH v2 7/6] exec: ensure the only one cpu_index allocation method is used

[Eduardo Habkost]

On Wed, Jul 27, 2016 at 11:24:54AM +0200, Igor Mammedov wrote:
> Make sure that cpu_index auto allocation isn't used in
> combination with manual cpu_index assignment. And
> dissallow out of order cpu removal if auto allocation
> is in use.
> 
> Target that wishes to support out of order unplug should
> switch to manual cpu_index assignment. Following patch
> could be used as an example:
>  (pc: init CPUState->cpu_index with index in possible_cpus[]))
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Sorry for the delay, applied to x86-next. Thanks!

-- 
Eduardo