From: Sean Christopherson <seanjc@google.com>
To: Alexey Dobriyan <adobriyan@gmail.com>
Cc: pbonzini@redhat.com, kvm@vger.kernel.org
Subject: Re: [PATCH] kvm, vmx: don't use "unsigned long" in vmx_vcpu_enter_exit()
Date: Fri, 18 Nov 2022 17:44:35 +0000 [thread overview]
Message-ID: <Y3fEg/0PcgJi7mWd@google.com> (raw)
In-Reply-To: <Y3e7UW0WNV2AZmsZ@p183>
Nit (because I really suck at case-insensitive searches), please capitalize
"KVM: VMX:" in the shortlog.
On Fri, Nov 18, 2022, Alexey Dobriyan wrote:
> __vmx_vcpu_run_flags() returns "unsigned int" and uses only 2 bits of it
> so using "unsigned long" is very much pointless.
And __vmx_vcpu_run() and vmx_spec_ctrl_restore_host() take an "unsigned int" as
well, i.e. actually relying on an "unsigned long" value won't actually work.
On a related topic, this code in __vmx_vcpu_run() is unnecessarily fragile as it
relies on VMX_RUN_VMRESUME being in bits 0-7.
/* Copy @flags to BL, _ASM_ARG3 is volatile. */
mov %_ASM_ARG3, %bl
...
/* Check if vmlaunch or vmresume is needed */
testb $VMX_RUN_VMRESUME, %bl
The "byte" logic is another holdover, from when "flags" was just "launched" and
was passed in as a boolean. I'll send a proper patch to do:
diff --git a/arch/x86/kvm/vmx/vmenter.S b/arch/x86/kvm/vmx/vmenter.S
index 0b5db4de4d09..5bd39f63497d 100644
--- a/arch/x86/kvm/vmx/vmenter.S
+++ b/arch/x86/kvm/vmx/vmenter.S
@@ -69,8 +69,8 @@ SYM_FUNC_START(__vmx_vcpu_run)
*/
push %_ASM_ARG2
- /* Copy @flags to BL, _ASM_ARG3 is volatile. */
- mov %_ASM_ARG3B, %bl
+ /* Copy @flags to EBX, _ASM_ARG3 is volatile. */
+ mov %_ASM_ARG3L, %ebx
lea (%_ASM_SP), %_ASM_ARG2
call vmx_update_host_rsp
@@ -106,7 +106,7 @@ SYM_FUNC_START(__vmx_vcpu_run)
mov (%_ASM_SP), %_ASM_AX
/* Check if vmlaunch or vmresume is needed */
- testb $VMX_RUN_VMRESUME, %bl
+ test $VMX_RUN_VMRESUME, %ebx
/* Load guest registers. Don't clobber flags. */
mov VCPU_RCX(%_ASM_AX), %_ASM_CX
@@ -128,7 +128,7 @@ SYM_FUNC_START(__vmx_vcpu_run)
/* Load guest RAX. This kills the @regs pointer! */
mov VCPU_RAX(%_ASM_AX), %_ASM_AX
- /* Check EFLAGS.ZF from 'testb' above */
+ /* Check EFLAGS.ZF from 'test VMX_RUN_VMRESUME' above */
jz .Lvmlaunch
/*
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> ---
Reviewed-by: Sean Christopherson <seanjc@google.com>
next prev parent reply other threads:[~2022-11-18 17:44 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-18 17:05 [PATCH] kvm, vmx: don't use "unsigned long" in vmx_vcpu_enter_exit() Alexey Dobriyan
2022-11-18 17:44 ` Sean Christopherson [this message]
2023-01-19 21:03 ` Sean Christopherson
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=Y3fEg/0PcgJi7mWd@google.com \
--to=seanjc@google.com \
--cc=adobriyan@gmail.com \
--cc=kvm@vger.kernel.org \
--cc=pbonzini@redhat.com \
/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.