Received-SPF: pass (google.com: domain of linux-kernel+bounces-215190-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Fri, 14 Jun 2024 17:58:04 +0200
From: Borislav Petkov <bp@alien8.de>
To: Sean Christopherson <seanjc@google.com>
Cc: kernel test robot <lkp@intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Josh Poimboeuf <jpoimboe@kernel.org>, oe-kbuild-all@lists.linux.dev,
	linux-kernel@vger.kernel.org, x86@kernel.org
Subject: Re: [tip:x86/alternatives 14/14] arch/x86/kvm/kvm.o: warning:
 objtool: .altinstr_replacement+0xc5: call without frame pointer save/setup
Message-ID: <20240614155804.GBZmxojDwTJig1ENBl@fat_crate.local>
References: <202406141648.jO9qNGLa-lkp@intel.com>
 <20240614152228.GAZmxgNPmozj7UzRdV@fat_crate.local>
 <ZmxjOxhPy67QOh5Q@google.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <ZmxjOxhPy67QOh5Q@google.com>

On Fri, Jun 14, 2024 at 08:35:23AM -0700, Sean Christopherson wrote:
> That function does create a stack frame, just without using FRAME_BEGIN/END so
> it's not dependent on using frame pointers.  Ironically, the intent is to try and
> placate objtool.

Sneaky.

> IIUC, this warning just stared showing up with the alternatives changes?

Yeah, perhaps the other one triggered this one too but it wasn't in the 0day
report. The two warnings I reproduced here are:

arch/x86/kvm/kvm.o: warning: objtool: .altinstr_replacement+0xc5: call without frame pointer save/setup
vmlinux.o: warning: objtool: .altinstr_replacement+0x2eb: call without frame pointer save/setup

and the other one is caused by the alternatives missing ASM_CALL_CONSTRAINT %rsp
handing into the inline asm.

> Does CALL_NOSPEC generate different code now?

Hm, so I've been staring at this for a long time today and maybe I've veered off
into the weeds already ... :-\

So we use the asm variant of that CALL_NOSPEC thing:

.globl vmx_do_interrupt_irqoff ; .balign 16, 0x90;; ; vmx_do_interrupt_irqoff: endbr64
 VMX_DO_EVENT_IRQOFF CALL_NOSPEC rdi
.type vmx_do_interrupt_irqoff STT_FUNC ; .set .L__sym_size_vmx_do_interrupt_irqoff, .-vmx_do_interrupt_irqoff ; .size vmx_do_interrupt_irqoff, .L__sym_size_vmx_do_interrupt_irqoff

with

.macro CALL_NOSPEC reg:req

 __CS_PREFIX \reg
 call __x86_indirect_thunk_\reg

.endm

so the end-result is, what, this perhaps?

ffffffff81070e20 <vmx_do_interrupt_irqoff>:
ffffffff81070e20:       f3 0f 1e fa             endbr64
ffffffff81070e24:       55                      push   %rbp
ffffffff81070e25:       48 89 e5                mov    %rsp,%rbp

<--- new stack frame

ffffffff81070e28:       48 83 e4 f0             and    $0xfffffffffffffff0,%rsp
ffffffff81070e2c:       6a 18                   push   $0x18
ffffffff81070e2e:       55                      push   %rbp
ffffffff81070e2f:       9c                      pushf
ffffffff81070e30:       6a 10                   push   $0x10

push things on the stack...

ffffffff81070e32:       e8 09 67 60 00          call   ffffffff81677540 <__x86_indirect_thunk_rdi>
ffffffff81070e37:       48 89 ec                mov    %rbp,%rsp
ffffffff81070e3a:       5d                      pop    %rbp

restore stack.

ffffffff81070e3b:       e9 30 68 60 00          jmp    ffffffff81677670 <__x86_return_thunk>

So that function has a proper stack frame.

Why doesn't objtool recognize that?

I betcha it is something with that has_valid_stack_frame() from

                        if (opts.stackval && func && !is_special_call(insn) &&
                            !has_valid_stack_frame(&state)) {
                                WARN_INSN(insn, "call without frame pointer save/setup");
                                return 1;
                        }

the objtool check. And I see "cfi" there and my brain says "nope". :)

Hmmm.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette