[v2,04/10] Introduce the CPU address space destruction function

Commit Message

Xianglai Li Sept. 12, 2023, 2:11 a.m. UTC

Introduce new function to destroy CPU address space resources
for cpu hot-(un)plug.

Co-authored-by: "Salil Mehta" <[e-mail]>
Cc: "Salil Mehta" <[e-mail]>
Cc: Xiaojuan Yang <[e-mail]>
Cc: Song Gao <[e-mail]>
Cc: "Michael S. Tsirkin" <[e-mail]>
Cc: Igor Mammedov <[e-mail]>
Cc: Ani Sinha <[e-mail]>
Cc: Paolo Bonzini <[e-mail]>
Cc: Richard Henderson <[e-mail]>
Cc: Eduardo Habkost <[e-mail]>
Cc: Marcel Apfelbaum <[e-mail]>
Cc: "Philippe Mathieu-Daudé" <[e-mail]>
Cc: Yanan Wang <[e-mail]>
Cc: "Daniel P. Berrangé" <[e-mail]>
Cc: Peter Xu <[e-mail]>
Cc: David Hildenbrand <[e-mail]>
Cc: Bibo Mao <[e-mail]>
Signed-off-by: xianglai li <[e-mail]>
---
 include/exec/cpu-common.h |  8 ++++++++
 include/hw/core/cpu.h     |  1 +
 softmmu/physmem.c         | 24 ++++++++++++++++++++++++
 3 files changed, 33 insertions(+)

Comments

David Hildenbrand Sept. 12, 2023, 7 a.m. UTC | #1

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?

Xianglai Li Sept. 14, 2023, 1 p.m. UTC | #2

Hi David:

> 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

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?

The address space destruction function of the CPU can be used to delete 
the residual address space of the CPU1.

Thanks,

xianglai.

David Hildenbrand Sept. 14, 2023, 1:26 p.m. UTC | #3

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?

> 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?

Xianglai Li Sept. 15, 2023, 2:48 a.m. UTC | #4

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/[e-mail]/), 


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.

Xianglai Li Sept. 15, 2023, 2:53 a.m. UTC | #5

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/[e-mail]/), 
>
>
> 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.
>

David Hildenbrand Sept. 15, 2023, 8:07 a.m. UTC | #6

On 15.09.23 04:53, lixianglai wrote:
> 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.

Ah, the last thing is the important bit. I can reproduce this with KVM 
easily.

Doing it a couple of times

address-space: cpu-memory-0
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1

Looks like a resource/memory leak.

[...]

