* [PATCH] x86/tsx: Cope with TSX deprecation on SKL/KBL/CFL/WHL
@ 2021-06-08 17:05 Andrew Cooper
2021-06-09 6:36 ` Jan Beulich
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Cooper @ 2021-06-08 17:05 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné, Wei Liu
The June 2021 microcode is formally de-featuring TSX on the older Skylake
client CPUs. The workaround from the March 2019 microcode is being dropped,
and replaced with additions to MSR_TSX_FORCE_ABORT to hide the HLE/RTM CPUID
bits.
With this microcode in place, TSX is disabled by default on these CPUs.
Backwards compatibility is provided in the same way as for TAA - RTM force
aborts, rather than suffering #UD, and the CPUID bits can be hidden to recover
performance.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
---
docs/misc/xen-command-line.pandoc | 13 +++++
tools/misc/xen-cpuid.c | 2 +-
xen/arch/x86/tsx.c | 76 +++++++++++++++++++++++++++--
xen/include/asm-x86/cpufeature.h | 1 +
xen/include/asm-x86/msr-index.h | 2 +
xen/include/public/arch-x86/cpufeatureset.h | 1 +
6 files changed, 89 insertions(+), 6 deletions(-)
diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index 1fae872626..3ece83a427 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -2309,6 +2309,12 @@ Several microcode updates are relevant:
and formally retiring HLE from the architecture. The user can disable TSX
to mitigate TAA, and elect to hide the HLE/RTM CPUID bits.
+ * June 2021, removing the workaround for March 2019 on client CPUs and
+ formally de-featured TSX on SKL/KBL/WHL/CFL (Note: SKX still retains the
+ March 2019 fix). Introduced the ability to hide the HLE/RTM CPUID bits.
+ PCR3 works fine, and TSX is disabled by default, but the user can re-enable
+ TSX at their own risk, accepting that the memory order erratum is unfixed.
+
On systems with the ability to configure TSX, this boolean offers system wide
control of whether TSX is enabled or disabled.
@@ -2326,6 +2332,13 @@ control of whether TSX is enabled or disabled.
ordering errata default to `true` to enable working TSX. Alternatively,
selecting `tsx=0` will disable TSX and restore PCR3 to a working state.
+ SKX and SKL/KBL/WHL/CFL on pre-June 2021 microcode default to `true`.
+ Alternatively, selecting `tsx=0` will disable TSX and restore PCR3 to a
+ working state.
+
+ SKL/KBL/WHL/CFL on the June 2021 microcode or later default to `false`.
+ Alternatively, selecting `tsx=1` will re-enable TSX at the users own risk.
+
### ucode
> `= List of [ <integer> | scan=<bool>, nmi=<bool>, allow-same=<bool> ]`
diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
index d4bc83d8c9..735bcf8f0e 100644
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -162,7 +162,7 @@ static const char *const str_7d0[32] =
[ 4] = "fsrm",
[ 8] = "avx512-vp2intersect", [ 9] = "srbds-ctrl",
- [10] = "md-clear",
+ [10] = "md-clear", [11] = "rtm-always-abort",
/* 12 */ [13] = "tsx-force-abort",
[14] = "serialize",
diff --git a/xen/arch/x86/tsx.c b/xen/arch/x86/tsx.c
index 338191df7f..085f1c82a7 100644
--- a/xen/arch/x86/tsx.c
+++ b/xen/arch/x86/tsx.c
@@ -60,6 +60,38 @@ void tsx_init(void)
*/
/*
+ * Probe for the June 2021 microcode which de-features TSX on
+ * client parts. (Note - this is a subset of parts impacted by
+ * the memory ordering errata.)
+ *
+ * RTM_ALWAYS_ABORT enumerates the new functionality, but is also
+ * read as zero if TSX_FORCE_ABORT.ENABLE_RTM has been set before
+ * we run.
+ *
+ * Undo this behaviour in Xen's view of the world.
+ */
+ bool has_rtm_always_abort = cpu_has_rtm_always_abort;
+
+ if ( !has_rtm_always_abort )
+ {
+ uint64_t val;
+
+ rdmsrl(MSR_TSX_FORCE_ABORT, val);
+
+ if ( val & TSX_ENABLE_RTM )
+ has_rtm_always_abort = true;
+ }
+
+ /*
+ * Always force RTM_ALWAYS_ABORT to be visibile, even if it
+ * currently is. If the user explicitly opts to enable TSX, we'll
+ * set TSX_FORCE_ABORT.ENABLE_RTM and hide RTM_ALWAYS_ABORT from
+ * the general CPUID scan later.
+ */
+ if ( has_rtm_always_abort )
+ setup_force_cpu_cap(X86_FEATURE_RTM_ALWAYS_ABORT);
+
+ /*
* If no explicit tsx= option is provided, pick a default.
*
* This deliberately overrides the implicit opt_tsx=-3 from
@@ -67,9 +99,16 @@ void tsx_init(void)
* - parse_spec_ctrl() ran before any CPU details where know.
* - We now know we're running on a CPU not affected by TAA (as
* TSX_FORCE_ABORT is enumerated).
+ * - When RTM_ALWAYS_ABORT is enumerated, TSX malfunctions, so we
+ * only ever want it enabled by explicit user choice.
+ *
+ * Without RTM_ALWAYS_ABORT, leave TSX active. In particular,
+ * this includes SKX where TSX is still supported.
+ *
+ * With RTM_ALWAYS_ABORT, disable TSX.
*/
if ( opt_tsx < 0 )
- opt_tsx = 1;
+ opt_tsx = !cpu_has_rtm_always_abort;
}
/*
@@ -90,7 +129,7 @@ void tsx_init(void)
* Force the features to be visible in Xen's view if we see any of the
* infrastructure capable of hiding them.
*/
- if ( cpu_has_tsx_ctrl )
+ if ( cpu_has_tsx_ctrl || cpu_has_tsx_force_abort )
{
setup_force_cpu_cap(X86_FEATURE_HLE);
setup_force_cpu_cap(X86_FEATURE_RTM);
@@ -131,9 +170,36 @@ void tsx_init(void)
/* Check bottom bit only. Higher bits are various sentinels. */
rtm_disabled = !(opt_tsx & 1);
- lo &= ~TSX_FORCE_ABORT_RTM;
- if ( rtm_disabled )
- lo |= TSX_FORCE_ABORT_RTM;
+ lo &= ~(TSX_FORCE_ABORT_RTM | TSX_CPUID_CLEAR | TSX_ENABLE_RTM);
+
+ if ( cpu_has_rtm_always_abort )
+ {
+ /*
+ * June 2021 microcode, on a client part with TSX de-featured:
+ * - There are no mitigations for the TSX memory ordering errata.
+ * - Performance counter 3 works. (I.e. it isn't being used by
+ * microcode to work around the memory ordering errata.)
+ * - TSX_FORCE_ABORT.FORCE_ABORT_RTM is fixed read1/write-discard.
+ * - TSX_FORCE_ABORT.TSX_CPUID_CLEAR can be used to hide the
+ * HLE/RTM CPUID bits.
+ * - TSX_FORCE_ABORT.ENABLE_RTM may be used to opt in to
+ * re-enabling RTM, at the users own risk.
+ */
+ lo |= rtm_disabled ? TSX_CPUID_CLEAR : TSX_ENABLE_RTM;
+ }
+ else
+ {
+ /*
+ * Either a server part where TSX isn't de-featured, or pre-June
+ * 2021 microcode:
+ * - By default, the TSX memory ordering errata is worked around
+ * in microcode at the cost of Performance Counter 3.
+ * - "Working TSX" vs "Working PCR3" can be selected by way of
+ * setting TSX_FORCE_ABORT.FORCE_ABORT_RTM.
+ */
+ if ( rtm_disabled )
+ lo |= TSX_FORCE_ABORT_RTM;
+ }
wrmsr(MSR_TSX_FORCE_ABORT, lo, hi);
}
diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h
index 9f5ae3aa0d..a539a4bacd 100644
--- a/xen/include/asm-x86/cpufeature.h
+++ b/xen/include/asm-x86/cpufeature.h
@@ -131,6 +131,7 @@
#define cpu_has_avx512_4vnniw boot_cpu_has(X86_FEATURE_AVX512_4VNNIW)
#define cpu_has_avx512_4fmaps boot_cpu_has(X86_FEATURE_AVX512_4FMAPS)
#define cpu_has_avx512_vp2intersect boot_cpu_has(X86_FEATURE_AVX512_VP2INTERSECT)
+#define cpu_has_rtm_always_abort boot_cpu_has(X86_FEATURE_RTM_ALWAYS_ABORT)
#define cpu_has_tsx_force_abort boot_cpu_has(X86_FEATURE_TSX_FORCE_ABORT)
#define cpu_has_serialize boot_cpu_has(X86_FEATURE_SERIALIZE)
#define cpu_has_arch_caps boot_cpu_has(X86_FEATURE_ARCH_CAPS)
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index bd3a3a1e7f..9a772c12b8 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -61,6 +61,8 @@
#define MSR_TSX_FORCE_ABORT 0x0000010f
#define TSX_FORCE_ABORT_RTM (_AC(1, ULL) << 0)
+#define TSX_CPUID_CLEAR (_AC(1, ULL) << 1)
+#define TSX_ENABLE_RTM (_AC(1, ULL) << 2)
#define MSR_TSX_CTRL 0x00000122
#define TSX_CTRL_RTM_DISABLE (_AC(1, ULL) << 0)
diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
index b65af42436..380b51b1b3 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -265,6 +265,7 @@ XEN_CPUFEATURE(FSRM, 9*32+ 4) /*A Fast Short REP MOVS */
XEN_CPUFEATURE(AVX512_VP2INTERSECT, 9*32+8) /*a VP2INTERSECT{D,Q} insns */
XEN_CPUFEATURE(SRBDS_CTRL, 9*32+ 9) /* MSR_MCU_OPT_CTRL and RNGDS_MITG_DIS. */
XEN_CPUFEATURE(MD_CLEAR, 9*32+10) /*A VERW clears microarchitectural buffers */
+XEN_CPUFEATURE(RTM_ALWAYS_ABORT, 9*32+11) /*! June 2021 TSX defeaturing in microcode. */
XEN_CPUFEATURE(TSX_FORCE_ABORT, 9*32+13) /* MSR_TSX_FORCE_ABORT.RTM_ABORT */
XEN_CPUFEATURE(SERIALIZE, 9*32+14) /*a SERIALIZE insn */
XEN_CPUFEATURE(CET_IBT, 9*32+20) /* CET - Indirect Branch Tracking */
--
2.11.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] x86/tsx: Cope with TSX deprecation on SKL/KBL/CFL/WHL
2021-06-08 17:05 [PATCH] x86/tsx: Cope with TSX deprecation on SKL/KBL/CFL/WHL Andrew Cooper
@ 2021-06-09 6:36 ` Jan Beulich
2021-06-09 12:19 ` Andrew Cooper
0 siblings, 1 reply; 3+ messages in thread
From: Jan Beulich @ 2021-06-09 6:36 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
On 08.06.2021 19:05, Andrew Cooper wrote:
> --- a/xen/arch/x86/tsx.c
> +++ b/xen/arch/x86/tsx.c
> @@ -60,6 +60,38 @@ void tsx_init(void)
> */
>
> /*
> + * Probe for the June 2021 microcode which de-features TSX on
> + * client parts. (Note - this is a subset of parts impacted by
> + * the memory ordering errata.)
> + *
> + * RTM_ALWAYS_ABORT enumerates the new functionality, but is also
> + * read as zero if TSX_FORCE_ABORT.ENABLE_RTM has been set before
> + * we run.
> + *
> + * Undo this behaviour in Xen's view of the world.
> + */
> + bool has_rtm_always_abort = cpu_has_rtm_always_abort;
> +
> + if ( !has_rtm_always_abort )
> + {
> + uint64_t val;
> +
> + rdmsrl(MSR_TSX_FORCE_ABORT, val);
> +
> + if ( val & TSX_ENABLE_RTM )
> + has_rtm_always_abort = true;
> + }
> +
> + /*
> + * Always force RTM_ALWAYS_ABORT to be visibile, even if it
> + * currently is. If the user explicitly opts to enable TSX, we'll
> + * set TSX_FORCE_ABORT.ENABLE_RTM and hide RTM_ALWAYS_ABORT from
> + * the general CPUID scan later.
> + */
> + if ( has_rtm_always_abort )
> + setup_force_cpu_cap(X86_FEATURE_RTM_ALWAYS_ABORT);
I understand the "we'll set" part, but I don't think "we'll hide"
anything explicitly. Aiui it is ...
> @@ -131,9 +170,36 @@ void tsx_init(void)
> /* Check bottom bit only. Higher bits are various sentinels. */
> rtm_disabled = !(opt_tsx & 1);
>
> - lo &= ~TSX_FORCE_ABORT_RTM;
> - if ( rtm_disabled )
> - lo |= TSX_FORCE_ABORT_RTM;
> + lo &= ~(TSX_FORCE_ABORT_RTM | TSX_CPUID_CLEAR | TSX_ENABLE_RTM);
> +
> + if ( cpu_has_rtm_always_abort )
> + {
> + /*
> + * June 2021 microcode, on a client part with TSX de-featured:
> + * - There are no mitigations for the TSX memory ordering errata.
> + * - Performance counter 3 works. (I.e. it isn't being used by
> + * microcode to work around the memory ordering errata.)
> + * - TSX_FORCE_ABORT.FORCE_ABORT_RTM is fixed read1/write-discard.
> + * - TSX_FORCE_ABORT.TSX_CPUID_CLEAR can be used to hide the
> + * HLE/RTM CPUID bits.
> + * - TSX_FORCE_ABORT.ENABLE_RTM may be used to opt in to
> + * re-enabling RTM, at the users own risk.
> + */
> + lo |= rtm_disabled ? TSX_CPUID_CLEAR : TSX_ENABLE_RTM;
... the setting of TSX_ENABLE_RTM here which, as a result, causes
RTM_ALWAYS_ABORT to be clear. If that's correct, perhaps the wording
in that earlier comment would better be something like "we'll set
TSX_FORCE_ABORT.ENABLE_RTM and hence cause RTM_ALWAYS_ABORT to be
hidden from the general CPUID scan later"?
If this understanding of mine is correct, then preferably with some
suitable adjustment to the comment wording
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Also Intel recommends this for SDVs only - can we consider such a
setup supported (not to speak of security supported) at all? I guess
you mean to express this by saying "at their own risk" in the
cmdline doc? If so, perhaps mentioning this in SUPPORT.md would be
a good thing nevertheless, notwithstanding the fact that we're not
really good at expressing there how command line option use affects
support status.
Jan
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] x86/tsx: Cope with TSX deprecation on SKL/KBL/CFL/WHL
2021-06-09 6:36 ` Jan Beulich
@ 2021-06-09 12:19 ` Andrew Cooper
0 siblings, 0 replies; 3+ messages in thread
From: Andrew Cooper @ 2021-06-09 12:19 UTC (permalink / raw)
To: Jan Beulich; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
On 09/06/2021 07:36, Jan Beulich wrote:
> On 08.06.2021 19:05, Andrew Cooper wrote:
>> --- a/xen/arch/x86/tsx.c
>> +++ b/xen/arch/x86/tsx.c
>> @@ -60,6 +60,38 @@ void tsx_init(void)
>> */
>>
>> /*
>> + * Probe for the June 2021 microcode which de-features TSX on
>> + * client parts. (Note - this is a subset of parts impacted by
>> + * the memory ordering errata.)
>> + *
>> + * RTM_ALWAYS_ABORT enumerates the new functionality, but is also
>> + * read as zero if TSX_FORCE_ABORT.ENABLE_RTM has been set before
>> + * we run.
>> + *
>> + * Undo this behaviour in Xen's view of the world.
>> + */
>> + bool has_rtm_always_abort = cpu_has_rtm_always_abort;
>> +
>> + if ( !has_rtm_always_abort )
>> + {
>> + uint64_t val;
>> +
>> + rdmsrl(MSR_TSX_FORCE_ABORT, val);
>> +
>> + if ( val & TSX_ENABLE_RTM )
>> + has_rtm_always_abort = true;
>> + }
>> +
>> + /*
>> + * Always force RTM_ALWAYS_ABORT to be visibile, even if it
>> + * currently is. If the user explicitly opts to enable TSX, we'll
>> + * set TSX_FORCE_ABORT.ENABLE_RTM and hide RTM_ALWAYS_ABORT from
>> + * the general CPUID scan later.
>> + */
>> + if ( has_rtm_always_abort )
>> + setup_force_cpu_cap(X86_FEATURE_RTM_ALWAYS_ABORT);
> I understand the "we'll set" part, but I don't think "we'll hide"
> anything explicitly. Aiui it is ...
>
>> @@ -131,9 +170,36 @@ void tsx_init(void)
>> /* Check bottom bit only. Higher bits are various sentinels. */
>> rtm_disabled = !(opt_tsx & 1);
>>
>> - lo &= ~TSX_FORCE_ABORT_RTM;
>> - if ( rtm_disabled )
>> - lo |= TSX_FORCE_ABORT_RTM;
>> + lo &= ~(TSX_FORCE_ABORT_RTM | TSX_CPUID_CLEAR | TSX_ENABLE_RTM);
>> +
>> + if ( cpu_has_rtm_always_abort )
>> + {
>> + /*
>> + * June 2021 microcode, on a client part with TSX de-featured:
>> + * - There are no mitigations for the TSX memory ordering errata.
>> + * - Performance counter 3 works. (I.e. it isn't being used by
>> + * microcode to work around the memory ordering errata.)
>> + * - TSX_FORCE_ABORT.FORCE_ABORT_RTM is fixed read1/write-discard.
>> + * - TSX_FORCE_ABORT.TSX_CPUID_CLEAR can be used to hide the
>> + * HLE/RTM CPUID bits.
>> + * - TSX_FORCE_ABORT.ENABLE_RTM may be used to opt in to
>> + * re-enabling RTM, at the users own risk.
>> + */
>> + lo |= rtm_disabled ? TSX_CPUID_CLEAR : TSX_ENABLE_RTM;
> ... the setting of TSX_ENABLE_RTM here which, as a result, causes
> RTM_ALWAYS_ABORT to be clear. If that's correct, perhaps the wording
> in that earlier comment would better be something like "we'll set
> TSX_FORCE_ABORT.ENABLE_RTM and hence cause RTM_ALWAYS_ABORT to be
> hidden from the general CPUID scan later"?
Yes - that is the intended meaning. I'll adjust.
> If this understanding of mine is correct, then preferably with some
> suitable adjustment to the comment wording
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Thanks.
> Also Intel recommends this for SDVs only - can we consider such a
> setup supported (not to speak of security supported) at all? I guess
> you mean to express this by saying "at their own risk" in the
> cmdline doc? If so, perhaps mentioning this in SUPPORT.md would be
> a good thing nevertheless, notwithstanding the fact that we're not
> really good at expressing there how command line option use affects
> support status.
I think this is too fine grained to be expressed in SUPPORT.md, but
given that there is a very clear specific issue, I wouldn't consider
this an unsupported configuration overall.
I don't expect people to want to use TSX on these CPUs in the first
place (which was a factor in choosing off-by-default), but if they do,
there is one specific memory ordering issue of read/writes with a
committed transaction.
None of our code uses RTM, so issues in Xen which manifest with tsx=1
won't be related to TSX being enabled.
Obviously, if someone does report an issue, we can tell them to
re-confirm the issue without tsx=1 just to rule out interactions, but I
don't think it is likely for it to be relevant to reported issues.
~Andrew
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-06-09 12:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-08 17:05 [PATCH] x86/tsx: Cope with TSX deprecation on SKL/KBL/CFL/WHL Andrew Cooper
2021-06-09 6:36 ` Jan Beulich
2021-06-09 12:19 ` Andrew Cooper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.