diff mbox series

[2/3] x86: Add support for AMD's Automatic IBRS

Message ID 20230526150044.31553-3-alejandro.vallejo@cloud.com (mailing list archive)
State Superseded
Headers show
Series Add Automatic IBRS support | expand

Commit Message

Alejandro Vallejo May 26, 2023, 3 p.m. UTC
In cases where AutoIBRS is supported by the host:

* Prefer AutoIBRS to retpolines as BTI mitigation in heuristics
  calculations.
* Always enable AutoIBRS if IBRS is chosen as a BTI mitigation.
* Avoid stuffing the RAS/RSB on VMEXIT if AutoIBRS is enabled.
* Delay setting AutoIBRS until after dom0 is set up, just like setting
  SPEC_CTRL.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
 xen/arch/x86/setup.c     |  3 +++
 xen/arch/x86/smpboot.c   |  3 +++
 xen/arch/x86/spec_ctrl.c | 52 ++++++++++++++++++++++++++++------------
 3 files changed, 43 insertions(+), 15 deletions(-)

Comments

Jason Andryuk May 26, 2023, 4:28 p.m. UTC | #1
On Fri, May 26, 2023 at 11:01 AM Alejandro Vallejo
<alejandro.vallejo@cloud.com> wrote:
>
> In cases where AutoIBRS is supported by the host:
>
> * Prefer AutoIBRS to retpolines as BTI mitigation in heuristics
>   calculations.
> * Always enable AutoIBRS if IBRS is chosen as a BTI mitigation.
> * Avoid stuffing the RAS/RSB on VMEXIT if AutoIBRS is enabled.
> * Delay setting AutoIBRS until after dom0 is set up, just like setting
>   SPEC_CTRL.
>
> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
> ---

> --- a/xen/arch/x86/spec_ctrl.c
> +++ b/xen/arch/x86/spec_ctrl.c
> @@ -390,7 +390,7 @@ custom_param("pv-l1tf", parse_pv_l1tf);

> @@ -399,7 +399,10 @@ static void __init print_details(enum ind_thunk thunk)
>      if ( max >= 2 )
>          cpuid_count(7, 2, &tmp, &tmp, &tmp, &_7d2);
>      if ( boot_cpu_data.extended_cpuid_level >= 0x80000008 )
> +    {
>          cpuid(0x80000008, &tmp, &e8b, &tmp, &tmp);
> +        cpuid(0x80000021, &e21a, &tmp, &tmp, &tmp);
> +    }

Do you need to check boot_cpu_data.extended_cpuid_level >= 0x80000021?

Regards,
Jason
Alejandro Vallejo May 26, 2023, 5:18 p.m. UTC | #2
On Fri, May 26, 2023 at 12:28:39PM -0400, Jason Andryuk wrote:
> >      if ( boot_cpu_data.extended_cpuid_level >= 0x80000008 )
> > +    {
> >          cpuid(0x80000008, &tmp, &e8b, &tmp, &tmp);
> > +        cpuid(0x80000021, &e21a, &tmp, &tmp, &tmp);
> > +    }
> 
> Do you need to check boot_cpu_data.extended_cpuid_level >= 0x80000021?
> 
> Regards,
> Jason

True that, yes. Will do on v2.

Alejandro
Jan Beulich May 30, 2023, 8:25 a.m. UTC | #3
On 26.05.2023 17:00, Alejandro Vallejo wrote:
> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -376,6 +376,9 @@ void start_secondary(void *unused)
>      {
>          wrmsrl(MSR_SPEC_CTRL, default_xen_spec_ctrl);
>          info->last_spec_ctrl = default_xen_spec_ctrl;
> +
> +        if ( cpu_has_auto_ibrs && (default_xen_spec_ctrl & SPEC_CTRL_IBRS) )
> +            write_efer(read_efer() | EFER_AIBRSE);
>      }

Did you consider using trampoline_efer instead, which would then also take
care of the S3 resume path (which otherwise I think you'd also need to
fiddle with)?

Jan
Alejandro Vallejo May 30, 2023, 9:57 a.m. UTC | #4
On Tue, May 30, 2023 at 10:25:36AM +0200, Jan Beulich wrote:
> On 26.05.2023 17:00, Alejandro Vallejo wrote:
> > --- a/xen/arch/x86/smpboot.c
> > +++ b/xen/arch/x86/smpboot.c
> > @@ -376,6 +376,9 @@ void start_secondary(void *unused)
> >      {
> >          wrmsrl(MSR_SPEC_CTRL, default_xen_spec_ctrl);
> >          info->last_spec_ctrl = default_xen_spec_ctrl;
> > +
> > +        if ( cpu_has_auto_ibrs && (default_xen_spec_ctrl & SPEC_CTRL_IBRS) )
> > +            write_efer(read_efer() | EFER_AIBRSE);
> >      }
> 
> Did you consider using trampoline_efer instead, which would then also take
> care of the S3 resume path (which otherwise I think you'd also need to
> fiddle with)?
> 
> Jan
I didn't because I didn't know about it. Good call though, it's indeed a
better place for it. Will do on v2.

