[RFC PATCH 05/20] x86/virt/tdx: Export tdx module attributes via sysfs

Chao Gao posted 20 patches 7 months ago
[RFC PATCH 05/20] x86/virt/tdx: Export tdx module attributes via sysfs
Posted by Chao Gao 7 months ago
TD-Preserving updates depend on a userspace tool to select the appropriate
module to load. To facilitate this decision-making process, expose the
necessary information to userspace.

Expose the current module versions so that userspace can verify
compatibility with new modules. version information is also valuable for
debugging, as knowing the exact module version can help reproduce
TDX-related issues.

Attach the TDX module attributes to the virtual TDX_TSM device, which
represents the TDX module and its features, such as TDX Connect.

Note changes to tdx_global_metadata.{hc} are auto-generated by following
the instructions detailed in [1], after modifying "version" to "versions"
in the TDX_STRUCT of tdx.py to accurately reflect that it is a collection
of versions.

Signed-off-by: Chao Gao <chao.gao@intel.com>
Tested-by: Farrah Chen <farrah.chen@intel.com>
Link: https://lore.kernel.org/kvm/20250226181453.2311849-12-pbonzini@redhat.com/ [1]
---
 Documentation/ABI/testing/sysfs-devices-tdx |  8 ++++++++
 MAINTAINERS                                 |  1 +
 arch/x86/include/asm/tdx_global_metadata.h  |  7 +++++++
 arch/x86/virt/vmx/tdx/tdx.c                 | 19 +++++++++++++++++++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 16 ++++++++++++++++
 5 files changed, 51 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-tdx

diff --git a/Documentation/ABI/testing/sysfs-devices-tdx b/Documentation/ABI/testing/sysfs-devices-tdx
new file mode 100644
index 000000000000..ccbe6431241e
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-devices-tdx
@@ -0,0 +1,8 @@
+What:		/sys/devices/virtual/tdx/tdx_tsm/version
+Date:		March 2025
+KernelVersion:	v6.15
+Contact:	linux-coco@lists.linux.dev
+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, TD-Preserving updates and etc.
diff --git a/MAINTAINERS b/MAINTAINERS
index c59316109e3f..0d58256c765b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26227,6 +26227,7 @@ L:	x86@kernel.org
 L:	linux-coco@lists.linux.dev
 S:	Supported
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/tdx
+F:	Documentation/ABI/testing/sysfs-devices-tdx
 F:	arch/x86/boot/compressed/tdx*
 F:	arch/x86/coco/tdx/
 F:	arch/x86/include/asm/shared/tdx.h
diff --git a/arch/x86/include/asm/tdx_global_metadata.h b/arch/x86/include/asm/tdx_global_metadata.h
index 060a2ad744bf..ce0370f4a5b9 100644
--- a/arch/x86/include/asm/tdx_global_metadata.h
+++ b/arch/x86/include/asm/tdx_global_metadata.h
@@ -5,6 +5,12 @@
 
 #include <linux/types.h>
 
+struct tdx_sys_info_versions {
+	u16 minor_version;
+	u16 major_version;
+	u16 update_version;
+};
+
 struct tdx_sys_info_features {
 	u64 tdx_features0;
 };
