[PATCH v4 00/13] accel: Fix vCPU memory leaks

[PATCH v4 00/13] accel: Fix vCPU memory leaks

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Hi,

This is a respin of Mark 'vCPU hotunplug related memory leaks' v3:
https://lore.kernel.org/qemu-devel/20220321141409.3112932-1-mark.kanda@oracle.com/

Instead I refactored to extract the common common_vcpu_thread_create()
from all accelerators (except TCG/RR, which requires a special-casing).

Not well tested...

Mark Kanda (3):
  cpu: Free cpu->cpu_ases in cpu_address_space_destroy()
  softmmu/cpus: Free cpu->thread in generic_destroy_vcpu_thread()
  softmmu/cpus: Free cpu->halt_cond in generic_destroy_vcpu_thread()

Philippe Mathieu-Daudé (10):
  target/i386/kvm: Free xsave_buf when destroying vCPU
  target/i386/hvf: Free resources when vCPU is destroyed
  accel/hvf: Remove pointless assertion
  accel/tcg: Init TCG cflags in vCPU thread handler
  accel/tcg: Reorganize tcg_accel_ops_init()
  accel-ops: Introduce create_vcpu_thread_precheck / postcheck handlers
  accel/tcg: Extract rr_create_vcpu_thread_precheck()
  accel/all: Extract common_vcpu_thread_create()
  accel-ops: Introduce common_vcpu_thread_destroy() and .precheck
    handler
  accel/tcg: Add rr_destroy_vcpu_thread_precheck()

 accel/accel-softmmu.c             |  2 +-
 accel/dummy-cpus.c                | 15 +----------
 accel/hvf/hvf-accel-ops.c         | 24 +++--------------
 accel/kvm/kvm-accel-ops.c         | 17 +++---------
 accel/qtest/qtest.c               |  3 ++-
 accel/tcg/tcg-accel-ops-mttcg.c   | 27 +++----------------
 accel/tcg/tcg-accel-ops-mttcg.h   |  3 +--
 accel/tcg/tcg-accel-ops-rr.c      | 44 +++++++++++++++----------------
 accel/tcg/tcg-accel-ops-rr.h      |  7 +++--
 accel/tcg/tcg-accel-ops.c         | 22 +++++++++-------
 accel/xen/xen-all.c               |  2 +-
 cpu.c                             |  1 +
 include/exec/cpu-common.h         |  7 +++++
 include/sysemu/accel-ops.h        |  9 ++++++-
 include/sysemu/cpus.h             |  4 +--
 softmmu/cpus.c                    | 40 +++++++++++++++++++++++++---
 softmmu/physmem.c                 |  5 ++++
 target/i386/hax/hax-accel-ops.c   | 20 ++------------
 target/i386/hvf/hvf.c             |  2 ++
 target/i386/kvm/kvm.c             |  2 ++
 target/i386/nvmm/nvmm-accel-ops.c | 17 +++---------
 target/i386/whpx/whpx-accel-ops.c | 20 +++-----------
 22 files changed, 127 insertions(+), 166 deletions(-)

-- 
2.35.1

[PATCH v4 00/13] accel: Fix vCPU memory leaks

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Hi,

This is a respin of Mark 'vCPU hotunplug related memory leaks' v3:
https://lore.kernel.org/qemu-devel/20220321141409.3112932-1-mark.kanda@oracle.com/

Instead I refactored to extract the common common_vcpu_thread_create()
from all accelerators (except TCG/RR, which requires a special-casing).

Not well tested...

Mark Kanda (3):
  cpu: Free cpu->cpu_ases in cpu_address_space_destroy()
  softmmu/cpus: Free cpu->thread in generic_destroy_vcpu_thread()
  softmmu/cpus: Free cpu->halt_cond in generic_destroy_vcpu_thread()

Philippe Mathieu-Daudé (10):
  target/i386/kvm: Free xsave_buf when destroying vCPU
  target/i386/hvf: Free resources when vCPU is destroyed
  accel/hvf: Remove pointless assertion
  accel/tcg: Init TCG cflags in vCPU thread handler
  accel/tcg: Reorganize tcg_accel_ops_init()
  accel-ops: Introduce create_vcpu_thread_precheck / postcheck handlers
  accel/tcg: Extract rr_create_vcpu_thread_precheck()
  accel/all: Extract common_vcpu_thread_create()
  accel-ops: Introduce common_vcpu_thread_destroy() and .precheck
    handler
  accel/tcg: Add rr_destroy_vcpu_thread_precheck()

 accel/accel-softmmu.c             |  2 +-
 accel/dummy-cpus.c                | 15 +----------
 accel/hvf/hvf-accel-ops.c         | 24 +++--------------
 accel/kvm/kvm-accel-ops.c         | 17 +++---------
 accel/qtest/qtest.c               |  3 ++-
 accel/tcg/tcg-accel-ops-mttcg.c   | 27 +++----------------
 accel/tcg/tcg-accel-ops-mttcg.h   |  3 +--
 accel/tcg/tcg-accel-ops-rr.c      | 44 +++++++++++++++----------------
 accel/tcg/tcg-accel-ops-rr.h      |  7 +++--
 accel/tcg/tcg-accel-ops.c         | 22 +++++++++-------
 accel/xen/xen-all.c               |  2 +-
 cpu.c                             |  1 +
 include/exec/cpu-common.h         |  7 +++++
 include/sysemu/accel-ops.h        |  9 ++++++-
 include/sysemu/cpus.h             |  4 +--
 softmmu/cpus.c                    | 40 +++++++++++++++++++++++++---
 softmmu/physmem.c                 |  5 ++++
 target/i386/hax/hax-accel-ops.c   | 20 ++------------
 target/i386/hvf/hvf.c             |  2 ++
 target/i386/kvm/kvm.c             |  2 ++
 target/i386/nvmm/nvmm-accel-ops.c | 17 +++---------
 target/i386/whpx/whpx-accel-ops.c | 20 +++-----------
 22 files changed, 127 insertions(+), 166 deletions(-)

-- 
2.35.1

[PATCH v4 01/13] cpu: Free cpu->cpu_ases in cpu_address_space_destroy()

[Philippe Mathieu-Daudé]

From: Mark Kanda <mark.kanda@oracle.com>

Create cpu_address_space_destroy() to free a CPU's cpu_ases list.

vCPU hotunplug related leak reported by Valgrind:

==132362== 216 bytes in 1 blocks are definitely lost in loss record 7,119 of 8,549
==132362==    at 0x4C3ADBB: calloc (vg_replace_malloc.c:1117)
==132362==    by 0x69EE4CD: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.5600.4)
==132362==    by 0x7E34AF: cpu_address_space_init (physmem.c:751)
==132362==    by 0x45053E: qemu_init_vcpu (cpus.c:635)
==132362==    by 0x76B4A7: x86_cpu_realizefn (cpu.c:6520)
==132362==    by 0x9343ED: device_set_realized (qdev.c:531)
==132362==    by 0x93E26F: property_set_bool (object.c:2273)
==132362==    by 0x93C23E: object_property_set (object.c:1408)
==132362==    by 0x9406DC: object_property_set_qobject (qom-qobject.c:28)
==132362==    by 0x93C5A9: object_property_set_bool (object.c:1477)
==132362==    by 0x933C81: qdev_realize (qdev.c:333)
==132362==    by 0x455E9A: qdev_device_add_from_qdict (qdev-monitor.c:713)

Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20220321141409.3112932-5-mark.kanda@oracle.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 cpu.c                     | 1 +
 include/exec/cpu-common.h | 7 +++++++
 softmmu/physmem.c         | 5 +++++
 3 files changed, 13 insertions(+)

diff --git a/cpu.c b/cpu.c
index be1f8b074c..59352a1487 100644
--- a/cpu.c
+++ b/cpu.c
@@ -174,6 +174,7 @@ void cpu_exec_unrealizefn(CPUState *cpu)
         tcg_exec_unrealizefn(cpu);
     }
 
+    cpu_address_space_destroy(cpu);
     cpu_list_remove(cpu);
 }
 
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 50a7d2912e..b17ad61ae4 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -111,6 +111,13 @@ size_t qemu_ram_pagesize_largest(void);
  */
 void cpu_address_space_init(CPUState *cpu, int asidx,
                             const char *prefix, MemoryRegion *mr);
+/**
+ * cpu_address_space_destroy:
+ * @cpu: CPU for this address space
+ *
+ * Cleanup CPU's cpu_ases list.
+ */
+void cpu_address_space_destroy(CPUState *cpu);
 
 void cpu_physical_memory_rw(hwaddr addr, void *buf,
                             hwaddr len, bool is_write);
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 43ae70fbe2..aec61ca07a 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -762,6 +762,11 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
     }
 }
 
+void cpu_address_space_destroy(CPUState *cpu)
+{
+    g_free(cpu->cpu_ases);
+}
+
 AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)
 {
     /* Return the AddressSpace corresponding to the specified index */
-- 
2.35.1

[PATCH v4 01/13] cpu: Free cpu->cpu_ases in cpu_address_space_destroy()

[Philippe Mathieu-Daudé]

From: Mark Kanda <mark.kanda@oracle.com>

Create cpu_address_space_destroy() to free a CPU's cpu_ases list.

vCPU hotunplug related leak reported by Valgrind:

==132362== 216 bytes in 1 blocks are definitely lost in loss record 7,119 of 8,549
==132362==    at 0x4C3ADBB: calloc (vg_replace_malloc.c:1117)
==132362==    by 0x69EE4CD: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.5600.4)
==132362==    by 0x7E34AF: cpu_address_space_init (physmem.c:751)
==132362==    by 0x45053E: qemu_init_vcpu (cpus.c:635)
==132362==    by 0x76B4A7: x86_cpu_realizefn (cpu.c:6520)
==132362==    by 0x9343ED: device_set_realized (qdev.c:531)
==132362==    by 0x93E26F: property_set_bool (object.c:2273)
==132362==    by 0x93C23E: object_property_set (object.c:1408)
==132362==    by 0x9406DC: object_property_set_qobject (qom-qobject.c:28)
==132362==    by 0x93C5A9: object_property_set_bool (object.c:1477)
==132362==    by 0x933C81: qdev_realize (qdev.c:333)
==132362==    by 0x455E9A: qdev_device_add_from_qdict (qdev-monitor.c:713)

Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20220321141409.3112932-5-mark.kanda@oracle.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 cpu.c                     | 1 +
 include/exec/cpu-common.h | 7 +++++++
 softmmu/physmem.c         | 5 +++++
 3 files changed, 13 insertions(+)

diff --git a/cpu.c b/cpu.c
index be1f8b074c..59352a1487 100644
--- a/cpu.c
+++ b/cpu.c
@@ -174,6 +174,7 @@ void cpu_exec_unrealizefn(CPUState *cpu)
         tcg_exec_unrealizefn(cpu);
     }
 
+    cpu_address_space_destroy(cpu);
     cpu_list_remove(cpu);
 }
 
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 50a7d2912e..b17ad61ae4 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -111,6 +111,13 @@ size_t qemu_ram_pagesize_largest(void);
  */
 void cpu_address_space_init(CPUState *cpu, int asidx,
                             const char *prefix, MemoryRegion *mr);
+/**
+ * cpu_address_space_destroy:
+ * @cpu: CPU for this address space
+ *
+ * Cleanup CPU's cpu_ases list.
+ */
+void cpu_address_space_destroy(CPUState *cpu);
 
 void cpu_physical_memory_rw(hwaddr addr, void *buf,
                             hwaddr len, bool is_write);
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 43ae70fbe2..aec61ca07a 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -762,6 +762,11 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
     }
 }
 
