Re: [PATCH 03/22] KVM: x86: Support IBPB_BRTYPE and SBPB

[Josh Poimboeuf]

On Mon, Aug 21, 2023 at 04:35:41PM +0000, Sean Christopherson wrote:
> There are more wrinkles though.  KVM passes through MSR_IA32_PRED_CMD based on
> IBPB.  If hardware supports both IBPB and SBPB, but userspace does NOT exposes
> SBPB to the guest, then KVM will create a virtualization hole where the guest can
> write SBPB against userspace's wishes.  I haven't read up on SBPB enought o know
> whether or not that's problematic.
> 
> And conversely, if userspace expoes SBPB but not IBPB, then KVM will intercept
> writes to MSR_IA32_PRED_CMD and probably tank guest performance.  Again, I haven't
> paid attention enough to know if this is a reasonable configuration, i.e. whether
> or not it's worth caring about in KVM.
> 
> If the virtualization holes are deemed safe, then the easiest thing would be to
> treat MSR_IA32_PRED_CMD as existing if either IBPB or SBPB exists.  E.g.

I can't think of a reason why the holes wouldn't be safe, i.e. AFAICT
there's no harm in letting the guest do whatever type of barrier it
wants even if it's not technically supported by their configuration.

Question: if we're just always passing PRED_CMD through, what's the
point of having any PRED_CMD code in kvm_set_msr_common at all?

Also, since you're clearly more qualified to write this patch than me,
can I nominate you to do so? :-)

FWIW, the below is my qemu patch which worked for me in testing:

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 97ad229d8b..4b17f0152b 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1054,8 +1054,8 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
             NULL, NULL, NULL, NULL,
             NULL, NULL, NULL, NULL,
             NULL, NULL, NULL, NULL,
-            NULL, NULL, NULL, NULL,
-            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, "sbpb",
+            "ibpb-brtype", "srso-no", NULL, NULL,
         },
         .cpuid = { .eax = 0x80000021, .reg = R_EAX, },
         .tcg_features = 0,
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index e0771a1043..ff3c714214 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -969,6 +969,12 @@ uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
 #define CPUID_8000_0021_EAX_NULL_SEL_CLR_BASE    (1U << 6)
 /* Automatic IBRS */
 #define CPUID_8000_0021_EAX_AUTO_IBRS   (1U << 8)
+/* Selective Branch Prediction Barrier */
+#define CPUID_8000_0021_EAX_SBPB        (1U << 27)
+/* MSR_PRED_CMD[IBPB] flushes all branch type predictions */
+#define CPUID_8000_0021_EAX_IBPB_BRTYPE (1U << 28)
+/* CPU is not affected by SRSO */
+#define CPUID_8000_0021_EAX_SRSO_NO     (1U << 29)
 
 #define CPUID_XSAVE_XSAVEOPT   (1U << 0)
 #define CPUID_XSAVE_XSAVEC     (1U << 1)