Re: [PATCH v3 09/26] coco/tdx-host: Expose P-SEAMLDR information via sysfs

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

>> +What:		/sys/devices/faux/tdx_host/seamldr/num_remaining_updates
>> +Contact:	linux-coco@lists.linux.dev
>> +Description:	(RO) Report the number of remaining updates that can be performed.
>> +		The CPU keeps track of TCB versions for each TDX Module that
>> +		has been loaded. Since this tracking database has finite
>> +		capacity, there's a maximum number of Module updates that can
>> +		be performed.
>
>Is it really the CPU? Or some SEAM software construct?

It is the CPU. The CPU provides the database and gives instructions to
P-SEAMLDR for adding records or cleaning up the entire database.

<snip>

>> +#ifdef CONFIG_INTEL_TDX_MODULE_UPDATE
>> +static ssize_t seamldr_version_show(struct device *dev, struct device_attribute *attr,
>> +				    char *buf)
>> +{
>> +	const struct seamldr_info *info = seamldr_get_info();
>
>Uhh... seamldr_get_info() calls down into the SEAMLDR. It happily zaps
>the VMCS and this is surely a slow thing. This also has 0444 permissions
>which means *ANYONE* can call this. Constantly. As fast as they can make
>a few syscalls.
>
>Right?

You are absolutely right. 

>
>Are there any concerns about making SEAMLDR calls? Are there any
>system-wide performance implications? How long of an interrupt-blocking
>blip is there for this?
>
>Also, what's the locking around seamldr_get_info()? It writes into a
>global, shared structure. I guess you disabled interrupts so it's
>preempt safe at least. <sigh>
>
>I guess it won't change *that* much. But, sheesh, it seems like an
>awfully bad idea to have lots of CPUs writing into a common data
>structure all at the same time.

/facepalm. Sorry for missing these important considerations.

I overlooked a critical constraint: only one CPU can call P-SEAMLDR at a time;
any second CPU gets VMFailInvalid. Patch 19 adds a lock for SEAMLDR.INSTALL
serialization, but we actually need to serialize all P-SEAMLDR calls or handle
VMFailInvalid with retries.

I will make the following changes to see how they look:

1. Move the lock from patch 19 to seamldr_call() to serialize all P-SEAMLDR calls
2. Cache seamldr_info and only update it after successful updates
3. Make seamldr_get_info() return cached data instead of calling P-SEAMLDR every time