diff mbox series

KVM: VMX: restore vmx_vmexit alignment

Message ID 20230531155821.80590-1-jon@nutanix.com
State New
Headers
Series KVM: VMX: restore vmx_vmexit alignment |

Commit Message

Jon Kohler May 31, 2023, 3:58 p.m. UTC 
  Commit 8bd200d23ec4 ("KVM: VMX: Flatten __vmx_vcpu_run()") changed
vmx_vmexit from SYM_FUNC_START to SYM_INNER_LABEL, accidentally
removing 16 byte alignment as SYM_FUNC_START uses SYM_A_ALIGN and
SYM_INNER_LABEL does not. Josh mentioned [1] this was unintentional.

Fix by changing to SYM_INNER_LABEL_ALIGN instead.

[1] https://lore.kernel.org/lkml/Y3adkSe%2FJ70PqUyt@p183

Fixes: 8bd200d23ec4 ("KVM: VMX: Flatten __vmx_vcpu_run()")
Signed-off-by: Jon Kohler <jon@nutanix.com>
Suggested-by: Alexey Dobriyan <adobriyan@gmail.com>
CC: Josh Poimboeuf <jpoimboe@kernel.org>
---
 arch/x86/kvm/vmx/vmenter.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Sean Christopherson May 31, 2023, 6:20 p.m. UTC | #1 
On Wed, May 31, 2023, Jon Kohler wrote:
> Commit 8bd200d23ec4 ("KVM: VMX: Flatten __vmx_vcpu_run()") changed
> vmx_vmexit from SYM_FUNC_START to SYM_INNER_LABEL, accidentally
> removing 16 byte alignment as SYM_FUNC_START uses SYM_A_ALIGN and
> SYM_INNER_LABEL does not. Josh mentioned [1] this was unintentional.

Anyone know if this is this stable material, or just nice to have?
Jon Kohler May 31, 2023, 7:36 p.m. UTC | #2 
> On May 31, 2023, at 2:20 PM, Sean Christopherson <seanjc@google.com> wrote:
> 
> On Wed, May 31, 2023, Jon Kohler wrote:
>> Commit 8bd200d23ec4 ("KVM: VMX: Flatten __vmx_vcpu_run()") changed
>> vmx_vmexit from SYM_FUNC_START to SYM_INNER_LABEL, accidentally
>> removing 16 byte alignment as SYM_FUNC_START uses SYM_A_ALIGN and
>> SYM_INNER_LABEL does not. Josh mentioned [1] this was unintentional.
> 
> Anyone know if this is this stable material, or just nice to have?

I’m on the fence, though my vote is nice to have, its been this way for a while, 
nothing actively breaks one way or the other, and I don’t think there is a specific
security concern. It’s trivial enough though so it could easily just go the other way. 

Will defer to Josh/the crowd if I’m missing something though.
Alexey Dobriyan May 31, 2023, 7:42 p.m. UTC | #3 
On Wed, May 31, 2023 at 11:20:31AM -0700, Sean Christopherson wrote:
> On Wed, May 31, 2023, Jon Kohler wrote:
> > Commit 8bd200d23ec4 ("KVM: VMX: Flatten __vmx_vcpu_run()") changed
> > vmx_vmexit from SYM_FUNC_START to SYM_INNER_LABEL, accidentally
> > removing 16 byte alignment as SYM_FUNC_START uses SYM_A_ALIGN and
> > SYM_INNER_LABEL does not. Josh mentioned [1] this was unintentional.
> 
> Anyone know if this is this stable material, or just nice to have?

Can this improve vmexit latency? I didn't measure it.
Jon Kohler May 31, 2023, 7:44 p.m. UTC | #4 
> On May 31, 2023, at 3:42 PM, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> 
> On Wed, May 31, 2023 at 11:20:31AM -0700, Sean Christopherson wrote:
>> On Wed, May 31, 2023, Jon Kohler wrote:
>>> Commit 8bd200d23ec4 ("KVM: VMX: Flatten __vmx_vcpu_run()") changed
>>> vmx_vmexit from SYM_FUNC_START to SYM_INNER_LABEL, accidentally
>>> removing 16 byte alignment as SYM_FUNC_START uses SYM_A_ALIGN and
>>> SYM_INNER_LABEL does not. Josh mentioned [1] this was unintentional.
>> 
>> Anyone know if this is this stable material, or just nice to have?
> 
> Can this improve vmexit latency? I didn't measure it.

Exit latency *appeared* to be the same before/after, might be a little bit better but
any improvement appeared to be lost in the noise. I didn’t see a regression though,
so thats nice. That puts it in the nice-to-have camp for me.

Jon
Josh Poimboeuf May 31, 2023, 10:48 p.m. UTC | #5 
On Wed, May 31, 2023 at 11:58:21AM -0400, Jon Kohler wrote:
> Commit 8bd200d23ec4 ("KVM: VMX: Flatten __vmx_vcpu_run()") changed
> vmx_vmexit from SYM_FUNC_START to SYM_INNER_LABEL, accidentally
> removing 16 byte alignment as SYM_FUNC_START uses SYM_A_ALIGN and
> SYM_INNER_LABEL does not. Josh mentioned [1] this was unintentional.
> 
> Fix by changing to SYM_INNER_LABEL_ALIGN instead.
> 
> [1] https://lore.kernel.org/lkml/Y3adkSe%2FJ70PqUyt@p183
> 
> Fixes: 8bd200d23ec4 ("KVM: VMX: Flatten __vmx_vcpu_run()")
> Signed-off-by: Jon Kohler <jon@nutanix.com>
> Suggested-by: Alexey Dobriyan <adobriyan@gmail.com>
> CC: Josh Poimboeuf <jpoimboe@kernel.org>

Acked-by: Josh Poimboeuf <jpoimboe@kernel.org>
Sean Christopherson June 2, 2023, 1:25 a.m. UTC | #6 
On Wed, 31 May 2023 11:58:21 -0400, Jon Kohler wrote:
> Commit 8bd200d23ec4 ("KVM: VMX: Flatten __vmx_vcpu_run()") changed
> vmx_vmexit from SYM_FUNC_START to SYM_INNER_LABEL, accidentally
> removing 16 byte alignment as SYM_FUNC_START uses SYM_A_ALIGN and
> SYM_INNER_LABEL does not. Josh mentioned [1] this was unintentional.
> 
> Fix by changing to SYM_INNER_LABEL_ALIGN instead.
> 
> [...]

Applied to kvm-x86 vmx, thanks!

[1/1] KVM: VMX: restore vmx_vmexit alignment
      https://github.com/kvm-x86/linux/commit/331f22976816

--
https://github.com/kvm-x86/linux/tree/next
https://github.com/kvm-x86/linux/tree/fixes
diff mbox series

Patch

diff --git a/arch/x86/kvm/vmx/vmenter.S b/arch/x86/kvm/vmx/vmenter.S
index 631fd7da2bc3..07e927d4d099 100644
--- a/arch/x86/kvm/vmx/vmenter.S
+++ b/arch/x86/kvm/vmx/vmenter.S
@@ -187,7 +187,7 @@  SYM_FUNC_START(__vmx_vcpu_run)
 	_ASM_EXTABLE(.Lvmresume, .Lfixup)
 	_ASM_EXTABLE(.Lvmlaunch, .Lfixup)
 
-SYM_INNER_LABEL(vmx_vmexit, SYM_L_GLOBAL)
+SYM_INNER_LABEL_ALIGN(vmx_vmexit, SYM_L_GLOBAL)
 
 	/* Restore unwind state from before the VMRESUME/VMLAUNCH. */
 	UNWIND_HINT_RESTORE