linux-kernel - Re: [PATCH 8/9] powerpc: Add HOTPLUG

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 1 Jun 2023 18:19:59 +0200
From:   Laurent Dufour <ldufour@...ux.ibm.com>
To:     Michael Ellerman <mpe@...erman.id.au>, linux-kernel@...r.kernel.org
Cc:     linux-arch@...r.kernel.org, x86@...nel.org,
        dave.hansen@...ux.intel.com, mingo@...hat.com, bp@...en8.de,
        tglx@...utronix.de, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH 8/9] powerpc: Add HOTPLUG_SMT support

On 01/06/2023 15:27:30, Laurent Dufour wrote:
> On 24/05/2023 17:56:29, Michael Ellerman wrote:
>> Add support for HOTPLUG_SMT, which enables the generic sysfs SMT support
>> files in /sys/devices/system/cpu/smt, as well as the "nosmt" boot
>> parameter.
> 
> Hi Michael,
> 
> It seems that there is now a conflict between with the PPC 'smt-enabled'
> boot option.
> 
> Booting the patched kernel with 'smt-enabled=4', later, change to the SMT
> level (for instance to 6) done through /sys/devices/system/cpu/smt/control
> are not applied. Nothing happens.
> Based on my early debug, I think the reasons is that cpu_smt_num_threads=8
> when entering __store_smt_control(). But I need to dig further.

I dug deeper.

FWIW, I think smt_enabled_at_boot should be passed to
cpu_smt_check_topology() in smp_prepare_cpus(), instead of
threads_per_core. But that's not enough to fix the issue because this value
is also used to set cpu_smt_max_threads.

To achieve that, cpu_smt_check_topology() should receive 2 parameters, the
current SMT level define at boot time, and the maximum SMT level.

The attached patch is fixing the issue on my ppc64 test LPAR.
This patch is not addressing the x86 architecture (I didn't get the time to
do it, but it should be doable) and should be spread among the patches 3
and 8 of your series.

Hope this helps.

Cheers,
Laurent.

> 
> BTW, should the 'smt-enabled' PPC specific option remain?
> 
> Cheers,
> Laurent.
> 
>> Implement the recently added hooks to allow partial SMT states, allow
>> any number of threads per core.
>>
>> Tie the config symbol to HOTPLUG_CPU, which enables it on the major
>> platforms that support SMT. If there are other platforms that want the
>> SMT support that can be tweaked in future.
>>
>> Signed-off-by: Michael Ellerman <mpe@...erman.id.au>
>> ---
>>  arch/powerpc/Kconfig                |  1 +
>>  arch/powerpc/include/asm/topology.h | 25 +++++++++++++++++++++++++
>>  arch/powerpc/kernel/smp.c           |  3 +++
>>  3 files changed, 29 insertions(+)
>>
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index 539d1f03ff42..5cf87ca10a9c 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -273,6 +273,7 @@ config PPC
>>  	select HAVE_SYSCALL_TRACEPOINTS
>>  	select HAVE_VIRT_CPU_ACCOUNTING
>>  	select HAVE_VIRT_CPU_ACCOUNTING_GEN
>> +	select HOTPLUG_SMT			if HOTPLUG_CPU
>>  	select HUGETLB_PAGE_SIZE_VARIABLE	if PPC_BOOK3S_64 && HUGETLB_PAGE
>>  	select IOMMU_HELPER			if PPC64
>>  	select IRQ_DOMAIN
>> diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
>> index 8a4d4f4d9749..1e9117a22d14 100644
>> --- a/arch/powerpc/include/asm/topology.h
>> +++ b/arch/powerpc/include/asm/topology.h
>> @@ -143,5 +143,30 @@ static inline int cpu_to_coregroup_id(int cpu)
>>  #endif
>>  #endif
>>  
>> +#ifdef CONFIG_HOTPLUG_SMT
>> +#include <linux/cpu_smt.h>
>> +#include <asm/cputhreads.h>
>> +
>> +static inline bool topology_smt_supported(void)
>> +{
>> +	return threads_per_core > 1;
>> +}
>> +
>> +static inline bool topology_smt_threads_supported(unsigned int num_threads)
>> +{
>> +	return num_threads <= threads_per_core;
>> +}
>> +
>> +static inline bool topology_is_primary_thread(unsigned int cpu)
>> +{
>> +	return cpu == cpu_first_thread_sibling(cpu);
>> +}
>> +
>> +static inline bool topology_smt_thread_allowed(unsigned int cpu)
>> +{
>> +	return cpu_thread_in_core(cpu) < cpu_smt_num_threads;
>> +}
>> +#endif
>> +
>>  #endif /* __KERNEL__ */
>>  #endif	/* _ASM_POWERPC_TOPOLOGY_H */
>> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
>> index 265801a3e94c..eed20b9253b7 100644
>> --- a/arch/powerpc/kernel/smp.c
>> +++ b/arch/powerpc/kernel/smp.c
>> @@ -1154,6 +1154,9 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
>>  
>>  	if (smp_ops && smp_ops->probe)
>>  		smp_ops->probe();
>> +
>> +	// Initalise the generic SMT topology support
>> +	cpu_smt_check_topology(threads_per_core);
>>  }
>>  
>>  void smp_prepare_boot_cpu(void)
> 

View attachment "0001-Consider-the-SMT-level-specify-at-boot-time.patch" of type "text/plain" (3074 bytes)