All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xenproject.org>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Jan Beulich" <JBeulich@suse.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>
Subject: [PATCH v2 7/9] x86/svm: VMEntry/Exit logic for MSR_SPEC_CTRL
Date: Fri, 28 Jan 2022 13:29:25 +0000	[thread overview]
Message-ID: <20220128132927.14997-8-andrew.cooper3@citrix.com> (raw)
In-Reply-To: <20220128132927.14997-1-andrew.cooper3@citrix.com>

Hardware maintains both host and guest versions of MSR_SPEC_CTRL, but guests
run with the logical OR of both values.  Therefore, in principle we want to
clear Xen's value before entering the guest.  However, for migration
compatibility, and for performance reasons with SEV-SNP guests, we want the
ability to use a nonzero value behind the guest's back.  Use vcpu_msrs to hold
this value, with the guest value in the VMCB.

On the VMEntry path, adjusting MSR_SPEC_CTRL must be done after CLGI so as to
be atomic with respect to NMIs/etc.  The loading of spec_ctrl_raw into %eax
was also stale from the unused old code, so can be dropped too.

Implement both pieces of logic as small pieces of C, and alternative the call
to get there based on X86_FEATURE_SC_MSR_HVM.  The use of double alternative
blocks is due to a quirk of the current infrastructure, where call
displacements only get fixed up for the first replacement instruction.  While
adjusting the clobber lists, drop the stale requirements on the VMExit side.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>

The RAS[:32] flushing side effect is under reconsideration.  It is actually a
very awkward side effect in practice, and not applicable to any
implementations (that I'm aware of), but for now, it's the documented safe
action to take.  Furthermore, it avoids complicating the logic with an lfence
in the else case for Spectre v1 safety.

v2:
 * Split last_spec_ctrl introduction into earlier patch.
 * Use STR() rather than __stringify() for brevity.
 * Use double alt blocks in order to pass function parameters.
---
 xen/arch/x86/hvm/svm/entry.S             | 12 +++++++-----
 xen/arch/x86/hvm/svm/svm.c               | 27 +++++++++++++++++++++++++++
 xen/arch/x86/include/asm/msr.h           |  9 +++++++++
 xen/arch/x86/include/asm/spec_ctrl_asm.h |  3 +++
 4 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/entry.S b/xen/arch/x86/hvm/svm/entry.S
index 276215d36aff..190f7095c65c 100644
--- a/xen/arch/x86/hvm/svm/entry.S
+++ b/xen/arch/x86/hvm/svm/entry.S
@@ -55,11 +55,12 @@ __UNLIKELY_END(nsvm_hap)
         mov  %rsp, %rdi
         call svm_vmenter_helper
 
-        mov VCPU_arch_msrs(%rbx), %rax
-        mov VCPUMSR_spec_ctrl_raw(%rax), %eax
+        clgi
 
         /* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
-        /* SPEC_CTRL_EXIT_TO_SVM   (nothing currently) */
+        /* SPEC_CTRL_EXIT_TO_SVM       Req:                           Clob: C   */
+        ALTERNATIVE "", STR(mov %rbx, %rdi; mov %rsp, %rsi), X86_FEATURE_SC_MSR_HVM
+        ALTERNATIVE "", STR(call vmentry_spec_ctrl), X86_FEATURE_SC_MSR_HVM
 
         pop  %r15
         pop  %r14
@@ -78,7 +79,6 @@ __UNLIKELY_END(nsvm_hap)
         pop  %rsi
         pop  %rdi
 
-        clgi
         sti
         vmrun
 
@@ -86,8 +86,10 @@ __UNLIKELY_END(nsvm_hap)
 
         GET_CURRENT(bx)
 
-        /* SPEC_CTRL_ENTRY_FROM_SVM    Req: b=curr %rsp=regs/cpuinfo, Clob: ac  */
+        /* SPEC_CTRL_ENTRY_FROM_SVM    Req:                           Clob: C   */
         ALTERNATIVE "", DO_OVERWRITE_RSB, X86_FEATURE_SC_RSB_HVM
+        ALTERNATIVE "", STR(mov %rsp, %rdi), X86_FEATURE_SC_MSR_HVM
+        ALTERNATIVE "", STR(call vmexit_spec_ctrl), X86_FEATURE_SC_MSR_HVM
         /* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */
 
         stgi
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index bb6b8e560a9f..f753bf48c252 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -3086,6 +3086,33 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
     vmcb_set_vintr(vmcb, intr);
 }
 
