hw/smbios: fix thead count field in type 4 table

[PATCH] hw/smbios: fix thead count field in type 4 table

Posted by Tianrui Zhao 11 months, 2 weeks ago

The thread_count value in smbios type_4 table should be
(ms->smp.cores * ms->smp.threads). As according to smbios spec 7.5
Processor Information (Type 4), the field "Thread Count" means the
"Number of threads per processor socket" rather than number of
threads per core.

When apply this patch, use "-smp 4,sockets=1,cores=2,threads=2" to
boot VM, the dmidecode -t 4 shows like:

Handle 0x0400, DMI type 4, 48 bytes
Processor Information
        Socket Designation: CPU 0
        ...
        Core Count: 2
        Core Enabled: 2
        Thread Count: 4
        Characteristics: None

Signed-off-by: Tianrui Zhao <zhaotianrui@loongson.cn>
---
 hw/smbios/smbios.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index d2007e70fb..56aeaa069d 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;
+    int count;
 
     if (smbios_ep_type == SMBIOS_ENTRY_POINT_TYPE_64) {
         tbl_len = SMBIOS_TYPE_4_LEN_V30;
@@ -749,15 +750,15 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
 
     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;
+    count = ms->smp.cores * ms->smp.threads;
+    t->thread_count = (count > 255) ? 0xFF : count;
 
     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(count);
     }
 
     SMBIOS_BUILD_TABLE_POST;
-- 
2.39.1

Re: [PATCH] hw/smbios: fix thead count field in type 4 table

Posted by Zhao Liu 11 months, 2 weeks ago

On Tue, May 30, 2023 at 08:20:34PM +0800, Tianrui Zhao wrote:
> Date: Tue, 30 May 2023 20:20:34 +0800
> From: Tianrui Zhao <zhaotianrui@loongson.cn>
> Subject: [PATCH] hw/smbios: fix thead count field in type 4 table
> X-Mailer: git-send-email 2.39.1
> 
> The thread_count value in smbios type_4 table should be
> (ms->smp.cores * ms->smp.threads). As according to smbios spec 7.5
> Processor Information (Type 4), the field "Thread Count" means the
> "Number of threads per processor socket" rather than number of
> threads per core.
> 
> When apply this patch, use "-smp 4,sockets=1,cores=2,threads=2" to
> boot VM, the dmidecode -t 4 shows like:
> 
> Handle 0x0400, DMI type 4, 48 bytes
> Processor Information
>         Socket Designation: CPU 0
>         ...
>         Core Count: 2
>         Core Enabled: 2
>         Thread Count: 4
>         Characteristics: None
> 
> Signed-off-by: Tianrui Zhao <zhaotianrui@loongson.cn>
> ---
>  hw/smbios/smbios.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index d2007e70fb..56aeaa069d 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;
> +    int count;
>  
>      if (smbios_ep_type == SMBIOS_ENTRY_POINT_TYPE_64) {
>          tbl_len = SMBIOS_TYPE_4_LEN_V30;
> @@ -749,15 +750,15 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
>  
>      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;
> +    count = ms->smp.cores * ms->smp.threads;

Hi Ani & Tianrui,

From the comment of CpuTopology (include/hw/boards.h):

ms->cores means the "the number of cores in one cluster".
ms->threads means the "the number of threads in one core".

So ms->cores * ms->threads means the number of threads in one cluster
not one socket.

That's why I count the number of threads in a socket by "ms->smp.max_cpus
/ ms->smp.sockets" in [1].

The other correct way is:
ms->smp.cluster * ms->smp.cores * ms->smp.threads.

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

Thanks,
Zhao

> +    t->thread_count = (count > 255) ? 0xFF : count;
>  
>      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(count);
>      }
>  
>      SMBIOS_BUILD_TABLE_POST;
> -- 
> 2.39.1
> 
>

Re: [PATCH] hw/smbios: fix thead count field in type 4 table

Posted by bibo, mao 11 months, 2 weeks ago


