On Thu, Apr 10, 2025 at 4:24 PM Sean Christopherson <sea...@google.com> wrote: > > On Mon, Mar 31, 2025, Xin Li (Intel) wrote: > > Signed-off-by: Xin Li (Intel) <x...@zytor.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 > > >
