Re: [tip: x86/alternatives] x86/alternatives, kvm: Fix a couple of CALLs without a frame pointer

[Borislav Petkov <bp@alien8.de>]

On Wed, Jun 19, 2024 at 08:39:52AM -0000, tip-bot2 for Borislav Petkov (AMD) wrote:
> The following commit has been merged into the x86/alternatives branch of tip:
> 
> Commit-ID:     93f78dadee5e56ae48aff567583d503868aa3bf2
> Gitweb:        https://git.kernel.org/tip/93f78dadee5e56ae48aff567583d503868aa3bf2
> Author:        Borislav Petkov (AMD) <bp@alien8.de>
> AuthorDate:    Tue, 18 Jun 2024 21:57:27 +02:00
> Committer:     Borislav Petkov (AMD) <bp@alien8.de>
> CommitterDate: Wed, 19 Jun 2024 10:33:25 +02:00
> 
> x86/alternatives, kvm: Fix a couple of CALLs without a frame pointer
> 
> objtool complains:
> 
>   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
> 
> Make sure rSP is an output operand to the respective asm() statements.
> 
> The test_cc() hunk courtesy of peterz. Also from him add some helpful
> debugging info to the documentation.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202406141648.jO9qNGLa-lkp@intel.com/
> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
> Acked-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/include/asm/alternative.h      |  2 +-
>  arch/x86/kernel/alternative.c           |  2 +-
>  arch/x86/kvm/emulate.c                  |  2 +-
>  tools/objtool/Documentation/objtool.txt | 19 +++++++++++++++++++
>  4 files changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h
> index 89fa50d..8cff462 100644
> --- a/arch/x86/include/asm/alternative.h
> +++ b/arch/x86/include/asm/alternative.h
> @@ -248,7 +248,7 @@ static inline int alternatives_text_reserved(void *start, void *end)
>   */
>  #define alternative_call(oldfunc, newfunc, ft_flags, output, input...)	\
>  	asm_inline volatile(ALTERNATIVE("call %c[old]", "call %c[new]", ft_flags) \
> -		: output : [old] "i" (oldfunc), [new] "i" (newfunc), ## input)
> +		: output, ASM_CALL_CONSTRAINT : [old] "i" (oldfunc), [new] "i" (newfunc), ## input)

Yeah, this doesn't fly currently:

https://lore.kernel.org/r/202406200507.AXxJ6Bmw-lkp@intel.com

because those atomic64_32.h macros do

        alternative_atomic64(set, /* no output */,
                             "S" (v), "b" (low), "c" (high)

so without an output, it ends up becoming:

asm __inline volatile("# ALT: oldinstr\n" ... ".popsection\n" : , "+r" (current_stack_pointer) : [old] "i" ...

note the preceding ",".

And I can't do "output..." macro argument with ellipsis and paste with "##
output" because "input..." already does that. :-\

So I am not sure what to do here. Removing the ASM_CALL_CONSTRAINT works,
let's see whether it passes build tests.

Or add dummy output arguments to the three atomic macros which have no
output?

Hm.

-- 
Regards/Gruss,
    Boris.

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