From 0d3db1f14abb4eb28613fbeb1e2ad92bac76debf Mon Sep 17 00:00:00 2001 From: "Borislav Petkov (AMD)" Date: Tue, 18 Jun 2024 21:57:27 +0200 Subject: x86/alternatives, kvm: Fix a couple of CALLs without a frame pointer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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... Closes: https://lore.kernel.org/oe-kbuild-all/202406141648.jO9qNGLa-lkp@intel.com/ Reported-by: kernel test robot Signed-off-by: Borislav Petkov (AMD) Acked-by: Sean Christopherson Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/r/20240625112056.GDZnqoGDXgYuWBDUwu@fat_crate.local --- tools/objtool/Documentation/objtool.txt | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'tools/objtool') diff --git a/tools/objtool/Documentation/objtool.txt b/tools/objtool/Documentation/objtool.txt index fe39c2a8ef0d..7c3ee959b63c 100644 --- a/tools/objtool/Documentation/objtool.txt +++ b/tools/objtool/Documentation/objtool.txt @@ -284,6 +284,25 @@ the objtool maintainers. Otherwise the stack frame may not get created before the call. + objtool can help with pinpointing the exact function where it happens: + + $ OBJTOOL_ARGS="--verbose" make arch/x86/kvm/ + + arch/x86/kvm/kvm.o: warning: objtool: .altinstr_replacement+0xc5: call without frame pointer save/setup + arch/x86/kvm/kvm.o: warning: objtool: em_loop.part.0+0x29: (alt) + arch/x86/kvm/kvm.o: warning: objtool: em_loop.part.0+0x0: <=== (sym) + LD [M] arch/x86/kvm/kvm-intel.o + 0000 0000000000028220 : + 0000 28220: 0f b6 47 61 movzbl 0x61(%rdi),%eax + 0004 28224: 3c e2 cmp $0xe2,%al + 0006 28226: 74 2c je 28254 + 0008 28228: 48 8b 57 10 mov 0x10(%rdi),%rdx + 000c 2822c: 83 f0 05 xor $0x5,%eax + 000f 2822f: 48 c1 e0 04 shl $0x4,%rax + 0013 28233: 25 f0 00 00 00 and $0xf0,%eax + 0018 28238: 81 e2 d5 08 00 00 and $0x8d5,%edx + 001e 2823e: 80 ce 02 or $0x2,%dh + ... 2. file.o: warning: objtool: .text+0x53: unreachable instruction -- cgit v1.2.3