All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH for-4.11] x86/spec_ctrl: Updates to retpoline-safety decision making
@ 2018-04-18 18:13 Andrew Cooper
  2018-04-19  9:00 ` Jan Beulich
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Andrew Cooper @ 2018-04-18 18:13 UTC (permalink / raw)
  To: Xen-devel; +Cc: Juergen Gross, Andrew Cooper, Jan Beulich

All of this is as recommended by the Intel whitepaper:

https://software.intel.com/sites/default/files/managed/1d/46/Retpoline-A-Branch-Target-Injection-Mitigation.pdf

The RSB-alternative bit in MSR_ARCH_CAPABILITIES may be set by a hypervisor to
indicate that the virtual machine may migrate to a processor which isn't
retpoline-safe.  Introduce a shortened name (to reduce code volume), treat it
as authorative in retpoline_safe(), and print its value along with the other
ARCH_CAPS bits.

The exact processor models which do have RSB semantics which fall back to BTB
predictions are enumerated, and include Kabylake and Cannonlake.  Leave a
printk() in the default case to help identify cases which aren't covered.

The exact microcode versions from Broadwell RSB-safety are taken from the
referenced microcode update file (adjusting for the known-bad microcode
versions).  In practice, this means that all Broadwell hardware with
up-to-date microcode will use retpoline in preference to IBRS.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Juergen Gross <jgross@suse.com>

This should be backported to everywhere which has Spectre mitigations, and
therefore should be considered for 4.11 at this point.
---
 xen/arch/x86/spec_ctrl.c        | 48 +++++++++++++++++++++++++++++++++++------
 xen/include/asm-x86/msr-index.h |  1 +
 2 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 5b5ec90..aff06f0 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -113,12 +113,13 @@ static void __init print_details(enum ind_thunk thunk)
     printk(XENLOG_DEBUG "Speculative mitigation facilities:\n");
 
     /* Hardware features which pertain to speculative mitigations. */
-    printk(XENLOG_DEBUG "  Hardware features:%s%s%s%s%s\n",
+    printk(XENLOG_DEBUG "  Hardware features:%s%s%s%s%s%s\n",
            (_7d0 & cpufeat_mask(X86_FEATURE_IBRSB)) ? " IBRS/IBPB" : "",
            (_7d0 & cpufeat_mask(X86_FEATURE_STIBP)) ? " STIBP"     : "",
            (e8b  & cpufeat_mask(X86_FEATURE_IBPB))  ? " IBPB"      : "",
            (caps & ARCH_CAPABILITIES_IBRS_ALL)      ? " IBRS_ALL"  : "",
-           (caps & ARCH_CAPABILITIES_RDCL_NO)       ? " RDCL_NO"   : "");
+           (caps & ARCH_CAPABILITIES_RDCL_NO)       ? " RDCL_NO"   : "",
+           (caps & ARCH_CAPS_RSBA)                  ? " RBSA"      : "");
 
     /* Compiled-in support which pertains to BTI mitigations. */
     if ( IS_ENABLED(CONFIG_INDIRECT_THUNK) )
@@ -151,6 +152,20 @@ static bool __init retpoline_safe(void)
          boot_cpu_data.x86 != 6 )
         return false;
 
+    if ( boot_cpu_has(X86_FEATURE_ARCH_CAPS) )
+    {
+        uint64_t caps;
+
+        rdmsrl(MSR_ARCH_CAPABILITIES, caps);
+
+        /*
+         * RBSA may be set by a hypervisor to indicate that we may move to a
+         * processor which isn't retpoline-safe.
+         */
+        if ( caps & ARCH_CAPS_RSBA )
+            return false;
+    }
+
     switch ( boot_cpu_data.x86_model )
     {
     case 0x17: /* Penryn */
@@ -177,18 +192,37 @@ static bool __init retpoline_safe(void)
          * versions.
          */
     case 0x3d: /* Broadwell */
-        return ucode_rev >= 0x28;
+        return ucode_rev >= 0x2a;
     case 0x47: /* Broadwell H */
-        return ucode_rev >= 0x1b;
+        return ucode_rev >= 0x1d;
     case 0x4f: /* Broadwell EP/EX */
-        return ucode_rev >= 0xb000025;
+        return ucode_rev >= 0xb000021;
     case 0x56: /* Broadwell D */
-        return false; /* TBD. */
+        switch ( boot_cpu_data.x86_mask )
+        {
+        case 2:  return ucode_rev >= 0x15;
+        case 3:  return ucode_rev >= 0x7000012;
+        case 4:  return ucode_rev >= 0xf000011;
+        case 5:  return ucode_rev >= 0xe000009;
+        default: return false;
+        }
+        break;
 
         /*
-         * Skylake and later processors are not retpoline-safe.
+         * Skylake, Kabylake and Cannonlake processors are not retpoline-safe.
          */
+    case 0x4e:
+    case 0x55:
+    case 0x5e:
+    case 0x66:
+    case 0x67:
+    case 0x8e:
+    case 0x9e:
+        return false;
+
     default:
+        printk("Unrecognised CPU model %#x - assuming not reptpoline safe\n",
+               boot_cpu_data.x86_model);
         return false;
     }
 }
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index 8416756..c9f44eb 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -42,6 +42,7 @@
 #define MSR_ARCH_CAPABILITIES		0x0000010a
 #define ARCH_CAPABILITIES_RDCL_NO	(_AC(1, ULL) << 0)
 #define ARCH_CAPABILITIES_IBRS_ALL	(_AC(1, ULL) << 1)
+#define ARCH_CAPS_RSBA			(_AC(1, ULL) << 2)
 
 /* Intel MSRs. Some also available on other CPUs */
 #define MSR_IA32_PERFCTR0		0x000000c1
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH for-4.11] x86/spec_ctrl: Updates to retpoline-safety decision making
  2018-04-18 18:13 [PATCH for-4.11] x86/spec_ctrl: Updates to retpoline-safety decision making Andrew Cooper
@ 2018-04-19  9:00 ` Jan Beulich
  2018-04-19 10:26   ` Andrew Cooper
  2018-04-19 14:25 ` [PATCH v2 " Andrew Cooper
  2018-04-23 21:16 ` [PATCH " Konrad Rzeszutek Wilk
  2 siblings, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2018-04-19  9:00 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Juergen Gross, Xen-devel

>>> On 18.04.18 at 20:13, <andrew.cooper3@citrix.com> wrote:
> @@ -151,6 +152,20 @@ static bool __init retpoline_safe(void)
>           boot_cpu_data.x86 != 6 )
>          return false;
>  
> +    if ( boot_cpu_has(X86_FEATURE_ARCH_CAPS) )
> +    {
> +        uint64_t caps;
> +
> +        rdmsrl(MSR_ARCH_CAPABILITIES, caps);
> +
> +        /*
> +         * RBSA may be set by a hypervisor to indicate that we may move to a
> +         * processor which isn't retpoline-safe.
> +         */
> +        if ( caps & ARCH_CAPS_RSBA )
> +            return false;
> +    }

I assume handing this through / forcing it on for guests is meant to be
done in a later patch?

> @@ -177,18 +192,37 @@ static bool __init retpoline_safe(void)
>           * versions.
>           */
>      case 0x3d: /* Broadwell */
> -        return ucode_rev >= 0x28;
> +        return ucode_rev >= 0x2a;
>      case 0x47: /* Broadwell H */
> -        return ucode_rev >= 0x1b;
> +        return ucode_rev >= 0x1d;
>      case 0x4f: /* Broadwell EP/EX */
> -        return ucode_rev >= 0xb000025;
> +        return ucode_rev >= 0xb000021;
>      case 0x56: /* Broadwell D */
> -        return false; /* TBD. */
> +        switch ( boot_cpu_data.x86_mask )
> +        {
> +        case 2:  return ucode_rev >= 0x15;
> +        case 3:  return ucode_rev >= 0x7000012;
> +        case 4:  return ucode_rev >= 0xf000011;
> +        case 5:  return ucode_rev >= 0xe000009;
> +        default: return false;
> +        }
> +        break;

Hmm, the white paper says
"The predictable speculative behavior of the RET instruction is the key to
 retpoline being a robust mitigation. RET has this behavior on all processors
 which are based on the Intel® microarchitecture codename Broadwell and
 earlier when updated with the latest microcode."

Am I to assume the text is imprecise, or else why is it that only Broadwells
are being checked for ucode version? Also the default case of the
Broadwell sub-switch would perhaps better gain a printk() just like ...

>          /*
> -         * Skylake and later processors are not retpoline-safe.
> +         * Skylake, Kabylake and Cannonlake processors are not retpoline-safe.
>           */
> +    case 0x4e:
> +    case 0x55:
> +    case 0x5e:
> +    case 0x66:
> +    case 0x67:
> +    case 0x8e:
> +    case 0x9e:
> +        return false;
> +
>      default:
> +        printk("Unrecognised CPU model %#x - assuming not reptpoline safe\n",
> +               boot_cpu_data.x86_model);
>          return false;

... this one?

Also, looking at ark.intel.com I find Coffee Lake, not Cannon Lake.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH for-4.11] x86/spec_ctrl: Updates to retpoline-safety decision making
  2018-04-19  9:00 ` Jan Beulich
@ 2018-04-19 10:26   ` Andrew Cooper
  2018-04-19 12:04     ` Jan Beulich
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Cooper @ 2018-04-19 10:26 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Juergen Gross, Xen-devel

On 19/04/18 10:00, Jan Beulich wrote:
>>>> On 18.04.18 at 20:13, <andrew.cooper3@citrix.com> wrote:
>> @@ -151,6 +152,20 @@ static bool __init retpoline_safe(void)
>>           boot_cpu_data.x86 != 6 )
>>          return false;
>>  
>> +    if ( boot_cpu_has(X86_FEATURE_ARCH_CAPS) )
>> +    {
>> +        uint64_t caps;
>> +
>> +        rdmsrl(MSR_ARCH_CAPABILITIES, caps);
>> +
>> +        /*
>> +         * RBSA may be set by a hypervisor to indicate that we may move to a
>> +         * processor which isn't retpoline-safe.
>> +         */
>> +        if ( caps & ARCH_CAPS_RSBA )
>> +            return false;
>> +    }
> I assume handing this through / forcing it on for guests is meant to be
> done in a later patch?

