* [PATCH v4 00/13] accel: Fix vCPU memory leaks
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Paul Durrant, Peter Xu, Yanan Wang, haxm-team, Colin Xu,
Stefano Stabellini, David Hildenbrand, Kamil Rytarowski,
Reinoud Zandijk, Anthony Perard, xen-devel, Laurent Vivier,
Thomas Huth, Richard Henderson, Cameron Esfahani,
Roman Bolshakov, Sunil Muthuswamy, Eduardo Habkost,
Marcelo Tosatti, Philippe Mathieu-Daudé,
Wenchao Wang, Paolo Bonzini
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
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH v4 01/13] cpu: Free cpu->cpu_ases in cpu_address_space_destroy()
2022-03-23 17:17 ` Philippe Mathieu-Daudé
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
-1 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Wenchao Wang, Laurent Vivier, David Hildenbrand, Yanan Wang,
Cameron Esfahani, Marcelo Tosatti, Sunil Muthuswamy,
Anthony Perard, haxm-team, Paul Durrant, Richard Henderson,
xen-devel, Philippe Mathieu-Daudé,
Roman Bolshakov, Reinoud Zandijk, Marcel Apfelbaum,
Kamil Rytarowski, Paolo Bonzini, Peter Xu, Eduardo Habkost,
Stefano Stabellini, Thomas Huth, Colin Xu, Mark Kanda
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v4 01/13] cpu: Free cpu->cpu_ases in cpu_address_space_destroy()
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Paul Durrant, Peter Xu, Yanan Wang, haxm-team, Colin Xu,
Stefano Stabellini, David Hildenbrand, Kamil Rytarowski,
Reinoud Zandijk, Anthony Perard, xen-devel, Laurent Vivier,
Thomas Huth, Richard Henderson, Cameron Esfahani,
Roman Bolshakov, Sunil Muthuswamy, Eduardo Habkost,
Marcelo Tosatti, Philippe Mathieu-Daudé,
Wenchao Wang, Paolo Bonzini
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH v4 01/13] cpu: Free cpu->cpu_ases in cpu_address_space_destroy()
2022-03-23 17:17 ` Philippe Mathieu-Daudé
@ 2022-03-23 18:56 ` Philippe Mathieu-Daudé
-1 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 18:56 UTC (permalink / raw)
To: qemu-devel
Cc: Wenchao Wang, Laurent Vivier, David Hildenbrand, Yanan Wang,
Cameron Esfahani, Marcelo Tosatti, Sunil Muthuswamy,
Anthony Perard, haxm-team, Paul Durrant, Richard Henderson,
xen-devel, Philippe Mathieu-Daudé,
Roman Bolshakov, Reinoud Zandijk, Marcel Apfelbaum,
Kamil Rytarowski, Paolo Bonzini, Peter Xu, Eduardo Habkost,
Stefano Stabellini, Thomas Huth, Colin Xu, Mark Kanda
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 */
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v4 01/13] cpu: Free cpu->cpu_ases in cpu_address_space_destroy()
@ 2022-03-23 18:56 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 18:56 UTC (permalink / raw)
To: qemu-devel
Cc: Paul Durrant, Peter Xu, Yanan Wang, haxm-team, Colin Xu,
Stefano Stabellini, David Hildenbrand, Kamil Rytarowski,
Reinoud Zandijk, Anthony Perard, xen-devel, Laurent Vivier,
Thomas Huth, Richard Henderson, Cameron Esfahani,
Roman Bolshakov, Sunil Muthuswamy, Eduardo Habkost,
Marcelo Tosatti, Philippe Mathieu-Daudé,
Wenchao Wang, Paolo Bonzini
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 */
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v4 01/13] cpu: Free cpu->cpu_ases in cpu_address_space_destroy()
2022-03-23 18:56 ` Philippe Mathieu-Daudé
@ 2022-03-23 19:14 ` Mark Kanda
-1 siblings, 0 replies; 47+ messages in thread
From: Mark Kanda @ 2022-03-23 19:14 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Paul Durrant, Peter Xu, Yanan Wang, haxm-team, Colin Xu,
Stefano Stabellini, David Hildenbrand, Kamil Rytarowski,
Reinoud Zandijk, Anthony Perard, xen-devel, Laurent Vivier,
Thomas Huth, Richard Henderson, Cameron Esfahani,
Roman Bolshakov, Sunil Muthuswamy, Eduardo Habkost,
Marcelo Tosatti, Philippe Mathieu-Daudé,
Wenchao Wang, Paolo Bonzini
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
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v4 01/13] cpu: Free cpu->cpu_ases in cpu_address_space_destroy()
@ 2022-03-23 19:14 ` Mark Kanda
0 siblings, 0 replies; 47+ messages in thread
From: Mark Kanda @ 2022-03-23 19:14 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Wenchao Wang, Laurent Vivier, David Hildenbrand, Yanan Wang,
Cameron Esfahani, Marcelo Tosatti, Sunil Muthuswamy,
Anthony Perard, haxm-team, Paul Durrant, Richard Henderson,
xen-devel, Philippe Mathieu-Daudé,
Roman Bolshakov, Reinoud Zandijk, Marcel Apfelbaum,
Kamil Rytarowski, Paolo Bonzini, Peter Xu, Eduardo Habkost,
Stefano Stabellini, Thomas Huth, Colin Xu
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
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v4 02/13] target/i386/kvm: Free xsave_buf when destroying vCPU
2022-03-23 17:17 ` Philippe Mathieu-Daudé
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
-1 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Wenchao Wang, Laurent Vivier, David Hildenbrand, Yanan Wang,
Cameron Esfahani, Marcelo Tosatti, Sunil Muthuswamy,
Anthony Perard, haxm-team, Paul Durrant, Richard Henderson,
xen-devel, Philippe Mathieu-Daudé,
Roman Bolshakov, Reinoud Zandijk, Marcel Apfelbaum,
Kamil Rytarowski, Paolo Bonzini, Peter Xu, Eduardo Habkost,
Stefano Stabellini, Thomas Huth, Colin Xu, Mark Kanda
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v4 02/13] target/i386/kvm: Free xsave_buf when destroying vCPU
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Paul Durrant, Peter Xu, Yanan Wang, haxm-team, Colin Xu,
Stefano Stabellini, David Hildenbrand, Kamil Rytarowski,
Reinoud Zandijk, Anthony Perard, xen-devel, Laurent Vivier,
Thomas Huth, Richard Henderson, Cameron Esfahani,
Roman Bolshakov, Sunil Muthuswamy, Eduardo Habkost,
Marcelo Tosatti, Philippe Mathieu-Daudé,
Wenchao Wang, Paolo Bonzini
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH v4 02/13] target/i386/kvm: Free xsave_buf when destroying vCPU
2022-03-23 17:17 ` Philippe Mathieu-Daudé
(?)
@ 2022-03-23 21:15 ` Richard Henderson
-1 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-03-23 21:15 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
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~
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v4 03/13] target/i386/hvf: Free resources when vCPU is destroyed
2022-03-23 17:17 ` Philippe Mathieu-Daudé
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
-1 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Wenchao Wang, Laurent Vivier, David Hildenbrand, Yanan Wang,
Cameron Esfahani, Marcelo Tosatti, Sunil Muthuswamy,
Anthony Perard, haxm-team, Paul Durrant, Richard Henderson,
xen-devel, Philippe Mathieu-Daudé,
Roman Bolshakov, Reinoud Zandijk, Marcel Apfelbaum,
Kamil Rytarowski, Paolo Bonzini, Peter Xu, Eduardo Habkost,
Stefano Stabellini, Thomas Huth, Colin Xu, Mark Kanda,
Igor Mammedov
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v4 03/13] target/i386/hvf: Free resources when vCPU is destroyed
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Paul Durrant, Peter Xu, Yanan Wang, haxm-team, Colin Xu,
Stefano Stabellini, David Hildenbrand, Kamil Rytarowski,
Reinoud Zandijk, Anthony Perard, xen-devel, Laurent Vivier,
Thomas Huth, Richard Henderson, Cameron Esfahani,
Roman Bolshakov, Igor Mammedov, Sunil Muthuswamy,
Eduardo Habkost, Marcelo Tosatti, Philippe Mathieu-Daudé,
Wenchao Wang, Paolo Bonzini
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH v4 03/13] target/i386/hvf: Free resources when vCPU is destroyed
2022-03-23 17:17 ` Philippe Mathieu-Daudé
(?)
@ 2022-03-23 21:19 ` Richard Henderson
-1 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-03-23 21:19 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
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~
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v4 04/13] accel/hvf: Remove pointless assertion
2022-03-23 17:17 ` Philippe Mathieu-Daudé
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
-1 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Wenchao Wang, Laurent Vivier, David Hildenbrand, Yanan Wang,
Cameron Esfahani, Marcelo Tosatti, Sunil Muthuswamy,
Anthony Perard, haxm-team, Paul Durrant, Richard Henderson,
xen-devel, Philippe Mathieu-Daudé,
Roman Bolshakov, Reinoud Zandijk, Marcel Apfelbaum,
Kamil Rytarowski, Paolo Bonzini, Peter Xu, Eduardo Habkost,
Stefano Stabellini, Thomas Huth, Colin Xu
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v4 04/13] accel/hvf: Remove pointless assertion
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Paul Durrant, Peter Xu, Yanan Wang, haxm-team, Colin Xu,
Stefano Stabellini, David Hildenbrand, Kamil Rytarowski,
Reinoud Zandijk, Anthony Perard, xen-devel, Laurent Vivier,
Thomas Huth, Richard Henderson, Cameron Esfahani,
Roman Bolshakov, Sunil Muthuswamy, Eduardo Habkost,
Marcelo Tosatti, Philippe Mathieu-Daudé,
Wenchao Wang, Paolo Bonzini
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
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v4 05/13] accel/tcg: Init TCG cflags in vCPU thread handler
2022-03-23 17:17 ` Philippe Mathieu-Daudé
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
-1 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Paul Durrant, Peter Xu, Yanan Wang, haxm-team, Colin Xu,
Stefano Stabellini, David Hildenbrand, Kamil Rytarowski,
Reinoud Zandijk, Anthony Perard, xen-devel, Laurent Vivier,
Thomas Huth, Richard Henderson, Cameron Esfahani,
Roman Bolshakov, Sunil Muthuswamy, Eduardo Habkost,
Marcelo Tosatti, Philippe Mathieu-Daudé,
Wenchao Wang, Paolo Bonzini
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v4 05/13] accel/tcg: Init TCG cflags in vCPU thread handler
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Wenchao Wang, Laurent Vivier, David Hildenbrand, Yanan Wang,
Cameron Esfahani, Marcelo Tosatti, Sunil Muthuswamy,
Anthony Perard, haxm-team, Paul Durrant, Richard Henderson,
xen-devel, Philippe Mathieu-Daudé,
Roman Bolshakov, Reinoud Zandijk, Marcel Apfelbaum,
Kamil Rytarowski, Paolo Bonzini, Peter Xu, Eduardo Habkost,
Stefano Stabellini, Thomas Huth, Colin Xu
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH v4 05/13] accel/tcg: Init TCG cflags in vCPU thread handler
2022-03-23 17:17 ` Philippe Mathieu-Daudé
(?)
@ 2022-03-23 21:29 ` Richard Henderson
-1 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-03-23 21:29 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
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~
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v4 05/13] accel/tcg: Init TCG cflags in vCPU thread handler
2022-03-23 17:17 ` Philippe Mathieu-Daudé
(?)
(?)
@ 2022-06-20 20:04 ` Richard Henderson
-1 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-06-20 20:04 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
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~
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v4 06/13] accel/tcg: Reorganize tcg_accel_ops_init()
2022-03-23 17:17 ` Philippe Mathieu-Daudé
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
-1 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Paul Durrant, Peter Xu, Yanan Wang, haxm-team, Colin Xu,
Stefano Stabellini, David Hildenbrand, Kamil Rytarowski,
Reinoud Zandijk, Anthony Perard, xen-devel, Laurent Vivier,
Thomas Huth, Richard Henderson, Cameron Esfahani,
Roman Bolshakov, Sunil Muthuswamy, Eduardo Habkost,
Marcelo Tosatti, Philippe Mathieu-Daudé,
Wenchao Wang, Paolo Bonzini
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v4 06/13] accel/tcg: Reorganize tcg_accel_ops_init()
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Wenchao Wang, Laurent Vivier, David Hildenbrand, Yanan Wang,
Cameron Esfahani, Marcelo Tosatti, Sunil Muthuswamy,
Anthony Perard, haxm-team, Paul Durrant, Richard Henderson,
xen-devel, Philippe Mathieu-Daudé,
Roman Bolshakov, Reinoud Zandijk, Marcel Apfelbaum,
Kamil Rytarowski, Paolo Bonzini, Peter Xu, Eduardo Habkost,
Stefano Stabellini, Thomas Huth, Colin Xu
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH v4 06/13] accel/tcg: Reorganize tcg_accel_ops_init()
2022-03-23 17:17 ` Philippe Mathieu-Daudé
(?)
@ 2022-03-23 21:31 ` Richard Henderson
-1 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-03-23 21:31 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
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~
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v4 06/13] accel/tcg: Reorganize tcg_accel_ops_init()
2022-03-23 17:17 ` Philippe Mathieu-Daudé
(?)
(?)
@ 2022-06-20 20:09 ` Richard Henderson
-1 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-06-20 20:09 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
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~
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v4 07/13] accel-ops: Introduce create_vcpu_thread_precheck / postcheck handlers
2022-03-23 17:17 ` Philippe Mathieu-Daudé
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
-1 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Paul Durrant, Peter Xu, Yanan Wang, haxm-team, Colin Xu,
Stefano Stabellini, David Hildenbrand, Kamil Rytarowski,
Reinoud Zandijk, Anthony Perard, xen-devel, Laurent Vivier,
Thomas Huth, Richard Henderson, Cameron Esfahani,
Roman Bolshakov, Sunil Muthuswamy, Eduardo Habkost,
Marcelo Tosatti, Philippe Mathieu-Daudé,
Wenchao Wang, Paolo Bonzini
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v4 07/13] accel-ops: Introduce create_vcpu_thread_precheck / postcheck handlers
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Wenchao Wang, Laurent Vivier, David Hildenbrand, Yanan Wang,
Cameron Esfahani, Marcelo Tosatti, Sunil Muthuswamy,
Anthony Perard, haxm-team, Paul Durrant, Richard Henderson,
xen-devel, Philippe Mathieu-Daudé,
Roman Bolshakov, Reinoud Zandijk, Marcel Apfelbaum,
Kamil Rytarowski, Paolo Bonzini, Peter Xu, Eduardo Habkost,
Stefano Stabellini, Thomas Huth, Colin Xu
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH v4 07/13] accel-ops: Introduce create_vcpu_thread_precheck / postcheck handlers
2022-03-23 17:17 ` Philippe Mathieu-Daudé
(?)
@ 2022-03-23 22:26 ` Richard Henderson
-1 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-03-23 22:26 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
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~
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v4 08/13] accel/tcg: Extract rr_create_vcpu_thread_precheck()
2022-03-23 17:17 ` Philippe Mathieu-Daudé
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
-1 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Paul Durrant, Peter Xu, Yanan Wang, haxm-team, Colin Xu,
Stefano Stabellini, David Hildenbrand, Kamil Rytarowski,
Reinoud Zandijk, Anthony Perard, xen-devel, Laurent Vivier,
Thomas Huth, Richard Henderson, Cameron Esfahani,
Roman Bolshakov, Sunil Muthuswamy, Eduardo Habkost,
Marcelo Tosatti, Philippe Mathieu-Daudé,
Wenchao Wang, Paolo Bonzini
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v4 08/13] accel/tcg: Extract rr_create_vcpu_thread_precheck()
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Wenchao Wang, Laurent Vivier, David Hildenbrand, Yanan Wang,
Cameron Esfahani, Marcelo Tosatti, Sunil Muthuswamy,
Anthony Perard, haxm-team, Paul Durrant, Richard Henderson,
xen-devel, Philippe Mathieu-Daudé,
Roman Bolshakov, Reinoud Zandijk, Marcel Apfelbaum,
Kamil Rytarowski, Paolo Bonzini, Peter Xu, Eduardo Habkost,
Stefano Stabellini, Thomas Huth, Colin Xu
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
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v4 09/13] accel/all: Extract common_vcpu_thread_create()
2022-03-23 17:17 ` Philippe Mathieu-Daudé
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
-1 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Wenchao Wang, Laurent Vivier, David Hildenbrand, Yanan Wang,
Cameron Esfahani, Marcelo Tosatti, Sunil Muthuswamy,
Anthony Perard, haxm-team, Paul Durrant, Richard Henderson,
xen-devel, Philippe Mathieu-Daudé,
Roman Bolshakov, Reinoud Zandijk, Marcel Apfelbaum,
Kamil Rytarowski, Paolo Bonzini, Peter Xu, Eduardo Habkost,
Stefano Stabellini, Thomas Huth, Colin Xu
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v4 09/13] accel/all: Extract common_vcpu_thread_create()
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Paul Durrant, Peter Xu, Yanan Wang, haxm-team, Colin Xu,
Stefano Stabellini, David Hildenbrand, Kamil Rytarowski,
Reinoud Zandijk, Anthony Perard, xen-devel, Laurent Vivier,
Thomas Huth, Richard Henderson, Cameron Esfahani,
Roman Bolshakov, Sunil Muthuswamy, Eduardo Habkost,
Marcelo Tosatti, Philippe Mathieu-Daudé,
Wenchao Wang, Paolo Bonzini
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH v4 09/13] accel/all: Extract common_vcpu_thread_create()
2022-03-23 17:17 ` Philippe Mathieu-Daudé
(?)
@ 2022-03-23 22:11 ` Richard Henderson
-1 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-03-23 22:11 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
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~
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v4 09/13] accel/all: Extract common_vcpu_thread_create()
2022-03-23 17:17 ` Philippe Mathieu-Daudé
(?)
(?)
@ 2022-03-23 22:15 ` Richard Henderson
-1 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-03-23 22:15 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
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~
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v4 10/13] accel-ops: Introduce common_vcpu_thread_destroy() and .precheck handler
2022-03-23 17:17 ` Philippe Mathieu-Daudé
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
-1 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Paul Durrant, Peter Xu, Yanan Wang, haxm-team, Colin Xu,
Stefano Stabellini, David Hildenbrand, Kamil Rytarowski,
Reinoud Zandijk, Anthony Perard, xen-devel, Laurent Vivier,
Thomas Huth, Richard Henderson, Cameron Esfahani,
Roman Bolshakov, Sunil Muthuswamy, Eduardo Habkost,
Marcelo Tosatti, Philippe Mathieu-Daudé,
Wenchao Wang, Paolo Bonzini
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v4 10/13] accel-ops: Introduce common_vcpu_thread_destroy() and .precheck handler
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Wenchao Wang, Laurent Vivier, David Hildenbrand, Yanan Wang,
Cameron Esfahani, Marcelo Tosatti, Sunil Muthuswamy,
Anthony Perard, haxm-team, Paul Durrant, Richard Henderson,
xen-devel, Philippe Mathieu-Daudé,
Roman Bolshakov, Reinoud Zandijk, Marcel Apfelbaum,
Kamil Rytarowski, Paolo Bonzini, Peter Xu, Eduardo Habkost,
Stefano Stabellini, Thomas Huth, Colin Xu
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH v4 10/13] accel-ops: Introduce common_vcpu_thread_destroy() and .precheck handler
2022-03-23 17:17 ` Philippe Mathieu-Daudé
(?)
@ 2022-03-23 22:31 ` Richard Henderson
-1 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-03-23 22:31 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
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~
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v4 11/13] accel/tcg: Add rr_destroy_vcpu_thread_precheck()
2022-03-23 17:17 ` Philippe Mathieu-Daudé
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
-1 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Wenchao Wang, Laurent Vivier, David Hildenbrand, Yanan Wang,
Cameron Esfahani, Marcelo Tosatti, Sunil Muthuswamy,
Anthony Perard, haxm-team, Paul Durrant, Richard Henderson,
xen-devel, Philippe Mathieu-Daudé,
Roman Bolshakov, Reinoud Zandijk, Marcel Apfelbaum,
Kamil Rytarowski, Paolo Bonzini, Peter Xu, Eduardo Habkost,
Stefano Stabellini, Thomas Huth, Colin Xu
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v4 11/13] accel/tcg: Add rr_destroy_vcpu_thread_precheck()
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Paul Durrant, Peter Xu, Yanan Wang, haxm-team, Colin Xu,
Stefano Stabellini, David Hildenbrand, Kamil Rytarowski,
Reinoud Zandijk, Anthony Perard, xen-devel, Laurent Vivier,
Thomas Huth, Richard Henderson, Cameron Esfahani,
Roman Bolshakov, Sunil Muthuswamy, Eduardo Habkost,
Marcelo Tosatti, Philippe Mathieu-Daudé,
Wenchao Wang, Paolo Bonzini
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH v4 11/13] accel/tcg: Add rr_destroy_vcpu_thread_precheck()
2022-03-23 17:17 ` Philippe Mathieu-Daudé
(?)
@ 2022-03-23 22:42 ` Richard Henderson
-1 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-03-23 22:42 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
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~
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v4 12/13] softmmu/cpus: Free cpu->thread in generic_destroy_vcpu_thread()
2022-03-23 17:17 ` Philippe Mathieu-Daudé
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
-1 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Paul Durrant, Peter Xu, Yanan Wang, haxm-team, Colin Xu,
Stefano Stabellini, David Hildenbrand, Kamil Rytarowski,
Reinoud Zandijk, Anthony Perard, xen-devel, Laurent Vivier,
Thomas Huth, Richard Henderson, Cameron Esfahani,
Roman Bolshakov, Sunil Muthuswamy, Eduardo Habkost,
Marcelo Tosatti, Philippe Mathieu-Daudé,
Wenchao Wang, Paolo Bonzini
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v4 12/13] softmmu/cpus: Free cpu->thread in generic_destroy_vcpu_thread()
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Wenchao Wang, Laurent Vivier, David Hildenbrand, Yanan Wang,
Cameron Esfahani, Marcelo Tosatti, Sunil Muthuswamy,
Anthony Perard, haxm-team, Paul Durrant, Richard Henderson,
xen-devel, Philippe Mathieu-Daudé,
Roman Bolshakov, Reinoud Zandijk, Marcel Apfelbaum,
Kamil Rytarowski, Paolo Bonzini, Peter Xu, Eduardo Habkost,
Stefano Stabellini, Thomas Huth, Colin Xu, Mark Kanda
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH v4 12/13] softmmu/cpus: Free cpu->thread in generic_destroy_vcpu_thread()
2022-03-23 17:17 ` Philippe Mathieu-Daudé
@ 2022-03-23 17:30 ` Mark Kanda
-1 siblings, 0 replies; 47+ messages in thread
From: Mark Kanda @ 2022-03-23 17:30 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Paul Durrant, Peter Xu, Yanan Wang, haxm-team, Colin Xu,
Stefano Stabellini, David Hildenbrand, Kamil Rytarowski,
Reinoud Zandijk, Anthony Perard, xen-devel, Laurent Vivier,
Thomas Huth, Richard Henderson, Cameron Esfahani,
Roman Bolshakov, Sunil Muthuswamy, Eduardo Habkost,
Marcelo Tosatti, Philippe Mathieu-Daudé,
Wenchao Wang, Paolo Bonzini
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)
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v4 12/13] softmmu/cpus: Free cpu->thread in generic_destroy_vcpu_thread()
@ 2022-03-23 17:30 ` Mark Kanda
0 siblings, 0 replies; 47+ messages in thread
From: Mark Kanda @ 2022-03-23 17:30 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Wenchao Wang, Laurent Vivier, David Hildenbrand, Yanan Wang,
Cameron Esfahani, Marcelo Tosatti, Sunil Muthuswamy,
Anthony Perard, haxm-team, Paul Durrant, Richard Henderson,
xen-devel, Philippe Mathieu-Daudé,
Roman Bolshakov, Reinoud Zandijk, Marcel Apfelbaum,
Kamil Rytarowski, Paolo Bonzini, Peter Xu, Eduardo Habkost,
Stefano Stabellini, Thomas Huth, Colin Xu
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)
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v4 12/13] softmmu/cpus: Free cpu->thread in generic_destroy_vcpu_thread()
2022-03-23 17:17 ` Philippe Mathieu-Daudé
(?)
(?)
@ 2022-03-23 22:42 ` Richard Henderson
-1 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-03-23 22:42 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
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~
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v4 13/13] softmmu/cpus: Free cpu->halt_cond in generic_destroy_vcpu_thread()
2022-03-23 17:17 ` Philippe Mathieu-Daudé
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
-1 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Paul Durrant, Peter Xu, Yanan Wang, haxm-team, Colin Xu,
Stefano Stabellini, David Hildenbrand, Kamil Rytarowski,
Reinoud Zandijk, Anthony Perard, xen-devel, Laurent Vivier,
Thomas Huth, Richard Henderson, Cameron Esfahani,
Roman Bolshakov, Sunil Muthuswamy, Eduardo Habkost,
Marcelo Tosatti, Philippe Mathieu-Daudé,
Wenchao Wang, Paolo Bonzini
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v4 13/13] softmmu/cpus: Free cpu->halt_cond in generic_destroy_vcpu_thread()
@ 2022-03-23 17:17 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 47+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-23 17:17 UTC (permalink / raw)
To: qemu-devel
Cc: Wenchao Wang, Laurent Vivier, David Hildenbrand, Yanan Wang,
Cameron Esfahani, Marcelo Tosatti, Sunil Muthuswamy,
Anthony Perard, haxm-team, Paul Durrant, Richard Henderson,
xen-devel, Philippe Mathieu-Daudé,
Roman Bolshakov, Reinoud Zandijk, Marcel Apfelbaum,
Kamil Rytarowski, Paolo Bonzini, Peter Xu, Eduardo Habkost,
Stefano Stabellini, Thomas Huth, Colin Xu, Mark Kanda
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH v4 13/13] softmmu/cpus: Free cpu->halt_cond in generic_destroy_vcpu_thread()
2022-03-23 17:17 ` Philippe Mathieu-Daudé
(?)
@ 2022-03-23 22:43 ` Richard Henderson
-1 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-03-23 22:43 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
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~
^ permalink raw reply [flat|nested] 47+ messages in thread