[PATCH v1] x86/nestedsvm: Require SpecCtrl

[chunjie.zhu@xxxxxxxxxx]

From: Chunjie Zhu <chunjie.zhu@xxxxxxxxxx>

Previously L0 did not expose this 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.

Require the SVM SpecCtrl feature before running any nested virt
guests.

Update docs/designs/nested-svm-cpu-features.md with the rationale
for requiring and exposing bit 20 (SpecCtrl).

Signed-off-by: Chunjie Zhu <chunjie.zhu@xxxxxxxxxx>
---
 docs/designs/nested-svm-cpu-features.md | 13 +++++++++++++
 xen/arch/x86/cpu-policy.c               |  3 ++-
 xen/arch/x86/hvm/svm/nestedsvm.c        | 26 ++++++++++++++++++++++++-
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/docs/designs/nested-svm-cpu-features.md 
b/docs/designs/nested-svm-cpu-features.md
index ce168e68e1b3..6999fcf1bb72 100644
--- a/docs/designs/nested-svm-cpu-features.md
+++ b/docs/designs/nested-svm-cpu-features.md
@@ -109,3 +109,16 @@ leaf 8000000A:edx
   Using it in L0 reduces the chance that we'll make some sort of error
   in the decode path.  And if hardware supports it, it's easy enough
   to provide to the L1.
+
+- 20 `SpecCtrl` *MSR_SPEC_CTRL virtualisation*: Require for L0, provide to L1
+
+  L0 already relies on this feature to virtualize MSR_SPEC_CTRL for the
+  L1 hypervisor itself (see `X86_FEATURE_SC_MSR_HVM`).  Requiring it
+  as a baseline for nested virt (first available on Zen2) means L1 can
+  rely on the same hardware virtualisation for its own L2 guests,
+  instead of having L0 unconditionally intercept and trap/emulate
+  MSR_SPEC_CTRL accesses coming from L2.  Providing it to L1 is
+  basically free once it's required of L0, and it removes a
+  significant amount of "CPU without the feature" fallback code that
+  would otherwise need to be implemented and maintained for nested
+  virt.
diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index 42dccdce5290..4d6de5dd5d0a 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -832,7 +832,8 @@ static void __init calculate_hvm_max_policy(void)
                                (1u << SVM_FEATURE_LBRV) |
                                (1u << SVM_FEATURE_NRIPS) |
                                (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);
     }
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index dc2b6a42534a..46fff4fab5d3 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -481,6 +481,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, clean.iopm);
     if ( rc )
@@ -1022,6 +1032,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;
 
     svm_vmsave_pa(nv->nv_n1vmcx_pa);
@@ -1035,6 +1046,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.
@@ -1678,5 +1701,6 @@ void __init start_nested_svm(struct hvm_function_table 
*hvm_function_table)
         cpu_has_svm_lbrv &&
         cpu_has_svm_nrips &&
         cpu_has_svm_flushbyasid &&
-        cpu_has_svm_decode;
+        cpu_has_svm_decode &&
+        cpu_has_svm_spec_ctrl;
 }
-- 
2.34.1