@@ -35,6 +41,7 @@ struct tdx_sys_info_td_conf {
 };
 
 struct tdx_sys_info {
+	struct tdx_sys_info_versions versions;
 	struct tdx_sys_info_features features;
 	struct tdx_sys_info_tdmr tdmr;
 	struct tdx_sys_info_td_ctrl td_ctrl;
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 9719df2f2634..5f1f463ddfe1 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1090,6 +1090,24 @@ struct tdx_tsm {
 	struct device dev;
 };
 
+static ssize_t version_show(struct device *dev, struct device_attribute *attr,
+			    char *buf)
+{
+	const struct tdx_sys_info_versions *v = &tdx_sysinfo.versions;
+
+	return sysfs_emit(buf, "%u.%u.%u\n", v->major_version,
+					     v->minor_version,
+					     v->update_version);
+}
+
+static DEVICE_ATTR_RO(version);
+
+static struct attribute *tdx_module_attrs[] = {
+	&dev_attr_version.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(tdx_module);
+
 static struct tdx_tsm *alloc_tdx_tsm(void)
 {
 	struct tdx_tsm *tsm = kzalloc(sizeof(*tsm), GFP_KERNEL);
@@ -1117,6 +1135,7 @@ static struct tdx_tsm *init_tdx_tsm(void)
 		return tsm;
 
 	dev = &tsm->dev;
+	dev->groups = tdx_module_groups;
 	ret = dev_set_name(dev, "tdx_tsm");
 	if (ret)
 		return ERR_PTR(ret);
diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
index 13ad2663488b..088e5bff4025 100644
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
@@ -7,6 +7,21 @@
  * Include this file to other C file instead.
  */
 
+static int get_tdx_sys_info_versions(struct tdx_sys_info_versions *sysinfo_versions)
+{
+	int ret = 0;
+	u64 val;
+
+	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000003, &val)))
+		sysinfo_versions->minor_version = val;
+	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000004, &val)))
+		sysinfo_versions->major_version = val;
+	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000005, &val)))
+		sysinfo_versions->update_version = val;
+
+	return ret;
+}
+
 static int get_tdx_sys_info_features(struct tdx_sys_info_features *sysinfo_features)
 {
 	int ret = 0;
@@ -89,6 +104,7 @@ static int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
 {
 	int ret = 0;
 
+	ret = ret ?: get_tdx_sys_info_versions(&sysinfo->versions);
 	ret = ret ?: get_tdx_sys_info_features(&sysinfo->features);
 	ret = ret ?: get_tdx_sys_info_tdmr(&sysinfo->tdmr);
 	ret = ret ?: get_tdx_sys_info_td_ctrl(&sysinfo->td_ctrl);
-- 
2.47.1
Re: [RFC PATCH 05/20] x86/virt/tdx: Export tdx module attributes via sysfs
Posted by Huang, Kai 6 months, 2 weeks ago
> 
> Note changes to tdx_global_metadata.{hc} are auto-generated by following
> the instructions detailed in [1], after modifying "version" to "versions"
> in the TDX_STRUCT of tdx.py to accurately reflect that it is a collection
> of versions.
> 

[...]

> +static ssize_t version_show(struct device *dev, struct device_attribute *attr,
> +			    char *buf)
> +{
> +	const struct tdx_sys_info_versions *v = &tdx_sysinfo.versions;
> +
> +	return sysfs_emit(buf, "%u.%u.%u\n", v->major_version,
> +					     v->minor_version,
> +					     v->update_version);
> +}
> +
> +static DEVICE_ATTR_RO(version);
> +

Then for this attribute, I think it is better to name it 'versions' as well?
Re: [RFC PATCH 05/20] x86/virt/tdx: Export tdx module attributes via sysfs
Posted by Chao Gao 6 months, 1 week ago
On Tue, Jun 03, 2025 at 07:49:17AM +0800, Huang, Kai wrote:
>
>> 
>> Note changes to tdx_global_metadata.{hc} are auto-generated by following
>> the instructions detailed in [1], after modifying "version" to "versions"
>> in the TDX_STRUCT of tdx.py to accurately reflect that it is a collection
>> of versions.
>> 
>
>[...]
>
>> +static ssize_t version_show(struct device *dev, struct device_attribute *attr,
>> +			    char *buf)
>> +{
>> +	const struct tdx_sys_info_versions *v = &tdx_sysinfo.versions;
>> +
>> +	return sysfs_emit(buf, "%u.%u.%u\n", v->major_version,
>> +					     v->minor_version,
>> +					     v->update_version);
>> +}
>> +
>> +static DEVICE_ATTR_RO(version);
>> +
>
>Then for this attribute, I think it is better to name it 'versions' as well?

Using 'versions' for sysfs might be confusing, as it could imply multiple TDX
modules. It makes more sense to me that each module has __a version__ in the
x.y.z format.

And the convention for sysfs file names is to use 'version'. E.g.,

# find . -type f -exec grep 'version_show' {} + |wc -l
185
# find . -type f -exec grep 'versions_show' {} + |wc -l
0

Concatenating major_version/minor_version is kinda common inside the kernel,
but 'versions' is not typically used as a sysfs name.
Re: [RFC PATCH 05/20] x86/virt/tdx: Export tdx module attributes via sysfs
Posted by Huang, Kai 6 months, 1 week ago
On Tue, 2025-06-10 at 09:37 +0800, Chao Gao wrote:
> On Tue, Jun 03, 2025 at 07:49:17AM +0800, Huang, Kai wrote:
> > 
> > > 
> > > Note changes to tdx_global_metadata.{hc} are auto-generated by following
> > > the instructions detailed in [1], after modifying "version" to "versions"
> > > in the TDX_STRUCT of tdx.py to accurately reflect that it is a collection
> > > of versions.
> > > 
> > 
> > [...]
> > 
> > > +static ssize_t version_show(struct device *dev, struct device_attribute *attr,
> > > +			    char *buf)
> > > +{
> > > +	const struct tdx_sys_info_versions *v = &tdx_sysinfo.versions;
> > > +
> > > +	return sysfs_emit(buf, "%u.%u.%u\n", v->major_version,
> > > +					     v->minor_version,
> > > +					     v->update_version);
> > > +}
> > > +
> > > +static DEVICE_ATTR_RO(version);
> > > +
> > 
> > Then for this attribute, I think it is better to name it 'versions' as well?
> 
> Using 'versions' for sysfs might be confusing, as it could imply multiple TDX
> modules. It makes more sense to me that each module has __a version__ in the
> x.y.z format.
> 
> And the convention for sysfs file names is to use 'version'. E.g.,
> 
> # find . -type f -exec grep 'version_show' {} + |wc -l
> 185
> # find . -type f -exec grep 'versions_show' {} + |wc -l
> 0
> 
> Concatenating major_version/minor_version is kinda common inside the kernel,
> but 'versions' is not typically used as a sysfs name.

Sure.

But then should we just use 'version' in the names of the structure and the
variable generated via the script?

It doesn't make a lot sense to me to have this inconsistency.
Re: [RFC PATCH 05/20] x86/virt/tdx: Export tdx module attributes via sysfs
Posted by Chao Gao 6 months, 1 week ago
On Wed, Jun 11, 2025 at 10:09:35AM +0800, Huang, Kai wrote:
>> Using 'versions' for sysfs might be confusing, as it could imply multiple TDX
>> modules. It makes more sense to me that each module has __a version__ in the
>> x.y.z format.
>> 
>> And the convention for sysfs file names is to use 'version'. E.g.,
>> 
>> # find . -type f -exec grep 'version_show' {} + |wc -l
>> 185
>> # find . -type f -exec grep 'versions_show' {} + |wc -l
>> 0
>> 
>> Concatenating major_version/minor_version is kinda common inside the kernel,
>> but 'versions' is not typically used as a sysfs name.
>
>Sure.
>
>But then should we just use 'version' in the names of the structure and the
>variable generated via the script?

Agreed. I will fix the struct name.