P-SEAMLDR is another component alongside the TDX module within the
protected SEAM range. P-SEAMLDR can update the TDX module at runtime.
Software can talk with P-SEAMLDR via SEAMCALLs with the bit 63 of RAX
(leaf number) set to 1 (a.k.a P-SEAMLDR SEAMCALLs).

P-SEAMLDR SEAMCALLs differ from SEAMCALLs of the TDX module in terms of
error codes and the handling of the current VMCS.

In preparation for adding support for P-SEAMLDR SEAMCALLs, do the two
following changes to SEAMCALL low-level helpers:

1) Tweak sc_retry() to retry on "lack of entropy" errors reported by
   P-SEAMLDR because it uses a different error code.

2) Add seamldr_err() to log error messages on P-SEAMLDR SEAMCALL failures.

Signed-off-by: Chao Gao <chao.gao@intel.com>
Tested-by: Farrah Chen <farrah.chen@intel.com>
---
Add seamldr_prerr() as a macro to be consistent with existing code. If
maintainers would like to switch these to static inline functions then I
would be happy to add a new patch to convert existing macros to static
inline functions and build on that.

v3:
 - print P-SEAMLDR leaf numbers in hex
 - use %# to print error code [Binbin]
 - mark the is_seamldr_call() call as unlikely [Binbin]
 - remove the function to get the error code for retry from leaf numbers
   [Yilun]
v2:
 - use a macro rather than an inline function for seamldr_err() for
   consistency.
---
 arch/x86/virt/vmx/tdx/seamcall.h | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/arch/x86/virt/vmx/tdx/seamcall.h b/arch/x86/virt/vmx/tdx/seamcall.h
index 0912e03fabfe..256f71d6ca70 100644
--- a/arch/x86/virt/vmx/tdx/seamcall.h
+++ b/arch/x86/virt/vmx/tdx/seamcall.h
@@ -34,15 +34,28 @@ static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
 	return func(fn, args);
 }
 
+#define SEAMLDR_RND_NO_ENTROPY	0x8000000000030001ULL
+
+#define SEAMLDR_SEAMCALL_MASK	_BITUL(63)
+
+static inline bool is_seamldr_call(u64 fn)
+{
+	return fn & SEAMLDR_SEAMCALL_MASK;
+}
+
 static __always_inline u64 sc_retry(sc_func_t func, u64 fn,
 			   struct tdx_module_args *args)
 {
+	u64 retry_code = TDX_RND_NO_ENTROPY;
 	int retry = RDRAND_RETRY_LOOPS;
 	u64 ret;
 
+	if (unlikely(is_seamldr_call(fn)))
+		retry_code = SEAMLDR_RND_NO_ENTROPY;
+
 	do {
 		ret = func(fn, args);
-	} while (ret == TDX_RND_NO_ENTROPY && --retry);
+	} while (ret == retry_code && --retry);
 
 	return ret;
 }
@@ -68,6 +81,16 @@ static inline void seamcall_err_ret(u64 fn, u64 err,
 			args->r9, args->r10, args->r11);
 }
 
+static inline void seamldr_err(u64 fn, u64 err, struct tdx_module_args *args)
+{
+	/*
+	 * Note: P-SEAMLDR leaf numbers are printed in hex as they have
+	 * bit 63 set, making them hard to read and understand if printed
+	 * in decimal
+	 */
+	pr_err("P-SEAMLDR (%llx) failed: %#016llx\n", fn, err);
+}
+
 static __always_inline int sc_retry_prerr(sc_func_t func,
 					  sc_err_func_t err_func,
 					  u64 fn, struct tdx_module_args *args)
