[PATCH] x86/cpu: Update cached HLE state on write to TSX_CTRL_CPUID_CLEAR

[PATCH] x86/cpu: Update cached HLE state on write to TSX_CTRL_CPUID_CLEAR

[Pawan Gupta]

/proc/cpuinfo currently reports Hardware Lock Elision (HLE) feature to
be present on boot cpu even if it was disabled during the bootup. This
is because cpuinfo_x86->x86_capability HLE bit is not updated after TSX
state is changed via a new MSR IA32_TSX_CTRL.

Update the cached HLE bit also since it is expected to change after an
update to CPUID_CLEAR bit in MSR IA32_TSX_CTRL.

Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
Tested-by: Neelima Krishnan <neelima.krishnan@intel.com>
Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
---
 arch/x86/kernel/cpu/tsx.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/cpu/tsx.c b/arch/x86/kernel/cpu/tsx.c
index 3e20d322bc98..032509adf9de 100644
--- a/arch/x86/kernel/cpu/tsx.c
+++ b/arch/x86/kernel/cpu/tsx.c
@@ -115,11 +115,12 @@ void __init tsx_init(void)
 		tsx_disable();
 
 		/*
-		 * tsx_disable() will change the state of the
-		 * RTM CPUID bit.  Clear it here since it is now
-		 * expected to be not set.
+		 * tsx_disable() will change the state of the RTM and HLE CPUID
+		 * bits. Clear them here since they are now expected to be not
+		 * set.
 		 */
 		setup_clear_cpu_cap(X86_FEATURE_RTM);
+		setup_clear_cpu_cap(X86_FEATURE_HLE);
 	} else if (tsx_ctrl_state == TSX_CTRL_ENABLE) {
 
 		/*
@@ -131,10 +132,10 @@ void __init tsx_init(void)
 		tsx_enable();
 
 		/*
-		 * tsx_enable() will change the state of the
-		 * RTM CPUID bit.  Force it here since it is now
-		 * expected to be set.
+		 * tsx_enable() will change the state of the RTM and HLE CPUID
+		 * bits. Force them here since they are now expected to be set.
 		 */
 		setup_force_cpu_cap(X86_FEATURE_RTM);
+		setup_force_cpu_cap(X86_FEATURE_HLE);
 	}
 }
-- 
2.21.1

Re: [PATCH] x86/cpu: Update cached HLE state on write to TSX_CTRL_CPUID_CLEAR

[Josh Poimboeuf]

On Fri, Jan 10, 2020 at 02:50:54PM -0800, Pawan Gupta wrote:
> /proc/cpuinfo currently reports Hardware Lock Elision (HLE) feature to
> be present on boot cpu even if it was disabled during the bootup. This
> is because cpuinfo_x86->x86_capability HLE bit is not updated after TSX
> state is changed via a new MSR IA32_TSX_CTRL.
> 
> Update the cached HLE bit also since it is expected to change after an
> update to CPUID_CLEAR bit in MSR IA32_TSX_CTRL.
> 
> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> Tested-by: Neelima Krishnan <neelima.krishnan@intel.com>
> Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>

From the Intel TAA deep dive page [1], it says:

  "On processors that enumerate IA32_ARCH_CAPABILITIES[TSX_CTRL] (bit
   7)=1, HLE prefix hints are always ignored."

So if the CPU has IA32_TSX_CTRL, HLE is implicitly disabled, so why
would the HLE bit have been set in CPUID in the first place?

[1] https://software.intel.com/security-software-guidance/insights/deep-dive-intel-transactional-synchronization-extensions-intel-tsx-asynchronous-abort

-- 
Josh

Re: [PATCH] x86/cpu: Update cached HLE state on write to TSX_CTRL_CPUID_CLEAR

[Luck, Tony]

On Wed, Jan 15, 2020 at 03:15:13PM -0600, Josh Poimboeuf wrote:
> From the Intel TAA deep dive page [1], it says:
> 
>   "On processors that enumerate IA32_ARCH_CAPABILITIES[TSX_CTRL] (bit
>    7)=1, HLE prefix hints are always ignored."
> 
> So if the CPU has IA32_TSX_CTRL, HLE is implicitly disabled, so why
> would the HLE bit have been set in CPUID in the first place?
> 
> [1] https://software.intel.com/security-software-guidance/insights/deep-dive-intel-transactional-synchronization-extensions-intel-tsx-asynchronous-abort

IIRC some VMM folks asked to not make gratuitous to CPUID feature
enumeration because it complicates setting up pools of systems.

-Tony

Re: [PATCH] x86/cpu: Update cached HLE state on write to TSX_CTRL_CPUID_CLEAR

[Andi Kleen]

On Wed, Jan 15, 2020 at 03:15:13PM -0600, Josh Poimboeuf wrote:
> On Fri, Jan 10, 2020 at 02:50:54PM -0800, Pawan Gupta wrote:
> > /proc/cpuinfo currently reports Hardware Lock Elision (HLE) feature to
> > be present on boot cpu even if it was disabled during the bootup. This
> > is because cpuinfo_x86->x86_capability HLE bit is not updated after TSX
> > state is changed via a new MSR IA32_TSX_CTRL.
> > 
> > Update the cached HLE bit also since it is expected to change after an
> > update to CPUID_CLEAR bit in MSR IA32_TSX_CTRL.
> > 
> > Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> > Tested-by: Neelima Krishnan <neelima.krishnan@intel.com>
> > Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
> 
> From the Intel TAA deep dive page [1], it says:
> 
>   "On processors that enumerate IA32_ARCH_CAPABILITIES[TSX_CTRL] (bit
>    7)=1, HLE prefix hints are always ignored."
> 
> So if the CPU has IA32_TSX_CTRL, HLE is implicitly disabled, so why
> would the HLE bit have been set in CPUID in the first place?

The CPUID is unchanged to avoid problems with software that checks 
for unchanged CPUID. Unfortunately that exists in the wild.

-Andi

Re: [PATCH] x86/cpu: Update cached HLE state on write to TSX_CTRL_CPUID_CLEAR

[Josh Poimboeuf]

On Fri, Jan 10, 2020 at 02:50:54PM -0800, Pawan Gupta wrote:
> /proc/cpuinfo currently reports Hardware Lock Elision (HLE) feature to
> be present on boot cpu even if it was disabled during the bootup. This
> is because cpuinfo_x86->x86_capability HLE bit is not updated after TSX
> state is changed via a new MSR IA32_TSX_CTRL.
> 
> Update the cached HLE bit also since it is expected to change after an
> update to CPUID_CLEAR bit in MSR IA32_TSX_CTRL.
> 
> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> Tested-by: Neelima Krishnan <neelima.krishnan@intel.com>
> Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>

Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>

-- 
Josh