openSUSE Build Service

Overview Repositories Revisions Requests Users Attributes Meta

File xsa435-0-12.patch of Package xen

From 817764ea33a44867f8b8a7bbd2fa4475ebe60de7 Mon Sep 17 00:00:00 2001
From: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Wed, 19 Jan 2022 19:55:02 +0000
Subject: x86/msr: Fix migration compatibility issue with MSR_SPEC_CTRL

This bug existed in early in 2018 between MSR_SPEC_CTRL arriving in microcode,
and SSBD arriving a few months later.  It went unnoticed presumably because
everyone was busy rebooting everything.

The same bug will reappear when adding PSFD support.

Clamp the guest MSR_SPEC_CTRL value to that permitted by CPUID on migrate.
The guest is already playing with reserved bits at this point, and clamping
the value will prevent a migration to a less capable host from failing.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit 969a57f73f6b011b2ebf4c0ab1715efc65837335)

--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -612,6 +612,24 @@ void __init init_guest_cpuid(void)
     calculate_hvm_max_policy();
 }
 
+/*
+ * Caller to confirm that MSR_SPEC_CTRL is available.  Intel and AMD have
+ * separate CPUID features for this functionality, but only set will be
+ * active.
+ */
+uint64_t msr_spec_ctrl_valid_bits(const struct cpuid_policy *cp)
+{
+    bool ssbd = cp->feat.ssbd || cp->extd.amd_ssbd;
+
+    /*
+     * Note: SPEC_CTRL_STIBP is specified as safe to use (i.e. ignored)
+     * when STIBP isn't enumerated in hardware.
+     */
+    return (SPEC_CTRL_IBRS | SPEC_CTRL_STIBP |
+            (ssbd       ? SPEC_CTRL_SSBD       : 0) |
+            0);
+}
+
 const uint32_t *lookup_deep_deps(uint32_t feature)
 {
     static const struct {
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3453,7 +3453,7 @@ int hvm_msr_read_intercept(unsigned int
         goto gp_fault;
 
     case MSR_SPEC_CTRL:
-        if ( !d->arch.cpuid->feat.ibrsb )
+        if ( !d->arch.cpuid->feat.ibrsb && !d->arch.cpuid->extd.ibrs )
             goto gp_fault;
         *msr_content = v->arch.spec_ctrl;
         break;
@@ -3637,17 +3637,9 @@ int hvm_msr_write_intercept(unsigned int
         break;
 
     case MSR_SPEC_CTRL:
-        if ( !d->arch.cpuid->feat.ibrsb )
-            goto gp_fault; /* MSR available? */
-
-        /*
-         * Note: SPEC_CTRL_STIBP is specified as safe to use (i.e. ignored)
-         * when STIBP isn't enumerated in hardware.
-         */
-
-        if ( msr_content & ~(SPEC_CTRL_IBRS | SPEC_CTRL_STIBP |
-                             (d->arch.cpuid->feat.ssbd ? SPEC_CTRL_SSBD : 0)) )
-            goto gp_fault; /* Rsvd bit set? */
+        if ( (!d->arch.cpuid->feat.ibrsb && !d->arch.cpuid->extd.ibrs) ||
+             (msr_content & ~msr_spec_ctrl_valid_bits(d->arch.cpuid)) )
+            goto gp_fault;
 
         v->arch.spec_ctrl = msr_content;
         break;
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -394,11 +394,33 @@ static int svm_load_vmcb_ctxt(struct vcp
 
 static unsigned int __init svm_init_msr(void)
 {
-    return boot_cpu_has(X86_FEATURE_DBEXT) ? 4 : 0;
+    return !!boot_cpu_has(X86_FEATURE_IBRS) +
+           (boot_cpu_has(X86_FEATURE_DBEXT) ? 4 : 0);
 }
 
 static void svm_save_msr(struct vcpu *v, struct hvm_msr *ctxt)
 {
+    if ( v->domain->arch.cpuid->extd.ibrs && v->arch.spec_ctrl )
+    {
+        /*
+         * Guests are given full access to certain MSRs for performance
+         * reasons.  A consequence is that Xen is unable to enforce that
+         * all bits disallowed by the CPUID policy yield #GP, and an
+         * enterprising guest may be able to set and use a bit it ought to
+         * leave alone.
+         *
+         * When migrating from a more capable host to a less capable one,
+         * such bits may be rejected by the destination, and the migration
+         * failed.
+         *
+         * Discard such bits here on the source side.  Such bits have
+         * reserved behaviour, and the guest has only itself to blame.
+         */
+        ctxt->msr[ctxt->count].index = MSR_SPEC_CTRL;
+        ctxt->msr[ctxt->count++].val = v->arch.spec_ctrl &
+            msr_spec_ctrl_valid_bits(v->domain->arch.cpuid);
+    }
+
     if ( boot_cpu_has(X86_FEATURE_DBEXT) )
     {
         ctxt->msr[ctxt->count].val = v->arch.hvm_svm.dr_mask[0];
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -944,8 +944,23 @@ static void vmx_save_msr(struct vcpu *v,
 
     if ( v->domain->arch.cpuid->feat.ibrsb && v->arch.spec_ctrl )
     {
+        /*
+         * Guests are given full access to certain MSRs for performance
+         * reasons.  A consequence is that Xen is unable to enforce that
+         * all bits disallowed by the CPUID policy yield #GP, and an
+         * enterprising guest may be able to set and use a bit it ought to
+         * leave alone.
+         *
+         * When migrating from a more capable host to a less capable one,
+         * such bits may be rejected by the destination, and the migration
+         * failed.
+         *
+         * Discard such bits here on the source side.  Such bits have
+         * reserved behaviour, and the guest has only itself to blame.
+         */
         ctxt->msr[ctxt->count].index = MSR_SPEC_CTRL;
-        ctxt->msr[ctxt->count++].val = v->arch.spec_ctrl;
+        ctxt->msr[ctxt->count++].val = v->arch.spec_ctrl &
+            msr_spec_ctrl_valid_bits(v->domain->arch.cpuid);
     }
 
     if ( cpu_has_mpx && cpu_has_vmx_mpx )
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -2671,7 +2671,7 @@ static int priv_op_read_msr(unsigned int
         break;
 
     case MSR_SPEC_CTRL:
-        if ( !currd->arch.cpuid->feat.ibrsb )
+        if ( !currd->arch.cpuid->feat.ibrsb && !currd->arch.cpuid->extd.ibrs )
             break;
         *val = curr->arch.spec_ctrl;
         return X86EMUL_OKAY;
@@ -2908,17 +2908,10 @@ static int priv_op_write_msr(unsigned in
         break;
 
     case MSR_SPEC_CTRL:
-        if ( !currd->arch.cpuid->feat.ibrsb )
-            break; /* MSR available? */
-
-        /*
-         * Note: SPEC_CTRL_STIBP is specified as safe to use (i.e. ignored)
-         * when STIBP isn't enumerated in hardware.
-         */
-
-        if ( val & ~(SPEC_CTRL_IBRS | SPEC_CTRL_STIBP |
-                     (currd->arch.cpuid->feat.ssbd ? SPEC_CTRL_SSBD : 0)) )
-            break; /* Rsvd bit set? */
+        if ( (!currd->arch.cpuid->feat.ibrsb &&
+              !currd->arch.cpuid->extd.ibrs) ||
+             (val & ~msr_spec_ctrl_valid_bits(currd->arch.cpuid)) )
+            break;
 
         curr->arch.spec_ctrl = val;
         return X86EMUL_OKAY;
--- a/xen/include/asm-x86/msr.h
+++ b/xen/include/asm-x86/msr.h
@@ -222,6 +222,8 @@ static inline void wrmsr_tsc_aux(uint32_
     }
 }
 
+uint64_t msr_spec_ctrl_valid_bits(const struct cpuid_policy *cp);
+
 /* MSR policy object for shared per-domain MSRs */
 struct msr_policy {
     /*

Places

File xsa435-0-12.patch of Package xen

Places