public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [tip:x86/tdx 8/12] vmlinux.o: warning: objtool: __tdx_hypercall+0x128: __tdx_hypercall_failed() is missing a __noreturn annotation
@ 2023-09-14  1:05 kernel test robot
  2023-09-14  1:23 ` Huang, Kai
  0 siblings, 1 reply; 12+ messages in thread
From: kernel test robot @ 2023-09-14  1:05 UTC (permalink / raw)
  To: Kai Huang
  Cc: oe-kbuild-all, linux-kernel, x86, Dave Hansen, Kirill A. Shutemov

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/tdx
head:   7b804135d4d1f0a2b9dda69c6303d3f2dcbe9d37
commit: c641cfb5c157b6c3062a824fd8ba190bf06fb952 [8/12] x86/tdx: Make TDX_HYPERCALL asm similar to TDX_MODULE_CALL
config: x86_64-rhel-8.3-bpf (https://download.01.org/0day-ci/archive/20230914/202309140828.9RdmlH2Z-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230914/202309140828.9RdmlH2Z-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309140828.9RdmlH2Z-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> vmlinux.o: warning: objtool: __tdx_hypercall+0x128: __tdx_hypercall_failed() is missing a __noreturn annotation


objdump-func vmlinux.o __tdx_hypercall:
0000 0000000000000050 <__tdx_hypercall_failed>:
0000       50:	f3 0f 1e fa          	endbr64
0004       54:	48 c7 c7 00 00 00 00 	mov    $0x0,%rdi	57: R_X86_64_32S	.rodata.str1.8+0xe18
000b       5b:	e8 00 00 00 00       	call   60 <__tdx_hypercall>	5c: R_X86_64_PLT32	panic-0x4
0000 0000000000000060 <__tdx_hypercall>:
0000       60:	f3 0f 1e fa          	endbr64
0004       64:	53                   	push   %rbx
0005       65:	48 89 fb             	mov    %rdi,%rbx
0008       68:	48 83 ec 70          	sub    $0x70,%rsp
000c       6c:	65 48 8b 04 25 28 00 00 00 	mov    %gs:0x28,%rax
0015       75:	48 89 44 24 68       	mov    %rax,0x68(%rsp)
001a       7a:	31 c0                	xor    %eax,%eax
001c       7c:	48 8b 47 58          	mov    0x58(%rdi),%rax
0020       80:	48 89 e6             	mov    %rsp,%rsi
0023       83:	48 c7 04 24 cc ff 00 00 	movq   $0xffcc,(%rsp)
002b       8b:	48 89 44 24 08       	mov    %rax,0x8(%rsp)
0030       90:	48 8b 07             	mov    (%rdi),%rax
0033       93:	48 89 44 24 10       	mov    %rax,0x10(%rsp)
0038       98:	48 8b 47 08          	mov    0x8(%rdi),%rax
003c       9c:	48 89 44 24 18       	mov    %rax,0x18(%rsp)
0041       a1:	48 8b 47 10          	mov    0x10(%rdi),%rax
0045       a5:	48 89 44 24 20       	mov    %rax,0x20(%rsp)
004a       aa:	48 8b 47 18          	mov    0x18(%rdi),%rax
004e       ae:	48 89 44 24 28       	mov    %rax,0x28(%rsp)
0053       b3:	48 8b 47 20          	mov    0x20(%rdi),%rax
0057       b7:	48 89 44 24 30       	mov    %rax,0x30(%rsp)
005c       bc:	48 8b 47 28          	mov    0x28(%rdi),%rax
0060       c0:	48 89 44 24 38       	mov    %rax,0x38(%rsp)
0065       c5:	48 8b 47 30          	mov    0x30(%rdi),%rax
0069       c9:	48 89 44 24 40       	mov    %rax,0x40(%rsp)
006e       ce:	48 8b 47 38          	mov    0x38(%rdi),%rax
0072       d2:	48 89 44 24 48       	mov    %rax,0x48(%rsp)
0077       d7:	48 8b 47 50          	mov    0x50(%rdi),%rax
007b       db:	48 89 44 24 50       	mov    %rax,0x50(%rsp)
0080       e0:	48 8b 47 40          	mov    0x40(%rdi),%rax
0084       e4:	48 89 44 24 58       	mov    %rax,0x58(%rsp)
0089       e9:	48 8b 47 48          	mov    0x48(%rdi),%rax
008d       ed:	31 ff                	xor    %edi,%edi
008f       ef:	48 89 44 24 60       	mov    %rax,0x60(%rsp)
0094       f4:	e8 00 00 00 00       	call   f9 <__tdx_hypercall+0x99>	f5: R_X86_64_PLT32	__tdcall_hypercall-0x4
0099       f9:	48 85 c0             	test   %rax,%rax
009c       fc:	0f 85 81 00 00 00    	jne    183 <__tdx_hypercall+0x123>
00a2      102:	48 8b 54 24 28       	mov    0x28(%rsp),%rdx
00a7      107:	48 8b 44 24 10       	mov    0x10(%rsp),%rax
00ac      10c:	48 89 53 18          	mov    %rdx,0x18(%rbx)
00b0      110:	48 8b 54 24 30       	mov    0x30(%rsp),%rdx
00b5      115:	48 89 03             	mov    %rax,(%rbx)
00b8      118:	48 8b 44 24 18       	mov    0x18(%rsp),%rax
00bd      11d:	48 89 53 20          	mov    %rdx,0x20(%rbx)
00c1      121:	48 8b 54 24 38       	mov    0x38(%rsp),%rdx
00c6      126:	48 89 43 08          	mov    %rax,0x8(%rbx)
00ca      12a:	48 8b 44 24 20       	mov    0x20(%rsp),%rax
00cf      12f:	48 89 53 28          	mov    %rdx,0x28(%rbx)
00d3      133:	48 8b 54 24 40       	mov    0x40(%rsp),%rdx
00d8      138:	48 89 43 10          	mov    %rax,0x10(%rbx)
00dc      13c:	48 89 53 30          	mov    %rdx,0x30(%rbx)
00e0      140:	48 8b 54 24 48       	mov    0x48(%rsp),%rdx
00e5      145:	48 89 53 38          	mov    %rdx,0x38(%rbx)
00e9      149:	48 8b 54 24 58       	mov    0x58(%rsp),%rdx
00ee      14e:	48 89 53 40          	mov    %rdx,0x40(%rbx)
00f2      152:	48 8b 54 24 60       	mov    0x60(%rsp),%rdx
00f7      157:	48 89 53 48          	mov    %rdx,0x48(%rbx)
00fb      15b:	48 8b 54 24 50       	mov    0x50(%rsp),%rdx
0100      160:	48 89 53 50          	mov    %rdx,0x50(%rbx)
0104      164:	48 8b 54 24 08       	mov    0x8(%rsp),%rdx
0109      169:	48 89 53 58          	mov    %rdx,0x58(%rbx)
010d      16d:	48 8b 54 24 68       	mov    0x68(%rsp),%rdx
0112      172:	65 48 2b 14 25 28 00 00 00 	sub    %gs:0x28,%rdx
011b      17b:	75 10                	jne    18d <__tdx_hypercall+0x12d>
011d      17d:	48 83 c4 70          	add    $0x70,%rsp
0121      181:	5b                   	pop    %rbx
0122      182:	c3                   	ret
0123      183:	e8 00 00 00 00       	call   188 <__tdx_hypercall+0x128>	184: R_X86_64_PLT32	__tdx_hypercall_failed-0x4
0128      188:	e9 75 ff ff ff       	jmp    102 <__tdx_hypercall+0xa2>
012d      18d:	e8 00 00 00 00       	call   192 <__tdx_hypercall+0x132>	18e: R_X86_64_PLT32	__stack_chk_fail-0x4
0132      192:	66 2e 0f 1f 84 00 00 00 00 00 	cs nopw 0x0(%rax,%rax,1)
013c      19c:	0f 1f 40 00          	nopl   0x0(%rax)

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [tip:x86/tdx 8/12] vmlinux.o: warning: objtool: __tdx_hypercall+0x128: __tdx_hypercall_failed() is missing a __noreturn annotation
  2023-09-14  1:05 [tip:x86/tdx 8/12] vmlinux.o: warning: objtool: __tdx_hypercall+0x128: __tdx_hypercall_failed() is missing a __noreturn annotation kernel test robot
@ 2023-09-14  1:23 ` Huang, Kai
  2023-09-14  3:21   ` Huang, Kai
  0 siblings, 1 reply; 12+ messages in thread
From: Huang, Kai @ 2023-09-14  1:23 UTC (permalink / raw)
  To: lkp
  Cc: kirill.shutemov@linux.intel.com, linux-kernel@vger.kernel.org,
	oe-kbuild-all@lists.linux.dev, x86@kernel.org,
	dave.hansen@linux.intel.com

On Thu, 2023-09-14 at 09:05 +0800, kernel test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/tdx
> head:   7b804135d4d1f0a2b9dda69c6303d3f2dcbe9d37
> commit: c641cfb5c157b6c3062a824fd8ba190bf06fb952 [8/12] x86/tdx: Make TDX_HYPERCALL asm similar to TDX_MODULE_CALL
> config: x86_64-rhel-8.3-bpf (https://download.01.org/0day-ci/archive/20230914/202309140828.9RdmlH2Z-lkp@intel.com/config)
> compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230914/202309140828.9RdmlH2Z-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202309140828.9RdmlH2Z-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
> > > vmlinux.o: warning: objtool: __tdx_hypercall+0x128: __tdx_hypercall_failed() is missing a __noreturn annotation
> 

Hmm.. The __tdx_hypercall_failed() is already annotated with __noreturn (I
explicitly added it to silent such warning):

/* Called from __tdx_hypercall() for unrecoverable failure */
noinstr void __noreturn __tdx_hypercall_failed(void)
{
        instrumentation_begin();
        panic("TDVMCALL failed. TDX module bug?");
}

Not sure why the objtool is still complaining this?

> 
> objdump-func vmlinux.o __tdx_hypercall:
> 0000 0000000000000050 <__tdx_hypercall_failed>:
> 0000       50:	f3 0f 1e fa          	endbr64
> 0004       54:	48 c7 c7 00 00 00 00 	mov    $0x0,%rdi	57: R_X86_64_32S	.rodata.str1.8+0xe18
> 000b       5b:	e8 00 00 00 00       	call   60 <__tdx_hypercall>	5c: R_X86_64_PLT32	panic-0x4
> 0000 0000000000000060 <__tdx_hypercall>:
> 0000       60:	f3 0f 1e fa          	endbr64
> 0004       64:	53                   	push   %rbx
> 0005       65:	48 89 fb             	mov    %rdi,%rbx
> 0008       68:	48 83 ec 70          	sub    $0x70,%rsp
> 000c       6c:	65 48 8b 04 25 28 00 00 00 	mov    %gs:0x28,%rax
> 0015       75:	48 89 44 24 68       	mov    %rax,0x68(%rsp)
> 001a       7a:	31 c0                	xor    %eax,%eax
> 001c       7c:	48 8b 47 58          	mov    0x58(%rdi),%rax
> 0020       80:	48 89 e6             	mov    %rsp,%rsi
> 0023       83:	48 c7 04 24 cc ff 00 00 	movq   $0xffcc,(%rsp)
> 002b       8b:	48 89 44 24 08       	mov    %rax,0x8(%rsp)
> 0030       90:	48 8b 07             	mov    (%rdi),%rax
> 0033       93:	48 89 44 24 10       	mov    %rax,0x10(%rsp)
> 0038       98:	48 8b 47 08          	mov    0x8(%rdi),%rax
> 003c       9c:	48 89 44 24 18       	mov    %rax,0x18(%rsp)
> 0041       a1:	48 8b 47 10          	mov    0x10(%rdi),%rax
> 0045       a5:	48 89 44 24 20       	mov    %rax,0x20(%rsp)
> 004a       aa:	48 8b 47 18          	mov    0x18(%rdi),%rax
> 004e       ae:	48 89 44 24 28       	mov    %rax,0x28(%rsp)
> 0053       b3:	48 8b 47 20          	mov    0x20(%rdi),%rax
> 0057       b7:	48 89 44 24 30       	mov    %rax,0x30(%rsp)
> 005c       bc:	48 8b 47 28          	mov    0x28(%rdi),%rax
> 0060       c0:	48 89 44 24 38       	mov    %rax,0x38(%rsp)
> 0065       c5:	48 8b 47 30          	mov    0x30(%rdi),%rax
> 0069       c9:	48 89 44 24 40       	mov    %rax,0x40(%rsp)
> 006e       ce:	48 8b 47 38          	mov    0x38(%rdi),%rax
> 0072       d2:	48 89 44 24 48       	mov    %rax,0x48(%rsp)
> 0077       d7:	48 8b 47 50          	mov    0x50(%rdi),%rax
> 007b       db:	48 89 44 24 50       	mov    %rax,0x50(%rsp)
> 0080       e0:	48 8b 47 40          	mov    0x40(%rdi),%rax
> 0084       e4:	48 89 44 24 58       	mov    %rax,0x58(%rsp)
> 0089       e9:	48 8b 47 48          	mov    0x48(%rdi),%rax
> 008d       ed:	31 ff                	xor    %edi,%edi
> 008f       ef:	48 89 44 24 60       	mov    %rax,0x60(%rsp)
> 0094       f4:	e8 00 00 00 00       	call   f9 <__tdx_hypercall+0x99>	f5: R_X86_64_PLT32	__tdcall_hypercall-0x4
> 0099       f9:	48 85 c0             	test   %rax,%rax
> 009c       fc:	0f 85 81 00 00 00    	jne    183 <__tdx_hypercall+0x123>
> 00a2      102:	48 8b 54 24 28       	mov    0x28(%rsp),%rdx
> 00a7      107:	48 8b 44 24 10       	mov    0x10(%rsp),%rax
> 00ac      10c:	48 89 53 18          	mov    %rdx,0x18(%rbx)
> 00b0      110:	48 8b 54 24 30       	mov    0x30(%rsp),%rdx
> 00b5      115:	48 89 03             	mov    %rax,(%rbx)
> 00b8      118:	48 8b 44 24 18       	mov    0x18(%rsp),%rax
> 00bd      11d:	48 89 53 20          	mov    %rdx,0x20(%rbx)
> 00c1      121:	48 8b 54 24 38       	mov    0x38(%rsp),%rdx
> 00c6      126:	48 89 43 08          	mov    %rax,0x8(%rbx)
> 00ca      12a:	48 8b 44 24 20       	mov    0x20(%rsp),%rax
> 00cf      12f:	48 89 53 28          	mov    %rdx,0x28(%rbx)
> 00d3      133:	48 8b 54 24 40       	mov    0x40(%rsp),%rdx
> 00d8      138:	48 89 43 10          	mov    %rax,0x10(%rbx)
> 00dc      13c:	48 89 53 30          	mov    %rdx,0x30(%rbx)
> 00e0      140:	48 8b 54 24 48       	mov    0x48(%rsp),%rdx
> 00e5      145:	48 89 53 38          	mov    %rdx,0x38(%rbx)
> 00e9      149:	48 8b 54 24 58       	mov    0x58(%rsp),%rdx
> 00ee      14e:	48 89 53 40          	mov    %rdx,0x40(%rbx)
> 00f2      152:	48 8b 54 24 60       	mov    0x60(%rsp),%rdx
> 00f7      157:	48 89 53 48          	mov    %rdx,0x48(%rbx)
> 00fb      15b:	48 8b 54 24 50       	mov    0x50(%rsp),%rdx
> 0100      160:	48 89 53 50          	mov    %rdx,0x50(%rbx)
> 0104      164:	48 8b 54 24 08       	mov    0x8(%rsp),%rdx
> 0109      169:	48 89 53 58          	mov    %rdx,0x58(%rbx)
> 010d      16d:	48 8b 54 24 68       	mov    0x68(%rsp),%rdx
> 0112      172:	65 48 2b 14 25 28 00 00 00 	sub    %gs:0x28,%rdx
> 011b      17b:	75 10                	jne    18d <__tdx_hypercall+0x12d>
> 011d      17d:	48 83 c4 70          	add    $0x70,%rsp
> 0121      181:	5b                   	pop    %rbx
> 0122      182:	c3                   	ret
> 0123      183:	e8 00 00 00 00       	call   188 <__tdx_hypercall+0x128>	184: R_X86_64_PLT32	__tdx_hypercall_failed-0x4
> 0128      188:	e9 75 ff ff ff       	jmp    102 <__tdx_hypercall+0xa2>
> 012d      18d:	e8 00 00 00 00       	call   192 <__tdx_hypercall+0x132>	18e: R_X86_64_PLT32	__stack_chk_fail-0x4
> 0132      192:	66 2e 0f 1f 84 00 00 00 00 00 	cs nopw 0x0(%rax,%rax,1)
> 013c      19c:	0f 1f 40 00          	nopl   0x0(%rax)
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [tip:x86/tdx 8/12] vmlinux.o: warning: objtool: __tdx_hypercall+0x128: __tdx_hypercall_failed() is missing a __noreturn annotation
  2023-09-14  1:23 ` Huang, Kai
