On Mon, Jun 27, 2022 at 8:08 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > On Wed, Jun 22, 2022, Jim Mattson wrote: > > RSB-stuffing after VM-exit is only needed for legacy CPUs without > > eIBRS. Move the RSB-stuffing code out of line. > > I assume the primary justification is purely to avoid the JMP on modern CPUs? > > > Preserve the non-sensical correlation of RSB-stuffing with retpoline. > > Either omit the blurb about retpoline, or better yet expand on why it's nonsensical > and speculate a bit on why it got tied to retpoline? I can expand on why it's nonsensical. I cannot speculate on why it got tied to retpoline, but perhaps someone on this list knows? > > Signed-off-by: Jim Mattson <jmattson@xxxxxxxxxx> > > --- > > arch/x86/kvm/vmx/vmenter.S | 10 +++++++--- > > 1 file changed, 7 insertions(+), 3 deletions(-) > > > > diff --git a/arch/x86/kvm/vmx/vmenter.S b/arch/x86/kvm/vmx/vmenter.S > > index 435c187927c4..39009a4c86bd 100644 > > --- a/arch/x86/kvm/vmx/vmenter.S > > +++ b/arch/x86/kvm/vmx/vmenter.S > > @@ -76,7 +76,12 @@ SYM_FUNC_END(vmx_vmenter) > > */ > > SYM_FUNC_START(vmx_vmexit) > > #ifdef CONFIG_RETPOLINE > > - ALTERNATIVE "jmp .Lvmexit_skip_rsb", "", X86_FEATURE_RETPOLINE > > + ALTERNATIVE "", "jmp .Lvmexit_stuff_rsb", X86_FEATURE_RETPOLINE > > +#endif > > +.Lvmexit_return: > > + RET > > +#ifdef CONFIG_RETPOLINE > > +.Lvmexit_stuff_rsb: > > /* Preserve guest's RAX, it's used to stuff the RSB. */ > > push %_ASM_AX > > There's a comment in the code here about stuffiing before RET, I think it makes > sense to keep that before the RET, i.e. hoist it out of the actual stuffing > sequence so that it looks like: > > #ifdef CONFIG_RETPOLINE > /* IMPORTANT: Stuff the RSB immediately after VM-Exit, before RET! */ > ALTERNATIVE "", "jmp .Lvmexit_stuff_rsb", X86_FEATURE_RETPOLINE > #endif > .Lvmexit_return: > RET > > Ha! Better idea. Rather than have a bunch of nops to eat through before the > !RETPOLINE case gets to the RET, encode the RET as the default. That allows using > a single #ifdef and avoids both the JMP over the RET as well as the JMP back to the > RET, and saves 4 nops to boot. I had considered that option, but I doubt that it will be long before someone wants to undo it. In any case, I will make that change in version 2. Thanks! --jim
