Re: [PATCH] x86/tdx: Make seamcall/tdcall CET-compliant

[Nikolay Borisov <nik.borisov@suse.com>]

On 10/22/25 13:30, Peter Zijlstra wrote:
> On Wed, Oct 22, 2025 at 01:21:25PM +0300, Nikolay Borisov wrote:
>>
>>
>> On 10/22/25 13:14, Huang, Kai wrote:
>>> On Wed, 2025-10-22 at 12:36 +0300, Nikolay Borisov wrote:
>>>> _seamcall/_ret/_saved_ret can be the target of indirect calls via
>>>> sc_retry_prerr/__seamcall_dirty_cache so on machines with CET enabled
>>>> such call chains result in a  splat and a BUG():
>>>>
>>>
>>> [...]
>>>
>>>>
>>>> Fix it by adding an ENBDR in TDX_MODULE_CALL macro to cover all
>>>> cases.
>>>>
>>>> Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
>>>> ---
>>>>
>>>> The kernel this was observed is a SLE, however it contains the current upstream
>>>> TDX patches. And looking at the usptream code the problem persists there as well.
>>>
>>> Does your kernel contain commit 0b3bc018e86af ("x86/virt/tdx: Avoid
>>> indirect calls to TDX assembly functions")?
>>>
>>> Some history about this commit:
>>>
>>> I firstly found __seamcall*() could be indirect calls in some randconfig
>>> when building the kernel, and tried to resolve it by (effectively) adding
>>> ENDBR:
>>>
>>> https://lore.kernel.org/lkml/20250604003848.13154-1-kai.huang@intel.com/
>>>
>>> Peter suggested that we could use __always_inline to keep compiler from
>>> generating indirect calls (which resulted in the above commit):
>>>
>>> https://lore.kernel.org/lkml/20250605145914.GW39944@noisy.programming.kicks-ass.net/
>>>
>>> I never met __tdcall*() could be indirect calls, though.
>>
>> Well, adding __always_inline to sc_retry means it will be inlined, but
>> inside the body of the function you do have:
>>
>> __seamcall_dirty_cache (which is also always inlined) but in it you have:
>> return func(fn, args);
>>
>> So you still have this indirect call, no ?
> 
> If you do always-inline, the function argument can be constant
> propagated, and thus func will be a known function and not result in an
> indirect call.
> 
> That is:
> 
> void foo(void);
> 
> __always_inline void bar(void (*func)(void))
> {
> 	func();
> }
> 
> void ponies(void)
> {
> 	bar(&foo);
> }
> 
> The compiler is clever enough to see that is a direct call of foo.

Thanks, I verified this, turns out we are using an earlier version of 
10df8607bf1a ("x86/virt/tdx: Mark memory cache state incoherent when 
making SEAMCALL")

Which contains do_seamcall instead of __seamcall_dirty_cache and that 
do_seamcall is missing always_inline so likely it's not being inlined.


Apologies for the noise in this case...


Though the fact that this __always_inline interacts with CET is somewhat 
subtle and not very evident from the changelogs.