@ 2023-09-14  3:21   ` Huang, Kai
  2023-09-14  7:29     ` Peter Zijlstra
  0 siblings, 1 reply; 12+ messages in thread
From: Huang, Kai @ 2023-09-14  3:21 UTC (permalink / raw)
  To: lkp
  Cc: kirill.shutemov@linux.intel.com, linux-kernel@vger.kernel.org,
	oe-kbuild-all@lists.linux.dev, x86@kernel.org,
	dave.hansen@linux.intel.com

On Thu, 2023-09-14 at 01:23 +0000, Huang, Kai wrote:
> On Thu, 2023-09-14 at 09:05 +0800, kernel test robot wrote:
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/tdx
> > head:   7b804135d4d1f0a2b9dda69c6303d3f2dcbe9d37
> > commit: c641cfb5c157b6c3062a824fd8ba190bf06fb952 [8/12] x86/tdx: Make TDX_HYPERCALL asm similar to TDX_MODULE_CALL
> > config: x86_64-rhel-8.3-bpf (https://download.01.org/0day-ci/archive/20230914/202309140828.9RdmlH2Z-lkp@intel.com/config)
> > compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230914/202309140828.9RdmlH2Z-lkp@intel.com/reproduce)
> > 
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > Closes: https://lore.kernel.org/oe-kbuild-all/202309140828.9RdmlH2Z-lkp@intel.com/
> > 
> > All warnings (new ones prefixed by >>):
> > 
> > > > vmlinux.o: warning: objtool: __tdx_hypercall+0x128: __tdx_hypercall_failed() is missing a __noreturn annotation
> > 
> 
> Hmm.. The __tdx_hypercall_failed() is already annotated with __noreturn (I
> explicitly added it to silent such warning):
> 
> /* Called from __tdx_hypercall() for unrecoverable failure */
> noinstr void __noreturn __tdx_hypercall_failed(void)
> {
>         instrumentation_begin();
>         panic("TDVMCALL failed. TDX module bug?");
> }
> 
> Not sure why the objtool is still complaining this?
> 

