Re: [PATCH v4 04/24] x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs

[Chao Gao <chao.gao@intel.com>]

On Fri, Feb 20, 2026 at 09:12:29AM +0800, Huang, Kai wrote:
>On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
>> 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 is actually another
>> component that runs in SEAM mode but it is separate from the TDX module:
>> the persistent SEAM loader or "P-SEAMLDR". Right now, the only component
>> that communicates with it 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 it.
>> 
>> P-SEAMLDR SEAMCALLs differ from TDX Module SEAMCALLs in areas such as
>> concurrency requirements. Add a P-SEAMLDR wrapper to handle these
>> differences and prepare for implementing concrete functions.
>> 
>> Note that unlike P-SEAMLDR, there is also a non-persistent SEAM loader
>> ("NP-SEAMLDR"). This is an authenticated code module (ACM) that is not
>> callable at runtime. Only BIOS launches it to load P-SEAMLDR at boot;
>
>[...]
>
>> the kernel does not interact with it.
>
>Nit:
>
>Again, to me this only describes what does the kernel do today.  It doesn't
>describe what the kernel needs to do for runtime updating.
>
>Maybe it can just be something like:
>
>  The kernel does not need to interact with it for runtime update.

I am fine with this. Will do.

>
>But I don't know why do you even need to talk about NP-SEAMLDR.

I included this because Dave had some confusion about NP-SEAMLDR [1], so I
wanted to clarify it.

[1]: https://lore.kernel.org/kvm/aXt0+lRvpvf5knKP@intel.com/

And, since NP-SEAMLDR and P-SEAMLDR have similar names, I thought it would be
helpful to clarify the difference. This follows Dave's earlier suggestion to
explain SEAM_INFO and SEAM_SEAMINFO SEAMCALLs for clarity [2].

[2]: https://lore.kernel.org/kvm/b2e2fd5e-8aff-4eda-a648-9ae9f8234d25@intel.com/

>
>> 
>> For details of P-SEAMLDR SEAMCALLs, see Intel® Trust Domain CPU
>> Architectural Extensions, Revision 343754-002, Chapter 2.3 "INSTRUCTION
>> SET REFERENCE".
>> 
>> Signed-off-by: Chao Gao <chao.gao@intel.com>
>> Tested-by: Farrah Chen <farrah.chen@intel.com>
>> Link: https://cdrdv2.intel.com/v1/dl/getContent/733582 # [1]
>> 
>
>[...]
>
>> + * Serialize P-SEAMLDR calls since the hardware only allows a single CPU to
>> + * interact with P-SEAMLDR simultaneously.
>> + */
>> +static DEFINE_RAW_SPINLOCK(seamldr_lock);
>> +
>> +static __maybe_unused int seamldr_call(u64 fn, struct tdx_module_args *args)
>> +{
>> +	/*
>> +	 * Serialize P-SEAMLDR calls and disable interrupts as the calls
>> +	 * can be made from IRQ context.
>> +	 */
>> +	guard(raw_spinlock_irqsave)(&seamldr_lock);
>
>Why do you need to disable IRQ?  A plain raw_spinlock should work with both
>cases where seamldr_call() is called from IRQ disabled context and normal
>task context? 

No, that's not safe. Without _irqsave, a deadlock can occur if an interrupt
fires while a task context already holds the lock, and the interrupt handler
also tries to acquire the same lock.

>
>> +	return seamcall_prerr(fn, args);
>> +}