Virtualisaing MSR_ARCH_CAPS for guests is in progress, but definitely
4.12 material.

It requires the toolstack CPUID/MSR improvements so this bit can be
levelled across resource pools, which means dusting off some fairly old
patches of mine, and getting them finished.

>
>> @@ -177,18 +192,37 @@ static bool __init retpoline_safe(void)
>>           * versions.
>>           */
>>      case 0x3d: /* Broadwell */
>> -        return ucode_rev >= 0x28;
>> +        return ucode_rev >= 0x2a;
>>      case 0x47: /* Broadwell H */
>> -        return ucode_rev >= 0x1b;
>> +        return ucode_rev >= 0x1d;
>>      case 0x4f: /* Broadwell EP/EX */
>> -        return ucode_rev >= 0xb000025;
>> +        return ucode_rev >= 0xb000021;
>>      case 0x56: /* Broadwell D */
>> -        return false; /* TBD. */
>> +        switch ( boot_cpu_data.x86_mask )
>> +        {
>> +        case 2:  return ucode_rev >= 0x15;
>> +        case 3:  return ucode_rev >= 0x7000012;
>> +        case 4:  return ucode_rev >= 0xf000011;
>> +        case 5:  return ucode_rev >= 0xe000009;
>> +        default: return false;
>> +        }
>> +        break;
> Hmm, the white paper says
> "The predictable speculative behavior of the RET instruction is the key to
>  retpoline being a robust mitigation. RET has this behavior on all processors
>  which are based on the Intel® microarchitecture codename Broadwell and
>  earlier when updated with the latest microcode."
>
> Am I to assume the text is imprecise, or else why is it that only Broadwells
> are being checked for ucode version?