@@ -96,4 +119,7 @@ static __always_inline int sc_retry_prerr(sc_func_t func,
 #define seamcall_prerr_ret(__fn, __args)					\
 	sc_retry_prerr(__seamcall_ret, seamcall_err_ret, (__fn), (__args))
 
+#define seamldr_prerr(__fn, __args)						\
+	sc_retry_prerr(__seamcall, seamldr_err, (__fn), (__args))
+
 #endif
-- 
2.47.3

On 1/23/26 06:55, Chao Gao wrote:
> P-SEAMLDR is another component alongside the TDX module within the
> protected SEAM range. P-SEAMLDR can update the TDX module at runtime.
> Software can talk with P-SEAMLDR via SEAMCALLs with the bit 63 of RAX
> (leaf number) set to 1 (a.k.a P-SEAMLDR SEAMCALLs).

This text kinda bugs me. It's OK, but needs improvement.

First, don't explain the ABI in the changelog. Nobody cares that it's
bit 63.


Background:

	The TDX architecture uses the "SEAMCALL" instruction to
	communicate with SEAM mode software. Right now, the only SEAM
	mode software that the kernel communicates with is the TDX
	module. But, there are actually some components that run in SEAM
	mode but that are separate from the TDX module: that SEAM
	loaders. Right now, the only component that communicates with
	them is the BIOS which loads the TDX module itself at boot. But,
	to support updating the TDX module, the kernel now needs to be
	able to talk to one of the the SEAM loaders: the Persistent
	loader or "P-SEAMLDR".

Then do this part:

> P-SEAMLDR SEAMCALLs differ from SEAMCALLs of the TDX module in terms of
> error codes and the handling of the current VMCS.
Except I don't even know how the TDX module handles the current VMCS.
That probably needs to be in there. Or, it should be brought up in the
patch itself that implements this. Or, uplifted to the cover letter.

> In preparation for adding support for P-SEAMLDR SEAMCALLs, do the two
> following changes to SEAMCALL low-level helpers:
> 
> 1) Tweak sc_retry() to retry on "lack of entropy" errors reported by
>    P-SEAMLDR because it uses a different error code.
> 
> 2) Add seamldr_err() to log error messages on P-SEAMLDR SEAMCALL failures.



> diff --git a/arch/x86/virt/vmx/tdx/seamcall.h b/arch/x86/virt/vmx/tdx/seamcall.h
> index 0912e03fabfe..256f71d6ca70 100644
> --- a/arch/x86/virt/vmx/tdx/seamcall.h
> +++ b/arch/x86/virt/vmx/tdx/seamcall.h
> @@ -34,15 +34,28 @@ static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
>  	return func(fn, args);
>  }
>  
> +#define SEAMLDR_RND_NO_ENTROPY	0x8000000000030001ULL

<sigh>

#define TDX_RND_NO_ENTROPY      0x8000020300000000ULL

So they're not even close values. They're not consistent or even a bit
off or anything.

Honestly, this needs a justification for why this was done this way. Why
can't "SEAM mode" be a monolithic thing from the kernel's perspective?

> +#define SEAMLDR_SEAMCALL_MASK	_BITUL(63)
> +
> +static inline bool is_seamldr_call(u64 fn)
> +{
> +	return fn & SEAMLDR_SEAMCALL_MASK;
> +}
> +
>  static __always_inline u64 sc_retry(sc_func_t func, u64 fn,
>  			   struct tdx_module_args *args)
>  {
> +	u64 retry_code = TDX_RND_NO_ENTROPY;
>  	int retry = RDRAND_RETRY_LOOPS;
>  	u64 ret;
>  
> +	if (unlikely(is_seamldr_call(fn)))
> +		retry_code = SEAMLDR_RND_NO_ENTROPY;

(un)likey() has two uses:

1. It's in performance critical code and compilers have been
   demonstrated to be generating bad code.
2. It's in code where it's not obvious what the fast path is
   and (un)likey() makes the code more readable.

Which one is this?

Second, this is nitpicky, but I'd rather this be:

	if (is_seamldr_call(fn))
		retry_code = SEAMLDR_RND_NO_ENTROPY;
	else
		retry_code = TDX_RND_NO_ENTROPY;

or even:

	retry_code = TDX_RND_NO_ENTROPY;
	if (is_seamldr_call(fn))
		retry_code = SEAMLDR_RND_NO_ENTROPY;

That makes it trivial that 'retry_code' can only have two values. It's
nitpicky because the original initialization is so close.

>  	do {
>  		ret = func(fn, args);
> -	} while (ret == TDX_RND_NO_ENTROPY && --retry);
> +	} while (ret == retry_code && --retry);
>  
>  	return ret;
>  }
> @@ -68,6 +81,16 @@ static inline void seamcall_err_ret(u64 fn, u64 err,
>  			args->r9, args->r10, args->r11);
>  }
>  
> +static inline void seamldr_err(u64 fn, u64 err, struct tdx_module_args *args)
> +{
> +	/*
> +	 * Note: P-SEAMLDR leaf numbers are printed in hex as they have
> +	 * bit 63 set, making them hard to read and understand if printed
> +	 * in decimal
> +	 */
> +	pr_err("P-SEAMLDR (%llx) failed: %#016llx\n", fn, err);
> +}

Oh, lovely.

