[PATCH next] KVM: VMX: add noinstr for is_td_vcpu and is_td

[PATCH next] KVM: VMX: add noinstr for is_td_vcpu and is_td

[Edward Adam Davis]

is_td() and is_td_vcpu() run in no instrumentation, so they are need
noinstr.

[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@qq.com>
---
 arch/x86/kvm/vmx/common.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/vmx/common.h b/arch/x86/kvm/vmx/common.h
index 8f46a06e2c44..70e0879c58f6 100644
--- a/arch/x86/kvm/vmx/common.h
+++ b/arch/x86/kvm/vmx/common.h
@@ -59,20 +59,20 @@ struct vcpu_vt {
 
 #ifdef CONFIG_KVM_INTEL_TDX
 
-static __always_inline bool is_td(struct kvm *kvm)
+static noinstr __always_inline bool is_td(struct kvm *kvm)
 {
 	return kvm->arch.vm_type == KVM_X86_TDX_VM;
 }
 
-static __always_inline bool is_td_vcpu(struct kvm_vcpu *vcpu)
+static noinstr __always_inline bool is_td_vcpu(struct kvm_vcpu *vcpu)
 {
 	return is_td(vcpu->kvm);
 }
 
 #else
 
-static inline bool is_td(struct kvm *kvm) { return false; }
-static inline bool is_td_vcpu(struct kvm_vcpu *vcpu) { return false; }
+static noinstr bool is_td(struct kvm *kvm) { return false; }
+static noinstr bool is_td_vcpu(struct kvm_vcpu *vcpu) { return false; }
 
 #endif
 
-- 
2.43.0

Re: [PATCH next] KVM: VMX: add noinstr for is_td_vcpu and is_td

[Binbin Wu]

On 5/27/2025 11:45 AM, Edward Adam Davis wrote:
> is_td() and is_td_vcpu() run in no instrumentation, so they are need
> noinstr.
>
> [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@qq.com>
> ---
>   arch/x86/kvm/vmx/common.h | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/common.h b/arch/x86/kvm/vmx/common.h
> index 8f46a06e2c44..70e0879c58f6 100644
> --- a/arch/x86/kvm/vmx/common.h
> +++ b/arch/x86/kvm/vmx/common.h
> @@ -59,20 +59,20 @@ struct vcpu_vt {
>   
>   #ifdef CONFIG_KVM_INTEL_TDX
>   
> -static __always_inline bool is_td(struct kvm *kvm)
> +static noinstr __always_inline bool is_td(struct kvm *kvm)
>   {
>   	return kvm->arch.vm_type == KVM_X86_TDX_VM;
>   }
>   
> -static __always_inline bool is_td_vcpu(struct kvm_vcpu *vcpu)
> +static noinstr __always_inline bool is_td_vcpu(struct kvm_vcpu *vcpu)
>   {
>   	return is_td(vcpu->kvm);
>   }

noinstr is not needed when the functions are __always_inline.

>   
>   #else
>   
> -static inline bool is_td(struct kvm *kvm) { return false; }
> -static inline bool is_td_vcpu(struct kvm_vcpu *vcpu) { return false; }
> +static noinstr bool is_td(struct kvm *kvm) { return false; }
> +static noinstr bool is_td_vcpu(struct kvm_vcpu *vcpu) { return false; }

Oops, overlooked the !CONFIG_KVM_INTEL_TDX case.

How about:

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
>   

Re: [PATCH next] KVM: VMX: add noinstr for is_td_vcpu and is_td

[Edward Adam Davis]

On Tue, 27 May 2025 13:31:12 +0800, Binbin Wu wrote:
> noinstr is not needed when the functions are __always_inline.
Right.
> 
> >   
> >   #else
> >   
> > -static inline bool is_td(struct kvm *kvm) { return false; }
> > -static inline bool is_td_vcpu(struct kvm_vcpu *vcpu) { return false; }
> > +static noinstr bool is_td(struct kvm *kvm) { return false; }
> > +static noinstr bool is_td_vcpu(struct kvm_vcpu *vcpu) { return false; }
> 
> Oops, overlooked the !CONFIG_KVM_INTEL_TDX case.
> 
> How about:
> 
> 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; }
Looks good.

[PATCH next V2] KVM: VMX: use __always_inline for is_td_vcpu and is_td

[Edward Adam Davis]

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@qq.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
 
-- 
2.43.0

Re: [PATCH next V2] KVM: VMX: use __always_inline for is_td_vcpu and is_td

[Peter Zijlstra]

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@qq.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 :-)

Re: [PATCH next V2] KVM: VMX: use __always_inline for is_td_vcpu and is_td

[Huang, Kai]

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@qq.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

Re: [PATCH next V2] KVM: VMX: use __always_inline for is_td_vcpu and is_td

[Peter Zijlstra]

On Tue, May 27, 2025 at 12:34:07PM +0000, Huang, Kai wrote:
> 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@qq.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 :-)

FWIW, the thing that typically happens is that the compiler first
inserts instrumentation (think *SAN) into the trivial stub function and
then figures its too big to inline.

> 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?

I am somewhat conflicted on this; using __flatten, while convenient,
would take away the immediate insight into what gets pulled in. Having
to explicitly mark functions with __always_inline is somewhat
inconvenient, but at least you don't pull in stuff by accident.

Re: [PATCH next V2] KVM: VMX: use __always_inline for is_td_vcpu and is_td

[Huang, Kai]

On Tue, 2025-05-27 at 15:47 +0200, Peter Zijlstra wrote:
> On Tue, May 27, 2025 at 12:34:07PM +0000, Huang, Kai wrote:
> > 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@qq.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 :-)
> 
> FWIW, the thing that typically happens is that the compiler first
> inserts instrumentation (think *SAN) into the trivial stub function and
> then figures its too big to inline.

