[patch V3 00/12] SMT control knobs

[patch V3 00/12] SMT control knobs

[Thomas Gleixner]

V3 of the SMT control knob series. It addresses the review feed back of
V2. No functional changes.

Thanks,

	tglx

[patch V3 01/12] sched/smt: Update sched_smt_present at runtime

[Thomas Gleixner]

Subject: [patch V3 01/12] sched/smt: Update sched_smt_present at runtime
From: Peter Zijlstra <peterz@infradead.org>

The static key sched_smt_present is only updated at boot time when SMT
siblings have been deteced. Booting with maxcpus=1 and bringing the
siblings online after boot rebuilds the scheduling domains correctly but
does not update the static key, so the SMT code is not enabled.

Let the key update in the scheduler CPU hotplug code to fix this.

Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
V2: Clarified comment

 kernel/sched/core.c |   30 ++++++++++++------------------
 kernel/sched/fair.c |    1 +
 2 files changed, 13 insertions(+), 18 deletions(-)

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5773,6 +5773,18 @@ int sched_cpu_activate(unsigned int cpu)
 	struct rq *rq = cpu_rq(cpu);
 	struct rq_flags rf;
 
+#ifdef CONFIG_SCHED_SMT
+	/*
+	 * The sched_smt_present static key needs to be evaluated on every
+	 * hotplug event because at boot time SMT might be disabled when
+	 * the number of booted CPUs is limited.
+	 *
+	 * If then later a sibling gets hotplugged, then the key would stay
+	 * off and SMT scheduling would never be functional.
+	 */
+	if (cpumask_weight(cpu_smt_mask(cpu)) > 1)
+		static_branch_enable_cpuslocked(&sched_smt_present);
+#endif
 	set_cpu_active(cpu, true);
 
 	if (sched_smp_initialized) {
@@ -5870,22 +5882,6 @@ int sched_cpu_dying(unsigned int cpu)
 }
 #endif
 
-#ifdef CONFIG_SCHED_SMT
-DEFINE_STATIC_KEY_FALSE(sched_smt_present);
-
-static void sched_init_smt(void)
-{
-	/*
-	 * We've enumerated all CPUs and will assume that if any CPU
-	 * has SMT siblings, CPU0 will too.
-	 */
-	if (cpumask_weight(cpu_smt_mask(0)) > 1)
-		static_branch_enable(&sched_smt_present);
-}
-#else
-static inline void sched_init_smt(void) { }
-#endif
-
 void __init sched_init_smp(void)
 {
 	sched_init_numa();
@@ -5907,8 +5903,6 @@ void __init sched_init_smp(void)
 	init_sched_rt_class();
 	init_sched_dl_class();
 
-	sched_init_smt();
-
 	sched_smp_initialized = true;
 }
 
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6238,6 +6238,7 @@ static inline int find_idlest_cpu(struct
 }
 
 #ifdef CONFIG_SCHED_SMT
+DEFINE_STATIC_KEY_FALSE(sched_smt_present);
 
 static inline void set_idle_cores(int cpu, int val)
 {

[patch V3 02/12] x86/smp: Provide topology_is_primary_thread()

[Thomas Gleixner]

Subject: [patch V3 02/12] x86/smp: Provide topology_is_primary_thread()
From: Thomas Gleixner <tglx@linutronix.de>

If the CPU is supporting SMT then the primary thread can be found by
checking the lower APIC ID bits for zero. smp_num_siblings is used to build
the mask for the APIC ID bits which need to be taken into account.

This uses the MPTABLE or ACPI/MADT supplied APIC ID, which can be different
than the initial APIC ID in CPUID. But according to AMD the lower bits have
to be consistent. Intel gave a tentative confirmation as well.

Preparatory patch to support disabling SMT at boot/runtime.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/include/asm/apic.h     |    6 ++++++
 arch/x86/include/asm/topology.h |    4 +++-
 arch/x86/kernel/apic/apic.c     |   15 +++++++++++++++
 arch/x86/kernel/smpboot.c       |    9 +++++++++
 4 files changed, 33 insertions(+), 1 deletion(-)

--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -502,6 +502,12 @@ extern int default_check_phys_apicid_pre
 
 #endif /* CONFIG_X86_LOCAL_APIC */
 
+#ifdef CONFIG_SMP
+bool apic_id_is_primary_thread(unsigned int id);
+#else
+static inline bool apic_id_is_primary_thread(unsigned int id) { return false; }
+#endif
+
 extern void irq_enter(void);
 extern void irq_exit(void);
 
--- a/arch/x86/include/asm/topology.h
+++ b/arch/x86/include/asm/topology.h
@@ -123,13 +123,15 @@ static inline int topology_max_smt_threa
 }
 
 int topology_update_package_map(unsigned int apicid, unsigned int cpu);
-extern int topology_phys_to_logical_pkg(unsigned int pkg);
+int topology_phys_to_logical_pkg(unsigned int pkg);
+bool topology_is_primary_thread(unsigned int cpu);
 #else
 #define topology_max_packages()			(1)
 static inline int
 topology_update_package_map(unsigned int apicid, unsigned int cpu) { return 0; }
 static inline int topology_phys_to_logical_pkg(unsigned int pkg) { return 0; }
 static inline int topology_max_smt_threads(void) { return 1; }
+static inline bool topology_is_primary_thread(unsigned int cpu) { return true; ]
 #endif
 
 static inline void arch_fix_phys_package_id(int num, u32 slot)
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2189,6 +2189,21 @@ static int cpuid_to_apicid[] = {
 	[0 ... NR_CPUS - 1] = -1,
 };
 
+/**
+ * apic_id_is_primary_thread - Check whether APIC ID belongs to a primary thread
+ * @id:	APIC ID to check
+ */
+bool apic_id_is_primary_thread(unsigned int apicid)
+{
+	u32 mask;
+
+	if (smp_num_siblings == 1)
+		return true;
+	/* Isolate the SMT bit(s) in the APICID and check for 0 */
+	mask = (1U << (fls(smp_num_siblings) - 1)) - 1;
+	return !(apicid & mask);
+}
+
 /*
  * Should use this API to allocate logical CPU IDs to keep nr_logical_cpuids
  * and cpuid_to_apicid[] synchronized.
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -266,6 +266,15 @@ static void notrace start_secondary(void
 }
 
 /**
+ * topology_is_primary_thread - Check whether CPU is the primary SMT thread
+ * @cpu:	CPU to check
+ */
+bool topology_is_primary_thread(unsigned int cpu)
+{
+	return apic_id_is_primary_thread(per_cpu(x86_cpu_to_apicid, cpu));
+}
+
+/**
  * topology_phys_to_logical_pkg - Map a physical package id to a logical
  *
  * Returns logical package id or -1 if not found

[patch V3 03/12] cpu/hotplug: Make bringup/teardown of smp threads symmetric

[Thomas Gleixner]

Subject: [patch V3 03/12] cpu/hotplug: Make bringup/teardown of smp threads symmetric
From: Thomas Gleixner <tglx@linutronix.de>

The asymmetry caused a warning to trigger if the bootup was stopped in state
CPUHP_AP_ONLINE_IDLE. The warning no longer triggers as kthread_park() can
now be invoked on already or still parked threads. But there is still no
reason to have this be asymmetric.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/cpu.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -754,7 +754,6 @@ static int takedown_cpu(unsigned int cpu
 
 	/* Park the smpboot threads */
 	kthread_park(per_cpu_ptr(&cpuhp_state, cpu)->thread);
