linux-kernel - Re: [RFC PATCH v1 10/15] KVM: VMX: Use WRMSRNS or its immediate form when available

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALMp9eRJkzA2YXf1Dfxt3ONP+P9aTA=WPraOPJPJ6C6j677+6Q@mail.gmail.com>
Date: Fri, 11 Apr 2025 14:12:27 -0700
From: Jim Mattson <jmattson@...gle.com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: "Xin Li (Intel)" <xin@...or.com>, linux-kernel@...r.kernel.org, 
	linux-perf-users@...r.kernel.org, linux-hyperv@...r.kernel.org, 
	virtualization@...ts.linux.dev, linux-edac@...r.kernel.org, 
	kvm@...r.kernel.org, xen-devel@...ts.xenproject.org, 
	linux-ide@...r.kernel.org, linux-pm@...r.kernel.org, bpf@...r.kernel.org, 
	llvm@...ts.linux.dev, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de, 
	dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com, jgross@...e.com, 
	andrew.cooper3@...rix.com, peterz@...radead.org, acme@...nel.org, 
	namhyung@...nel.org, mark.rutland@....com, alexander.shishkin@...ux.intel.com, 
	jolsa@...nel.org, irogers@...gle.com, adrian.hunter@...el.com, 
	kan.liang@...ux.intel.com, wei.liu@...nel.org, ajay.kaher@...adcom.com, 
	alexey.amakhalov@...adcom.com, bcm-kernel-feedback-list@...adcom.com, 
	tony.luck@...el.com, pbonzini@...hat.com, vkuznets@...hat.com, 
	luto@...nel.org, boris.ostrovsky@...cle.com, kys@...rosoft.com, 
	haiyangz@...rosoft.com, decui@...rosoft.com
Subject: Re: [RFC PATCH v1 10/15] KVM: VMX: Use WRMSRNS or its immediate form
 when available

On Thu, Apr 10, 2025 at 4:24 PM Sean Christopherson <seanjc@...gle.com> wrote:
>
> On Mon, Mar 31, 2025, Xin Li (Intel) wrote:
> > Signed-off-by: Xin Li (Intel) <xin@...or.com>
> > ---
> >  arch/x86/include/asm/msr-index.h |  6 ++++++
> >  arch/x86/kvm/vmx/vmenter.S       | 28 ++++++++++++++++++++++++----
> >  2 files changed, 30 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> > index e6134ef2263d..04244c3ba374 100644
> > --- a/arch/x86/include/asm/msr-index.h
> > +++ b/arch/x86/include/asm/msr-index.h
> > @@ -1226,4 +1226,10 @@
> >                                               * a #GP
> >                                               */
> >
> > +/* Instruction opcode for WRMSRNS supported in binutils >= 2.40 */
> > +#define ASM_WRMSRNS          _ASM_BYTES(0x0f,0x01,0xc6)
> > +
> > +/* Instruction opcode for the immediate form RDMSR/WRMSRNS */
> > +#define ASM_WRMSRNS_RAX              _ASM_BYTES(0xc4,0xe7,0x7a,0xf6,0xc0)
> > +
> >  #endif /* _ASM_X86_MSR_INDEX_H */
> > diff --git a/arch/x86/kvm/vmx/vmenter.S b/arch/x86/kvm/vmx/vmenter.S
> > index f6986dee6f8c..9fae43723c44 100644
> > --- a/arch/x86/kvm/vmx/vmenter.S
> > +++ b/arch/x86/kvm/vmx/vmenter.S
> > @@ -64,6 +64,29 @@
> >       RET
> >  .endm
> >
> > +/*
> > + * Write EAX to MSR_IA32_SPEC_CTRL.
> > + *
> > + * Choose the best WRMSR instruction based on availability.
> > + *
> > + * Replace with 'wrmsrns' and 'wrmsrns %rax, $MSR_IA32_SPEC_CTRL' once binutils support them.
> > + */
> > +.macro WRITE_EAX_TO_MSR_IA32_SPEC_CTRL
> > +     ALTERNATIVE_2 __stringify(mov $MSR_IA32_SPEC_CTRL, %ecx;                \
> > +                               xor %edx, %edx;                               \
> > +                               mov %edi, %eax;                               \
> > +                               ds wrmsr),                                    \
> > +                   __stringify(mov $MSR_IA32_SPEC_CTRL, %ecx;                \
> > +                               xor %edx, %edx;                               \
> > +                               mov %edi, %eax;                               \
> > +                               ASM_WRMSRNS),                                 \
> > +                   X86_FEATURE_WRMSRNS,                                      \
> > +                   __stringify(xor %_ASM_AX, %_ASM_AX;                       \
> > +                               mov %edi, %eax;                               \
> > +                               ASM_WRMSRNS_RAX; .long MSR_IA32_SPEC_CTRL),   \
> > +                   X86_FEATURE_MSR_IMM
> > +.endm
>
> This is quite hideous.  I have no objection to optimizing __vmx_vcpu_run(), but
> I would much prefer that a macro like this live in generic code, and that it be
> generic.  It should be easy enough to provide an assembly friendly equivalent to
> __native_wrmsr_constant().

Surely, any CPU that has WRMSRNS also supports "Virtualize
IA32_SPEC_CTRL," right? Shouldn't we be using that feature rather than
swapping host and guest values with some form of WRMSR?

> > +
> >  .section .noinstr.text, "ax"
> >
> >  /**
> > @@ -123,10 +146,7 @@ SYM_FUNC_START(__vmx_vcpu_run)
> >       movl PER_CPU_VAR(x86_spec_ctrl_current), %esi
> >       cmp %edi, %esi
> >       je .Lspec_ctrl_done
> > -     mov $MSR_IA32_SPEC_CTRL, %ecx
> > -     xor %edx, %edx
> > -     mov %edi, %eax
> > -     wrmsr
> > +     WRITE_EAX_TO_MSR_IA32_SPEC_CTRL
> >
> >  .Lspec_ctrl_done:
> >
> > --
> > 2.49.0
> >
>