Re: [tip:x86/tdx 8/12] vmlinux.o: warning: objtool: __tdx_hypercall+0x128: __tdx_hypercall_failed() is missing a __noreturn annotation

[Huang, Kai]

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@xxxxxxxxx/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@xxxxxxxxx/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@xxxxxxxxx>
> > > > > Closes: https://lore.kernel.org/oe-kbuild-all/202309140828.9RdmlH2Z-lkp@xxxxxxxxx/
> > > > 
> > > > 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.