+void cpu_address_space_destroy(CPUState *cpu)
+{
+    g_free(cpu->cpu_ases);
+}
+
 AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)
 {
     /* Return the AddressSpace corresponding to the specified index */
-- 
2.35.1

[PATCH v4 02/13] target/i386/kvm: Free xsave_buf when destroying vCPU

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Fix vCPU hot-unplug related leak reported by Valgrind:

  ==132362== 4,096 bytes in 1 blocks are definitely lost in loss record 8,440 of 8,549
  ==132362==    at 0x4C3B15F: memalign (vg_replace_malloc.c:1265)
  ==132362==    by 0x4C3B288: posix_memalign (vg_replace_malloc.c:1429)
  ==132362==    by 0xB41195: qemu_try_memalign (memalign.c:53)
  ==132362==    by 0xB41204: qemu_memalign (memalign.c:73)
  ==132362==    by 0x7131CB: kvm_init_xsave (kvm.c:1601)
  ==132362==    by 0x7148ED: kvm_arch_init_vcpu (kvm.c:2031)
  ==132362==    by 0x91D224: kvm_init_vcpu (kvm-all.c:516)
  ==132362==    by 0x9242C9: kvm_vcpu_thread_fn (kvm-accel-ops.c:40)
  ==132362==    by 0xB2EB26: qemu_thread_start (qemu-thread-posix.c:556)
  ==132362==    by 0x7EB2159: start_thread (in /usr/lib64/libpthread-2.28.so)
  ==132362==    by 0x9D45DD2: clone (in /usr/lib64/libc-2.28.so)

Reported-by: Mark Kanda <mark.kanda@oracle.com>
Tested-by: Mark Kanda <mark.kanda@oracle.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/i386/kvm/kvm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index ef2c68a6f4..e93440e774 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -2072,6 +2072,8 @@ int kvm_arch_destroy_vcpu(CPUState *cs)
     X86CPU *cpu = X86_CPU(cs);
     CPUX86State *env = &cpu->env;
 
+    g_free(env->xsave_buf);
+
     if (cpu->kvm_msr_buf) {
         g_free(cpu->kvm_msr_buf);
         cpu->kvm_msr_buf = NULL;
-- 
2.35.1

[PATCH v4 02/13] target/i386/kvm: Free xsave_buf when destroying vCPU

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Fix vCPU hot-unplug related leak reported by Valgrind:

  ==132362== 4,096 bytes in 1 blocks are definitely lost in loss record 8,440 of 8,549
  ==132362==    at 0x4C3B15F: memalign (vg_replace_malloc.c:1265)
  ==132362==    by 0x4C3B288: posix_memalign (vg_replace_malloc.c:1429)
  ==132362==    by 0xB41195: qemu_try_memalign (memalign.c:53)
  ==132362==    by 0xB41204: qemu_memalign (memalign.c:73)
  ==132362==    by 0x7131CB: kvm_init_xsave (kvm.c:1601)
  ==132362==    by 0x7148ED: kvm_arch_init_vcpu (kvm.c:2031)
  ==132362==    by 0x91D224: kvm_init_vcpu (kvm-all.c:516)
  ==132362==    by 0x9242C9: kvm_vcpu_thread_fn (kvm-accel-ops.c:40)
  ==132362==    by 0xB2EB26: qemu_thread_start (qemu-thread-posix.c:556)
  ==132362==    by 0x7EB2159: start_thread (in /usr/lib64/libpthread-2.28.so)
  ==132362==    by 0x9D45DD2: clone (in /usr/lib64/libc-2.28.so)

Reported-by: Mark Kanda <mark.kanda@oracle.com>
Tested-by: Mark Kanda <mark.kanda@oracle.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/i386/kvm/kvm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index ef2c68a6f4..e93440e774 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -2072,6 +2072,8 @@ int kvm_arch_destroy_vcpu(CPUState *cs)
     X86CPU *cpu = X86_CPU(cs);
     CPUX86State *env = &cpu->env;
 
+    g_free(env->xsave_buf);
+
     if (cpu->kvm_msr_buf) {
         g_free(cpu->kvm_msr_buf);
         cpu->kvm_msr_buf = NULL;
-- 
2.35.1

[PATCH v4 03/13] target/i386/hvf: Free resources when vCPU is destroyed

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Both xsave_buf and hvf_caps are allocated in hvf_arch_init_vcpu(),
free them in hvf_arch_vcpu_destroy().

Reported-by: Mark Kanda <mark.kanda@oracle.com>
Suggested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/i386/hvf/hvf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index fc12c02fb2..39fa4641b9 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -163,7 +163,9 @@ void hvf_arch_vcpu_destroy(CPUState *cpu)
     X86CPU *x86_cpu = X86_CPU(cpu);
     CPUX86State *env = &x86_cpu->env;
 
+    g_free(env->xsave_buf);
     g_free(env->hvf_mmio_buf);
+    g_free(hvf_state->hvf_caps);
 }
 
 static void init_tsc_freq(CPUX86State *env)
-- 
2.35.1

[PATCH v4 03/13] target/i386/hvf: Free resources when vCPU is destroyed

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Both xsave_buf and hvf_caps are allocated in hvf_arch_init_vcpu(),
free them in hvf_arch_vcpu_destroy().

Reported-by: Mark Kanda <mark.kanda@oracle.com>
Suggested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/i386/hvf/hvf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index fc12c02fb2..39fa4641b9 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -163,7 +163,9 @@ void hvf_arch_vcpu_destroy(CPUState *cpu)
     X86CPU *x86_cpu = X86_CPU(cpu);
     CPUX86State *env = &x86_cpu->env;
 
+    g_free(env->xsave_buf);
     g_free(env->hvf_mmio_buf);
+    g_free(hvf_state->hvf_caps);
 }
 
 static void init_tsc_freq(CPUX86State *env)
-- 
2.35.1

[PATCH v4 04/13] accel/hvf: Remove pointless assertion

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Both the comment and the hvf_enabled() check are duplicated
at the beginning of hvf_cpu_thread_fn(), which is the thread
callback created by hvf_start_vcpu_thread(). Remove.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 accel/hvf/hvf-accel-ops.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/accel/hvf/hvf-accel-ops.c b/accel/hvf/hvf-accel-ops.c
index 54457c76c2..5c33dc602e 100644
--- a/accel/hvf/hvf-accel-ops.c
+++ b/accel/hvf/hvf-accel-ops.c
@@ -446,12 +446,6 @@ static void hvf_start_vcpu_thread(CPUState *cpu)
 {
     char thread_name[VCPU_THREAD_NAME_SIZE];
 
-    /*
-     * HVF currently does not support TCG, and only runs in
-     * unrestricted-guest mode.
-     */
-    assert(hvf_enabled());
-
     cpu->thread = g_malloc0(sizeof(QemuThread));
     cpu->halt_cond = g_malloc0(sizeof(QemuCond));
     qemu_cond_init(cpu->halt_cond);
-- 
2.35.1

[PATCH v4 04/13] accel/hvf: Remove pointless assertion

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Both the comment and the hvf_enabled() check are duplicated
at the beginning of hvf_cpu_thread_fn(), which is the thread
callback created by hvf_start_vcpu_thread(). Remove.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 accel/hvf/hvf-accel-ops.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/accel/hvf/hvf-accel-ops.c b/accel/hvf/hvf-accel-ops.c
index 54457c76c2..5c33dc602e 100644
--- a/accel/hvf/hvf-accel-ops.c
+++ b/accel/hvf/hvf-accel-ops.c
@@ -446,12 +446,6 @@ static void hvf_start_vcpu_thread(CPUState *cpu)
 {
     char thread_name[VCPU_THREAD_NAME_SIZE];
 
-    /*
-     * HVF currently does not support TCG, and only runs in
-     * unrestricted-guest mode.
-     */
-    assert(hvf_enabled());
-
     cpu->thread = g_malloc0(sizeof(QemuThread));
     cpu->halt_cond = g_malloc0(sizeof(QemuCond));
     qemu_cond_init(cpu->halt_cond);
-- 
2.35.1

[PATCH v4 05/13] accel/tcg: Init TCG cflags in vCPU thread handler

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Move TCG cflags initialization to thread handler.
Remove the duplicated assert checks.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 accel/tcg/tcg-accel-ops-mttcg.c | 5 ++---
 accel/tcg/tcg-accel-ops-rr.c    | 7 +++----
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/accel/tcg/tcg-accel-ops-mttcg.c b/accel/tcg/tcg-accel-ops-mttcg.c
index ea2b741deb..80609964a6 100644
--- a/accel/tcg/tcg-accel-ops-mttcg.c
+++ b/accel/tcg/tcg-accel-ops-mttcg.c
@@ -71,6 +71,8 @@ static void *mttcg_cpu_thread_fn(void *arg)
     assert(tcg_enabled());
     g_assert(!icount_enabled());
 
+    tcg_cpu_init_cflags(cpu, current_machine->smp.max_cpus > 1);
+
     rcu_register_thread();
     force_rcu.notifier.notify = mttcg_force_rcu;
     force_rcu.cpu = cpu;
@@ -140,9 +142,6 @@ void mttcg_start_vcpu_thread(CPUState *cpu)
 {
     char thread_name[VCPU_THREAD_NAME_SIZE];
 
-    g_assert(tcg_enabled());
-    tcg_cpu_init_cflags(cpu, current_machine->smp.max_cpus > 1);
-
     cpu->thread = g_new0(QemuThread, 1);
     cpu->halt_cond = g_malloc0(sizeof(QemuCond));
     qemu_cond_init(cpu->halt_cond);
diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c
index b287110766..de8af32af7 100644
--- a/accel/tcg/tcg-accel-ops-rr.c
+++ b/accel/tcg/tcg-accel-ops-rr.c
@@ -153,7 +153,9 @@ static void *rr_cpu_thread_fn(void *arg)
     Notifier force_rcu;
     CPUState *cpu = arg;
 
-    assert(tcg_enabled());
+    g_assert(tcg_enabled());
+    tcg_cpu_init_cflags(cpu, false);
+
     rcu_register_thread();
     force_rcu.notify = rr_force_rcu;
     rcu_add_force_rcu_notifier(&force_rcu);
@@ -276,9 +278,6 @@ void rr_start_vcpu_thread(CPUState *cpu)
     static QemuCond *single_tcg_halt_cond;
     static QemuThread *single_tcg_cpu_thread;
 
-    g_assert(tcg_enabled());
-    tcg_cpu_init_cflags(cpu, false);
-
     if (!single_tcg_cpu_thread) {
         cpu->thread = g_new0(QemuThread, 1);
         cpu->halt_cond = g_new0(QemuCond, 1);
-- 
2.35.1

[PATCH v4 05/13] accel/tcg: Init TCG cflags in vCPU thread handler

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Move TCG cflags initialization to thread handler.
Remove the duplicated assert checks.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 accel/tcg/tcg-accel-ops-mttcg.c | 5 ++---
 accel/tcg/tcg-accel-ops-rr.c    | 7 +++----
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/accel/tcg/tcg-accel-ops-mttcg.c b/accel/tcg/tcg-accel-ops-mttcg.c
index ea2b741deb..80609964a6 100644
--- a/accel/tcg/tcg-accel-ops-mttcg.c
+++ b/accel/tcg/tcg-accel-ops-mttcg.c
@@ -71,6 +71,8 @@ static void *mttcg_cpu_thread_fn(void *arg)
     assert(tcg_enabled());
     g_assert(!icount_enabled());
 
+    tcg_cpu_init_cflags(cpu, current_machine->smp.max_cpus > 1);
+
     rcu_register_thread();
     force_rcu.notifier.notify = mttcg_force_rcu;
     force_rcu.cpu = cpu;
@@ -140,9 +142,6 @@ void mttcg_start_vcpu_thread(CPUState *cpu)
 {
     char thread_name[VCPU_THREAD_NAME_SIZE];
 
-    g_assert(tcg_enabled());
-    tcg_cpu_init_cflags(cpu, current_machine->smp.max_cpus > 1);
-
     cpu->thread = g_new0(QemuThread, 1);
     cpu->halt_cond = g_malloc0(sizeof(QemuCond));
     qemu_cond_init(cpu->halt_cond);
diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c
index b287110766..de8af32af7 100644
--- a/accel/tcg/tcg-accel-ops-rr.c
+++ b/accel/tcg/tcg-accel-ops-rr.c
@@ -153,7 +153,9 @@ static void *rr_cpu_thread_fn(void *arg)
     Notifier force_rcu;
     CPUState *cpu = arg;
 
-    assert(tcg_enabled());
+    g_assert(tcg_enabled());
+    tcg_cpu_init_cflags(cpu, false);
+
     rcu_register_thread();
     force_rcu.notify = rr_force_rcu;
     rcu_add_force_rcu_notifier(&force_rcu);
@@ -276,9 +278,6 @@ void rr_start_vcpu_thread(CPUState *cpu)
     static QemuCond *single_tcg_halt_cond;
     static QemuThread *single_tcg_cpu_thread;
 
-    g_assert(tcg_enabled());
-    tcg_cpu_init_cflags(cpu, false);
-
     if (!single_tcg_cpu_thread) {
         cpu->thread = g_new0(QemuThread, 1);
         cpu->halt_cond = g_new0(QemuCond, 1);
-- 
2.35.1

[PATCH v4 06/13] accel/tcg: Reorganize tcg_accel_ops_init()

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reorg TCG AccelOpsClass initialization to emphasis icount
mode share more code with single-threaded TCG.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 accel/tcg/tcg-accel-ops.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
index ea7dcad674..d2181ea1e5 100644
--- a/accel/tcg/tcg-accel-ops.c
+++ b/accel/tcg/tcg-accel-ops.c
@@ -98,16 +98,17 @@ static void tcg_accel_ops_init(AccelOpsClass *ops)
         ops->create_vcpu_thread = mttcg_start_vcpu_thread;
         ops->kick_vcpu_thread = mttcg_kick_vcpu_thread;
         ops->handle_interrupt = tcg_handle_interrupt;
-    } else if (icount_enabled()) {
-        ops->create_vcpu_thread = rr_start_vcpu_thread;
-        ops->kick_vcpu_thread = rr_kick_vcpu_thread;
-        ops->handle_interrupt = icount_handle_interrupt;
-        ops->get_virtual_clock = icount_get;
-        ops->get_elapsed_ticks = icount_get;
     } else {
         ops->create_vcpu_thread = rr_start_vcpu_thread;
         ops->kick_vcpu_thread = rr_kick_vcpu_thread;
-        ops->handle_interrupt = tcg_handle_interrupt;
+
+        if (icount_enabled()) {
+            ops->handle_interrupt = icount_handle_interrupt;
+            ops->get_virtual_clock = icount_get;
+            ops->get_elapsed_ticks = icount_get;
+        } else {
+            ops->handle_interrupt = tcg_handle_interrupt;
+        }
     }
 }
 
-- 
2.35.1

[PATCH v4 06/13] accel/tcg: Reorganize tcg_accel_ops_init()

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reorg TCG AccelOpsClass initialization to emphasis icount
mode share more code with single-threaded TCG.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 accel/tcg/tcg-accel-ops.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
index ea7dcad674..d2181ea1e5 100644
--- a/accel/tcg/tcg-accel-ops.c
+++ b/accel/tcg/tcg-accel-ops.c
@@ -98,16 +98,17 @@ static void tcg_accel_ops_init(AccelOpsClass *ops)
         ops->create_vcpu_thread = mttcg_start_vcpu_thread;
         ops->kick_vcpu_thread = mttcg_kick_vcpu_thread;
         ops->handle_interrupt = tcg_handle_interrupt;
-    } else if (icount_enabled()) {
-        ops->create_vcpu_thread = rr_start_vcpu_thread;
-        ops->kick_vcpu_thread = rr_kick_vcpu_thread;
-        ops->handle_interrupt = icount_handle_interrupt;
-        ops->get_virtual_clock = icount_get;
-        ops->get_elapsed_ticks = icount_get;
     } else {
         ops->create_vcpu_thread = rr_start_vcpu_thread;
         ops->kick_vcpu_thread = rr_kick_vcpu_thread;
-        ops->handle_interrupt = tcg_handle_interrupt;
+
+        if (icount_enabled()) {
+            ops->handle_interrupt = icount_handle_interrupt;
+            ops->get_virtual_clock = icount_get;
+            ops->get_elapsed_ticks = icount_get;
+        } else {
+            ops->handle_interrupt = tcg_handle_interrupt;
+        }
     }
 }
 
-- 
2.35.1

[PATCH v4 07/13] accel-ops: Introduce create_vcpu_thread_precheck / postcheck handlers

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Introduce precheck/postcheck handlers which will help to
refactor code common to the various create_vcpu_thread()
implementations.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/sysemu/accel-ops.h | 4 ++++
 softmmu/cpus.c             | 8 +++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/sysemu/accel-ops.h b/include/sysemu/accel-ops.h
index 6013c9444c..26b542d35c 100644
--- a/include/sysemu/accel-ops.h
+++ b/include/sysemu/accel-ops.h
@@ -31,6 +31,10 @@ struct AccelOpsClass {
     bool (*cpus_are_resettable)(void);
 
     void (*create_vcpu_thread)(CPUState *cpu); /* MANDATORY NON-NULL */
+    /* If non-NULL, return whether common vCPU thread must be created */
+    bool (*create_vcpu_thread_precheck)(CPUState *cpu);
+    void (*create_vcpu_thread_postcheck)(CPUState *cpu);
+
     void (*kick_vcpu_thread)(CPUState *cpu);
     bool (*cpu_thread_is_idle)(CPUState *cpu);
 
diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index 7b75bb66d5..857e2081ba 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -637,7 +637,13 @@ void qemu_init_vcpu(CPUState *cpu)
 
     /* accelerators all implement the AccelOpsClass */
     g_assert(cpus_accel != NULL && cpus_accel->create_vcpu_thread != NULL);
-    cpus_accel->create_vcpu_thread(cpu);
+    if (cpus_accel->create_vcpu_thread_precheck == NULL
+            || cpus_accel->create_vcpu_thread_precheck(cpu)) {
+        cpus_accel->create_vcpu_thread(cpu);
+    }
+    if (cpus_accel->create_vcpu_thread_postcheck) {
+        cpus_accel->create_vcpu_thread_postcheck(cpu);
+    }
 
     while (!cpu->created) {
         qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex);
-- 
2.35.1

[PATCH v4 07/13] accel-ops: Introduce create_vcpu_thread_precheck / postcheck handlers

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Introduce precheck/postcheck handlers which will help to
refactor code common to the various create_vcpu_thread()
implementations.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/sysemu/accel-ops.h | 4 ++++
 softmmu/cpus.c             | 8 +++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/sysemu/accel-ops.h b/include/sysemu/accel-ops.h
index 6013c9444c..26b542d35c 100644
--- a/include/sysemu/accel-ops.h
+++ b/include/sysemu/accel-ops.h
@@ -31,6 +31,10 @@ struct AccelOpsClass {
     bool (*cpus_are_resettable)(void);
 
     void (*create_vcpu_thread)(CPUState *cpu); /* MANDATORY NON-NULL */
+    /* If non-NULL, return whether common vCPU thread must be created */
+    bool (*create_vcpu_thread_precheck)(CPUState *cpu);
+    void (*create_vcpu_thread_postcheck)(CPUState *cpu);
+
     void (*kick_vcpu_thread)(CPUState *cpu);
     bool (*cpu_thread_is_idle)(CPUState *cpu);
 
diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index 7b75bb66d5..857e2081ba 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -637,7 +637,13 @@ void qemu_init_vcpu(CPUState *cpu)
 
     /* accelerators all implement the AccelOpsClass */
     g_assert(cpus_accel != NULL && cpus_accel->create_vcpu_thread != NULL);
-    cpus_accel->create_vcpu_thread(cpu);
+    if (cpus_accel->create_vcpu_thread_precheck == NULL
+            || cpus_accel->create_vcpu_thread_precheck(cpu)) {
+        cpus_accel->create_vcpu_thread(cpu);
+    }
+    if (cpus_accel->create_vcpu_thread_postcheck) {
+        cpus_accel->create_vcpu_thread_postcheck(cpu);
+    }
 
     while (!cpu->created) {
         qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex);
-- 
2.35.1

[PATCH v4 08/13] accel/tcg: Extract rr_create_vcpu_thread_precheck()

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

We are going to extract common pattern from rr_start_vcpu_thread().
First extract the rr_create_vcpu_thread_precheck() helper.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 accel/tcg/tcg-accel-ops-rr.c | 7 +++++++
 accel/tcg/tcg-accel-ops-rr.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c
index de8af32af7..3da684b8e6 100644
--- a/accel/tcg/tcg-accel-ops-rr.c
+++ b/accel/tcg/tcg-accel-ops-rr.c
@@ -272,6 +272,13 @@ static void *rr_cpu_thread_fn(void *arg)
     return NULL;
 }
 
+static QemuThread *single_tcg_cpu_thread;
+
+bool rr_create_vcpu_thread_precheck(CPUState *cpu)
+{
+    return !single_tcg_cpu_thread;
+}
+
 void rr_start_vcpu_thread(CPUState *cpu)
 {
     char thread_name[VCPU_THREAD_NAME_SIZE];
diff --git a/accel/tcg/tcg-accel-ops-rr.h b/accel/tcg/tcg-accel-ops-rr.h
index 54f6ae6e86..e2273b66d4 100644
--- a/accel/tcg/tcg-accel-ops-rr.h
+++ b/accel/tcg/tcg-accel-ops-rr.h
@@ -15,6 +15,7 @@
 /* Kick all RR vCPUs. */
 void rr_kick_vcpu_thread(CPUState *unused);
 
+bool rr_create_vcpu_thread_precheck(CPUState *cpu);
 /* start the round robin vcpu thread */
 void rr_start_vcpu_thread(CPUState *cpu);
 
-- 
2.35.1

[PATCH v4 08/13] accel/tcg: Extract rr_create_vcpu_thread_precheck()

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

We are going to extract common pattern from rr_start_vcpu_thread().
First extract the rr_create_vcpu_thread_precheck() helper.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 accel/tcg/tcg-accel-ops-rr.c | 7 +++++++
 accel/tcg/tcg-accel-ops-rr.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c
index de8af32af7..3da684b8e6 100644
--- a/accel/tcg/tcg-accel-ops-rr.c
+++ b/accel/tcg/tcg-accel-ops-rr.c
@@ -272,6 +272,13 @@ static void *rr_cpu_thread_fn(void *arg)
     return NULL;
 }
 
+static QemuThread *single_tcg_cpu_thread;
+
+bool rr_create_vcpu_thread_precheck(CPUState *cpu)
+{
+    return !single_tcg_cpu_thread;
+}
+
 void rr_start_vcpu_thread(CPUState *cpu)
 {
     char thread_name[VCPU_THREAD_NAME_SIZE];
diff --git a/accel/tcg/tcg-accel-ops-rr.h b/accel/tcg/tcg-accel-ops-rr.h
index 54f6ae6e86..e2273b66d4 100644
--- a/accel/tcg/tcg-accel-ops-rr.h
+++ b/accel/tcg/tcg-accel-ops-rr.h
@@ -15,6 +15,7 @@
 /* Kick all RR vCPUs. */
 void rr_kick_vcpu_thread(CPUState *unused);
 
+bool rr_create_vcpu_thread_precheck(CPUState *cpu);
 /* start the round robin vcpu thread */
 void rr_start_vcpu_thread(CPUState *cpu);
 
-- 
2.35.1

[PATCH v4 09/13] accel/all: Extract common_vcpu_thread_create()

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

All accelerators implement a very similar create_vcpu_thread()
handler. Extract the common part as common_vcpu_thread_create(),
which only requires the AccelOpsClass::vcpu_thread_fn() routine
and the accelerator AccelOpsClass::thread_name for debugging
purpose.

The single exception is TCG/RR which re-use a single vCPU. Have
it use the same logic by using the .precheck/.postcheck handlers.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 accel/accel-softmmu.c             |  2 +-
 accel/dummy-cpus.c                | 15 +--------------
 accel/hvf/hvf-accel-ops.c         | 18 +++---------------
 accel/kvm/kvm-accel-ops.c         | 17 +++--------------
 accel/qtest/qtest.c               |  3 ++-
 accel/tcg/tcg-accel-ops-mttcg.c   | 22 +---------------------
 accel/tcg/tcg-accel-ops-mttcg.h   |  3 +--
 accel/tcg/tcg-accel-ops-rr.c      | 21 +++------------------
 accel/tcg/tcg-accel-ops-rr.h      |  6 ++++--
 accel/tcg/tcg-accel-ops.c         |  6 ++++--
 accel/xen/xen-all.c               |  2 +-
 include/sysemu/accel-ops.h        |  3 ++-
 include/sysemu/cpus.h             |  4 ++--
 softmmu/cpus.c                    | 23 ++++++++++++++++++++---
 target/i386/hax/hax-accel-ops.c   | 20 ++------------------
 target/i386/nvmm/nvmm-accel-ops.c | 17 +++--------------
 target/i386/whpx/whpx-accel-ops.c | 20 +++-----------------
 17 files changed, 56 insertions(+), 146 deletions(-)

diff --git a/accel/accel-softmmu.c b/accel/accel-softmmu.c
index 67276e4f52..7800df0234 100644
--- a/accel/accel-softmmu.c
+++ b/accel/accel-softmmu.c
@@ -77,7 +77,7 @@ void accel_init_ops_interfaces(AccelClass *ac)
 
     /*
      * all accelerators need to define ops, providing at least a mandatory
-     * non-NULL create_vcpu_thread operation.
+     * non-NULL vcpu_thread_fn operation.
      */
     g_assert(ops != NULL);
     if (ops->ops_init) {
diff --git a/accel/dummy-cpus.c b/accel/dummy-cpus.c
index 10429fdfb2..9840057969 100644
--- a/accel/dummy-cpus.c
+++ b/accel/dummy-cpus.c
@@ -18,7 +18,7 @@
 #include "qemu/main-loop.h"
 #include "hw/core/cpu.h"
 
-static void *dummy_cpu_thread_fn(void *arg)
+void *dummy_vcpu_thread_fn(void *arg)
 {
     CPUState *cpu = arg;
     sigset_t waitset;
@@ -57,16 +57,3 @@ static void *dummy_cpu_thread_fn(void *arg)
     rcu_unregister_thread();
     return NULL;
 }
-
-void dummy_start_vcpu_thread(CPUState *cpu)
-{
-    char thread_name[VCPU_THREAD_NAME_SIZE];
-
-    cpu->thread = g_malloc0(sizeof(QemuThread));
-    cpu->halt_cond = g_malloc0(sizeof(QemuCond));
-    qemu_cond_init(cpu->halt_cond);
-    snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/DUMMY",
-             cpu->cpu_index);
-    qemu_thread_create(cpu->thread, thread_name, dummy_cpu_thread_fn, cpu,
-                       QEMU_THREAD_JOINABLE);
-}
diff --git a/accel/hvf/hvf-accel-ops.c b/accel/hvf/hvf-accel-ops.c
index 5c33dc602e..91d65036b4 100644
--- a/accel/hvf/hvf-accel-ops.c
+++ b/accel/hvf/hvf-accel-ops.c
@@ -442,25 +442,13 @@ static void *hvf_cpu_thread_fn(void *arg)
     return NULL;
 }
 
-static void hvf_start_vcpu_thread(CPUState *cpu)
-{
-    char thread_name[VCPU_THREAD_NAME_SIZE];
-
-    cpu->thread = g_malloc0(sizeof(QemuThread));
-    cpu->halt_cond = g_malloc0(sizeof(QemuCond));
-    qemu_cond_init(cpu->halt_cond);
-
-    snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/HVF",
-             cpu->cpu_index);
-    qemu_thread_create(cpu->thread, thread_name, hvf_cpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE);
-}
-
 static void hvf_accel_ops_class_init(ObjectClass *oc, void *data)
 {
     AccelOpsClass *ops = ACCEL_OPS_CLASS(oc);
 
-    ops->create_vcpu_thread = hvf_start_vcpu_thread;
+    ops->thread_name = "HVF";
+    ops->vcpu_thread_fn = hvf_cpu_thread_fn;
+
     ops->kick_vcpu_thread = hvf_kick_vcpu_thread;
 
     ops->synchronize_post_reset = hvf_cpu_synchronize_post_reset;
diff --git a/accel/kvm/kvm-accel-ops.c b/accel/kvm/kvm-accel-ops.c
index c4244a23c6..3d13efce0f 100644
--- a/accel/kvm/kvm-accel-ops.c
+++ b/accel/kvm/kvm-accel-ops.c
@@ -61,19 +61,6 @@ static void *kvm_vcpu_thread_fn(void *arg)
     return NULL;
 }
 
-static void kvm_start_vcpu_thread(CPUState *cpu)
-{
-    char thread_name[VCPU_THREAD_NAME_SIZE];
-
-    cpu->thread = g_malloc0(sizeof(QemuThread));
-    cpu->halt_cond = g_malloc0(sizeof(QemuCond));
-    qemu_cond_init(cpu->halt_cond);
-    snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/KVM",
-             cpu->cpu_index);
-    qemu_thread_create(cpu->thread, thread_name, kvm_vcpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE);
-}
-
 static bool kvm_vcpu_thread_is_idle(CPUState *cpu)
 {
     return !kvm_halt_in_kernel();
@@ -88,7 +75,9 @@ static void kvm_accel_ops_class_init(ObjectClass *oc, void *data)
 {
     AccelOpsClass *ops = ACCEL_OPS_CLASS(oc);
 
-    ops->create_vcpu_thread = kvm_start_vcpu_thread;
+    ops->thread_name = "KVM";
+    ops->vcpu_thread_fn = kvm_vcpu_thread_fn;
+
     ops->cpu_thread_is_idle = kvm_vcpu_thread_is_idle;
     ops->cpus_are_resettable = kvm_cpus_are_resettable;
     ops->synchronize_post_reset = kvm_cpu_synchronize_post_reset;
diff --git a/accel/qtest/qtest.c b/accel/qtest/qtest.c
index f6056ac836..1d0b1c855c 100644
--- a/accel/qtest/qtest.c
+++ b/accel/qtest/qtest.c
@@ -50,7 +50,8 @@ static void qtest_accel_ops_class_init(ObjectClass *oc, void *data)
 {
     AccelOpsClass *ops = ACCEL_OPS_CLASS(oc);
 
-    ops->create_vcpu_thread = dummy_start_vcpu_thread;
+    ops->vcpu_thread_fn = dummy_vcpu_thread_fn;
+
     ops->get_virtual_clock = qtest_get_virtual_clock;
 };
 
diff --git a/accel/tcg/tcg-accel-ops-mttcg.c b/accel/tcg/tcg-accel-ops-mttcg.c
index 80609964a6..c7836332d7 100644
--- a/accel/tcg/tcg-accel-ops-mttcg.c
+++ b/accel/tcg/tcg-accel-ops-mttcg.c
@@ -63,7 +63,7 @@ static void mttcg_force_rcu(Notifier *notify, void *data)
  * current CPUState for a given thread.
  */
 
-static void *mttcg_cpu_thread_fn(void *arg)
+void *mttcg_vcpu_thread_fn(void *arg)
 {
     MttcgForceRcuNotifier force_rcu;
     CPUState *cpu = arg;
@@ -137,23 +137,3 @@ void mttcg_kick_vcpu_thread(CPUState *cpu)
 {
     cpu_exit(cpu);
 }
-
-void mttcg_start_vcpu_thread(CPUState *cpu)
-{
-    char thread_name[VCPU_THREAD_NAME_SIZE];
-
-    cpu->thread = g_new0(QemuThread, 1);
-    cpu->halt_cond = g_malloc0(sizeof(QemuCond));
-    qemu_cond_init(cpu->halt_cond);
-
-    /* create a thread per vCPU with TCG (MTTCG) */
-    snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/TCG",
-             cpu->cpu_index);
-
-    qemu_thread_create(cpu->thread, thread_name, mttcg_cpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE);
-
-#ifdef _WIN32
-    cpu->hThread = qemu_thread_get_handle(cpu->thread);
-#endif
-}
diff --git a/accel/tcg/tcg-accel-ops-mttcg.h b/accel/tcg/tcg-accel-ops-mttcg.h
index 9fdc5a2ab5..b61aff5c1d 100644
--- a/accel/tcg/tcg-accel-ops-mttcg.h
+++ b/accel/tcg/tcg-accel-ops-mttcg.h
@@ -13,7 +13,6 @@
 /* kick MTTCG vCPU thread */
 void mttcg_kick_vcpu_thread(CPUState *cpu);
 
-/* start an mttcg vCPU thread */
-void mttcg_start_vcpu_thread(CPUState *cpu);
+void *mttcg_vcpu_thread_fn(void *arg);
 
 #endif /* TCG_CPUS_MTTCG_H */
diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c
index 3da684b8e6..006b787289 100644
--- a/accel/tcg/tcg-accel-ops-rr.c
+++ b/accel/tcg/tcg-accel-ops-rr.c
@@ -148,7 +148,7 @@ static void rr_force_rcu(Notifier *notify, void *data)
  * elsewhere.
  */
 
-static void *rr_cpu_thread_fn(void *arg)
+void *rr_vcpu_thread_fn(void *arg)
 {
     Notifier force_rcu;
     CPUState *cpu = arg;
@@ -279,28 +279,13 @@ bool rr_create_vcpu_thread_precheck(CPUState *cpu)
     return !single_tcg_cpu_thread;
 }
 
-void rr_start_vcpu_thread(CPUState *cpu)
+void rr_create_vcpu_thread_postcheck(CPUState *cpu)
 {
-    char thread_name[VCPU_THREAD_NAME_SIZE];
     static QemuCond *single_tcg_halt_cond;
-    static QemuThread *single_tcg_cpu_thread;
-
-    if (!single_tcg_cpu_thread) {
-        cpu->thread = g_new0(QemuThread, 1);
-        cpu->halt_cond = g_new0(QemuCond, 1);
-        qemu_cond_init(cpu->halt_cond);
-
-        /* share a single thread for all cpus with TCG */
-        snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "ALL CPUs/TCG");
-        qemu_thread_create(cpu->thread, thread_name,
-                           rr_cpu_thread_fn,
-                           cpu, QEMU_THREAD_JOINABLE);
 
+    if (! single_tcg_cpu_thread) {
         single_tcg_halt_cond = cpu->halt_cond;
         single_tcg_cpu_thread = cpu->thread;
-#ifdef _WIN32
-        cpu->hThread = qemu_thread_get_handle(cpu->thread);
-#endif
     } else {
         /* we share the thread */
         cpu->thread = single_tcg_cpu_thread;
diff --git a/accel/tcg/tcg-accel-ops-rr.h b/accel/tcg/tcg-accel-ops-rr.h
index e2273b66d4..a1e75e7afb 100644
--- a/accel/tcg/tcg-accel-ops-rr.h
+++ b/accel/tcg/tcg-accel-ops-rr.h
@@ -16,7 +16,9 @@
 void rr_kick_vcpu_thread(CPUState *unused);
 
 bool rr_create_vcpu_thread_precheck(CPUState *cpu);
-/* start the round robin vcpu thread */
-void rr_start_vcpu_thread(CPUState *cpu);
+void rr_create_vcpu_thread_postcheck(CPUState *cpu);
+bool rr_destroy_vcpu_thread_precheck(CPUState *cpu);
+
+void *rr_vcpu_thread_fn(void *arg);
 
 #endif /* TCG_CPUS_RR_H */
diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
index d2181ea1e5..127dd6fee5 100644
--- a/accel/tcg/tcg-accel-ops.c
+++ b/accel/tcg/tcg-accel-ops.c
@@ -95,11 +95,13 @@ void tcg_handle_interrupt(CPUState *cpu, int mask)
 static void tcg_accel_ops_init(AccelOpsClass *ops)
 {
     if (qemu_tcg_mttcg_enabled()) {
-        ops->create_vcpu_thread = mttcg_start_vcpu_thread;
+        ops->vcpu_thread_fn = mttcg_vcpu_thread_fn;
         ops->kick_vcpu_thread = mttcg_kick_vcpu_thread;
         ops->handle_interrupt = tcg_handle_interrupt;
     } else {
-        ops->create_vcpu_thread = rr_start_vcpu_thread;
+        ops->vcpu_thread_fn = rr_vcpu_thread_fn;
+        ops->create_vcpu_thread_precheck = rr_create_vcpu_thread_precheck;
+        ops->create_vcpu_thread_postcheck = rr_create_vcpu_thread_postcheck;
         ops->kick_vcpu_thread = rr_kick_vcpu_thread;
 
         if (icount_enabled()) {
diff --git a/accel/xen/xen-all.c b/accel/xen/xen-all.c
index 69aa7d018b..ef40f626e2 100644
--- a/accel/xen/xen-all.c
+++ b/accel/xen/xen-all.c
@@ -219,7 +219,7 @@ static void xen_accel_ops_class_init(ObjectClass *oc, void *data)
 {
     AccelOpsClass *ops = ACCEL_OPS_CLASS(oc);
 
-    ops->create_vcpu_thread = dummy_start_vcpu_thread;
+    ops->vcpu_thread_fn = dummy_vcpu_thread_fn;
 }
 
 static const TypeInfo xen_accel_ops_type = {
diff --git a/include/sysemu/accel-ops.h b/include/sysemu/accel-ops.h
index 26b542d35c..caf337f61f 100644
--- a/include/sysemu/accel-ops.h
+++ b/include/sysemu/accel-ops.h
@@ -30,7 +30,8 @@ struct AccelOpsClass {
 
     bool (*cpus_are_resettable)(void);
 
-    void (*create_vcpu_thread)(CPUState *cpu); /* MANDATORY NON-NULL */
+    const char *thread_name;
+    void *(*vcpu_thread_fn)(void *arg); /* MANDATORY NON-NULL */
     /* If non-NULL, return whether common vCPU thread must be created */
     bool (*create_vcpu_thread_precheck)(CPUState *cpu);
     void (*create_vcpu_thread_postcheck)(CPUState *cpu);
diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h
index b5c87d48b3..bf5629c58f 100644
--- a/include/sysemu/cpus.h
+++ b/include/sysemu/cpus.h
@@ -9,8 +9,8 @@ void cpus_register_accel(const AccelOpsClass *i);
 
 /* accel/dummy-cpus.c */
 
-/* Create a dummy vcpu for AccelOpsClass->create_vcpu_thread */
-void dummy_start_vcpu_thread(CPUState *);
+/* Create a dummy vcpu for AccelOpsClass->vcpu_thread_fn */
+void *dummy_vcpu_thread_fn(void *arg);
 
 /* interface available for cpus accelerator threads */
 
diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index 857e2081ba..cf430ac486 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -601,6 +601,22 @@ void resume_all_vcpus(void)
     }
 }
 
+static void common_vcpu_thread_create(CPUState *cpu)
+{
+    char thread_name[VCPU_THREAD_NAME_SIZE];
+
+    cpu->thread = g_new0(QemuThread, 1);
+    cpu->halt_cond = g_new0(QemuCond, 1);
+    qemu_cond_init(cpu->halt_cond);
+    snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/%s",
+             cpu->cpu_index, cpus_accel->thread_name ?: "DUMMY");
+    qemu_thread_create(cpu->thread, thread_name, cpus_accel->vcpu_thread_fn,
+                       cpu, QEMU_THREAD_JOINABLE);
+#ifdef _WIN32
+    cpu->hThread = qemu_thread_get_handle(cpu->thread);
+#endif
+}
+
 void cpu_remove_sync(CPUState *cpu)
 {
     cpu->stop = true;
@@ -614,7 +630,7 @@ void cpu_remove_sync(CPUState *cpu)
 void cpus_register_accel(const AccelOpsClass *ops)
 {
     assert(ops != NULL);
-    assert(ops->create_vcpu_thread != NULL); /* mandatory */
+    assert(ops->vcpu_thread_fn != NULL); /* mandatory */
     cpus_accel = ops;
 }
 
@@ -636,10 +652,11 @@ void qemu_init_vcpu(CPUState *cpu)
     }
 
     /* accelerators all implement the AccelOpsClass */
-    g_assert(cpus_accel != NULL && cpus_accel->create_vcpu_thread != NULL);
+    g_assert(cpus_accel != NULL && cpus_accel->vcpu_thread_fn != NULL);
+
     if (cpus_accel->create_vcpu_thread_precheck == NULL
             || cpus_accel->create_vcpu_thread_precheck(cpu)) {
-        cpus_accel->create_vcpu_thread(cpu);
+        common_vcpu_thread_create(cpu);
     }
     if (cpus_accel->create_vcpu_thread_postcheck) {
         cpus_accel->create_vcpu_thread_postcheck(cpu);
diff --git a/target/i386/hax/hax-accel-ops.c b/target/i386/hax/hax-accel-ops.c
index 18114fe34d..2fc2a9b8a4 100644
--- a/target/i386/hax/hax-accel-ops.c
+++ b/target/i386/hax/hax-accel-ops.c
@@ -57,28 +57,12 @@ static void *hax_cpu_thread_fn(void *arg)
     return NULL;
 }
 
-static void hax_start_vcpu_thread(CPUState *cpu)
-{
-    char thread_name[VCPU_THREAD_NAME_SIZE];
-
-    cpu->thread = g_new0(QemuThread, 1);
-    cpu->halt_cond = g_new0(QemuCond, 1);
-    qemu_cond_init(cpu->halt_cond);
-
-    snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/HAX",
-             cpu->cpu_index);
-    qemu_thread_create(cpu->thread, thread_name, hax_cpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE);
-#ifdef _WIN32
-    cpu->hThread = qemu_thread_get_handle(cpu->thread);
-#endif
-}
-
 static void hax_accel_ops_class_init(ObjectClass *oc, void *data)
 {
     AccelOpsClass *ops = ACCEL_OPS_CLASS(oc);
 
-    ops->create_vcpu_thread = hax_start_vcpu_thread;
+    ops->thread_name = "HAX";
+    ops->vcpu_thread_fn = hax_cpu_thread_fn;
     ops->kick_vcpu_thread = hax_kick_vcpu_thread;
 
     ops->synchronize_post_reset = hax_cpu_synchronize_post_reset;
diff --git a/target/i386/nvmm/nvmm-accel-ops.c b/target/i386/nvmm/nvmm-accel-ops.c
index 6c46101ac1..a6dc73aa35 100644
--- a/target/i386/nvmm/nvmm-accel-ops.c
+++ b/target/i386/nvmm/nvmm-accel-ops.c
@@ -60,19 +60,6 @@ static void *qemu_nvmm_cpu_thread_fn(void *arg)
     return NULL;
 }
 
-static void nvmm_start_vcpu_thread(CPUState *cpu)
-{
-    char thread_name[VCPU_THREAD_NAME_SIZE];
-
-    cpu->thread = g_new0(QemuThread, 1);
-    cpu->halt_cond = g_new0(QemuCond, 1);
-    qemu_cond_init(cpu->halt_cond);
-    snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/NVMM",
-             cpu->cpu_index);
-    qemu_thread_create(cpu->thread, thread_name, qemu_nvmm_cpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE);
-}
-
 /*
  * Abort the call to run the virtual processor by another thread, and to
  * return the control to that thread.
@@ -87,7 +74,9 @@ static void nvmm_accel_ops_class_init(ObjectClass *oc, void *data)
 {
     AccelOpsClass *ops = ACCEL_OPS_CLASS(oc);
 
-    ops->create_vcpu_thread = nvmm_start_vcpu_thread;
+    ops->thread_name = "NVMM";
+    ops->vcpu_thread_fn = qemu_nvmm_cpu_thread_fn;
+
     ops->kick_vcpu_thread = nvmm_kick_vcpu_thread;
 
     ops->synchronize_post_reset = nvmm_cpu_synchronize_post_reset;
diff --git a/target/i386/whpx/whpx-accel-ops.c b/target/i386/whpx/whpx-accel-ops.c
index dd2a9f7657..6498eb2060 100644
--- a/target/i386/whpx/whpx-accel-ops.c
+++ b/target/i386/whpx/whpx-accel-ops.c
@@ -60,22 +60,6 @@ static void *whpx_cpu_thread_fn(void *arg)
     return NULL;
 }
 
-static void whpx_start_vcpu_thread(CPUState *cpu)
-{
-    char thread_name[VCPU_THREAD_NAME_SIZE];
-
-    cpu->thread = g_new0(QemuThread, 1);
-    cpu->halt_cond = g_new0(QemuCond, 1);
-    qemu_cond_init(cpu->halt_cond);
-    snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/WHPX",
-             cpu->cpu_index);
-    qemu_thread_create(cpu->thread, thread_name, whpx_cpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE);
-#ifdef _WIN32
-    cpu->hThread = qemu_thread_get_handle(cpu->thread);
-#endif
-}
-
 static void whpx_kick_vcpu_thread(CPUState *cpu)
 {
     if (!qemu_cpu_is_self(cpu)) {
@@ -92,7 +76,9 @@ static void whpx_accel_ops_class_init(ObjectClass *oc, void *data)
 {
     AccelOpsClass *ops = ACCEL_OPS_CLASS(oc);
 
-    ops->create_vcpu_thread = whpx_start_vcpu_thread;
+    ops->thread_name = "WHPX";
+    ops->vcpu_thread_fn = whpx_cpu_thread_fn;
+
     ops->kick_vcpu_thread = whpx_kick_vcpu_thread;
     ops->cpu_thread_is_idle = whpx_vcpu_thread_is_idle;
 
-- 
2.35.1

[PATCH v4 09/13] accel/all: Extract common_vcpu_thread_create()

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

All accelerators implement a very similar create_vcpu_thread()
handler. Extract the common part as common_vcpu_thread_create(),
which only requires the AccelOpsClass::vcpu_thread_fn() routine
and the accelerator AccelOpsClass::thread_name for debugging
purpose.

The single exception is TCG/RR which re-use a single vCPU. Have
it use the same logic by using the .precheck/.postcheck handlers.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 accel/accel-softmmu.c             |  2 +-
 accel/dummy-cpus.c                | 15 +--------------
 accel/hvf/hvf-accel-ops.c         | 18 +++---------------
 accel/kvm/kvm-accel-ops.c         | 17 +++--------------
 accel/qtest/qtest.c               |  3 ++-
 accel/tcg/tcg-accel-ops-mttcg.c   | 22 +---------------------
 accel/tcg/tcg-accel-ops-mttcg.h   |  3 +--
 accel/tcg/tcg-accel-ops-rr.c      | 21 +++------------------
 accel/tcg/tcg-accel-ops-rr.h      |  6 ++++--
 accel/tcg/tcg-accel-ops.c         |  6 ++++--
 accel/xen/xen-all.c               |  2 +-
 include/sysemu/accel-ops.h        |  3 ++-
 include/sysemu/cpus.h             |  4 ++--
 softmmu/cpus.c                    | 23 ++++++++++++++++++++---
 target/i386/hax/hax-accel-ops.c   | 20 ++------------------
 target/i386/nvmm/nvmm-accel-ops.c | 17 +++--------------
 target/i386/whpx/whpx-accel-ops.c | 20 +++-----------------
 17 files changed, 56 insertions(+), 146 deletions(-)

diff --git a/accel/accel-softmmu.c b/accel/accel-softmmu.c
index 67276e4f52..7800df0234 100644
--- a/accel/accel-softmmu.c
+++ b/accel/accel-softmmu.c
@@ -77,7 +77,7 @@ void accel_init_ops_interfaces(AccelClass *ac)
 
     /*
      * all accelerators need to define ops, providing at least a mandatory
-     * non-NULL create_vcpu_thread operation.
+     * non-NULL vcpu_thread_fn operation.
      */
     g_assert(ops != NULL);
     if (ops->ops_init) {
diff --git a/accel/dummy-cpus.c b/accel/dummy-cpus.c
index 10429fdfb2..9840057969 100644
--- a/accel/dummy-cpus.c
+++ b/accel/dummy-cpus.c
@@ -18,7 +18,7 @@
 #include "qemu/main-loop.h"
 #include "hw/core/cpu.h"
 
-static void *dummy_cpu_thread_fn(void *arg)
+void *dummy_vcpu_thread_fn(void *arg)
 {
     CPUState *cpu = arg;
     sigset_t waitset;
@@ -57,16 +57,3 @@ static void *dummy_cpu_thread_fn(void *arg)
     rcu_unregister_thread();
     return NULL;
 }
-
-void dummy_start_vcpu_thread(CPUState *cpu)
-{
-    char thread_name[VCPU_THREAD_NAME_SIZE];
-
-    cpu->thread = g_malloc0(sizeof(QemuThread));
-    cpu->halt_cond = g_malloc0(sizeof(QemuCond));
-    qemu_cond_init(cpu->halt_cond);
-    snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/DUMMY",
-             cpu->cpu_index);
-    qemu_thread_create(cpu->thread, thread_name, dummy_cpu_thread_fn, cpu,
-                       QEMU_THREAD_JOINABLE);
-}
diff --git a/accel/hvf/hvf-accel-ops.c b/accel/hvf/hvf-accel-ops.c
index 5c33dc602e..91d65036b4 100644
--- a/accel/hvf/hvf-accel-ops.c
+++ b/accel/hvf/hvf-accel-ops.c
@@ -442,25 +442,13 @@ static void *hvf_cpu_thread_fn(void *arg)
     return NULL;
 }
 
-static void hvf_start_vcpu_thread(CPUState *cpu)
-{
-    char thread_name[VCPU_THREAD_NAME_SIZE];
-
-    cpu->thread = g_malloc0(sizeof(QemuThread));
-    cpu->halt_cond = g_malloc0(sizeof(QemuCond));
-    qemu_cond_init(cpu->halt_cond);
-
-    snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/HVF",
-             cpu->cpu_index);
-    qemu_thread_create(cpu->thread, thread_name, hvf_cpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE);
-}
-
 static void hvf_accel_ops_class_init(ObjectClass *oc, void *data)
 {
     AccelOpsClass *ops = ACCEL_OPS_CLASS(oc);
 
-    ops->create_vcpu_thread = hvf_start_vcpu_thread;
+    ops->thread_name = "HVF";
+    ops->vcpu_thread_fn = hvf_cpu_thread_fn;
+
     ops->kick_vcpu_thread = hvf_kick_vcpu_thread;
 
     ops->synchronize_post_reset = hvf_cpu_synchronize_post_reset;
diff --git a/accel/kvm/kvm-accel-ops.c b/accel/kvm/kvm-accel-ops.c
index c4244a23c6..3d13efce0f 100644
--- a/accel/kvm/kvm-accel-ops.c
+++ b/accel/kvm/kvm-accel-ops.c
@@ -61,19 +61,6 @@ static void *kvm_vcpu_thread_fn(void *arg)
     return NULL;
 }
 
-static void kvm_start_vcpu_thread(CPUState *cpu)
-{
-    char thread_name[VCPU_THREAD_NAME_SIZE];
-
-    cpu->thread = g_malloc0(sizeof(QemuThread));
-    cpu->halt_cond = g_malloc0(sizeof(QemuCond));
-    qemu_cond_init(cpu->halt_cond);
-    snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/KVM",
-             cpu->cpu_index);
-    qemu_thread_create(cpu->thread, thread_name, kvm_vcpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE);
-}
-
 static bool kvm_vcpu_thread_is_idle(CPUState *cpu)
 {
     return !kvm_halt_in_kernel();
@@ -88,7 +75,9 @@ static void kvm_accel_ops_class_init(ObjectClass *oc, void *data)
 {
     AccelOpsClass *ops = ACCEL_OPS_CLASS(oc);
 
-    ops->create_vcpu_thread = kvm_start_vcpu_thread;
+    ops->thread_name = "KVM";
+    ops->vcpu_thread_fn = kvm_vcpu_thread_fn;
+
     ops->cpu_thread_is_idle = kvm_vcpu_thread_is_idle;
     ops->cpus_are_resettable = kvm_cpus_are_resettable;
     ops->synchronize_post_reset = kvm_cpu_synchronize_post_reset;
diff --git a/accel/qtest/qtest.c b/accel/qtest/qtest.c
index f6056ac836..1d0b1c855c 100644
--- a/accel/qtest/qtest.c
+++ b/accel/qtest/qtest.c
@@ -50,7 +50,8 @@ static void qtest_accel_ops_class_init(ObjectClass *oc, void *data)
 {
     AccelOpsClass *ops = ACCEL_OPS_CLASS(oc);
 
-    ops->create_vcpu_thread = dummy_start_vcpu_thread;
+    ops->vcpu_thread_fn = dummy_vcpu_thread_fn;
+
     ops->get_virtual_clock = qtest_get_virtual_clock;
 };
 
diff --git a/accel/tcg/tcg-accel-ops-mttcg.c b/accel/tcg/tcg-accel-ops-mttcg.c
index 80609964a6..c7836332d7 100644
--- a/accel/tcg/tcg-accel-ops-mttcg.c
+++ b/accel/tcg/tcg-accel-ops-mttcg.c
@@ -63,7 +63,7 @@ static void mttcg_force_rcu(Notifier *notify, void *data)
  * current CPUState for a given thread.
  */
 
-static void *mttcg_cpu_thread_fn(void *arg)
+void *mttcg_vcpu_thread_fn(void *arg)
 {
     MttcgForceRcuNotifier force_rcu;
     CPUState *cpu = arg;
@@ -137,23 +137,3 @@ void mttcg_kick_vcpu_thread(CPUState *cpu)
 {
     cpu_exit(cpu);
 }
-
-void mttcg_start_vcpu_thread(CPUState *cpu)
-{
-    char thread_name[VCPU_THREAD_NAME_SIZE];
-
-    cpu->thread = g_new0(QemuThread, 1);
-    cpu->halt_cond = g_malloc0(sizeof(QemuCond));
-    qemu_cond_init(cpu->halt_cond);
-
-    /* create a thread per vCPU with TCG (MTTCG) */
-    snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/TCG",
-             cpu->cpu_index);
-
-    qemu_thread_create(cpu->thread, thread_name, mttcg_cpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE);
-
-#ifdef _WIN32
-    cpu->hThread = qemu_thread_get_handle(cpu->thread);
-#endif
-}
diff --git a/accel/tcg/tcg-accel-ops-mttcg.h b/accel/tcg/tcg-accel-ops-mttcg.h
index 9fdc5a2ab5..b61aff5c1d 100644
--- a/accel/tcg/tcg-accel-ops-mttcg.h
+++ b/accel/tcg/tcg-accel-ops-mttcg.h
@@ -13,7 +13,6 @@
 /* kick MTTCG vCPU thread */
 void mttcg_kick_vcpu_thread(CPUState *cpu);
 
-/* start an mttcg vCPU thread */
-void mttcg_start_vcpu_thread(CPUState *cpu);
+void *mttcg_vcpu_thread_fn(void *arg);
 
 #endif /* TCG_CPUS_MTTCG_H */
diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c
index 3da684b8e6..006b787289 100644
--- a/accel/tcg/tcg-accel-ops-rr.c
+++ b/accel/tcg/tcg-accel-ops-rr.c
@@ -148,7 +148,7 @@ static void rr_force_rcu(Notifier *notify, void *data)
  * elsewhere.
  */
 
-static void *rr_cpu_thread_fn(void *arg)
+void *rr_vcpu_thread_fn(void *arg)
 {
     Notifier force_rcu;
     CPUState *cpu = arg;
@@ -279,28 +279,13 @@ bool rr_create_vcpu_thread_precheck(CPUState *cpu)
     return !single_tcg_cpu_thread;
 }
 
-void rr_start_vcpu_thread(CPUState *cpu)
+void rr_create_vcpu_thread_postcheck(CPUState *cpu)
 {
-    char thread_name[VCPU_THREAD_NAME_SIZE];
     static QemuCond *single_tcg_halt_cond;
-    static QemuThread *single_tcg_cpu_thread;
-
-    if (!single_tcg_cpu_thread) {
-        cpu->thread = g_new0(QemuThread, 1);
-        cpu->halt_cond = g_new0(QemuCond, 1);
-        qemu_cond_init(cpu->halt_cond);
-
-        /* share a single thread for all cpus with TCG */
-        snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "ALL CPUs/TCG");
-        qemu_thread_create(cpu->thread, thread_name,
-                           rr_cpu_thread_fn,
-                           cpu, QEMU_THREAD_JOINABLE);
 
+    if (! single_tcg_cpu_thread) {
         single_tcg_halt_cond = cpu->halt_cond;
         single_tcg_cpu_thread = cpu->thread;
-#ifdef _WIN32
-        cpu->hThread = qemu_thread_get_handle(cpu->thread);
-#endif
     } else {
         /* we share the thread */
         cpu->thread = single_tcg_cpu_thread;
diff --git a/accel/tcg/tcg-accel-ops-rr.h b/accel/tcg/tcg-accel-ops-rr.h
index e2273b66d4..a1e75e7afb 100644
--- a/accel/tcg/tcg-accel-ops-rr.h
+++ b/accel/tcg/tcg-accel-ops-rr.h
@@ -16,7 +16,9 @@
 void rr_kick_vcpu_thread(CPUState *unused);
 
 bool rr_create_vcpu_thread_precheck(CPUState *cpu);
-/* start the round robin vcpu thread */
-void rr_start_vcpu_thread(CPUState *cpu);
+void rr_create_vcpu_thread_postcheck(CPUState *cpu);
+bool rr_destroy_vcpu_thread_precheck(CPUState *cpu);
+
+void *rr_vcpu_thread_fn(void *arg);
 
 #endif /* TCG_CPUS_RR_H */
diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
index d2181ea1e5..127dd6fee5 100644
--- a/accel/tcg/tcg-accel-ops.c
+++ b/accel/tcg/tcg-accel-ops.c
@@ -95,11 +95,13 @@ void tcg_handle_interrupt(CPUState *cpu, int mask)
 static void tcg_accel_ops_init(AccelOpsClass *ops)
 {
     if (qemu_tcg_mttcg_enabled()) {
-        ops->create_vcpu_thread = mttcg_start_vcpu_thread;
+        ops->vcpu_thread_fn = mttcg_vcpu_thread_fn;
         ops->kick_vcpu_thread = mttcg_kick_vcpu_thread;
         ops->handle_interrupt = tcg_handle_interrupt;
     } else {
-        ops->create_vcpu_thread = rr_start_vcpu_thread;
+        ops->vcpu_thread_fn = rr_vcpu_thread_fn;
+        ops->create_vcpu_thread_precheck = rr_create_vcpu_thread_precheck;
+        ops->create_vcpu_thread_postcheck = rr_create_vcpu_thread_postcheck;
         ops->kick_vcpu_thread = rr_kick_vcpu_thread;
 
         if (icount_enabled()) {
diff --git a/accel/xen/xen-all.c b/accel/xen/xen-all.c
index 69aa7d018b..ef40f626e2 100644
--- a/accel/xen/xen-all.c
+++ b/accel/xen/xen-all.c
@@ -219,7 +219,7 @@ static void xen_accel_ops_class_init(ObjectClass *oc, void *data)
 {
     AccelOpsClass *ops = ACCEL_OPS_CLASS(oc);
 
-    ops->create_vcpu_thread = dummy_start_vcpu_thread;
+    ops->vcpu_thread_fn = dummy_vcpu_thread_fn;
 }
 
 static const TypeInfo xen_accel_ops_type = {
diff --git a/include/sysemu/accel-ops.h b/include/sysemu/accel-ops.h
index 26b542d35c..caf337f61f 100644
--- a/include/sysemu/accel-ops.h
+++ b/include/sysemu/accel-ops.h
@@ -30,7 +30,8 @@ struct AccelOpsClass {
 
     bool (*cpus_are_resettable)(void);
 
-    void (*create_vcpu_thread)(CPUState *cpu); /* MANDATORY NON-NULL */
+    const char *thread_name;
+    void *(*vcpu_thread_fn)(void *arg); /* MANDATORY NON-NULL */
     /* If non-NULL, return whether common vCPU thread must be created */
     bool (*create_vcpu_thread_precheck)(CPUState *cpu);
     void (*create_vcpu_thread_postcheck)(CPUState *cpu);
diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h
index b5c87d48b3..bf5629c58f 100644
--- a/include/sysemu/cpus.h
+++ b/include/sysemu/cpus.h
@@ -9,8 +9,8 @@ void cpus_register_accel(const AccelOpsClass *i);
 
 /* accel/dummy-cpus.c */
 
-/* Create a dummy vcpu for AccelOpsClass->create_vcpu_thread */
-void dummy_start_vcpu_thread(CPUState *);
+/* Create a dummy vcpu for AccelOpsClass->vcpu_thread_fn */
+void *dummy_vcpu_thread_fn(void *arg);
 
 /* interface available for cpus accelerator threads */
 
diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index 857e2081ba..cf430ac486 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -601,6 +601,22 @@ void resume_all_vcpus(void)
     }
 }
 
+static void common_vcpu_thread_create(CPUState *cpu)
+{
+    char thread_name[VCPU_THREAD_NAME_SIZE];
+
+    cpu->thread = g_new0(QemuThread, 1);
+    cpu->halt_cond = g_new0(QemuCond, 1);
+    qemu_cond_init(cpu->halt_cond);
+    snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/%s",
+             cpu->cpu_index, cpus_accel->thread_name ?: "DUMMY");
+    qemu_thread_create(cpu->thread, thread_name, cpus_accel->vcpu_thread_fn,
+                       cpu, QEMU_THREAD_JOINABLE);
+#ifdef _WIN32
+    cpu->hThread = qemu_thread_get_handle(cpu->thread);
+#endif
+}
+
 void cpu_remove_sync(CPUState *cpu)
 {
     cpu->stop = true;
@@ -614,7 +630,7 @@ void cpu_remove_sync(CPUState *cpu)
 void cpus_register_accel(const AccelOpsClass *ops)
 {
     assert(ops != NULL);
-    assert(ops->create_vcpu_thread != NULL); /* mandatory */
+    assert(ops->vcpu_thread_fn != NULL); /* mandatory */
     cpus_accel = ops;
 }
 
@@ -636,10 +652,11 @@ void qemu_init_vcpu(CPUState *cpu)
     }
 
     /* accelerators all implement the AccelOpsClass */
-    g_assert(cpus_accel != NULL && cpus_accel->create_vcpu_thread != NULL);
+    g_assert(cpus_accel != NULL && cpus_accel->vcpu_thread_fn != NULL);
+
     if (cpus_accel->create_vcpu_thread_precheck == NULL
             || cpus_accel->create_vcpu_thread_precheck(cpu)) {
-        cpus_accel->create_vcpu_thread(cpu);
+        common_vcpu_thread_create(cpu);
     }
     if (cpus_accel->create_vcpu_thread_postcheck) {
         cpus_accel->create_vcpu_thread_postcheck(cpu);
diff --git a/target/i386/hax/hax-accel-ops.c b/target/i386/hax/hax-accel-ops.c
index 18114fe34d..2fc2a9b8a4 100644
--- a/target/i386/hax/hax-accel-ops.c
+++ b/target/i386/hax/hax-accel-ops.c
@@ -57,28 +57,12 @@ static void *hax_cpu_thread_fn(void *arg)
     return NULL;
 }
 
-static void hax_start_vcpu_thread(CPUState *cpu)
-{
-    char thread_name[VCPU_THREAD_NAME_SIZE];
-
-    cpu->thread = g_new0(QemuThread, 1);
-    cpu->halt_cond = g_new0(QemuCond, 1);
-    qemu_cond_init(cpu->halt_cond);
-
-    snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/HAX",
-             cpu->cpu_index);
-    qemu_thread_create(cpu->thread, thread_name, hax_cpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE);
-#ifdef _WIN32
-    cpu->hThread = qemu_thread_get_handle(cpu->thread);
-#endif
-}
-
 static void hax_accel_ops_class_init(ObjectClass *oc, void *data)
 {
     AccelOpsClass *ops = ACCEL_OPS_CLASS(oc);
 
-    ops->create_vcpu_thread = hax_start_vcpu_thread;
+    ops->thread_name = "HAX";
+    ops->vcpu_thread_fn = hax_cpu_thread_fn;
     ops->kick_vcpu_thread = hax_kick_vcpu_thread;
 
     ops->synchronize_post_reset = hax_cpu_synchronize_post_reset;
diff --git a/target/i386/nvmm/nvmm-accel-ops.c b/target/i386/nvmm/nvmm-accel-ops.c
index 6c46101ac1..a6dc73aa35 100644
--- a/target/i386/nvmm/nvmm-accel-ops.c
+++ b/target/i386/nvmm/nvmm-accel-ops.c
@@ -60,19 +60,6 @@ static void *qemu_nvmm_cpu_thread_fn(void *arg)
     return NULL;
 }
 
-static void nvmm_start_vcpu_thread(CPUState *cpu)
-{
-    char thread_name[VCPU_THREAD_NAME_SIZE];
-
-    cpu->thread = g_new0(QemuThread, 1);
-    cpu->halt_cond = g_new0(QemuCond, 1);
-    qemu_cond_init(cpu->halt_cond);
-    snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/NVMM",
-             cpu->cpu_index);
-    qemu_thread_create(cpu->thread, thread_name, qemu_nvmm_cpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE);
-}
-
 /*
  * Abort the call to run the virtual processor by another thread, and to
  * return the control to that thread.
@@ -87,7 +74,9 @@ static void nvmm_accel_ops_class_init(ObjectClass *oc, void *data)
 {
     AccelOpsClass *ops = ACCEL_OPS_CLASS(oc);
 
-    ops->create_vcpu_thread = nvmm_start_vcpu_thread;
+    ops->thread_name = "NVMM";
+    ops->vcpu_thread_fn = qemu_nvmm_cpu_thread_fn;
+
     ops->kick_vcpu_thread = nvmm_kick_vcpu_thread;
 
     ops->synchronize_post_reset = nvmm_cpu_synchronize_post_reset;
diff --git a/target/i386/whpx/whpx-accel-ops.c b/target/i386/whpx/whpx-accel-ops.c
index dd2a9f7657..6498eb2060 100644
--- a/target/i386/whpx/whpx-accel-ops.c
+++ b/target/i386/whpx/whpx-accel-ops.c
@@ -60,22 +60,6 @@ static void *whpx_cpu_thread_fn(void *arg)
     return NULL;
 }
 
-static void whpx_start_vcpu_thread(CPUState *cpu)
-{
-    char thread_name[VCPU_THREAD_NAME_SIZE];
-
-    cpu->thread = g_new0(QemuThread, 1);
-    cpu->halt_cond = g_new0(QemuCond, 1);
-    qemu_cond_init(cpu->halt_cond);
-    snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/WHPX",
-             cpu->cpu_index);
-    qemu_thread_create(cpu->thread, thread_name, whpx_cpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE);
-#ifdef _WIN32
-    cpu->hThread = qemu_thread_get_handle(cpu->thread);
-#endif
-}
-
 static void whpx_kick_vcpu_thread(CPUState *cpu)
 {
     if (!qemu_cpu_is_self(cpu)) {
@@ -92,7 +76,9 @@ static void whpx_accel_ops_class_init(ObjectClass *oc, void *data)
 {
     AccelOpsClass *ops = ACCEL_OPS_CLASS(oc);
 
-    ops->create_vcpu_thread = whpx_start_vcpu_thread;
+    ops->thread_name = "WHPX";
+    ops->vcpu_thread_fn = whpx_cpu_thread_fn;
+
     ops->kick_vcpu_thread = whpx_kick_vcpu_thread;
     ops->cpu_thread_is_idle = whpx_vcpu_thread_is_idle;
 
-- 
2.35.1

[PATCH v4 10/13] accel-ops: Introduce common_vcpu_thread_destroy() and .precheck handler

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Introduce an empty common_vcpu_thread_destroy() function, and
provide a AccelOpsClass::destroy_vcpu_thread_precheck() callback
so accelerators can choose whether to call common_vcpu_thread_destroy.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/sysemu/accel-ops.h | 2 ++
 softmmu/cpus.c             | 9 +++++++++
 2 files changed, 11 insertions(+)

diff --git a/include/sysemu/accel-ops.h b/include/sysemu/accel-ops.h
index caf337f61f..b47f6de3f9 100644
--- a/include/sysemu/accel-ops.h
+++ b/include/sysemu/accel-ops.h
@@ -35,6 +35,8 @@ struct AccelOpsClass {
     /* If non-NULL, return whether common vCPU thread must be created */
     bool (*create_vcpu_thread_precheck)(CPUState *cpu);
     void (*create_vcpu_thread_postcheck)(CPUState *cpu);
+    /* If non-NULL, return whether common vCPU thread must be destroyed */
+    bool (*destroy_vcpu_thread_precheck)(CPUState *cpu);
 
     void (*kick_vcpu_thread)(CPUState *cpu);
     bool (*cpu_thread_is_idle)(CPUState *cpu);
diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index cf430ac486..37325b3b8d 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -617,6 +617,10 @@ static void common_vcpu_thread_create(CPUState *cpu)
 #endif
 }
 
+static void common_vcpu_thread_destroy(CPUState *cpu)
+{
+}
+
 void cpu_remove_sync(CPUState *cpu)
 {
     cpu->stop = true;
@@ -625,6 +629,11 @@ void cpu_remove_sync(CPUState *cpu)
     qemu_mutex_unlock_iothread();
     qemu_thread_join(cpu->thread);
     qemu_mutex_lock_iothread();
+
+    if (cpus_accel->destroy_vcpu_thread_precheck == NULL
+            || cpus_accel->destroy_vcpu_thread_precheck(cpu)) {
+        common_vcpu_thread_destroy(cpu);
+    }
 }
 
 void cpus_register_accel(const AccelOpsClass *ops)
-- 
2.35.1

[PATCH v4 10/13] accel-ops: Introduce common_vcpu_thread_destroy() and .precheck handler

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Introduce an empty common_vcpu_thread_destroy() function, and
provide a AccelOpsClass::destroy_vcpu_thread_precheck() callback
so accelerators can choose whether to call common_vcpu_thread_destroy.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/sysemu/accel-ops.h | 2 ++
 softmmu/cpus.c             | 9 +++++++++
 2 files changed, 11 insertions(+)

diff --git a/include/sysemu/accel-ops.h b/include/sysemu/accel-ops.h
index caf337f61f..b47f6de3f9 100644
--- a/include/sysemu/accel-ops.h
+++ b/include/sysemu/accel-ops.h
@@ -35,6 +35,8 @@ struct AccelOpsClass {
     /* If non-NULL, return whether common vCPU thread must be created */
     bool (*create_vcpu_thread_precheck)(CPUState *cpu);
     void (*create_vcpu_thread_postcheck)(CPUState *cpu);
+    /* If non-NULL, return whether common vCPU thread must be destroyed */
+    bool (*destroy_vcpu_thread_precheck)(CPUState *cpu);
 
     void (*kick_vcpu_thread)(CPUState *cpu);
     bool (*cpu_thread_is_idle)(CPUState *cpu);
diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index cf430ac486..37325b3b8d 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -617,6 +617,10 @@ static void common_vcpu_thread_create(CPUState *cpu)
 #endif
 }
 
+static void common_vcpu_thread_destroy(CPUState *cpu)
+{
+}
+
 void cpu_remove_sync(CPUState *cpu)
 {
     cpu->stop = true;
@@ -625,6 +629,11 @@ void cpu_remove_sync(CPUState *cpu)
     qemu_mutex_unlock_iothread();
     qemu_thread_join(cpu->thread);
     qemu_mutex_lock_iothread();
+
+    if (cpus_accel->destroy_vcpu_thread_precheck == NULL
+            || cpus_accel->destroy_vcpu_thread_precheck(cpu)) {
+        common_vcpu_thread_destroy(cpu);
+    }
 }
 
 void cpus_register_accel(const AccelOpsClass *ops)
-- 
2.35.1

[PATCH v4 11/13] accel/tcg: Add rr_destroy_vcpu_thread_precheck()

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

TCG/RR is special and creates a single vCPU. It only have
to release its resources once. Implement the .precheck()
for that purpose.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 accel/tcg/tcg-accel-ops-rr.c | 9 +++++++++
 accel/tcg/tcg-accel-ops.c    | 1 +
 2 files changed, 10 insertions(+)

diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c
index 006b787289..6fe8e20356 100644
--- a/accel/tcg/tcg-accel-ops-rr.c
+++ b/accel/tcg/tcg-accel-ops-rr.c
@@ -295,3 +295,12 @@ void rr_create_vcpu_thread_postcheck(CPUState *cpu)
         cpu->created = true;
     }
 }
+
+bool rr_destroy_vcpu_thread_precheck(CPUState *cpu)
+{
+    if (single_tcg_cpu_thread) {
+        single_tcg_cpu_thread = NULL;
+        return true;
+    }
+    return false;
+}
diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
index 127dd6fee5..0b0dbcc47a 100644
--- a/accel/tcg/tcg-accel-ops.c
+++ b/accel/tcg/tcg-accel-ops.c
@@ -102,6 +102,7 @@ static void tcg_accel_ops_init(AccelOpsClass *ops)
         ops->vcpu_thread_fn = rr_vcpu_thread_fn;
         ops->create_vcpu_thread_precheck = rr_create_vcpu_thread_precheck;
         ops->create_vcpu_thread_postcheck = rr_create_vcpu_thread_postcheck;
+        ops->destroy_vcpu_thread_precheck = rr_destroy_vcpu_thread_precheck;
         ops->kick_vcpu_thread = rr_kick_vcpu_thread;
 
         if (icount_enabled()) {
-- 
2.35.1

[PATCH v4 11/13] accel/tcg: Add rr_destroy_vcpu_thread_precheck()

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

TCG/RR is special and creates a single vCPU. It only have
to release its resources once. Implement the .precheck()
for that purpose.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 accel/tcg/tcg-accel-ops-rr.c | 9 +++++++++
 accel/tcg/tcg-accel-ops.c    | 1 +
 2 files changed, 10 insertions(+)

diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c
index 006b787289..6fe8e20356 100644
--- a/accel/tcg/tcg-accel-ops-rr.c
+++ b/accel/tcg/tcg-accel-ops-rr.c
@@ -295,3 +295,12 @@ void rr_create_vcpu_thread_postcheck(CPUState *cpu)
         cpu->created = true;
     }
 }
+
+bool rr_destroy_vcpu_thread_precheck(CPUState *cpu)
+{
+    if (single_tcg_cpu_thread) {
+        single_tcg_cpu_thread = NULL;
+        return true;
+    }
+    return false;
+}
diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
index 127dd6fee5..0b0dbcc47a 100644
--- a/accel/tcg/tcg-accel-ops.c
+++ b/accel/tcg/tcg-accel-ops.c
@@ -102,6 +102,7 @@ static void tcg_accel_ops_init(AccelOpsClass *ops)
         ops->vcpu_thread_fn = rr_vcpu_thread_fn;
         ops->create_vcpu_thread_precheck = rr_create_vcpu_thread_precheck;
         ops->create_vcpu_thread_postcheck = rr_create_vcpu_thread_postcheck;
+        ops->destroy_vcpu_thread_precheck = rr_destroy_vcpu_thread_precheck;
         ops->kick_vcpu_thread = rr_kick_vcpu_thread;
 
         if (icount_enabled()) {
-- 
2.35.1

[PATCH v4 12/13] softmmu/cpus: Free cpu->thread in generic_destroy_vcpu_thread()

[Philippe Mathieu-Daudé]

From: Mark Kanda <mark.kanda@oracle.com>

Free cpu->thread in a new AccelOpsClass::destroy_vcpu_thread() handler
generic_destroy_vcpu_thread().

vCPU hotunplug related leak reported by Valgrind:

  ==102631== 8 bytes in 1 blocks are definitely lost in loss record 1,037 of 8,555
  ==102631==    at 0x4C3ADBB: calloc (vg_replace_malloc.c:1117)
  ==102631==    by 0x69EE4CD: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.5600.4)
  ==102631==    by 0x92443A: kvm_start_vcpu_thread (kvm-accel-ops.c:68)
  ==102631==    by 0x4505C2: qemu_init_vcpu (cpus.c:643)
  ==102631==    by 0x76B4D1: x86_cpu_realizefn (cpu.c:6520)
  ==102631==    by 0x9344A7: device_set_realized (qdev.c:531)
  ==102631==    by 0x93E329: property_set_bool (object.c:2273)
  ==102631==    by 0x93C2F8: object_property_set (object.c:1408)
  ==102631==    by 0x940796: object_property_set_qobject (qom-qobject.c:28)
  ==102631==    by 0x93C663: object_property_set_bool (object.c:1477)
  ==102631==    by 0x933D3B: qdev_realize (qdev.c:333)
  ==102631==    by 0x455EC4: qdev_device_add_from_qdict (qdev-monitor.c:713)

Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
Message-Id: <20220321141409.3112932-3-mark.kanda@oracle.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 softmmu/cpus.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index 37325b3b8d..efa8397f04 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -619,6 +619,7 @@ static void common_vcpu_thread_create(CPUState *cpu)
 
 static void common_vcpu_thread_destroy(CPUState *cpu)
 {
+    g_free(cpu->thread);
 }
 
 void cpu_remove_sync(CPUState *cpu)
-- 
2.35.1

[PATCH v4 12/13] softmmu/cpus: Free cpu->thread in generic_destroy_vcpu_thread()

[Philippe Mathieu-Daudé]

From: Mark Kanda <mark.kanda@oracle.com>

Free cpu->thread in a new AccelOpsClass::destroy_vcpu_thread() handler
generic_destroy_vcpu_thread().

vCPU hotunplug related leak reported by Valgrind:

  ==102631== 8 bytes in 1 blocks are definitely lost in loss record 1,037 of 8,555
  ==102631==    at 0x4C3ADBB: calloc (vg_replace_malloc.c:1117)
  ==102631==    by 0x69EE4CD: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.5600.4)
  ==102631==    by 0x92443A: kvm_start_vcpu_thread (kvm-accel-ops.c:68)
  ==102631==    by 0x4505C2: qemu_init_vcpu (cpus.c:643)
  ==102631==    by 0x76B4D1: x86_cpu_realizefn (cpu.c:6520)
  ==102631==    by 0x9344A7: device_set_realized (qdev.c:531)
  ==102631==    by 0x93E329: property_set_bool (object.c:2273)
  ==102631==    by 0x93C2F8: object_property_set (object.c:1408)
  ==102631==    by 0x940796: object_property_set_qobject (qom-qobject.c:28)
  ==102631==    by 0x93C663: object_property_set_bool (object.c:1477)
  ==102631==    by 0x933D3B: qdev_realize (qdev.c:333)
  ==102631==    by 0x455EC4: qdev_device_add_from_qdict (qdev-monitor.c:713)

Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
Message-Id: <20220321141409.3112932-3-mark.kanda@oracle.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 softmmu/cpus.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index 37325b3b8d..efa8397f04 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -619,6 +619,7 @@ static void common_vcpu_thread_create(CPUState *cpu)
 
 static void common_vcpu_thread_destroy(CPUState *cpu)
 {
+    g_free(cpu->thread);
 }
 
 void cpu_remove_sync(CPUState *cpu)
-- 
2.35.1

[PATCH v4 13/13] softmmu/cpus: Free cpu->halt_cond in generic_destroy_vcpu_thread()

[Philippe Mathieu-Daudé]

From: Mark Kanda <mark.kanda@oracle.com>

vCPU hotunplug related leak reported by Valgrind:

  ==102631== 56 bytes in 1 blocks are definitely lost in loss record 5,089 of 8,555
  ==102631==    at 0x4C3ADBB: calloc (vg_replace_malloc.c:1117)
  ==102631==    by 0x69EE4CD: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.5600.4)
  ==102631==    by 0x924452: kvm_start_vcpu_thread (kvm-accel-ops.c:69)
  ==102631==    by 0x4505C2: qemu_init_vcpu (cpus.c:643)
  ==102631==    by 0x76B4D1: x86_cpu_realizefn (cpu.c:6520)
  ==102631==    by 0x9344A7: device_set_realized (qdev.c:531)
  ==102631==    by 0x93E329: property_set_bool (object.c:2273)
  ==102631==    by 0x93C2F8: object_property_set (object.c:1408)
  ==102631==    by 0x940796: object_property_set_qobject (qom-qobject.c:28)
  ==102631==    by 0x93C663: object_property_set_bool (object.c:1477)
  ==102631==    by 0x933D3B: qdev_realize (qdev.c:333)
  ==102631==    by 0x455EC4: qdev_device_add_from_qdict (qdev-monitor.c:713)

Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
Message-Id: <20220321141409.3112932-4-mark.kanda@oracle.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 softmmu/cpus.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index efa8397f04..23bed29545 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -620,6 +620,7 @@ static void common_vcpu_thread_create(CPUState *cpu)
 static void common_vcpu_thread_destroy(CPUState *cpu)
 {
     g_free(cpu->thread);
+    g_free(cpu->halt_cond);
 }
 
 void cpu_remove_sync(CPUState *cpu)
-- 
2.35.1

[PATCH v4 13/13] softmmu/cpus: Free cpu->halt_cond in generic_destroy_vcpu_thread()

[Philippe Mathieu-Daudé]

From: Mark Kanda <mark.kanda@oracle.com>

vCPU hotunplug related leak reported by Valgrind:

  ==102631== 56 bytes in 1 blocks are definitely lost in loss record 5,089 of 8,555
  ==102631==    at 0x4C3ADBB: calloc (vg_replace_malloc.c:1117)
  ==102631==    by 0x69EE4CD: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.5600.4)
  ==102631==    by 0x924452: kvm_start_vcpu_thread (kvm-accel-ops.c:69)
  ==102631==    by 0x4505C2: qemu_init_vcpu (cpus.c:643)
  ==102631==    by 0x76B4D1: x86_cpu_realizefn (cpu.c:6520)
  ==102631==    by 0x9344A7: device_set_realized (qdev.c:531)
  ==102631==    by 0x93E329: property_set_bool (object.c:2273)
  ==102631==    by 0x93C2F8: object_property_set (object.c:1408)
  ==102631==    by 0x940796: object_property_set_qobject (qom-qobject.c:28)
  ==102631==    by 0x93C663: object_property_set_bool (object.c:1477)
  ==102631==    by 0x933D3B: qdev_realize (qdev.c:333)
  ==102631==    by 0x455EC4: qdev_device_add_from_qdict (qdev-monitor.c:713)

Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
Message-Id: <20220321141409.3112932-4-mark.kanda@oracle.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 softmmu/cpus.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index efa8397f04..23bed29545 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -620,6 +620,7 @@ static void common_vcpu_thread_create(CPUState *cpu)
 static void common_vcpu_thread_destroy(CPUState *cpu)
 {
     g_free(cpu->thread);
+    g_free(cpu->halt_cond);
 }
 
 void cpu_remove_sync(CPUState *cpu)
