linux-kernel - Re: [PATCH] x86/tdx: mark tdh_vp_enter() as _

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <dba9b129a3d6407948f165b885f9c72975e3231b.camel@intel.com>
Date: Mon, 26 May 2025 23:10:37 +0000
From: "Huang, Kai" <kai.huang@...el.com>
To: "kvm@...r.kernel.org" <kvm@...r.kernel.org>, "pbonzini@...hat.com"
	<pbonzini@...hat.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
CC: "Li, Xiaoyao" <xiaoyao.li@...el.com>, lkp <lkp@...el.com>
Subject: Re: [PATCH] x86/tdx: mark tdh_vp_enter() as __flatten

On Mon, 2025-05-26 at 16:45 -0400, Paolo Bonzini wrote:
> In some cases tdx_tdvpr_pa() is not fully inlined into tdh_vp_enter(), which
> causes the following warning:
> 
>   vmlinux.o: warning: objtool: tdh_vp_enter+0x8: call to tdx_tdvpr_pa() leaves .noinstr.text section
> 
> This happens if the compiler considers tdx_tdvpr_pa() to be "large", for example
> because CONFIG_SPARSEMEM adds two function calls to page_to_section() and
> __section_mem_map_addr():
> 
> ({      const struct page *__pg = (pg);                         \
>         int __sec = page_to_section(__pg);                      \
>         (unsigned long)(__pg - __section_mem_map_addr(__nr_to_section(__sec)));
> \
> })

Just FYI the above warning can also be triggered when CONFIG_SPARSEMEM_VMEMMAP=y
(and CONFIG_SPARSEMEM=y as well), in which case the __page_to_pfn() is simply:

  #define __page_to_pfn(page)     (unsigned long)((page) - vmemmap)

The function call to page_to_section() and __section_mem_map_addr() only happens
when CONFIG_SPARSEMEM_VMEMMAP=n while CONFIG_SPARSEMEM=y.

> 
> Because exiting the noinstr section is a no-no, just mark tdh_vp_enter() for
> full inlining.
> 
> Reported-by: kernel test robot <lkp@...el.com>
> Analyzed-by: Xiaoyao Li <xiaoyao.li@...el.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202505240530.5KktQ5mX-lkp@intel.com/
> Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>
> ---
>  arch/x86/virt/vmx/tdx/tdx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index f5e2a937c1e7..2457d13c3f9e 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -1517,7 +1517,7 @@ static void tdx_clflush_page(struct page *page)
>  	clflush_cache_range(page_to_virt(page), PAGE_SIZE);
>  }
>  
> -noinstr u64 tdh_vp_enter(struct tdx_vp *td, struct tdx_module_args *args)
> +noinstr __flatten u64 tdh_vp_enter(struct tdx_vp *td, struct tdx_module_args *args)
>  {
>  	args->rcx = tdx_tdvpr_pa(td);
>  

I didn't know this __flatten attribute.  Thanks.

I did some test and can confirm this patch can silence the warning mentioned in
the changelog.

However, with this patch applied, I also tested the case that
CONFIG_SPARSEMEM_VMEMMAP=n and CONFIG_SPARSEMEM=y [1], but I still got:

vmlinux.o: warning: objtool: tdh_vp_enter+0x10: call to page_to_section() leaves
.noinstr.text section

Not sure why, but it seems __flatten failed to work as expected, as least
recursively.

Then I did some dig.  The gcc doc [2] explicitly says it will do it recursively:

  flatten

  Generally, inlining into a function is limited. For a function marked with 
  this attribute, every call inside this function is inlined including the calls
  such inlining introduces to the function (but not recursive calls to the 
  function itself), if possible. 

But the clang doc [3] doesn't explicitly say it:

  flatten

  The flatten attribute causes calls within the attributed function to be 
  inlined unless it is impossible to do so, for example if the body of the 
  callee is unavailable or if the callee has the noinline attribute.

One "AI Overview" provided by google also says below:

  Compiler Behavior:
  While GCC supports recursive inlining with flatten, other compilers like  
  Clang might only perform a single level of inlining.

So it seems __flatten may not work as expected in clang.  But I guess we can
blame clang on this, so not sure whether it matters.

[1]: after fixing the build error I mentioned to you in the last reply.
[2]: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html
[3]: https://clang.llvm.org/docs/AttributeReference.html