[PATCH] x86/spec-ctrl: Skip RSB overwriting when safe to do so

[PATCH] x86/spec-ctrl: Skip RSB overwriting when safe to do so

[Andrew Cooper]

In some configurations, it is safe to not overwrite the RSB on entry to Xen.
Both Intel and AMD have guidelines in this area, because of the performance
difference it makes for native kernels.

A simple microperf test, measuring the amount of time a XENVER_version
hypercall takes, shows the following improvements:

  KabyLake:     -13.9175% +/- 6.85387%
  CoffeeLake-R:  -9.1183% +/- 5.04519%
  Milan:        -17.7803% +/- 1.29808%

This is best case improvement, because no real workloads are making
XENVER_version hypercalls in a tight loop.  However, this is the hypercall
used by PV kernels to force evtchn delivery if one is pending, so it is a
common hypercall to see, especially in dom0.

The avoidance of RSB-overwriting speeds up all interrupts, exceptions and
system calls from PV or Xen context.  RSB-overwriting is still required on
VMExit from HVM guests for now.

In terms of more realistic testing, LMBench in dom0 on an AMD Rome system
shows improvements across the board, with the best improvement at 8% for
simple syscall and simple write.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>

Backporting note.  This depends on the order of CR4 setup in __start_xen()
being ahead of the call to init_speculation().  I'm not sure how stable the
order was going backwards.

Raw stats data is here: https://paste.debian.net/hidden/0d59b024/

I was surprised that CoffeeLake's improvement wasn't better than KabyLake.
CFL-R has meltdown fixed vs KBL, so XPTI is a hit which ought to be visible in
this benchmark.  Also, MSR_SPEC_CTRL differes between the ucode-retrofitted
version and one of the early in-hardware versions.  We have yet to make use of
eIBRS so there is definitely an improvement to be had on CFL-R.
---
 xen/arch/x86/spec_ctrl.c | 67 ++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 57 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 739b7913ff86..750110e9dfcc 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -33,7 +33,7 @@
 /* Cmdline controls for Xen's alternative blocks. */
 static bool __initdata opt_msr_sc_pv = true;
 static bool __initdata opt_msr_sc_hvm = true;
-static bool __initdata opt_rsb_pv = true;
+static int8_t __initdata opt_rsb_pv = -1;
 static bool __initdata opt_rsb_hvm = true;
 static int8_t __initdata opt_md_clear_pv = -1;
 static int8_t __initdata opt_md_clear_hvm = -1;
@@ -554,6 +554,35 @@ static bool __init retpoline_safe(uint64_t caps)
     }
 }
 