-- 
2.35.1

Re: [PATCH v4 12/13] softmmu/cpus: Free cpu->thread in generic_destroy_vcpu_thread()

[Mark Kanda]

Thanks Philippe,

In the patch subject, 'generic_destroy_vcpu_thread()' should be changed to 
'common_vcpu_thread_destroy()'.
Same goes for the next patch (Free cpu->halt_cond).

Thanks/regards,
-Mark

On 3/23/2022 12:17 PM, Philippe Mathieu-Daudé wrote:
> From: Mark Kanda <mark.kanda@oracle.com>
>
> Free cpu->thread in a new AccelOpsClass::destroy_vcpu_thread() handler
> generic_destroy_vcpu_thread().
>
> vCPU hotunplug related leak reported by Valgrind:
>
>    ==102631== 8 bytes in 1 blocks are definitely lost in loss record 1,037 of 8,555
>    ==102631==    at 0x4C3ADBB: calloc (vg_replace_malloc.c:1117)
>    ==102631==    by 0x69EE4CD: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.5600.4)
>    ==102631==    by 0x92443A: kvm_start_vcpu_thread (kvm-accel-ops.c:68)
>    ==102631==    by 0x4505C2: qemu_init_vcpu (cpus.c:643)
>    ==102631==    by 0x76B4D1: x86_cpu_realizefn (cpu.c:6520)
>    ==102631==    by 0x9344A7: device_set_realized (qdev.c:531)
>    ==102631==    by 0x93E329: property_set_bool (object.c:2273)
>    ==102631==    by 0x93C2F8: object_property_set (object.c:1408)
>    ==102631==    by 0x940796: object_property_set_qobject (qom-qobject.c:28)
>    ==102631==    by 0x93C663: object_property_set_bool (object.c:1477)
>    ==102631==    by 0x933D3B: qdev_realize (qdev.c:333)
>    ==102631==    by 0x455EC4: qdev_device_add_from_qdict (qdev-monitor.c:713)
>
> Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
> Message-Id: <20220321141409.3112932-3-mark.kanda@oracle.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   softmmu/cpus.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
> index 37325b3b8d..efa8397f04 100644
> --- a/softmmu/cpus.c
> +++ b/softmmu/cpus.c
> @@ -619,6 +619,7 @@ static void common_vcpu_thread_create(CPUState *cpu)
>   
>   static void common_vcpu_thread_destroy(CPUState *cpu)
>   {
> +    g_free(cpu->thread);
>   }
>   
>   void cpu_remove_sync(CPUState *cpu)

Re: [PATCH v4 12/13] softmmu/cpus: Free cpu->thread in generic_destroy_vcpu_thread()

[Mark Kanda]

Thanks Philippe,

In the patch subject, 'generic_destroy_vcpu_thread()' should be changed to 
'common_vcpu_thread_destroy()'.
Same goes for the next patch (Free cpu->halt_cond).

Thanks/regards,
-Mark

On 3/23/2022 12:17 PM, Philippe Mathieu-Daudé wrote:
> From: Mark Kanda <mark.kanda@oracle.com>
>
> Free cpu->thread in a new AccelOpsClass::destroy_vcpu_thread() handler
> generic_destroy_vcpu_thread().
>
> vCPU hotunplug related leak reported by Valgrind:
>
>    ==102631== 8 bytes in 1 blocks are definitely lost in loss record 1,037 of 8,555
>    ==102631==    at 0x4C3ADBB: calloc (vg_replace_malloc.c:1117)
>    ==102631==    by 0x69EE4CD: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.5600.4)
>    ==102631==    by 0x92443A: kvm_start_vcpu_thread (kvm-accel-ops.c:68)
>    ==102631==    by 0x4505C2: qemu_init_vcpu (cpus.c:643)
>    ==102631==    by 0x76B4D1: x86_cpu_realizefn (cpu.c:6520)
>    ==102631==    by 0x9344A7: device_set_realized (qdev.c:531)
>    ==102631==    by 0x93E329: property_set_bool (object.c:2273)
>    ==102631==    by 0x93C2F8: object_property_set (object.c:1408)
>    ==102631==    by 0x940796: object_property_set_qobject (qom-qobject.c:28)
>    ==102631==    by 0x93C663: object_property_set_bool (object.c:1477)
>    ==102631==    by 0x933D3B: qdev_realize (qdev.c:333)
>    ==102631==    by 0x455EC4: qdev_device_add_from_qdict (qdev-monitor.c:713)
>
> Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
> Message-Id: <20220321141409.3112932-3-mark.kanda@oracle.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   softmmu/cpus.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
> index 37325b3b8d..efa8397f04 100644
> --- a/softmmu/cpus.c
> +++ b/softmmu/cpus.c
> @@ -619,6 +619,7 @@ static void common_vcpu_thread_create(CPUState *cpu)
>   
>   static void common_vcpu_thread_destroy(CPUState *cpu)
>   {
> +    g_free(cpu->thread);
>   }
>   
>   void cpu_remove_sync(CPUState *cpu)