Hmm yes - that does look like poor wording in the whitepaper.  It is the
case that Broadwell is the only uarch which needs the microcode check.

> Also the default case of the
> Broadwell sub-switch would perhaps better gain a printk() just like ...

Will do.

>
>>          /*
>> -         * Skylake and later processors are not retpoline-safe.
>> +         * Skylake, Kabylake and Cannonlake processors are not retpoline-safe.
>>           */
>> +    case 0x4e:
>> +    case 0x55:
>> +    case 0x5e:
>> +    case 0x66:
>> +    case 0x67:
>> +    case 0x8e:
>> +    case 0x9e:
>> +        return false;
>> +
>>      default:
>> +        printk("Unrecognised CPU model %#x - assuming not reptpoline safe\n",
>> +               boot_cpu_data.x86_model);
>>          return false;
> ... this one?
>
> Also, looking at ark.intel.com I find Coffee Lake, not Cannon Lake.

I was going from Linux's arch/x86/include/asm/intel-family.h which is
still on my TODO list to stea^W port to Xen, unless someone feels like
beating me to it.

It seems that Cannonlake may not actually be out yet, while Coffeelake
does look like it is definitely out.  I'm completely confused....

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH for-4.11] x86/spec_ctrl: Updates to retpoline-safety decision making
  2018-04-19 10:26   ` Andrew Cooper
@ 2018-04-19 12:04     ` Jan Beulich
  0 siblings, 0 replies; 9+ messages in thread
