| Message ID | 20240606110448.2540261-1-Sergiy_Kibrik@epam.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [XEN,v1] x86/intel: optional build of TSX support | expand |
On 06.06.2024 13:04, Sergiy Kibrik wrote: > --- a/xen/arch/x86/spec_ctrl.c > +++ b/xen/arch/x86/spec_ctrl.c > @@ -116,8 +116,10 @@ static int __init cf_check parse_spec_ctrl(const char *s) > if ( opt_pv_l1tf_domu < 0 ) > opt_pv_l1tf_domu = 0; > > +#ifdef CONFIG_INTEL > if ( opt_tsx == -1 ) > opt_tsx = -3; > +#endif Personally I prefer using the direct check in such cases, rather one on a prereq symbol. I.e. "#ifndef opt_tsx" both here and below. Other maintainers may have a different view, though, so I won't insist unless at least one of them shares this perspective with me. Other than this: Looks largely okay to me. Jan
On 06/06/2024 12:04 pm, Sergiy Kibrik wrote: > Transactional Synchronization Extensions are available for certain Intel's > CPUs only, hence can be put under CONFIG_INTEL build option. Careful with "available" vs "supported". They're only supported on certain Intel CPUs, but suffice it to say that c0dd53b8cb was discovered because of Xen's TSX unit testing. > > The whole TSX support, even if supported by CPU, may need to be disabled via > options, by microcode or through spec-ctrl, depending on a set of specific > conditions. To make sure nothing gets accidentally rutime-broken all runtime > modifications of global TSX configuration variables is secured by #ifdef's, > while variables themselves redefined to 0, so that ones can't mistakenly be > written to. > > Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com> > --- > xen/arch/x86/Makefile | 2 +- > xen/arch/x86/include/asm/processor.h | 8 ++++++++ > xen/arch/x86/spec_ctrl.c | 4 ++++ > 3 files changed, 13 insertions(+), 1 deletion(-) This needs a command line adjustment too. diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc index 1dea7431fab6..c8d32c13bbaa 100644 --- a/docs/misc/xen-command-line.pandoc +++ b/docs/misc/xen-command-line.pandoc @@ -2584,10 +2584,11 @@ pages) must also be specified via the tbuf_size parameter. ### tsx = <bool> - Applicability: x86 + Applicability: x86 with CONFIG_INTEL active Default: false on parts vulnerable to TAA, true otherwise -Controls for the use of Transactional Synchronization eXtensions. +Controls for the use of Transactional Synchronization eXtensions, available if +Xen was compiled with `CONFIG_INTEL` active. Several microcode updates are relevant: > diff --git a/xen/arch/x86/include/asm/processor.h b/xen/arch/x86/include/asm/processor.h > index c26ef9090c..8b12627ab0 100644 > --- a/xen/arch/x86/include/asm/processor.h > +++ b/xen/arch/x86/include/asm/processor.h > @@ -503,9 +503,17 @@ static inline uint8_t get_cpu_family(uint32_t raw, uint8_t *model, > return fam; > } > > +#ifdef CONFIG_INTEL > extern int8_t opt_tsx; > extern bool rtm_disabled; > void tsx_init(void); > +#else > +#define opt_tsx 0 /* explicitly indicate TSX is off */ > +#define rtm_disabled false /* RTM was not force-disabled */ > +static inline void tsx_init(void) > +{ > +} For trivial things like this, we allow static inline void tsx_init(void) {} All can be fixed on commit, but none of this is tagged for 4.19 and is 4.20 material IMO. ~Andrew
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile index d902fb7acc..286c003ec3 100644 --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -67,7 +67,7 @@ obj-y += srat.o obj-y += string.o obj-y += time.o obj-y += traps.o -obj-y += tsx.o +obj-$(CONFIG_INTEL) += tsx.o obj-y += usercopy.o obj-y += x86_emulate.o obj-$(CONFIG_TBOOT) += tboot.o diff --git a/xen/arch/x86/include/asm/processor.h b/xen/arch/x86/include/asm/processor.h index c26ef9090c..8b12627ab0 100644 --- a/xen/arch/x86/include/asm/processor.h +++ b/xen/arch/x86/include/asm/processor.h @@ -503,9 +503,17 @@ static inline uint8_t get_cpu_family(uint32_t raw, uint8_t *model, return fam; } +#ifdef CONFIG_INTEL extern int8_t opt_tsx; extern bool rtm_disabled; void tsx_init(void); +#else +#define opt_tsx 0 /* explicitly indicate TSX is off */ +#define rtm_disabled false /* RTM was not force-disabled */ +static inline void tsx_init(void) +{ +} +#endif void update_mcu_opt_ctrl(void); void set_in_mcu_opt_ctrl(uint32_t mask, uint32_t val); diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c index 40f6ae0170..6b3631e375 100644 --- a/xen/arch/x86/spec_ctrl.c +++ b/xen/arch/x86/spec_ctrl.c @@ -116,8 +116,10 @@ static int __init cf_check parse_spec_ctrl(const char *s) if ( opt_pv_l1tf_domu < 0 ) opt_pv_l1tf_domu = 0; +#ifdef CONFIG_INTEL if ( opt_tsx == -1 ) opt_tsx = -3; +#endif disable_common: opt_rsb_pv = false; @@ -2264,6 +2266,7 @@ void __init init_speculation_mitigations(void) * plausibly value TSX higher than Hyperthreading...), disable TSX to * mitigate TAA. */ +#ifdef CONFIG_INTEL if ( opt_tsx == -1 && cpu_has_bug_taa && cpu_has_tsx_ctrl && ((hw_smt_enabled && opt_smt) || !boot_cpu_has(X86_FEATURE_SC_VERW_IDLE)) ) @@ -2271,6 +2274,7 @@ void __init init_speculation_mitigations(void) opt_tsx = 0; tsx_init(); } +#endif /* * On some SRBDS-affected hardware, it may be safe to relax srb-lock by
Transactional Synchronization Extensions are available for certain Intel's CPUs only, hence can be put under CONFIG_INTEL build option. The whole TSX support, even if supported by CPU, may need to be disabled via options, by microcode or through spec-ctrl, depending on a set of specific conditions. To make sure nothing gets accidentally rutime-broken all modifications of global TSX configuration variables is secured by #ifdef's, while variables themselves redefined to 0, so that ones can't mistakenly be written to. Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com> --- xen/arch/x86/Makefile | 2 +- xen/arch/x86/include/asm/processor.h | 8 ++++++++ xen/arch/x86/spec_ctrl.c | 4 ++++ 3 files changed, 13 insertions(+), 1 deletion(-)