-	smpboot_park_threads(cpu);
 
 	/*
 	 * Prevent irq alloc/free while the dying cpu reorganizes the
@@ -1332,7 +1331,7 @@ static struct cpuhp_step cpuhp_hp_states
 	[CPUHP_AP_SMPBOOT_THREADS] = {
 		.name			= "smpboot/threads:online",
 		.startup.single		= smpboot_unpark_threads,
-		.teardown.single	= NULL,
+		.teardown.single	= smpboot_park_threads,
 	},
 	[CPUHP_AP_IRQ_AFFINITY_ONLINE] = {
 		.name			= "irq/affinity:online",

[patch V3 04/12] cpu/hotplug: Split do_cpu_down()

[Thomas Gleixner]

Subject: [patch V3 04/12] cpu/hotplug: Split do_cpu_down()
From: Thomas Gleixner <tglx@linutronix.de>

Split out the inner workings of do_cpu_down() to allow reuse of that
function for the upcoming SMT disabling mechanism.

No functional change.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 kernel/cpu.c |   17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -906,20 +906,19 @@ static int __ref _cpu_down(unsigned int
 	return ret;
 }
 
+static int cpu_down_maps_locked(unsigned int cpu, enum cpuhp_state target)
+{
+	if (cpu_hotplug_disabled)
+		return -EBUSY;
+	return _cpu_down(cpu, 0, target);
+}
+
 static int do_cpu_down(unsigned int cpu, enum cpuhp_state target)
 {
 	int err;
 
 	cpu_maps_update_begin();
-
-	if (cpu_hotplug_disabled) {
-		err = -EBUSY;
-		goto out;
-	}
-
-	err = _cpu_down(cpu, 0, target);
-
-out:
+	err = cpu_down_maps_locked(cpu, target);
 	cpu_maps_update_done();
 	return err;
 }

[patch V3 05/12] cpu/hotplug: Provide knobs to control SMT

[Thomas Gleixner]

Subject: [patch V3 05/12] cpu/hotplug: Provide knobs to control SMT
From: Thomas Gleixner <tglx@linutronix.de>

Provide a command line and a sysfs knob to control SMT.

The command line options are:

 'nosmt':	Enumerate secondary threads, but do not online them
 		
 'nosmt=force': Ignore secondary threads completely during enumeration
 		via MP table and ACPI/MADT.

The sysfs control file has the following states (read/write):

 'on':		SMT is enabled. Secondary threads can be freely onlined
 'off':		SMT is disabled. Secondary threads, even if enumerated
 		cannot be onlined
 'forceoff':	SMT is permanentely disabled. Writes to the control
 		file are rejected.

The command line option 'nosmt' sets the sysfs control to 'off'. This
can be changed to 'on' to reenable SMT during runtime.

The command line option 'nosmt=force' sets the sysfs control to
'forceoff'. This cannot be changed during runtime.

When SMT is 'on' and the control file is changed to 'off' then all online
secondary threads are offlined and attempts to online a secondary thread
later are rejected.

When SMT is 'off' and the control file is changed to 'on' then secondary
threads can be onlined again. The 'off' -> 'on' transition does not
automatically online the secondary threads.

When the control file is set to 'forceoff', the behaviour is the same as
setting it to 'off', but the operation is irreversible and later writes to
the control file are rejected.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
V2: Rewrote changelog. Make the enum named and use it for the state string
    array. Rename the evaluation function to cpu_smt_allowed().

 Documentation/ABI/testing/sysfs-devices-system-cpu |   16 ++
 Documentation/admin-guide/kernel-parameters.txt    |    8 +
 arch/Kconfig                                       |    3 
 arch/x86/Kconfig                                   |    1 
 include/linux/cpu.h                                |   12 +
 kernel/cpu.c                                       |  141 +++++++++++++++++++++
 6 files changed, 181 insertions(+)

--- a/Documentation/ABI/testing/sysfs-devices-system-cpu
+++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
@@ -490,3 +490,19 @@ Description:	Information about CPU vulne
 		"Not affected"	  CPU is not affected by the vulnerability
 		"Vulnerable"	  CPU is affected and no mitigation in effect
 		"Mitigation: $M"  CPU is affected and mitigation $M is in effect
+
+What:		/sys/devices/system/cpu/smt
+		/sys/devices/system/cpu/smt/active
+		/sys/devices/system/cpu/smt/control
+Date:		June 2018
+Contact:	Linux kernel mailing list <linux-kernel@vger.kernel.org>
+Description:	Control Symetric Multi Threading (SMT)
+
+		active:  Tells whether SMT is active (enabled and available)
+
+		control: Read/write interface to control SMT. Possible
+			 values:
+
+			 "on"		SMT is enabled
+			 "off"		SMT is disabled
+			 "forceoff"	SMT is force disabled. Cannot be changed.
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2687,6 +2687,14 @@
 	nosmt		[KNL,S390] Disable symmetric multithreading (SMT).
 			Equivalent to smt=1.
 
+			[KNL,x86] Disable symmetric multithreading (SMT).
+			nosmt=force: Force disable SMT, similar to disabling
+				     it in the BIOS except that some of the
+				     resource partitioning effects which are
+				     caused by having SMT enabled in the BIOS
+				     cannot be undone. Depending on the CPU
+				     type this might have a performance impact.
+
 	nospectre_v2	[X86] Disable all mitigations for the Spectre variant 2
 			(indirect branch prediction) vulnerability. System may
 			allow data leaks with this option, which is equivalent
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -13,6 +13,9 @@ config KEXEC_CORE
 config HAVE_IMA_KEXEC
 	bool
 
+config HOTPLUG_SMT
+	bool
+
 config OPROFILE
 	tristate "OProfile system profiling"
 	depends on PROFILING
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -187,6 +187,7 @@ config X86
 	select HAVE_SYSCALL_TRACEPOINTS
 	select HAVE_UNSTABLE_SCHED_CLOCK
 	select HAVE_USER_RETURN_NOTIFIER
+	select HOTPLUG_SMT			if SMP
 	select IRQ_FORCED_THREADING
 	select NEED_SG_DMA_LENGTH
 	select PCI_LOCKLESS_CONFIG
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -166,4 +166,16 @@ void cpuhp_report_idle_dead(void);
 static inline void cpuhp_report_idle_dead(void) { }
 #endif /* #ifdef CONFIG_HOTPLUG_CPU */
 