From: Jan Beulich @ 2018-04-19 12:04 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Juergen Gross, Xen-devel

>>> On 19.04.18 at 12:26, <andrew.cooper3@citrix.com> wrote:
> On 19/04/18 10:00, Jan Beulich wrote:
>>>>> On 18.04.18 at 20:13, <andrew.cooper3@citrix.com> wrote:
>>> @@ -177,18 +192,37 @@ static bool __init retpoline_safe(void)
>>>           * versions.
>>>           */
>>>      case 0x3d: /* Broadwell */
>>> -        return ucode_rev >= 0x28;
>>> +        return ucode_rev >= 0x2a;
>>>      case 0x47: /* Broadwell H */
>>> -        return ucode_rev >= 0x1b;
>>> +        return ucode_rev >= 0x1d;
>>>      case 0x4f: /* Broadwell EP/EX */
>>> -        return ucode_rev >= 0xb000025;
>>> +        return ucode_rev >= 0xb000021;
>>>      case 0x56: /* Broadwell D */
>>> -        return false; /* TBD. */
>>> +        switch ( boot_cpu_data.x86_mask )
>>> +        {
>>> +        case 2:  return ucode_rev >= 0x15;
>>> +        case 3:  return ucode_rev >= 0x7000012;
>>> +        case 4:  return ucode_rev >= 0xf000011;
>>> +        case 5:  return ucode_rev >= 0xe000009;
>>> +        default: return false;
>>> +        }
>>> +        break;
>> Hmm, the white paper says
>> "The predictable speculative behavior of the RET instruction is the key to
>>  retpoline being a robust mitigation. RET has this behavior on all processors
>>  which are based on the Intel® microarchitecture codename Broadwell and
>>  earlier when updated with the latest microcode."
>>
>> Am I to assume the text is imprecise, or else why is it that only Broadwells
>> are being checked for ucode version?
> 
> Hmm yes - that does look like poor wording in the whitepaper.  It is the
> case that Broadwell is the only uarch which needs the microcode check.

