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 <[email protected]>
Suggested-by: Alexey Dobriyan <[email protected]>
CC: Josh Poimboeuf <[email protected]>
---
arch/x86/kvm/vmx/vmenter.S | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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
--
2.30.1 (Apple Git-130)
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?
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.
> On May 31, 2023, at 2:20 PM, Sean Christopherson <[email protected]> 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.
> On May 31, 2023, at 3:42 PM, Alexey Dobriyan <[email protected]> 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
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 <[email protected]>
> Suggested-by: Alexey Dobriyan <[email protected]>
> CC: Josh Poimboeuf <[email protected]>
Acked-by: Josh Poimboeuf <[email protected]>
--
Josh
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