Re: [PATCH v4 01/13] cpu: Free cpu->cpu_ases in cpu_address_space_destroy()

[Philippe Mathieu-Daudé]

On 23/3/22 18:17, Philippe Mathieu-Daudé wrote:
> From: Mark Kanda <mark.kanda@oracle.com>
> 
> Create cpu_address_space_destroy() to free a CPU's cpu_ases list.

This seems incorrect...

> vCPU hotunplug related leak reported by Valgrind:
> 
> ==132362== 216 bytes in 1 blocks are definitely lost in loss record 7,119 of 8,549
> ==132362==    at 0x4C3ADBB: calloc (vg_replace_malloc.c:1117)
> ==132362==    by 0x69EE4CD: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.5600.4)
> ==132362==    by 0x7E34AF: cpu_address_space_init (physmem.c:751)
> ==132362==    by 0x45053E: qemu_init_vcpu (cpus.c:635)
> ==132362==    by 0x76B4A7: x86_cpu_realizefn (cpu.c:6520)
> ==132362==    by 0x9343ED: device_set_realized (qdev.c:531)
> ==132362==    by 0x93E26F: property_set_bool (object.c:2273)
> ==132362==    by 0x93C23E: object_property_set (object.c:1408)
> ==132362==    by 0x9406DC: object_property_set_qobject (qom-qobject.c:28)
> ==132362==    by 0x93C5A9: object_property_set_bool (object.c:1477)
> ==132362==    by 0x933C81: qdev_realize (qdev.c:333)
> ==132362==    by 0x455E9A: qdev_device_add_from_qdict (qdev-monitor.c:713)
> 
> Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Message-Id: <20220321141409.3112932-5-mark.kanda@oracle.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   cpu.c                     | 1 +
>   include/exec/cpu-common.h | 7 +++++++
>   softmmu/physmem.c         | 5 +++++
>   3 files changed, 13 insertions(+)
> 
> diff --git a/cpu.c b/cpu.c
> index be1f8b074c..59352a1487 100644
> --- a/cpu.c
> +++ b/cpu.c
> @@ -174,6 +174,7 @@ void cpu_exec_unrealizefn(CPUState *cpu)
>           tcg_exec_unrealizefn(cpu);
>       }
>   
> +    cpu_address_space_destroy(cpu);
>       cpu_list_remove(cpu);
>   }
>   
> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
> index 50a7d2912e..b17ad61ae4 100644
> --- a/include/exec/cpu-common.h
> +++ b/include/exec/cpu-common.h
> @@ -111,6 +111,13 @@ size_t qemu_ram_pagesize_largest(void);
>    */
>   void cpu_address_space_init(CPUState *cpu, int asidx,
>                               const char *prefix, MemoryRegion *mr);

