Re: [PATCH v2 04/10] Introduce the CPU address space destruction function

[lixianglai]

Hi David Hildenbrand:

> 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"
First I use qemu tcg, and then the cpu needs to be removed after the 
operating system is booted.

Thanks,

xianglai.


> > 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.mehta@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.