Alejandro
Alejandro Vallejo May 30, 2023, 10:17 a.m. UTC | #5
On Fri, May 26, 2023 at 04:00:43PM +0100, Alejandro Vallejo wrote:
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 74e3915a4d..09cfef2676 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -2036,6 +2036,9 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>          barrier();
>          wrmsrl(MSR_SPEC_CTRL, default_xen_spec_ctrl);
>          info->last_spec_ctrl = default_xen_spec_ctrl;
> +
> +        if ( cpu_has_auto_ibrs && (default_xen_spec_ctrl & SPEC_CTRL_IBRS) )
> +            write_efer(read_efer() | EFER_AIBRSE);
>      }
After thinking things through I think I'll get rid of this "delay AutoIBRS"
setting. I initially thought there might have been some handshake between
the newly created dom0 and Xen on this path, but that doesn't seem to be
the case. If so, I can remove some of this disjoint logic by setting AIBRSE
on the local EFER and trampoline_efer during init_speculation_mitigation.
Then the BSP will have the correct setting, the APs will pick it up on boot
and S3 wakeups will do the right thing too.

I'm assuming then bsp_delay_spec_ctrl is there mostly to delay STIBP for as
long as possible?

Alejandro
diff mbox series

Patch

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 74e3915a4d..09cfef2676 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -2036,6 +2036,9 @@  void __init noreturn __start_xen(unsigned long mbi_p)
         barrier();
         wrmsrl(MSR_SPEC_CTRL, default_xen_spec_ctrl);
         info->last_spec_ctrl = default_xen_spec_ctrl;
+
+        if ( cpu_has_auto_ibrs && (default_xen_spec_ctrl & SPEC_CTRL_IBRS) )
+            write_efer(read_efer() | EFER_AIBRSE);
     }
 
     /* Copy the cpu info block, and move onto the BSP stack. */
diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index cf9bb220f9..1d52c1dd0a 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -376,6 +376,9 @@  void start_secondary(void *unused)
     {
         wrmsrl(MSR_SPEC_CTRL, default_xen_spec_ctrl);
         info->last_spec_ctrl = default_xen_spec_ctrl;
+
+        if ( cpu_has_auto_ibrs && (default_xen_spec_ctrl & SPEC_CTRL_IBRS) )
+            write_efer(read_efer() | EFER_AIBRSE);
     }
     update_mcu_opt_ctrl();
 
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 50d467f74c..c887fc3df9 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -390,7 +390,7 @@  custom_param("pv-l1tf", parse_pv_l1tf);
 
 static void __init print_details(enum ind_thunk thunk)
 {
-    unsigned int _7d0 = 0, _7d2 = 0, e8b = 0, max = 0, tmp;
+    unsigned int _7d0 = 0, _7d2 = 0, e8b = 0, e21a = 0, max = 0, tmp;
     uint64_t caps = 0;
 
     /* Collect diagnostics about available mitigations. */
@@ -399,7 +399,10 @@  static void __init print_details(enum ind_thunk thunk)
     if ( max >= 2 )
         cpuid_count(7, 2, &tmp, &tmp, &tmp, &_7d2);
     if ( boot_cpu_data.extended_cpuid_level >= 0x80000008 )
+    {
         cpuid(0x80000008, &tmp, &e8b, &tmp, &tmp);
+        cpuid(0x80000021, &e21a, &tmp, &tmp, &tmp);
+    }
     if ( cpu_has_arch_caps )
         rdmsrl(MSR_ARCH_CAPABILITIES, caps);
 
@@ -430,11 +433,12 @@  static void __init print_details(enum ind_thunk thunk)
            (e8b  & cpufeat_mask(X86_FEATURE_IBPB_RET))       ? " IBPB_RET"       : "");
 
     /* Hardware features which need driving to mitigate issues. */
