Hi David Hildenbrand:
On 14.09.23 15:00, lixianglai wrote:
Hi David:
Hi!
On 12.09.23 04:11, xianglai li wrote:
Introduce new function to destroy CPU address space resources
for cpu hot-(un)plug.
How do other archs handle that? Or how are they able to get away
without destroying?
They do not remove the cpu address space, taking the X86 architecture as
an example:
1.Start the x86 VM:
./qemu-system-x86_64 \
-machine q35 \
-cpu Broadwell-IBRS \
-smp 1,maxcpus=100,sockets=100,cores=1,threads=1 \
-m 4G \
-drive file=~/anolis-8.8.qcow2 \
-serial stdio \
-monitor telnet:localhost:4498,server,nowait \
-nographic
2.Connect the qemu monitor
telnet 127.0.0.1 4498
info mtree
address-space: cpu-memory-0
address-space: memory
0000000000000000-ffffffffffffffff (prio 0, i/o): system
0000000000000000-000000007fffffff (prio 0, ram): alias
ram-below-4g
@pc.ram 0000000000000000-000000007fffffff
0000000000000000-ffffffffffffffff (prio -1, i/o): pci
00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem
3.Perform cpu hot swap int qemu monitor
device_add
Broadwell-IBRS-x86_64-cpu,socket-id=1,core-id=0,thread-id=0,id=cpu1
device_del cpu1
Hm, doesn't seem to work for me on upstream QEMU for some reason:
"Error: acpi: device unplug request for not supported device type:
Broadwell-IBRS-x86_64-cpu"
What happens if you re-add that CPU? Will we reuse the previous
address space?
Here is the memory layout where I inserted cpu1 again. It does not
appear that the original address space was reused, and the address space
is now duplicated
info mtree
address-space: cpu-memory-0
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: memory
0000000000000000-ffffffffffffffff (prio 0, i/o): system
0000000000000000-000000007fffffff (prio 0, ram): alias ram-below-4g
@pc.ram 0000000000000000-000000007fffffff
0000000000000000-ffffffffffffffff (prio -1, i/o): pci
00000000000a0000-00000000000affff (prio 2, ram): alias vga.chain4
@vga.vram 0000000000000000-000000000000ffff
00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem
00000000000c0000-00000000000dffff (prio 1, rom): pc.rom
00000000000e0000-00000000000fffff (prio 1, rom): alias isa-bios
@pc.bios 0000000000020000-000000000003ffff
00000000fd000000-00000000fdffffff (prio 1, ram): vga.vram
In addition, I do not find the corresponding resource release action for
cpu->cpu_ases requested in function cpu_address_space_init.
I wonder if there is a leak in the memory space requested here. Maybe
qemu automatically reclaims memory space
or frees resources somewhere else I didn't find? I thought I'd try
running the following valgrind to see if I could verify my suspicions.
void cpu_address_space_init(CPUState *cpu, int asidx,
const char *prefix, MemoryRegion *mr)
{
...
if (!cpu->cpu_ases) {
cpu->cpu_ases = g_new0(CPUAddressSpace, cpu->num_ases);
}
...
}
info mtree
address-space: cpu-memory-0
address-space: cpu-memory-1
address-space: memory
0000000000000000-ffffffffffffffff (prio 0, i/o): system
0000000000000000-000000007fffffff (prio 0, ram): alias
ram-below-4g
@pc.ram 0000000000000000-000000007fffffff
0000000000000000-ffffffffffffffff (prio -1, i/o): pci
00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem
From the above test, you can see whether the address space of cpu1 is
residual after a cpu hot swap, and whether it is reasonable?
Probably we should teach other archs to destroy that address space as
well.
Can we do that from the core, instead of having to do that in each CPU
unrealize function?
I think it can also be done in the public code flow. Since I refer to
arm's scheme
(https://lore.kernel.org/all/20200613213629.21984-1-salil.me...@huawei.com/),
and arm's patch will be issued soon, I will conduct rebase based on arm
patch in the future.
Therefore, I would like to see if arm has any good suggestions. If there
are no good suggestions at this stage,
I think we can shelve this problem for the first time, and I can
consider not referencing this function for the first time,
and we can submit another patch to solve this problem.
Hi Salil Mehta:
Is the cpu_address_space_destroy function still present in the new patch
version of arm?
Can we put this function on the public path of cpu destroy?
Thanks,
xianglai.