在 2023/5/31 16:42, Zhao Liu 写道:
> On Tue, May 30, 2023 at 08:20:34PM +0800, Tianrui Zhao wrote:
>> Date: Tue, 30 May 2023 20:20:34 +0800
>> From: Tianrui Zhao <zhaotianrui@loongson.cn>
>> Subject: [PATCH] hw/smbios: fix thead count field in type 4 table
>> X-Mailer: git-send-email 2.39.1
>>
>> The thread_count value in smbios type_4 table should be
>> (ms->smp.cores * ms->smp.threads). As according to smbios spec 7.5
>> Processor Information (Type 4), the field "Thread Count" means the
>> "Number of threads per processor socket" rather than number of
>> threads per core.
>>
>> When apply this patch, use "-smp 4,sockets=1,cores=2,threads=2" to
>> boot VM, the dmidecode -t 4 shows like:
>>
>> Handle 0x0400, DMI type 4, 48 bytes
>> Processor Information
>>          Socket Designation: CPU 0
>>          ...
>>          Core Count: 2
>>          Core Enabled: 2
>>          Thread Count: 4
>>          Characteristics: None
>>
>> Signed-off-by: Tianrui Zhao <zhaotianrui@loongson.cn>
>> ---
>>   hw/smbios/smbios.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
>> index d2007e70fb..56aeaa069d 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;
>> +    int count;
>>   
>>       if (smbios_ep_type == SMBIOS_ENTRY_POINT_TYPE_64) {
>>           tbl_len = SMBIOS_TYPE_4_LEN_V30;
>> @@ -749,15 +750,15 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
>>   
>>       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;
>> +    count = ms->smp.cores * ms->smp.threads;
> 
> Hi Ani & Tianrui,
> 
>  From the comment of CpuTopology (include/hw/boards.h):
> 
> ms->cores means the "the number of cores in one cluster".
> ms->threads means the "the number of threads in one core".
> 
> So ms->cores * ms->threads means the number of threads in one cluster
> not one socket.
> 
> That's why I count the number of threads in a socket by "ms->smp.max_cpus
> / ms->smp.sockets" in [1].
> 
> The other correct way is:
> ms->smp.cluster * ms->smp.cores * ms->smp.threads.
> 
> [1]: https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg07229.html
ohh, we do not notice your patch. Your patch is better than us. one 
small nit about core_count calcuation if cluster/die is support. core 
count should be core number
  per package rather than per cluster or per die.

so it should be something like this?
     t->core_count = cpus_per_socket / ms->smp.threads
rather than ms->smp.cores


Regards
Bibo, mao
> 
> Thanks,
> Zhao
> 
>> +    t->thread_count = (count > 255) ? 0xFF : count;
>>   
>>       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(count);
>>       }
>>   
>>       SMBIOS_BUILD_TABLE_POST;
>> -- 
>> 2.39.1
>>
>>

Re: [PATCH] hw/smbios: fix thead count field in type 4 table

Posted by Zhao Liu 11 months, 1 week ago

