[PATCH 0/3] Add Automatic IBRS support

[PATCH 0/3] Add Automatic IBRS support

[Alejandro Vallejo]

Adds support for AMD's Automatic IBRS. It's a set-and-forget feature that
prevents lower privileged executions from affecting speculations of higher
privileged executions, so retpolines are not required. Furthermore, it
clears the RSB upon VMEXIT, so we can avoid doing it if the feature is
present.

Patch 1 adds the relevant bit definitions for CPUID and EFER.

Patch 2 Hooks up AutoIBRS to spec_ctrl. so it's used when IBRS is picked.
        It also tweaks the heuristics so AutoIBRS is preferred over
        retpolines as BTI mitigation. This is enough to protect Xen.

Patch 3 exposes the feature to HVM guests.

Alejandro Vallejo (3):
  x86: Add bit definitions for Automatic IBRS
  x86: Add support for AMD's Automatic IBRS
  x86: Expose Automatic IBRS to guests

 tools/libs/light/libxl_cpuid.c              |  1 +
 tools/misc/xen-cpuid.c                      |  2 +
 xen/arch/x86/hvm/hvm.c                      |  3 ++
 xen/arch/x86/include/asm/cpufeature.h       |  1 +
 xen/arch/x86/include/asm/msr-index.h        |  4 +-
 xen/arch/x86/pv/emul-priv-op.c              |  4 +-
 xen/arch/x86/setup.c                        |  3 ++
 xen/arch/x86/smpboot.c                      |  3 ++
 xen/arch/x86/spec_ctrl.c                    | 52 +++++++++++++++------
 xen/include/public/arch-x86/cpufeatureset.h |  1 +
 10 files changed, 56 insertions(+), 18 deletions(-)

-- 
2.34.1

[PATCH 1/3] x86: Add bit definitions for Automatic IBRS

[Alejandro Vallejo]

This is AMD's version of Intel's Enhanced IBRS. Exposed in CPUID
and toggled in EFER.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
 tools/libs/light/libxl_cpuid.c              | 1 +
 tools/misc/xen-cpuid.c                      | 2 ++
 xen/arch/x86/include/asm/cpufeature.h       | 1 +
 xen/arch/x86/include/asm/msr-index.h        | 1 +
 xen/include/public/arch-x86/cpufeatureset.h | 1 +
 5 files changed, 6 insertions(+)

diff --git a/tools/libs/light/libxl_cpuid.c b/tools/libs/light/libxl_cpuid.c
index cca0f19d93..f5ce9f9795 100644
--- a/tools/libs/light/libxl_cpuid.c
+++ b/tools/libs/light/libxl_cpuid.c
@@ -317,6 +317,7 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list *cpuid, const char* str)
 
         {"lfence+",      0x80000021, NA, CPUID_REG_EAX,  2,  1},
         {"nscb",         0x80000021, NA, CPUID_REG_EAX,  6,  1},
+        {"auto-ibrs",    0x80000021, NA, CPUID_REG_EAX,  8,  1},
         {"cpuid-user-dis", 0x80000021, NA, CPUID_REG_EAX, 17, 1},
 
         {"maxhvleaf",    0x40000000, NA, CPUID_REG_EAX,  0,  8},
diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
index 5d0c64a45f..e487885a5c 100644
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -200,6 +200,8 @@ static const char *const str_e21a[32] =
     [ 2] = "lfence+",
     [ 6] = "nscb",
 
+    [ 8] = "auto-ibrs",
+
     /* 16 */                [17] = "cpuid-user-dis",
 };
 
diff --git a/xen/arch/x86/include/asm/cpufeature.h b/xen/arch/x86/include/asm/cpufeature.h
index 50235f098d..d5947a6826 100644
--- a/xen/arch/x86/include/asm/cpufeature.h
+++ b/xen/arch/x86/include/asm/cpufeature.h
@@ -161,6 +161,7 @@ static inline bool boot_cpu_has(unsigned int feat)
 #define cpu_has_amd_ssbd        boot_cpu_has(X86_FEATURE_AMD_SSBD)
 #define cpu_has_virt_ssbd       boot_cpu_has(X86_FEATURE_VIRT_SSBD)
 #define cpu_has_ssb_no          boot_cpu_has(X86_FEATURE_SSB_NO)
+#define cpu_has_auto_ibrs       boot_cpu_has(X86_FEATURE_AUTOMATIC_IBRS)
 
 /* CPUID level 0x00000007:0.edx */
 #define cpu_has_avx512_4vnniw   boot_cpu_has(X86_FEATURE_AVX512_4VNNIW)
diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h
index 082fb2e0d9..73d0af2615 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -175,6 +175,7 @@
 #define  EFER_NXE                           (_AC(1, ULL) << 11) /* No Execute Enable */
 #define  EFER_SVME                          (_AC(1, ULL) << 12) /* Secure Virtual Machine Enable */
 #define  EFER_FFXSE                         (_AC(1, ULL) << 14) /* Fast FXSAVE/FXRSTOR */
