Re: [PATCH 8/9] powerpc: Add HOTPLUG_SMT support

From: Laurent Dufour
Date: Thu Jun 01 2023 - 12:20:43 EST

Next message: Sean Christopherson: "Re: [PATCH v2] KVM: allow KVM_BUG/KVM_BUG_ON to handle 64-bit cond"
Previous message: Dave Hansen: "Re: [PATCH 0/6] Memory Mapping (VMA) protection using PKU - set 1"
In reply to: Laurent Dufour: "Re: [PATCH 8/9] powerpc: Add HOTPLUG_SMT support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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@xxxxxxxxxxxxxx>
>> ---
>> 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)
>
From 682e7d78fb98d6298926e88e5093e2172488ea6f Mon Sep 17 00:00:00 2001
From: Laurent Dufour <ldufour@xxxxxxxxxxxxx>
Date: Thu, 1 Jun 2023 18:02:55 +0200
Subject: [PATCH] Consider the SMT level specify at boot time

This allows PPC kernel to boot with a SMT level different from 1 or threads
per core value.

Signed-off-by: Laurent Dufour <ldufour@xxxxxxxxxxxxx>
---
arch/powerpc/kernel/smp.c | 2 +-
include/linux/cpu_smt.h | 6 ++++--
kernel/cpu.c | 9 +++++++--
3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index eed20b9253b7..ec8ccf3d6294 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1156,7 +1156,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
smp_ops->probe();

// Initalise the generic SMT topology support
- cpu_smt_check_topology(threads_per_core);
+ cpu_smt_check_topology(smt_enabled_at_boot, threads_per_core);
}

void smp_prepare_boot_cpu(void)
diff --git a/include/linux/cpu_smt.h b/include/linux/cpu_smt.h
index 8d4ae26047c9..b5f13acb323f 100644
--- a/include/linux/cpu_smt.h
+++ b/include/linux/cpu_smt.h
@@ -14,7 +14,8 @@ enum cpuhp_smt_control {
extern enum cpuhp_smt_control cpu_smt_control;
extern unsigned int cpu_smt_num_threads;
extern void cpu_smt_disable(bool force);
-extern void cpu_smt_check_topology(unsigned int num_threads);
+extern void cpu_smt_check_topology(unsigned int num_threads,
+ unsigned int max_threads);
extern bool cpu_smt_possible(void);
extern int cpuhp_smt_enable(void);
extern int cpuhp_smt_disable(enum cpuhp_smt_control ctrlval);
@@ -22,7 +23,8 @@ extern int cpuhp_smt_disable(enum cpuhp_smt_control ctrlval);
# define cpu_smt_control (CPU_SMT_NOT_IMPLEMENTED)
# define cpu_smt_num_threads 1
static inline void cpu_smt_disable(bool force) { }
-static inline void cpu_smt_check_topology(unsigned int num_threads) { }
+static inline void cpu_smt_check_topology(unsigned int num_threads,
+ unsigned int max_threads) { }
static inline bool cpu_smt_possible(void) { return false; }
static inline int cpuhp_smt_enable(void) { return 0; }
static inline int cpuhp_smt_disable(enum cpuhp_smt_control ctrlval) { return 0; }
diff --git a/kernel/cpu.c b/kernel/cpu.c
index aca23c7b4547..268d386bd4e7 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -435,12 +435,17 @@ void __init cpu_smt_disable(bool force)
* The decision whether SMT is supported can only be done after the full
* CPU identification. Called from architecture code.
*/
-void __init cpu_smt_check_topology(unsigned int num_threads)
+void __init cpu_smt_check_topology(unsigned int num_threads,
+ unsigned int max_threads)
{
if (!topology_smt_supported())
cpu_smt_control = CPU_SMT_NOT_SUPPORTED;

- cpu_smt_max_threads = num_threads;
+ cpu_smt_max_threads = max_threads;
+
+ WARN_ON(num_threads > max_threads);
+ if (num_threads > max_threads)
+ num_threads = max_threads;

// May already be disabled by nosmt command line parameter
if (cpu_smt_control != CPU_SMT_ENABLED)
--
2.40.1

Next message: Sean Christopherson: "Re: [PATCH v2] KVM: allow KVM_BUG/KVM_BUG_ON to handle 64-bit cond"
Previous message: Dave Hansen: "Re: [PATCH 0/6] Memory Mapping (VMA) protection using PKU - set 1"
In reply to: Laurent Dufour: "Re: [PATCH 8/9] powerpc: Add HOTPLUG_SMT support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]