[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] x86/tsx: Cope with TSX deprecation on SKL/KBL/CFL/WHL
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@xxxxxxxx> 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
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |