[PATCH v2 0/5] vCPU hotunplug related memory leaks

[PATCH v2 0/5] vCPU hotunplug related memory leaks

[Mark Kanda]

This series addresses a few vCPU hotunplug related leaks (found with Valgrind).

v2: Create AccelOpsClass::destroy_vcpu_thread() for vcpu thread related cleanup
(Philippe)

Mark Kanda (5):
  accel: Introduce AccelOpsClass::destroy_vcpu_thread()
  softmmu/cpus: Free cpu->thread in destroy_vcpu_thread_generic()
  softmmu/cpus: Free cpu->halt_cond in destroy_vcpu_thread_generic()
  cpu: Free cpu->cpu_ases in cpu_exec_unrealizefn()
  i386/cpu: Free env->xsave_buf in x86_cpu_unrealizefn()

 accel/accel-common.c              | 7 +++++++
 accel/hvf/hvf-accel-ops.c         | 1 +
 accel/kvm/kvm-accel-ops.c         | 1 +
 accel/qtest/qtest.c               | 1 +
 accel/tcg/tcg-accel-ops.c         | 1 +
 accel/xen/xen-all.c               | 1 +
 cpu.c                             | 1 +
 include/sysemu/accel-ops.h        | 3 +++
 softmmu/cpus.c                    | 3 +++
 target/i386/cpu.c                 | 5 +++++
 target/i386/hax/hax-accel-ops.c   | 1 +
 target/i386/nvmm/nvmm-accel-ops.c | 1 +
 target/i386/whpx/whpx-accel-ops.c | 1 +
 13 files changed, 27 insertions(+)

-- 
2.27.0

[PATCH v2 1/5] accel: Introduce AccelOpsClass::destroy_vcpu_thread()

[Mark Kanda]

Add destroy_vcpu_thread() to AccelOps as a method for vcpu thread cleanup.
This will be used in subsequent patches.

Suggested-by: Philippe Mathieu-Daude <philippe.mathieu.daude@gmail.com>
Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
---
 include/sysemu/accel-ops.h | 1 +
 softmmu/cpus.c             | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/include/sysemu/accel-ops.h b/include/sysemu/accel-ops.h
index 6013c9444c..e296b27b82 100644
--- a/include/sysemu/accel-ops.h
+++ b/include/sysemu/accel-ops.h
@@ -31,6 +31,7 @@ struct AccelOpsClass {
     bool (*cpus_are_resettable)(void);
 
     void (*create_vcpu_thread)(CPUState *cpu); /* MANDATORY NON-NULL */
+    void (*destroy_vcpu_thread)(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..622f8b4608 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -609,6 +609,9 @@ 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) {
+        cpus_accel->destroy_vcpu_thread(cpu);
+    }
 }
 
 void cpus_register_accel(const AccelOpsClass *ops)
-- 
2.27.0

Re: [PATCH v2 1/5] accel: Introduce AccelOpsClass::destroy_vcpu_thread()

[Philippe Mathieu-Daudé]

On 18/3/22 16:15, Mark Kanda wrote:
> Add destroy_vcpu_thread() to AccelOps as a method for vcpu thread cleanup.
> This will be used in subsequent patches.
> 
> Suggested-by: Philippe Mathieu-Daude <philippe.mathieu.daude@gmail.com>

Thanks, but preferably:
Suggested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
> ---
>   include/sysemu/accel-ops.h | 1 +
>   softmmu/cpus.c             | 3 +++
>   2 files changed, 4 insertions(+)

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

[PATCH v2 2/5] softmmu/cpus: Free cpu->thread in destroy_vcpu_thread_generic()

[Mark Kanda]

Use a new AccelOpsClass::destroy_vcpu_thread() handler
destroy_vcpu_thread_generic() to free cpu->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>
---
 accel/accel-common.c              | 6 ++++++
 accel/hvf/hvf-accel-ops.c         | 1 +
 accel/kvm/kvm-accel-ops.c         | 1 +
 accel/qtest/qtest.c               | 1 +
 accel/tcg/tcg-accel-ops.c         | 1 +
 accel/xen/xen-all.c               | 1 +
 include/sysemu/accel-ops.h        | 2 ++
 target/i386/hax/hax-accel-ops.c   | 1 +
 target/i386/nvmm/nvmm-accel-ops.c | 1 +
 target/i386/whpx/whpx-accel-ops.c | 1 +
 10 files changed, 16 insertions(+)

diff --git a/accel/accel-common.c b/accel/accel-common.c
index 7b8ec7e0f7..80b0d909b2 100644
--- a/accel/accel-common.c
+++ b/accel/accel-common.c
@@ -28,6 +28,7 @@
 
 #include "cpu.h"
 #include "hw/core/accel-cpu.h"
+#include "sysemu/accel-ops.h"
 
 #ifndef CONFIG_USER_ONLY
 #include "accel-softmmu.h"