It appears the __noreturn must be annotated to the function declaration but not
the function body.  I'll send out the fix as soon as I confirm the fix with LKP.

Sorry for the noise.
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [tip:x86/tdx 8/12] vmlinux.o: warning: objtool: __tdx_hypercall+0x128: __tdx_hypercall_failed() is missing a __noreturn annotation
  2023-09-14  3:21   ` Huang, Kai
@ 2023-09-14  7:29     ` Peter Zijlstra
  2023-09-14  7:54       ` Huang, Kai
  2023-09-14 14:16       ` Ingo Molnar
  0 siblings, 2 replies; 12+ messages in thread
From: Peter Zijlstra @ 2023-09-14  7:29 UTC (permalink / raw)
  To: Huang, Kai
  Cc: lkp, kirill.shutemov@linux.intel.com,
	linux-kernel@vger.kernel.org, oe-kbuild-all@lists.linux.dev,
	x86@kernel.org, dave.hansen@linux.intel.com

On Thu, Sep 14, 2023 at 03:21:29AM +0000, Huang, Kai wrote:
> On Thu, 2023-09-14 at 01:23 +0000, Huang, Kai wrote:
> > On Thu, 2023-09-14 at 09:05 +0800, kernel test robot wrote:
> > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/tdx
> > > head:   7b804135d4d1f0a2b9dda69c6303d3f2dcbe9d37
> > > commit: c641cfb5c157b6c3062a824fd8ba190bf06fb952 [8/12] x86/tdx: Make TDX_HYPERCALL asm similar to TDX_MODULE_CALL
> > > config: x86_64-rhel-8.3-bpf (https://download.01.org/0day-ci/archive/20230914/202309140828.9RdmlH2Z-lkp@intel.com/config)
> > > compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> > > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230914/202309140828.9RdmlH2Z-lkp@intel.com/reproduce)
> > > 
> > > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > > the same patch/commit), kindly add following tags
> > > > Reported-by: kernel test robot <lkp@intel.com>
> > > > Closes: https://lore.kernel.org/oe-kbuild-all/202309140828.9RdmlH2Z-lkp@intel.com/
> > > 
> > > All warnings (new ones prefixed by >>):
> > > 
> > > > > vmlinux.o: warning: objtool: __tdx_hypercall+0x128: __tdx_hypercall_failed() is missing a __noreturn annotation
> > > 
> > 
> > Hmm.. The __tdx_hypercall_failed() is already annotated with __noreturn (I
> > explicitly added it to silent such warning):
> > 
> > /* Called from __tdx_hypercall() for unrecoverable failure */
> > noinstr void __noreturn __tdx_hypercall_failed(void)
> > {
> >         instrumentation_begin();
> >         panic("TDVMCALL failed. TDX module bug?");
> > }
> > 
> > Not sure why the objtool is still complaining this?
> > 
> 
> It appears the __noreturn must be annotated to the function declaration but not
> the function body.  I'll send out the fix as soon as I confirm the fix with LKP.