... cpu_address_space_init() creates a single AS, ...

> +/**
> + * cpu_address_space_destroy:
> + * @cpu: CPU for this address space
> + *
> + * Cleanup CPU's cpu_ases list.
> + */
> +void cpu_address_space_destroy(CPUState *cpu);
>   
>   void cpu_physical_memory_rw(hwaddr addr, void *buf,
>                               hwaddr len, bool is_write);
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index 43ae70fbe2..aec61ca07a 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -762,6 +762,11 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
>       }
>   }
>   
> +void cpu_address_space_destroy(CPUState *cpu)
> +{
> +    g_free(cpu->cpu_ases);

... but here you destroy all the ASes.

> +}
> +
>   AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)
>   {
>       /* Return the AddressSpace corresponding to the specified index */

Re: [PATCH v4 01/13] cpu: Free cpu->cpu_ases in cpu_address_space_destroy()

[Philippe Mathieu-Daudé]

On 23/3/22 18:17, Philippe Mathieu-Daudé wrote:
> From: Mark Kanda <mark.kanda@oracle.com>
> 
> Create cpu_address_space_destroy() to free a CPU's cpu_ases list.

This seems incorrect...

> vCPU hotunplug related leak reported by Valgrind:
> 
> ==132362== 216 bytes in 1 blocks are definitely lost in loss record 7,119 of 8,549
> ==132362==    at 0x4C3ADBB: calloc (vg_replace_malloc.c:1117)
> ==132362==    by 0x69EE4CD: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.5600.4)
> ==132362==    by 0x7E34AF: cpu_address_space_init (physmem.c:751)
> ==132362==    by 0x45053E: qemu_init_vcpu (cpus.c:635)
> ==132362==    by 0x76B4A7: x86_cpu_realizefn (cpu.c:6520)
> ==132362==    by 0x9343ED: device_set_realized (qdev.c:531)
> ==132362==    by 0x93E26F: property_set_bool (object.c:2273)
> ==132362==    by 0x93C23E: object_property_set (object.c:1408)
> ==132362==    by 0x9406DC: object_property_set_qobject (qom-qobject.c:28)
> ==132362==    by 0x93C5A9: object_property_set_bool (object.c:1477)
> ==132362==    by 0x933C81: qdev_realize (qdev.c:333)
> ==132362==    by 0x455E9A: qdev_device_add_from_qdict (qdev-monitor.c:713)
> 
> Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Message-Id: <20220321141409.3112932-5-mark.kanda@oracle.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   cpu.c                     | 1 +
>   include/exec/cpu-common.h | 7 +++++++
>   softmmu/physmem.c         | 5 +++++
>   3 files changed, 13 insertions(+)
> 
> diff --git a/cpu.c b/cpu.c
> index be1f8b074c..59352a1487 100644
> --- a/cpu.c
> +++ b/cpu.c
> @@ -174,6 +174,7 @@ void cpu_exec_unrealizefn(CPUState *cpu)
>           tcg_exec_unrealizefn(cpu);
>       }
>   
> +    cpu_address_space_destroy(cpu);
>       cpu_list_remove(cpu);
>   }
>   
> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
> index 50a7d2912e..b17ad61ae4 100644
> --- a/include/exec/cpu-common.h
> +++ b/include/exec/cpu-common.h
> @@ -111,6 +111,13 @@ size_t qemu_ram_pagesize_largest(void);
>    */
>   void cpu_address_space_init(CPUState *cpu, int asidx,
>                               const char *prefix, MemoryRegion *mr);

