Hi Xianglai,
> From: lixianglai <lixiang...@loongson.cn>
> Sent: Friday, September 15, 2023 3:48 AM
> To: David Hildenbrand <da...@redhat.com>; qemu-devel@nongnu.org; Salil
> Mehta <salil.me...@huawei.com>
> Cc: Salil Mehta <salil.me...@opnsrc.net>; Xiaojuan Yang
> <yangxiaoj...@loongson.cn>; Song Gao <gaos...@loongson.cn>; Michael S.
> Tsirkin <m...@redhat.com>; Igor Mammedov <imamm...@redhat.com>; Ani Sinha
> <anisi...@redhat.com>; Paolo Bonzini <pbonz...@redhat.com>; Richard
> Henderson <richard.hender...@linaro.org>; Eduardo Habkost
> <edua...@habkost.net>; Marcel Apfelbaum <marcel.apfelb...@gmail.com>;
> Philippe Mathieu-Daudé <phi...@linaro.org>; wangyanan (Y)
> <wangyana...@huawei.com>; Daniel P. Berrangé <berra...@redhat.com>; Peter
> Xu <pet...@redhat.com>; Bibo Mao <maob...@loongson.cn>
> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space destruction
> function
>
>
> 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.
Here it is:
https://lore.kernel.org/qemu-devel/20230926100436.28284-1-salil.me...@huawei.com/T/#m523b37819c4811c7827333982004e07a1ef03879
>
> 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?
Yes, this is present in the RFC V2. Please find it here
https://lore.kernel.org/qemu-devel/20230926100436.28284-1-salil.me...@huawei.com/T/#mfb2a525081c412917a0026d558e72f48875e386d
>
> Can we put this function on the public path of cpu destroy?
Yes, AddressSpace destruction is already part of the Architecture
agnostic patches. Please rebase you patch-set and you will
see your bugs disappearing :)
Thanks
Salil.
