arch/x86/kvm/vmx/common.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
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.0On 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
>
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
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
>
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.
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.0On 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>
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>
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>
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 :-)
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
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.
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.
© 2016 - 2025 Red Hat, Inc.