Re: ERROR: modpost: "kmsan_handle_dma" [drivers/virtio/virtio_ring.ko] undefined!

[Josh Poimboeuf]

On Tue, Feb 18, 2025 at 08:47:26AM -0800, Josh Poimboeuf wrote:
> On Tue, Feb 18, 2025 at 02:28:55PM +0100, Peter Zijlstra wrote:
> > On Tue, Feb 18, 2025 at 12:48:57PM +0100, Sebastian Andrzej Siewior wrote:
> > > On 2025-02-15 06:42:36 [+0800], kernel test robot wrote:
> > > > >> arch/x86/kvm/cpuid.o: warning: objtool: do_cpuid_func+0x2428: undefined stack state
> > > 
> > 
> > > From the assembly it seems to make sense:
> > > |   110ae:       49 89 e0                mov    %rsp,%r8
> > > stash for later
> > > |   110b1:       48 85 db                test   %rbx,%rbx
> > > |   110b4:       c7 00 00 00 00 00       movl   $0x0,(%rax)
> > > |   110ba:       45 89 7e 14             mov    %r15d,0x14(%r14)
> > > |   110be:       0f 85 40 01 00 00       jne    11204 <do_cpuid_func+0x22f4>
> > > …
> > > |   11204:       44 8b 74 24 38          mov    0x38(%rsp),%r14d
> > > |   11209:       44 89 f7                mov    %r14d,%edi
> > > |   1120c:       4d 89 c7                mov    %r8,%r15

[ Adding Masahiro for #1 ]

1)

Masahiro, one problem we've seen is that "make LLVM=1
arch/x86/kvm/cpuid.s" doesn't match the final compiled .o binary.  Is it
a problem with cmd_cc_s_c?  The original .config is here:

  https://lore.kernel.org/202502150634.qjxwSeJR-lkp@xxxxxxxxx

If I manually run the original gcc command with -S, cpuid.s looks right.

2)

Peter, Sebastian, the objtool warning is caused by Clang interpreting
ASM_CALL_CONSTRAINT a little too literally.  It's stashing the stack
pointer and restoring it later to comply with the "+r" rsp constraint.
Which makes perfect sense, but is also 100% unexpected.

I "fixed" it with the below, but it's not a proper fix as
ASM_CALL_CONSTRAINT is still needed for CONFIG_UNWINDER_FRAME_POINTER.

So I think the options are:

  - Make objtool smarter

  - Improve ASM_CALL_CONSTRAINT somehow.  /me wonders if "memory"
    clobber has the same effect?

  - Drop support for x86-64 frame pointers altogether, along with
    ASM_CALL_CONSTRAINT.  Is there any reason to keep frame pointers
    around anymore for 64 bit?  AFAICT objtool is 100% mandatory for all
    configs, and ORC has proven to be solid.

diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index fea56b04f436..7414f0c7cc0e 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -394,7 +394,7 @@ int paravirt_disable_iospace(void);
 		PVOP_TEST_NULL(op);					\
 		asm volatile(ALTERNATIVE(PARAVIRT_CALL, ALT_CALL_INSTR,	\
 				ALT_CALL_ALWAYS)			\
-			     : call_clbr, ASM_CALL_CONSTRAINT		\
+			     : call_clbr				\
 			     : paravirt_ptr(op),			\
 			       ##__VA_ARGS__				\
 			     : "memory", "cc" extra_clbr);		\
@@ -409,7 +409,7 @@ int paravirt_disable_iospace(void);
 		asm volatile(ALTERNATIVE_2(PARAVIRT_CALL,		\
 				 ALT_CALL_INSTR, ALT_CALL_ALWAYS,	\
 				 alt, cond)				\
-			     : call_clbr, ASM_CALL_CONSTRAINT		\
+			     : call_clbr				\
 			     : paravirt_ptr(op),			\
 			       ##__VA_ARGS__				\
 			     : "memory", "cc" extra_clbr);		\

-- 
Josh