+/* Called with GIF=0. */
+void vmexit_spec_ctrl(struct cpu_info *info)
+{
+    unsigned int val = info->xen_spec_ctrl;
+
+    /*
+     * Write to MSR_SPEC_CTRL unconditionally, for the RAS[:32] flushing side
+     * effect.
+     */
+    wrmsr(MSR_SPEC_CTRL, val, 0);
+    info->last_spec_ctrl = val;
+}
+
+/* Called with GIF=0. */
+void vmentry_spec_ctrl(const struct vcpu *curr, struct cpu_info *info)
+{
+    unsigned int val = curr->arch.msrs->spec_ctrl.raw;
+
+    if ( val != info->last_spec_ctrl )
+    {
+        wrmsr(MSR_SPEC_CTRL, val, 0);
+        info->last_spec_ctrl = val;
+    }
+
+    /* No Spectre v1 concerns.  Execution is going to hit VMRUN imminently. */
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/include/asm/msr.h b/xen/arch/x86/include/asm/msr.h
index 657a3295613d..ce4fe51afe54 100644
--- a/xen/arch/x86/include/asm/msr.h
+++ b/xen/arch/x86/include/asm/msr.h
@@ -297,6 +297,15 @@ struct vcpu_msrs
      *
      * For VT-x guests, the guest value is held in the MSR guest load/save
      * list.
+     *
+     * For SVM, the guest value lives in the VMCB, and hardware saves/restores
+     * the host value automatically.  However, guests run with the OR of the
+     * host and guest value, which allows Xen to set protections behind the
+     * guest's back.
+     *
+     * We must clear/restore Xen's value before/after VMRUN to avoid unduly
+     * influencing the guest.  In order to support "behind the guest's back"
+     * protections, we load this value (commonly 0) before VMRUN.
      */
     struct {
         uint32_t raw;
diff --git a/xen/arch/x86/include/asm/spec_ctrl_asm.h b/xen/arch/x86/include/asm/spec_ctrl_asm.h
index 9c0c7622c41f..02b3b18ce69f 100644
--- a/xen/arch/x86/include/asm/spec_ctrl_asm.h
+++ b/xen/arch/x86/include/asm/spec_ctrl_asm.h
@@ -46,6 +46,9 @@
  *   - On VMX by using MSR load/save lists to have vmentry/exit atomically
  *     load/save the guest value.  Xen's value is loaded in regular code, and
  *     there is no need to use the shadow logic (below).
+ *   - On SVM by altering MSR_SPEC_CTRL inside the CLGI/STGI region.  This
+ *     makes the changes atomic with respect to NMIs/etc, so no need for
+ *     shadowing logic.
  *
  * Factor 2 is harder.  We maintain a shadow_spec_ctrl value, and a use_shadow
  * boolean in the per cpu spec_ctrl_flags.  The synchronous use is:
-- 
2.11.0



  parent reply	other threads:[~2022-01-28 13:30 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-28 13:29 [PATCH v2 0/9] x86: MSR_SPEC_CTRL support for SVM guests Andrew Cooper
2022-01-28 13:29 ` [PATCH v2 1/9] x86/cpuid: Advertise SSB_NO to guests by default Andrew Cooper
2022-01-28 14:31   ` Jan Beulich
2022-01-31  9:41   ` Roger Pau Monné
2022-01-31 11:15     ` Andrew Cooper
2022-01-31 11:23       ` Roger Pau Monné
2022-01-28 13:29 ` [PATCH v2 2/9] x86/spec-ctrl: Drop use_spec_ctrl boolean Andrew Cooper
2022-01-28 13:29 ` [PATCH v2 3/9] x86/spec-ctrl: Introduce new has_spec_ctrl boolean Andrew Cooper
2022-01-28 13:29 ` [PATCH v2 4/9] x86/spec-ctrl: Don't use spec_ctrl_{enter,exit}_idle() for S3 Andrew Cooper
2022-01-29  1:09   ` Andrew Cooper
2022-01-31 10:15   ` Jan Beulich
2022-01-31 11:23     ` Andrew Cooper
2022-01-31 14:24       ` Andrew Cooper
2022-01-28 13:29 ` [PATCH v2 5/9] x86/spec-ctrl: Record the last write to MSR_SPEC_CTRL Andrew Cooper
2022-01-31 10:20   ` Jan Beulich
2022-01-31 11:35     ` Andrew Cooper
2022-01-28 13:29 ` [PATCH v2 6/9] x86/spec-ctrl: Use common MSR_SPEC_CTRL logic for AMD Andrew Cooper
2022-01-31 10:25   ` Jan Beulich
2022-01-28 13:29 ` Andrew Cooper [this message]
2022-01-31 10:33   ` [PATCH v2 7/9] x86/svm: VMEntry/Exit logic for MSR_SPEC_CTRL Jan Beulich
2022-01-31 11:47     ` Andrew Cooper
2022-01-31 12:55   ` Jan Beulich
2022-01-31 14:04     ` Andrew Cooper
2022-01-31 15:36   ` [PATCH v3 " Andrew Cooper
2022-02-01 11:47     ` Jan Beulich
2022-02-01 12:28       ` Andrew Cooper
2022-02-01 12:40         ` Jan Beulich
2022-02-01 12:46           ` Andrew Cooper
2022-01-28 13:29 ` [PATCH v2 8/9] x86/msr: AMD MSR_SPEC_CTRL infrastructure Andrew Cooper
2022-01-28 13:29 ` [PATCH v2 9/9] x86/cpuid: Enable MSR_SPEC_CTRL in SVM guests by default Andrew Cooper
2022-01-31 10:39   ` Jan Beulich
2022-01-31 11:54     ` Andrew Cooper

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220128132927.14997-8-andrew.cooper3@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=JBeulich@suse.com \
    --cc=roger.pau@citrix.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.