This is helpful.  Thanks!

> 
> > 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?
> 
> I am somewhat conflicted on this; using __flatten, while convenient,
> would take away the immediate insight into what gets pulled in. Having
> to explicitly mark functions with __always_inline is somewhat
> inconvenient, but at least you don't pull in stuff by accident.

Yeah, thanks anyway for the insight.

Re: [PATCH next V2] KVM: VMX: use __always_inline for is_td_vcpu and is_td

[Edgecombe, Rick P]

On Tue, 2025-05-27 at 16:44 +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@qq.com>
> ---
> V1 -> V2: using __always_inline to replace noinstr

Argh, for some reason the original report was sent just to Paolo and so I didn't
see this until now:
https://lore.kernel.org/oe-kbuild-all/202505071640.fUgzT6SF-lkp@intel.com/

You (or Paolo) might want to add that link for [1]. Fix looks good.

Tested-by: Rick Edgecombe <rick.p.edgecombe@intel.com>

Re: [PATCH next V2] KVM: VMX: use __always_inline for is_td_vcpu and is_td

[Huang, Kai]

On Tue, 2025-05-27 at 17:53 +0000, Edgecombe, Rick P wrote:
> On Tue, 2025-05-27 at 16:44 +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@qq.com>
> > ---
> > V1 -> V2: using __always_inline to replace noinstr
> 
> Argh, for some reason the original report was sent just to Paolo and so I didn't
> see this until now:
> https://lore.kernel.org/oe-kbuild-all/202505071640.fUgzT6SF-lkp@intel.com/
> 
> You (or Paolo) might want to add that link for [1]. Fix looks good.
> 
> Tested-by: Rick Edgecombe <rick.p.edgecombe@intel.com>

Also,

Reviewed-by: Kai Huang <kai.huang@intel.com>

Re: [PATCH next V2] KVM: VMX: use __always_inline for is_td_vcpu and is_td

[Binbin Wu]