Would you mind clarifying this in the patch description (I don't think Intel
would, even if we told them, be overly quick with changing that wording
to match reality)?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 for-4.11] x86/spec_ctrl: Updates to retpoline-safety decision making
  2018-04-18 18:13 [PATCH for-4.11] x86/spec_ctrl: Updates to retpoline-safety decision making Andrew Cooper
  2018-04-19  9:00 ` Jan Beulich
@ 2018-04-19 14:25 ` Andrew Cooper
  2018-04-19 15:02   ` Jan Beulich
  2018-04-19 15:15   ` Juergen Gross
  2018-04-23 21:16 ` [PATCH " Konrad Rzeszutek Wilk
  2 siblings, 2 replies; 9+ messages in thread
From: Andrew Cooper @ 2018-04-19 14:25 UTC (permalink / raw)
  To: Xen-devel; +Cc: Juergen Gross, Andrew Cooper, Jan Beulich

All of this is as recommended by the Intel whitepaper:

https://software.intel.com/sites/default/files/managed/1d/46/Retpoline-A-Branch-Target-Injection-Mitigation.pdf

The 'RSB Alternative' bit in MSR_ARCH_CAPABILITIES may be set by a hypervisor
to indicate that the virtual machine may migrate to a processor which isn't
retpoline-safe.  Introduce a shortened name (to reduce code volume), treat it
as authorative in retpoline_safe(), and print its value along with the other
ARCH_CAPS bits.

The exact processor models which do have RSB semantics which fall back to BTB
predictions are enumerated, and include Kabylake and Coffeelake.  Leave a
printk() in the default case to help identify cases which aren't covered.

The exact microcode versions from Broadwell RSB-safety are taken from the
referenced microcode update file (adjusting for the known-bad microcode
versions).  Despite the exact wording of the text, it is only Broadwell
processors which need a microcode check.

In practice, this means that all Broadwell hardware with up-to-date microcode
will use retpoline in preference to IBRS, which will be a performance
improvement for desktop and server systems which would previously always opt
for IBRS over retpoline.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Juergen Gross <jgross@suse.com>

v2:
 * Add a Broadwell stepping default printk()
 * Clarify the Broadwell microcode statement in the commit message

This should be backported to everywhere which has Spectre mitigations, and
therefore should be considered for 4.11 at this point.
---
 xen/arch/x86/spec_ctrl.c        | 51 +++++++++++++++++++++++++++++++++++------
 xen/include/asm-x86/msr-index.h |  1 +
 2 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 5b5ec90..bab8595 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -113,12 +113,13 @@ static void __init print_details(enum ind_thunk thunk)
     printk(XENLOG_DEBUG "Speculative mitigation facilities:\n");
 
     /* Hardware features which pertain to speculative mitigations. */
-    printk(XENLOG_DEBUG "  Hardware features:%s%s%s%s%s\n",
+    printk(XENLOG_DEBUG "  Hardware features:%s%s%s%s%s%s\n",
            (_7d0 & cpufeat_mask(X86_FEATURE_IBRSB)) ? " IBRS/IBPB" : "",
            (_7d0 & cpufeat_mask(X86_FEATURE_STIBP)) ? " STIBP"     : "",
            (e8b  & cpufeat_mask(X86_FEATURE_IBPB))  ? " IBPB"      : "",
            (caps & ARCH_CAPABILITIES_IBRS_ALL)      ? " IBRS_ALL"  : "",
-           (caps & ARCH_CAPABILITIES_RDCL_NO)       ? " RDCL_NO"   : "");
+           (caps & ARCH_CAPABILITIES_RDCL_NO)       ? " RDCL_NO"   : "",
+           (caps & ARCH_CAPS_RSBA)                  ? " RBSA"      : "");
 
     /* Compiled-in support which pertains to BTI mitigations. */
     if ( IS_ENABLED(CONFIG_INDIRECT_THUNK) )
@@ -151,6 +152,20 @@ static bool __init retpoline_safe(void)
          boot_cpu_data.x86 != 6 )
         return false;
 
+    if ( boot_cpu_has(X86_FEATURE_ARCH_CAPS) )
+    {
+        uint64_t caps;
+
+        rdmsrl(MSR_ARCH_CAPABILITIES, caps);
+
+        /*
+         * RBSA may be set by a hypervisor to indicate that we may move to a
+         * processor which isn't retpoline-safe.
+         */
+        if ( caps & ARCH_CAPS_RSBA )
+            return false;
+    }
+
     switch ( boot_cpu_data.x86_model )
     {
     case 0x17: /* Penryn */
@@ -177,18 +192,40 @@ static bool __init retpoline_safe(void)
          * versions.
          */
     case 0x3d: /* Broadwell */
-        return ucode_rev >= 0x28;
+        return ucode_rev >= 0x2a;
     case 0x47: /* Broadwell H */
-        return ucode_rev >= 0x1b;
+        return ucode_rev >= 0x1d;
     case 0x4f: /* Broadwell EP/EX */
-        return ucode_rev >= 0xb000025;
+        return ucode_rev >= 0xb000021;
     case 0x56: /* Broadwell D */
-        return false; /* TBD. */
+        switch ( boot_cpu_data.x86_mask )
+        {
+        case 2:  return ucode_rev >= 0x15;
+        case 3:  return ucode_rev >= 0x7000012;
+        case 4:  return ucode_rev >= 0xf000011;
+        case 5:  return ucode_rev >= 0xe000009;
+        default:
+            printk("Unrecognised CPU stepping %#x - assuming not reptpoline safe\n",
+                   boot_cpu_data.x86_mask);
+            return false;
+        }
+        break;
 
         /*
-         * Skylake and later processors are not retpoline-safe.
+         * Skylake, Kabylake and Cannonlake processors are not retpoline-safe.
          */
+    case 0x4e:
+    case 0x55:
+    case 0x5e:
+    case 0x66:
+    case 0x67:
+    case 0x8e:
+    case 0x9e:
+        return false;
+
     default:
+        printk("Unrecognised CPU model %#x - assuming not reptpoline safe\n",
+               boot_cpu_data.x86_model);
         return false;
     }
 }
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index 8416756..c9f44eb 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -42,6 +42,7 @@
 #define MSR_ARCH_CAPABILITIES		0x0000010a
 #define ARCH_CAPABILITIES_RDCL_NO	(_AC(1, ULL) << 0)
 #define ARCH_CAPABILITIES_IBRS_ALL	(_AC(1, ULL) << 1)
+#define ARCH_CAPS_RSBA			(_AC(1, ULL) << 2)
 
 /* Intel MSRs. Some also available on other CPUs */
 #define MSR_IA32_PERFCTR0		0x000000c1
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 for-4.11] x86/spec_ctrl: Updates to retpoline-safety decision making
  2018-04-19 14:25 ` [PATCH v2 " Andrew Cooper
@ 2018-04-19 15:02   ` Jan Beulich
  2018-04-19 15:15   ` Juergen Gross
  1 sibling, 0 replies; 9+ messages in thread
From: Jan Beulich @ 2018-04-19 15:02 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Juergen Gross, Xen-devel

>>> On 19.04.18 at 16:25, <andrew.cooper3@citrix.com> wrote:
> All of this is as recommended by the Intel whitepaper:
> 
> https://software.intel.com/sites/default/files/managed/1d/46/Retpoline-A-Branch-Target-Injection-Mitigation.pdf
> 
> The 'RSB Alternative' bit in MSR_ARCH_CAPABILITIES may be set by a hypervisor
> to indicate that the virtual machine may migrate to a processor which isn't
> retpoline-safe.  Introduce a shortened name (to reduce code volume), treat it
> as authorative in retpoline_safe(), and print its value along with the other
> ARCH_CAPS bits.
> 
> The exact processor models which do have RSB semantics which fall back to BTB
> predictions are enumerated, and include Kabylake and Coffeelake.  Leave a
> printk() in the default case to help identify cases which aren't covered.
> 
> The exact microcode versions from Broadwell RSB-safety are taken from the
> referenced microcode update file (adjusting for the known-bad microcode
> versions).  Despite the exact wording of the text, it is only Broadwell
> processors which need a microcode check.
> 
> In practice, this means that all Broadwell hardware with up-to-date microcode
> will use retpoline in preference to IBRS, which will be a performance
> improvement for desktop and server systems which would previously always opt
> for IBRS over retpoline.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 for-4.11] x86/spec_ctrl: Updates to retpoline-safety decision making
  2018-04-19 14:25 ` [PATCH v2 " Andrew Cooper
  2018-04-19 15:02   ` Jan Beulich