@@ -135,3 +136,8 @@ static void register_accel_types(void)
 }
 
 type_init(register_accel_types);
+
+void destroy_vcpu_thread_generic(CPUState *cpu)
+{
+    g_free(cpu->thread);
+}
diff --git a/accel/hvf/hvf-accel-ops.c b/accel/hvf/hvf-accel-ops.c
index 54457c76c2..69c23f6763 100644
--- a/accel/hvf/hvf-accel-ops.c
+++ b/accel/hvf/hvf-accel-ops.c
@@ -467,6 +467,7 @@ 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->destroy_vcpu_thread = destroy_vcpu_thread_generic;
     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..fd439f8e23 100644
--- a/accel/kvm/kvm-accel-ops.c
+++ b/accel/kvm/kvm-accel-ops.c
@@ -89,6 +89,7 @@ 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->destroy_vcpu_thread = destroy_vcpu_thread_generic;
     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..3ea148ed0e 100644
--- a/accel/qtest/qtest.c
+++ b/accel/qtest/qtest.c
@@ -51,6 +51,7 @@ 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->destroy_vcpu_thread = destroy_vcpu_thread_generic;
     ops->get_virtual_clock = qtest_get_virtual_clock;
 };
 
diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
index ea7dcad674..4ef80c81e4 100644
--- a/accel/tcg/tcg-accel-ops.c
+++ b/accel/tcg/tcg-accel-ops.c
@@ -94,6 +94,7 @@ void tcg_handle_interrupt(CPUState *cpu, int mask)
 
 static void tcg_accel_ops_init(AccelOpsClass *ops)
 {
+    ops->destroy_vcpu_thread = destroy_vcpu_thread_generic;
     if (qemu_tcg_mttcg_enabled()) {
         ops->create_vcpu_thread = mttcg_start_vcpu_thread;
         ops->kick_vcpu_thread = mttcg_kick_vcpu_thread;
diff --git a/accel/xen/xen-all.c b/accel/xen/xen-all.c
index 69aa7d018b..c5982a782c 100644
--- a/accel/xen/xen-all.c
+++ b/accel/xen/xen-all.c
@@ -220,6 +220,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->destroy_vcpu_thread = destroy_vcpu_thread_generic;
 }
 
 static const TypeInfo xen_accel_ops_type = {
diff --git a/include/sysemu/accel-ops.h b/include/sysemu/accel-ops.h
index e296b27b82..46e3190119 100644
--- a/include/sysemu/accel-ops.h
+++ b/include/sysemu/accel-ops.h
@@ -46,4 +46,6 @@ struct AccelOpsClass {
     int64_t (*get_elapsed_ticks)(void);
 };
 
+/* free vcpu thread structures */
+void destroy_vcpu_thread_generic(CPUState *cpu);
 #endif /* ACCEL_OPS_H */
diff --git a/target/i386/hax/hax-accel-ops.c b/target/i386/hax/hax-accel-ops.c
index 136630e9b2..8b6715d047 100644
--- a/target/i386/hax/hax-accel-ops.c
+++ b/target/i386/hax/hax-accel-ops.c
@@ -79,6 +79,7 @@ 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->destroy_vcpu_thread = destroy_vcpu_thread_generic;
     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 f788f75289..f08292406c 100644
--- a/target/i386/nvmm/nvmm-accel-ops.c
+++ b/target/i386/nvmm/nvmm-accel-ops.c
@@ -88,6 +88,7 @@ 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->destroy_vcpu_thread = destroy_vcpu_thread_generic;
     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 1d30e4e2ed..17a385324c 100644
--- a/target/i386/whpx/whpx-accel-ops.c
+++ b/target/i386/whpx/whpx-accel-ops.c
@@ -93,6 +93,7 @@ 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->destroy_vcpu_thread = destroy_vcpu_thread_generic;
     ops->kick_vcpu_thread = whpx_kick_vcpu_thread;
     ops->cpu_thread_is_idle = whpx_vcpu_thread_is_idle;
 
-- 
2.27.0

[PATCH v2 3/5] softmmu/cpus: Free cpu->halt_cond in destroy_vcpu_thread_generic()

[Mark Kanda]

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>
---
 accel/accel-common.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/accel/accel-common.c b/accel/accel-common.c
index 80b0d909b2..ae71a27799 100644
--- a/accel/accel-common.c
+++ b/accel/accel-common.c
@@ -140,4 +140,5 @@ type_init(register_accel_types);
 void destroy_vcpu_thread_generic(CPUState *cpu)
 {
     g_free(cpu->thread);
+    g_free(cpu->halt_cond);
 }
-- 
2.27.0

[PATCH v2 4/5] cpu: Free cpu->cpu_ases in cpu_exec_unrealizefn()

[Mark Kanda]

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>
---
 cpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/cpu.c b/cpu.c
index be1f8b074c..6a3475022f 100644
--- a/cpu.c
+++ b/cpu.c
@@ -173,6 +173,7 @@ void cpu_exec_unrealizefn(CPUState *cpu)
     if (tcg_enabled()) {
         tcg_exec_unrealizefn(cpu);
     }
+    g_free(cpu->cpu_ases);
 
     cpu_list_remove(cpu);
 }
-- 
2.27.0

Re: [PATCH v2 4/5] cpu: Free cpu->cpu_ases in cpu_exec_unrealizefn()

[Philippe Mathieu-Daudé]

On 18/3/22 16:15, Mark Kanda wrote:
> 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>
> ---
>   cpu.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/cpu.c b/cpu.c
> index be1f8b074c..6a3475022f 100644
> --- a/cpu.c
> +++ b/cpu.c
> @@ -173,6 +173,7 @@ void cpu_exec_unrealizefn(CPUState *cpu)
>       if (tcg_enabled()) {
>           tcg_exec_unrealizefn(cpu);
>       }
> +    g_free(cpu->cpu_ases);

There is an API mismatch here. We miss cpu_address_space_destroy().

cpu_exec_unrealizefn() then calls cpu_address_space_destroy(),
and cpu_address_space_destroy() frees cpu_ases.

Otherwise other cpu_address_space_init() calls will keep leaking.

>       cpu_list_remove(cpu);
>   }

