diff mbox series

[1/3] x86/tsx: Add feature bit for TSX control MSR support

Message ID 8592af5e3b95b197231445beb8c3123948ced15a.1663025154.git.pawan.kumar.gupta@linux.intel.com
State Superseded
Headers show
Series Check enumeration before MSR save/restore | expand

Commit Message

Pawan Gupta Sept. 12, 2022, 11:39 p.m. UTC
Support for TSX control MSR is enumerated in MSR_IA32_ARCH_CAPABILITIES.
This is different from how other CPU features are enumerated i.e. via
CPUID. Enumerating support for TSX control currently has an overhead of
reading the MSR every time which can be avoided.

Set a new feature bit X86_FEATURE_TSX_CTRL when MSR_IA32_TSX_CTRL is
present. Also make tsx_ctrl_is_supported() use feature bit instead of
reading the MSR.

This will also be useful for any code that wants to use the feature bit
instead of a calling tsx_ctrl_is_supported().

Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
---
 arch/x86/include/asm/cpufeatures.h |  1 +
 arch/x86/kernel/cpu/tsx.c          | 30 +++++++++++++++---------------
 2 files changed, 16 insertions(+), 15 deletions(-)

Comments

Dave Hansen Nov. 8, 2022, 6:27 p.m. UTC | #1
On 9/12/22 16:39, Pawan Gupta wrote:
> Support for TSX control MSR is enumerated in MSR_IA32_ARCH_CAPABILITIES.
> This is different from how other CPU features are enumerated i.e. via
> CPUID. Enumerating support for TSX control currently has an overhead of
> reading the MSR every time which can be avoided.

I only see tsx_ctrl_is_supported() getting called in three places:

> 1 tsx.c tsx_clear_cpuid       138 } else if (tsx_ctrl_is_supported()) {
> 2 tsx.c tsx_dev_mode_disable  161 if (!boot_cpu_has_bug(X86_BUG_TAA) || !tsx_ctrl_is_supported() ||
> 3 tsx.c tsx_init              194 if (!tsx_ctrl_is_supported()) {

Those all look like boot-time things to me.  Why does the overhead matter?
Pawan Gupta Nov. 8, 2022, 10:06 p.m. UTC | #2
On Tue, Nov 08, 2022 at 10:27:56AM -0800, Dave Hansen wrote:
>On 9/12/22 16:39, Pawan Gupta wrote:
>> Support for TSX control MSR is enumerated in MSR_IA32_ARCH_CAPABILITIES.
>> This is different from how other CPU features are enumerated i.e. via
>> CPUID. Enumerating support for TSX control currently has an overhead of
>> reading the MSR every time which can be avoided.
>
>I only see tsx_ctrl_is_supported() getting called in three places:
>
>> 1 tsx.c tsx_clear_cpuid       138 } else if (tsx_ctrl_is_supported()) {
>> 2 tsx.c tsx_dev_mode_disable  161 if (!boot_cpu_has_bug(X86_BUG_TAA) || !tsx_ctrl_is_supported() ||
>> 3 tsx.c tsx_init              194 if (!tsx_ctrl_is_supported()) {
>
>Those all look like boot-time things to me.

Except tsx_clear_cpuid() could be called during S3 resume as part of
secondary CPU's init, but still its not too often.

>Why does the overhead matter?

This patch is mainly needed for patch 3/3 that relies on feature bits to
decide which MSRs to save/restore during suspend/resume.

I just gave a hint about it in the commit message:

     This will also be useful for any code that wants to use the feature bit
     instead of a calling tsx_ctrl_is_supported().

I will fix the commit message with this as the main reason for adding
the feature bit.
diff mbox series

Patch

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index ef4775c6db01..dd173733e40d 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -304,6 +304,7 @@ 
 #define X86_FEATURE_UNRET		(11*32+15) /* "" AMD BTB untrain return */
 #define X86_FEATURE_USE_IBPB_FW		(11*32+16) /* "" Use IBPB during runtime firmware calls */
 #define X86_FEATURE_RSB_VMEXIT_LITE	(11*32+17) /* "" Fill RSB on VM exit when EIBRS is enabled */
+#define X86_FEATURE_MSR_TSX_CTRL	(11*32+18) /* "" MSR IA32_TSX_CTRL */
 
 /* Intel-defined CPU features, CPUID level 0x00000007:1 (EAX), word 12 */
 #define X86_FEATURE_AVX_VNNI		(12*32+ 4) /* AVX VNNI instructions */
diff --git a/arch/x86/kernel/cpu/tsx.c b/arch/x86/kernel/cpu/tsx.c
index ec7bbac3a9f2..9fe488dbed15 100644
--- a/arch/x86/kernel/cpu/tsx.c
+++ b/arch/x86/kernel/cpu/tsx.c
@@ -60,20 +60,7 @@  static void tsx_enable(void)
 
 static bool tsx_ctrl_is_supported(void)
 {
-	u64 ia32_cap = x86_read_arch_cap_msr();
-
-	/*
-	 * TSX is controlled via MSR_IA32_TSX_CTRL.  However, support for this
-	 * MSR is enumerated by ARCH_CAP_TSX_MSR bit in MSR_IA32_ARCH_CAPABILITIES.
-	 *
-	 * TSX control (aka MSR_IA32_TSX_CTRL) is only available after a
-	 * microcode update on CPUs that have their MSR_IA32_ARCH_CAPABILITIES
-	 * bit MDS_NO=1. CPUs with MDS_NO=0 are not planned to get
-	 * MSR_IA32_TSX_CTRL support even after a microcode update. Thus,
-	 * tsx= cmdline requests will do nothing on CPUs without
-	 * MSR_IA32_TSX_CTRL support.
-	 */
-	return !!(ia32_cap & ARCH_CAP_TSX_CTRL_MSR);
+	return cpu_feature_enabled(X86_FEATURE_MSR_TSX_CTRL);
 }
 
 static enum tsx_ctrl_states x86_get_tsx_auto_mode(void)
@@ -191,7 +178,20 @@  void __init tsx_init(void)
 		return;
 	}
 
-	if (!tsx_ctrl_is_supported()) {
+	/*
+	 * TSX is controlled via MSR_IA32_TSX_CTRL.  However, support for this
+	 * MSR is enumerated by ARCH_CAP_TSX_MSR bit in MSR_IA32_ARCH_CAPABILITIES.
+	 *
+	 * TSX control (aka MSR_IA32_TSX_CTRL) is only available after a
+	 * microcode update on CPUs that have their MSR_IA32_ARCH_CAPABILITIES
+	 * bit MDS_NO=1. CPUs with MDS_NO=0 are not planned to get
+	 * MSR_IA32_TSX_CTRL support even after a microcode update. Thus,
+	 * tsx= cmdline requests will do nothing on CPUs without
+	 * MSR_IA32_TSX_CTRL support.
+	 */
+	if (x86_read_arch_cap_msr() & ARCH_CAP_TSX_CTRL_MSR) {
+		setup_force_cpu_cap(X86_FEATURE_MSR_TSX_CTRL);
+	} else {
 		tsx_ctrl_state = TSX_CTRL_NOT_SUPPORTED;
 		return;
 	}