diff mbox series

x86: Black list more Intel CPUs for TSX [BZ #27398]

Message ID 20220118164238.3761164-1-hjl.tools@gmail.com
State Superseded
Headers show
Series x86: Black list more Intel CPUs for TSX [BZ #27398] | expand

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

H.J. Lu Jan. 18, 2022, 4:42 p.m. UTC
Disable TSX and enable RTM_ALWAYS_ABORT for Intel CPUs listed in:

https://www.intel.com/content/www/us/en/support/articles/000059422/processors.html

This fixes BZ #27398.
---
 sysdeps/x86/cpu-features.c | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

Comments

Noah Goldstein Jan. 18, 2022, 7:55 p.m. UTC | #1
On Tue, Jan 18, 2022 at 10:43 AM H.J. Lu via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> Disable TSX and enable RTM_ALWAYS_ABORT for Intel CPUs listed in:
>
> https://www.intel.com/content/www/us/en/support/articles/000059422/processors.html
>
> This fixes BZ #27398.
> ---
>  sysdeps/x86/cpu-features.c | 35 ++++++++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
> index 772ccf8e91..034d9feff7 100644
> --- a/sysdeps/x86/cpu-features.c
> +++ b/sysdeps/x86/cpu-features.c
> @@ -507,11 +507,40 @@ init_cpu_features (struct cpu_features *cpu_features)
>               break;
>             }
>
> -        /* Disable TSX on some Haswell processors to avoid TSX on kernels that
> -           weren't updated with the latest microcode package (which disables
> -           broken feature by default).  */
> +        /* Disable TSX on some processors to avoid TSX on kernels that
> +           weren't updated with the latest microcode package (which
> +           disables broken feature by default).  */
>          switch (model)
>             {
> +           case 0x55:
> +             if (stepping <= 5)
> +               goto disable_tsx;
> +             break;
> +           case 0x8e:
> +             /* NB: The errata says that only 0xb stepping or lower are
> +                impacted.  But 0xc stepping also exhibits the similar
> +                behavior.  */

Do you have any link for this or is this just from internal testing?

> +             if (stepping <= 0xc)
> +               goto disable_tsx;
If you want you could make `case 0x8e` fall through into `case 0x9e`
given that both
disable for `stepping <= 0xc`. This isn't a critical path and clarity
might be more
important, especially since the `case 0x8e` goes against documentation.

> +             break;
> +           case 0x9e:
> +             if (stepping > 0xc)
> +               break;
> +             /* Fall through.  */
> +           case 0x4e:
> +           case 0x5e:
> +             {
> +               /* Disable Intel TSX and enable RTM_ALWAYS_ABORT for
> +                  processors listed in:
> +
> +https://www.intel.com/content/www/us/en/support/articles/000059422/processors.html
> +                */
> +disable_tsx:
> +               CPU_FEATURE_UNSET (cpu_features, HLE);
> +               CPU_FEATURE_UNSET (cpu_features, RTM);
> +               CPU_FEATURE_SET (cpu_features, RTM_ALWAYS_ABORT);
> +             }
> +             break;
>             case 0x3f:
>               /* Xeon E7 v3 with stepping >= 4 has working TSX.  */
>               if (stepping >= 4)
> --
> 2.34.1
>

LGTM although have a few comments.
H.J. Lu Jan. 18, 2022, 8:33 p.m. UTC | #2
On Tue, Jan 18, 2022 at 11:55 AM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
>
> On Tue, Jan 18, 2022 at 10:43 AM H.J. Lu via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
> >
> > Disable TSX and enable RTM_ALWAYS_ABORT for Intel CPUs listed in:
> >
> > https://www.intel.com/content/www/us/en/support/articles/000059422/processors.html
> >
> > This fixes BZ #27398.
> > ---
> >  sysdeps/x86/cpu-features.c | 35 ++++++++++++++++++++++++++++++++---
> >  1 file changed, 32 insertions(+), 3 deletions(-)
> >
> > diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
> > index 772ccf8e91..034d9feff7 100644
> > --- a/sysdeps/x86/cpu-features.c
> > +++ b/sysdeps/x86/cpu-features.c
> > @@ -507,11 +507,40 @@ init_cpu_features (struct cpu_features *cpu_features)
> >               break;
> >             }
> >
> > -        /* Disable TSX on some Haswell processors to avoid TSX on kernels that
> > -           weren't updated with the latest microcode package (which disables
> > -           broken feature by default).  */
> > +        /* Disable TSX on some processors to avoid TSX on kernels that
> > +           weren't updated with the latest microcode package (which
> > +           disables broken feature by default).  */
> >          switch (model)
> >             {
> > +           case 0x55:
> > +             if (stepping <= 5)
> > +               goto disable_tsx;
> > +             break;
> > +           case 0x8e:
> > +             /* NB: The errata says that only 0xb stepping or lower are
> > +                impacted.  But 0xc stepping also exhibits the similar
> > +                behavior.  */
>
> Do you have any link for this or is this just from internal testing?

I asked and was told that "the intention was to disable TSX on all
client parts on all steppings.".
Apparently, the errata missed at least one stepping.

> > +             if (stepping <= 0xc)
> > +               goto disable_tsx;
> If you want you could make `case 0x8e` fall through into `case 0x9e`
> given that both
> disable for `stepping <= 0xc`. This isn't a critical path and clarity
> might be more
> important, especially since the `case 0x8e` goes against documentation.

Good point.   Will fix it.

> > +             break;
> > +           case 0x9e:
> > +             if (stepping > 0xc)
> > +               break;
> > +             /* Fall through.  */
> > +           case 0x4e:
> > +           case 0x5e:
> > +             {
> > +               /* Disable Intel TSX and enable RTM_ALWAYS_ABORT for
> > +                  processors listed in:
> > +
> > +https://www.intel.com/content/www/us/en/support/articles/000059422/processors.html
> > +                */
> > +disable_tsx:
> > +               CPU_FEATURE_UNSET (cpu_features, HLE);
> > +               CPU_FEATURE_UNSET (cpu_features, RTM);
> > +               CPU_FEATURE_SET (cpu_features, RTM_ALWAYS_ABORT);
> > +             }
> > +             break;
> >             case 0x3f:
> >               /* Xeon E7 v3 with stepping >= 4 has working TSX.  */
> >               if (stepping >= 4)
> > --
> > 2.34.1
> >
>
> LGTM although have a few comments.

Thanks.
diff mbox series

Patch

diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
index 772ccf8e91..034d9feff7 100644
--- a/sysdeps/x86/cpu-features.c
+++ b/sysdeps/x86/cpu-features.c
@@ -507,11 +507,40 @@  init_cpu_features (struct cpu_features *cpu_features)
 	      break;
 	    }
 
-	 /* Disable TSX on some Haswell processors to avoid TSX on kernels that
-	    weren't updated with the latest microcode package (which disables
-	    broken feature by default).  */
+	 /* Disable TSX on some processors to avoid TSX on kernels that
+	    weren't updated with the latest microcode package (which
+	    disables broken feature by default).  */
 	 switch (model)
 	    {
+	    case 0x55:
+	      if (stepping <= 5)
+		goto disable_tsx;
+	      break;
+	    case 0x8e:
+	      /* NB: The errata says that only 0xb stepping or lower are
+		 impacted.  But 0xc stepping also exhibits the similar
+		 behavior.  */
+	      if (stepping <= 0xc)
+		goto disable_tsx;
+	      break;
+	    case 0x9e:
+	      if (stepping > 0xc)
+		break;
+	      /* Fall through.  */
+	    case 0x4e:
+	    case 0x5e:
+	      {
+		/* Disable Intel TSX and enable RTM_ALWAYS_ABORT for
+		   processors listed in:
+
+https://www.intel.com/content/www/us/en/support/articles/000059422/processors.html
+		 */
+disable_tsx:
+		CPU_FEATURE_UNSET (cpu_features, HLE);
+		CPU_FEATURE_UNSET (cpu_features, RTM);
+		CPU_FEATURE_SET (cpu_features, RTM_ALWAYS_ABORT);
+	      }
+	      break;
 	    case 0x3f:
 	      /* Xeon E7 v3 with stepping >= 4 has working TSX.  */
 	      if (stepping >= 4)