>> I think it can also be done in the public code flow. Since I refer to
>> arm's scheme
>>
>> (https://lore.kernel.org/all/[e-mail]/),
>>
>>
>> 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?

Looks like this has to be fixed for all archs that support VCPU unplug.

The CPU implementation end up call qemu_init_vcpu() in their realize 
function; there should be something like qemu_destroy_vcpu() on the 
unrealize path that takes care of undoing any cpu_address_space_init().

We seem to have cpu_common_unrealizefn()->cpu_exec_unrealizefn() but 
that doesn't take care of address spaces.

Also, in qemu_init_vcpu() we do a cpus_accel->create_vcpu_thread(cpu). 
I'm, curious if we destroy that thread somehow.

Xianglai Li Sept. 15, 2023, 9:54 a.m. UTC | #7

Hi David :
> On 15.09.23 04:53, lixianglai wrote:
>> 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.
>
> Ah, the last thing is the important bit. I can reproduce this with KVM 
> easily.
>
> Doing it a couple of times
>
> address-space: cpu-memory-0
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
>
> Looks like a resource/memory leak.
>
> [...]
>
>>> I think it can also be done in the public code flow. Since I refer to
>>> arm's scheme
>>>
>>> (https://lore.kernel.org/all/[e-mail]/), 
>>>
>>>
>>>
>>> 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?
>
> Looks like this has to be fixed for all archs that support VCPU unplug.
>
> The CPU implementation end up call qemu_init_vcpu() in their realize 
> function; there should be something like qemu_destroy_vcpu() on the 
> unrealize path that takes care of undoing any cpu_address_space_init().
>
> We seem to have cpu_common_unrealizefn()->cpu_exec_unrealizefn() but 
> that doesn't take care of address spaces.
>
> Also, in qemu_init_vcpu() we do a cpus_accel->create_vcpu_thread(cpu). 
> I'm, curious if we destroy that thread somehow.
>

Although there is no visible destory of the vcpu thread, no trace of the 
thread was found when debugging through gdb,

it should be the loop in the thread ended the thread naturally exited.

Thanks,

xianglai.

Philippe Mathieu-Daudé Sept. 15, 2023, 2:19 p.m. UTC | #8

On 15/9/23 10:07, David Hildenbrand wrote:

> The CPU implementation end up call qemu_init_vcpu() in their realize 
> function; there should be something like qemu_destroy_vcpu() on the 
> unrealize path that takes care of undoing any cpu_address_space_init().
> 
> We seem to have cpu_common_unrealizefn()->cpu_exec_unrealizefn() but 
> that doesn't take care of address spaces.
> 
> Also, in qemu_init_vcpu() we do a cpus_accel->create_vcpu_thread(cpu). 
> I'm, curious if we destroy that thread somehow.

Implicitly in cpu_remove_sync().

David Hildenbrand Sept. 15, 2023, 3:22 p.m. UTC | #9

On 15.09.23 16:19, Philippe Mathieu-Daudé wrote:
> On 15/9/23 10:07, David Hildenbrand wrote:
> 
>> The CPU implementation end up call qemu_init_vcpu() in their realize
>> function; there should be something like qemu_destroy_vcpu() on the
>> unrealize path that takes care of undoing any cpu_address_space_init().
>>
>> We seem to have cpu_common_unrealizefn()->cpu_exec_unrealizefn() but
>> that doesn't take care of address spaces.
>>
>> Also, in qemu_init_vcpu() we do a cpus_accel->create_vcpu_thread(cpu).
>> I'm, curious if we destroy that thread somehow.
> 
> Implicitly in cpu_remove_sync().

Ah, indeed. there is the qemu_thread_join().

Salil Mehta Sept. 26, 2023, 11:03 a.m. UTC | #10

Hi David,

> From: qemu-devel-bounces+salil.mehta=[e-mail] <qemu-devel-
> bounces+salil.mehta=[e-mail]> On Behalf Of David Hildenbrand
> Sent: Tuesday, September 12, 2023 8:00 AM
> To: xianglai li <[e-mail]>; [e-mail]
> Cc: Salil Mehta <[e-mail]>; Xiaojuan Yang
> <[e-mail]>; Song Gao <[e-mail]>; Michael S.
> Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani Sinha
> <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
> Henderson <[e-mail]>; Eduardo Habkost
> <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
> Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
> <[e-mail]>; Daniel P. Berrangé <[e-mail]>; Peter
> Xu <[e-mail]>; Bibo Mao <[e-mail]>
> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space destruction
> function
> 
> 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?

This patch-set is based on the ARM RFC. We do destroy AddressSpace there.
Is there any reason you are hinting why it should not be done?

I have posted the RFC V2 Virtual CPU Hotplug Support on ARM today and
You are CC'ed in it. Please have a look at the implementation:

https://lore.kernel.org/qemu-devel/[e-mail]/T/#m523b37819c4811c7827333982004e07a1ef03879

Thanks
Salil.

Salil Mehta Sept. 26, 2023, 11:06 a.m. UTC | #11

> From: qemu-devel-bounces+salil.mehta=[e-mail] <qemu-devel-
> bounces+salil.mehta=[e-mail]> On Behalf Of lixianglai
> Sent: Thursday, September 14, 2023 2:01 PM
> To: David Hildenbrand <[e-mail]>; [e-mail]
> Cc: Salil Mehta <[e-mail]>; Xiaojuan Yang
> <[e-mail]>; Song Gao <[e-mail]>; Michael S.
> Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani Sinha
> <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
> Henderson <[e-mail]>; Eduardo Habkost
> <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
> Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
> <[e-mail]>; Daniel P. Berrangé <[e-mail]>; Peter
> Xu <[e-mail]>; Bibo Mao <[e-mail]>
> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space destruction
> function
> 
> Hi David:
> 
> > 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
> 
> 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?
> 
> The address space destruction function of the CPU can be used to delete
> the residual address space of the CPU1.

Yes, it can be but I would interested to know if there is any problem
which might have gone unnoticed while doing it.

Many thanks
Salil.

Salil Mehta Sept. 26, 2023, 11:11 a.m. UTC | #12

Hi Xianglai,

> From: lixianglai <[e-mail]>
> Sent: Friday, September 15, 2023 3:48 AM
> To: David Hildenbrand <[e-mail]>; [e-mail]; Salil
> Mehta <[e-mail]>
> Cc: Salil Mehta <[e-mail]>; Xiaojuan Yang
> <[e-mail]>; Song Gao <[e-mail]>; Michael S.
> Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani Sinha
> <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
> Henderson <[e-mail]>; Eduardo Habkost
> <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
> Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
> <[e-mail]>; Daniel P. Berrangé <[e-mail]>; Peter
> Xu <[e-mail]>; Bibo Mao <[e-mail]>
> 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-
> [e-mail]/),
> 
> 
> 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/[e-mail]/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/[e-mail]/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.

Salil Mehta Sept. 26, 2023, 11:21 a.m. UTC | #13

Hi David,

> From: David Hildenbrand <[e-mail]>
> Sent: Friday, September 15, 2023 9:07 AM
> To: lixianglai <[e-mail]>; [e-mail]; Salil Mehta
> <[e-mail]>
> Cc: Salil Mehta <[e-mail]>; Xiaojuan Yang
> <[e-mail]>; Song Gao <[e-mail]>; Michael S.
> Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani Sinha
> <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
> Henderson <[e-mail]>; Eduardo Habkost
> <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
> Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
> <[e-mail]>; Daniel P. Berrangé <[e-mail]>; Peter
> Xu <[e-mail]>; Bibo Mao <[e-mail]>
> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space destruction
> function
> 
> On 15.09.23 04:53, lixianglai wrote:
> > 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.
> 
> Ah, the last thing is the important bit. I can reproduce this with KVM
> easily.
> 
> Doing it a couple of times
> 
> address-space: cpu-memory-0
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> 
> Looks like a resource/memory leak.

Yes, there was. Thanks for identifying it. I have fixed in the
latest RFC V2. Please check here:

https://lore.kernel.org/qemu-devel/[e-mail]/T/#m5f5ae40b091d69d01012880d7500d96874a9d39c

I have tested and AddressSpace comes and goes away cleanly
on CPU hot(un)plug action.



[...]


> >> 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?
> 
> Looks like this has to be fixed for all archs that support VCPU unplug.
> 
> The CPU implementation end up call qemu_init_vcpu() in their realize
> function; there should be something like qemu_destroy_vcpu() on the
> unrealize path that takes care of undoing any cpu_address_space_init().
> 
> We seem to have cpu_common_unrealizefn()->cpu_exec_unrealizefn() but
> that doesn't take care of address spaces.

Yes, the current concept can be extended

> 
> Also, in qemu_init_vcpu() we do a cpus_accel->create_vcpu_thread(cpu).
> I'm, curious if we destroy that thread somehow.


Yes we do. In ARM RFC, this happens after CPU has been unplugged and
CPU is being destroyed and unparented (all of this happens in context
ACPI _EJx method evaluated by OSPM). This eventually leads to
architecture specific CPU unrealize and call to cpu_remove_sync().


Thanks
Salil.

Salil Mehta Sept. 26, 2023, 11:25 a.m. UTC | #14

> From: David Hildenbrand <[e-mail]>
> Sent: Friday, September 15, 2023 4:22 PM
> To: Philippe Mathieu-Daudé <[e-mail]>; lixianglai
> <[e-mail]>; [e-mail]; Salil Mehta
> <[e-mail]>
> Cc: Salil Mehta <[e-mail]>; Xiaojuan Yang
> <[e-mail]>; Song Gao <[e-mail]>; Michael S.
> Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani Sinha
> <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
> Henderson <[e-mail]>; Eduardo Habkost
> <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
> wangyanan (Y) <[e-mail]>; Daniel P. Berrangé
> <[e-mail]>; Peter Xu <[e-mail]>; Bibo Mao
> <[e-mail]>
> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space destruction
> function
> 
> On 15.09.23 16:19, Philippe Mathieu-Daudé wrote:
> > On 15/9/23 10:07, David Hildenbrand wrote:
> >
> >> The CPU implementation end up call qemu_init_vcpu() in their realize
> >> function; there should be something like qemu_destroy_vcpu() on the
> >> unrealize path that takes care of undoing any cpu_address_space_init().
> >>
> >> We seem to have cpu_common_unrealizefn()->cpu_exec_unrealizefn() but
> >> that doesn't take care of address spaces.
> >>
> >> Also, in qemu_init_vcpu() we do a cpus_accel->create_vcpu_thread(cpu).
> >> I'm, curious if we destroy that thread somehow.
> >
> > Implicitly in cpu_remove_sync().
> 
> Ah, indeed. there is the qemu_thread_join().

Correct. This one. 

For ARM, this was necessary because of the design approach
we have taken as a workaround to the ARM architecture limitations.
Th KVM needs to know everything at the VM initialization time.

Request to help review the patches:
https://lore.kernel.org/qemu-devel/[e-mail]/T/#m523b37819c4811c7827333982004e07a1ef03879


Thanks
Salil.

Salil Mehta Sept. 26, 2023, 11:55 a.m. UTC | #15

> From: Salil Mehta
> Sent: Tuesday, September 26, 2023 12:21 PM
> To: 'David Hildenbrand' <[e-mail]>; lixianglai
> <[e-mail]>; [e-mail]
> Cc: Salil Mehta <[e-mail]>; Xiaojuan Yang
> <[e-mail]>; Song Gao <[e-mail]>; Michael S.
> Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani Sinha
> <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
> Henderson <[e-mail]>; Eduardo Habkost
> <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
> Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
> <[e-mail]>; Daniel P. Berrangé <[e-mail]>; Peter
> Xu <[e-mail]>; Bibo Mao <[e-mail]>
> Subject: RE: [PATCH v2 04/10] Introduce the CPU address space destruction
> function
> 
> Hi David,
> 
> > From: David Hildenbrand <[e-mail]>
> > Sent: Friday, September 15, 2023 9:07 AM
> > To: lixianglai <[e-mail]>; [e-mail]; Salil
> Mehta
> > <[e-mail]>
> > Cc: Salil Mehta <[e-mail]>; Xiaojuan Yang
> > <[e-mail]>; Song Gao <[e-mail]>; Michael S.
> > Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani Sinha
> > <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
> > Henderson <[e-mail]>; Eduardo Habkost
> > <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
> > Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
> > <[e-mail]>; Daniel P. Berrangé <[e-mail]>; Peter
> > Xu <[e-mail]>; Bibo Mao <[e-mail]>
> > Subject: Re: [PATCH v2 04/10] Introduce the CPU address space destruction
> > function
> >
> > On 15.09.23 04:53, lixianglai wrote:
> > > 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.
> >
> > Ah, the last thing is the important bit. I can reproduce this with KVM
> > easily.
> >
> > Doing it a couple of times
> >
> > address-space: cpu-memory-0
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> >
> > Looks like a resource/memory leak.
> 
> Yes, there was. Thanks for identifying it. I have fixed in the
> latest RFC V2. Please check here:
> 
> https://lore.kernel.org/qemu-devel/20230926100436.28284-1-
> [e-mail]/T/#m5f5ae40b091d69d01012880d7500d96874a9d39c
> 
> I have tested and AddressSpace comes and goes away cleanly
> on CPU hot(un)plug action.

Hi David/Xianglai,
Are you okay if I put Reported-by and give reference to this
conversation?

Many thanks
Salil

David Hildenbrand Sept. 26, 2023, 12:23 p.m. UTC | #16

On 26.09.23 13:55, Salil Mehta wrote:
>> From: Salil Mehta
>> Sent: Tuesday, September 26, 2023 12:21 PM
>> To: 'David Hildenbrand' <[e-mail]>; lixianglai
>> <[e-mail]>; [e-mail]
>> Cc: Salil Mehta <[e-mail]>; Xiaojuan Yang
>> <[e-mail]>; Song Gao <[e-mail]>; Michael S.
>> Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani Sinha
>> <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
>> Henderson <[e-mail]>; Eduardo Habkost
>> <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
>> Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
>> <[e-mail]>; Daniel P. Berrangé <[e-mail]>; Peter
>> Xu <[e-mail]>; Bibo Mao <[e-mail]>
>> Subject: RE: [PATCH v2 04/10] Introduce the CPU address space destruction
>> function
>>
>> Hi David,
>>
>>> From: David Hildenbrand <[e-mail]>
>>> Sent: Friday, September 15, 2023 9:07 AM
>>> To: lixianglai <[e-mail]>; [e-mail]; Salil
>> Mehta
>>> <[e-mail]>
>>> Cc: Salil Mehta <[e-mail]>; Xiaojuan Yang
>>> <[e-mail]>; Song Gao <[e-mail]>; Michael S.
>>> Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani Sinha
>>> <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
>>> Henderson <[e-mail]>; Eduardo Habkost
>>> <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
>>> Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
>>> <[e-mail]>; Daniel P. Berrangé <[e-mail]>; Peter
>>> Xu <[e-mail]>; Bibo Mao <[e-mail]>
>>> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space destruction
>>> function
>>>
>>> On 15.09.23 04:53, lixianglai wrote:
>>>> 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.
>>>
>>> Ah, the last thing is the important bit. I can reproduce this with KVM
>>> easily.
>>>
>>> Doing it a couple of times
>>>
>>> address-space: cpu-memory-0
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>>
>>> Looks like a resource/memory leak.
>>
>> Yes, there was. Thanks for identifying it. I have fixed in the
>> latest RFC V2. Please check here:
>>
>> https://lore.kernel.org/qemu-devel/20230926100436.28284-1-
>> [e-mail]/T/#m5f5ae40b091d69d01012880d7500d96874a9d39c
>>
>> I have tested and AddressSpace comes and goes away cleanly
>> on CPU hot(un)plug action.
> 
> Hi David/Xianglai,
> Are you okay if I put Reported-by and give reference to this
> conversation?

Yes. And ideally, send the fixes separately from the other arm patches.

Salil Mehta Sept. 26, 2023, 12:32 p.m. UTC | #17

> From: David Hildenbrand <[e-mail]>
> Sent: Tuesday, September 26, 2023 1:24 PM
> 
> On 26.09.23 13:55, Salil Mehta wrote:
> >> From: Salil Mehta
> >> Sent: Tuesday, September 26, 2023 12:21 PM
> >> To: 'David Hildenbrand' <[e-mail]>; lixianglai
> >> <[e-mail]>; [e-mail]
> >> Cc: Salil Mehta <[e-mail]>; Xiaojuan Yang
> >> <[e-mail]>; Song Gao <[e-mail]>; Michael S.
> >> Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani Sinha
> >> <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
> >> Henderson <[e-mail]>; Eduardo Habkost
> >> <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
> >> Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
> >> <[e-mail]>; Daniel P. Berrangé <[e-mail]>;
> Peter
> >> Xu <[e-mail]>; Bibo Mao <[e-mail]>
> >> Subject: RE: [PATCH v2 04/10] Introduce the CPU address space
> destruction
> >> function
> >>
> >> Hi David,
> >>
> >>> From: David Hildenbrand <[e-mail]>
> >>> Sent: Friday, September 15, 2023 9:07 AM
> >>> To: lixianglai <[e-mail]>; [e-mail]; Salil
> >> Mehta
> >>> <[e-mail]>
> >>> Cc: Salil Mehta <[e-mail]>; Xiaojuan Yang
> >>> <[e-mail]>; Song Gao <[e-mail]>; Michael S.
> >>> Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani
> Sinha
> >>> <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
> >>> Henderson <[e-mail]>; Eduardo Habkost
> >>> <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
> >>> Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
> >>> <[e-mail]>; Daniel P. Berrangé <[e-mail]>;
> Peter
> >>> Xu <[e-mail]>; Bibo Mao <[e-mail]>
> >>> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space
> destruction
> >>> function
> >>>
> >>> On 15.09.23 04:53, lixianglai wrote:
> >>>> 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.
> >>>
> >>> Ah, the last thing is the important bit. I can reproduce this with KVM
> >>> easily.
> >>>
> >>> Doing it a couple of times
> >>>
> >>> address-space: cpu-memory-0
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>>
> >>> Looks like a resource/memory leak.
> >>
> >> Yes, there was. Thanks for identifying it. I have fixed in the
> >> latest RFC V2. Please check here:
> >>
> >> https://lore.kernel.org/qemu-devel/20230926100436.28284-1-
> >> [e-mail]/T/#m5f5ae40b091d69d01012880d7500d96874a9d39c
> >>
> >> I have tested and AddressSpace comes and goes away cleanly
> >> on CPU hot(un)plug action.
> >
> > Hi David/Xianglai,
> > Are you okay if I put Reported-by and give reference to this
> > conversation?
> 
> Yes. And ideally, send the fixes separately from the other arm patches.

ARM Virtual CPU Hotplug support patches are still under review.

Thanks
Salil.

David Hildenbrand Sept. 26, 2023, 12:37 p.m. UTC | #18

On 26.09.23 14:32, Salil Mehta wrote:
>> From: David Hildenbrand <[e-mail]>
>> Sent: Tuesday, September 26, 2023 1:24 PM
>>
>> On 26.09.23 13:55, Salil Mehta wrote:
>>>> From: Salil Mehta
>>>> Sent: Tuesday, September 26, 2023 12:21 PM
>>>> To: 'David Hildenbrand' <[e-mail]>; lixianglai
>>>> <[e-mail]>; [e-mail]
>>>> Cc: Salil Mehta <[e-mail]>; Xiaojuan Yang
>>>> <[e-mail]>; Song Gao <[e-mail]>; Michael S.
>>>> Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani Sinha
>>>> <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
>>>> Henderson <[e-mail]>; Eduardo Habkost
>>>> <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
>>>> Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
>>>> <[e-mail]>; Daniel P. Berrangé <[e-mail]>;
>> Peter
>>>> Xu <[e-mail]>; Bibo Mao <[e-mail]>
>>>> Subject: RE: [PATCH v2 04/10] Introduce the CPU address space
>> destruction
>>>> function
>>>>
>>>> Hi David,
>>>>
>>>>> From: David Hildenbrand <[e-mail]>
>>>>> Sent: Friday, September 15, 2023 9:07 AM
>>>>> To: lixianglai <[e-mail]>; [e-mail]; Salil
>>>> Mehta
>>>>> <[e-mail]>
>>>>> Cc: Salil Mehta <[e-mail]>; Xiaojuan Yang
>>>>> <[e-mail]>; Song Gao <[e-mail]>; Michael S.
>>>>> Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani
>> Sinha
>>>>> <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
>>>>> Henderson <[e-mail]>; Eduardo Habkost
>>>>> <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
>>>>> Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
>>>>> <[e-mail]>; Daniel P. Berrangé <[e-mail]>;
>> Peter
>>>>> Xu <[e-mail]>; Bibo Mao <[e-mail]>
>>>>> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space
>> destruction
>>>>> function
>>>>>
>>>>> On 15.09.23 04:53, lixianglai wrote:
>>>>>> 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.
>>>>>
>>>>> Ah, the last thing is the important bit. I can reproduce this with KVM
>>>>> easily.
>>>>>
>>>>> Doing it a couple of times
>>>>>
>>>>> address-space: cpu-memory-0
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>>
>>>>> Looks like a resource/memory leak.
>>>>
>>>> Yes, there was. Thanks for identifying it. I have fixed in the
>>>> latest RFC V2. Please check here:
>>>>
>>>> https://lore.kernel.org/qemu-devel/20230926100436.28284-1-
>>>> [e-mail]/T/#m5f5ae40b091d69d01012880d7500d96874a9d39c
>>>>
>>>> I have tested and AddressSpace comes and goes away cleanly
>>>> on CPU hot(un)plug action.
>>>
>>> Hi David/Xianglai,
>>> Are you okay if I put Reported-by and give reference to this
>>> conversation?
>>
>> Yes. And ideally, send the fixes separately from the other arm patches.
> 
> ARM Virtual CPU Hotplug support patches are still under review.

The other architectures (as shown, x86 is affected) can be fixed 
independent of that support.

Salil Mehta Sept. 26, 2023, 12:44 p.m. UTC | #19

> From: David Hildenbrand <[e-mail]>
> Sent: Tuesday, September 26, 2023 1:37 PM
> To: Salil Mehta <[e-mail]>; lixianglai
> <[e-mail]>; [e-mail]
> Cc: Salil Mehta <[e-mail]>; Xiaojuan Yang
> <[e-mail]>; Song Gao <[e-mail]>; Michael S.
> Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani Sinha
> <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
> Henderson <[e-mail]>; Eduardo Habkost
> <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
> Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
> <[e-mail]>; Daniel P. Berrangé <[e-mail]>; Peter
> Xu <[e-mail]>; Bibo Mao <[e-mail]>
> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space destruction
> function
> 
> On 26.09.23 14:32, Salil Mehta wrote:
> >> From: David Hildenbrand <[e-mail]>
> >> Sent: Tuesday, September 26, 2023 1:24 PM
> >>
> >> On 26.09.23 13:55, Salil Mehta wrote:
> >>>> From: Salil Mehta
> >>>> Sent: Tuesday, September 26, 2023 12:21 PM
> >>>> To: 'David Hildenbrand' <[e-mail]>; lixianglai
> >>>> <[e-mail]>; [e-mail]
> >>>> Cc: Salil Mehta <[e-mail]>; Xiaojuan Yang
> >>>> <[e-mail]>; Song Gao <[e-mail]>; Michael S.
> >>>> Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani
> Sinha
> >>>> <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
> >>>> Henderson <[e-mail]>; Eduardo Habkost
> >>>> <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
> >>>> Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
> >>>> <[e-mail]>; Daniel P. Berrangé <[e-mail]>;
> >> Peter
> >>>> Xu <[e-mail]>; Bibo Mao <[e-mail]>
> >>>> Subject: RE: [PATCH v2 04/10] Introduce the CPU address space
> >> destruction
> >>>> function
> >>>>
> >>>> Hi David,
> >>>>
> >>>>> From: David Hildenbrand <[e-mail]>
> >>>>> Sent: Friday, September 15, 2023 9:07 AM
> >>>>> To: lixianglai <[e-mail]>; [e-mail]; Salil
> >>>> Mehta
> >>>>> <[e-mail]>
> >>>>> Cc: Salil Mehta <[e-mail]>; Xiaojuan Yang
> >>>>> <[e-mail]>; Song Gao <[e-mail]>; Michael
> S.
> >>>>> Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani
> >> Sinha
> >>>>> <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
> >>>>> Henderson <[e-mail]>; Eduardo Habkost
> >>>>> <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
> >>>>> Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
> >>>>> <[e-mail]>; Daniel P. Berrangé <[e-mail]>;
> >> Peter
> >>>>> Xu <[e-mail]>; Bibo Mao <[e-mail]>
> >>>>> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space
> >> destruction
> >>>>> function
> >>>>>
> >>>>> On 15.09.23 04:53, lixianglai wrote:
> >>>>>> 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.
> >>>>>
> >>>>> Ah, the last thing is the important bit. I can reproduce this with
> KVM
> >>>>> easily.
> >>>>>
> >>>>> Doing it a couple of times
> >>>>>
> >>>>> address-space: cpu-memory-0
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>>
> >>>>> Looks like a resource/memory leak.
> >>>>
> >>>> Yes, there was. Thanks for identifying it. I have fixed in the
> >>>> latest RFC V2. Please check here:
> >>>>
> >>>> https://lore.kernel.org/qemu-devel/20230926100436.28284-1-
> >>>> [e-mail]/T/#m5f5ae40b091d69d01012880d7500d96874a9d39c
> >>>>
> >>>> I have tested and AddressSpace comes and goes away cleanly
> >>>> on CPU hot(un)plug action.
> >>>
> >>> Hi David/Xianglai,
> >>> Are you okay if I put Reported-by and give reference to this
> >>> conversation?
> >>
> >> Yes. And ideally, send the fixes separately from the other arm patches.
> >
> > ARM Virtual CPU Hotplug support patches are still under review.
> 
> The other architectures (as shown, x86 is affected) can be fixed
> independent of that support.

Yes, they can be and the TCG as well. Unrealize part of TCG is
broken even on ARM. Need some way to cleanly unassign Translation
blocks from the Region trees. That’s a pending work at our end.
But you are more than welcome to help and contribute in that
as well.

Also, I can help to contribute, if required.

Many thanks
Salil.

David Hildenbrand Sept. 26, 2023, 12:52 p.m. UTC | #20

On 26.09.23 14:44, Salil Mehta wrote:
>> From: David Hildenbrand <[e-mail]>
>> Sent: Tuesday, September 26, 2023 1:37 PM
>> To: Salil Mehta <[e-mail]>; lixianglai
>> <[e-mail]>; [e-mail]
>> Cc: Salil Mehta <[e-mail]>; Xiaojuan Yang
>> <[e-mail]>; Song Gao <[e-mail]>; Michael S.
>> Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani Sinha
>> <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
>> Henderson <[e-mail]>; Eduardo Habkost
>> <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
>> Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
>> <[e-mail]>; Daniel P. Berrangé <[e-mail]>; Peter
>> Xu <[e-mail]>; Bibo Mao <[e-mail]>
>> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space destruction
>> function
>>
>> On 26.09.23 14:32, Salil Mehta wrote:
>>>> From: David Hildenbrand <[e-mail]>
>>>> Sent: Tuesday, September 26, 2023 1:24 PM
>>>>
>>>> On 26.09.23 13:55, Salil Mehta wrote:
>>>>>> From: Salil Mehta
>>>>>> Sent: Tuesday, September 26, 2023 12:21 PM
>>>>>> To: 'David Hildenbrand' <[e-mail]>; lixianglai
>>>>>> <[e-mail]>; [e-mail]
>>>>>> Cc: Salil Mehta <[e-mail]>; Xiaojuan Yang
>>>>>> <[e-mail]>; Song Gao <[e-mail]>; Michael S.
>>>>>> Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani
>> Sinha
>>>>>> <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
>>>>>> Henderson <[e-mail]>; Eduardo Habkost
>>>>>> <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
>>>>>> Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
>>>>>> <[e-mail]>; Daniel P. Berrangé <[e-mail]>;
>>>> Peter
>>>>>> Xu <[e-mail]>; Bibo Mao <[e-mail]>
>>>>>> Subject: RE: [PATCH v2 04/10] Introduce the CPU address space
>>>> destruction
>>>>>> function
>>>>>>
>>>>>> Hi David,
>>>>>>
>>>>>>> From: David Hildenbrand <[e-mail]>
>>>>>>> Sent: Friday, September 15, 2023 9:07 AM
>>>>>>> To: lixianglai <[e-mail]>; [e-mail]; Salil
>>>>>> Mehta
>>>>>>> <[e-mail]>
>>>>>>> Cc: Salil Mehta <[e-mail]>; Xiaojuan Yang
>>>>>>> <[e-mail]>; Song Gao <[e-mail]>; Michael
>> S.
>>>>>>> Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani
>>>> Sinha
>>>>>>> <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
>>>>>>> Henderson <[e-mail]>; Eduardo Habkost
>>>>>>> <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
>>>>>>> Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
>>>>>>> <[e-mail]>; Daniel P. Berrangé <[e-mail]>;
>>>> Peter
>>>>>>> Xu <[e-mail]>; Bibo Mao <[e-mail]>
>>>>>>> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space
>>>> destruction
>>>>>>> function
>>>>>>>
>>>>>>> On 15.09.23 04:53, lixianglai wrote:
>>>>>>>> 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.
>>>>>>>
>>>>>>> Ah, the last thing is the important bit. I can reproduce this with
>> KVM
>>>>>>> easily.
>>>>>>>
>>>>>>> Doing it a couple of times
>>>>>>>
>>>>>>> address-space: cpu-memory-0
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>>
>>>>>>> Looks like a resource/memory leak.
>>>>>>
>>>>>> Yes, there was. Thanks for identifying it. I have fixed in the
>>>>>> latest RFC V2. Please check here:
>>>>>>
>>>>>> https://lore.kernel.org/qemu-devel/20230926100436.28284-1-
>>>>>> [e-mail]/T/#m5f5ae40b091d69d01012880d7500d96874a9d39c
>>>>>>
>>>>>> I have tested and AddressSpace comes and goes away cleanly
>>>>>> on CPU hot(un)plug action.
>>>>>
>>>>> Hi David/Xianglai,
>>>>> Are you okay if I put Reported-by and give reference to this
>>>>> conversation?
>>>>
>>>> Yes. And ideally, send the fixes separately from the other arm patches.
>>>
>>> ARM Virtual CPU Hotplug support patches are still under review.
>>
>> The other architectures (as shown, x86 is affected) can be fixed
>> independent of that support.
> 
> Yes, they can be and the TCG as well. Unrealize part of TCG is
> broken even on ARM. Need some way to cleanly unassign Translation
> blocks from the Region trees. That’s a pending work at our end.
> But you are more than welcome to help and contribute in that
> as well.

I have absolutely no time for that, happy to review patches.

Xianglai Li Sept. 27, 2023, 2:16 a.m. UTC | #21

Hi Salil Mehta:
>> From: Salil Mehta
>> Sent: Tuesday, September 26, 2023 12:21 PM
>> To: 'David Hildenbrand' <[e-mail]>; lixianglai
>> <[e-mail]>; [e-mail]
>> Cc: Salil Mehta <[e-mail]>; Xiaojuan Yang
>> <[e-mail]>; Song Gao <[e-mail]>; Michael S.
>> Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani Sinha
>> <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
>> Henderson <[e-mail]>; Eduardo Habkost
>> <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
>> Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
>> <[e-mail]>; Daniel P. Berrangé <[e-mail]>; Peter
>> Xu <[e-mail]>; Bibo Mao <[e-mail]>
>> Subject: RE: [PATCH v2 04/10] Introduce the CPU address space destruction
>> function
>>
>> Hi David,
>>
>>> From: David Hildenbrand <[e-mail]>
>>> Sent: Friday, September 15, 2023 9:07 AM
>>> To: lixianglai <[e-mail]>; [e-mail]; Salil
>> Mehta
>>> <[e-mail]>
>>> Cc: Salil Mehta <[e-mail]>; Xiaojuan Yang
>>> <[e-mail]>; Song Gao <[e-mail]>; Michael S.
>>> Tsirkin <[e-mail]>; Igor Mammedov <[e-mail]>; Ani Sinha
>>> <[e-mail]>; Paolo Bonzini <[e-mail]>; Richard
>>> Henderson <[e-mail]>; Eduardo Habkost
>>> <[e-mail]>; Marcel Apfelbaum <[e-mail]>;
>>> Philippe Mathieu-Daudé <[e-mail]>; wangyanan (Y)
>>> <[e-mail]>; Daniel P. Berrangé <[e-mail]>; Peter
>>> Xu <[e-mail]>; Bibo Mao <[e-mail]>
>>> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space destruction
>>> function
>>>
>>> On 15.09.23 04:53, lixianglai wrote:
>>>> 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.
>>> Ah, the last thing is the important bit. I can reproduce this with KVM
>>> easily.
>>>
>>> Doing it a couple of times
>>>
>>> address-space: cpu-memory-0
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>>
>>> Looks like a resource/memory leak.
>> Yes, there was. Thanks for identifying it. I have fixed in the
>> latest RFC V2. Please check here:
>>
>> https://lore.kernel.org/qemu-devel/20230926100436.28284-1-
>> [e-mail]/T/#m5f5ae40b091d69d01012880d7500d96874a9d39c
>>
>> I have tested and AddressSpace comes and goes away cleanly
>> on CPU hot(un)plug action.
> Hi David/Xianglai,
> Are you okay if I put Reported-by and give reference to this
> conversation?

That's Ok for me.

Thanks,

Xianglai.


> Many thanks
> Salil
>
>

Patch

diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 41788c0bdd..eb56a228a2 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -120,6 +120,14 @@  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 which address space needs to be destroyed
+ * @asidx: integer index of this address space
+ *
+ * Note that with KVM only one address space is supported.
+ */
+void cpu_address_space_destroy(CPUState *cpu, int asidx);
 
 void cpu_physical_memory_rw(hwaddr addr, void *buf,
                             hwaddr len, bool is_write);
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 92a4234439..c90cf3a162 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -366,6 +366,7 @@  struct CPUState {
     QSIMPLEQ_HEAD(, qemu_work_item) work_list;
 
     CPUAddressSpace *cpu_ases;
+    int cpu_ases_ref_count;
     int num_ases;
     AddressSpace *as;
     MemoryRegion *memory;
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 18277ddd67..c75e3e8042 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -761,6 +761,7 @@  void cpu_address_space_init(CPUState *cpu, int asidx,
 
     if (!cpu->cpu_ases) {
         cpu->cpu_ases = g_new0(CPUAddressSpace, cpu->num_ases);
+        cpu->cpu_ases_ref_count = cpu->num_ases;
     }
 
     newas = &cpu->cpu_ases[asidx];
@@ -774,6 +775,29 @@  void cpu_address_space_init(CPUState *cpu, int asidx,
     }
 }
 
+void cpu_address_space_destroy(CPUState *cpu, int asidx)
+{
+    CPUAddressSpace *cpuas;
+
+    assert(asidx < cpu->num_ases);
+    assert(asidx == 0 || !kvm_enabled());
+    assert(cpu->cpu_ases);
+
+    cpuas = &cpu->cpu_ases[asidx];
+    if (tcg_enabled()) {
+        memory_listener_unregister(&cpuas->tcg_as_listener);
+    }
+
+    address_space_destroy(cpuas->as);
+
+    cpu->cpu_ases_ref_count--;
+    if (cpu->cpu_ases_ref_count == 0) {
+        g_free(cpu->cpu_ases);
+        cpu->cpu_ases = NULL;
+    }
+
+}
+
 AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)
 {
     /* Return the AddressSpace corresponding to the specified index */