[x86_64] Disable TSX on some Haswell processors

Message ID CAMXFM3vgYW8oJk257MvNReErGoUELUVmCVVZJYBWFADC2mgO8A@mail.gmail.com
State New, archived
Headers

Commit Message

Andrew Senkevich Dec. 13, 2016, 6:48 p.m. UTC
  Hi,

this is a workaround which disables TSX on some Haswell processors to
avoid TSX on kernel's that weren't updated with the latest microcode
package (which is needed to disable TSX by default due to errata).
Is this patch Ok?

2016-12-13  Andrew Senkevich  <andrew.senkevich@intel.com>

        * sysdeps/x86/cpu-features.c (get_common_indeces): Add
stepping identification.
        (init_cpu_features): Add handle of Haswell.




--
WBR,
Andrew
  

Comments

Andreas Schwab Dec. 13, 2016, 7:39 p.m. UTC | #1
On Dez 13 2016, Andrew Senkevich <andrew.n.senkevich@gmail.com> wrote:

> + to avoid TSX on kernel's that weren't updated with the latest

s/kernel's/kernels/

Andreas.
  
Carlos O'Donell Dec. 14, 2016, 7:13 p.m. UTC | #2
On 12/13/2016 01:48 PM, Andrew Senkevich wrote:
> Hi,
> 
> this is a workaround which disables TSX on some Haswell processors to
> avoid TSX on kernel's that weren't updated with the latest microcode
> package (which is needed to disable TSX by default due to errata).
> Is this patch Ok?

Andrew,

Thank you for putting this patch together.

One serious question: What about Broadwell?

> 2016-12-13  Andrew Senkevich  <andrew.senkevich@intel.com>
> 
>         * sysdeps/x86/cpu-features.c (get_common_indeces): Add
> stepping identification.
>         (init_cpu_features): Add handle of Haswell.
> 
> diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
> index e228a76..a903390 100644
> --- a/sysdeps/x86/cpu-features.c
> +++ b/sysdeps/x86/cpu-features.c
> @@ -22,7 +22,7 @@
>  static void
>  get_common_indeces (struct cpu_features *cpu_features,
>      unsigned int *family, unsigned int *model,
> -    unsigned int *extended_model)
> +    unsigned int *extended_model, unsigned int *stepping)

OK.

