| Message ID | 8592af5e3b95b197231445beb8c3123948ced15a.1663025154.git.pawan.kumar.gupta@linux.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Check enumeration before MSR save/restore | expand |
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?
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 --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; }
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(-)