@ 2018-04-19 15:15   ` Juergen Gross
  1 sibling, 0 replies; 9+ messages in thread
From: Juergen Gross @ 2018-04-19 15:15 UTC (permalink / raw)
  To: Andrew Cooper, Xen-devel; +Cc: Jan Beulich

On 19/04/18 16:25, Andrew Cooper wrote:
> All of this is as recommended by the Intel whitepaper:
> 
> https://software.intel.com/sites/default/files/managed/1d/46/Retpoline-A-Branch-Target-Injection-Mitigation.pdf
> 
> The 'RSB Alternative' bit in MSR_ARCH_CAPABILITIES may be set by a hypervisor
> to indicate that the virtual machine may migrate to a processor which isn't
> retpoline-safe.  Introduce a shortened name (to reduce code volume), treat it
> as authorative in retpoline_safe(), and print its value along with the other
> ARCH_CAPS bits.
> 
> The exact processor models which do have RSB semantics which fall back to BTB
> predictions are enumerated, and include Kabylake and Coffeelake.  Leave a
> printk() in the default case to help identify cases which aren't covered.
> 
> The exact microcode versions from Broadwell RSB-safety are taken from the
> referenced microcode update file (adjusting for the known-bad microcode
> versions).  Despite the exact wording of the text, it is only Broadwell
> processors which need a microcode check.
> 
> In practice, this means that all Broadwell hardware with up-to-date microcode
> will use retpoline in preference to IBRS, which will be a performance
> improvement for desktop and server systems which would previously always opt
> for IBRS over retpoline.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Release-acked-by: Juergen Gross <jgross@suse.com>


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH for-4.11] x86/spec_ctrl: Updates to retpoline-safety decision making
  2018-04-18 18:13 [PATCH for-4.11] x86/spec_ctrl: Updates to retpoline-safety decision making Andrew Cooper
  2018-04-19  9:00 ` Jan Beulich
  2018-04-19 14:25 ` [PATCH v2 " Andrew Cooper
@ 2018-04-23 21:16 ` Konrad Rzeszutek Wilk
  2018-04-23 23:09   ` Andrew Cooper
  2 siblings, 1 reply; 9+ messages in thread
From: Konrad Rzeszutek Wilk @ 2018-04-23 21:16 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Juergen Gross, Jan Beulich, Xen-devel

> diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
> index 5b5ec90..aff06f0 100644
> --- a/xen/arch/x86/spec_ctrl.c
> +++ b/xen/arch/x86/spec_ctrl.c
> @@ -113,12 +113,13 @@ static void __init print_details(enum ind_thunk thunk)
>      printk(XENLOG_DEBUG "Speculative mitigation facilities:\n");
>  
>      /* Hardware features which pertain to speculative mitigations. */
> -    printk(XENLOG_DEBUG "  Hardware features:%s%s%s%s%s\n",
> +    printk(XENLOG_DEBUG "  Hardware features:%s%s%s%s%s%s\n",
>             (_7d0 & cpufeat_mask(X86_FEATURE_IBRSB)) ? " IBRS/IBPB" : "",
>             (_7d0 & cpufeat_mask(X86_FEATURE_STIBP)) ? " STIBP"     : "",
>             (e8b  & cpufeat_mask(X86_FEATURE_IBPB))  ? " IBPB"      : "",
>             (caps & ARCH_CAPABILITIES_IBRS_ALL)      ? " IBRS_ALL"  : "",
> -           (caps & ARCH_CAPABILITIES_RDCL_NO)       ? " RDCL_NO"   : "");
> +           (caps & ARCH_CAPABILITIES_RDCL_NO)       ? " RDCL_NO"   : "",
> +           (caps & ARCH_CAPS_RSBA)                  ? " RBSA"      : "");

You have RSBA on the left and RBSA on the right. Which one is it?

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH for-4.11] x86/spec_ctrl: Updates to retpoline-safety decision making
  2018-04-23 21:16 ` [PATCH " Konrad Rzeszutek Wilk
@ 2018-04-23 23:09   ` Andrew Cooper
  0 siblings, 0 replies; 9+ messages in thread
From: Andrew Cooper @ 2018-04-23 23:09 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: Juergen Gross, Jan Beulich, Xen-devel

On 23/04/2018 22:16, Konrad Rzeszutek Wilk wrote:
>> diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
>> index 5b5ec90..aff06f0 100644
>> --- a/xen/arch/x86/spec_ctrl.c
>> +++ b/xen/arch/x86/spec_ctrl.c
>> @@ -113,12 +113,13 @@ static void __init print_details(enum ind_thunk thunk)
>>      printk(XENLOG_DEBUG "Speculative mitigation facilities:\n");
>>  
>>      /* Hardware features which pertain to speculative mitigations. */
>> -    printk(XENLOG_DEBUG "  Hardware features:%s%s%s%s%s\n",
>> +    printk(XENLOG_DEBUG "  Hardware features:%s%s%s%s%s%s\n",
>>             (_7d0 & cpufeat_mask(X86_FEATURE_IBRSB)) ? " IBRS/IBPB" : "",
>>             (_7d0 & cpufeat_mask(X86_FEATURE_STIBP)) ? " STIBP"     : "",
>>             (e8b  & cpufeat_mask(X86_FEATURE_IBPB))  ? " IBPB"      : "",
>>             (caps & ARCH_CAPABILITIES_IBRS_ALL)      ? " IBRS_ALL"  : "",
>> -           (caps & ARCH_CAPABILITIES_RDCL_NO)       ? " RDCL_NO"   : "");
>> +           (caps & ARCH_CAPABILITIES_RDCL_NO)       ? " RDCL_NO"   : "",
>> +           (caps & ARCH_CAPS_RSBA)                  ? " RBSA"      : "");
> You have RSBA on the left and RBSA on the right. Which one is it?

Bah - these are impossible acronyms to type.  It is RSB-Alternative, so
the LHS is correct and I'll submit a correction for the RHS.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-04-23 23:09 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-18 18:13 [PATCH for-4.11] x86/spec_ctrl: Updates to retpoline-safety decision making Andrew Cooper
2018-04-19  9:00 ` Jan Beulich
2018-04-19 10:26   ` Andrew Cooper
2018-04-19 12:04     ` Jan Beulich
2018-04-19 14:25 ` [PATCH v2 " Andrew Cooper
2018-04-19 15:02   ` Jan Beulich
2018-04-19 15:15   ` Juergen Gross
2018-04-23 21:16 ` [PATCH " Konrad Rzeszutek Wilk
2018-04-23 23:09   ` Andrew Cooper

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.