Re: [PATCH v9 18/21] hw/i386/pc: Support smp.modules for x86 PC machine

[Moger, Babu]

On 2/29/24 01:32, Zhao Liu wrote:
> Hi Babu,
> 
>>>  DEF("smp", HAS_ARG, QEMU_OPTION_smp,
>>>      "-smp 
>>> [[cpus=]n][,maxcpus=maxcpus][,drawers=drawers][,books=books][,sockets=sockets]\n"
> 
> Here the "drawers" and "books" are listed...
> 
>>> -    "               
>>> [,dies=dies][,clusters=clusters][,cores=cores][,threads=threads]\n"
>>> +    "               
>>> [,dies=dies][,clusters=clusters][,modules=modules][,cores=cores]\n"
>>> +    "               [,threads=threads]\n"
>>>      "                set the number of initial CPUs to 'n' [default=1]\n"
>>>      "                maxcpus= maximum number of total CPUs, including\n"
>>>      "                offline CPUs for hotplug, etc\n"
>>> @@ -290,7 +291,8 @@ DEF("smp", HAS_ARG, QEMU_OPTION_smp,
>>>      "                sockets= number of sockets in one book\n"
>>>      "                dies= number of dies in one socket\n"
>>>      "                clusters= number of clusters in one die\n"
>>> -    "                cores= number of cores in one cluster\n"
>>> +    "                modules= number of modules in one cluster\n"
>>> +    "                cores= number of cores in one module\n"
>>>      "                threads= number of threads in one core\n"
>>>      "Note: Different machines may have different subsets of the CPU 
>>> topology\n"
>>>      "      parameters supported, so the actual meaning of the supported 
>>> parameters\n"
>>> @@ -306,7 +308,7 @@ DEF("smp", HAS_ARG, QEMU_OPTION_smp,
>>>      "      must be set as 1 in the purpose of correct parsing.\n",
>>>      QEMU_ARCH_ALL)
>>>  SRST
>>> -``-smp 
>>> [[cpus=]n][,maxcpus=maxcpus][,sockets=sockets][,dies=dies][,clusters=clusters][,cores=cores][,threads=threads]``
>>> +``-smp 
>>> [[cpus=]n][,maxcpus=maxcpus][,drawers=drawers][,books=books][,sockets=sockets][,dies=dies][,clusters=clusters][,modules=modules][,cores=cores][,threads=threads]``
>>
>> You have added drawers, books here. Were they missing before?
>>
> 
> ...so yes, I think those 2 parameters are missed at this place.

ok. If there is another revision then add a line about this change in the
commit message. Otherwise it is fine.

Reviewed-by: Babu Moger <babu.moger@amd.com>

> 
> Thank you for reviewing this.
> 
> Regards,
> Zhao
> 

-- 
Thanks
Babu Moger