From: Sean Christopherson <seanjc@google.com>
To: Uros Bizjak <ubizjak@gmail.com>
Cc: kvm@vger.kernel.org, x86@kernel.org,
linux-kernel@vger.kernel.org,
Paolo Bonzini <pbonzini@redhat.com>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@kernel.org>, Borislav Petkov <bp@alien8.de>,
Dave Hansen <dave.hansen@linux.intel.com>,
"H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH] KVM: VMX: Micro-optimize SPEC_CTRL handling in __vmx_vcpu_run()
Date: Wed, 5 Nov 2025 17:12:02 -0800 [thread overview]
Message-ID: <aQv14mmAkUPL-Fap@google.com> (raw)
In-Reply-To: <CAFULd4Y6W0hJbA8Ki2yB60537mC8+ohXyUgxD+HuKDQhq7zGmA@mail.gmail.com>
On Wed, Aug 20, 2025, Uros Bizjak wrote:
> On Wed, Aug 20, 2025 at 8:10 AM Uros Bizjak <ubizjak@gmail.com> wrote:
...
> VMX patch is at [1]. SVM patch is a bit more involved, because new
> 32-bit code needs to clobber one additional register. The SVM patch is
> attached to this message, but while I compile tested it, I have no
> means of testing it with runtime tests. Can you please put it through
> your torture tests?
>
> [1] https://lore.kernel.org/lkml/20250820100007.356761-1-ubizjak@gmail.com/
Finally got around to testing this (such a pain to test this code). I hacked
a host to allow any value for MSR_IA32_SPEC_CTRL, and then ran in a VM to verify
KVM could actually save/restore 64-bit values (and that KVM elides the WRMSRs
when possible).
The VMX patch looks good (I'll get it applied shortly). The SVM version has a
bug, but I've got it working and will post a patch shortly.
>
> Uros.
> diff --git a/arch/x86/kvm/svm/vmenter.S b/arch/x86/kvm/svm/vmenter.S
> index 235c4af6b692..a1b9f2ac713c 100644
> --- a/arch/x86/kvm/svm/vmenter.S
> +++ b/arch/x86/kvm/svm/vmenter.S
> @@ -52,11 +52,23 @@
> * there must not be any returns or indirect branches between this code
> * and vmentry.
> */
> - movl SVM_spec_ctrl(%_ASM_DI), %eax
> - cmp PER_CPU_VAR(x86_spec_ctrl_current), %eax
> +#ifdef CONFIG_X86_64
> + mov SVM_spec_ctrl(%rdi), %rdx
> + cmp PER_CPU_VAR(x86_spec_ctrl_current), %rdx
> + je 801b
> + movl %edx, %eax
> + shr $32, %rdx
> +#else
> + mov SVM_spec_ctrl(%edi), %eax
> + mov PER_CPU_VAR(x86_spec_ctrl_current), %ecx
> + xor %eax, %ecx
> + mov SVM_spec_ctrl + 4(%edi), %edx
> + mov PER_CPU_VAR(x86_spec_ctrl_current + 4), %esi
> + xor %edx, %esi
> + or %esi, %ecx
> je 801b
> +#endif
> mov $MSR_IA32_SPEC_CTRL, %ecx
> - xor %edx, %edx
> wrmsr
> jmp 801b
> .endm
> @@ -80,14 +92,31 @@
> cmpb $0, \spec_ctrl_intercepted
> jnz 998f
> rdmsr
> - movl %eax, SVM_spec_ctrl(%_ASM_DI)
> +#ifdef CONFIG_X86_64
> + shl $32, %rdx
> + or %rax, %rdx
> + mov %rdx, SVM_spec_ctrl(%rdi)
> 998:
To avoid defining the 998 label separately for 64-bit vs. 32-bit, I think it's
worth making two 32-bit writes to svm->spec_ctrl even on 64-bit kernels, e.g.
cmpb $0, \spec_ctrl_intercepted
jnz 998f
rdmsr
movl %eax, SVM_spec_ctrl(%_ASM_DI)
movl %edx, SVM_spec_ctrl + 4(%_ASM_DI)
998:
/* Now restore the host value of the MSR if different from the guest's. */
> -
> /* Now restore the host value of the MSR if different from the guest's. */
> - movl PER_CPU_VAR(x86_spec_ctrl_current), %eax
> - cmp SVM_spec_ctrl(%_ASM_DI), %eax
> + mov SVM_spec_ctrl(%rdi), %rdx
> + cmp PER_CPU_VAR(x86_spec_ctrl_current), %rdx
> je 901b
> - xor %edx, %edx
> + movl %edx, %eax
> + shr $32, %rdx
> +#else
> + mov %eax, SVM_spec_ctrl(%edi)
> + mov %edx, SVM_spec_ctrl + 4(%edi)
> +998:
> + /* Now restore the host value of the MSR if different from the guest's. */
> + mov SVM_spec_ctrl(%edi), %eax
> + mov PER_CPU_VAR(x86_spec_ctrl_current), %esi
> + xor %eax, %esi
> + mov SVM_spec_ctrl + 4(%edi), %edx
> + mov PER_CPU_VAR(x86_spec_ctrl_current + 4), %edi
> + xor %edx, %edi
> + or %edi, %esi
> + je 901b
This particular flow is backwards, in that it loads the guest value into EDX:EAX
instead of the host values.
> +#endif
> wrmsr
> jmp 901b
> .endm
next prev parent reply other threads:[~2025-11-06 1:12 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-07 6:36 [PATCH] KVM: VMX: Micro-optimize SPEC_CTRL handling in __vmx_vcpu_run() Uros Bizjak
2025-08-19 14:59 ` Sean Christopherson
2025-08-19 16:24 ` Uros Bizjak
2025-08-19 18:56 ` Sean Christopherson
2025-08-20 6:10 ` Uros Bizjak
2025-08-20 11:50 ` Uros Bizjak
2025-11-06 1:12 ` Sean Christopherson [this message]
2025-11-06 18:29 ` Uros Bizjak
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=aQv14mmAkUPL-Fap@google.com \
--to=seanjc@google.com \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=hpa@zytor.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=pbonzini@redhat.com \
--cc=tglx@linutronix.de \
--cc=ubizjak@gmail.com \
--cc=x86@kernel.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.