FWIW, the reason being that...

The point of noreturn is that the caller should know to stop generating
code. For that the declaration needs the attribute, because call sites
typically do not have access to the function definition in C.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [tip:x86/tdx 8/12] vmlinux.o: warning: objtool: __tdx_hypercall+0x128: __tdx_hypercall_failed() is missing a __noreturn annotation
  2023-09-14  7:29     ` Peter Zijlstra
@ 2023-09-14  7:54       ` Huang, Kai
  2023-09-14  8:59         ` Peter Zijlstra
  2023-09-14 14:16       ` Ingo Molnar
  1 sibling, 1 reply; 12+ messages in thread
From: Huang, Kai @ 2023-09-14  7:54 UTC (permalink / raw)
  To: peterz@infradead.org
  Cc: kirill.shutemov@linux.intel.com, dave.hansen@linux.intel.com,
	linux-kernel@vger.kernel.org, lkp, oe-kbuild-all@lists.linux.dev,
	x86@kernel.org

On Thu, 2023-09-14 at 09:29 +0200, Peter Zijlstra wrote:
> On Thu, Sep 14, 2023 at 03:21:29AM +0000, Huang, Kai wrote:
> > On Thu, 2023-09-14 at 01:23 +0000, Huang, Kai wrote:
> > > On Thu, 2023-09-14 at 09:05 +0800, kernel test robot wrote:
> > > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/tdx
> > > > head:   7b804135d4d1f0a2b9dda69c6303d3f2dcbe9d37
> > > > commit: c641cfb5c157b6c3062a824fd8ba190bf06fb952 [8/12] x86/tdx: Make TDX_HYPERCALL asm similar to TDX_MODULE_CALL
> > > > config: x86_64-rhel-8.3-bpf (https://download.01.org/0day-ci/archive/20230914/202309140828.9RdmlH2Z-lkp@intel.com/config)
> > > > compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> > > > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230914/202309140828.9RdmlH2Z-lkp@intel.com/reproduce)
> > > > 
> > > > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > > > the same patch/commit), kindly add following tags
> > > > > Reported-by: kernel test robot <lkp@intel.com>
> > > > > Closes: https://lore.kernel.org/oe-kbuild-all/202309140828.9RdmlH2Z-lkp@intel.com/
> > > > 
> > > > All warnings (new ones prefixed by >>):
> > > > 
> > > > > > vmlinux.o: warning: objtool: __tdx_hypercall+0x128: __tdx_hypercall_failed() is missing a __noreturn annotation
> > > > 
> > > 
> > > Hmm.. The __tdx_hypercall_failed() is already annotated with __noreturn (I
> > > explicitly added it to silent such warning):
> > > 
> > > /* Called from __tdx_hypercall() for unrecoverable failure */
> > > noinstr void __noreturn __tdx_hypercall_failed(void)
> > > {
> > >         instrumentation_begin();
> > >         panic("TDVMCALL failed. TDX module bug?");
> > > }
> > > 
> > > Not sure why the objtool is still complaining this?
> > > 
> > 
> > It appears the __noreturn must be annotated to the function declaration but not
> > the function body.  I'll send out the fix as soon as I confirm the fix with LKP.
> 
> FWIW, the reason being that...
> 
> The point of noreturn is that the caller should know to stop generating
> code. For that the declaration needs the attribute, because call sites
> typically do not have access to the function definition in C.

