Skip to content

Commit 9d061df

Browse files
pm215Michael Tokarev
authored andcommitted
physmem: Destroy all CPU AddressSpaces on unrealize
When we unrealize a CPU object (which happens on vCPU hot-unplug), we should destroy all the AddressSpace objects we created via calls to cpu_address_space_init() when the CPU was realized. Commit 24bec42 added a function to do this for a specific AddressSpace, but did not add any places where the function was called. Since we always want to destroy all the AddressSpaces on unrealize, regardless of the target architecture, we don't need to try to keep track of how many are still undestroyed, or make the target architecture code manually call a destroy function for each AS it created. Instead we can adjust the function to always completely destroy the whole cpu->ases array, and arrange for it to be called during CPU unrealize as part of the common code. Without this fix, AddressSanitizer will report a leak like this from a run where we hot-plugged and then hot-unplugged an x86 KVM vCPU: Direct leak of 416 byte(s) in 1 object(s) allocated from: #0 0x5b638565053d in calloc (/data_nvme1n1/linaro/qemu-from-laptop/qemu/build/x86-tgts-asan/qemu-system-x86_64+0x1ee153d) (BuildId: c1cd6022b195142106e1bffeca23498c2b752bca) #1 0x7c28083f77b1 in g_malloc0 (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x637b1) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75) #2 0x5b6386999c7c in cpu_address_space_init /data_nvme1n1/linaro/qemu-from-laptop/qemu/build/x86-tgts-asan/../../system/physmem.c:797:25 #3 0x5b638727f049 in kvm_cpu_realizefn /data_nvme1n1/linaro/qemu-from-laptop/qemu/build/x86-tgts-asan/../../target/i386/kvm/kvm-cpu.c:102:5 #4 0x5b6385745f40 in accel_cpu_common_realize /data_nvme1n1/linaro/qemu-from-laptop/qemu/build/x86-tgts-asan/../../accel/accel-common.c:101:13 #5 0x5b638568fe3c in cpu_exec_realizefn /data_nvme1n1/linaro/qemu-from-laptop/qemu/build/x86-tgts-asan/../../hw/core/cpu-common.c:232:10 #6 0x5b63874a2cd5 in x86_cpu_realizefn /data_nvme1n1/linaro/qemu-from-laptop/qemu/build/x86-tgts-asan/../../target/i386/cpu.c:9321:5 #7 0x5b6387a0469a in device_set_realized /data_nvme1n1/linaro/qemu-from-laptop/qemu/build/x86-tgts-asan/../../hw/core/qdev.c:494:13 #8 0x5b6387a27d9e in property_set_bool /data_nvme1n1/linaro/qemu-from-laptop/qemu/build/x86-tgts-asan/../../qom/object.c:2375:5 #9 0x5b6387a2090b in object_property_set /data_nvme1n1/linaro/qemu-from-laptop/qemu/build/x86-tgts-asan/../../qom/object.c:1450:5 #10 0x5b6387a35b05 in object_property_set_qobject /data_nvme1n1/linaro/qemu-from-laptop/qemu/build/x86-tgts-asan/../../qom/qom-qobject.c:28:10 #11 0x5b6387a21739 in object_property_set_bool /data_nvme1n1/linaro/qemu-from-laptop/qemu/build/x86-tgts-asan/../../qom/object.c:1520:15 #12 0x5b63879fe510 in qdev_realize /data_nvme1n1/linaro/qemu-from-laptop/qemu/build/x86-tgts-asan/../../hw/core/qdev.c:276:12 Cc: qemu-stable@nongnu.org Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2517 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: David Hildenbrand <david@redhat.com> Link: https://lore.kernel.org/r/20250929144228.1994037-4-peter.maydell@linaro.org Signed-off-by: Peter Xu <peterx@redhat.com> (cherry picked from commit 300a87c) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
1 parent 470f16d commit 9d061df

6 files changed

Lines changed: 37 additions & 23 deletions

File tree

hw/core/cpu-common.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,7 @@ void cpu_exec_unrealizefn(CPUState *cpu)
248248
* accel_cpu_common_unrealize, which may free fields using call_rcu.
249249
*/
250250
accel_cpu_common_unrealize(cpu);
251+
cpu_destroy_address_spaces(cpu);
251252
}
252253

253254
static void cpu_common_initfn(Object *obj)

