[PATCH v2 0/3] Add Automatic IBRS support

[PATCH v2 0/3] Add Automatic IBRS support

[Alejandro Vallejo]

v2:
  * Renamed AUTOMATIC to AUTO
  * Style change in xen-cpuid.c
  * Swapped patches 2 and 3
  * Modified trampoline_efer from the BSP so APs use it during boot and S3
    wakeups pick it up.
  * Avoid the delay setting AutoIBRS

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 exposes the feature to HVM guests.

Patch 3 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.

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

 tools/libs/light/libxl_cpuid.c              |  1 +
 tools/misc/xen-cpuid.c                      |  1 +
 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/spec_ctrl.c                    | 45 ++++++++++++++++-----
 xen/include/public/arch-x86/cpufeatureset.h |  1 +
 8 files changed, 46 insertions(+), 14 deletions(-)

-- 
2.34.1

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

[Alejandro Vallejo]

This is an AMD feature to reduce the IBRS handling overhead. Once enabled,
processes running at CPL=0 are automatically IBRS-protected even if
SPEC_CTRL.IBRS is not set. Furthermore, the RAS/RSB is cleared on VMEXIT.

The feature is exposed in CPUID and toggled in EFER.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
v2:
  * Renamed AUTOMATIC -> AUTO
  * Newline removal in xen-cpuid.c
---
 tools/libs/light/libxl_cpuid.c              | 1 +
 tools/misc/xen-cpuid.c                      | 1 +
 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, 5 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..c65d9e01bf 100644
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -199,6 +199,7 @@ 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..ace31e3b1f 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_AUTO_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..3ac144100e 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(AUTO_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 v2 2/3] x86: Expose Automatic IBRS to guests

[Alejandro Vallejo]

Expose AutoIBRS to HVM guests. EFER is swapped by VMRUN, so Xen only has to
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>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
v2:
  * Moved to patch2 from v1/patch3
---
 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..2d6e4bb9c6 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.auto_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 3ac144100e..51d737a125 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(AUTO_IBRS,          11*32+ 8) /*   HW can handle IBRS on its own */
