Wrap the CPU target specific parameters together into a single
variable except generic sockets/cores/threads, to make related
code lines shorter and more concise.

No functional change intended.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
---
 hw/core/machine-smp.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c
index b39ed21e65..4547d7bbdc 100644
--- a/hw/core/machine-smp.c
+++ b/hw/core/machine-smp.c
@@ -79,6 +79,7 @@ void machine_parse_smp_config(MachineState *ms,
     unsigned cores   = config->has_cores ? config->cores : 0;
     unsigned threads = config->has_threads ? config->threads : 0;
     unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0;
+    unsigned others;
 
     /*
      * Specified CPU topology parameters must be greater than zero,
@@ -111,6 +112,8 @@ void machine_parse_smp_config(MachineState *ms,
     dies = dies > 0 ? dies : 1;
     clusters = clusters > 0 ? clusters : 1;
 
+    others = dies * clusters;
+
     /* compute missing values based on the provided ones */
     if (cpus == 0 && maxcpus == 0) {
         sockets = sockets > 0 ? sockets : 1;
@@ -124,30 +127,30 @@ void machine_parse_smp_config(MachineState *ms,
             if (sockets == 0) {
                 cores = cores > 0 ? cores : 1;
                 threads = threads > 0 ? threads : 1;
-                sockets = maxcpus / (dies * clusters * cores * threads);
+                sockets = maxcpus / (cores * threads * others);
             } else if (cores == 0) {
                 threads = threads > 0 ? threads : 1;
-                cores = maxcpus / (sockets * dies * clusters * threads);
+                cores = maxcpus / (sockets * threads * others);
             }
         } else {
             /* prefer cores over sockets since 6.2 */
             if (cores == 0) {
                 sockets = sockets > 0 ? sockets : 1;
                 threads = threads > 0 ? threads : 1;
-                cores = maxcpus / (sockets * dies * clusters * threads);
+                cores = maxcpus / (sockets * threads * others);
             } else if (sockets == 0) {
                 threads = threads > 0 ? threads : 1;
-                sockets = maxcpus / (dies * clusters * cores * threads);
+                sockets = maxcpus / (cores * threads * others);
             }
         }
 
         /* try to calculate omitted threads at last */
         if (threads == 0) {
-            threads = maxcpus / (sockets * dies * clusters * cores);
+            threads = maxcpus / (sockets * cores * others);
         }
     }
 
-    maxcpus = maxcpus > 0 ? maxcpus : sockets * dies * clusters * cores * threads;
+    maxcpus = maxcpus > 0 ? maxcpus : sockets * cores * threads * others;
     cpus = cpus > 0 ? cpus : maxcpus;
 
     ms->smp.cpus = cpus;
@@ -159,7 +162,7 @@ void machine_parse_smp_config(MachineState *ms,
     ms->smp.max_cpus = maxcpus;
 
     /* sanity-check of the computed topology */
-    if (sockets * dies * clusters * cores * threads != maxcpus) {
+    if (sockets * cores * threads * others != maxcpus) {
         g_autofree char *topo_msg = cpu_hierarchy_to_string(ms);
         error_setg(errp, "Invalid CPU topology: "
                    "product of the hierarchy must match maxcpus: "
-- 
2.27.0

On 12/28/21 10:22, Yanan Wang wrote:
> Wrap the CPU target specific parameters together into a single
> variable except generic sockets/cores/threads, to make related
> code lines shorter and more concise.
> 
> No functional change intended.
> 
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
>  hw/core/machine-smp.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c
> index b39ed21e65..4547d7bbdc 100644
> --- a/hw/core/machine-smp.c
> +++ b/hw/core/machine-smp.c
> @@ -79,6 +79,7 @@ void machine_parse_smp_config(MachineState *ms,
>      unsigned cores   = config->has_cores ? config->cores : 0;
>      unsigned threads = config->has_threads ? config->threads : 0;
>      unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0;
> +    unsigned others;
>  
>      /*
>       * Specified CPU topology parameters must be greater than zero,
> @@ -111,6 +112,8 @@ void machine_parse_smp_config(MachineState *ms,
>      dies = dies > 0 ? dies : 1;
>      clusters = clusters > 0 ? clusters : 1;
>  
> +    others = dies * clusters;

This doesn't seem clearer to me...

>      /* compute missing values based on the provided ones */
>      if (cpus == 0 && maxcpus == 0) {
>          sockets = sockets > 0 ? sockets : 1;
> @@ -124,30 +127,30 @@ void machine_parse_smp_config(MachineState *ms,
>              if (sockets == 0) {
>                  cores = cores > 0 ? cores : 1;
>                  threads = threads > 0 ? threads : 1;
> -                sockets = maxcpus / (dies * clusters * cores * threads);
> +                sockets = maxcpus / (cores * threads * others);
>              } else if (cores == 0) {
>                  threads = threads > 0 ? threads : 1;
> -                cores = maxcpus / (sockets * dies * clusters * threads);
> +                cores = maxcpus / (sockets * threads * others);
>              }
>          } else {
>              /* prefer cores over sockets since 6.2 */
>              if (cores == 0) {
>                  sockets = sockets > 0 ? sockets : 1;
>                  threads = threads > 0 ? threads : 1;
> -                cores = maxcpus / (sockets * dies * clusters * threads);
> +                cores = maxcpus / (sockets * threads * others);
>              } else if (sockets == 0) {
>                  threads = threads > 0 ? threads : 1;
> -                sockets = maxcpus / (dies * clusters * cores * threads);
> +                sockets = maxcpus / (cores * threads * others);
>              }
>          }
>  
>          /* try to calculate omitted threads at last */
>          if (threads == 0) {
> -            threads = maxcpus / (sockets * dies * clusters * cores);
> +            threads = maxcpus / (sockets * cores * others);
>          }
>      }
>  
> -    maxcpus = maxcpus > 0 ? maxcpus : sockets * dies * clusters * cores * threads;
> +    maxcpus = maxcpus > 0 ? maxcpus : sockets * cores * threads * others;
>      cpus = cpus > 0 ? cpus : maxcpus;
>  
>      ms->smp.cpus = cpus;
> @@ -159,7 +162,7 @@ void machine_parse_smp_config(MachineState *ms,
>      ms->smp.max_cpus = maxcpus;
>  
>      /* sanity-check of the computed topology */
> -    if (sockets * dies * clusters * cores * threads != maxcpus) {
> +    if (sockets * cores * threads * others != maxcpus) {
>          g_autofree char *topo_msg = cpu_hierarchy_to_string(ms);
>          error_setg(errp, "Invalid CPU topology: "
>                     "product of the hierarchy must match maxcpus: "

On 2021/12/29 3:23, Philippe Mathieu-Daudé wrote:
> On 12/28/21 10:22, Yanan Wang wrote:
>> Wrap the CPU target specific parameters together into a single
>> variable except generic sockets/cores/threads, to make related
>> code lines shorter and more concise.
>>
>> No functional change intended.
>>
>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
>> ---
>>   hw/core/machine-smp.c | 17 ++++++++++-------
>>   1 file changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c
>> index b39ed21e65..4547d7bbdc 100644
>> --- a/hw/core/machine-smp.c
>> +++ b/hw/core/machine-smp.c
>> @@ -79,6 +79,7 @@ void machine_parse_smp_config(MachineState *ms,
>>       unsigned cores   = config->has_cores ? config->cores : 0;
>>       unsigned threads = config->has_threads ? config->threads : 0;
>>       unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0;
>> +    unsigned others;
>>   
>>       /*
>>        * Specified CPU topology parameters must be greater than zero,
>> @@ -111,6 +112,8 @@ void machine_parse_smp_config(MachineState *ms,
>>       dies = dies > 0 ? dies : 1;
>>       clusters = clusters > 0 ? clusters : 1;
>>   
>> +    others = dies * clusters;
> This doesn't seem clearer to me...
Ok. We can just drop this patch.

Thanks,
Yanan
>>       /* compute missing values based on the provided ones */
>>       if (cpus == 0 && maxcpus == 0) {
>>           sockets = sockets > 0 ? sockets : 1;
>> @@ -124,30 +127,30 @@ void machine_parse_smp_config(MachineState *ms,
>>               if (sockets == 0) {
>>                   cores = cores > 0 ? cores : 1;
>>                   threads = threads > 0 ? threads : 1;
>> -                sockets = maxcpus / (dies * clusters * cores * threads);
>> +                sockets = maxcpus / (cores * threads * others);
>>               } else if (cores == 0) {
>>                   threads = threads > 0 ? threads : 1;
>> -                cores = maxcpus / (sockets * dies * clusters * threads);
>> +                cores = maxcpus / (sockets * threads * others);
>>               }
>>           } else {
>>               /* prefer cores over sockets since 6.2 */
>>               if (cores == 0) {
>>                   sockets = sockets > 0 ? sockets : 1;
>>                   threads = threads > 0 ? threads : 1;
>> -                cores = maxcpus / (sockets * dies * clusters * threads);
>> +                cores = maxcpus / (sockets * threads * others);
>>               } else if (sockets == 0) {
>>                   threads = threads > 0 ? threads : 1;
>> -                sockets = maxcpus / (dies * clusters * cores * threads);
>> +                sockets = maxcpus / (cores * threads * others);
>>               }
>>           }
>>   
>>           /* try to calculate omitted threads at last */
>>           if (threads == 0) {
>> -            threads = maxcpus / (sockets * dies * clusters * cores);
>> +            threads = maxcpus / (sockets * cores * others);
>>           }
>>       }
>>   
>> -    maxcpus = maxcpus > 0 ? maxcpus : sockets * dies * clusters * cores * threads;
>> +    maxcpus = maxcpus > 0 ? maxcpus : sockets * cores * threads * others;
>>       cpus = cpus > 0 ? cpus : maxcpus;
>>   
>>       ms->smp.cpus = cpus;
>> @@ -159,7 +162,7 @@ void machine_parse_smp_config(MachineState *ms,
>>       ms->smp.max_cpus = maxcpus;
>>   
>>       /* sanity-check of the computed topology */
>> -    if (sockets * dies * clusters * cores * threads != maxcpus) {
>> +    if (sockets * cores * threads * others != maxcpus) {
>>           g_autofree char *topo_msg = cpu_hierarchy_to_string(ms);
>>           error_setg(errp, "Invalid CPU topology: "
>>                      "product of the hierarchy must match maxcpus: "
> .