include/exec/cpu-common.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -132,13 +132,13 @@ size_t qemu_ram_pagesize_largest(void);
132132
void cpu_address_space_init(CPUState *cpu, int asidx,
133133
const char *prefix, MemoryRegion *mr);
134134
/**
135-
* cpu_address_space_destroy:
136-
* @cpu: CPU for which address space needs to be destroyed
137-
* @asidx: integer index of this address space
135+
* cpu_destroy_address_spaces:
136+
* @cpu: CPU for which address spaces need to be destroyed
138137
*
139-
* Note that with KVM only one address space is supported.
138+
* Destroy all address spaces associated with this CPU; this
139+
* is called as part of unrealizing the CPU.
140140
*/
141-
void cpu_address_space_destroy(CPUState *cpu, int asidx);
141+
void cpu_destroy_address_spaces(CPUState *cpu);
142142

143143
void cpu_physical_memory_rw(hwaddr addr, void *buf,
144144
hwaddr len, bool is_write);

include/hw/core/cpu.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -500,7 +500,6 @@ struct CPUState {
500500
QSIMPLEQ_HEAD(, qemu_work_item) work_list;
501501

502502
struct CPUAddressSpace *cpu_ases;
503-
int cpu_ases_count;
504503
int num_ases;
505504
AddressSpace *as;
506505
MemoryRegion *memory;

stubs/cpu-destroy-address-spaces.c

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/* SPDX-License-Identifier: GPL-2.0-or-later */
2+
3+
#include "qemu/osdep.h"
4+
#include "exec/cpu-common.h"
5+
6+
/*
7+
* user-mode CPUs never create address spaces with
8+
* cpu_address_space_init(), so the cleanup function doesn't
9+
* need to do anything. We need this stub because cpu-common.c
10+
* is built-once so it can't #ifndef CONFIG_USER around the
11+
* call; the real function is in physmem.c which is system-only.
12+
*/
13+
void cpu_destroy_address_spaces(CPUState *cpu)
14+
{
15+
}

stubs/meson.build

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ endif
5555
if have_user
5656
# Symbols that are used by hw/core.
5757
stub_ss.add(files('cpu-synchronize-state.c'))
58+
stub_ss.add(files('cpu-destroy-address-spaces.c'))
5859

5960
# Stubs for QAPI events. Those can always be included in the build, but
6061
# they are not built at all for --disable-system builds.

system/physmem.c

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -765,7 +765,6 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
765765

766766
if (!cpu->cpu_ases) {
767767
cpu->cpu_ases = g_new0(CPUAddressSpace, cpu->num_ases);
768-
cpu->cpu_ases_count = cpu->num_ases;
769768
}
770769

771770
newas = &cpu->cpu_ases[asidx];
@@ -779,30 +778,29 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
779778
}
780779
}
781780

782-
void cpu_address_space_destroy(CPUState *cpu, int asidx)
781+
void cpu_destroy_address_spaces(CPUState *cpu)
783782
{
784783
CPUAddressSpace *cpuas;
784+
int asidx;
785785

786786
assert(cpu->cpu_ases);
787-
assert(asidx >= 0 && asidx < cpu->num_ases);
788787

789-
cpuas = &cpu->cpu_ases[asidx];
790-
if (tcg_enabled()) {
791-
memory_listener_unregister(&cpuas->tcg_as_listener);
792-
}
788+
/* convenience alias just points to some cpu_ases[n] */
789+
cpu->as = NULL;
793790

794-
address_space_destroy(cpuas->as);
795-
g_free_rcu(cpuas->as, rcu);
796-
797-
if (asidx == 0) {
798-
/* reset the convenience alias for address space 0 */
799-
cpu->as = NULL;
791+
for (asidx = 0; asidx < cpu->num_ases; asidx++) {
792+
cpuas = &cpu->cpu_ases[asidx];
793+
if (!cpuas->as) {
794+
/* This index was never initialized; no deinit needed */
795+
continue;
796+
}
797+
if (tcg_enabled()) {
798+
memory_listener_unregister(&cpuas->tcg_as_listener);
799+
}
800+
g_clear_pointer(&cpuas->as, address_space_destroy_free);
800801
}
801802

802-
if (--cpu->cpu_ases_count == 0) {
803-
g_free(cpu->cpu_ases);
804-
cpu->cpu_ases = NULL;
805-
}
803+
g_clear_pointer(&cpu->cpu_ases, g_free);
806804
}
807805

808806
AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)

0 commit comments

Comments
 (0)