[PATCH v2] x86/nestedsvm: Expose SpecCtrl to L1 when L0 virtualises MSR_SPEC_CTRL

[chunjie.zhu@xxxxxxxxxx]

From: Chunjie Zhu <chunjie.zhu@xxxxxxxxxx>

Previously L0 did not expose the SVM SpecCtrl bit, forcing L1 to
intercept and emulate MSR_SPEC_CTRL accesses from L2 in software, which
is functionally correct but suboptimal. Per APM vol.2 rev 3.44 "SPEC_CTRL
Virtualization", hardware supports combining host and guest SPEC_CTRL
values via VMCB.spec_ctrl with a logical OR while a guest runs, and
saves the (possibly guest-modified) value back into VMCB.spec_ctrl
on #VMEXIT.

Handle VMCB.spec_ctrl across nested VMRUN/#VMEXIT, and expose the
SpecCtrl bit to L1 if L0 virtualises MSR_SPEC_CTRL for HVM guests
(X86_FEATURE_SC_MSR_HVM, which on AMD implies hardware support for the
bit). Otherwise IBRS is hidden from L1 and there is nothing to do, so
SpecCtrl is not required for nested virt.

Update docs/designs/nested-svm-cpu-features.md with the rationale.

Signed-off-by: Chunjie Zhu <chunjie.zhu@xxxxxxxxxx>
---
v2:
 * Don't require SVM SpecCtrl for nested virt (Teddy): on AMD,
   X86_FEATURE_SC_MSR_HVM already implies hardware support, and without it
   IBRS is hidden from L1.
 * Only expose SpecCtrl to L1 when X86_FEATURE_SC_MSR_HVM is set.
 * Update the design doc and commit message accordingly.
 docs/designs/nested-svm-cpu-features.md | 12 ++++++++++++
 xen/arch/x86/cpu-policy.c               | 10 +++++++++-
 xen/arch/x86/hvm/svm/nestedsvm.c        | 23 +++++++++++++++++++++++
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/docs/designs/nested-svm-cpu-features.md 
b/docs/designs/nested-svm-cpu-features.md
index 482edca2b5f5..6c1925e062f4 100644
--- a/docs/designs/nested-svm-cpu-features.md
+++ b/docs/designs/nested-svm-cpu-features.md
@@ -114,3 +114,15 @@ leaf 8000000A:edx
 
   L0 uses the hardware virtual GIF to virtualize the GIF of both L1 and
   its L2, which removes the need to shadow it in software.
+
+- 20 `SpecCtrl` *MSR_SPEC_CTRL virtualisation*: Not required for L0, provide
+  to L1 if L0 virtualises MSR_SPEC_CTRL
+
+  Without MSR_SPEC_CTRL virtualisation in L0 (hardware without
+  MSR_SPEC_CTRL, or `X86_FEATURE_SC_MSR_HVM` disabled), IBRS is hidden
+  from the L1 and there is nothing for nested virt to do, so the feature
+  is not required.  When L0 does virtualise MSR_SPEC_CTRL (see
+  `X86_FEATURE_SC_MSR_HVM`, which implies hardware support for this
+  bit), provide it to L1 so that L1 can use the same hardware
+  virtualisation for its own L2 guests, instead of intercepting and
+  emulating MSR_SPEC_CTRL accesses from L2.
diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index 48a3185eed8d..88f5a949e583 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -845,9 +845,17 @@ static void __init calculate_hvm_max_policy(void)
                                (1u << SVM_FEATURE_NRIPS) |
                                (1u << SVM_FEATURE_FLUSHBYASID) |
                                (1u << SVM_FEATURE_PAUSEFILTER) |
-                               (1u << SVM_FEATURE_DECODEASSISTS));
+                               (1u << SVM_FEATURE_DECODEASSISTS) |
+                               (1u << SVM_FEATURE_SPEC_CTRL));
         /* Enable features which are always emulated. */
         p->extd.raw[0xa].d |= (1u << SVM_FEATURE_VMCBCLEAN);
+
+        /*
+         * SpecCtrl is only meaningful if Xen virtualises MSR_SPEC_CTRL for
+         * HVM guests, which implies hardware support.
+         */
+        if ( !boot_cpu_has(X86_FEATURE_SC_MSR_HVM) )
+            p->extd.raw[0xa].d &= ~(1u << SVM_FEATURE_SPEC_CTRL);
     }
     
     guest_common_max_feature_adjustments(fs);
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index e896c9a0f29f..0c4ce9312415 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -417,6 +417,16 @@ static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct 
cpu_user_regs *regs)
     /* TSC offset */
     n2vmcb->_tsc_offset = n1vmcb->_tsc_offset + ns_vmcb->_tsc_offset;
 
+    /*
+     * SPEC_CTRL: When in host mode, the host SPEC_CTRL value is in effect
+     * and writes update the host SPEC_CTRL register. On a VMRUN, the
+     * processor loads the guest SPEC_CTRL register value from the VMCB or
+     * VMSA. For most guests, processor behavior is controlled by the logical
+     * OR of the two registers.  (APM vol.2 rev 3.44
+     * "SPEC_CTRL Virtualization").
+     */
+    n2vmcb->spec_ctrl = n1vmcb->spec_ctrl | ns_vmcb->spec_ctrl;
+
     /* Nested IO permission bitmaps */
     rc = nsvm_vmrun_permissionmap(v);
     if ( rc )
@@ -988,6 +998,7 @@ nsvm_vmcb_prepare4vmexit(struct vcpu *v, struct 
cpu_user_regs *regs)
     struct nestedvcpu *nv = &vcpu_nestedhvm(v);
     struct nestedsvm *svm = &vcpu_nestedsvm(v);
     struct vmcb_struct *ns_vmcb = nv->nv_vvmcx;
+    struct vmcb_struct *n1vmcb = nv->nv_n1vmcx;
     struct vmcb_struct *n2vmcb = nv->nv_n2vmcx;
 
     /* Cache guest physical address of virtual vmcb
@@ -999,6 +1010,18 @@ nsvm_vmcb_prepare4vmexit(struct vcpu *v, struct 
cpu_user_regs *regs)
 
     /* Pausefilter - keep it as is */
 
+    /*
+     * SPEC_CTRL: on real hardware, only the guest's own SPEC_CTRL bits are
+     * ever saved back to the VMCB on #VMEXIT (APM vol.2 rev 3.44 "SPEC_CTRL
+     * Virtualization"); the OR with the host's bits is a transient
+     * execution-time effect, never written back into the guest register.
+     * Here, n2vmcb->spec_ctrl was seeded with n1vmcb's bits OR'd in at VMRUN
+     * time (see nsvm_vmcb_prepare4vmrun()), and will still contain them if L2
+     * never wrote the MSR itself.  Mask those back out so L1's own SPEC_CTRL
+     * doesn't leak into, and persist in, L2's saved state.
+     */
+    ns_vmcb->spec_ctrl = n2vmcb->spec_ctrl & ~n1vmcb->spec_ctrl;
+
     /* Nested IO permission bitmap */
     /* Just keep the iopm_base_pa and msrpm_base_pa values.
      * The guest must not see the virtualized values.
-- 
2.34.1