linux-kernel - Re: [PATCH next V2] KVM: VMX: use __always_inline for is_td_vcpu and is

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: <5a6187af0c4a73245ae527bc44135d4eb1a9b3c0.camel@intel.com>
Date: Tue, 27 May 2025 12:34:07 +0000
From: "Huang, Kai" <kai.huang@...el.com>
To: "peterz@...radead.org" <peterz@...radead.org>, "eadavis@...com"
	<eadavis@...com>
CC: "seanjc@...gle.com" <seanjc@...gle.com>, "bp@...en8.de" <bp@...en8.de>,
	"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
	"binbin.wu@...ux.intel.com" <binbin.wu@...ux.intel.com>, "hpa@...or.com"
	<hpa@...or.com>, "mingo@...hat.com" <mingo@...hat.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"tglx@...utronix.de" <tglx@...utronix.de>, "pbonzini@...hat.com"
	<pbonzini@...hat.com>, "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	"x86@...nel.org" <x86@...nel.org>
Subject: Re: [PATCH next V2] KVM: VMX: use __always_inline for is_td_vcpu and
 is_td

On Tue, 2025-05-27 at 13:07 +0200, Peter Zijlstra wrote:
> On Tue, May 27, 2025 at 04:44:37PM +0800, Edward Adam Davis wrote:
> > is_td() and is_td_vcpu() run in no instrumentation, so use __always_inline
> > to replace inline.
> > 
> > [1]
> > vmlinux.o: error: objtool: vmx_handle_nmi+0x47:
> >         call to is_td_vcpu.isra.0() leaves .noinstr.text section
> > 
> > Fixes: 7172c753c26a ("KVM: VMX: Move common fields of struct vcpu_{vmx,tdx} to a struct")
> > Signed-off-by: Edward Adam Davis <eadavis@...com>
> > ---
> > V1 -> V2: using __always_inline to replace noinstr
> > 
> >  arch/x86/kvm/vmx/common.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/vmx/common.h b/arch/x86/kvm/vmx/common.h
> > index 8f46a06e2c44..a0c5e8781c33 100644
> > --- a/arch/x86/kvm/vmx/common.h
> > +++ b/arch/x86/kvm/vmx/common.h
> > @@ -71,8 +71,8 @@ static __always_inline bool is_td_vcpu(struct kvm_vcpu *vcpu)
> >  
> >  #else
> >  
> > -static inline bool is_td(struct kvm *kvm) { return false; }
> > -static inline bool is_td_vcpu(struct kvm_vcpu *vcpu) { return false; }
> > +static __always_inline bool is_td(struct kvm *kvm) { return false; }
> > +static __always_inline bool is_td_vcpu(struct kvm_vcpu *vcpu) { return false; }
> >  
> >  #endif
> 
> Right; this is the 'right' fix. Although the better fix would be for the
> compiler to not be stupid :-)

Hi Peter,

Just out of curiosity, I have a related question.

I just learned there's a 'flatten' attribute ('__flatten' in linux kernel)
supported by both gcc and clang.  IIUC it forces all function calls inside one
function to be inlined if that function is annotated with this attribute.

However, it seems gcc and clang handles "recursive inlining" differently.  gcc
seems supports recursive inlining with flatten, but clang seems not.

This is the gcc doc [1] says, which explicitly tells recursive inlining is
supported IIUC:

  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.

And this is the clang doc [2] says, which doesn't say about recursive inlining:

  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.

Also, 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.

Just wondering whether you can happen to confirm this?

That also being said, if the __flatten could always be "recursive inlining", it
seems to me that __flatten would be a better annotation when we want some
function to be noinstr.  But if it's behaviour is compiler dependent, it seems
it's not a good idea to use it.

What's your opinion on this?

[1]: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html
[2]: https://clang.llvm.org/docs/AttributeReference.html