+/*
+ * https://software.intel.com/content/www/us/en/develop/articles/software-security-guidance/technical-documentation/retpoline-branch-target-injection-mitigation.html
+ *
+ * Silvermont and Airmont based cores are 64bit but only have a 32bit wide
+ * RSB, which impacts the safety of using SMEP to avoid RSB-overwriting.
+ */
+static bool __init rsb_is_full_width(void)
+{
+    if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
+         boot_cpu_data.x86 != 6 )
+        return true;
+
+    switch ( boot_cpu_data.x86_model )
+    {
+    case 0x37: /* Baytrail / Valleyview (Silvermont) */
+    case 0x4a: /* Merrifield */
+    case 0x4c: /* Cherrytrail / Brasswell */
+    case 0x4d: /* Avaton / Rangely (Silvermont) */
+    case 0x5a: /* Moorefield */
+    case 0x5d: /* SoFIA 3G Granite/ES2.1 */
+    case 0x65: /* SoFIA LTE AOSP */
+    case 0x6e: /* Cougar Mountain */
+    case 0x75: /* Lightning Mountain */
+        return false;
+    }
+
+    return true;
+}
+
 /* Calculate whether this CPU speculates past #NM */
 static bool __init should_use_eager_fpu(void)
 {
@@ -992,18 +1021,36 @@ void __init init_speculation_mitigations(void)
         default_xen_spec_ctrl |= SPEC_CTRL_SSBD;
 
     /*
-     * PV guests can poison the RSB to any virtual address from which
-     * they can execute a call instruction.  This is necessarily outside
-     * of the Xen supervisor mappings.
+     * PV guests can create RSB entries for any linear address they control,
+     * which are outside of Xen's mappings.
+     *
+     * SMEP inhibits speculation to any user mappings, so in principle it is
+     * safe to not overwrite the RSB when SMEP is active.
+     *
+     * However, some caveats apply:
+     *
+     * 1) CALL instructions push the next sequential linear address into the
+     *    RSB, meaning that there is a boundary case at the user=>supervisor
+     *    split.  This can be compensated for by having an unmapped or NX
+     *    page, or an instruction which halts speculation.
      *
-     * With SMEP enabled, the processor won't speculate into user mappings.
-     * Therefore, in this case, we don't need to worry about poisoned entries
-     * from 64bit PV guests.
+     *    For Xen, the next sequential linear address is the start of M2P
+     *    (mapped NX), or a zapped hole (unmapped).
      *
-     * 32bit PV guest kernels run in ring 1, so use supervisor mappings.
-     * If a processors speculates to 32bit PV guest kernel mappings, it is
-     * speculating in 64bit supervisor mode, and can leak data.
+     * 2) 32bit PV kernels execute in Ring 1 and use supervisor mappings.
+     *    SMEP offers no protection in this case.
+     *
+     * 3) Some CPUs have RSBs which are not full width, which allow the
+     *    attacker's entries to alias Xen addresses.
+     *
+     * It is safe to turn off RSB stuffing when Xen is using SMEP itself, and
+     * 32bit PV guests are disabled, and when the RSB is full width.
      */
+    BUILD_BUG_ON(RO_MPT_VIRT_START != PML4_ADDR(256));
+    if ( opt_rsb_pv == -1 && boot_cpu_has(X86_FEATURE_XEN_SMEP) &&
+         !opt_pv32 && rsb_is_full_width() )
+        opt_rsb_pv = 0;
+
     if ( opt_rsb_pv )
     {
         setup_force_cpu_cap(X86_FEATURE_SC_RSB_PV);
-- 
2.11.0

Re: [PATCH] x86/spec-ctrl: Skip RSB overwriting when safe to do so

[Jan Beulich]

On 19.08.2021 18:26, Andrew Cooper wrote:
> In some configurations, it is safe to not overwrite the RSB on entry to Xen.
> Both Intel and AMD have guidelines in this area, because of the performance
> difference it makes for native kernels.

I don't think I've come across AMD's guidelines - would you happen to
have a pointer? Nevertheless ...

> A simple microperf test, measuring the amount of time a XENVER_version
> hypercall takes, shows the following improvements:
> 
>   KabyLake:     -13.9175% +/- 6.85387%
>   CoffeeLake-R:  -9.1183% +/- 5.04519%
>   Milan:        -17.7803% +/- 1.29808%
> 
> This is best case improvement, because no real workloads are making
> XENVER_version hypercalls in a tight loop.  However, this is the hypercall
> used by PV kernels to force evtchn delivery if one is pending, so it is a
> common hypercall to see, especially in dom0.
> 
> The avoidance of RSB-overwriting speeds up all interrupts, exceptions and
> system calls from PV or Xen context.  RSB-overwriting is still required on
> VMExit from HVM guests for now.
> 
> In terms of more realistic testing, LMBench in dom0 on an AMD Rome system
> shows improvements across the board, with the best improvement at 8% for
> simple syscall and simple write.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
albeit with one further remark / request:

> @@ -992,18 +1021,36 @@ void __init init_speculation_mitigations(void)
>          default_xen_spec_ctrl |= SPEC_CTRL_SSBD;
>  
>      /*
> -     * PV guests can poison the RSB to any virtual address from which
> -     * they can execute a call instruction.  This is necessarily outside
> -     * of the Xen supervisor mappings.
> +     * PV guests can create RSB entries for any linear address they control,
> +     * which are outside of Xen's mappings.
> +     *
> +     * SMEP inhibits speculation to any user mappings, so in principle it is
> +     * safe to not overwrite the RSB when SMEP is active.
> +     *
> +     * However, some caveats apply:
> +     *
> +     * 1) CALL instructions push the next sequential linear address into the
> +     *    RSB, meaning that there is a boundary case at the user=>supervisor
> +     *    split.  This can be compensated for by having an unmapped or NX
> +     *    page, or an instruction which halts speculation.
>       *
> -     * With SMEP enabled, the processor won't speculate into user mappings.
> -     * Therefore, in this case, we don't need to worry about poisoned entries
> -     * from 64bit PV guests.
> +     *    For Xen, the next sequential linear address is the start of M2P
> +     *    (mapped NX), or a zapped hole (unmapped).

IIUC you mean the compat M2P here - perhaps worth being explicit? I'm
also not sure why you use "zapped": Nothing can ever be mapped into
the non-canonical hole.

Jan

Re: [PATCH] x86/spec-ctrl: Skip RSB overwriting when safe to do so

[Andrew Cooper]

On 24/08/2021 14:04, Jan Beulich wrote:
> On 19.08.2021 18:26, Andrew Cooper wrote:
>> In some configurations, it is safe to not overwrite the RSB on entry to Xen.
>> Both Intel and AMD have guidelines in this area, because of the performance
>> difference it makes for native kernels.
> I don't think I've come across AMD's guidelines - would you happen to
> have a pointer?

APM Vol2 3.2.9 "Speculation Control MSRs"

The information about SMEP is in the final paragraph before describing
MSR_SPEC_CTRL.STIBP.

>  Nevertheless ...
>
>> A simple microperf test, measuring the amount of time a XENVER_version
>> hypercall takes, shows the following improvements:
>>
>>   KabyLake:     -13.9175% +/- 6.85387%
>>   CoffeeLake-R:  -9.1183% +/- 5.04519%
>>   Milan:        -17.7803% +/- 1.29808%
>>
>> This is best case improvement, because no real workloads are making
>> XENVER_version hypercalls in a tight loop.  However, this is the hypercall
>> used by PV kernels to force evtchn delivery if one is pending, so it is a
>> common hypercall to see, especially in dom0.
>>
>> The avoidance of RSB-overwriting speeds up all interrupts, exceptions and
>> system calls from PV or Xen context.  RSB-overwriting is still required on
>> VMExit from HVM guests for now.
>>
>> In terms of more realistic testing, LMBench in dom0 on an AMD Rome system
>> shows improvements across the board, with the best improvement at 8% for
>> simple syscall and simple write.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thanks.

> albeit with one further remark / request:
>
>> @@ -992,18 +1021,36 @@ void __init init_speculation_mitigations(void)
>>          default_xen_spec_ctrl |= SPEC_CTRL_SSBD;
>>  
>>      /*
>> -     * PV guests can poison the RSB to any virtual address from which
>> -     * they can execute a call instruction.  This is necessarily outside
>> -     * of the Xen supervisor mappings.
>> +     * PV guests can create RSB entries for any linear address they control,
>> +     * which are outside of Xen's mappings.
>> +     *
>> +     * SMEP inhibits speculation to any user mappings, so in principle it is
>> +     * safe to not overwrite the RSB when SMEP is active.
>> +     *
>> +     * However, some caveats apply:
>> +     *
>> +     * 1) CALL instructions push the next sequential linear address into the
>> +     *    RSB, meaning that there is a boundary case at the user=>supervisor
>> +     *    split.  This can be compensated for by having an unmapped or NX
>> +     *    page, or an instruction which halts speculation.
>>       *
>> -     * With SMEP enabled, the processor won't speculate into user mappings.
>> -     * Therefore, in this case, we don't need to worry about poisoned entries
>> -     * from 64bit PV guests.
>> +     *    For Xen, the next sequential linear address is the start of M2P
>> +     *    (mapped NX), or a zapped hole (unmapped).
> IIUC you mean the compat M2P here - perhaps worth being explicit? I'm
> also not sure why you use "zapped": Nothing can ever be mapped into
> the non-canonical hole.

The non-canonical hole doesn't behave as "a unmapped gap" when the
microarchitecture uses 48 bit pointers internally.

~Andrew

Re: [PATCH] x86/spec-ctrl: Skip RSB overwriting when safe to do so

[Jan Beulich]

On 25.08.2021 14:12, Andrew Cooper wrote:
> On 24/08/2021 14:04, Jan Beulich wrote:
>> On 19.08.2021 18:26, Andrew Cooper wrote:
>>> In some configurations, it is safe to not overwrite the RSB on entry to Xen.
>>> Both Intel and AMD have guidelines in this area, because of the performance
>>> difference it makes for native kernels.
>> I don't think I've come across AMD's guidelines - would you happen to
>> have a pointer?
> 
> APM Vol2 3.2.9 "Speculation Control MSRs"
> 
> The information about SMEP is in the final paragraph before describing
> MSR_SPEC_CTRL.STIBP.

Ah yes, thanks. Still need to get used to this now being in the PM rather
than in one or more separate docs. I now recall reading through this.

Jan

Re: [PATCH] x86/spec-ctrl: Skip RSB overwriting when safe to do so

[Andrew Cooper]

On 25/08/2021 15:36, Jan Beulich wrote:
> On 25.08.2021 14:12, Andrew Cooper wrote:
>> On 24/08/2021 14:04, Jan Beulich wrote:
>>> On 19.08.2021 18:26, Andrew Cooper wrote:
>>>> In some configurations, it is safe to not overwrite the RSB on entry to Xen.
>>>> Both Intel and AMD have guidelines in this area, because of the performance
>>>> difference it makes for native kernels.
>>> I don't think I've come across AMD's guidelines - would you happen to
>>> have a pointer?
>> APM Vol2 3.2.9 "Speculation Control MSRs"
>>
>> The information about SMEP is in the final paragraph before describing
>> MSR_SPEC_CTRL.STIBP.
> Ah yes, thanks. Still need to get used to this now being in the PM rather
> than in one or more separate docs. I now recall reading through this.

I'm still pushing Intel to add a section/chapter to the SDM, because
this is is all architectural functionality and interfaces.

~Andrew