mbox series

[0/2] i386: fixup number of logical CPUs when host-cache-info=on

Message ID 20220524151020.2541698-1-imammedo@redhat.com (mailing list archive)
Headers show
Series i386: fixup number of logical CPUs when host-cache-info=on | expand

Message

Igor Mammedov May 24, 2022, 3:10 p.m. UTC
Igor Mammedov (2):
  x86: cpu: make sure number of addressable IDs for processor cores
    meets the spec
  x86: cpu: fixup number of addressable IDs for logical processors
    sharing cache

 target/i386/cpu.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

Comments

Igor Mammedov May 24, 2022, 3:19 p.m. UTC | #1
On Tue, 24 May 2022 11:10:18 -0400
Igor Mammedov <imammedo@redhat.com> wrote:

CCing AMD folks as that might be of interest to them

> Igor Mammedov (2):
>   x86: cpu: make sure number of addressable IDs for processor cores
>     meets the spec
>   x86: cpu: fixup number of addressable IDs for logical processors
>     sharing cache
> 
>  target/i386/cpu.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
>
Babu Moger May 24, 2022, 7:48 p.m. UTC | #2
On 5/24/22 10:19, Igor Mammedov wrote:
> On Tue, 24 May 2022 11:10:18 -0400
> Igor Mammedov <imammedo@redhat.com> wrote:
>
> CCing AMD folks as that might be of interest to them

I am trying to recreate the bug on my AMD system here.. Seeing this message..

qemu-system-x86_64: -numa node,nodeid=0,memdev=ram-node0: memdev=ram-node0
is ambiguous

Here is my command line..

#qemu-system-x86_64 -name rhel8 -m 4096 -hda vdisk.qcow2 -enable-kvm -net
nic  -nographic -machine q35,accel=kvm -cpu
host,host-cache-info=on,l3-cache=off -smp
20,sockets=2,dies=1,cores=10,threads=1 -numa
node,nodeid=0,memdev=ram-node0 -numa node,nodeid=1,memdev=ram-node1 -numa
cpu,socket-id=0,node-id=0 -numa cpu,socket-id=1,node-id=1

Am I missing something?


>
>> Igor Mammedov (2):
>>   x86: cpu: make sure number of addressable IDs for processor cores
>>     meets the spec
>>   x86: cpu: fixup number of addressable IDs for logical processors
>>     sharing cache
>>
>>  target/i386/cpu.c | 20 ++++++++++++++++----
>>  1 file changed, 16 insertions(+), 4 deletions(-)
>>
Alejandro Jimenez May 24, 2022, 11:23 p.m. UTC | #3
On 5/24/2022 3:48 PM, Moger, Babu wrote:
> 
> On 5/24/22 10:19, Igor Mammedov wrote:
>> On Tue, 24 May 2022 11:10:18 -0400
>> Igor Mammedov <imammedo@redhat.com> wrote:
>>
>> CCing AMD folks as that might be of interest to them
> 
> I am trying to recreate the bug on my AMD system here.. Seeing this message..
> 
> qemu-system-x86_64: -numa node,nodeid=0,memdev=ram-node0: memdev=ram-node0
> is ambiguous
> 
> Here is my command line..
> 
> #qemu-system-x86_64 -name rhel8 -m 4096 -hda vdisk.qcow2 -enable-kvm -net
> nic  -nographic -machine q35,accel=kvm -cpu
> host,host-cache-info=on,l3-cache=off -smp
> 20,sockets=2,dies=1,cores=10,threads=1 -numa
> node,nodeid=0,memdev=ram-node0 -numa node,nodeid=1,memdev=ram-node1 -numa
> cpu,socket-id=0,node-id=0 -numa cpu,socket-id=1,node-id=1
> 
> Am I missing something?
Hi Babu,