+enum cpuhp_smt_control {
+	CPU_SMT_ENABLED,
+	CPU_SMT_DISABLED,
+	CPU_SMT_FORCE_DISABLED,
+};
+
+#if defined(CONFIG_SMP) && defined(CONFIG_HOTPLUG_SMT)
+extern enum cpuhp_smt_control cpu_smt_control;
+#else
+# define cpu_smt_control		(CPU_SMT_ENABLED)
+#endif
+
 #endif /* _LINUX_CPU_H_ */
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -933,6 +933,29 @@ EXPORT_SYMBOL(cpu_down);
 #define takedown_cpu		NULL
 #endif /*CONFIG_HOTPLUG_CPU*/
 
+#ifdef CONFIG_HOTPLUG_SMT
+enum cpuhp_smt_control cpu_smt_control __read_mostly = CPU_SMT_ENABLED;
+
+static int __init smt_cmdline_disable(char *str)
+{
+	cpu_smt_control = CPU_SMT_DISABLED;
+	if (str && !strcmp(str, "force")) {
+		pr_info("SMT: Force disabled\n");
+		cpu_smt_control = CPU_SMT_FORCE_DISABLED;
+	}
+	return 0;
+}
+early_param("nosmt", smt_cmdline_disable);
+
+static inline bool cpu_smt_allowed(unsigned int cpu)
+{
+	return cpu_smt_control == CPU_SMT_ENABLED ||
+		topology_is_primary_thread(cpu);
+}
+#else
+static inline bool cpu_smt_allowed(unsigned int cpu) { return true; }
+#endif
+
 /**
  * notify_cpu_starting(cpu) - Invoke the callbacks on the starting CPU
  * @cpu: cpu that just started
@@ -1056,6 +1079,10 @@ static int do_cpu_up(unsigned int cpu, e
 		err = -EBUSY;
 		goto out;
 	}
+	if (!cpu_smt_allowed(cpu)) {
+		err = -EPERM;
+		goto out;
+	}
 
 	err = _cpu_up(cpu, 0, target);
 out:
@@ -1904,10 +1931,124 @@ static const struct attribute_group cpuh
 	NULL
 };
 
+#ifdef CONFIG_HOTPLUG_SMT
+
+static const char *smt_states[] = {
+	[CPU_SMT_ENABLED]		= "on",
+	[CPU_SMT_DISABLED]		= "off",
+	[CPU_SMT_FORCE_DISABLED]	= "forceoff",
+};
+
+static ssize_t
+show_smt_control(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE - 2, "%s\n", smt_states[cpu_smt_control]);
+}
+
+static int cpuhp_smt_disable(enum cpuhp_smt_control ctrlval)
+{
+	int cpu, ret = 0;
+
+	cpu_maps_update_begin();
+	for_each_online_cpu(cpu) {
+		if (topology_is_primary_thread(cpu))
+			continue;
+		ret = cpu_down_maps_locked(cpu, CPUHP_OFFLINE);
+		if (ret)
+			break;
+	}
+	if (!ret)
+		cpu_smt_control = ctrlval;
+	cpu_maps_update_done();
+	return ret;
+}
+
+static int cpuhp_smt_enable(void)
+{
+	cpu_maps_update_begin();
+	cpu_smt_control = CPU_SMT_ENABLED;
+	cpu_maps_update_done();
+	return 0;
+}
+
+static ssize_t
+store_smt_control(struct device *dev, struct device_attribute *attr,
+		  const char *buf, size_t count)
+{
+	int ctrlval, ret;
+
+	if (sysfs_streq(buf, "on"))
+		ctrlval = CPU_SMT_ENABLED;
+	else if (sysfs_streq(buf, "off"))
+		ctrlval = CPU_SMT_DISABLED;
+	else if (sysfs_streq(buf, "forceoff"))
+		ctrlval = CPU_SMT_FORCE_DISABLED;
+	else
+		return -EINVAL;
+
+	if (cpu_smt_control == CPU_SMT_FORCE_DISABLED)
+		return -EPERM;
+
+	ret = lock_device_hotplug_sysfs();
+	if (ret)
+		return ret;
+
+	if (ctrlval != cpu_smt_control) {
+		switch (ctrlval) {
+		case CPU_SMT_ENABLED:
+			ret = cpuhp_smt_enable();
+			break;
+		case CPU_SMT_DISABLED:
+		case CPU_SMT_FORCE_DISABLED:
+			ret = cpuhp_smt_disable(ctrlval);
+			break;
+		}
+	}
+
+	unlock_device_hotplug();
+	return ret ? ret : count;
+}
+static DEVICE_ATTR(control, 0644, show_smt_control, store_smt_control);
+
+static ssize_t
+show_smt_active(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	bool active = topology_max_smt_threads() > 1;
+
+	return snprintf(buf, PAGE_SIZE - 2, "%d\n", active);
+}
+static DEVICE_ATTR(active, 0444, show_smt_active, NULL);
+
+static struct attribute *cpuhp_smt_attrs[] = {
+	&dev_attr_control.attr,
+	&dev_attr_active.attr,
+	NULL
+};
+
+static const struct attribute_group cpuhp_smt_attr_group = {
+	.attrs = cpuhp_smt_attrs,
+	.name = "smt",
+	NULL
+};
+
+static int __init cpu_smt_state_init(void)
+{
+	return sysfs_create_group(&cpu_subsys.dev_root->kobj,
+				  &cpuhp_smt_attr_group);
+}
+
+#else
+static inline int cpu_smt_state_init(void) { return 0; }
+#endif
+
 static int __init cpuhp_sysfs_init(void)
 {
 	int cpu, ret;
 
+	ret = cpu_smt_state_init();
+	if (ret)
+		return ret;
+
 	ret = sysfs_create_group(&cpu_subsys.dev_root->kobj,
 				 &cpuhp_cpu_root_attr_group);
 	if (ret)

[patch V3 06/12] x86/cpu: Remove the pointless CPU printout

[Thomas Gleixner]

Subject: [patch V3 06/12] x86/cpu: Remove the pointless CPU printout
From: Thomas Gleixner <tglx@linutronix.de>

The value of this printout is dubious at best and there is no point in
having it in two different places along with convoluted ways to reach it.

Remove it completely.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/kernel/cpu/common.c   |   20 +++++---------------
 arch/x86/kernel/cpu/topology.c |   10 ----------
 2 files changed, 5 insertions(+), 25 deletions(-)

--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -663,13 +663,12 @@ void detect_ht(struct cpuinfo_x86 *c)
 #ifdef CONFIG_SMP
 	u32 eax, ebx, ecx, edx;
 	int index_msb, core_bits;
-	static bool printed;
 
 	if (!cpu_has(c, X86_FEATURE_HT))
 		return;
 
 	if (cpu_has(c, X86_FEATURE_CMP_LEGACY))
-		goto out;
+		return;
 
 	if (cpu_has(c, X86_FEATURE_XTOPOLOGY))
 		return;
@@ -678,14 +677,14 @@ void detect_ht(struct cpuinfo_x86 *c)
 
 	smp_num_siblings = (ebx & 0xff0000) >> 16;
 
+	if (!smp_num_siblings)
+		smp_num_siblings = 1;
+
 	if (smp_num_siblings == 1) {
 		pr_info_once("CPU0: Hyper-Threading is disabled\n");
-		goto out;
+		return;
 	}
 
-	if (smp_num_siblings <= 1)
-		goto out;
-
 	index_msb = get_count_order(smp_num_siblings);
 	c->phys_proc_id = apic->phys_pkg_id(c->initial_apicid, index_msb);
 
@@ -697,15 +696,6 @@ void detect_ht(struct cpuinfo_x86 *c)
 
 	c->cpu_core_id = apic->phys_pkg_id(c->initial_apicid, index_msb) &
 				       ((1 << core_bits) - 1);
-
-out:
-	if (!printed && (c->x86_max_cores * smp_num_siblings) > 1) {
-		pr_info("CPU: Physical Processor ID: %d\n",
-			c->phys_proc_id);
-		pr_info("CPU: Processor Core ID: %d\n",
-			c->cpu_core_id);
-		printed = 1;
-	}
 #endif
 }
 
--- a/arch/x86/kernel/cpu/topology.c
+++ b/arch/x86/kernel/cpu/topology.c
@@ -33,7 +33,6 @@ int detect_extended_topology(struct cpui
 	unsigned int eax, ebx, ecx, edx, sub_index;
 	unsigned int ht_mask_width, core_plus_mask_width;
 	unsigned int core_select_mask, core_level_siblings;
-	static bool printed;
 
 	if (c->cpuid_level < 0xb)
 		return -1;
@@ -86,15 +85,6 @@ int detect_extended_topology(struct cpui
 	c->apicid = apic->phys_pkg_id(c->initial_apicid, 0);
 
 	c->x86_max_cores = (core_level_siblings / smp_num_siblings);
-
-	if (!printed) {
-		pr_info("CPU: Physical Processor ID: %d\n",
-		       c->phys_proc_id);
-		if (c->x86_max_cores > 1)
-			pr_info("CPU: Processor Core ID: %d\n",
-			       c->cpu_core_id);
-		printed = 1;
-	}
 #endif
 	return 0;
 }

[patch V3 07/12] x86/cpu/AMD: Remove the pointless detect_ht() call

[Thomas Gleixner]

Subject: [patch V3 07/12] x86/cpu/AMD: Remove the pointless detect_ht() call
From: Thomas Gleixner <tglx@linutronix.de>

Real 32bit AMD CPUs do not have SMT and the only value of the call was to
reach the magic printout which got removed.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/kernel/cpu/amd.c |    4 ----
 1 file changed, 4 deletions(-)

--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -855,10 +855,6 @@ static void init_amd(struct cpuinfo_x86
 		srat_detect_node(c);
 	}
 
-#ifdef CONFIG_X86_32
-	detect_ht(c);
-#endif
-
 	init_amd_cacheinfo(c);
 
 	if (c->x86 >= 0xf)

[patch V3 08/12] x86/cpu/common: Provide detect_ht_early()

[Thomas Gleixner]

Subject: [patch V3 08/12] x86/cpu/common: Provide detect_ht_early()
From: Thomas Gleixner <tglx@linutronix.de>

To support force disabling of SMT it's required to know the number of
thread siblings early. detect_ht() cannot be called before the APIC driver
is selected, so split out the part which initializes smp_num_siblings.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/kernel/cpu/common.c |   24 ++++++++++++++----------
 arch/x86/kernel/cpu/cpu.h    |    1 +
 2 files changed, 15 insertions(+), 10 deletions(-)

--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -658,32 +658,36 @@ static void cpu_detect_tlb(struct cpuinf
 		tlb_lld_4m[ENTRIES], tlb_lld_1g[ENTRIES]);
 }
 
-void detect_ht(struct cpuinfo_x86 *c)
+int detect_ht_early(struct cpuinfo_x86 *c)
 {
 #ifdef CONFIG_SMP
 	u32 eax, ebx, ecx, edx;
-	int index_msb, core_bits;
 
 	if (!cpu_has(c, X86_FEATURE_HT))
-		return;
+		return -1;
 
 	if (cpu_has(c, X86_FEATURE_CMP_LEGACY))
-		return;
+		return -1;
 
 	if (cpu_has(c, X86_FEATURE_XTOPOLOGY))
-		return;
+		return -1;
 
 	cpuid(1, &eax, &ebx, &ecx, &edx);
 
 	smp_num_siblings = (ebx & 0xff0000) >> 16;
+	if (smp_num_siblings == 1)
+		pr_info_once("CPU0: Hyper-Threading is disabled\n");
+#endif
+	return 0;
+}
 
-	if (!smp_num_siblings)
-		smp_num_siblings = 1;
+void detect_ht(struct cpuinfo_x86 *c)
+{
+#ifdef CONFIG_SMP
+	int index_msb, core_bits;
 
-	if (smp_num_siblings == 1) {
-		pr_info_once("CPU0: Hyper-Threading is disabled\n");
+	if (detect_ht_early(c) < 0)
 		return;
-	}
 
 	index_msb = get_count_order(smp_num_siblings);
 	c->phys_proc_id = apic->phys_pkg_id(c->initial_apicid, index_msb);
--- a/arch/x86/kernel/cpu/cpu.h
+++ b/arch/x86/kernel/cpu/cpu.h
@@ -56,6 +56,7 @@ extern void init_amd_cacheinfo(struct cp
 
 extern void detect_num_cpu_cores(struct cpuinfo_x86 *c);
 extern int detect_extended_topology(struct cpuinfo_x86 *c);
+extern int detect_ht_early(struct cpuinfo_x86 *c);
 extern void detect_ht(struct cpuinfo_x86 *c);
 
 unsigned int aperfmperf_get_khz(int cpu);

[patch V3 09/12] x86/cpu/topology: Provide detect_extended_topology_early()

[Thomas Gleixner]

Subject: [patch V3 09/12] x86/cpu/topology: Provide detect_extended_topology_early()
From: Thomas Gleixner <tglx@linutronix.de>

To support force disabling of SMT it's required to know the number of
thread siblings early. detect_extended_topology() cannot be called before
the APIC driver is selected, so split out the part which initializes
smp_num_siblings.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/kernel/cpu/cpu.h      |    1 +
 arch/x86/kernel/cpu/topology.c |   31 ++++++++++++++++++++++---------
 2 files changed, 23 insertions(+), 9 deletions(-)

--- a/arch/x86/kernel/cpu/cpu.h
+++ b/arch/x86/kernel/cpu/cpu.h
@@ -55,6 +55,7 @@ extern void init_intel_cacheinfo(struct
 extern void init_amd_cacheinfo(struct cpuinfo_x86 *c);
 
 extern void detect_num_cpu_cores(struct cpuinfo_x86 *c);
+extern int detect_extended_topology_early(struct cpuinfo_x86 *c);
 extern int detect_extended_topology(struct cpuinfo_x86 *c);
 extern int detect_ht_early(struct cpuinfo_x86 *c);
 extern void detect_ht(struct cpuinfo_x86 *c);
--- a/arch/x86/kernel/cpu/topology.c
+++ b/arch/x86/kernel/cpu/topology.c
@@ -22,17 +22,10 @@
 #define BITS_SHIFT_NEXT_LEVEL(eax)	((eax) & 0x1f)
 #define LEVEL_MAX_SIBLINGS(ebx)		((ebx) & 0xffff)
 
-/*
- * Check for extended topology enumeration cpuid leaf 0xb and if it
- * exists, use it for populating initial_apicid and cpu topology
- * detection.
- */
-int detect_extended_topology(struct cpuinfo_x86 *c)
+int detect_extended_topology_early(struct cpuinfo_x86 *c)
 {
 #ifdef CONFIG_SMP
-	unsigned int eax, ebx, ecx, edx, sub_index;
-	unsigned int ht_mask_width, core_plus_mask_width;
-	unsigned int core_select_mask, core_level_siblings;
+	unsigned int eax, ebx, ecx, edx;
 
 	if (c->cpuid_level < 0xb)
 		return -1;
@@ -51,10 +44,30 @@ int detect_extended_topology(struct cpui
 	 * initial apic id, which also represents 32-bit extended x2apic id.
 	 */
 	c->initial_apicid = edx;
+	smp_num_siblings = LEVEL_MAX_SIBLINGS(ebx);
+#endif
+	return 0;
+}
+
+/*
+ * Check for extended topology enumeration cpuid leaf 0xb and if it
+ * exists, use it for populating initial_apicid and cpu topology
+ * detection.
+ */
+int detect_extended_topology(struct cpuinfo_x86 *c)
+{
+#ifdef CONFIG_SMP
+	unsigned int eax, ebx, ecx, edx, sub_index;
+	unsigned int ht_mask_width, core_plus_mask_width;
+	unsigned int core_select_mask, core_level_siblings;
+
+	if (detect_extended_topology_early(c) < 0)
+		return -1;
 
 	/*
 	 * Populate HT related information from sub-leaf level 0.
 	 */
+	cpuid_count(0xb, SMT_LEVEL, &eax, &ebx, &ecx, &edx);
 	core_level_siblings = smp_num_siblings = LEVEL_MAX_SIBLINGS(ebx);
 	core_plus_mask_width = ht_mask_width = BITS_SHIFT_NEXT_LEVEL(eax);
 

[patch V3 10/12] x86/cpu/intel: Evaluate smp_num_siblings early

[Thomas Gleixner]

Subject: [patch V3 10/12] x86/cpu/intel: Evaluate smp_num_siblings early
From: Thomas Gleixner <tglx@linutronix.de>

Make use of the new early detection function to initialize smp_num_siblings
on the boot cpu before the MP-Table or ACPI/MADT scan happens. That's
required for force disabling SMT.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
V2: Drop the bogus 32bit conditional

 arch/x86/kernel/cpu/intel.c |    7 +++++++
 1 file changed, 7 insertions(+)

--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -301,6 +301,13 @@ static void early_init_intel(struct cpui
 	}
 
 	check_mpx_erratum(c);
+
+	/*
+	 * Get the number of SMT siblings early from the extended topology
+	 * leaf, if available. Otherwise try the legacy SMT detection.
+	 */
+	if (detect_extended_topology_early(c) < 0)
+		detect_ht_early(c);
 }
 
 #ifdef CONFIG_X86_32

[patch V3 11/12] x86/cpu/AMD: Evaluate smp_num_siblings early

[Thomas Gleixner]

Subject: [patch V3 11/12] x86/cpu/AMD: Evaluate smp_num_siblings early
From: Thomas Gleixner <tglx@linutronix.de>

To support force disabling of SMT it's required to know the number of
thread siblings early. amd_get_topology() cannot be called before the APIC
driver is selected, so split out the part which initializes
smp_num_siblings and invoke it from amd_early_init().

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/kernel/cpu/amd.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -315,6 +315,18 @@ static void legacy_fixup_core_id(struct
 	c->cpu_core_id %= cus_per_node;
 }
 
+
+static void amd_get_topology_early(struct cpuinfo_x86 *c)
+{
+	if (boot_cpu_has(X86_FEATURE_TOPOEXT) &&
+	    c->extended_cpuid_level >= 0x80000008) {
+		u32 eax, ebx, ecx, edx;
+
+		cpuid(0x8000001e, &eax, &ebx, &ecx, &edx);
+		smp_num_siblings = ((ebx >> 8) & 0xff) + 1;
+	}
+}
+
 /*
  * Fixup core topology information for
  * (1) AMD multi-node processors
@@ -681,6 +693,8 @@ static void early_init_amd(struct cpuinf
 		set_cpu_bug(c, X86_BUG_AMD_E400);
 
 	early_detect_mem_encrypt(c);
+
+	amd_get_topology_early(c);
 }
 
 static void init_amd_k8(struct cpuinfo_x86 *c)

[patch V3 12/12] x86/apic: Ignore secondary threads if nosmt=force

[Thomas Gleixner]

Subject: [patch V3 12/12] x86/apic: Ignore secondary threads if nosmt=force
From: Thomas Gleixner <tglx@linutronix.de>

nosmt on the kernel command line merily prevents the onlining of the
secondary SMT siblings.

nosmt=force makes the APIC detection code ignore the secondary SMT siblings
completely, so they even do not show up as possible CPUs.

This is not fully equivalent to disabling SMT in the BIOS because the low
level SMT enabling in the BIOS can result in partitioning of resources
between the siblings, which is not undone by just ignoring them. Some CPUs
can use the full resources when their sibling is not onlined, but this is
depending on the CPU family and model and it's not well documented whether
this applies to all partitioned resources. That means depending on the
workload disabling SMT in the BIOS might result in better performance.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
V2: Moved the mask related hunk to the proper patch and clarified that
    the command line switch might not be completely equivalent to the
    BIOS switch.

 arch/x86/include/asm/apic.h |    2 ++
 arch/x86/kernel/acpi/boot.c |    3 ++-
 arch/x86/kernel/apic/apic.c |   19 +++++++++++++++++++
 3 files changed, 23 insertions(+), 1 deletion(-)

--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -504,8 +504,10 @@ extern int default_check_phys_apicid_pre
 
 #ifdef CONFIG_SMP
 bool apic_id_is_primary_thread(unsigned int id);
+bool apic_id_disabled(unsigned int id);
 #else
 static inline bool apic_id_is_primary_thread(unsigned int id) { return false; }
+static inline bool apic_id_disabled(unsigned int id) { return false; }
 #endif
 
 extern void irq_enter(void);
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -181,7 +181,8 @@ static int acpi_register_lapic(int id, u
 	}
 
 	if (!enabled) {
-		++disabled_cpus;
+		if (!apic_id_disabled(id))
+			++disabled_cpus;
 		return -EINVAL;
 	}
 
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2204,6 +2204,16 @@ bool apic_id_is_primary_thread(unsigned
 	return !(apicid & mask);
 }
 
+/**
+ * apic_id_disabled - Check whether APIC ID is disabled via SMT control
+ * @id:	APIC ID to check
+ */
+bool apic_id_disabled(unsigned int id)
+{
+	return (cpu_smt_control == CPU_SMT_FORCE_DISABLED &&
+		!apic_id_is_primary_thread(id));
+}
+
 /*
  * Should use this API to allocate logical CPU IDs to keep nr_logical_cpuids
  * and cpuid_to_apicid[] synchronized.
@@ -2299,6 +2309,15 @@ int generic_processor_info(int apicid, i
 		return -EINVAL;
 	}
 
+	/*
+	 * If SMT is force disabled and the APIC ID belongs to
+	 * a secondary thread, ignore it.
+	 */
+	if (apic_id_disabled(apicid)) {
+		pr_info_once("Ignoring secondary SMT threads\n");
+		return -EINVAL;
+	}
+
 	if (apicid == boot_cpu_physical_apicid) {
 		/*
 		 * x86_bios_cpu_apicid is required to have processors listed

[MODERATED] Re: [patch V3 11/12] x86/cpu/AMD: Evaluate smp_num_siblings early

[Andrew Cooper]

On 15/06/18 17:28, speck for Thomas Gleixner wrote:
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -315,6 +315,18 @@ static void legacy_fixup_core_id(struct
>  	c->cpu_core_id %= cus_per_node;
>  }
>  
> +
> +static void amd_get_topology_early(struct cpuinfo_x86 *c)
> +{
> +	if (boot_cpu_has(X86_FEATURE_TOPOEXT) &&
> +	    c->extended_cpuid_level >= 0x80000008) {

Presumably 0x8000001e as that is the leaf you actually use?

~Andrew

> +		u32 eax, ebx, ecx, edx;
> +
> +		cpuid(0x8000001e, &eax, &ebx, &ecx, &edx);
> +		smp_num_siblings = ((ebx >> 8) & 0xff) + 1;
> +	}
> +}
> +
>  /*
>   * Fixup core topology information for
>   * (1) AMD multi-node processors
>

Re: [patch V3 11/12] x86/cpu/AMD: Evaluate smp_num_siblings early

[Thomas Gleixner]

On Fri, 15 Jun 2018, speck for Andrew Cooper wrote:

> On 15/06/18 17:28, speck for Thomas Gleixner wrote:
> > --- a/arch/x86/kernel/cpu/amd.c
> > +++ b/arch/x86/kernel/cpu/amd.c
> > @@ -315,6 +315,18 @@ static void legacy_fixup_core_id(struct
> >  	c->cpu_core_id %= cus_per_node;
> >  }
> >  
> > +
> > +static void amd_get_topology_early(struct cpuinfo_x86 *c)
> > +{
> > +	if (boot_cpu_has(X86_FEATURE_TOPOEXT) &&
> > +	    c->extended_cpuid_level >= 0x80000008) {
> 
> Presumably 0x8000001e as that is the leaf you actually use?

Duh, That's what you get for blind copy and paste. That obviously want's to
be fixed in the existing code first. Boris?

Thanks,

	tglx

[MODERATED] Re: Re: [patch V3 11/12] x86/cpu/AMD: Evaluate smp_num_siblings early

[Borislav Petkov]

On Fri, Jun 15, 2018 at 07:30:49PM +0200, speck for Thomas Gleixner wrote:
> Duh, That's what you get for blind copy and paste. That obviously want's to
> be fixed in the existing code first. Boris?

There's an innacuracy in the docs, lemme clarify that first and will get
back to you.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

[MODERATED] Re: [patch V3 01/12] sched/smt: Update sched_smt_present at runtime

[Konrad Rzeszutek Wilk]

On Fri, Jun 15, 2018 at 06:28:02PM +0200, speck for Thomas Gleixner wrote:
> Subject: [patch V3 01/12] sched/smt: Update sched_smt_present at runtime
> From: Peter Zijlstra <peterz@infradead.org>
> 
> The static key sched_smt_present is only updated at boot time when SMT
> siblings have been deteced. Booting with maxcpus=1 and bringing the

I got some _really_ (hah) important reviews in case you are respinning or
applying them now:

s/deteced/detected/

> siblings online after boot rebuilds the scheduling domains correctly but
> does not update the static key, so the SMT code is not enabled.
> 
> Let the key update in the scheduler CPU hotplug code to fix this.

s/key update/key be updated/

[MODERATED] Re: Re: [patch V3 11/12] x86/cpu/AMD: Evaluate smp_num_siblings early

[Borislav Petkov]

On Fri, Jun 15, 2018 at 07:47:27PM +0200, Borislav Petkov wrote:
> On Fri, Jun 15, 2018 at 07:30:49PM +0200, speck for Thomas Gleixner wrote:
> > Duh, That's what you get for blind copy and paste. That obviously want's to
> > be fixed in the existing code first. Boris?
> 
> There's an innacuracy in the docs, lemme clarify that first and will get
> back to you.

So I *think* that you can access leaf 0x8000001e when X86_FEATURE_TOPOEXT is
present but lemme confirm that first.

Also, we can do the following (I'd need to audit it again and test it
but from staring at it so far, this is old remnant where it used to read
out CPUID_Fn80000008_ECX[NC] and those functions don't do that now -
early_init_amd_mc() does.)

---
From: Borislav Petkov <bp@suse.de>
Date: Fri, 15 Jun 2018 20:48:39 +0200
Subject: [PATCH] x86/CPU/AMD: Do not check CPUID max ext level before parsing
 SMP info

Old code used to check whether CPUID ext max level is >= 0x80000008
because that last leaf contains the number of cores of the physical CPU.
The three functions called there now do not depend on that leaf anymore
so the check can go.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/cpu/amd.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 082d7875cef8..9493e865588a 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -848,12 +848,9 @@ static void init_amd(struct cpuinfo_x86 *c)
 
 	cpu_detect_cache_sizes(c);
 
-	/* Multi core CPU? */
-	if (c->extended_cpuid_level >= 0x80000008) {
-		amd_detect_cmp(c);
-		amd_get_topology(c);
-		srat_detect_node(c);
-	}
+	amd_detect_cmp(c);
+	amd_get_topology(c);
+	srat_detect_node(c);
 
 #ifdef CONFIG_X86_32
 	detect_ht(c);
-- 
2.17.0.582.gccdcbd54c

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

[MODERATED] Re: [patch V3 03/12] cpu/hotplug: Make bringup/teardown of smp threads symmetric

[Konrad Rzeszutek Wilk]

On Fri, Jun 15, 2018 at 06:28:04PM +0200, speck for Thomas Gleixner wrote:
> Subject: [patch V3 03/12] cpu/hotplug: Make bringup/teardown of smp threads symmetric
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> The asymmetry caused a warning to trigger if the bootup was stopped in state
> CPUHP_AP_ONLINE_IDLE. The warning no longer triggers as kthread_park() can
> now be invoked on already or still parked threads. But there is still no
> reason to have this be asymmetric.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Thank you!
> ---
>  kernel/cpu.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -754,7 +754,6 @@ static int takedown_cpu(unsigned int cpu
>  
>  	/* Park the smpboot threads */
>  	kthread_park(per_cpu_ptr(&cpuhp_state, cpu)->thread);
> -	smpboot_park_threads(cpu);
>  
>  	/*
>  	 * Prevent irq alloc/free while the dying cpu reorganizes the
> @@ -1332,7 +1331,7 @@ static struct cpuhp_step cpuhp_hp_states
>  	[CPUHP_AP_SMPBOOT_THREADS] = {
>  		.name			= "smpboot/threads:online",
>  		.startup.single		= smpboot_unpark_threads,
> -		.teardown.single	= NULL,
> +		.teardown.single	= smpboot_park_threads,
>  	},
>  	[CPUHP_AP_IRQ_AFFINITY_ONLINE] = {
>  		.name			= "irq/affinity:online",
> 

[MODERATED] Re: [patch V3 05/12] cpu/hotplug: Provide knobs to control SMT

[Konrad Rzeszutek Wilk]

On Fri, Jun 15, 2018 at 06:28:06PM +0200, speck for Thomas Gleixner wrote:
> Subject: [patch V3 05/12] cpu/hotplug: Provide knobs to control SMT
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> Provide a command line and a sysfs knob to control SMT.
> 
> The command line options are:
> 
>  'nosmt':	Enumerate secondary threads, but do not online them
>  		
>  'nosmt=force': Ignore secondary threads completely during enumeration
>  		via MP table and ACPI/MADT.
> 
> The sysfs control file has the following states (read/write):
> 
>  'on':		SMT is enabled. Secondary threads can be freely onlined
>  'off':		SMT is disabled. Secondary threads, even if enumerated
>  		cannot be onlined
>  'forceoff':	SMT is permanentely disabled. Writes to the control
>  		file are rejected.
> 
> The command line option 'nosmt' sets the sysfs control to 'off'. This
> can be changed to 'on' to reenable SMT during runtime.
> 
> The command line option 'nosmt=force' sets the sysfs control to
> 'forceoff'. This cannot be changed during runtime.
> 
> When SMT is 'on' and the control file is changed to 'off' then all online
> secondary threads are offlined and attempts to online a secondary thread
> later are rejected.

s/later are/later on are/

With this change or not:

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Thank you!

[MODERATED] Re: [patch V3 10/12] x86/cpu/intel: Evaluate smp_num_siblings early

[Konrad Rzeszutek Wilk]

On Fri, Jun 15, 2018 at 06:28:11PM +0200, speck for Thomas Gleixner wrote:
> Subject: [patch V3 10/12] x86/cpu/intel: Evaluate smp_num_siblings early
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> Make use of the new early detection function to initialize smp_num_siblings
> on the boot cpu before the MP-Table or ACPI/MADT scan happens. That's
> required for force disabling SMT.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Thank you!
> ---
> V2: Drop the bogus 32bit conditional
> 
>  arch/x86/kernel/cpu/intel.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -301,6 +301,13 @@ static void early_init_intel(struct cpui
>  	}
>  
>  	check_mpx_erratum(c);
> +
> +	/*
> +	 * Get the number of SMT siblings early from the extended topology
> +	 * leaf, if available. Otherwise try the legacy SMT detection.
> +	 */
> +	if (detect_extended_topology_early(c) < 0)
> +		detect_ht_early(c);
>  }
>  
>  #ifdef CONFIG_X86_32
> 

[MODERATED] Re: [patch V3 12/12] x86/apic: Ignore secondary threads if nosmt=force

[Andi Kleen]

> nosmt=force makes the APIC detection code ignore the secondary SMT siblings
> completely, so they even do not show up as possible CPUs.

What's the point of this? Please describe a concrete use case
or drop.

Also it doesn't seem to describe what is actually implemented
by this patch.

> 
> This is not fully equivalent to disabling SMT in the BIOS because the low
> level SMT enabling in the BIOS can result in partitioning of resources
> between the siblings, which is not undone by just ignoring them. Some CPUs
> can use the full resources when their sibling is not onlined, but this is
> depending on the CPU family and model and it's not well documented whether
> this applies to all partitioned resources. That means depending on the
> workload disabling SMT in the BIOS might result in better performance.

This paragraph is quite confused and doesn't really match what
the actual micro architecture does. I would recommend to drop it.

-Andi

[MODERATED] Re: [patch V3 05/12] cpu/hotplug: Provide knobs to control SMT

[Borislav Petkov]

Just nitpicks:

On Fri, Jun 15, 2018 at 06:28:06PM +0200, speck for Thomas Gleixner wrote:
> Subject: [patch V3 05/12] cpu/hotplug: Provide knobs to control SMT
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> Provide a command line and a sysfs knob to control SMT.

...

> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -933,6 +933,29 @@ EXPORT_SYMBOL(cpu_down);
>  #define takedown_cpu		NULL
>  #endif /*CONFIG_HOTPLUG_CPU*/
>  
> +#ifdef CONFIG_HOTPLUG_SMT
> +enum cpuhp_smt_control cpu_smt_control __read_mostly = CPU_SMT_ENABLED;
> +
> +static int __init smt_cmdline_disable(char *str)
> +{
> +	cpu_smt_control = CPU_SMT_DISABLED;
> +	if (str && !strcmp(str, "force")) {
> +		pr_info("SMT: Force disabled\n");

Maybe check if SMT is even supported on the system, if at all possible
that early, before even doing anything here...

> +		cpu_smt_control = CPU_SMT_FORCE_DISABLED;
> +	}
> +	return 0;
> +}
> +early_param("nosmt", smt_cmdline_disable);

...

> +static int cpuhp_smt_enable(void)
> +{
> +	cpu_maps_update_begin();
> +	cpu_smt_control = CPU_SMT_ENABLED;
> +	cpu_maps_update_done();
> +	return 0;
> +}

That can be void.

> +
> +static ssize_t
> +store_smt_control(struct device *dev, struct device_attribute *attr,
> +		  const char *buf, size_t count)
> +{
> +	int ctrlval, ret;
> +
> +	if (sysfs_streq(buf, "on"))
> +		ctrlval = CPU_SMT_ENABLED;
> +	else if (sysfs_streq(buf, "off"))
> +		ctrlval = CPU_SMT_DISABLED;
> +	else if (sysfs_streq(buf, "forceoff"))
> +		ctrlval = CPU_SMT_FORCE_DISABLED;
> +	else
> +		return -EINVAL;
> +
> +	if (cpu_smt_control == CPU_SMT_FORCE_DISABLED)
> +		return -EPERM;

You could do that as the first thing when entering this function.

> +
> +	ret = lock_device_hotplug_sysfs();
> +	if (ret)
> +		return ret;
> +
> +	if (ctrlval != cpu_smt_control) {
> +		switch (ctrlval) {
> +		case CPU_SMT_ENABLED:
> +			ret = cpuhp_smt_enable();
> +			break;
> +		case CPU_SMT_DISABLED:
> +		case CPU_SMT_FORCE_DISABLED:
> +			ret = cpuhp_smt_disable(ctrlval);
> +			break;
> +		}
> +	}
> +
> +	unlock_device_hotplug();
> +	return ret ? ret : count;
> +}
> +static DEVICE_ATTR(control, 0644, show_smt_control, store_smt_control);

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

[MODERATED] Re: Re: [patch V3 11/12] x86/cpu/AMD: Evaluate smp_num_siblings early

[Borislav Petkov]

On Fri, Jun 15, 2018 at 09:02:14PM +0200, Borislav Petkov wrote:
> On Fri, Jun 15, 2018 at 07:47:27PM +0200, Borislav Petkov wrote:
> > On Fri, Jun 15, 2018 at 07:30:49PM +0200, speck for Thomas Gleixner wrote:
> > > Duh, That's what you get for blind copy and paste. That obviously want's to
> > > be fixed in the existing code first. Boris?
> > 
> > There's an innacuracy in the docs, lemme clarify that first and will get
> > back to you.
> 
> So I *think* that you can access leaf 0x8000001e when X86_FEATURE_TOPOEXT is
> present but lemme confirm that first.

Yes, got confirmation. X86_FEATURE_TOPOEXT means CPUID 0x8000001e is present.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

[MODERATED] Re: [patch V3 12/12] x86/apic: Ignore secondary threads if nosmt=force

[Peter Zijlstra]

On Fri, Jun 15, 2018 at 03:22:14PM -0700, speck for Andi Kleen wrote:
> > nosmt=force makes the APIC detection code ignore the secondary SMT siblings
> > completely, so they even do not show up as possible CPUs.
> 
> What's the point of this? Please describe a concrete use case
> or drop.

It's pretty obvious -- if you never have smt enabled, using this saves
you on percpu resources. For example, percpu memory is allocated based
on possible_mask, so shrinking that by half reduces the amount of memory
used for per-cpu allocations by half as well -- which can be quite
significant.

> Also it doesn't seem to describe what is actually implemented
> by this patch.

It does; it stops siblings being accounted on @disabled_cpus and
possible_mask.

> > This is not fully equivalent to disabling SMT in the BIOS because the low
> > level SMT enabling in the BIOS can result in partitioning of resources
> > between the siblings, which is not undone by just ignoring them. Some CPUs
> > can use the full resources when their sibling is not onlined, but this is
> > depending on the CPU family and model and it's not well documented whether
> > this applies to all partitioned resources. That means depending on the
> > workload disabling SMT in the BIOS might result in better performance.
> 
> This paragraph is quite confused and doesn't really match what
> the actual micro architecture does. I would recommend to drop it.

This is based on text in the optimization manual, see Linus' email on
that last round, fully quoted below in case you have trouble finding it.

  Message-ID: <alpine.LFD.2.21.999.1806061245350.12980@i7.lan>

Basically the text seems to suggest core resources can be statically
partitioned if HT is enabled by the BIOS, irrespective of the OS
actually using the second thread. If this is not the case, the document
would need fixing, not only this Changelog.


---

On Wed, 6 Jun 2018, speck for Thomas Gleixner wrote:
>
> nosmt=force makes the APIC detection code ignore the secondary SMT siblings
> completely, so they even do not show up as possible CPUs. This is more or
> less equivalent to disabling SMT in the BIOS.

So if we go down this route, I do think it should be very clearly
documented that the BIOS disable can (and does) end up having resource
allocation advantages that we cannot do in software later on.

The intel optimization manual is not very clear on what the partitioning
rules are.

I find:

  "In general, the buffers for staging instructions between major pipe
   stages  are partitioned. These buffers include µop queues after the
   execution trace cache, the queues after the register rename stage, the
   reorder buffer which stages instructions for retirement, and the load
   and store buffers.

   In the case of load and store buffers, partitioning also provided an
   easier implementation to maintain memory ordering for each logical
   processor and detect memory ordering violations"

but some of that partitioning may be relaxed if the HT thread is "not
active":

  "In Intel microarchitecture code name Sandy Bridge, the micro-op queue
   is statically partitioned to provide 28 entries for each logical
   processor,  irrespective of software executing in single thread or
   multiple threads. If one logical processor is not active in Intel
   microarchitecture code name Ivy Bridge, then a single thread executing
   on that processor  core can use the 56 entries in the micro-op queue"

but I do not know what "not active" means, and how dynamic it is. Some of
that partitioning may be entirely static and depend on the early BIOS
disabling of HT, and even if we park the cores, the resources will just be
wasted.

From the above, it does sound like even if some of the instruction queues
may be relaxed when one thread is idle (if that is what "not active")
means, load and store buffers may be entirely statically partitioned.

So I would really wanmt to have a big note along with the

  "This is more or less equivalent to disabling SMT in the BIOS"

because on some loads it might be a case of "less equivalent" than not.

               Linus

[MODERATED] Re: [patch V3 00/12] SMT control knobs

[Jiri Kosina]

On Fri, 15 Jun 2018, speck for Thomas Gleixner wrote:

> V3 of the SMT control knob series. It addresses the review feed back of
> V2. No functional changes.

	https://www.phoronix.com/scan.php?page=news_item&px=OpenBSD-Disabling-SMT
	https://www.mail-archive.com/source-changes@openbsd.org/msg99141.html

It's not the full sotry yet, but somehow it feels like late-august will 
not be the final CRD.

-- 
Jiri Kosina
SUSE Labs

[MODERATED] Re: [patch V3 00/12] SMT control knobs

[Konrad Rzeszutek Wilk]

On Wed, Jun 20, 2018 at 07:03:30AM +0200, speck for Jiri Kosina wrote:
> On Fri, 15 Jun 2018, speck for Thomas Gleixner wrote:
> 
> > V3 of the SMT control knob series. It addresses the review feed back of
> > V2. No functional changes.
> 
> 	https://www.phoronix.com/scan.php?page=news_item&px=OpenBSD-Disabling-SMT
> 	https://www.mail-archive.com/source-changes@openbsd.org/msg99141.html
> 
> It's not the full sotry yet, but somehow it feels like late-august will 
> not be the final CRD.

Thomas, would you be OK sending your smt.v3?(v4?) git bundle? I am hoping to finish
the KVM parts this week to have something ready in case Intel decides that
the embargo broke early?

Thank you.

Re: [patch V3 00/12] SMT control knobs

[Thomas Gleixner]

On Wed, 20 Jun 2018, speck for Konrad Rzeszutek Wilk wrote:

> On Wed, Jun 20, 2018 at 07:03:30AM +0200, speck for Jiri Kosina wrote:
> > On Fri, 15 Jun 2018, speck for Thomas Gleixner wrote:
> > 
> > > V3 of the SMT control knob series. It addresses the review feed back of
> > > V2. No functional changes.
> > 
> > 	https://www.phoronix.com/scan.php?page=news_item&px=OpenBSD-Disabling-SMT
> > 	https://www.mail-archive.com/source-changes@openbsd.org/msg99141.html
> > 
> > It's not the full sotry yet, but somehow it feels like late-august will 
> > not be the final CRD.
> 
> Thomas, would you be OK sending your smt.v3?(v4?) git bundle? I am hoping to finish
> the KVM parts this week to have something ready in case Intel decides that
> the embargo broke early?

I'm at incorporating the latest feedback and fixing a nasty sysfs issue,
but I should be done later tonight.

Thanks,

	tglx