Re: [PATCH v7 06/22] coco/tdx-host: Expose P-SEAMLDR information via sysfs

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

On Tue, Mar 31, 2026 at 07:58:14AM -0700, Dave Hansen wrote:
>On 3/31/26 05:41, Chao Gao wrote:
>> +/*
>> + * Open-code DEVICE_ATTR_ADMIN_RO to specify a different 'show' function
>> + * for P-SEAMLDR version as version_show() is used for TDX module version.
>> + *
>> + * Admin-only readable as reading these attributes calls into P-SEAMLDR,
>> + * which may have potential performance and system impact.
>> + */
>> +static struct device_attribute dev_attr_seamldr_version =
>> +	__ATTR(version, 0400, seamldr_version_show, NULL);
>> +static DEVICE_ATTR_ADMIN_RO(num_remaining_updates);
>
>I don't like that these are really *exactly* the same, except for the
>name but are defined so differently. I see three alternatives that are
>better:
>
>1. Open code *both* so they look the same
>2. Define a macro that both can use that has a seamldr_## prefix
>3. Move the code to a new file to avoid symbol conflicts.
>
>I'd honestly be fine with any of those.
>
>The other option is to just put the module information at the top
>directory so the sysfs name differentiates things.
>
>For real, how many things are going to be in the seamldr directory? If
>it's just two, is it worth having a directory?

Good question. There are two for this series, and I don't expect there will be
more soon.

The only reason to have a separate directory is that P-SEAMLDR is a different
component from the TDX module. But I think that's a bit weak. By renaming the
attribute to seamldr_version instead of version, there's no symbol conflict.

I will apply this incremental change:

diff --git a/Documentation/ABI/testing/sysfs-devices-faux-tdx-host b/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
index f7221f2e5fec..1ace37b7f0e9 100644
--- a/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
+++ b/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
@@ -5,14 +5,14 @@ Description:	(RO) Report the version of the loaded TDX module. The TDX module
		"y" is the minor version and "z" is the update version. Versions
		are used for bug reporting, TDX module updates etc.
 
-What:		/sys/devices/faux/tdx_host/seamldr/version
+What:		/sys/devices/faux/tdx_host/seamldr_version
 Contact:	linux-coco@lists.linux.dev
 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 checks.
 
-What:		/sys/devices/faux/tdx_host/seamldr/num_remaining_updates
+What:		/sys/devices/faux/tdx_host/num_remaining_updates
 Contact:	linux-coco@lists.linux.dev
 Description:	(RO) Report the number of remaining updates. TDX maintains a
		log about each TDX module that has been loaded. This log has
diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
index 5a672126f372..cd84108094cf 100644
--- a/drivers/virt/coco/tdx-host/tdx-host.c
+++ b/drivers/virt/coco/tdx-host/tdx-host.c
@@ -75,15 +75,7 @@ static ssize_t num_remaining_updates_show(struct device *dev,
	return sysfs_emit(buf, "%u\n", info.num_remaining_updates);
 }
 
-/*
- * Open-code DEVICE_ATTR_ADMIN_RO to specify a different 'show' function
- * for P-SEAMLDR version as version_show() is used for TDX module version.
- *
- * Admin-only readable as reading these attributes calls into P-SEAMLDR,
- * which may have potential performance and system impact.
- */
-static struct device_attribute dev_attr_seamldr_version =
-	__ATTR(version, 0400, seamldr_version_show, NULL);
+static DEVICE_ATTR_ADMIN_RO(seamldr_version);
 static DEVICE_ATTR_ADMIN_RO(num_remaining_updates);
 
 static struct attribute *seamldr_attrs[] = {
@@ -105,7 +97,6 @@ static bool seamldr_group_visible(struct kobject *kobj)
 DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(seamldr);
 
 static const struct attribute_group seamldr_group = {
-	.name = "seamldr",
	.attrs = seamldr_attrs,
	.is_visible = SYSFS_GROUP_VISIBLE(seamldr),
 };