Re: [PATCH v2 1/3] hw/smbios: Fix smbios_smp_sockets caculation

[Igor Mammedov]

On Thu,  1 Jun 2023 17:29:50 +0800
Zhao Liu <zhao1.liu@linux.intel.com> wrote:

> From: Zhao Liu <zhao1.liu@intel.com>
> 
> Here're 2 mistakes:
> 1. 003f230e37d7 ("machine: Tweak the order of topology members in struct
>    CpuTopology") changes the meaning of smp.cores but doesn't fix
>    original smp.cores uses. And because of the introduction of cluster,
>    now smp.cores means the number of cores in one cluster. So smp.cores
>    * smp.threads just means the cpus in a cluster not in a socket.

> 2. smp.cpus means the number of initial online cpus, not the total
>    number of cpus. For such topology calculation, smp.max_cpus
>    should be considered.
that's probably not relevant to the patch.


> 
> Since the number of sockets has already been recorded in smp structure,
> use smp.sockets directly.


I'd rephrase commit message to something like this:
---
CPU topology is calculated by ..., and trying to recalculate it here
with another rules leads to an error, such as 

 ... example follows ..

So stop reinventing the another wheel and use topo values that ... has 
calculated. 

> 
> Fixes: 003f230e37d7 ("machine: Tweak the order of topology members in struct 
> CpuTopology")
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>  hw/smbios/smbios.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index d2007e70fb05..d67415d44dd8 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -1088,8 +1088,7 @@ void smbios_get_tables(MachineState *ms,
>          smbios_build_type_2_table();
>          smbios_build_type_3_table();
>  
> -        smbios_smp_sockets = DIV_ROUND_UP(ms->smp.cpus,
> -                                          ms->smp.cores * ms->smp.threads);
> +        smbios_smp_sockets = ms->smp.sockets;
>          assert(smbios_smp_sockets >= 1);
>  
>          for (i = 0; i < smbios_smp_sockets; i++) {