Ah that makes perfect sense.  Thanks!

Then I assume we don't need to annotate __noreturn in the function body, but
only in the declaration?  Because the compiler must already have seen the
declaration when it generates the code for the function body.

Btw, I happened to notice that the objtool documentation suggests that we should
also add the the function to tools/objtool/noreturns.h:

3. file.o: warning: objtool: foo+0x48c: bar() is missing a __noreturn annotation

   The call from foo() to bar() doesn't return, but bar() is missing the
   __noreturn annotation.  NOTE: In addition to annotating the function
   with __noreturn, please also add it to tools/objtool/noreturns.h.

Is it a behaviour that we still need to follow?

I am asking because the old kernel code doesn't have it so perhaps I am missing
something here.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [tip:x86/tdx 8/12] vmlinux.o: warning: objtool: __tdx_hypercall+0x128: __tdx_hypercall_failed() is missing a __noreturn annotation
  2023-09-14  7:54       ` Huang, Kai
@ 2023-09-14  8:59         ` Peter Zijlstra
  2023-09-14  9:18           ` Huang, Kai
  2023-09-14 10:02           ` Huang, Kai
  0 siblings, 2 replies; 12+ messages in thread
From: Peter Zijlstra @ 2023-09-14  8:59 UTC (permalink / raw)
  To: Huang, Kai
  Cc: kirill.shutemov@linux.intel.com, dave.hansen@linux.intel.com,
	linux-kernel@vger.kernel.org, lkp, oe-kbuild-all@lists.linux.dev,
	x86@kernel.org