-    printk("  Hardware features:%s%s%s%s%s%s%s%s%s%s%s%s\n",
+    printk("  Hardware features:%s%s%s%s%s%s%s%s%s%s%s%s%s\n",
            (e8b  & cpufeat_mask(X86_FEATURE_IBPB)) ||
            (_7d0 & cpufeat_mask(X86_FEATURE_IBRSB))          ? " IBPB"           : "",
            (e8b  & cpufeat_mask(X86_FEATURE_IBRS)) ||
            (_7d0 & cpufeat_mask(X86_FEATURE_IBRSB))          ? " IBRS"           : "",
+           (e21a & cpufeat_mask(X86_FEATURE_AUTOMATIC_IBRS)) ? " AUTO_IBRS"      : "",
            (e8b  & cpufeat_mask(X86_FEATURE_AMD_STIBP)) ||
            (_7d0 & cpufeat_mask(X86_FEATURE_STIBP))          ? " STIBP"          : "",
            (e8b  & cpufeat_mask(X86_FEATURE_AMD_SSBD)) ||
@@ -468,7 +472,9 @@  static void __init print_details(enum ind_thunk thunk)
            thunk == THUNK_JMP       ? "JMP" : "?",
            (!boot_cpu_has(X86_FEATURE_IBRSB) &&
             !boot_cpu_has(X86_FEATURE_IBRS))         ? "No" :
-           (default_xen_spec_ctrl & SPEC_CTRL_IBRS)  ? "IBRS+" :  "IBRS-",
+           (cpu_has_auto_ibrs &&
+            (default_xen_spec_ctrl & SPEC_CTRL_IBRS)) ? "AUTO_IBRS+" :
+            (default_xen_spec_ctrl & SPEC_CTRL_IBRS)  ? "IBRS+" : "IBRS-",
            (!boot_cpu_has(X86_FEATURE_STIBP) &&
             !boot_cpu_has(X86_FEATURE_AMD_STIBP))    ? "" :
            (default_xen_spec_ctrl & SPEC_CTRL_STIBP) ? " STIBP+" : " STIBP-",
@@ -1150,15 +1156,20 @@  void __init init_speculation_mitigations(void)
     }
     else
     {
-        /*
-         * Evaluate the safest Branch Target Injection mitigations to use.
-         * First, begin with compiler-aided mitigations.
-         */
-        if ( IS_ENABLED(CONFIG_INDIRECT_THUNK) )
+        /* Evaluate the safest BTI mitigations with lowest overhead */
+        if ( cpu_has_auto_ibrs )
+        {
+            /*
+             * We'd rather use Automatic IBRS if present. It helps in order
+             * to avoid stuffing the RSB manually on every VMEXIT.
+             */
+            ibrs = true;
+        }
+        else if ( IS_ENABLED(CONFIG_INDIRECT_THUNK) )
         {
             /*
-             * On all hardware, we'd like to use retpoline in preference to
-             * IBRS, but only if it is safe on this hardware.
+             * Otherwise, we'd like to use retpoline in preference to
+             * plain IBRS, but only if it is safe on this hardware.
              */
             if ( retpoline_safe() )
                 thunk = THUNK_RETPOLINE;
@@ -1357,7 +1368,9 @@  void __init init_speculation_mitigations(void)
      */
     if ( opt_rsb_hvm )
     {
-        setup_force_cpu_cap(X86_FEATURE_SC_RSB_HVM);
+        /* Automatic IBRS wipes the RSB for us on VMEXIT */
+        if ( !(ibrs && cpu_has_auto_ibrs) )
+            setup_force_cpu_cap(X86_FEATURE_SC_RSB_HVM);
 
         /*
          * For SVM, Xen's RSB safety actions are performed before STGI, so
@@ -1582,17 +1595,26 @@  void __init init_speculation_mitigations(void)
 
         bsp_delay_spec_ctrl = !cpu_has_hypervisor && default_xen_spec_ctrl;
 
-        /*
-         * If delaying MSR_SPEC_CTRL setup, use the same mechanism as
-         * spec_ctrl_enter_idle(), by using a shadow value of zero.
-         */
         if ( bsp_delay_spec_ctrl )
         {
+            /*
+             * If delaying MSR_SPEC_CTRL setup, use the same mechanism as
+             * spec_ctrl_enter_idle(), by using a shadow value of zero.
+             */
             info->shadow_spec_ctrl = 0;
             barrier();
             info->spec_ctrl_flags |= SCF_use_shadow;
             barrier();
         }
+        else if ( ibrs && cpu_has_auto_ibrs )
+        {
+            /*
+             * If we're not delaying setting SPEC_CTRL there's no need to
+             * delay setting Automatic IBRS either. Flip the toggle if
+             * supported and IBRS is expected.
+             */
+            write_efer(read_efer() | EFER_AIBRSE);
+        }
 
         val = bsp_delay_spec_ctrl ? 0 : default_xen_spec_ctrl;