On Thu, Jun 01, 2023 at 09:09:13AM +0800, bibo, mao wrote:
> Date: Thu, 1 Jun 2023 09:09:13 +0800
> From: "bibo, mao" <maobibo@loongson.cn>
> Subject: Re: [PATCH] hw/smbios: fix thead count field in type 4 table
> 
> 
> 
> 在 2023/5/31 16:42, Zhao Liu 写道:
> > On Tue, May 30, 2023 at 08:20:34PM +0800, Tianrui Zhao wrote:
> > > Date: Tue, 30 May 2023 20:20:34 +0800
> > > From: Tianrui Zhao <zhaotianrui@loongson.cn>
> > > Subject: [PATCH] hw/smbios: fix thead count field in type 4 table
> > > X-Mailer: git-send-email 2.39.1
> > > 
> > > The thread_count value in smbios type_4 table should be
> > > (ms->smp.cores * ms->smp.threads). As according to smbios spec 7.5
> > > Processor Information (Type 4), the field "Thread Count" means the
> > > "Number of threads per processor socket" rather than number of
> > > threads per core.
> > > 
> > > When apply this patch, use "-smp 4,sockets=1,cores=2,threads=2" to
> > > boot VM, the dmidecode -t 4 shows like:
> > > 
> > > Handle 0x0400, DMI type 4, 48 bytes
> > > Processor Information
> > >          Socket Designation: CPU 0
> > >          ...
> > >          Core Count: 2
> > >          Core Enabled: 2
> > >          Thread Count: 4
> > >          Characteristics: None
> > > 
> > > Signed-off-by: Tianrui Zhao <zhaotianrui@loongson.cn>
> > > ---
> > >   hw/smbios/smbios.c | 7 ++++---
> > >   1 file changed, 4 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> > > index d2007e70fb..56aeaa069d 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;
> > > +    int count;
> > >       if (smbios_ep_type == SMBIOS_ENTRY_POINT_TYPE_64) {
> > >           tbl_len = SMBIOS_TYPE_4_LEN_V30;
> > > @@ -749,15 +750,15 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
> > >       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;
> > > +    count = ms->smp.cores * ms->smp.threads;
> > 
> > Hi Ani & Tianrui,
> > 
> >  From the comment of CpuTopology (include/hw/boards.h):
> > 
> > ms->cores means the "the number of cores in one cluster".
> > ms->threads means the "the number of threads in one core".
> > 
> > So ms->cores * ms->threads means the number of threads in one cluster
> > not one socket.
> > 
> > That's why I count the number of threads in a socket by "ms->smp.max_cpus
> > / ms->smp.sockets" in [1].
> > 
> > The other correct way is:
> > ms->smp.cluster * ms->smp.cores * ms->smp.threads.
> > 
> > [1]: https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg07229.html
> ohh, we do not notice your patch. Your patch is better than us. one small
> nit about core_count calcuation if cluster/die is support. core count should
> be core number
>  per package rather than per cluster or per die.
> 
> so it should be something like this?
>     t->core_count = cpus_per_socket / ms->smp.threads
> rather than ms->smp.cores

Yes, I also fixed this in the third patch of my patch series [2].

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

Regards,
Zhao

> 
> 
> Regards
> Bibo, mao
> > 
> > Thanks,
> > Zhao
> > 
> > > +    t->thread_count = (count > 255) ? 0xFF : count;
> > >       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(count);
> > >       }
> > >       SMBIOS_BUILD_TABLE_POST;
> > > -- 
> > > 2.39.1
> > > 
> > > 
> 
>

Re: [PATCH] hw/smbios: fix thead count field in type 4 table

Posted by bibo, mao 11 months, 1 week ago