On Thu, Sep 14, 2023 at 07:54:10AM +0000, Huang, Kai wrote:

> > The point of noreturn is that the caller should know to stop generating
> > code. For that the declaration needs the attribute, because call sites
> > typically do not have access to the function definition in C.
> 
> Ah that makes perfect sense.  Thanks!
> 
> Then I assume we don't need to annotate __noreturn in the function body, but
> only in the declaration?  Because the compiler must already have seen the
> declaration when it generates the code for the function body.

I think so, I'm sure it'll tell you if it disagrees :-)

> Btw, I happened to notice that the objtool documentation suggests that we should
> also add the the function to tools/objtool/noreturns.h:
> 
> 3. file.o: warning: objtool: foo+0x48c: bar() is missing a __noreturn annotation
> 
>    The call from foo() to bar() doesn't return, but bar() is missing the
>    __noreturn annotation.  NOTE: In addition to annotating the function
>    with __noreturn, please also add it to tools/objtool/noreturns.h.
> 
> Is it a behaviour that we still need to follow?

Yes. objtool has some heuristics to detect noreturn, but is is very
difficult. Sadly noreturn is not something that is reflected in the ELF
object file so we have to guess.

For now manually adding it to the objtool list is required, we're trying
to get to the point where it is generated/validated by the compiler,
perhaps with a plugin, but we're not there yet.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [tip:x86/tdx 8/12] vmlinux.o: warning: objtool: __tdx_hypercall+0x128: __tdx_hypercall_failed() is missing a __noreturn annotation
  2023-09-14  8:59         ` Peter Zijlstra
@ 2023-09-14  9:18           ` Huang, Kai
  2023-09-14 10:02           ` Huang, Kai
  1 sibling, 0 replies; 12+ messages in thread
From: Huang, Kai @ 2023-09-14  9:18 UTC (permalink / raw)
  To: peterz@infradead.org
  Cc: kirill.shutemov@linux.intel.com, oe-kbuild-all@lists.linux.dev,
	linux-kernel@vger.kernel.org, lkp, dave.hansen@linux.intel.com,
	x86@kernel.org

On Thu, 2023-09-14 at 10:59 +0200, Peter Zijlstra wrote:
> On Thu, Sep 14, 2023 at 07:54:10AM +0000, Huang, Kai wrote:
> 
> > > The point of noreturn is that the caller should know to stop generating
> > > code. For that the declaration needs the attribute, because call sites
> > > typically do not have access to the function definition in C.
> > 
> > Ah that makes perfect sense.  Thanks!
> > 
> > Then I assume we don't need to annotate __noreturn in the function body, but
> > only in the declaration?  Because the compiler must already have seen the
> > declaration when it generates the code for the function body.
> 
> I think so, I'm sure it'll tell you if it disagrees :-)
> 
> > Btw, I happened to notice that the objtool documentation suggests that we should
> > also add the the function to tools/objtool/noreturns.h:
> > 
> > 3. file.o: warning: objtool: foo+0x48c: bar() is missing a __noreturn annotation
> > 
> >    The call from foo() to bar() doesn't return, but bar() is missing the
> >    __noreturn annotation.  NOTE: In addition to annotating the function
> >    with __noreturn, please also add it to tools/objtool/noreturns.h.
> > 
> > Is it a behaviour that we still need to follow?
> 
> Yes. objtool has some heuristics to detect noreturn, but is is very
> difficult. Sadly noreturn is not something that is reflected in the ELF
> object file so we have to guess.
> 
> For now manually adding it to the objtool list is required, we're trying
> to get to the point where it is generated/validated by the compiler,
> perhaps with a plugin, but we're not there yet.

Thanks Peter!

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [tip:x86/tdx 8/12] vmlinux.o: warning: objtool: __tdx_hypercall+0x128: __tdx_hypercall_failed() is missing a __noreturn annotation
  2023-09-14  8:59         ` Peter Zijlstra
  2023-09-14  9:18           ` Huang, Kai
@ 2023-09-14 10:02           ` Huang, Kai
  2023-09-14 11:06             ` Peter Zijlstra
  1 sibling, 1 reply; 12+ messages in thread
From: Huang, Kai @ 2023-09-14 10:02 UTC (permalink / raw)
  To: peterz@infradead.org
  Cc: kirill.shutemov@linux.intel.com, oe-kbuild-all@lists.linux.dev,
	linux-kernel@vger.kernel.org, lkp, dave.hansen@linux.intel.com,
	x86@kernel.org

On Thu, 2023-09-14 at 10:59 +0200, Peter Zijlstra wrote:
> On Thu, Sep 14, 2023 at 07:54:10AM +0000, Huang, Kai wrote:
> 
> > > The point of noreturn is that the caller should know to stop generating
> > > code. For that the declaration needs the attribute, because call sites
> > > typically do not have access to the function definition in C.
> > 
> > Ah that makes perfect sense.  Thanks!
> > 
> > Then I assume we don't need to annotate __noreturn in the function body, but
> > only in the declaration?  Because the compiler must already have seen the
> > declaration when it generates the code for the function body.
> 
> I think so, I'm sure it'll tell you if it disagrees :-)
> 
> > Btw, I happened to notice that the objtool documentation suggests that we should
> > also add the the function to tools/objtool/noreturns.h:
> > 
> > 3. file.o: warning: objtool: foo+0x48c: bar() is missing a __noreturn annotation
> > 
> >    The call from foo() to bar() doesn't return, but bar() is missing the
> >    __noreturn annotation.  NOTE: In addition to annotating the function
> >    with __noreturn, please also add it to tools/objtool/noreturns.h.
> > 
> > Is it a behaviour that we still need to follow?
> 
> Yes. objtool has some heuristics to detect noreturn, but is is very
> difficult. Sadly noreturn is not something that is reflected in the ELF
> object file so we have to guess.
> 
> For now manually adding it to the objtool list is required, we're trying
> to get to the point where it is generated/validated by the compiler,
> perhaps with a plugin, but we're not there yet.

Sorry one more question, do you know what Kconfig option triggers this
__noreturn objtool check?  I couldn't reproduce it on my own machine but have to
depend on LKP for now.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [tip:x86/tdx 8/12] vmlinux.o: warning: objtool: __tdx_hypercall+0x128: __tdx_hypercall_failed() is missing a __noreturn annotation
  2023-09-14 10:02           ` Huang, Kai
