[PATCH 3/3] x86/spec-ctrl: Shrink further entry paths due to %rdx being 0

Andrew Cooper posted 3 patches 3 years, 6 months ago
[PATCH 3/3] x86/spec-ctrl: Shrink further entry paths due to %rdx being 0
Posted by Andrew Cooper 3 years, 6 months ago
This is a continuation of the observation from:
  e9b8d31981f1 ("x86/spec-ctrl: Rework SPEC_CTRL_ENTRY_FROM_INTR_IST")
  53a570b28569 ("x86/spec-ctrl: Support IBPB-on-entry")

With %rdx known to be zero and not clobbered on the early entry path, we don't
need to re-zero it every time want to write to an MSR.

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>
---
 xen/arch/x86/hvm/vmx/entry.S             | 4 +---
 xen/arch/x86/include/asm/spec_ctrl_asm.h | 3 +--
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/entry.S b/xen/arch/x86/hvm/vmx/entry.S
index 5f5de45a1309..392aca42b864 100644
--- a/xen/arch/x86/hvm/vmx/entry.S
+++ b/xen/arch/x86/hvm/vmx/entry.S
@@ -33,13 +33,12 @@ ENTRY(vmx_asm_vmexit_handler)
         movb $1,VCPU_vmx_launched(%rbx)
         mov  %rax,VCPU_hvm_guest_cr2(%rbx)
 
-        /* SPEC_CTRL_ENTRY_FROM_VMX    Req: b=curr %rsp=regs/cpuinfo, Clob: acd */
+        /* SPEC_CTRL_ENTRY_FROM_VMX    Req: %rsp=regs/cpuinfo, %rdx=0, Clob: acd */
         ALTERNATIVE "", DO_OVERWRITE_RSB, X86_FEATURE_SC_RSB_HVM
 
         .macro restore_spec_ctrl
             mov    $MSR_SPEC_CTRL, %ecx
             movzbl CPUINFO_xen_spec_ctrl(%rsp), %eax
-            xor    %edx, %edx
             wrmsr
         .endm
         ALTERNATIVE "", restore_spec_ctrl, X86_FEATURE_SC_MSR_HVM
@@ -49,7 +48,6 @@ ENTRY(vmx_asm_vmexit_handler)
         .macro restore_lbr
             mov $IA32_DEBUGCTLMSR_LBR, %eax
             mov $MSR_IA32_DEBUGCTLMSR, %ecx
-            xor %edx, %edx
             wrmsr
         .endm
         ALTERNATIVE "", restore_lbr, X86_FEATURE_XEN_LBR
diff --git a/xen/arch/x86/include/asm/spec_ctrl_asm.h b/xen/arch/x86/include/asm/spec_ctrl_asm.h
index fab27ff5532b..61eed8510ba9 100644
--- a/xen/arch/x86/include/asm/spec_ctrl_asm.h
+++ b/xen/arch/x86/include/asm/spec_ctrl_asm.h
@@ -176,7 +176,7 @@
 .macro DO_SPEC_CTRL_ENTRY maybexen:req
 /*
  * Requires %rsp=regs (also cpuinfo if !maybexen)
- * Requires %r14=stack_end (if maybexen)
+ * Requires %r14=stack_end (if maybexen), %rdx=0
  * Clobbers %rax, %rcx, %rdx
  *
  * PV guests can't update MSR_SPEC_CTRL behind Xen's back, so no need to read
@@ -184,7 +184,6 @@
  * while entries from Xen must leave shadowing in its current state.
  */
     mov $MSR_SPEC_CTRL, %ecx
-    xor %edx, %edx
 
     /*
      * Clear SPEC_CTRL shadowing *before* loading Xen's value.  If entering
-- 
2.11.0


Re: [PATCH 3/3] x86/spec-ctrl: Shrink further entry paths due to %rdx being 0
Posted by Jan Beulich 3 years, 6 months ago
On 18.07.2022 22:50, Andrew Cooper wrote:
> --- a/xen/arch/x86/hvm/vmx/entry.S
> +++ b/xen/arch/x86/hvm/vmx/entry.S
> @@ -33,13 +33,12 @@ ENTRY(vmx_asm_vmexit_handler)
>          movb $1,VCPU_vmx_launched(%rbx)
>          mov  %rax,VCPU_hvm_guest_cr2(%rbx)
>  
> -        /* SPEC_CTRL_ENTRY_FROM_VMX    Req: b=curr %rsp=regs/cpuinfo, Clob: acd */
> +        /* SPEC_CTRL_ENTRY_FROM_VMX    Req: %rsp=regs/cpuinfo, %rdx=0, Clob: acd */
>          ALTERNATIVE "", DO_OVERWRITE_RSB, X86_FEATURE_SC_RSB_HVM

Leaving %rdx documented as clobbered here is misleading - the scope of
the comment is likely meant to extend to ...

>          .macro restore_spec_ctrl
>              mov    $MSR_SPEC_CTRL, %ecx
>              movzbl CPUINFO_xen_spec_ctrl(%rsp), %eax
> -            xor    %edx, %edx
>              wrmsr
>          .endm
>          ALTERNATIVE "", restore_spec_ctrl, X86_FEATURE_SC_MSR_HVM
> @@ -49,7 +48,6 @@ ENTRY(vmx_asm_vmexit_handler)
>          .macro restore_lbr
>              mov $IA32_DEBUGCTLMSR_LBR, %eax
>              mov $MSR_IA32_DEBUGCTLMSR, %ecx
> -            xor %edx, %edx
>              wrmsr
>          .endm
>          ALTERNATIVE "", restore_lbr, X86_FEATURE_XEN_LBR

... here, but that's not necessarily what a reader might gain. Plus
with the change the register isn't clobbered anymore.

> --- a/xen/arch/x86/include/asm/spec_ctrl_asm.h
> +++ b/xen/arch/x86/include/asm/spec_ctrl_asm.h
> @@ -176,7 +176,7 @@
>  .macro DO_SPEC_CTRL_ENTRY maybexen:req
>  /*
>   * Requires %rsp=regs (also cpuinfo if !maybexen)
> - * Requires %r14=stack_end (if maybexen)
> + * Requires %r14=stack_end (if maybexen), %rdx=0
>   * Clobbers %rax, %rcx, %rdx
>   *
>   * PV guests can't update MSR_SPEC_CTRL behind Xen's back, so no need to read
> @@ -184,7 +184,6 @@
>   * while entries from Xen must leave shadowing in its current state.
>   */
>      mov $MSR_SPEC_CTRL, %ecx
> -    xor %edx, %edx
>  
>      /*
>       * Clear SPEC_CTRL shadowing *before* loading Xen's value.  If entering

This is used in SPEC_CTRL_ENTRY_FROM_{INTR,PV} after
DO_SPEC_CTRL_COND_IBPB, which documents %rdx as clobbered. Since it
doesn't actually clobber the register, it's the documentation line
which needs updating along with making this code change.

With the respective adjustments
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan