Re: [PATCH v2] hw/i386: Expand the range of CPU topologies between smp and maxcpus

[Like Xu <like.xu@linux.intel.com>]

On 2021/4/26 21:30, Daniel P. Berrangé wrote:
> On Mon, Apr 26, 2021 at 10:08:52AM +0800, caodongli@kingsoft.com wrote:
>> Change the criteria for the initial CPU topology and maxcpus, user can
>> have more settings
> 
> Can you provide a better explanation of why this is needed. What
> valid usage scenario is blocked by the current check ?
> 
> AFAICT, it partially reverts an intentional change done in several
> years ago in :
> 
> 
>    commit bc1fb850a31468ac4976f3895f01a6d981e06d0a
>    Author: Igor Mammedov <imammedo@redhat.com>
>    Date:   Thu Sep 13 13:06:01 2018 +0200
> 
>      vl.c deprecate incorrect CPUs topology
>      
>      -smp [cpus],sockets/cores/threads[,maxcpus] should describe topology
>      so that total number of logical CPUs [sockets * cores * threads]
>      would be equal to [maxcpus], however historically we didn't have
>      such check in QEMU and it is possible to start VM with an invalid
>      topology.
>      Deprecate invalid options combination so we can make sure that
>      the topology VM started with is always correct in the future.
>      Users with an invalid sockets/cores/threads/maxcpus values should
>      fix their CLI to make sure that
>         [sockets * cores * threads] == [maxcpus]
> 
> 

Another helpful commit would be:

commit c4332cd1dcf2964c23893ab4c0bf8d774e42a3cf
Author: Igor Mammedov <imammedo@redhat.com>
Date:   Fri Sep 11 09:32:02 2020 -0400

     smp: drop support for deprecated (invalid topologies)

     it's was deprecated since 3.1

     Support for invalid topologies is removed, the user must ensure
     that topologies described with -smp include all possible cpus,
     i.e. (sockets * cores * threads) == maxcpus or QEMU will
     exit with error.


So is the following statement correct:

When we explicitly set the topology, we must ensure that the combination 
(sockets/dies/cores/threads/maxcpus) is always valid. If we need hot plug 
testing, we can only use something like "-smp 1,maxcpus = 4" since 3.1.

?


> 
>>
>> Signed-off-by: Dongli Cao <caodongli@kingsoft.com>
>> ---
>> hw/i386/pc.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 8a84b25..ef2e819 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -751,7 +751,7 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts)
>>               exit(1);
>>           }
>>
>> -        if (sockets * dies * cores * threads != ms->smp.max_cpus) {
>> +        if (sockets * dies * cores * threads > ms->smp.max_cpus) {
>>               error_report("Invalid CPU topology deprecated: "
>>                            "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
>>                            "!= maxcpus (%u)",
> 
> This is
> 
>> --
>> 1.8.3.1
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> caodongli@kingsoft.com
>>
>>
> 
> Regards,
> Daniel
>