+#define  EFER_AIBRSE                        (_AC(1, ULL) << 21) /* Automatic IBRS Enable */
 
 #define EFER_KNOWN_MASK \
     (EFER_SCE | EFER_LME | EFER_LMA | EFER_NXE | EFER_SVME | EFER_FFXSE)
diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
index 777041425e..e3952f62bc 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -287,6 +287,7 @@ XEN_CPUFEATURE(AVX_IFMA,     10*32+23) /*A  AVX-IFMA Instructions */
 /* AMD-defined CPU features, CPUID level 0x80000021.eax, word 11 */
 XEN_CPUFEATURE(LFENCE_DISPATCH,    11*32+ 2) /*A  LFENCE always serializing */
 XEN_CPUFEATURE(NSCB,               11*32+ 6) /*A  Null Selector Clears Base (and limit too) */
+XEN_CPUFEATURE(AUTOMATIC_IBRS,     11*32+ 8) /*   HW can handle IBRS on its own */
 XEN_CPUFEATURE(CPUID_USER_DIS,     11*32+17) /*   CPUID disable for CPL > 0 software */
 
 /* Intel-defined CPU features, CPUID level 0x00000007:1.ebx, word 12 */
-- 
2.34.1

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

[Alejandro Vallejo]

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(-)

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;
 
-- 
2.34.1

[PATCH 3/3] x86: Expose Automatic IBRS to guests

[Alejandro Vallejo]

Expose AutoIBRS to HVM guests, because they can just use it. Make sure
writes to EFER:AIBRSE are gated on the feature being exposed. Also hide
EFER:AIBRSE from PV guests as they have no say in the matter.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
 xen/arch/x86/hvm/hvm.c                      | 3 +++
 xen/arch/x86/include/asm/msr-index.h        | 3 ++-
 xen/arch/x86/pv/emul-priv-op.c              | 4 ++--
 xen/include/public/arch-x86/cpufeatureset.h | 2 +-
 4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index d7d31b5393..07f39d5e61 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -936,6 +936,9 @@ const char *hvm_efer_valid(const struct vcpu *v, uint64_t value,
     if ( (value & EFER_FFXSE) && !p->extd.ffxsr )
         return "FFXSE without feature";
 
+    if ( (value & EFER_AIBRSE) && !p->extd.automatic_ibrs )
+        return "AutoIBRS without feature";
+
     return NULL;
 }
 
diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h
index 73d0af2615..49cb334c61 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -178,7 +178,8 @@
 #define  EFER_AIBRSE                        (_AC(1, ULL) << 21) /* Automatic IBRS Enable */
 
 #define EFER_KNOWN_MASK \