... cpu_address_space_init() creates a single AS, ...

> +/**
> + * cpu_address_space_destroy:
> + * @cpu: CPU for this address space
> + *
> + * Cleanup CPU's cpu_ases list.
> + */
> +void cpu_address_space_destroy(CPUState *cpu);
>   
>   void cpu_physical_memory_rw(hwaddr addr, void *buf,
>                               hwaddr len, bool is_write);
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index 43ae70fbe2..aec61ca07a 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -762,6 +762,11 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
>       }
>   }
>   
> +void cpu_address_space_destroy(CPUState *cpu)
> +{
> +    g_free(cpu->cpu_ases);

... but here you destroy all the ASes.

> +}
> +
>   AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)
>   {
>       /* Return the AddressSpace corresponding to the specified index */

Re: [PATCH v4 01/13] cpu: Free cpu->cpu_ases in cpu_address_space_destroy()

[Mark Kanda]

On 3/23/2022 1:56 PM, Philippe Mathieu-Daudé wrote:
> On 23/3/22 18:17, Philippe Mathieu-Daudé wrote:
>> From: Mark Kanda <mark.kanda@oracle.com>
>>
>> Create cpu_address_space_destroy() to free a CPU's cpu_ases list.
>
> This seems incorrect...
>
>> vCPU hotunplug related leak reported by Valgrind:
>>
>> ==132362== 216 bytes in 1 blocks are definitely lost in loss record 7,119 of 
>> 8,549
>> ==132362==    at 0x4C3ADBB: calloc (vg_replace_malloc.c:1117)
>> ==132362==    by 0x69EE4CD: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.5600.4)
>> ==132362==    by 0x7E34AF: cpu_address_space_init (physmem.c:751)
>> ==132362==    by 0x45053E: qemu_init_vcpu (cpus.c:635)
>> ==132362==    by 0x76B4A7: x86_cpu_realizefn (cpu.c:6520)
>> ==132362==    by 0x9343ED: device_set_realized (qdev.c:531)
>> ==132362==    by 0x93E26F: property_set_bool (object.c:2273)
>> ==132362==    by 0x93C23E: object_property_set (object.c:1408)
>> ==132362==    by 0x9406DC: object_property_set_qobject (qom-qobject.c:28)
>> ==132362==    by 0x93C5A9: object_property_set_bool (object.c:1477)
>> ==132362==    by 0x933C81: qdev_realize (qdev.c:333)
>> ==132362==    by 0x455E9A: qdev_device_add_from_qdict (qdev-monitor.c:713)
>>
>> Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
>> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> Message-Id: <20220321141409.3112932-5-mark.kanda@oracle.com>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   cpu.c                     | 1 +
>>   include/exec/cpu-common.h | 7 +++++++
>>   softmmu/physmem.c         | 5 +++++
>>   3 files changed, 13 insertions(+)
>>
>> diff --git a/cpu.c b/cpu.c
>> index be1f8b074c..59352a1487 100644
>> --- a/cpu.c
>> +++ b/cpu.c
>> @@ -174,6 +174,7 @@ void cpu_exec_unrealizefn(CPUState *cpu)
>>           tcg_exec_unrealizefn(cpu);
>>       }
>>   +    cpu_address_space_destroy(cpu);
>>       cpu_list_remove(cpu);
>>   }
>>   diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
>> index 50a7d2912e..b17ad61ae4 100644
>> --- a/include/exec/cpu-common.h
>> +++ b/include/exec/cpu-common.h
>> @@ -111,6 +111,13 @@ size_t qemu_ram_pagesize_largest(void);
>>    */
>>   void cpu_address_space_init(CPUState *cpu, int asidx,
>>                               const char *prefix, MemoryRegion *mr);
>
> ... cpu_address_space_init() creates a single AS, ...
>
>> +/**
>> + * cpu_address_space_destroy:
>> + * @cpu: CPU for this address space
>> + *
>> + * Cleanup CPU's cpu_ases list.
>> + */
>> +void cpu_address_space_destroy(CPUState *cpu);
>>     void cpu_physical_memory_rw(hwaddr addr, void *buf,
>>                               hwaddr len, bool is_write);
>> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
>> index 43ae70fbe2..aec61ca07a 100644
>> --- a/softmmu/physmem.c
>> +++ b/softmmu/physmem.c
>> @@ -762,6 +762,11 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
>>       }
>>   }
>>   +void cpu_address_space_destroy(CPUState *cpu)
>> +{
>> +    g_free(cpu->cpu_ases);
>
> ... but here you destroy all the ASes.

I was thinking the whole ASes list should be freed because the CPU is going away...

Thanks/regards,
-Mark

Re: [PATCH v4 01/13] cpu: Free cpu->cpu_ases in cpu_address_space_destroy()

[Mark Kanda]

On 3/23/2022 1:56 PM, Philippe Mathieu-Daudé wrote:
> On 23/3/22 18:17, Philippe Mathieu-Daudé wrote:
>> From: Mark Kanda <mark.kanda@oracle.com>
>>
>> Create cpu_address_space_destroy() to free a CPU's cpu_ases list.
>
> This seems incorrect...
>
>> vCPU hotunplug related leak reported by Valgrind:
>>
>> ==132362== 216 bytes in 1 blocks are definitely lost in loss record 7,119 of 
>> 8,549
>> ==132362==    at 0x4C3ADBB: calloc (vg_replace_malloc.c:1117)
>> ==132362==    by 0x69EE4CD: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.5600.4)
>> ==132362==    by 0x7E34AF: cpu_address_space_init (physmem.c:751)
>> ==132362==    by 0x45053E: qemu_init_vcpu (cpus.c:635)
>> ==132362==    by 0x76B4A7: x86_cpu_realizefn (cpu.c:6520)
>> ==132362==    by 0x9343ED: device_set_realized (qdev.c:531)
>> ==132362==    by 0x93E26F: property_set_bool (object.c:2273)
>> ==132362==    by 0x93C23E: object_property_set (object.c:1408)
>> ==132362==    by 0x9406DC: object_property_set_qobject (qom-qobject.c:28)
>> ==132362==    by 0x93C5A9: object_property_set_bool (object.c:1477)
>> ==132362==    by 0x933C81: qdev_realize (qdev.c:333)
>> ==132362==    by 0x455E9A: qdev_device_add_from_qdict (qdev-monitor.c:713)
>>
>> Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
>> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> Message-Id: <20220321141409.3112932-5-mark.kanda@oracle.com>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   cpu.c                     | 1 +
>>   include/exec/cpu-common.h | 7 +++++++
>>   softmmu/physmem.c         | 5 +++++
>>   3 files changed, 13 insertions(+)
>>
>> diff --git a/cpu.c b/cpu.c
>> index be1f8b074c..59352a1487 100644
>> --- a/cpu.c
>> +++ b/cpu.c
>> @@ -174,6 +174,7 @@ void cpu_exec_unrealizefn(CPUState *cpu)
>>           tcg_exec_unrealizefn(cpu);
>>       }
>>   +    cpu_address_space_destroy(cpu);
>>       cpu_list_remove(cpu);
>>   }
>>   diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
>> index 50a7d2912e..b17ad61ae4 100644
>> --- a/include/exec/cpu-common.h
>> +++ b/include/exec/cpu-common.h
>> @@ -111,6 +111,13 @@ size_t qemu_ram_pagesize_largest(void);
>>    */
>>   void cpu_address_space_init(CPUState *cpu, int asidx,
>>                               const char *prefix, MemoryRegion *mr);
>
> ... cpu_address_space_init() creates a single AS, ...
>
>> +/**
>> + * cpu_address_space_destroy:
>> + * @cpu: CPU for this address space
>> + *
>> + * Cleanup CPU's cpu_ases list.
>> + */
>> +void cpu_address_space_destroy(CPUState *cpu);
>>     void cpu_physical_memory_rw(hwaddr addr, void *buf,
>>                               hwaddr len, bool is_write);
>> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
>> index 43ae70fbe2..aec61ca07a 100644
>> --- a/softmmu/physmem.c
>> +++ b/softmmu/physmem.c
>> @@ -762,6 +762,11 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
>>       }
>>   }
>>   +void cpu_address_space_destroy(CPUState *cpu)
>> +{
>> +    g_free(cpu->cpu_ases);
>
> ... but here you destroy all the ASes.

I was thinking the whole ASes list should be freed because the CPU is going away...

Thanks/regards,
-Mark

Re: [PATCH v4 02/13] target/i386/kvm: Free xsave_buf when destroying vCPU

[Richard Henderson]

On 3/23/22 10:17, Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daudé<f4bug@amsat.org>
> 
> Fix vCPU hot-unplug related leak reported by Valgrind:
> 
>    ==132362== 4,096 bytes in 1 blocks are definitely lost in loss record 8,440 of 8,549
>    ==132362==    at 0x4C3B15F: memalign (vg_replace_malloc.c:1265)
>    ==132362==    by 0x4C3B288: posix_memalign (vg_replace_malloc.c:1429)
>    ==132362==    by 0xB41195: qemu_try_memalign (memalign.c:53)
>    ==132362==    by 0xB41204: qemu_memalign (memalign.c:73)
>    ==132362==    by 0x7131CB: kvm_init_xsave (kvm.c:1601)
>    ==132362==    by 0x7148ED: kvm_arch_init_vcpu (kvm.c:2031)
>    ==132362==    by 0x91D224: kvm_init_vcpu (kvm-all.c:516)
>    ==132362==    by 0x9242C9: kvm_vcpu_thread_fn (kvm-accel-ops.c:40)
>    ==132362==    by 0xB2EB26: qemu_thread_start (qemu-thread-posix.c:556)
>    ==132362==    by 0x7EB2159: start_thread (in /usr/lib64/libpthread-2.28.so)
>    ==132362==    by 0x9D45DD2: clone (in /usr/lib64/libc-2.28.so)
> 
> Reported-by: Mark Kanda<mark.kanda@oracle.com>
> Tested-by: Mark Kanda<mark.kanda@oracle.com>
> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> ---
>   target/i386/kvm/kvm.c | 2 ++
>   1 file changed, 2 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v4 03/13] target/i386/hvf: Free resources when vCPU is destroyed