Re: [PATCH v2 4/5] cpu: Free cpu->cpu_ases in cpu_exec_unrealizefn()

[Mark Kanda]

On 3/18/2022 11:26 AM, Philippe Mathieu-Daudé wrote:
> On 18/3/22 16:15, Mark Kanda wrote:
>> 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>
>> ---
>>   cpu.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/cpu.c b/cpu.c
>> index be1f8b074c..6a3475022f 100644
>> --- a/cpu.c
>> +++ b/cpu.c
>> @@ -173,6 +173,7 @@ void cpu_exec_unrealizefn(CPUState *cpu)
>>       if (tcg_enabled()) {
>>           tcg_exec_unrealizefn(cpu);
>>       }
>> +    g_free(cpu->cpu_ases);
>
> There is an API mismatch here. We miss cpu_address_space_destroy().
>
> cpu_exec_unrealizefn() then calls cpu_address_space_destroy(),
> and cpu_address_space_destroy() frees cpu_ases.
>
> Otherwise other cpu_address_space_init() calls will keep leaking.
>

Will fix in v3.

Thanks Philippe,
-Mark

[PATCH v2 5/5] i386/cpu: Free env->xsave_buf in x86_cpu_unrealizefn()

[Mark Kanda]

vCPU hotunplug 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)

Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
---
 target/i386/cpu.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index a88d6554c8..014a716c36 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6572,6 +6572,11 @@ static void x86_cpu_unrealizefn(DeviceState *dev)
     }
 
     xcc->parent_unrealize(dev);
+
+#if defined(CONFIG_KVM) || defined(CONFIG_HVF)
+    CPUX86State *env = &cpu->env;
+    g_free(env->xsave_buf);
+#endif
 }
 
 typedef struct BitProperty {
-- 
2.27.0

Re: [PATCH v2 5/5] i386/cpu: Free env->xsave_buf in x86_cpu_unrealizefn()

[Philippe Mathieu-Daudé]

On 18/3/22 16:15, Mark Kanda wrote:
> vCPU hotunplug 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)
> 
> Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
> ---
>   target/i386/cpu.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index a88d6554c8..014a716c36 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -6572,6 +6572,11 @@ static void x86_cpu_unrealizefn(DeviceState *dev)
>       }
>   
>       xcc->parent_unrealize(dev);
> +
> +#if defined(CONFIG_KVM) || defined(CONFIG_HVF)
> +    CPUX86State *env = &cpu->env;
> +    g_free(env->xsave_buf);

This belong to hvf_arch_vcpu_destroy().

And for KVM, in the missing kvm_arch_destroy_vcpu().

> +#endif
>   }
>   
>   typedef struct BitProperty {

Re: [PATCH v2 5/5] i386/cpu: Free env->xsave_buf in x86_cpu_unrealizefn()

[Mark Kanda]

On 3/18/2022 11:32 AM, Philippe Mathieu-Daudé wrote:
> On 18/3/22 16:15, Mark Kanda wrote:
>> vCPU hotunplug 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)
>>
>> Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
>> ---
>>   target/i386/cpu.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index a88d6554c8..014a716c36 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -6572,6 +6572,11 @@ static void x86_cpu_unrealizefn(DeviceState *dev)
>>       }
>>         xcc->parent_unrealize(dev);
>> +
>> +#if defined(CONFIG_KVM) || defined(CONFIG_HVF)
>> +    CPUX86State *env = &cpu->env;
>> +    g_free(env->xsave_buf);
>
> This belong to hvf_arch_vcpu_destroy().
>
> And for KVM, in the missing kvm_arch_destroy_vcpu().
>

Will fix in v3.

Thanks Philippe,
-Mark