Didn't you just propose changing the module SEAMCALL leaf numbers in
decimal? Isn't it a little crazy to do one in decimal and the other in hex?

I'd really rather just see the TDX documentation changed.

But, honestly, I'd probably just leave the thing in hex, drop this hunk,
and go thwack someone that writes TDX module documentation instead.

>  static __always_inline int sc_retry_prerr(sc_func_t func,
>  					  sc_err_func_t err_func,
>  					  u64 fn, struct tdx_module_args *args)
> @@ -96,4 +119,7 @@ static __always_inline int sc_retry_prerr(sc_func_t func,
>  #define seamcall_prerr_ret(__fn, __args)					\
>  	sc_retry_prerr(__seamcall_ret, seamcall_err_ret, (__fn), (__args))
>  
> +#define seamldr_prerr(__fn, __args)						\
> +	sc_retry_prerr(__seamcall, seamldr_err, (__fn), (__args))
> +
>  #endif

So, honestly, for me, it's a NAK for this whole patch.

Go change the P-SEAMLDR to use the same error code as the TDX module,
and fix the documentation. No kernel changes, please.

On Wed, Jan 28, 2026 at 03:03:14PM -0800, Dave Hansen wrote:
>On 1/23/26 06:55, Chao Gao wrote:
>> P-SEAMLDR is another component alongside the TDX module within the
>> protected SEAM range. P-SEAMLDR can update the TDX module at runtime.
>> Software can talk with P-SEAMLDR via SEAMCALLs with the bit 63 of RAX
>> (leaf number) set to 1 (a.k.a P-SEAMLDR SEAMCALLs).
>
>This text kinda bugs me. It's OK, but needs improvement.
>
>First, don't explain the ABI in the changelog. Nobody cares that it's
>bit 63.
>
>
>Background:
>
>	The TDX architecture uses the "SEAMCALL" instruction to
>	communicate with SEAM mode software. Right now, the only SEAM
>	mode software that the kernel communicates with is the TDX
>	module. But, there are actually some components that run in SEAM
>	mode but that are separate from the TDX module: that SEAM
>	loaders. Right now, the only component that communicates with
>	them is the BIOS which loads the TDX module itself at boot. But,
>	to support updating the TDX module, the kernel now needs to be
>	able to talk to one of the the SEAM loaders: the Persistent
>	loader or "P-SEAMLDR".

Thanks. This is much clearer than my version.

One tiny nit: NP-SEAMLDR isn't SEAM mode software. It is an authenticated code
module (ACM).

>
>Then do this part:
>
>> P-SEAMLDR SEAMCALLs differ from SEAMCALLs of the TDX module in terms of
>> error codes and the handling of the current VMCS.
>Except I don't even know how the TDX module handles the current VMCS.
>That probably needs to be in there. Or, it should be brought up in the
>patch itself that implements this. Or, uplifted to the cover letter.

My logic was:

1. The kernel communicates with P-SEAMLDR via SEAMCALL, just like with the TDX
   Module.
2. But P-SEAMLDR SEAMCALLs and TDX Module SEAMCALLs are slightly different.

So we need some tweaks to the low-level helpers to add separate wrappers for
P-SEAMLDR SEAMCALLs.

To me, without mentioning #2, these tweaks in this patch (for separate wrappers
in the next patch) aren't justified.

<snip>