>  {
>    if (family)
>      {
> @@ -34,6 +34,7 @@ get_common_indeces (struct cpu_features *cpu_features,
>        *family = (eax >> 8) & 0x0f;
>        *model = (eax >> 4) & 0x0f;
>        *extended_model = (eax >> 12) & 0xf0;
> +      *stepping = eax & 0x0f;

OK.

>        if (*family == 0x0f)
>   {
>    *family += (eax >> 20) & 0xff;
> @@ -116,11 +117,12 @@ init_cpu_features (struct cpu_features *cpu_features)
>    /* This spells out "GenuineIntel".  */
>    if (ebx == 0x756e6547 && ecx == 0x6c65746e && edx == 0x49656e69)
>      {
> -      unsigned int extended_model;
> +      unsigned int extended_model, stepping;

OK

> 
>        kind = arch_kind_intel;
> 
> -      get_common_indeces (cpu_features, &family, &model, &extended_model);
> +      get_common_indeces (cpu_features, &family, &model, &extended_model,
> +  &stepping);

OK.

> 
>        if (family == 0x06)
>   {
> @@ -201,6 +203,19 @@ init_cpu_features (struct cpu_features *cpu_features)
>      | bit_arch_Fast_Unaligned_Copy
>      | bit_arch_Prefer_PMINUB_for_stringop);
>        break;
> +

Case 0x3f needs a comment explaining why we break out early.

> +    case 0x3f:
> +      if (stepping >= 4)
> + break;
>
> +    case 0x3c:
> +    case 0x45:
> +    case 0x46:
> +      /* Disable Intel TSX on Haswell processors (except Xeon E7 v3)
> + to avoid TSX on kernel's that weren't updated with the latest
> + microcode package (which disables broken feature
> + by default).  */
> +      cpu_features->cpuid[COMMON_CPUID_INDEX_7].ebx &= ~(bit_cpu_RTM);
> +      break;

You mention only Haswell, but public announcements claimed that early
Broadwell-Y silicon also had the same problem?

Have you ruled this out or identified that those early Broadwell parts
always shiped with microcode that disabled RTM?

>      }
>   }
> 
> @@ -227,11 +242,12 @@ init_cpu_features (struct cpu_features *cpu_features)
>    /* This spells out "AuthenticAMD".  */
>    else if (ebx == 0x68747541 && ecx == 0x444d4163 && edx == 0x69746e65)
>      {
> -      unsigned int extended_model;
> +      unsigned int extended_model, stepping;

OK.

> 
>        kind = arch_kind_amd;
> 
> -      get_common_indeces (cpu_features, &family, &model, &extended_model);
> +      get_common_indeces (cpu_features, &family, &model, &extended_model,
> +  &stepping);

OK.

> 
>        ecx = cpu_features->cpuid[COMMON_CPUID_INDEX_1].ecx;
> 
> @@ -268,7 +284,7 @@ init_cpu_features (struct cpu_features *cpu_features)
>    else
>      {
>        kind = arch_kind_other;
> -      get_common_indeces (cpu_features, NULL, NULL, NULL);
> +      get_common_indeces (cpu_features, NULL, NULL, NULL, NULL);

OK.

>      }
> 
>    /* Support i586 if CX8 is available.  */
> 
> 
> 
> --
> WBR,
> Andrew
>
  
Andi Kleen Dec. 14, 2016, 7:36 p.m. UTC | #3
Carlos O'Donell <carlos@redhat.com> writes:
>
> One serious question: What about Broadwell?

TSX works on all production Broadwells (when enabled). While there is an
errata documented it is fixed in all production level BIOSes.

-Andi
  
Carlos O'Donell Dec. 16, 2016, 8:58 p.m. UTC | #4
On 12/14/2016 02:36 PM, Andi Kleen wrote:
> Carlos O'Donell <carlos@redhat.com> writes:
>>
>> One serious question: What about Broadwell?
> 
> TSX works on all production Broadwells (when enabled). While there is an
> errata documented it is fixed in all production level BIOSes.

Perfect. Thanks.
  

Patch

diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
index e228a76..a903390 100644
--- a/sysdeps/x86/cpu-features.c
+++ b/sysdeps/x86/cpu-features.c
@@ -22,7 +22,7 @@ 
 static void
 get_common_indeces (struct cpu_features *cpu_features,
     unsigned int *family, unsigned int *model,
-    unsigned int *extended_model)
+    unsigned int *extended_model, unsigned int *stepping)
 {
   if (family)
     {
@@ -34,6 +34,7 @@  get_common_indeces (struct cpu_features *cpu_features,
       *family = (eax >> 8) & 0x0f;
       *model = (eax >> 4) & 0x0f;
       *extended_model = (eax >> 12) & 0xf0;
+      *stepping = eax & 0x0f;
       if (*family == 0x0f)
  {
   *family += (eax >> 20) & 0xff;
@@ -116,11 +117,12 @@  init_cpu_features (struct cpu_features *cpu_features)
   /* This spells out "GenuineIntel".  */
   if (ebx == 0x756e6547 && ecx == 0x6c65746e && edx == 0x49656e69)
     {
-      unsigned int extended_model;
+      unsigned int extended_model, stepping;

       kind = arch_kind_intel;

-      get_common_indeces (cpu_features, &family, &model, &extended_model);
+      get_common_indeces (cpu_features, &family, &model, &extended_model,
+  &stepping);

       if (family == 0x06)
  {
@@ -201,6 +203,19 @@  init_cpu_features (struct cpu_features *cpu_features)
     | bit_arch_Fast_Unaligned_Copy
     | bit_arch_Prefer_PMINUB_for_stringop);
       break;
+
+    case 0x3f:
+      if (stepping >= 4)
+ break;
+    case 0x3c:
+    case 0x45:
+    case 0x46:
+      /* Disable Intel TSX on Haswell processors (except Xeon E7 v3)
+ to avoid TSX on kernel's that weren't updated with the latest
+ microcode package (which disables broken feature
+ by default).  */
+      cpu_features->cpuid[COMMON_CPUID_INDEX_7].ebx &= ~(bit_cpu_RTM);
+      break;
     }
  }

@@ -227,11 +242,12 @@  init_cpu_features (struct cpu_features *cpu_features)
   /* This spells out "AuthenticAMD".  */
   else if (ebx == 0x68747541 && ecx == 0x444d4163 && edx == 0x69746e65)
     {
-      unsigned int extended_model;
+      unsigned int extended_model, stepping;

       kind = arch_kind_amd;

-      get_common_indeces (cpu_features, &family, &model, &extended_model);
+      get_common_indeces (cpu_features, &family, &model, &extended_model,
+  &stepping);

       ecx = cpu_features->cpuid[COMMON_CPUID_INDEX_1].ecx;

@@ -268,7 +284,7 @@  init_cpu_features (struct cpu_features *cpu_features)
   else
     {
       kind = arch_kind_other;
-      get_common_indeces (cpu_features, NULL, NULL, NULL);
+      get_common_indeces (cpu_features, NULL, NULL, NULL, NULL);
     }

   /* Support i586 if CX8 is available.  */