-    (EFER_SCE | EFER_LME | EFER_LMA | EFER_NXE | EFER_SVME | EFER_FFXSE)
+    (EFER_SCE | EFER_LME | EFER_LMA | EFER_NXE | EFER_SVME | EFER_FFXSE | \
+     EFER_AIBRSE)
 
 #define MSR_STAR                            0xc0000081 /* legacy mode SYSCALL target */
 #define MSR_LSTAR                           0xc0000082 /* long mode SYSCALL target */
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 8a4ef9c35e..142bc4818c 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -853,8 +853,8 @@ static uint64_t guest_efer(const struct domain *d)
 {
     uint64_t val;
 
-    /* Hide unknown bits, and unconditionally hide SVME from guests. */
-    val = read_efer() & EFER_KNOWN_MASK & ~EFER_SVME;
+    /* Hide unknown bits, and unconditionally hide SVME and AIBRSE from guests. */
+    val = read_efer() & EFER_KNOWN_MASK & ~(EFER_SVME | EFER_AIBRSE);
     /*
      * Hide the 64-bit features from 32-bit guests.  SCE has
      * vendor-dependent behaviour.
diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
index e3952f62bc..42401e9452 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -287,7 +287,7 @@ XEN_CPUFEATURE(AVX_IFMA,     10*32+23) /*A  AVX-IFMA Instructions */
 /* AMD-defined CPU features, CPUID level 0x80000021.eax, word 11 */
 XEN_CPUFEATURE(LFENCE_DISPATCH,    11*32+ 2) /*A  LFENCE always serializing */
 XEN_CPUFEATURE(NSCB,               11*32+ 6) /*A  Null Selector Clears Base (and limit too) */
-XEN_CPUFEATURE(AUTOMATIC_IBRS,     11*32+ 8) /*   HW can handle IBRS on its own */
+XEN_CPUFEATURE(AUTOMATIC_IBRS,     11*32+ 8) /*S  HW can handle IBRS on its own */
 XEN_CPUFEATURE(CPUID_USER_DIS,     11*32+17) /*   CPUID disable for CPL > 0 software */
 
 /* Intel-defined CPU features, CPUID level 0x00000007:1.ebx, word 12 */
-- 
2.34.1

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

[Jason Andryuk]

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

Re: [PATCH 1/3] x86: Add bit definitions for Automatic IBRS

[Andrew Cooper]

On 26/05/2023 4:00 pm, Alejandro Vallejo wrote:
> This is AMD's version of Intel's Enhanced IBRS. Exposed in CPUID
> and toggled in EFER.

AIBRS and EIBRS are very much not the same, and I argued hard to not
have Linux confuse the too, but alas.

Don't mention EIBRS at all.

Simply "Auto IBRS is a new feature in AMD Zen4 CPUs and late, intended
to reduce the overhead involved with operating IBRS", or something along
these lines.

> diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
> index 5d0c64a45f..e487885a5c 100644
> --- a/tools/misc/xen-cpuid.c
> +++ b/tools/misc/xen-cpuid.c
> @@ -200,6 +200,8 @@ static const char *const str_e21a[32] =
>      [ 2] = "lfence+",
>      [ 6] = "nscb",
>  
> +    [ 8] = "auto-ibrs",
> +

This wants to be:

     [ 6] = "nscb",
+    [ 8] = "auto-ibrs",

as they are adjacent with names in two columns.  Gaps are only for
discontinuities in numbering.

> diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
> index 777041425e..e3952f62bc 100644
> --- a/xen/include/public/arch-x86/cpufeatureset.h
> +++ b/xen/include/public/arch-x86/cpufeatureset.h
> @@ -287,6 +287,7 @@ XEN_CPUFEATURE(AVX_IFMA,     10*32+23) /*A  AVX-IFMA Instructions */
>  /* AMD-defined CPU features, CPUID level 0x80000021.eax, word 11 */
>  XEN_CPUFEATURE(LFENCE_DISPATCH,    11*32+ 2) /*A  LFENCE always serializing */
>  XEN_CPUFEATURE(NSCB,               11*32+ 6) /*A  Null Selector Clears Base (and limit too) */
> +XEN_CPUFEATURE(AUTOMATIC_IBRS,     11*32+ 8) /*   HW can handle IBRS on its own */

Were possible, we want to use the same names.  AUTO_IBRS is fine here,
and shorter to use throughout Xen.

Furthermore, it must match the cpu_has_* name, and that's already in the
better form.

~Andrew

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

[Alejandro Vallejo]

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

Re: [PATCH 1/3] x86: Add bit definitions for Automatic IBRS

[Alejandro Vallejo]

On Fri, May 26, 2023 at 05:46:51PM +0100, Andrew Cooper wrote:
> AIBRS and EIBRS are very much not the same, and I argued hard to not
> have Linux confuse the too, but alas.
> 
> Don't mention EIBRS at all.
> 
> Simply "Auto IBRS is a new feature in AMD Zen4 CPUs and late, intended
> to reduce the overhead involved with operating IBRS", or something along
> these lines.
Sure. I did go out of my way to avoid ambiguityin the code, but it's true
the commit message is unfortunate.

> 
> > diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
> > index 5d0c64a45f..e487885a5c 100644
> > --- a/tools/misc/xen-cpuid.c
> > +++ b/tools/misc/xen-cpuid.c
> > @@ -200,6 +200,8 @@ static const char *const str_e21a[32] =
> >      [ 2] = "lfence+",
> >      [ 6] = "nscb",
> >  
> > +    [ 8] = "auto-ibrs",
> > +
> 
> This wants to be:
> 
>      [ 6] = "nscb",
> +    [ 8] = "auto-ibrs",
> 
> as they are adjacent with names in two columns.  Gaps are only for
> discontinuities in numbering.
>
> [...]
>
> Were possible, we want to use the same names.  AUTO_IBRS is fine here,
> and shorter to use throughout Xen.
Sure to both.

Alejandro

Re: [PATCH 3/3] x86: Expose Automatic IBRS to guests

[Andrew Cooper]

On 26/05/2023 4:00 pm, Alejandro Vallejo wrote:
> Expose AutoIBRS to HVM guests, because they can just use it. Make sure
> writes to EFER:AIBRSE are gated on the feature being exposed. Also hide
> EFER:AIBRSE from PV guests as they have no say in the matter.
>
> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>

It's worth saying "EFER is fully switched by VMRUN, so there's nothing
further for Xen to do in order for HVM guests to use AutoIBRS".

We can in principle support AutoIBRS on PV guests, but it's fine not to
for now.

This patch probably wants reordering to #2, because it is entirely
independent of what Xen is doing with AutoIBRS for spec safety.

It will need a minor rebase over the bit name shortening, but otherwise
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

[Jan Beulich]

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

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

[Alejandro Vallejo]

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

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

[Alejandro Vallejo]

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