@ 2023-09-14 11:06             ` Peter Zijlstra
  0 siblings, 0 replies; 12+ messages in thread
From: Peter Zijlstra @ 2023-09-14 11:06 UTC (permalink / raw)
  To: Huang, Kai
  Cc: kirill.shutemov@linux.intel.com, oe-kbuild-all@lists.linux.dev,
	linux-kernel@vger.kernel.org, lkp, dave.hansen@linux.intel.com,
	x86@kernel.org

On Thu, Sep 14, 2023 at 10:02:40AM +0000, Huang, Kai wrote:
> On Thu, 2023-09-14 at 10:59 +0200, Peter Zijlstra wrote:
> > On Thu, Sep 14, 2023 at 07:54:10AM +0000, Huang, Kai wrote:
> > 
> > > > The point of noreturn is that the caller should know to stop generating
> > > > code. For that the declaration needs the attribute, because call sites
> > > > typically do not have access to the function definition in C.
> > > 
> > > Ah that makes perfect sense.  Thanks!
> > > 
> > > Then I assume we don't need to annotate __noreturn in the function body, but
> > > only in the declaration?  Because the compiler must already have seen the
> > > declaration when it generates the code for the function body.
> > 
> > I think so, I'm sure it'll tell you if it disagrees :-)
> > 
> > > Btw, I happened to notice that the objtool documentation suggests that we should
> > > also add the the function to tools/objtool/noreturns.h:
> > > 
> > > 3. file.o: warning: objtool: foo+0x48c: bar() is missing a __noreturn annotation
> > > 
> > >    The call from foo() to bar() doesn't return, but bar() is missing the
> > >    __noreturn annotation.  NOTE: In addition to annotating the function
> > >    with __noreturn, please also add it to tools/objtool/noreturns.h.
> > > 
> > > Is it a behaviour that we still need to follow?
> > 
> > Yes. objtool has some heuristics to detect noreturn, but is is very
> > difficult. Sadly noreturn is not something that is reflected in the ELF
> > object file so we have to guess.
> > 
> > For now manually adding it to the objtool list is required, we're trying
> > to get to the point where it is generated/validated by the compiler,
> > perhaps with a plugin, but we're not there yet.
> 
> Sorry one more question, do you know what Kconfig option triggers this
> __noreturn objtool check?  I couldn't reproduce it on my own machine but have to
> depend on LKP for now.

Nope, sorry. Could be very GCC version specific through, this is all
about a specific code-gen pattern.

AFAICT we emit this warning when objtool finds this function is a
noreturn but code-gen continued.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [tip:x86/tdx 8/12] vmlinux.o: warning: objtool: __tdx_hypercall+0x128: __tdx_hypercall_failed() is missing a __noreturn annotation
  2023-09-14  7:29     ` Peter Zijlstra
  2023-09-14  7:54       ` Huang, Kai
@ 2023-09-14 14:16       ` Ingo Molnar
  2023-09-14 14:26         ` Peter Zijlstra
  1 sibling, 1 reply; 12+ messages in thread
From: Ingo Molnar @ 2023-09-14 14:16 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Huang, Kai, lkp, kirill.shutemov@linux.intel.com,
	linux-kernel@vger.kernel.org, oe-kbuild-all@lists.linux.dev,
	x86@kernel.org, dave.hansen@linux.intel.com


* Peter Zijlstra <peterz@infradead.org> wrote:

> > It appears the __noreturn must be annotated to the function declaration 
> > but not the function body.  I'll send out the fix as soon as I confirm 
> > the fix with LKP.
> 
> FWIW, the reason being that...
> 
> The point of noreturn is that the caller should know to stop generating 
> code. For that the declaration needs the attribute, because call sites 
> typically do not have access to the function definition in C.

BTW., arguably shouldn't the compiler generate a warning to begin with, 
when it encounters a noreturn function definition whose prototype doesn't 
have the attribute?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [tip:x86/tdx 8/12] vmlinux.o: warning: objtool: __tdx_hypercall+0x128: __tdx_hypercall_failed() is missing a __noreturn annotation
  2023-09-14 14:16       ` Ingo Molnar
