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

[Sean Christopherson <seanjc@google.com>]

On Tue, Jun 25, 2024, Borislav Petkov wrote:
> ---
> From: "Borislav Petkov (AMD)" <bp@alien8.de>
> Date: Tue, 18 Jun 2024 21:57:27 +0200
> Subject: [PATCH] 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 and ALT_OUTPUT_SP() courtesy of peterz. Also from him
> add some helpful debugging info to the documentation.
> 
> Now on to the explanations:
> 
> tl;dr: The alternatives macros are pretty fragile.
> 
> If I do ALT_OUTPUT_SP(output) in order to be able to package in a %rsp
> reference for objtool so that a stack frame gets properly generated, the
> inline asm input operand with positional argument 0 in clear_page():
> 
> 	"0" (page)
> 
> gets "renumbered" due to the added
> 
> 	: "+r" (current_stack_pointer), "=D" (page)
> 
> and then gcc says:
> 
>   ./arch/x86/include/asm/page_64.h:53:9: error: inconsistent operand constraints in an ‘asm’
> 
> The fix is to use an explicit "D" constraint which points to a singleton
> register class (gcc terminology) which ends up doing what is expected
> here: the page pointer - input and output - should be in the same %rdi
> register.
> 
> Other register classes have more than one register in them - example:
> "r" and "=r" or "A":
> 
>   ‘A’
> 	The ‘a’ and ‘d’ registers.  This class is used for
> 	instructions that return double word results in the ‘ax:dx’
> 	register pair.  Single word values will be allocated either in
> 	‘ax’ or ‘dx’.
> 
> so using "D" and "=D" just works in this particular case.
> 
> And yes, one would say, sure, why don't you do "+D" but then:
> 
>         : "+r" (current_stack_pointer), "+D" (page)
>         : [old] "i" (clear_page_orig), [new1] "i" (clear_page_rep), [new2] "i" (clear_page_erms),
>         : "cc", "memory", "rax", "rcx")
> 
> now find the Waldo^Wcomma which throws a wrench into all this.
> 
> Because that silly macro has an "input..." consume-all last macro arg
> and in it, one is supposed to supply input *and* clobbers, leading to
> silly syntax snafus.
> 
> Yap, they need to be cleaned up, one fine day...
> 
> Cc: Michael Matz <matz@suse.de>
> 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>