[PATCH v3 0/4] hw/smbios: Cleanup topology related variables

[PATCH v3 0/4] hw/smbios: Cleanup topology related variables

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

Hi all,

This is my v3 patch series based on 48ab886d3da4f ("Merge tag 'pull-
target-arm-20230619' of https://git.linaro.org/people/pmaydell/qemu-arm
into staging").

Compared with v2 [1], v3 introduces 2 helpers to wrap the calculation of
threads/cores per socket so that smbios can use these 2 helpers directly
to avoid calculation error caused by missing topology changes.

Also due to the introduction of these two helpers, I involve more people
for review of this v3.


Introduction
============

This patchset is split from my previous hybrid topology RFC [2].

There are three places for topology-related cleanup:

1. Fix the calculation of the number of sockets.

   Due to the misuse of the smp.cpus variable and the change in the
   meaning of smp.cores, the calculation of socket number in smbios is
   incorrect. This can be fixed by using smp.sockets directly.

2. Fix core count in type4.

   The meaning of smp.cores changed so that the calculation of cores
   per socket became wrong.

   v3 introduces the helper "machine_topo_get_cores_per_socket()" to
   wrap the calculation of cores per socket. This can help other modules
   avoid calculation error caused by missing topology changes.

3. Fix thread count in type4.

   I also found that the definition of thread count in type4 doesn't
   match the spec (smbios 3.0.0) and cleaned it up as well.

   Similar to core count, v3 uses a new helper to fix this.

[1]: https://lists.gnu.org/archive/html/qemu-devel/2023-06/msg00072.html
[2]: https://mail.gnu.org/archive/html/qemu-devel/2023-02/msg03205.html


Regards,
Zhao

---
Zhao Liu (4):
  machine: Add helpers to get cores/threads per socket
  hw/smbios: Fix smbios_smp_sockets caculation
  hw/smbios: Fix thread count in type4
  hw/smbios: Fix core count in type4

 hw/smbios/smbios.c  | 16 ++++++++++------
 include/hw/boards.h | 12 ++++++++++++
 2 files changed, 22 insertions(+), 6 deletions(-)

-- 
2.34.1

[PATCH v3 1/4] machine: Add helpers to get cores/threads per socket

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

The number of cores/threads per socket are needed for smbios, and are
also useful for other modules.

Provide the helpers to wrap the calculation of cores/threads per socket
so that we can avoid calculation errors caused by other modules miss
topology changes.

Suggested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
v3:
 * The new patch to wrap the calculation of cores/threads per socket.
---
 include/hw/boards.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index a385010909d5..40ee22fd93e3 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -384,6 +384,18 @@ struct MachineState {
     } \
     type_init(machine_initfn##_register_types)
 
+static inline
+unsigned int machine_topo_get_cores_per_socket(const MachineState *ms)
+{
+    return ms->smp.cores * ms->smp.clusters * ms->smp.dies;
+}
+
+static inline
+unsigned int machine_topo_get_threads_per_socket(const MachineState *ms)
+{
+    return ms->smp.threads * machine_topo_get_cores_per_socket(ms);
+}
+
 extern GlobalProperty hw_compat_8_0[];
 extern const size_t hw_compat_8_0_len;
 
-- 
2.34.1

Re: [PATCH v3 1/4] machine: Add helpers to get cores/threads per socket

[Igor Mammedov]

On Tue, 20 Jun 2023 18:39:55 +0800
Zhao Liu <zhao1.liu@linux.intel.com> wrote:

> From: Zhao Liu <zhao1.liu@intel.com>
> 
> The number of cores/threads per socket are needed for smbios, and are
> also useful for other modules.
> 
> Provide the helpers to wrap the calculation of cores/threads per socket
> so that we can avoid calculation errors caused by other modules miss
> topology changes.
> 
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
> v3:
>  * The new patch to wrap the calculation of cores/threads per socket.
> ---
>  include/hw/boards.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index a385010909d5..40ee22fd93e3 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -384,6 +384,18 @@ struct MachineState {
>      } \
>      type_init(machine_initfn##_register_types)
>  
> +static inline
> +unsigned int machine_topo_get_cores_per_socket(const MachineState *ms)
> +{
> +    return ms->smp.cores * ms->smp.clusters * ms->smp.dies;
> +}
> +
> +static inline
> +unsigned int machine_topo_get_threads_per_socket(const MachineState *ms)
> +{
> +    return ms->smp.threads * machine_topo_get_cores_per_socket(ms);
> +}

I'd put those before/after machine_parse_smp_config
, just declarations.

And put definitions into hw/core/machine-smp.c again close to machine_parse_smp_config

> +
>  extern GlobalProperty hw_compat_8_0[];
>  extern const size_t hw_compat_8_0_len;
>  

Re: [PATCH v3 1/4] machine: Add helpers to get cores/threads per socket

[Zhao Liu]

On Mon, Jun 26, 2023 at 03:43:25PM +0200, Igor Mammedov wrote:
> Date: Mon, 26 Jun 2023 15:43:25 +0200
> From: Igor Mammedov <imammedo@redhat.com>
> Subject: Re: [PATCH v3 1/4] machine: Add helpers to get cores/threads per
>  socket
> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; x86_64-redhat-linux-gnu)
> 
> On Tue, 20 Jun 2023 18:39:55 +0800
> Zhao Liu <zhao1.liu@linux.intel.com> wrote:
> 
> > From: Zhao Liu <zhao1.liu@intel.com>
> > 
> > The number of cores/threads per socket are needed for smbios, and are
> > also useful for other modules.
> > 
> > Provide the helpers to wrap the calculation of cores/threads per socket
> > so that we can avoid calculation errors caused by other modules miss
> > topology changes.
> > 
> > Suggested-by: Igor Mammedov <imammedo@redhat.com>
> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > ---
> > v3:
> >  * The new patch to wrap the calculation of cores/threads per socket.
> > ---
> >  include/hw/boards.h | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > index a385010909d5..40ee22fd93e3 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -384,6 +384,18 @@ struct MachineState {
> >      } \
> >      type_init(machine_initfn##_register_types)
> >  
> > +static inline
> > +unsigned int machine_topo_get_cores_per_socket(const MachineState *ms)
> > +{
> > +    return ms->smp.cores * ms->smp.clusters * ms->smp.dies;
> > +}
> > +
> > +static inline
> > +unsigned int machine_topo_get_threads_per_socket(const MachineState *ms)
> > +{
> > +    return ms->smp.threads * machine_topo_get_cores_per_socket(ms);
> > +}
> 
> I'd put those before/after machine_parse_smp_config
> , just declarations.
> 
> And put definitions into hw/core/machine-smp.c again close to machine_parse_smp_config

Okay, thanks!

> 
> > +
> >  extern GlobalProperty hw_compat_8_0[];
> >  extern const size_t hw_compat_8_0_len;
> >  
> 

[PATCH v3 2/4] hw/smbios: Fix smbios_smp_sockets caculation

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

smp.sockets is the number of sockets which is configured by "-smp" (
otherwise, the default is 1). Trying to recalculate it here with another
rules leads to errors, such as:

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.

   With the introduction of cluster, now smp.cores means the number of
   cores in one cluster. So smp.cores * smp.threads just means the
   threads in a cluster not in a socket.

2. On the other hand, we shouldn't use smp.cpus here because it
   indicates the initial number of online CPUs at the boot time, and is
   not mathematically related to smp.sockets.

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

Fixes: 003f230e37d7 ("machine: Tweak the order of topology members in struct CpuTopology")
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
v3:
 * Reorganized changlog. (Igor)

v2:
 * None.
---
 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++) {
-- 
2.34.1

[PATCH v3 3/4] hw/smbios: Fix thread count in type4

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

From SMBIOS 3.0 specification, thread count field means:

Thread Count is the total number of threads detected by the BIOS for
this processor socket. It is a processor-wide count, not a
thread-per-core count. [1]

So here we should use threads per socket other than threads per core.

[1] SMBIOS 3.0.0, section 7.5.8, Processor Information - Thread Count

Fixes: c97294ec1b9e ("SMBIOS: Build aggregate smbios tables and entry point")
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
v3:
 * Use the wrapped hepler to get threads per socket.

v2:
 * Rename cpus_per_socket to threads_per_socket.
 * Add the comment about smp.max_cpus. Thread count and core count will
   be calculated in 2 ways and will add a sanity check to ensure we
   don't miss any topology level.
---
 hw/smbios/smbios.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index d67415d44dd8..3aae9328c014 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -713,6 +713,7 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
 {
     char sock_str[128];
     size_t tbl_len = SMBIOS_TYPE_4_LEN_V28;
+    unsigned threads_per_socket;
 
     if (smbios_ep_type == SMBIOS_ENTRY_POINT_TYPE_64) {
         tbl_len = SMBIOS_TYPE_4_LEN_V30;
@@ -747,17 +748,19 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
     SMBIOS_TABLE_SET_STR(4, asset_tag_number_str, type4.asset);
     SMBIOS_TABLE_SET_STR(4, part_number_str, type4.part);
 
+    threads_per_socket = machine_topo_get_threads_per_socket(ms);
+
     t->core_count = (ms->smp.cores > 255) ? 0xFF : ms->smp.cores;
     t->core_enabled = t->core_count;
 
-    t->thread_count = (ms->smp.threads > 255) ? 0xFF : ms->smp.threads;
+    t->thread_count = (threads_per_socket > 255) ? 0xFF : threads_per_socket;
 
     t->processor_characteristics = cpu_to_le16(0x02); /* Unknown */
     t->processor_family2 = cpu_to_le16(0x01); /* Other */
 
     if (tbl_len == SMBIOS_TYPE_4_LEN_V30) {
         t->core_count2 = t->core_enabled2 = cpu_to_le16(ms->smp.cores);
-        t->thread_count2 = cpu_to_le16(ms->smp.threads);
+        t->thread_count2 = cpu_to_le16(threads_per_socket);
     }
 
     SMBIOS_BUILD_TABLE_POST;
-- 
2.34.1

Re: [PATCH v3 3/4] hw/smbios: Fix thread count in type4

[Igor Mammedov]

On Tue, 20 Jun 2023 18:39:57 +0800
Zhao Liu <zhao1.liu@linux.intel.com> wrote:

> From: Zhao Liu <zhao1.liu@intel.com>
> 
> From SMBIOS 3.0 specification, thread count field means:
> 
> Thread Count is the total number of threads detected by the BIOS for
> this processor socket. It is a processor-wide count, not a
> thread-per-core count. [1]
> 
> So here we should use threads per socket other than threads per core.
> 
> [1] SMBIOS 3.0.0, section 7.5.8, Processor Information - Thread Count
> 
> Fixes: c97294ec1b9e ("SMBIOS: Build aggregate smbios tables and entry point")
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
> v3:
>  * Use the wrapped hepler to get threads per socket.
> 
> v2:
>  * Rename cpus_per_socket to threads_per_socket.
>  * Add the comment about smp.max_cpus. Thread count and core count will
>    be calculated in 2 ways and will add a sanity check to ensure we
>    don't miss any topology level.
> ---
>  hw/smbios/smbios.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index d67415d44dd8..3aae9328c014 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -713,6 +713,7 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
>  {
>      char sock_str[128];
>      size_t tbl_len = SMBIOS_TYPE_4_LEN_V28;
> +    unsigned threads_per_socket;
>  
>      if (smbios_ep_type == SMBIOS_ENTRY_POINT_TYPE_64) {
>          tbl_len = SMBIOS_TYPE_4_LEN_V30;
> @@ -747,17 +748,19 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
>      SMBIOS_TABLE_SET_STR(4, asset_tag_number_str, type4.asset);
>      SMBIOS_TABLE_SET_STR(4, part_number_str, type4.part);
>  
> +    threads_per_socket = machine_topo_get_threads_per_socket(ms);
                                ^^^^
Are there any other places we can clean up and reuse this wrapper?

> +
>      t->core_count = (ms->smp.cores > 255) ? 0xFF : ms->smp.cores;
>      t->core_enabled = t->core_count;
>  
> -    t->thread_count = (ms->smp.threads > 255) ? 0xFF : ms->smp.threads;
> +    t->thread_count = (threads_per_socket > 255) ? 0xFF : threads_per_socket;
>  
>      t->processor_characteristics = cpu_to_le16(0x02); /* Unknown */
>      t->processor_family2 = cpu_to_le16(0x01); /* Other */
>  
>      if (tbl_len == SMBIOS_TYPE_4_LEN_V30) {
>          t->core_count2 = t->core_enabled2 = cpu_to_le16(ms->smp.cores);
> -        t->thread_count2 = cpu_to_le16(ms->smp.threads);
> +        t->thread_count2 = cpu_to_le16(threads_per_socket);
>      }
>  
>      SMBIOS_BUILD_TABLE_POST;

Re: [PATCH v3 3/4] hw/smbios: Fix thread count in type4

[Zhao Liu]

On Mon, Jun 26, 2023 at 03:44:49PM +0200, Igor Mammedov wrote:
> Date: Mon, 26 Jun 2023 15:44:49 +0200
> From: Igor Mammedov <imammedo@redhat.com>
> Subject: Re: [PATCH v3 3/4] hw/smbios: Fix thread count in type4
> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; x86_64-redhat-linux-gnu)
> 
> On Tue, 20 Jun 2023 18:39:57 +0800
> Zhao Liu <zhao1.liu@linux.intel.com> wrote:
> 
> > From: Zhao Liu <zhao1.liu@intel.com>
> > 
> > From SMBIOS 3.0 specification, thread count field means:
> > 
> > Thread Count is the total number of threads detected by the BIOS for
> > this processor socket. It is a processor-wide count, not a
> > thread-per-core count. [1]
> > 
> > So here we should use threads per socket other than threads per core.
> > 
> > [1] SMBIOS 3.0.0, section 7.5.8, Processor Information - Thread Count
> > 
> > Fixes: c97294ec1b9e ("SMBIOS: Build aggregate smbios tables and entry point")
> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > ---
> > v3:
> >  * Use the wrapped hepler to get threads per socket.
> > 
> > v2:
> >  * Rename cpus_per_socket to threads_per_socket.
> >  * Add the comment about smp.max_cpus. Thread count and core count will
> >    be calculated in 2 ways and will add a sanity check to ensure we
> >    don't miss any topology level.
> > ---
> >  hw/smbios/smbios.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> > index d67415d44dd8..3aae9328c014 100644
> > --- a/hw/smbios/smbios.c
> > +++ b/hw/smbios/smbios.c
> > @@ -713,6 +713,7 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
> >  {
> >      char sock_str[128];
> >      size_t tbl_len = SMBIOS_TYPE_4_LEN_V28;
> > +    unsigned threads_per_socket;
> >  
> >      if (smbios_ep_type == SMBIOS_ENTRY_POINT_TYPE_64) {
> >          tbl_len = SMBIOS_TYPE_4_LEN_V30;
> > @@ -747,17 +748,19 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
> >      SMBIOS_TABLE_SET_STR(4, asset_tag_number_str, type4.asset);
> >      SMBIOS_TABLE_SET_STR(4, part_number_str, type4.part);
> >  
> > +    threads_per_socket = machine_topo_get_threads_per_socket(ms);
>                                 ^^^^
> Are there any other places we can clean up and reuse this wrapper?

Yes, in the struct CPUState of include/hw/core/cpu.h, the "nr_cores"
need this helper to wrap the calculation. Also there are scenarios in
the i386 CPUID emulation/MSR_CORE_THREAD_COUNT related things that
require these information.

I have another patchset that contains the related fixes/changes [1].
To avoid conflicts, I thought I would follow up and use our newly
added helper after that patchset has been fully received.

Add WHPX also need cores per socket, but I've used another way to fix
[2].

[1]: https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg07179.html
[2]: https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg07196.html

- Zhao

> 
> > +
> >      t->core_count = (ms->smp.cores > 255) ? 0xFF : ms->smp.cores;
> >      t->core_enabled = t->core_count;
> >  
> > -    t->thread_count = (ms->smp.threads > 255) ? 0xFF : ms->smp.threads;
> > +    t->thread_count = (threads_per_socket > 255) ? 0xFF : threads_per_socket;
> >  
> >      t->processor_characteristics = cpu_to_le16(0x02); /* Unknown */
> >      t->processor_family2 = cpu_to_le16(0x01); /* Other */
> >  
> >      if (tbl_len == SMBIOS_TYPE_4_LEN_V30) {
> >          t->core_count2 = t->core_enabled2 = cpu_to_le16(ms->smp.cores);
> > -        t->thread_count2 = cpu_to_le16(ms->smp.threads);
> > +        t->thread_count2 = cpu_to_le16(threads_per_socket);
> >      }
> >  
> >      SMBIOS_BUILD_TABLE_POST;
> 

[PATCH v3 4/4] hw/smbios: Fix core count in type4

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

From SMBIOS 3.0 specification, core count field means:

Core Count is the number of cores detected by the BIOS for this
processor socket. [1]

Before 003f230e37d7 ("machine: Tweak the order of topology members in
struct CpuTopology"), MachineState.smp.cores means "the number of cores
in one package", and it's correct to use smp.cores for core count.

But 003f230e37d7 changes the smp.cores' meaning to "the number of cores
in one die" and doesn't change the original smp.cores' use in smbios as
well, which makes core count in type4 go wrong.

Fix this issue with the correct "cores per socket" caculation.

[1] SMBIOS 3.0.0, section 7.5.6, Processor Information - Core Count

Fixes: 003f230e37d7 ("machine: Tweak the order of topology members in struct CpuTopology")
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
v3:
 * Use the wrapped helper to get cores per socket.

v2:
 * Calculate cores_per_socket in a different way from
   threads_per_socket.
 * Add the sanity check to ensure consistency of results between these 2
   ways. This can help not miss any future change of cpu topology.
---
 hw/smbios/smbios.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index 3aae9328c014..10cd22f610ef 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -714,6 +714,7 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
     char sock_str[128];
     size_t tbl_len = SMBIOS_TYPE_4_LEN_V28;
     unsigned threads_per_socket;
+    unsigned cores_per_socket;
 
     if (smbios_ep_type == SMBIOS_ENTRY_POINT_TYPE_64) {
         tbl_len = SMBIOS_TYPE_4_LEN_V30;
@@ -749,8 +750,9 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
     SMBIOS_TABLE_SET_STR(4, part_number_str, type4.part);
 
     threads_per_socket = machine_topo_get_threads_per_socket(ms);
+    cores_per_socket = machine_topo_get_cores_per_socket(ms);
 
-    t->core_count = (ms->smp.cores > 255) ? 0xFF : ms->smp.cores;
+    t->core_count = (cores_per_socket > 255) ? 0xFF : cores_per_socket;
     t->core_enabled = t->core_count;
 
     t->thread_count = (threads_per_socket > 255) ? 0xFF : threads_per_socket;
@@ -759,7 +761,7 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
     t->processor_family2 = cpu_to_le16(0x01); /* Other */
 
     if (tbl_len == SMBIOS_TYPE_4_LEN_V30) {
-        t->core_count2 = t->core_enabled2 = cpu_to_le16(ms->smp.cores);
+        t->core_count2 = t->core_enabled2 = cpu_to_le16(cores_per_socket);
         t->thread_count2 = cpu_to_le16(threads_per_socket);
     }
 
-- 
2.34.1

Re: [PATCH v3 0/4] hw/smbios: Cleanup topology related variables

[Igor Mammedov]

On Tue, 20 Jun 2023 18:39:54 +0800
Zhao Liu <zhao1.liu@linux.intel.com> wrote:

> From: Zhao Liu <zhao1.liu@intel.com>
> 
> Hi all,
> 
> This is my v3 patch series based on 48ab886d3da4f ("Merge tag 'pull-
> target-arm-20230619' of https://git.linaro.org/people/pmaydell/qemu-arm
> into staging").
> 
> Compared with v2 [1], v3 introduces 2 helpers to wrap the calculation of
> threads/cores per socket so that smbios can use these 2 helpers directly
> to avoid calculation error caused by missing topology changes.
> 
> Also due to the introduction of these two helpers, I involve more people
> for review of this v3.

other than nitpicking in patch 1/4, the series looks good to me.

> 
> 
> Introduction
> ============
> 
> This patchset is split from my previous hybrid topology RFC [2].
> 
> There are three places for topology-related cleanup:
> 
> 1. Fix the calculation of the number of sockets.
> 
>    Due to the misuse of the smp.cpus variable and the change in the
>    meaning of smp.cores, the calculation of socket number in smbios is
>    incorrect. This can be fixed by using smp.sockets directly.
> 
> 2. Fix core count in type4.
> 
>    The meaning of smp.cores changed so that the calculation of cores
>    per socket became wrong.
> 
>    v3 introduces the helper "machine_topo_get_cores_per_socket()" to
>    wrap the calculation of cores per socket. This can help other modules
>    avoid calculation error caused by missing topology changes.
> 
> 3. Fix thread count in type4.
> 
>    I also found that the definition of thread count in type4 doesn't
>    match the spec (smbios 3.0.0) and cleaned it up as well.
> 
>    Similar to core count, v3 uses a new helper to fix this.
> 
> [1]: https://lists.gnu.org/archive/html/qemu-devel/2023-06/msg00072.html
> [2]: https://mail.gnu.org/archive/html/qemu-devel/2023-02/msg03205.html
> 
> 
> Regards,
> Zhao
> 
> ---
> Zhao Liu (4):
>   machine: Add helpers to get cores/threads per socket
>   hw/smbios: Fix smbios_smp_sockets caculation
>   hw/smbios: Fix thread count in type4
>   hw/smbios: Fix core count in type4
> 
>  hw/smbios/smbios.c  | 16 ++++++++++------
>  include/hw/boards.h | 12 ++++++++++++
>  2 files changed, 22 insertions(+), 6 deletions(-)
> 

Re: [PATCH v3 0/4] hw/smbios: Cleanup topology related variables

[Zhao Liu]

On Mon, Jun 26, 2023 at 03:48:17PM +0200, Igor Mammedov wrote:
> Date: Mon, 26 Jun 2023 15:48:17 +0200
> From: Igor Mammedov <imammedo@redhat.com>
> Subject: Re: [PATCH v3 0/4] hw/smbios: Cleanup topology related variables
> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; x86_64-redhat-linux-gnu)
> 
> On Tue, 20 Jun 2023 18:39:54 +0800
> Zhao Liu <zhao1.liu@linux.intel.com> wrote:
> 
> > From: Zhao Liu <zhao1.liu@intel.com>
> > 
> > Hi all,
> > 
> > This is my v3 patch series based on 48ab886d3da4f ("Merge tag 'pull-
> > target-arm-20230619' of https://git.linaro.org/people/pmaydell/qemu-arm
> > into staging").
> > 
> > Compared with v2 [1], v3 introduces 2 helpers to wrap the calculation of
> > threads/cores per socket so that smbios can use these 2 helpers directly
> > to avoid calculation error caused by missing topology changes.
> > 
> > Also due to the introduction of these two helpers, I involve more people
> > for review of this v3.
> 
> other than nitpicking in patch 1/4, the series looks good to me.

Thanks Igor! I'll fix the 1/4 and refresh a new version.

-Zhao

> 
> > 
> > 
> > Introduction
> > ============
> > 
> > This patchset is split from my previous hybrid topology RFC [2].
> > 
> > There are three places for topology-related cleanup:
> > 
> > 1. Fix the calculation of the number of sockets.
> > 
> >    Due to the misuse of the smp.cpus variable and the change in the
> >    meaning of smp.cores, the calculation of socket number in smbios is
> >    incorrect. This can be fixed by using smp.sockets directly.
> > 
> > 2. Fix core count in type4.
> > 
> >    The meaning of smp.cores changed so that the calculation of cores
> >    per socket became wrong.
> > 
> >    v3 introduces the helper "machine_topo_get_cores_per_socket()" to
> >    wrap the calculation of cores per socket. This can help other modules
> >    avoid calculation error caused by missing topology changes.
> > 
> > 3. Fix thread count in type4.
> > 
> >    I also found that the definition of thread count in type4 doesn't
> >    match the spec (smbios 3.0.0) and cleaned it up as well.
> > 
> >    Similar to core count, v3 uses a new helper to fix this.
> > 
> > [1]: https://lists.gnu.org/archive/html/qemu-devel/2023-06/msg00072.html
> > [2]: https://mail.gnu.org/archive/html/qemu-devel/2023-02/msg03205.html
> > 
> > 
> > Regards,
> > Zhao
> > 
> > ---
> > Zhao Liu (4):
> >   machine: Add helpers to get cores/threads per socket
> >   hw/smbios: Fix smbios_smp_sockets caculation
> >   hw/smbios: Fix thread count in type4
> >   hw/smbios: Fix core count in type4
> > 
> >  hw/smbios/smbios.c  | 16 ++++++++++------
> >  include/hw/boards.h | 12 ++++++++++++
> >  2 files changed, 22 insertions(+), 6 deletions(-)
> > 
>