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

[Dave Hansen]

On 1/23/26 06:55, Chao Gao wrote:
...
> diff --git a/Documentation/ABI/testing/sysfs-devices-faux-tdx-host b/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
> index 901abbae2e61..a3f155977016 100644
> --- a/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
> +++ b/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
> @@ -4,3 +4,28 @@ Description:	(RO) Report the version of the loaded TDX Module. The TDX Module
>  		version is formatted as x.y.z, where "x" is the major version,
>  		"y" is the minor version and "z" is the update version. Versions
>  		are used for bug reporting, TDX Module updates and etc.
> +
> +What:		/sys/devices/faux/tdx_host/seamldr/version
> +Contact:	linux-coco@xxxxxxxxxxxxxxx
> +Description:	(RO) Report the version of the loaded SEAM loader. The SEAM
> +		loader version is formatted as x.y.z, where "x" is the major
> +		version, "y" is the minor version and "z" is the update version.
> +		Versions are used for bug reporting and compatibility check.

								checks ^

> +What:		/sys/devices/faux/tdx_host/seamldr/num_remaining_updates
> +Contact:	linux-coco@xxxxxxxxxxxxxxx
> +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?

> +		After each successful update, the number reduces by one. Once it
> +		reaches zero, further updates will fail until next reboot. The
> +		number is always zero if P-SEAMLDR doesn't support updates.
> +
> +		See Intel® Trust Domain Extensions - SEAM Loader (SEAMLDR)
> +		Interface Specification Chapter 3.3 "SEAMLDR_INFO" and Chapter
> +		4.2 "SEAMLDR.INSTALL" for more information. The documentation is
> +		available at:
> +		https://cdrdv2-public.intel.com/739045/intel-tdx-seamldr-interface-specification.pdf

Zap the URL. It's just going bit rot. Keep the document name. That's
googleable.

> diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
> index 0424933b2560..f4ce89522806 100644
> --- a/drivers/virt/coco/tdx-host/tdx-host.c
> +++ b/drivers/virt/coco/tdx-host/tdx-host.c
> @@ -11,6 +11,7 @@
>  #include <linux/sysfs.h>
>  
>  #include <asm/cpu_device_id.h>
> +#include <asm/seamldr.h>
>  #include <asm/tdx.h>
>  
>  static const struct x86_cpu_id tdx_host_ids[] = {
> @@ -40,7 +41,64 @@ static struct attribute *tdx_host_attrs[] = {
>  	&dev_attr_version.attr,
>  	NULL,
>  };
> -ATTRIBUTE_GROUPS(tdx_host);
> +
> +struct attribute_group tdx_host_group = {
> +	.attrs = tdx_host_attrs,
> +};
> +
> +#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?

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.