>>  static __always_inline u64 sc_retry(sc_func_t func, u64 fn,
>>  			   struct tdx_module_args *args)
>>  {
>> +	u64 retry_code = TDX_RND_NO_ENTROPY;
>>  	int retry = RDRAND_RETRY_LOOPS;
>>  	u64 ret;
>>  
>> +	if (unlikely(is_seamldr_call(fn)))
>> +		retry_code = SEAMLDR_RND_NO_ENTROPY;
>
>(un)likey() has two uses:
>
>1. It's in performance critical code and compilers have been
>   demonstrated to be generating bad code.
>2. It's in code where it's not obvious what the fast path is
>   and (un)likey() makes the code more readable.
>
>Which one is this?

I think #2 although I am happy to drop "unlikely".

>
>Second, this is nitpicky, but I'd rather this be:
>
>	if (is_seamldr_call(fn))
>		retry_code = SEAMLDR_RND_NO_ENTROPY;
>	else
>		retry_code = TDX_RND_NO_ENTROPY;

Will do.

<snip>

>> +static inline void seamldr_err(u64 fn, u64 err, struct tdx_module_args *args)
>> +{
>> +	/*
>> +	 * Note: P-SEAMLDR leaf numbers are printed in hex as they have
>> +	 * bit 63 set, making them hard to read and understand if printed
>> +	 * in decimal
>> +	 */
>> +	pr_err("P-SEAMLDR (%llx) failed: %#016llx\n", fn, err);
>> +}
>
>Oh, lovely.
>
>Didn't you just propose changing the module SEAMCALL leaf numbers in
>decimal? Isn't it a little crazy to do one in decimal and the other in hex?

Yes, that's crazy. I'll just reuse seamcall_err(), so leaf numbers will be
printed in hex for both the TDX Module and P-SEAMLDR

>
>I'd really rather just see the TDX documentation changed.

I'll submit a request for TDX documentation to display leaf numbers in both hex
and decimal.

>
>But, honestly, I'd probably just leave the thing in hex, drop this hunk,
>and go thwack someone that writes TDX module documentation instead.
>
>>  static __always_inline int sc_retry_prerr(sc_func_t func,
>>  					  sc_err_func_t err_func,
>>  					  u64 fn, struct tdx_module_args *args)
>> @@ -96,4 +119,7 @@ static __always_inline int sc_retry_prerr(sc_func_t func,
>>  #define seamcall_prerr_ret(__fn, __args)					\
>>  	sc_retry_prerr(__seamcall_ret, seamcall_err_ret, (__fn), (__args))
>>  
>> +#define seamldr_prerr(__fn, __args)						\
>> +	sc_retry_prerr(__seamcall, seamldr_err, (__fn), (__args))

This can be dropped if we don't need to add seamldr_err().

On 1/29/26 06:55, Chao Gao wrote:
> On Wed, Jan 28, 2026 at 03:03:14PM -0800, Dave Hansen wrote:
...
> Thanks. This is much clearer than my version.
> 
> One tiny nit: NP-SEAMLDR isn't SEAM mode software. It is an authenticated code
> module (ACM).

Ahhh, thanks for the correction!

>> Then do this part:
>>
>>> P-SEAMLDR SEAMCALLs differ from SEAMCALLs of the TDX module in terms of
>>> error codes and the handling of the current VMCS.
>> Except I don't even know how the TDX module handles the current VMCS.
>> That probably needs to be in there. Or, it should be brought up in the
>> patch itself that implements this. Or, uplifted to the cover letter.
> 
> My logic was:
> 
> 1. The kernel communicates with P-SEAMLDR via SEAMCALL, just like with the TDX
>    Module.
> 2. But P-SEAMLDR SEAMCALLs and TDX Module SEAMCALLs are slightly different.
> 
> So we need some tweaks to the low-level helpers to add separate wrappers for
> P-SEAMLDR SEAMCALLs.
> 
> To me, without mentioning #2, these tweaks in this patch (for separate wrappers
> in the next patch) aren't justified.

My objection is that you talk about the VMCS handling in here but
there's no actual VMCS handling. This is the changelog for patch 06/26,
not 07/26.

Don't talk about the VMCS handling here. When you do, make sure you give
enough background.

>>>  static __always_inline u64 sc_retry(sc_func_t func, u64 fn,
>>>  			   struct tdx_module_args *args)
>>>  {
>>> +	u64 retry_code = TDX_RND_NO_ENTROPY;
>>>  	int retry = RDRAND_RETRY_LOOPS;
>>>  	u64 ret;
>>>  
>>> +	if (unlikely(is_seamldr_call(fn)))
>>> +		retry_code = SEAMLDR_RND_NO_ENTROPY;
>>
>> (un)likey() has two uses:
>>
>> 1. It's in performance critical code and compilers have been
>>   demonstrated to be generating bad code.
>> 2. It's in code where it's not obvious what the fast path is
>>   and (un)likey() makes the code more readable.
>>
>> Which one is this?
> 
> I think #2 although I am happy to drop "unlikely".

But why does it *MATTER*? Is it important to understand that SEAMLDR
calls are rarer than TDX module calls?

> >  static __always_inline int sc_retry_prerr(sc_func_t func,
> >  					  sc_err_func_t err_func,
> >  					  u64 fn, struct tdx_module_args *args)
> > @@ -96,4 +119,7 @@ static __always_inline int sc_retry_prerr(sc_func_t func,
> >  #define seamcall_prerr_ret(__fn, __args)					\
> >  	sc_retry_prerr(__seamcall_ret, seamcall_err_ret, (__fn), (__args))
> >  
> > +#define seamldr_prerr(__fn, __args)						\
> > +	sc_retry_prerr(__seamcall, seamldr_err, (__fn), (__args))
> > +
> >  #endif
> 
> So, honestly, for me, it's a NAK for this whole patch.
> 
> Go change the P-SEAMLDR to use the same error code as the TDX module,
> and fix the documentation. No kernel changes, please.

I'm thinking of ways to avoid a new pseamldr version.

Could we just ask for a unified error code space for both SEAMCALL &
SEAMLDR CALL, eliminating overlaps. There is no overlap now, so this is
just another documentation fix.

Then with all the doc fixes, we only need minor code change:


@@ -127,7 +127,8 @@ static __always_inline u64 sc_retry(sc_func_t func, u64 fn,
                preempt_disable();
                ret = __seamcall_dirty_cache(func, fn, args);
                preempt_enable();
-       } while (ret == TDX_RND_NO_ENTROPY && --retry);
+       } while ((ret == TDX_RND_NO_ENTROPY ||
+                 ret == SEAMLDR_RND_NO_ENTROPY) && --retry);


I think this is a balance. The existing error code philosophy for SEAM
is as informative as possible, e.g. all kinds of xxx_INVALID,
SEAMLDR_RND_NO_ENTROPY is not that evil among 200+ other error codes.

On 1/29/26 01:46, Xu Yilun wrote:
...
> Then with all the doc fixes, we only need minor code change:
> 
> @@ -127,7 +127,8 @@ static __always_inline u64 sc_retry(sc_func_t func, u64 fn,
>                 preempt_disable();
>                 ret = __seamcall_dirty_cache(func, fn, args);
>                 preempt_enable();
> -       } while (ret == TDX_RND_NO_ENTROPY && --retry);
> +       } while ((ret == TDX_RND_NO_ENTROPY ||
> +                 ret == SEAMLDR_RND_NO_ENTROPY) && --retry);

That's better than what was there in the past, but I do think even this
is pretty silly.

I mean, we (Intel) control all the components. These errors are for the
same dang thing. The people who wrote both components probably sit next
to each other. :)