在 2023/6/1 10:26, Zhao Liu 写道:
> On Thu, Jun 01, 2023 at 09:09:13AM +0800, bibo, mao wrote:
>> Date: Thu, 1 Jun 2023 09:09:13 +0800
>> From: "bibo, mao" <maobibo@loongson.cn>
>> Subject: Re: [PATCH] hw/smbios: fix thead count field in type 4 table
>>
>>
>>
>> 在 2023/5/31 16:42, Zhao Liu 写道:
>>> On Tue, May 30, 2023 at 08:20:34PM +0800, Tianrui Zhao wrote:
>>>> Date: Tue, 30 May 2023 20:20:34 +0800
>>>> From: Tianrui Zhao <zhaotianrui@loongson.cn>
>>>> Subject: [PATCH] hw/smbios: fix thead count field in type 4 table
>>>> X-Mailer: git-send-email 2.39.1
>>>>
>>>> The thread_count value in smbios type_4 table should be
>>>> (ms->smp.cores * ms->smp.threads). As according to smbios spec 7.5
>>>> Processor Information (Type 4), the field "Thread Count" means the
>>>> "Number of threads per processor socket" rather than number of
>>>> threads per core.
>>>>
>>>> When apply this patch, use "-smp 4,sockets=1,cores=2,threads=2" to
>>>> boot VM, the dmidecode -t 4 shows like:
>>>>
>>>> Handle 0x0400, DMI type 4, 48 bytes
>>>> Processor Information
>>>>           Socket Designation: CPU 0
>>>>           ...
>>>>           Core Count: 2
>>>>           Core Enabled: 2
>>>>           Thread Count: 4
>>>>           Characteristics: None
>>>>
>>>> Signed-off-by: Tianrui Zhao <zhaotianrui@loongson.cn>
>>>> ---
>>>>    hw/smbios/smbios.c | 7 ++++---
>>>>    1 file changed, 4 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
>>>> index d2007e70fb..56aeaa069d 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;
>>>> +    int count;
>>>>        if (smbios_ep_type == SMBIOS_ENTRY_POINT_TYPE_64) {
>>>>            tbl_len = SMBIOS_TYPE_4_LEN_V30;
>>>> @@ -749,15 +750,15 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
>>>>        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;
>>>> +    count = ms->smp.cores * ms->smp.threads;
>>>
>>> Hi Ani & Tianrui,
>>>
>>>   From the comment of CpuTopology (include/hw/boards.h):
>>>
>>> ms->cores means the "the number of cores in one cluster".
>>> ms->threads means the "the number of threads in one core".
>>>
>>> So ms->cores * ms->threads means the number of threads in one cluster
>>> not one socket.
>>>
>>> That's why I count the number of threads in a socket by "ms->smp.max_cpus
>>> / ms->smp.sockets" in [1].
>>>
>>> The other correct way is:
>>> ms->smp.cluster * ms->smp.cores * ms->smp.threads.
>>>
>>> [1]: https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg07229.html
>> ohh, we do not notice your patch. Your patch is better than us. one small
>> nit about core_count calcuation if cluster/die is support. core count should
>> be core number
>>   per package rather than per cluster or per die.
>>
>> so it should be something like this?
>>      t->core_count = cpus_per_socket / ms->smp.threads
>> rather than ms->smp.cores
> 
> Yes, I also fixed this in the third patch of my patch series [2].
> 
> [2]: https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg07231.html

Great, that sounds good to me.

Regards
Bibo, Mao

> 
> Regards,
> Zhao
> 
>>
>>
>> Regards
>> Bibo, mao
>>>
>>> Thanks,
>>> Zhao
>>>
>>>> +    t->thread_count = (count > 255) ? 0xFF : count;
>>>>        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(count);
>>>>        }
>>>>        SMBIOS_BUILD_TABLE_POST;
>>>> -- 
>>>> 2.39.1
>>>>
>>>>
>>
>>

Re: [PATCH] hw/smbios: fix thead count field in type 4 table

Posted by Ani Sinha 11 months, 2 weeks ago


> On 31-May-2023, at 2:12 PM, Zhao Liu <zhao1.liu@intel.com> wrote:
> 
> On Tue, May 30, 2023 at 08:20:34PM +0800, Tianrui Zhao wrote:
>> Date: Tue, 30 May 2023 20:20:34 +0800
>> From: Tianrui Zhao <zhaotianrui@loongson.cn>
>> Subject: [PATCH] hw/smbios: fix thead count field in type 4 table
>> X-Mailer: git-send-email 2.39.1
>> 
>> The thread_count value in smbios type_4 table should be
>> (ms->smp.cores * ms->smp.threads). As according to smbios spec 7.5
>> Processor Information (Type 4), the field "Thread Count" means the
>> "Number of threads per processor socket" rather than number of
>> threads per core.
>> 
>> When apply this patch, use "-smp 4,sockets=1,cores=2,threads=2" to
>> boot VM, the dmidecode -t 4 shows like:
>> 
>> Handle 0x0400, DMI type 4, 48 bytes
>> Processor Information
>>        Socket Designation: CPU 0
>>        ...
>>        Core Count: 2
>>        Core Enabled: 2
>>        Thread Count: 4
>>        Characteristics: None
>> 
>> Signed-off-by: Tianrui Zhao <zhaotianrui@loongson.cn>
>> ---
>> hw/smbios/smbios.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>> 
>> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
>> index d2007e70fb..56aeaa069d 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;
>> +    int count;
>> 
>>     if (smbios_ep_type == SMBIOS_ENTRY_POINT_TYPE_64) {
>>         tbl_len = SMBIOS_TYPE_4_LEN_V30;
>> @@ -749,15 +750,15 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
>> 
>>     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;
>> +    count = ms->smp.cores * ms->smp.threads;
> 
> Hi Ani & Tianrui,
> 
> From the comment of CpuTopology (include/hw/boards.h):
> 
> ms->cores means the "the number of cores in one cluster".
> ms->threads means the "the number of threads in one core".
> 
> So ms->cores * ms->threads means the number of threads in one cluster
> not one socket.

Yes ok other than arm/virt I do not see any other arch that supports clusters.

> 
> That's why I count the number of threads in a socket by "ms->smp.max_cpus
> / ms->smp.sockets" in [1].
> 
> The other correct way is:
> ms->smp.cluster * ms->smp.cores * ms->smp.threads.
> 

Sure, I prefer this calculation than what you have used.


> [1]: https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg07229.html
> 
> Thanks,
> Zhao
> 
>> +    t->thread_count = (count > 255) ? 0xFF : count;
>> 
>>     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(count);
>>     }
>> 
>>     SMBIOS_BUILD_TABLE_POST;
>> -- 
>> 2.39.1

