| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAAhV-H51t3FT6y8iYOU02FOoZ5dNsqwdpqjaWWwiQwhhuUbbYQ@mail.gmail.com>
Date: Thu, 27 Nov 2025 17:35:40 +0800
From: Huacai Chen <chenhuacai@...nel.org>
To: Qiang Ma <maqianga@...ontech.com>
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
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
want to add a comment, so I think the below is OK.
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