I think I'd be a bit less grumpy if there was _anything_ else that
demanded a retry. So, let's try to extract the guarantee that the error
spaces are at least unified, in that a TDX module will never return
SEAMLDR_RND_NO_ENTROPY to mean something else and a SEAMLDR will never
return TDX_RND_NO_ENTROPY. Then, maybe talk them into doing a unified
thing from here on out.

But, for now, drop this patch. We'll just assume the P-SEAMLDR doesn't
have "no entropy" errors until this is sorted.

On 1/23/2026 10:55 PM, Chao Gao wrote:
> P-SEAMLDR is another component alongside the TDX module within the
> protected SEAM range. P-SEAMLDR can update the TDX module at runtime.
> Software can talk with P-SEAMLDR via SEAMCALLs with the bit 63 of RAX
> (leaf number) set to 1 (a.k.a P-SEAMLDR SEAMCALLs).
> 
> P-SEAMLDR SEAMCALLs differ from SEAMCALLs of the TDX module in terms of
> error codes and the handling of the current VMCS.
> 
> In preparation for adding support for P-SEAMLDR SEAMCALLs, do the two
> following changes to SEAMCALL low-level helpers:
> 
> 1) Tweak sc_retry() to retry on "lack of entropy" errors reported by
>    P-SEAMLDR because it uses a different error code.
> 
> 2) Add seamldr_err() to log error messages on P-SEAMLDR SEAMCALL failures.
> 
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Tested-by: Farrah Chen <farrah.chen@intel.com>

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

On Fri, Jan 23, 2026 at 06:55:14AM -0800, Chao Gao wrote:
> P-SEAMLDR is another component alongside the TDX module within the
> protected SEAM range. P-SEAMLDR can update the TDX module at runtime.
> Software can talk with P-SEAMLDR via SEAMCALLs with the bit 63 of RAX
> (leaf number) set to 1 (a.k.a P-SEAMLDR SEAMCALLs).
> 
> P-SEAMLDR SEAMCALLs differ from SEAMCALLs of the TDX module in terms of
> error codes and the handling of the current VMCS.
> 
> In preparation for adding support for P-SEAMLDR SEAMCALLs, do the two
> following changes to SEAMCALL low-level helpers:
> 
> 1) Tweak sc_retry() to retry on "lack of entropy" errors reported by
>    P-SEAMLDR because it uses a different error code.
> 
> 2) Add seamldr_err() to log error messages on P-SEAMLDR SEAMCALL failures.

Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>