Re: [PATCH] hw/smbios: fix thead count field in type 4 table

Posted by Tianrui Zhao 11 months, 2 weeks ago


在 2023年05月31日 16:42, Zhao Liu 写道:
> On Tue, May 30, 2023 at 08:20:34PM +0800, Tianrui Zhao wrote:
>> Date: Tue, 30 May 2023 20:20:34 +0800
>> From: Tianrui Zhao <zhaotianrui@loongson.cn>
>> Subject: [PATCH] hw/smbios: fix thead count field in type 4 table
>> X-Mailer: git-send-email 2.39.1
>>
>> The thread_count value in smbios type_4 table should be
>> (ms->smp.cores * ms->smp.threads). As according to smbios spec 7.5
>> Processor Information (Type 4), the field "Thread Count" means the
>> "Number of threads per processor socket" rather than number of
>> threads per core.
>>
>> When apply this patch, use "-smp 4,sockets=1,cores=2,threads=2" to
>> boot VM, the dmidecode -t 4 shows like:
>>
>> Handle 0x0400, DMI type 4, 48 bytes
>> Processor Information
>>          Socket Designation: CPU 0
>>          ...
>>          Core Count: 2
>>          Core Enabled: 2
>>          Thread Count: 4
>>          Characteristics: None
>>
>> Signed-off-by: Tianrui Zhao <zhaotianrui@loongson.cn>
>> ---
>>   hw/smbios/smbios.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
>> index d2007e70fb..56aeaa069d 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;
>> +    int count;
>>   
>>       if (smbios_ep_type == SMBIOS_ENTRY_POINT_TYPE_64) {
>>           tbl_len = SMBIOS_TYPE_4_LEN_V30;
>> @@ -749,15 +750,15 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
>>   
>>       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;
>> +    count = ms->smp.cores * ms->smp.threads;
> Hi Ani & Tianrui,
>
>  From the comment of CpuTopology (include/hw/boards.h):
>
> ms->cores means the "the number of cores in one cluster".
> ms->threads means the "the number of threads in one core".
>
> So ms->cores * ms->threads means the number of threads in one cluster
> not one socket.
>
> That's why I count the number of threads in a socket by "ms->smp.max_cpus
> / ms->smp.sockets" in [1].
>
> The other correct way is:
> ms->smp.cluster * ms->smp.cores * ms->smp.threads.
>
> [1]: https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg07229.html
>
> Thanks,
> Zhao
Thanks for your explanations for  the meaning of the number of threads 
per socket, I look up the relevant code again and I agree with your 
patch [1].

Thanks
Tianrui Zhao
>> +    t->thread_count = (count > 255) ? 0xFF : count;
>>   
>>       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(count);
>>       }
>>   
>>       SMBIOS_BUILD_TABLE_POST;
>> -- 
>> 2.39.1
>>
>>