[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CB1053E012AD1BAD+5d6129b8-86d7-421c-806b-d872f5a3ea0a@uniontech.com>
Date: Thu, 27 Nov 2025 17:48:49 +0800
From: Qiang Ma <maqianga@...ontech.com>
To: Huacai Chen <chenhuacai@...nel.org>
Cc: kernel@...0n.name, hengqi.chen@...il.com, loongarch@...ts.linux.dev,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] LoongArch: Correct the calculation logic of
thread_count
在 2025/11/27 17:35, Huacai Chen 写道:
> On Thu, Nov 27, 2025 at 4:56 PM Qiang Ma <maqianga@...ontech.com> wrote:
>>
>> 在 2025/11/27 15:36, Huacai Chen 写道:
>>> Hi, Qiang,
>>>
>>> On Thu, Nov 27, 2025 at 11:32 AM Qiang Ma <maqianga@...ontech.com> wrote:
>>>> For thread_count, the current calculation method has a maximum of 255,
>>>> which may not be sufficient in the future. Therefore, we are correcting
>>>> it now.
>>>>
>>>> Reference: SMBIOS Specification, 7.5 Processor Information (Type 4)[1]
>>>>
>>>> [1]: https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.9.0.pdf
>>>>
>>>> Signed-off-by: Qiang Ma <maqianga@...ontech.com>
>>>> ---
>>>> arch/loongarch/kernel/setup.c | 9 ++++++++-
>>>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
>>>> index 25a87378e48e..8dd25fc4bd13 100644
>>>> --- a/arch/loongarch/kernel/setup.c
>>>> +++ b/arch/loongarch/kernel/setup.c
>>>> @@ -56,6 +56,7 @@
>>>> #define SMBIOS_FREQLOW_MASK 0xFF
>>>> #define SMBIOS_CORE_PACKAGE_OFFSET 0x23
>>>> #define SMBIOS_THREAD_PACKAGE_OFFSET 0x25
>>>> +#define SMBIOS_THREAD_PACKAGE_2_OFFSET 0x2E
>>>> #define LOONGSON_EFI_ENABLE (1 << 3)
>>>>
>>>> unsigned long fw_arg0, fw_arg1, fw_arg2;
>>>> @@ -120,13 +121,19 @@ static void __init parse_cpu_table(const struct dmi_header *dm)
>>>> {
>>>> long freq_temp = 0;
>>>> char *dmi_data = (char *)dm;
>>>> + u8 thread_count;
>>>>
>>>> freq_temp = ((*(dmi_data + SMBIOS_FREQHIGH_OFFSET) << 8) +
>>>> ((*(dmi_data + SMBIOS_FREQLOW_OFFSET)) & SMBIOS_FREQLOW_MASK));
>>>> cpu_clock_freq = freq_temp * 1000000;
>>>>
>>>> loongson_sysconf.cpuname = (void *)dmi_string_parse(dm, dmi_data[16]);
>>>> - loongson_sysconf.cores_per_package = *(dmi_data + SMBIOS_THREAD_PACKAGE_OFFSET);
>>>> + thread_count = *(dmi_data + SMBIOS_THREAD_PACKAGE_OFFSET);
>> Yes, this one should need to be converted:
>> thread_count = *(u8 *)(dmi_data + SMBIOS_THREAD_PACKAGE_OFFSET);
>>>> + if (thread_count != 0)
>>>> + loongson_sysconf.cores_per_package =
>>>> + dm->length >= 0x30 && thread_count == 0xFF ?
>>>> + *(u16 *)(dmi_data + SMBIOS_THREAD_PACKAGE_2_OFFSET) :
>>>> + thread_count;
>> This patch method refers to dmidecode, which is the same as it, like this:
>>
>> if (data[0x25] != 0)
>> pr_attr("Thread Count", "%u",
>> h->length >= 0x30 && data[0x25]
>> == 0xFF ?
>>
>> WORD(data + 0x2E) : data[0x25]);
>>
>>
>>> According SPEC, it uses THREAD_COUNT_2 only if dm->length >= 30 and
>>> thread_count == 0xff, so the correct and simplest way is like this:
>>>
>>> cpu_clock_freq = freq_temp * 1000000;
>>>
>>> loongson_sysconf.cpuname = (void *)dmi_string_parse(dm, dmi_data[16]);
>>> - loongson_sysconf.cores_per_package = *(dmi_data +
>>> SMBIOS_THREAD_PACKAGE_OFFSET);
>>> + if (dm->length < 0x30)
>>> + thread_count = *(u8 *)(dmi_data + SMBIOS_THREAD_PACKAGE_OFFSET);
>>> + else if (thread_count == 0xff)
>>> + thread_count = *(u16 *)(dmi_data +
>>> SMBIOS_THREAD_PACKAGE_2_OFFSET);
>>> + loongson_sysconf.cores_per_package = thread_count;
>>>
>>> pr_info("CpuClock = %llu\n", cpu_clock_freq);
>> I feel there is a problem with this method.
>> I saw on the 3C6000/Q machine a few days ago that the dm->lenght is 0x30,
>> but only one byte is used, and the thread_count is 128, not 0xFF.
>> Is the initialization thread_count = *(u8 *)(dmi_data +
>> SMBIOS_THREAD_PACKAGE_OFFSET) missing?
> Yes, you are right. Kernel code prefers 0xff rather than 0xFF, and I
I see.
> want to add a comment, so I think the below is OK.
Yes, I also think it's okay. I will update it to v3.
> loongson_sysconf.cores_per_package = *(u8 *)(dmi_data +
> SMBIOS_THREAD_PACKAGE_OFFSET);
> if (dm->length >= 0x30 && loongson_sysconf.cores_per_package == 0xff) {
> /* SMBIOS 3.0+ has ThreadCount2 for more than 255 threads */
> loongson_sysconf.cores_per_package = *(u16 *)(dmi_data
> + SMBIOS_THREAD_PACKAGE_2_OFFSET);
> }
> Huacai
>
>> Wouldn't it be better to change it this way?
>>
>> + u8 thread_count;
>>
>> freq_temp = ((*(dmi_data + SMBIOS_FREQHIGH_OFFSET) << 8) +
>> ((*(dmi_data + SMBIOS_FREQLOW_OFFSET)) &
>> SMBIOS_FREQLOW_MASK));
>> cpu_clock_freq = freq_temp * 1000000;
>>
>> loongson_sysconf.cpuname = (void *)dmi_string_parse(dm,
>> dmi_data[16]);
>> - loongson_sysconf.cores_per_package = *(dmi_data +
>> SMBIOS_THREAD_PACKAGE_OFFSET);
>> + thread_count = *(u8 *)(dmi_data + SMBIOS_THREAD_PACKAGE_OFFSET);
>> + if (thread_count == 0xFF && dm->length >= 0x30)
>> + thread_count = *(u16 *)(dmi_data +
>> SMBIOS_THREAD_PACKAGE_2_OFFSET);
>> + loongson_sysconf.cores_per_package = thread_count;
>>
>> pr_info("CpuClock = %llu\n", cpu_clock_freq);
>> }
>>> Huacai
>>>
>>>> pr_info("CpuClock = %llu\n", cpu_clock_freq);
>>>> }
>>>> --
>>>> 2.20.1
>>>>
Powered by blists - more mailing lists