+XEN_CPUFEATURE(AUTO_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

[PATCH v2 3/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.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
v2:
  * Gated CPUID read to e21a by the presence the leaf
  * Add auto-ibrs to trampoline_efer if chosen
  * Remove smpboot.c modifications, as they are not needed after
    trampoline_efer is modified
  * Avoid the AutoIBRS delay as it doesn't provide any benefit.
---
 xen/arch/x86/spec_ctrl.c | 45 ++++++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 11 deletions(-)

diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 50d467f74c..36231e65fb 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. */
@@ -400,6 +400,8 @@ static void __init print_details(enum ind_thunk thunk)
         cpuid_count(7, 2, &tmp, &tmp, &tmp, &_7d2);
     if ( boot_cpu_data.extended_cpuid_level >= 0x80000008 )
         cpuid(0x80000008, &tmp, &e8b, &tmp, &tmp);
+    if ( boot_cpu_data.extended_cpuid_level >= 0x80000021 )
+        cpuid(0x80000021, &e21a, &tmp, &tmp, &tmp);
     if ( cpu_has_arch_caps )
         rdmsrl(MSR_ARCH_CAPABILITIES, caps);
 
@@ -430,11 +432,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_AUTO_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 +471,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 +1155,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 +1367,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
@@ -1594,6 +1606,17 @@ void __init init_speculation_mitigations(void)
             barrier();
         }
 
+        /*
+         * If we're to use AutoIBRS, then set it now for the BSP and mark
+         * it in trampoline_efer so it's picked up by the wakeup code. It
+         * will be used while starting up the APs and during S3 wakeups.
+         */
+        if ( ibrs && cpu_has_auto_ibrs )
+        {
+            write_efer(read_efer() | EFER_AIBRSE);
+            bootsym(trampoline_efer) |= EFER_AIBRSE;
+        }
+
         val = bsp_delay_spec_ctrl ? 0 : default_xen_spec_ctrl;
 
         wrmsrl(MSR_SPEC_CTRL, val);
-- 
2.34.1

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

[Andrew Cooper]

On 30/05/2023 2:58 pm, Alejandro Vallejo wrote:
> This is an AMD feature to reduce the IBRS handling overhead. Once enabled,
> processes running at CPL=0 are automatically IBRS-protected even if
> SPEC_CTRL.IBRS is not set. Furthermore, the RAS/RSB is cleared on VMEXIT.
>
> The feature is exposed in CPUID and toggled in EFER.
>
> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, but...

> diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
> index 777041425e..3ac144100e 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(AUTO_IBRS,          11*32+ 8) /*   HW can handle IBRS on its own */

... I've changed this on commit to just "Automatic IBRS".  The behaviour
is more far complicated than this, and anyone who wants to know needs to
read the manual extra carefully.

For one, there's a behaviour which depends on whether SEV-SNP was
enabled in firmware...

~Andrew

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

[Andrew Cooper]

On 30/05/2023 2:58 pm, Alejandro Vallejo wrote:
> Expose AutoIBRS to HVM guests. EFER is swapped by VMRUN, so Xen only has to
> 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>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

I've committed this, but made two tweaks to the commit message.  First,
"x86/hvm" in the subject because it's important context at a glance.

Second, I've adjusted the bit about PV guests.  The reason why we can't
expose it yet is because Xen doesn't currently context switch EFER
between PV guests.

~Andrew

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

[Alejandro Vallejo]

On Tue, May 30, 2023 at 06:29:14PM +0100, Andrew Cooper wrote:
> ... I've changed this on commit to just "Automatic IBRS".  The behaviour
> is more far complicated than this, and anyone who wants to know needs to
> read the manual extra carefully.
> 
> For one, there's a behaviour which depends on whether SEV-SNP was
> enabled in firmware...
> 
> ~Andrew
Ack.

Alejandro

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

[Alejandro Vallejo]

On Tue, May 30, 2023 at 06:31:03PM +0100, Andrew Cooper wrote:
> I've committed this, but made two tweaks to the commit message.  First,
> "x86/hvm" in the subject because it's important context at a glance.
Sure, that makes sense.

> Second, I've adjusted the bit about PV guests.  The reason why we can't
> expose it yet is because Xen doesn't currently context switch EFER
> between PV guests.
> 
> ~Andrew
We could of course context switch EFER sensibly, but what would that mean
for Automatic IBRS? It can't be trivially used for domain-to-domain
isolation because every domain is in a co-equal protection level. Is there
a non-obvious edge that exposing some interface to it gives for PV? The
only useful case I can think of is PVH, and that seems to be subsumed by
HVM.

Alejandro

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

[Andrew Cooper]

On 31/05/2023 10:01 am, Alejandro Vallejo wrote:
> On Tue, May 30, 2023 at 06:31:03PM +0100, Andrew Cooper wrote:
>> I've committed this, but made two tweaks to the commit message.  First,
>> "x86/hvm" in the subject because it's important context at a glance.
> Sure, that makes sense.
>
>> Second, I've adjusted the bit about PV guests.  The reason why we can't
>> expose it yet is because Xen doesn't currently context switch EFER
>> between PV guests.
>>
>> ~Andrew
> We could of course context switch EFER sensibly, but what would that mean
> for Automatic IBRS? It can't be trivially used for domain-to-domain
> isolation because every domain is in a co-equal protection level. Is there
> a non-obvious edge that exposing some interface to it gives for PV? The
> only useful case I can think of is PVH, and that seems to be subsumed by
> HVM.

Hence why it's fine to not worry about PV for now.

Right now, when we decide to use IBRS on AMD, we set it unilaterally. 
This turns out to be better performance than flipping it on privilege
changes (whether that's non-Xen <-> Xen, or guest user <-> kernel).

PV guests are obscure corner cases these days, and fall outside of
anything the hardware vendors care about when it comes to prediction
mode.  The only sane option is to have Xen explicitly tell the the PV
guest what Xen is doing, and let the guest decide if it wants to do
anything further in terms of protections.

~Andrew

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

[Jan Beulich]

On 30.05.2023 15:58, Alejandro Vallejo wrote:
> @@ -1150,15 +1155,20 @@ void __init init_speculation_mitigations(void)
>      }
>      else
>      {
> -        /*
> -         * Evaluate the safest Branch Target Injection mitigations to use.
> -         * First, begin with compiler-aided mitigations.
> -         */

This is the only place where BTI is spelled out, so ...

> -        if ( IS_ENABLED(CONFIG_INDIRECT_THUNK) )
> +        /* Evaluate the safest BTI mitigations with lowest overhead */

... I'd like to ask that you replace the acronym here. Then
Reviewed-by: Jan Beulich <jbeulich@suse.com>

> @@ -1357,7 +1367,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);

I'll need to remember to adjust "x86: limit issuing of IBPB during context
switch" once this change has gone in, as there's a use of the bit for
other than alternatives patching.

Jan

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

[Andrew Cooper]

On 01/06/2023 11:35 am, Jan Beulich wrote:
> On 30.05.2023 15:58, Alejandro Vallejo wrote:
>> @@ -1150,15 +1155,20 @@ void __init init_speculation_mitigations(void)
>>      }
>>      else
>>      {
>> -        /*
>> -         * Evaluate the safest Branch Target Injection mitigations to use.
>> -         * First, begin with compiler-aided mitigations.
>> -         */
> This is the only place where BTI is spelled out, so ...
>
>> -        if ( IS_ENABLED(CONFIG_INDIRECT_THUNK) )
>> +        /* Evaluate the safest BTI mitigations with lowest overhead */
> ... I'd like to ask that you replace the acronym here. Then
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>
>> @@ -1357,7 +1367,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);
> I'll need to remember to adjust "x86: limit issuing of IBPB during context
> switch" once this change has gone in, as there's a use of the bit for
> other than alternatives patching.

Please hold off for the moment.  I think I've got a cleanup patch in
mind which ought to simplify this substantially, but I'll need a bit of
time to experiment.

~Andrew