Re: [XEN PATCH v1] x86/intel: optional build of TSX support

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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@xxxxxxxx>
> ---
>  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