[Richard Henderson]

On 3/23/22 10:17, Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daudé<f4bug@amsat.org>
> 
> Both xsave_buf and hvf_caps are allocated in hvf_arch_init_vcpu(),
> free them in hvf_arch_vcpu_destroy().
> 
> Reported-by: Mark Kanda<mark.kanda@oracle.com>
> Suggested-by: Igor Mammedov<imammedo@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> ---
>   target/i386/hvf/hvf.c | 2 ++
>   1 file changed, 2 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v4 05/13] accel/tcg: Init TCG cflags in vCPU thread handler

[Richard Henderson]

On 3/23/22 10:17, Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daudé<f4bug@amsat.org>
> 
> Move TCG cflags initialization to thread handler.
> Remove the duplicated assert checks.
> 
> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> ---
>   accel/tcg/tcg-accel-ops-mttcg.c | 5 ++---
>   accel/tcg/tcg-accel-ops-rr.c    | 7 +++----
>   2 files changed, 5 insertions(+), 7 deletions(-)

Why move into the thread handler?  Seems fine where it is.
I agree with the removal of duplicate asserts, but I'd have
removed them the other way around...


r~

Re: [PATCH v4 06/13] accel/tcg: Reorganize tcg_accel_ops_init()

[Richard Henderson]

On 3/23/22 10:17, Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daudé<f4bug@amsat.org>
> 
> Reorg TCG AccelOpsClass initialization to emphasis icount
> mode share more code with single-threaded TCG.
> 
> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> ---
>   accel/tcg/tcg-accel-ops.c | 15 ++++++++-------
>   1 file changed, 8 insertions(+), 7 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v4 09/13] accel/all: Extract common_vcpu_thread_create()

[Richard Henderson]

On 3/23/22 10:17, Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> All accelerators implement a very similar create_vcpu_thread()
> handler. Extract the common part as common_vcpu_thread_create(),
> which only requires the AccelOpsClass::vcpu_thread_fn() routine
> and the accelerator AccelOpsClass::thread_name for debugging
> purpose.
> 
> The single exception is TCG/RR which re-use a single vCPU. Have
> it use the same logic by using the .precheck/.postcheck handlers.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Too much all at once.
But I see now why you moved code in the direction you did in patch 5.

> @@ -279,28 +279,13 @@ bool rr_create_vcpu_thread_precheck(CPUState *cpu)
>       return !single_tcg_cpu_thread;
>   }
>   
> -void rr_start_vcpu_thread(CPUState *cpu)
> +void rr_create_vcpu_thread_postcheck(CPUState *cpu)
>   {
> -    char thread_name[VCPU_THREAD_NAME_SIZE];
>       static QemuCond *single_tcg_halt_cond;
> -    static QemuThread *single_tcg_cpu_thread;

Patch 8 is not really standalone, since you didn't move this variable.
I think it's probably a mistake to split out precheck and postcheck separately.

> -
> -    if (!single_tcg_cpu_thread) {
> +    if (! single_tcg_cpu_thread) {

Extraneous whitespace.

> @@ -30,7 +30,8 @@ struct AccelOpsClass {
>   
>       bool (*cpus_are_resettable)(void);
>   
> -    void (*create_vcpu_thread)(CPUState *cpu); /* MANDATORY NON-NULL */
> +    const char *thread_name;

Why don't we just get this name from the AccelClass?

> +static void common_vcpu_thread_create(CPUState *cpu)
> +{
> +    char thread_name[VCPU_THREAD_NAME_SIZE];
> +
> +    cpu->thread = g_new0(QemuThread, 1);
> +    cpu->halt_cond = g_new0(QemuCond, 1);
> +    qemu_cond_init(cpu->halt_cond);
> +    snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/%s",
> +             cpu->cpu_index, cpus_accel->thread_name ?: "DUMMY");

Surely g_strdup_printf is better than the static size buffer.


r~

Re: [PATCH v4 09/13] accel/all: Extract common_vcpu_thread_create()

[Richard Henderson]

On 3/23/22 10:17, Philippe Mathieu-Daudé wrote:
> +void rr_create_vcpu_thread_postcheck(CPUState *cpu)
>   {
> -    char thread_name[VCPU_THREAD_NAME_SIZE];
>       static QemuCond *single_tcg_halt_cond;
> -    static QemuThread *single_tcg_cpu_thread;
> -
> -    if (!single_tcg_cpu_thread) {
> -        cpu->thread = g_new0(QemuThread, 1);
> -        cpu->halt_cond = g_new0(QemuCond, 1);
> -        qemu_cond_init(cpu->halt_cond);
> -
> -        /* share a single thread for all cpus with TCG */
> -        snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "ALL CPUs/TCG");
> -        qemu_thread_create(cpu->thread, thread_name,
> -                           rr_cpu_thread_fn,
> -                           cpu, QEMU_THREAD_JOINABLE);
>   
> +    if (! single_tcg_cpu_thread) {
>           single_tcg_halt_cond = cpu->halt_cond;
>           single_tcg_cpu_thread = cpu->thread;
> -#ifdef _WIN32
> -        cpu->hThread = qemu_thread_get_handle(cpu->thread);
> -#endif
>       } else {
>           /* we share the thread */
>           cpu->thread = single_tcg_cpu_thread;

This doesn't treat cpu->halt_cond correctly for other than the first cpu.


r~

Re: [PATCH v4 07/13] accel-ops: Introduce create_vcpu_thread_precheck / postcheck handlers

[Richard Henderson]

On 3/23/22 10:17, Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> Introduce precheck/postcheck handlers which will help to
> refactor code common to the various create_vcpu_thread()
> implementations.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   include/sysemu/accel-ops.h | 4 ++++
>   softmmu/cpus.c             | 8 +++++++-
>   2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/include/sysemu/accel-ops.h b/include/sysemu/accel-ops.h
> index 6013c9444c..26b542d35c 100644
> --- a/include/sysemu/accel-ops.h
> +++ b/include/sysemu/accel-ops.h
> @@ -31,6 +31,10 @@ struct AccelOpsClass {
>       bool (*cpus_are_resettable)(void);
>   
>       void (*create_vcpu_thread)(CPUState *cpu); /* MANDATORY NON-NULL */
> +    /* If non-NULL, return whether common vCPU thread must be created */
> +    bool (*create_vcpu_thread_precheck)(CPUState *cpu);
> +    void (*create_vcpu_thread_postcheck)(CPUState *cpu);


I don't think this is the correct trade-off.
These new hooks are only used by rr, and at least in this patch set, incorrectly.

I think you should keep the single create_vcpu_thread hook, and if null, use your new 
common_vcpu_thread_create.  Leave rr to be the single accel setting the hook, and then 
it's easier not to break rr during the reorg as well.


r~

Re: [PATCH v4 10/13] accel-ops: Introduce common_vcpu_thread_destroy() and .precheck handler

[Richard Henderson]

On 3/23/22 10:17, Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> Introduce an empty common_vcpu_thread_destroy() function, and
> provide a AccelOpsClass::destroy_vcpu_thread_precheck() callback
> so accelerators can choose whether to call common_vcpu_thread_destroy.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   include/sysemu/accel-ops.h | 2 ++
>   softmmu/cpus.c             | 9 +++++++++
>   2 files changed, 11 insertions(+)

My comments here are similar to the create precheck hook.
Do not add a "precheck" hook, but simply a hook to perform the whole job.



r~

Re: [PATCH v4 11/13] accel/tcg: Add rr_destroy_vcpu_thread_precheck()

[Richard Henderson]

On 3/23/22 10:17, Philippe Mathieu-Daudé wrote:
> +bool rr_destroy_vcpu_thread_precheck(CPUState *cpu)
> +{
> +    if (single_tcg_cpu_thread) {
> +        single_tcg_cpu_thread = NULL;
> +        return true;
> +    }
> +    return false;
> +}

This would become

void rr_destroy_vcpu_thread(CPUState *cpu)
{
     if (single_tcg_cpu_thread) {
         g_free(single_tcg_cpu_thread);
         g_free(single_tcg_halt_cond);
         single_tcg_cpu_thread = NULL;
         single_tcg_halt_cond = NULL;
     }
}


r~

Re: [PATCH v4 12/13] softmmu/cpus: Free cpu->thread in generic_destroy_vcpu_thread()

[Richard Henderson]

On 3/23/22 10:17, Philippe Mathieu-Daudé wrote:
>   static void common_vcpu_thread_destroy(CPUState *cpu)
>   {
> +    g_free(cpu->thread);
>   }

Missing free of halt_cond.


r~

Re: [PATCH v4 13/13] softmmu/cpus: Free cpu->halt_cond in generic_destroy_vcpu_thread()

[Richard Henderson]

On 3/23/22 10:17, Philippe Mathieu-Daudé wrote:
> From: Mark Kanda<mark.kanda@oracle.com>
> 
> vCPU hotunplug related leak reported by Valgrind:
> 
>    ==102631== 56 bytes in 1 blocks are definitely lost in loss record 5,089 of 8,555
>    ==102631==    at 0x4C3ADBB: calloc (vg_replace_malloc.c:1117)
>    ==102631==    by 0x69EE4CD: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.5600.4)
>    ==102631==    by 0x924452: kvm_start_vcpu_thread (kvm-accel-ops.c:69)
>    ==102631==    by 0x4505C2: qemu_init_vcpu (cpus.c:643)
>    ==102631==    by 0x76B4D1: x86_cpu_realizefn (cpu.c:6520)
>    ==102631==    by 0x9344A7: device_set_realized (qdev.c:531)
>    ==102631==    by 0x93E329: property_set_bool (object.c:2273)
>    ==102631==    by 0x93C2F8: object_property_set (object.c:1408)
>    ==102631==    by 0x940796: object_property_set_qobject (qom-qobject.c:28)
>    ==102631==    by 0x93C663: object_property_set_bool (object.c:1477)
>    ==102631==    by 0x933D3B: qdev_realize (qdev.c:333)
>    ==102631==    by 0x455EC4: qdev_device_add_from_qdict (qdev-monitor.c:713)
> 
> Signed-off-by: Mark Kanda<mark.kanda@oracle.com>
> Message-Id:<20220321141409.3112932-4-mark.kanda@oracle.com>
> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> ---
>   softmmu/cpus.c | 1 +
>   1 file changed, 1 insertion(+)

Eh.  Merge with previous.


r~

Re: [PATCH v4 05/13] accel/tcg: Init TCG cflags in vCPU thread handler

[Richard Henderson]

On 3/23/22 10:17, Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> Move TCG cflags initialization to thread handler.
> Remove the duplicated assert checks.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   accel/tcg/tcg-accel-ops-mttcg.c | 5 ++---
>   accel/tcg/tcg-accel-ops-rr.c    | 7 +++----
>   2 files changed, 5 insertions(+), 7 deletions(-)

Queued to tcg-next.


r~

Re: [PATCH v4 06/13] accel/tcg: Reorganize tcg_accel_ops_init()

[Richard Henderson]

On 3/23/22 10:17, Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> Reorg TCG AccelOpsClass initialization to emphasis icount
> mode share more code with single-threaded TCG.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   accel/tcg/tcg-accel-ops.c | 15 ++++++++-------
>   1 file changed, 8 insertions(+), 7 deletions(-)

Queued to tcg-next.


r~