linux-kernel - Re: [PATCH v11 3/4] arm64: topology: Support SMT control on ACPI based system

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <86e32fb3-0ff5-f4f0-3d44-222e63b5a69f@huawei.com>
Date: Wed, 5 Mar 2025 17:01:34 +0800
From: Yicong Yang <yangyicong@...wei.com>
To: Pierre Gondois <pierre.gondois@....com>, Sudeep Holla
	<sudeep.holla@....com>, <dietmar.eggemann@....com>
CC: <yangyicong@...ilicon.com>, <catalin.marinas@....com>, <will@...nel.org>,
	<tglx@...utronix.de>, <peterz@...radead.org>, <mpe@...erman.id.au>,
	<linux-arm-kernel@...ts.infradead.org>, <mingo@...hat.com>, <bp@...en8.de>,
	<dave.hansen@...ux.intel.com>, <linuxppc-dev@...ts.ozlabs.org>,
	<x86@...nel.org>, <linux-kernel@...r.kernel.org>, <morten.rasmussen@....com>,
	<msuchanek@...e.de>, <gregkh@...uxfoundation.org>, <rafael@...nel.org>,
	<jonathan.cameron@...wei.com>, <prime.zeng@...ilicon.com>,
	<linuxarm@...wei.com>, <xuwei5@...wei.com>, <guohanjun@...wei.com>,
	<sshegde@...ux.ibm.com>
Subject: Re: [PATCH v11 3/4] arm64: topology: Support SMT control on ACPI
 based system

On 2025/3/4 23:07, Pierre Gondois wrote:
> 
> 
> On 3/4/25 11:02, Sudeep Holla wrote:
>> On Tue, Mar 04, 2025 at 09:25:02AM +0100, Pierre Gondois wrote:
>>>
>>>
>>> On 3/3/25 15:40, Yicong Yang wrote:
>>>> On 2025/3/3 19:16, Sudeep Holla wrote:
>>>>> On Mon, Mar 03, 2025 at 10:56:12AM +0100, Pierre Gondois wrote:
>>>>>> On 2/28/25 20:06, Sudeep Holla wrote:
>>>>>>>>>
>>>>>>>>> Ditto as previous patch, can get rid if it is default 1.
>>>>>>>>>
>>>>>>>>
>>>>>>>> On non-SMT platforms, not calling cpu_smt_set_num_threads() leaves
>>>>>>>> cpu_smt_num_threads uninitialized to UINT_MAX:
>>>>>>>>
>>>>>>>> smt/active:0
>>>>>>>> smt/control:-1
>>>>>>>>
>>>>>>>> If cpu_smt_set_num_threads() is called:
>>>>>>>> active:0
>>>>>>>> control:notsupported
>>>>>>>>
>>>>>>>> So it might be slightly better to still initialize max_smt_thread_num.
>>>>>>>>
>>>>>>>
>>>>>>> Sure, what I meant is to have max_smt_thread_num set to 1 by default is
>>>>>>> that is what needed anyways and the above code does that now.
>>>>>>>
>>>>>>> Why not start with initialised to 1 instead ?
>>>>>>> Of course some current logic needs to change around testing it for zero.
>>>>>>>
>>>>>>
>>>>>> I think there would still be a way to check against the default value.
>>>>>> If we have:
>>>>>> unsigned int max_smt_thread_num = 1;
>>>>>>
>>>>>> then on a platform with 2 threads, the detection condition would trigger:
>>>>>> xa_for_each(&hetero_cpu, hetero_id, entry) {
>>>>>>       if (entry->thread_num != max_smt_thread_num && max_smt_thread_num)     <---- (entry->thread_num=2) and (max_smt_thread_num=1)
>>>>>>           pr_warn_once("Heterogeneous SMT topology is partly
>>>>>>                         supported by SMT control\n");
>>>>>>
>>>>>> so we would need an additional variable:
>>>>>> bool is_initialized = false;
>>>>>
>>>>> Sure, we could do that or skip the check if max_smt_thread_num == 1 ?
>>>>>
>>>>> I mean
>>>>>     if (entry->thread_num != max_smt_thread_num && max_smt_thread_num != 1)
>>>>>
>>>
>>> I think it will be problematic if we parse:
>>> - first a CPU with 1 thread
>>> - then a CPU with 2 threads
>>>
>>> in that case we should detect the 'Heterogeneous SMT topology',
>>> but we cannot because we don't know whether max_smt_thread_num=1
>>> because 1 is the default value or we found a CPU with one thread.
>>
>> Right, but as per Dietmar's and my previous response, it may be a valid
>> case. See latest response from Dietmar which is explicitly requesting
>> support for this. It may need some special handling if we decide to support
>> that.
> 
> Ah ok, right indeed.
> For heterogeneous SMT platforms, the 'smt/control' file is able to accept
> on/off/forceoff strings. But providing the max #count of threads as an integer would
> be wrong if the CPU doesn't have this #count of threads.
> 
> Initially the idea was to just warn that support might be needed for heterogeneous
> SMT platforms, and let whoever would have such platform solve this case, but just
> disabling the integer interface in this case would solve the issue generically.
> 

ok so let's regard the asymmetric platform as a valid case as suggested (also mentioned
by Dietmar on another thread) and remove the check here. Will update and test.

Thanks.