On 5/28/2025 5:48 AM, Huang, Kai wrote:
> On Tue, 2025-05-27 at 17:53 +0000, Edgecombe, Rick P wrote:
>> On Tue, 2025-05-27 at 16:44 +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@qq.com>
>>> ---
>>> V1 -> V2: using __always_inline to replace noinstr
>> Argh, for some reason the original report was sent just to Paolo and so I didn't
>> see this until now:
>> https://lore.kernel.org/oe-kbuild-all/202505071640.fUgzT6SF-lkp@intel.com/
>>
>> You (or Paolo) might want to add that link for [1]. Fix looks good.
>>
>> Tested-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Also,
>
> Reviewed-by: Kai Huang <kai.huang@intel.com>
Also,

Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>

Re: [PATCH next] KVM: VMX: add noinstr for is_td_vcpu and is_td

[kernel test robot]

Hi Edward,

kernel test robot noticed the following build warnings:

[auto build test WARNING on next-20250526]

url:    https://github.com/intel-lab-lkp/linux/commits/Edward-Adam-Davis/KVM-VMX-add-noinstr-for-is_td_vcpu-and-is_td/20250527-114712
base:   next-20250526
patch link:    https://lore.kernel.org/r/tencent_27A451976AF76E66DF1379C3604976A3A505%40qq.com
patch subject: [PATCH next] KVM: VMX: add noinstr for is_td_vcpu and is_td
config: x86_64-buildonly-randconfig-001-20250527 (https://download.01.org/0day-ci/archive/20250527/202505271443.0QM8TjLH-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/20250527/202505271443.0QM8TjLH-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/202505271443.0QM8TjLH-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from arch/x86/kvm/vmx/vmx.h:20,
                    from arch/x86/kvm/vmx/hyperv.h:7,
                    from arch/x86/kvm/vmx/nested.c:13:
>> arch/x86/kvm/vmx/common.h:74:21: warning: 'is_td' defined but not used [-Wunused-function]
      74 | static noinstr bool is_td(struct kvm *kvm) { return false; }
         |                     ^~~~~


vim +/is_td +74 arch/x86/kvm/vmx/common.h

    73	
  > 74	static noinstr bool is_td(struct kvm *kvm) { return false; }
    75	static noinstr bool is_td_vcpu(struct kvm_vcpu *vcpu) { return false; }
    76	

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

Re: [PATCH next] KVM: VMX: add noinstr for is_td_vcpu and is_td

[Peter Zijlstra]

On Tue, May 27, 2025 at 11:45:16AM +0800, Edward Adam Davis wrote:
> is_td() and is_td_vcpu() run in no instrumentation, so they are need
> noinstr.
> 
> [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@qq.com>
> ---
>  arch/x86/kvm/vmx/common.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/common.h b/arch/x86/kvm/vmx/common.h
> index 8f46a06e2c44..70e0879c58f6 100644
> --- a/arch/x86/kvm/vmx/common.h
> +++ b/arch/x86/kvm/vmx/common.h
> @@ -59,20 +59,20 @@ struct vcpu_vt {
>  
>  #ifdef CONFIG_KVM_INTEL_TDX
>  
> -static __always_inline bool is_td(struct kvm *kvm)
> +static noinstr __always_inline bool is_td(struct kvm *kvm)

noinstr and __always_inline are not compatible. Specifically noinstr
implies noinline.

>  {
>  	return kvm->arch.vm_type == KVM_X86_TDX_VM;
>  }
>  
> -static __always_inline bool is_td_vcpu(struct kvm_vcpu *vcpu)
> +static noinstr __always_inline bool is_td_vcpu(struct kvm_vcpu *vcpu)
>  {
>  	return is_td(vcpu->kvm);
>  }
>  
>  #else
>  
> -static inline bool is_td(struct kvm *kvm) { return false; }
> -static inline bool is_td_vcpu(struct kvm_vcpu *vcpu) { return false; }
> +static noinstr bool is_td(struct kvm *kvm) { return false; }
> +static noinstr bool is_td_vcpu(struct kvm_vcpu *vcpu) { return false; }

The problem is likely your compiler is silly and managed to out-of-line
these; make these __always_inline and try again.

>  
>  #endif
>  
> -- 
> 2.43.0
>