Hopefully this will help you reproduce the issue if you are testing on 
Milan/Genoa. Joao (CC'd) pointed out this warning to me late last year, 
while I was working on patches for encoding the topology CPUID leaf in 
different Zen platforms.

What I found from my experiments on Milan, is that the warning will 
appear whenever the NUMA topology requested in QEMU cmdline assigns a 
number of CPUs to each node that is smaller than the default # of CPUs 
sharing a LLC on the host platform. In short, on a Milan host where we 
have 16 CPUs sharing a CCX:

# cat /sys/devices/system/cpu/cpu0/cache/index3/shared_cpu_list
0-7,128-135

If a guest is launched with the following arguments:

-cpu host,+topoext \
-smp cpus=64,cores=32,threads=2,sockets=1 \
-numa node,nodeid=0,cpus=0-7 -numa node,nodeid=1,cpus=8-15 \
-numa node,nodeid=2,cpus=16-23 -numa node,nodeid=3,cpus=24-31 \
-numa node,nodeid=4,cpus=32-39 -numa node,nodeid=5,cpus=40-47 \
-numa node,nodeid=6,cpus=48-55 -numa node,nodeid=7,cpus=56-63 \

it assigns 8 cpus to each NUMA node, causing the error above to be 
displayed.

Note that ultimately the guest topology is built based on the NUMA 
information, so the LLC domains on the guest only end up spanning a 
single NUMA node. e.g.:

# cat /sys/devices/system/cpu/cpu0/cache/index3/shared_cpu_list
0-7

Hope that helps,
Alejandro
> 
> 
>>
>>> Igor Mammedov (2):
>>>    x86: cpu: make sure number of addressable IDs for processor cores
>>>      meets the spec
>>>    x86: cpu: fixup number of addressable IDs for logical processors
>>>      sharing cache
>>>
>>>   target/i386/cpu.c | 20 ++++++++++++++++----
>>>   1 file changed, 16 insertions(+), 4 deletions(-)
>>>
Igor Mammedov May 25, 2022, 7:05 a.m. UTC | #4
On Tue, 24 May 2022 14:48:29 -0500
"Moger, Babu" <babu.moger@amd.com> wrote:

> On 5/24/22 10:19, Igor Mammedov wrote:
> > On Tue, 24 May 2022 11:10:18 -0400
> > Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > CCing AMD folks as that might be of interest to them  
> 
> I am trying to recreate the bug on my AMD system here.. Seeing this message..
> 
> qemu-system-x86_64: -numa node,nodeid=0,memdev=ram-node0: memdev=ram-node0
> is ambiguous
> 
> Here is my command line..
> 
> #qemu-system-x86_64 -name rhel8 -m 4096 -hda vdisk.qcow2 -enable-kvm -net
> nic  -nographic -machine q35,accel=kvm -cpu
> host,host-cache-info=on,l3-cache=off -smp
> 20,sockets=2,dies=1,cores=10,threads=1 -numa
> node,nodeid=0,memdev=ram-node0 -numa node,nodeid=1,memdev=ram-node1 -numa
> cpu,socket-id=0,node-id=0 -numa cpu,socket-id=1,node-id=1
> 
> Am I missing something?
Yep, sorry I've omitted -object memory-backend-foo definitions for
ram-node0 and ram-node1

one can use any memory backend, it doesn't really matter in this case,
for example following should do:
  -object memory-backend-ram,id=ram-node0,size=2G \
  -object memory-backend-ram,id=ram-node1,size=2G 


> 
> 
> >  
> >> Igor Mammedov (2):
> >>   x86: cpu: make sure number of addressable IDs for processor cores
> >>     meets the spec
> >>   x86: cpu: fixup number of addressable IDs for logical processors
> >>     sharing cache
> >>
> >>  target/i386/cpu.c | 20 ++++++++++++++++----
> >>  1 file changed, 16 insertions(+), 4 deletions(-)
> >>
Babu Moger May 25, 2022, 7:56 p.m. UTC | #5
On 5/24/22 18:23, Alejandro Jimenez wrote:
> On 5/24/2022 3:48 PM, Moger, Babu wrote:
>>
>> On 5/24/22 10:19, Igor Mammedov wrote:
>>> On Tue, 24 May 2022 11:10:18 -0400
>>> Igor Mammedov <imammedo@redhat.com> wrote:
>>>
>>> CCing AMD folks as that might be of interest to them
>>
>> I am trying to recreate the bug on my AMD system here.. Seeing this
>> message..
>>
>> qemu-system-x86_64: -numa node,nodeid=0,memdev=ram-node0: memdev=ram-node0
>> is ambiguous
>>
>> Here is my command line..
>>
>> #qemu-system-x86_64 -name rhel8 -m 4096 -hda vdisk.qcow2 -enable-kvm -net
>> nic  -nographic -machine q35,accel=kvm -cpu
>> host,host-cache-info=on,l3-cache=off -smp
>> 20,sockets=2,dies=1,cores=10,threads=1 -numa
>> node,nodeid=0,memdev=ram-node0 -numa node,nodeid=1,memdev=ram-node1 -numa
>> cpu,socket-id=0,node-id=0 -numa cpu,socket-id=1,node-id=1
>>
>> Am I missing something?
> Hi Babu,
>
> Hopefully this will help you reproduce the issue if you are testing on
> Milan/Genoa. Joao (CC'd) pointed out this warning to me late last year,
> while I was working on patches for encoding the topology CPUID leaf in
> different Zen platforms.
>
> What I found from my experiments on Milan, is that the warning will
> appear whenever the NUMA topology requested in QEMU cmdline assigns a
> number of CPUs to each node that is smaller than the default # of CPUs
> sharing a LLC on the host platform. In short, on a Milan host where we
> have 16 CPUs sharing a CCX:

Yes. I recreated the issue with this following command line.

#qemu-system-x86_64 -name rhel8 -m 4096 -hda vdisk.qcow2 -enable-kvm -net
nic  -nographic -machine q35,accel=kvm -cpu host,+topoext -smp
16,sockets=1,dies=1,cores=16,threads=1 -object
memory-backend-ram,id=ram-node0,size=2G -object
memory-backend-ram,id=ram-node1,size=2G  -numa
node,nodeid=0,cpus=0-7,memdev=ram-node0 -numa
node,nodeid=1,cpus=8-15,memdev=ram-node1

But solving this will be bit complicated. For AMD, this information comes
from CPUID 0x8000001d. But, when this cpuid is being populated we don't
have all the information about numa nodes etc..

But you can work-around it by modifying the command line by including
dies(dies=2 in this case) information.  Something like this.

#qemu-system-x86_64 -name rhel8 -m 4096 -hda vdisk.qcow2 -enable-kvm -net
nic  -nographic -machine q35,accel=kvm -cpu
host,+topoext,host-cache-info=on -smp
16,sockets=1,dies=2,cores=8,threads=1 -object
memory-backend-ram,id=ram-node0,size=2G -object
memory-backend-ram,id=ram-node1,size=2G  -numa
node,nodeid=0,cpus=0-7,memdev=ram-node0 -numa
node,nodeid=1,cpus=8-15,memdev=ram-node1

But this may not be acceptable solution in all the cases.

>
> # cat /sys/devices/system/cpu/cpu0/cache/index3/shared_cpu_list
> 0-7,128-135
>
> If a guest is launched with the following arguments:
>
> -cpu host,+topoext \
> -smp cpus=64,cores=32,threads=2,sockets=1 \
> -numa node,nodeid=0,cpus=0-7 -numa node,nodeid=1,cpus=8-15 \
> -numa node,nodeid=2,cpus=16-23 -numa node,nodeid=3,cpus=24-31 \
> -numa node,nodeid=4,cpus=32-39 -numa node,nodeid=5,cpus=40-47 \
> -numa node,nodeid=6,cpus=48-55 -numa node,nodeid=7,cpus=56-63 \
>
> it assigns 8 cpus to each NUMA node, causing the error above to be
> displayed.
>
> Note that ultimately the guest topology is built based on the NUMA
> information, so the LLC domains on the guest only end up spanning a
> single NUMA node. e.g.:
>
> # cat /sys/devices/system/cpu/cpu0/cache/index3/shared_cpu_list
> 0-7
>
> Hope that helps,
> Alejandro
>>
>>
>>>
>>>> Igor Mammedov (2):
>>>>    x86: cpu: make sure number of addressable IDs for processor cores
>>>>      meets the spec
>>>>    x86: cpu: fixup number of addressable IDs for logical processors
>>>>      sharing cache
>>>>
>>>>   target/i386/cpu.c | 20 ++++++++++++++++----
>>>>   1 file changed, 16 insertions(+), 4 deletions(-)
>>>>
>
Babu Moger May 25, 2022, 8:04 p.m. UTC | #6
On 5/25/22 02:05, Igor Mammedov wrote:
> On Tue, 24 May 2022 14:48:29 -0500
> "Moger, Babu" <babu.moger@amd.com> wrote:
>
>> On 5/24/22 10:19, Igor Mammedov wrote:
>>> On Tue, 24 May 2022 11:10:18 -0400
>>> Igor Mammedov <imammedo@redhat.com> wrote:
>>>
>>> CCing AMD folks as that might be of interest to them  
>> I am trying to recreate the bug on my AMD system here.. Seeing this message..
>>
>> qemu-system-x86_64: -numa node,nodeid=0,memdev=ram-node0: memdev=ram-node0
>> is ambiguous
>>
>> Here is my command line..
>>
>> #qemu-system-x86_64 -name rhel8 -m 4096 -hda vdisk.qcow2 -enable-kvm -net
>> nic  -nographic -machine q35,accel=kvm -cpu
>> host,host-cache-info=on,l3-cache=off -smp
>> 20,sockets=2,dies=1,cores=10,threads=1 -numa
>> node,nodeid=0,memdev=ram-node0 -numa node,nodeid=1,memdev=ram-node1 -numa
>> cpu,socket-id=0,node-id=0 -numa cpu,socket-id=1,node-id=1
>>
>> Am I missing something?
> Yep, sorry I've omitted -object memory-backend-foo definitions for
> ram-node0 and ram-node1
>
> one can use any memory backend, it doesn't really matter in this case,
> for example following should do:
>   -object memory-backend-ram,id=ram-node0,size=2G \
>   -object memory-backend-ram,id=ram-node1,size=2G 

Thanks Igor. However these changes(patch 1 and 2) does not affect AMD
systems as far i can see.

Thanks

Babu

>
>>
>>>  
>>>> Igor Mammedov (2):
>>>>   x86: cpu: make sure number of addressable IDs for processor cores
>>>>     meets the spec
>>>>   x86: cpu: fixup number of addressable IDs for logical processors
>>>>     sharing cache
>>>>
>>>>  target/i386/cpu.c | 20 ++++++++++++++++----
>>>>  1 file changed, 16 insertions(+), 4 deletions(-)
>>>>
Alejandro Jimenez May 25, 2022, 9:20 p.m. UTC | #7
On 5/25/2022 3:56 PM, Moger, Babu wrote:
> 
> On 5/24/22 18:23, Alejandro Jimenez wrote:
>> On 5/24/2022 3:48 PM, Moger, Babu wrote:
>>>
>>> On 5/24/22 10:19, Igor Mammedov wrote:
>>>> On Tue, 24 May 2022 11:10:18 -0400
>>>> Igor Mammedov <imammedo@redhat.com> wrote:
>>>>
>>>> CCing AMD folks as that might be of interest to them
>>>
>>> I am trying to recreate the bug on my AMD system here.. Seeing this
>>> message..
>>>
>>> qemu-system-x86_64: -numa node,nodeid=0,memdev=ram-node0: memdev=ram-node0
>>> is ambiguous
>>>
>>> Here is my command line..
>>>
>>> #qemu-system-x86_64 -name rhel8 -m 4096 -hda vdisk.qcow2 -enable-kvm -net
>>> nic  -nographic -machine q35,accel=kvm -cpu
>>> host,host-cache-info=on,l3-cache=off -smp
>>> 20,sockets=2,dies=1,cores=10,threads=1 -numa
>>> node,nodeid=0,memdev=ram-node0 -numa node,nodeid=1,memdev=ram-node1 -numa
>>> cpu,socket-id=0,node-id=0 -numa cpu,socket-id=1,node-id=1
>>>
>>> Am I missing something?
>> Hi Babu,
>>
>> Hopefully this will help you reproduce the issue if you are testing on
>> Milan/Genoa. Joao (CC'd) pointed out this warning to me late last year,
>> while I was working on patches for encoding the topology CPUID leaf in
>> different Zen platforms.
>>
>> What I found from my experiments on Milan, is that the warning will
>> appear whenever the NUMA topology requested in QEMU cmdline assigns a
>> number of CPUs to each node that is smaller than the default # of CPUs
>> sharing a LLC on the host platform. In short, on a Milan host where we
>> have 16 CPUs sharing a CCX:
> 
> Yes. I recreated the issue with this following command line.
> 
> #qemu-system-x86_64 -name rhel8 -m 4096 -hda vdisk.qcow2 -enable-kvm -net
> nic  -nographic -machine q35,accel=kvm -cpu host,+topoext -smp
> 16,sockets=1,dies=1,cores=16,threads=1 -object
> memory-backend-ram,id=ram-node0,size=2G -object
> memory-backend-ram,id=ram-node1,size=2G  -numa
> node,nodeid=0,cpus=0-7,memdev=ram-node0 -numa
> node,nodeid=1,cpus=8-15,memdev=ram-node1
> 
> But solving this will be bit complicated. For AMD, this information comes
> from CPUID 0x8000001d. But, when this cpuid is being populated we don't
> have all the information about numa nodes etc..
> 
> But you can work-around it by modifying the command line by including
> dies(dies=2 in this case) information.  Something like this.
Makes sense; using dies=2 makes it so the cache topology leaf is built 
with 8cores/CCX, matching the # of NUMA nodes so all is well.
> 
> #qemu-system-x86_64 -name rhel8 -m 4096 -hda vdisk.qcow2 -enable-kvm -net
> nic  -nographic -machine q35,accel=kvm -cpu
> host,+topoext,host-cache-info=on -smp
> 16,sockets=1,dies=2,cores=8,threads=1 -object
> memory-backend-ram,id=ram-node0,size=2G -object
> memory-backend-ram,id=ram-node1,size=2G  -numa
> node,nodeid=0,cpus=0-7,memdev=ram-node0 -numa
> node,nodeid=1,cpus=8-15,memdev=ram-node1
> 
> But this may not be acceptable solution in all the cases.
This is not specific to host-cache-info behavior so it is probably 
better to discuss it separately. With that being said...

The idea that I considered was to automatically calculate a value of 
'dies' iff a explicit value was not requested via the '-smp' options, 
instead of just using the current default of dies=1. i.e. automatically 
mimic the host cache topology in the guest so that if we are running on 
Rome, the guest OS sees 4cores/CCX, but when running on Milan it sees 
8cores/CCX. This can be done by querying the host CPUID and using that 
info to build the guest CPUID leaf in QEMU, similar to what Igor is 
doing here but also adjusting the number of dies that is encoded.

I built prototype code that seemed to work correctly, but did not 
consider the complication added by '-numa' options.

I think there is a much larger debate involved about what defaults are 
"sane", so rather than derailing this thread more, I'll send a follow up 
message in the future when I can take another look at the prototype 
patches I have.

Thank you,
Alejandro
> 
>>
>> # cat /sys/devices/system/cpu/cpu0/cache/index3/shared_cpu_list
>> 0-7,128-135
>>
>> If a guest is launched with the following arguments:
>>
>> -cpu host,+topoext \
>> -smp cpus=64,cores=32,threads=2,sockets=1 \
>> -numa node,nodeid=0,cpus=0-7 -numa node,nodeid=1,cpus=8-15 \
>> -numa node,nodeid=2,cpus=16-23 -numa node,nodeid=3,cpus=24-31 \
>> -numa node,nodeid=4,cpus=32-39 -numa node,nodeid=5,cpus=40-47 \
>> -numa node,nodeid=6,cpus=48-55 -numa node,nodeid=7,cpus=56-63 \
>>
>> it assigns 8 cpus to each NUMA node, causing the error above to be
>> displayed.
>>
>> Note that ultimately the guest topology is built based on the NUMA
>> information, so the LLC domains on the guest only end up spanning a
>> single NUMA node. e.g.:
>>
>> # cat /sys/devices/system/cpu/cpu0/cache/index3/shared_cpu_list
>> 0-7
>>
>> Hope that helps,
>> Alejandro
>>>
>>>
>>>>
>>>>> Igor Mammedov (2):
>>>>>     x86: cpu: make sure number of addressable IDs for processor cores
>>>>>       meets the spec
>>>>>     x86: cpu: fixup number of addressable IDs for logical processors
>>>>>       sharing cache
>>>>>
>>>>>    target/i386/cpu.c | 20 ++++++++++++++++----
>>>>>    1 file changed, 16 insertions(+), 4 deletions(-)
>>>>>
>>
Igor Mammedov May 31, 2022, 12:44 p.m. UTC | #8
Paolo,
 can you pick this up if it looks fine, please?

On Tue, 24 May 2022 11:10:18 -0400
Igor Mammedov <imammedo@redhat.com> wrote:

> Igor Mammedov (2):
>   x86: cpu: make sure number of addressable IDs for processor cores
>     meets the spec
>   x86: cpu: fixup number of addressable IDs for logical processors
>     sharing cache
> 
>  target/i386/cpu.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
>
Paolo Bonzini June 1, 2022, 10:02 a.m. UTC | #9
On 5/31/22 14:44, Igor Mammedov wrote:
> Paolo,
>   can you pick this up if it looks fine, please?
> 
> On Tue, 24 May 2022 11:10:18 -0400
> Igor Mammedov <imammedo@redhat.com> wrote:
> 
>> Igor Mammedov (2):
>>    x86: cpu: make sure number of addressable IDs for processor cores
>>      meets the spec
>>    x86: cpu: fixup number of addressable IDs for logical processors
>>      sharing cache
>>
>>   target/i386/cpu.c | 20 ++++++++++++++++----
>>   1 file changed, 16 insertions(+), 4 deletions(-)
>>
> 

Yup, queued now.  Thanks,

Paolo