@ 2023-09-14 14:26         ` Peter Zijlstra
  2023-09-14 14:52           ` Michael Matz
  0 siblings, 1 reply; 12+ messages in thread
From: Peter Zijlstra @ 2023-09-14 14:26 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Huang, Kai, lkp, kirill.shutemov@linux.intel.com,
	linux-kernel@vger.kernel.org, oe-kbuild-all@lists.linux.dev,
	x86@kernel.org, dave.hansen@linux.intel.com, matz

On Thu, Sep 14, 2023 at 04:16:47PM +0200, Ingo Molnar wrote:
> 
> * Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > > It appears the __noreturn must be annotated to the function declaration 
> > > but not the function body.  I'll send out the fix as soon as I confirm 
> > > the fix with LKP.
> > 
> > FWIW, the reason being that...
> > 
> > The point of noreturn is that the caller should know to stop generating 
> > code. For that the declaration needs the attribute, because call sites 
> > typically do not have access to the function definition in C.
> 
> BTW., arguably shouldn't the compiler generate a warning to begin with, 
> when it encounters a noreturn function definition whose prototype doesn't 
> have the attribute?

Yeah, I would agree with that, but I think the problem is that gnu
attributes are all considered 'optional' and do not factor into the
actual signature.

Added Michael to Cc so he may clarify if I'm talking nonsense.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [tip:x86/tdx 8/12] vmlinux.o: warning: objtool: __tdx_hypercall+0x128: __tdx_hypercall_failed() is missing a __noreturn annotation
  2023-09-14 14:26         ` Peter Zijlstra
@ 2023-09-14 14:52           ` Michael Matz
  0 siblings, 0 replies; 12+ messages in thread
From: Michael Matz @ 2023-09-14 14:52 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, Huang, Kai, lkp, kirill.shutemov@linux.intel.com,
	linux-kernel@vger.kernel.org, oe-kbuild-all@lists.linux.dev,
	x86@kernel.org, dave.hansen@linux.intel.com

Hey,

On Thu, 14 Sep 2023, Peter Zijlstra wrote:

> > > > It appears the __noreturn must be annotated to the function declaration 
> > > > but not the function body.  I'll send out the fix as soon as I confirm 
> > > > the fix with LKP.
> > > 
> > > FWIW, the reason being that...
> > > 
> > > The point of noreturn is that the caller should know to stop generating 
> > > code. For that the declaration needs the attribute, because call sites 
> > > typically do not have access to the function definition in C.
> > 
> > BTW., arguably shouldn't the compiler generate a warning to begin with, 
> > when it encounters a noreturn function definition whose prototype doesn't 
> > have the attribute?
> 
> Yeah, I would agree with that,

That makes sense, yeah.  We actually have a warning -Wmissing-attributes 
that would fit this usecase, but currently doesn't implement this case (it 
only applies to aliases, not to decl vs. def).

> but I think the problem is that gnu
> attributes are all considered 'optional' and do not factor into the
> actual signature.

That actually depends on the attribute :)  Most attributes are like that, 
true, but some aren't optional in that sense as they influence the 
callee-caller contract (e.g. those that change the ABI, like fastcall).  
Those must then be part of the functions type.

'noreturn' is optional in that sense.  But a warning might still be 
warranted.


Ciao,
Michael.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-09-14 14:52 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-14  1:05 [tip:x86/tdx 8/12] vmlinux.o: warning: objtool: __tdx_hypercall+0x128: __tdx_hypercall_failed() is missing a __noreturn annotation kernel test robot
2023-09-14  1:23 ` Huang, Kai
2023-09-14  3:21   ` Huang, Kai
2023-09-14  7:29     ` Peter Zijlstra
2023-09-14  7:54       ` Huang, Kai
2023-09-14  8:59         ` Peter Zijlstra
2023-09-14  9:18           ` Huang, Kai
2023-09-14 10:02           ` Huang, Kai
2023-09-14 11:06             ` Peter Zijlstra
2023-09-14 14:16       ` Ingo Molnar
2023-09-14 14:26         ` Peter Zijlstra
2023-09-14 14:52           ` Michael Matz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox