[PATCH 0/2] Expose TDX Module version

[PATCH 0/2] Expose TDX Module version

[Chao Gao]

Currently, there is no user interface to get the TDX Module version.
However, in bug reporting or analysis scenarios, the first question
normally asked is which TDX Module version is on your system, to determine
if this is a known issue or a new regression.

To address this issue, this series exposes the TDX Module version as
sysfs attributes of the tdx_host device [*].

This series is built on the VMX always-on series, which enables VMXON
during CPU bringup, and the "tdx_host" device, which creates a virtual
device to serve as the user interface for all TDX host features, e.g.,
TDX Connect.

The base-commit is:
https://git.kernel.org/pub/scm/linux/kernel/git/devsec/tsm.git/commit/?h=tdx&id=9332e088937f

[*]: https://lore.kernel.org/linux-coco/20250919142237.418648-2-dan.j.williams@intel.com/

Chao Gao (2):
  x86/virt/tdx: Retrieve TDX module version
  coco/tdx-host: Expose TDX module version

 .../ABI/testing/sysfs-devices-faux-tdx-host   |  6 +++++
 MAINTAINERS                                   |  1 +
 arch/x86/include/asm/tdx_global_metadata.h    |  7 ++++++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c   | 16 ++++++++++++
 drivers/virt/coco/tdx-host/tdx-host.c         | 25 ++++++++++++++++++-
 5 files changed, 54 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-faux-tdx-host

-- 
2.47.3

[PATCH 1/2] x86/virt/tdx: Retrieve TDX module version

[Chao Gao]

Each TDX module is associated with a version in the x.y.z format, where x
represents the major version, y the minor version, and z the update
version. Knowing the running TDX module version is valuable for bug
reporting and debugging.

Retrieve the TDX module version using the existing metadata reading
interface, in preparation for exposing it to userspace via sysfs.

Note changes to tdx_global_metadata.{hc} are auto-generated by the
following command.

 $ python tdx.py global_metadata.json tdx_global_metadata.h \
        tdx_global_metadata.c

The 'tdx.py' can be fetched from [1]. The 'global_metadata.json' can be
fetched from [2]. And 'tdx.py' has "BUILD_DATE", "BUILD_NUM" and
"INTERNAL_VERSION" in TDX_STRUCTS; they are removed before running
the above command as they are not needed for now.

Signed-off-by: Chao Gao <chao.gao@intel.com>
Link: https://lore.kernel.org/kvm/0853b155ec9aac09c594caa60914ed6ea4dc0a71.camel@intel.com/ # [1]
Link: https://cdrdv2.intel.com/v1/dl/getContent/795381 # [2]
---
 arch/x86/include/asm/tdx_global_metadata.h  |  7 +++++++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 16 ++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/arch/x86/include/asm/tdx_global_metadata.h b/arch/x86/include/asm/tdx_global_metadata.h
index 060a2ad744bf..40689c8dc67e 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_version {
+	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_version version;
 	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_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
index 13ad2663488b..0454124803f3 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_version(struct tdx_sys_info_version *sysinfo_version)
+{
+	int ret = 0;
+	u64 val;
+
+	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000003, &val)))
+		sysinfo_version->minor_version = val;
+	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000004, &val)))
+		sysinfo_version->major_version = val;
+	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000005, &val)))
+		sysinfo_version->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_version(&sysinfo->version);
 	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.3

[PATCH 2/2] coco/tdx-host: Expose TDX module version

[Chao Gao]

Currently these is no way to know the TDX module version from the
userspace. such information is helpful for bug reporting or
debugging.

With the tdx-host device in place, expose the TDX module version as
a device attribute via sysfs.

Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 .../ABI/testing/sysfs-devices-faux-tdx-host   |  6 +++++
 MAINTAINERS                                   |  1 +
 drivers/virt/coco/tdx-host/tdx-host.c         | 25 ++++++++++++++++++-
 3 files changed, 31 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-faux-tdx-host

diff --git a/Documentation/ABI/testing/sysfs-devices-faux-tdx-host b/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
new file mode 100644
index 000000000000..18d4a4a71b80
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
@@ -0,0 +1,6 @@
+What:		/sys/devices/faux/tdx_host/version
+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 c1ad1294560c..7560dcf8a53d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27333,6 +27333,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-faux-tdx-host
 F:	Documentation/ABI/testing/sysfs-devices-virtual-misc-tdx_guest
 F:	arch/x86/boot/compressed/tdx*
 F:	arch/x86/coco/tdx/
diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
index 49c205913ef6..968a19f4e01a 100644
--- a/drivers/virt/coco/tdx-host/tdx-host.c
+++ b/drivers/virt/coco/tdx-host/tdx-host.c
@@ -22,6 +22,29 @@ MODULE_DEVICE_TABLE(x86cpu, tdx_host_ids);
 
 static struct faux_device *fdev;
 
+static ssize_t version_show(struct device *dev, struct device_attribute *attr,
+			    char *buf)
+{
+	const struct tdx_sys_info *tdx_sysinfo = tdx_get_sysinfo();
+	const struct tdx_sys_info_version *ver;
+
+	if (!tdx_sysinfo)
+		return -ENXIO;
+
+	ver = &tdx_sysinfo->version;
+
+	return sysfs_emit(buf, "%u.%u.%02u\n", ver->major_version,
+					       ver->minor_version,
+					       ver->update_version);
+}
+static DEVICE_ATTR_RO(version);
+
+static struct attribute *tdx_host_attrs[] = {
+	&dev_attr_version.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(tdx_host);
+
 static int __init tdx_host_init(void)
 {
 	int r;
@@ -34,7 +57,7 @@ static int __init tdx_host_init(void)
 	if (r)
 		return r;
 
-	fdev = faux_device_create(KBUILD_MODNAME, NULL, NULL);
+	fdev = faux_device_create_with_groups(KBUILD_MODNAME, NULL, NULL, tdx_host_groups);
 	if (!fdev)
 		return -ENODEV;
 
-- 
2.47.3

Re: [PATCH 2/2] coco/tdx-host: Expose TDX module version

[Huang, Kai]

On Tue, 2025-09-30 at 19:22 -0700, Chao Gao wrote:
> Currently these is no way to know the TDX module version from the
	    ^
	    there is

> userspace. such information is helpful for bug reporting or
	     ^
	     Such

> debugging.
> 
> 

[...]

> +static ssize_t version_show(struct device *dev, struct device_attribute *attr,
> +			    char *buf)
> +{
> +	const struct tdx_sys_info *tdx_sysinfo = tdx_get_sysinfo();
> +	const struct tdx_sys_info_version *ver;
> +
> +	if (!tdx_sysinfo)
> +		return -ENXIO;
> +
> +	ver = &tdx_sysinfo->version;
> +
> +	return sysfs_emit(buf, "%u.%u.%02u\n", ver->major_version,
> +					       ver->minor_version,
> +					       ver->update_version);

Nit: not sure whether the "%u.%u.%02u" needs a comment, e.g., why the %02u
is used for the update_version?

Re: [PATCH 2/2] coco/tdx-host: Expose TDX module version

[Kiryl Shutsemau]

On Wed, Oct 01, 2025 at 04:12:10AM +0000, Huang, Kai wrote:
> On Tue, 2025-09-30 at 19:22 -0700, Chao Gao wrote:
> > Currently these is no way to know the TDX module version from the
> 	    ^
> 	    there is
> 
> > userspace. such information is helpful for bug reporting or
> 	     ^
> 	     Such
> 
> > debugging.
> > 
> > 
> 
> [...]
> 
> > +static ssize_t version_show(struct device *dev, struct device_attribute *attr,
> > +			    char *buf)
> > +{
> > +	const struct tdx_sys_info *tdx_sysinfo = tdx_get_sysinfo();
> > +	const struct tdx_sys_info_version *ver;
> > +
> > +	if (!tdx_sysinfo)
> > +		return -ENXIO;
> > +
> > +	ver = &tdx_sysinfo->version;
> > +
> > +	return sysfs_emit(buf, "%u.%u.%02u\n", ver->major_version,
> > +					       ver->minor_version,
> > +					       ver->update_version);
> 
> Nit: not sure whether the "%u.%u.%02u" needs a comment, e.g., why the %02u
> is used for the update_version?

That's how TDX module version formatted:

https://github.com/intel/tdx-module/tags

I think it is good idea to match it.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCH 2/2] coco/tdx-host: Expose TDX module version

[Kiryl Shutsemau]

On Tue, Sep 30, 2025 at 07:22:45PM -0700, Chao Gao wrote:
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c1ad1294560c..7560dcf8a53d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -27333,6 +27333,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-faux-tdx-host
>  F:	Documentation/ABI/testing/sysfs-devices-virtual-misc-tdx_guest
>  F:	arch/x86/boot/compressed/tdx*
>  F:	arch/x86/coco/tdx/

The entry was updated recently:

https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/tree/MAINTAINERS?h=x86/tdx#n27325

I think you need to rebase onto current tip tree.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCH 1/2] x86/virt/tdx: Retrieve TDX module version

[Dave Hansen]

On 9/30/25 19:22, Chao Gao wrote:
> +	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000003, &val)))
> +		sysinfo_version->minor_version = val;
> +	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000004, &val)))
> +		sysinfo_version->major_version = val;
> +	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000005, &val)))
> +		sysinfo_version->update_version = val;

Heh, how long does this take in practice to get 6 bytes of data out of
the module? When is the point that we move  over to TDH.SYS.RDALL?

Re: [PATCH 2/2] coco/tdx-host: Expose TDX module version

[Huang, Kai]

On Wed, 2025-10-01 at 12:27 +0100, Kiryl Shutsemau wrote:
> On Wed, Oct 01, 2025 at 04:12:10AM +0000, Huang, Kai wrote:
> > On Tue, 2025-09-30 at 19:22 -0700, Chao Gao wrote:
> > > Currently these is no way to know the TDX module version from the
> > 	    ^
> > 	    there is
> > 
> > > userspace. such information is helpful for bug reporting or
> > 	     ^
> > 	     Such
> > 
> > > debugging.
> > > 
> > > 
> > 
> > [...]
> > 
> > > +static ssize_t version_show(struct device *dev, struct device_attribute *attr,
> > > +			    char *buf)
> > > +{
> > > +	const struct tdx_sys_info *tdx_sysinfo = tdx_get_sysinfo();
> > > +	const struct tdx_sys_info_version *ver;
> > > +
> > > +	if (!tdx_sysinfo)
> > > +		return -ENXIO;
> > > +
> > > +	ver = &tdx_sysinfo->version;
> > > +
> > > +	return sysfs_emit(buf, "%u.%u.%02u\n", ver->major_version,
> > > +					       ver->minor_version,
> > > +					       ver->update_version);
> > 
> > Nit: not sure whether the "%u.%u.%02u" needs a comment, e.g., why the %02u
> > is used for the update_version?
> 
> That's how TDX module version formatted:
> 
> https://github.com/intel/tdx-module/tags
> 
> I think it is good idea to match it.

Yeah agreed!

Re: [PATCH 1/2] x86/virt/tdx: Retrieve TDX module version

[Chao Gao]

On Wed, Oct 01, 2025 at 08:15:46AM -0700, Dave Hansen wrote:
>On 9/30/25 19:22, Chao Gao wrote:
>> +	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000003, &val)))
>> +		sysinfo_version->minor_version = val;
>> +	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000004, &val)))
>> +		sysinfo_version->major_version = val;
>> +	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000005, &val)))
>> +		sysinfo_version->update_version = val;
>
>Heh, how long does this take in practice to get 6 bytes of data out of
>the module?

~8us. And the whole metadata reading process (i.e., get_tdx_sys_info()) takes
~113us.

>When is the point that we move  over to TDH.SYS.RDALL?

TDH.SYS.RDALL takes ~16us.

I'm uncertain whether the saved CPU time of ~100us justifies implementing
TDH.SYS.RDALL.

TDH.SYS.RDALL returns all metadata as a list of arrays, requiring the kernel to
parse this structure and iterate through all fields.

One advantage of TDH.SYS.RDALL is that it eliminates the need to check field
existence before reading, since it simply dumps all available fields rather
than targeting specific ones. For example, TDH.SYS.RDALL removes the need for
the tdx_supports_ABC() check:

	in the DPAMT series:

	+	if (!ret && tdx_supports_dynamic_pamt(&tdx_sysinfo) &&
	+	    !(ret = read_sys_metadata_field(0x9100000100000013, &val)))
	+		sysinfo_tdmr->pamt_page_bitmap_entry_bits = val;

	in the TDX Module update series:

	+	if (tdx_supports_runtime_update(&tdx_sysinfo) &&
	+	    !(ret = read_sys_metadata_field(0x8900000100000000, &val)))
	+		sysinfo_handoff->module_hv = val;

While iterating through the array list adds some complexity, this is a one-time
cost. Once the loop structure is in place, adding new fields only requires
inserting a "switch-case" clause within the loop.

Please see the draft code below. If TDH.SYS.RDALL is the right direction, I can
refine the code and submit a formal patch series.

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 128e6ffba736..fa9bb6d47a87 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -226,22 +226,23 @@ static int build_tdx_memlist(struct list_head *tmb_list)
	return ret;
 }
 
-static int read_sys_metadata_field(u64 field_id, u64 *data)
+static int read_sys_metadata_field(u64 *field_id, void *ptr)
 {
	struct tdx_module_args args = {};
	int ret;
 
	/*
-	 * TDH.SYS.RD -- reads one global metadata field
-	 *  - RDX (in): the field to read
-	 *  - R8 (out): the field data
+	 * TDH.SYS.RDALL -- reads all global metadata fields
+	 *  - RDX (in): the physical address of the buffer to store
+	 *  - R8 (in/out): the initial field ID to read (in) and
+	 *		   the next field ID to read (out).
	 */
-	args.rdx = field_id;
-	ret = seamcall_prerr_ret(TDH_SYS_RD, &args);
+	args.rdx = __pa(ptr);
+	args.r8  = *field_id;
+	ret = seamcall_prerr_ret(TDH_SYS_RDALL, &args);
	if (ret)
		return ret;
-
-	*data = args.r8;
+	*field_id = args.r8;
 
	return 0;
 }
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index 1965adb63f1f..44d92047073e 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -42,6 +42,7 @@
 #define TDH_SYS_RD			34
 #define TDH_SYS_LP_INIT			35
 #define TDH_SYS_TDMR_INIT		36
+#define TDH_SYS_RDALL			37
 #define TDH_MEM_TRACK			38
 #define TDH_PHYMEM_CACHE_WB		40
 #define TDH_PHYMEM_PAGE_WBINVD		41
diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
index 3fdd5cbc21d8..f4b16367ef2f 100644
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
@@ -7,121 +7,177 @@
  * Include this file to other C file instead.
  */
 
-static int get_tdx_sys_info_version(struct tdx_sys_info_version *sysinfo_version)
+struct md_field_id
 {
-	int ret = 0;
-	u64 val;
-
-	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000003, &val)))
-		sysinfo_version->minor_version = val;
-	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000004, &val)))
-		sysinfo_version->major_version = val;
-	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000005, &val)))
-		sysinfo_version->update_version = val;
-
-	return ret;
-}
-
-static int get_tdx_sys_info_features(struct tdx_sys_info_features *sysinfo_features)
-{
-	int ret = 0;
-	u64 val;
-
-	if (!ret && !(ret = read_sys_metadata_field(0x0A00000300000008, &val)))
-		sysinfo_features->tdx_features0 = val;
-
-	return ret;
-}
-
-static int get_tdx_sys_info_tdmr(struct tdx_sys_info_tdmr *sysinfo_tdmr)
+	union {
+		struct {
+			u32 field_code;				// Bits 31:0
+			u32 element_size_code		: 2;	// Bits 33:32
+			u32 last_element_in_field	: 4;	// Bits 37:34
+			u32 last_field_in_sequence	: 9;	// Bits 46:38
+			u32 reserved_1			: 3;	// Bits 49:47
+			u32 inc_size			: 1;	// Bit 50
+			u32 write_mask_valid		: 1;	// Bit 51
+			u32 context_code		: 3;	// Bits 54:52
+			u32 reserved_2			: 1;	// Bit 55
+			u32 class_code			: 6;	// Bits 61:56
+			u32 reserved_3			: 1;	// Bit 62
+			u32 non_arch			: 1;	// Bit 63
+		};
+		u64 raw;
+	};
+};
+struct md_list_header
 {
-	int ret = 0;
-	u64 val;
-
-	if (!ret && !(ret = read_sys_metadata_field(0x9100000100000008, &val)))
-		sysinfo_tdmr->max_tdmrs = val;
-	if (!ret && !(ret = read_sys_metadata_field(0x9100000100000009, &val)))
-		sysinfo_tdmr->max_reserved_per_tdmr = val;
-	if (!ret && !(ret = read_sys_metadata_field(0x9100000100000010, &val)))
-		sysinfo_tdmr->pamt_4k_entry_size = val;
-	if (!ret && !(ret = read_sys_metadata_field(0x9100000100000011, &val)))
-		sysinfo_tdmr->pamt_2m_entry_size = val;
-	if (!ret && !(ret = read_sys_metadata_field(0x9100000100000012, &val)))
-		sysinfo_tdmr->pamt_1g_entry_size = val;
-
-	return ret;
-}
+	u16 list_buff_size;
+	u16 num_sequences;
+	u32 reserved;
+};
 
-static int get_tdx_sys_info_td_ctrl(struct tdx_sys_info_td_ctrl *sysinfo_td_ctrl)
+struct md_sequence
 {
-	int ret = 0;
-	u64 val;
+	struct md_field_id sequence_header;
+	u64 element[0];
+};
+
+#define TDX_MD_ID_MINOR_VERSION			0x0800000100000003
+#define TDX_MD_ID_MAJOR_VERSION			0x0800000100000004
+#define TDX_MD_ID_UPDATE_VERSION		0x0800000100000005
+#define TDX_MD_ID_MODULE_HV			0x8900000100000000
+#define TDX_MD_ID_TDX_FEATURES0			0x0A00000300000008
+#define TDX_MD_ID_MAX_TDMRS			0x9100000100000008
+#define TDX_MD_ID_MAX_RESERVED_PER_TDMR		0x9100000100000009
+#define TDX_MD_ID_PAMT_4K_ENTRY_SIZE		0x9100000100000010
+#define TDX_MD_ID_PAMT_2M_ENTRY_SIZE		0x9100000100000011
+#define TDX_MD_ID_PAMT_1G_ENTRY_SIZE		0x9100000100000012
+#define TDX_MD_ID_TDR_BASE_SIZE			0x9800000100000000
+#define TDX_MD_ID_TDCS_BASE_SIZE		0x9800000100000100
+#define TDX_MD_ID_TDVPS_BASE_SIZE		0x9800000100000200
+#define TDX_MD_ID_ATTRIBUTES_FIXED0		0x1900000300000000
+#define TDX_MD_ID_ATTRIBUTES_FIXED1		0x1900000300000001
+#define TDX_MD_ID_XFAM_FIXED0			0x1900000300000002
+#define TDX_MD_ID_XFAM_FIXED1			0x1900000300000003
+#define TDX_MD_ID_NUM_CPUID_CONFIG		0x9900000100000004
+#define TDX_MD_ID_MAX_VCPUS_PER_TD		0x9900000100000008
+#define TDX_MD_ID_CPUID_CONFIG_LEAVES		0x9900000300000400
+#define TDX_MD_ID_CPUID_CONFIG_VALUES		0x9900000300000500
 
-	if (!ret && !(ret = read_sys_metadata_field(0x9800000100000000, &val)))
-		sysinfo_td_ctrl->tdr_base_size = val;
-	if (!ret && !(ret = read_sys_metadata_field(0x9800000100000100, &val)))
-		sysinfo_td_ctrl->tdcs_base_size = val;
-	if (!ret && !(ret = read_sys_metadata_field(0x9800000100000200, &val)))
-		sysinfo_td_ctrl->tdvps_base_size = val;
-
-	return ret;
-}
+/*
+ * Extract CLASS_CODE (bits 61:56), CONTEXT_CODE(bits 54:52),
+ * FIELD_CODE(bits 23:0) from metadata IDs. Other bits in metadata IDs
+ * cannot be used for comparison.
+ */
+#define TDX_MD_ID_MASK				0x3f70000000ffffff
 
-static int get_tdx_sys_info_td_conf(struct tdx_sys_info_td_conf *sysinfo_td_conf)
+static void parse_md_sequence(struct md_sequence *sequence, struct tdx_sys_info *sysinfo)
 {
-	int ret = 0;
-	u64 val;
+	u32 num_elements = (sequence->sequence_header.last_element_in_field + 1) *
+			   (sequence->sequence_header.last_field_in_sequence + 1);
+	u64 base_id = sequence->sequence_header.raw & TDX_MD_ID_MASK;
+	u64 field_id;
+	u64 *data;
	int i, j;
 
-	if (!ret && !(ret = read_sys_metadata_field(0x1900000300000000, &val)))
-		sysinfo_td_conf->attributes_fixed0 = val;
-	if (!ret && !(ret = read_sys_metadata_field(0x1900000300000001, &val)))
-		sysinfo_td_conf->attributes_fixed1 = val;
-	if (!ret && !(ret = read_sys_metadata_field(0x1900000300000002, &val)))
-		sysinfo_td_conf->xfam_fixed0 = val;
-	if (!ret && !(ret = read_sys_metadata_field(0x1900000300000003, &val)))
-		sysinfo_td_conf->xfam_fixed1 = val;
-	if (!ret && !(ret = read_sys_metadata_field(0x9900000100000004, &val)))
-		sysinfo_td_conf->num_cpuid_config = val;
-	if (!ret && !(ret = read_sys_metadata_field(0x9900000100000008, &val)))
-		sysinfo_td_conf->max_vcpus_per_td = val;
-	if (sysinfo_td_conf->num_cpuid_config > ARRAY_SIZE(sysinfo_td_conf->cpuid_config_leaves))
-		return -EINVAL;
-	for (i = 0; i < sysinfo_td_conf->num_cpuid_config; i++)
-		if (!ret && !(ret = read_sys_metadata_field(0x9900000300000400 + i, &val)))
-			sysinfo_td_conf->cpuid_config_leaves[i] = val;
-	if (sysinfo_td_conf->num_cpuid_config > ARRAY_SIZE(sysinfo_td_conf->cpuid_config_values))
-		return -EINVAL;
-	for (i = 0; i < sysinfo_td_conf->num_cpuid_config; i++)
-		for (j = 0; j < 2; j++)
-			if (!ret && !(ret = read_sys_metadata_field(0x9900000300000500 + i * 2 + j, &val)))
-				sysinfo_td_conf->cpuid_config_values[i][j] = val;
-
-	return ret;
+	pr_info("header 0x%016llx id 0x%016llx elements_in_field %d field %d\n", sequence->sequence_header.raw, base_id,
+		sequence->sequence_header.last_element_in_field + 1,
+		sequence->sequence_header.last_field_in_sequence + 1);
+
+	/* Iterate over all elements in the sequence */
+	for (i = 0; i < num_elements; i++) {
+		field_id = base_id + i;
+		data = sequence->element + i;
+
+		switch (field_id) {
+		#define READ_TDX_MD(name, field)			\
+			case (TDX_MD_ID_##name & TDX_MD_ID_MASK):	\
+				field = *data;				\
+				break;
+
+		READ_TDX_MD(MINOR_VERSION, sysinfo->version.minor_version);
+		READ_TDX_MD(MAJOR_VERSION, sysinfo->version.major_version);
+		READ_TDX_MD(UPDATE_VERSION, sysinfo->version.update_version);
+
+		READ_TDX_MD(MODULE_HV, sysinfo->handoff.module_hv);
+
+		READ_TDX_MD(TDX_FEATURES0, sysinfo->features.tdx_features0);
+
+		READ_TDX_MD(MAX_TDMRS, sysinfo->tdmr.max_tdmrs);
+		READ_TDX_MD(MAX_RESERVED_PER_TDMR, sysinfo->tdmr.max_reserved_per_tdmr);
+		READ_TDX_MD(PAMT_4K_ENTRY_SIZE, sysinfo->tdmr.pamt_4k_entry_size);
+		READ_TDX_MD(PAMT_2M_ENTRY_SIZE, sysinfo->tdmr.pamt_2m_entry_size);
+		READ_TDX_MD(PAMT_1G_ENTRY_SIZE, sysinfo->tdmr.pamt_1g_entry_size);
+
+		READ_TDX_MD(TDR_BASE_SIZE, sysinfo->td_ctrl.tdr_base_size);
+		READ_TDX_MD(TDCS_BASE_SIZE, sysinfo->td_ctrl.tdcs_base_size);
+		READ_TDX_MD(TDVPS_BASE_SIZE, sysinfo->td_ctrl.tdvps_base_size);
+
+		READ_TDX_MD(ATTRIBUTES_FIXED0, sysinfo->td_conf.attributes_fixed0);
+		READ_TDX_MD(ATTRIBUTES_FIXED1, sysinfo->td_conf.attributes_fixed1);
+		READ_TDX_MD(XFAM_FIXED0, sysinfo->td_conf.xfam_fixed0);
+		READ_TDX_MD(XFAM_FIXED1, sysinfo->td_conf.xfam_fixed1);
+		READ_TDX_MD(NUM_CPUID_CONFIG, sysinfo->td_conf.num_cpuid_config);
+		READ_TDX_MD(MAX_VCPUS_PER_TD, sysinfo->td_conf.max_vcpus_per_td);
+		#undef READ_TDX_MD
+
+		case TDX_MD_ID_CPUID_CONFIG_LEAVES & TDX_MD_ID_MASK:
+			for (j = 0; j < sysinfo->td_conf.num_cpuid_config; j++)
+				sysinfo->td_conf.cpuid_config_leaves[j] = data[j];
+			/* return as all elements in this sequence are handled */
+			return;
+		case TDX_MD_ID_CPUID_CONFIG_VALUES & TDX_MD_ID_MASK:
+			for (j = 0; j < sysinfo->td_conf.num_cpuid_config; j++) {
+				sysinfo->td_conf.cpuid_config_values[j][0] = data[2*j];
+				sysinfo->td_conf.cpuid_config_values[j][1] = data[2*j+1];
+			}
+			/* return as all elements in this sequence are handled */
+			return;
+		default:
+			break;
+		}
+	}
 }
 
-static int get_tdx_sys_info_handoff(struct tdx_sys_info_handoff *sysinfo_handoff)
+static void parse_md_list(struct md_list_header *list_header, struct tdx_sys_info *sysinfo)
 {
-	int ret = 0;
-	u64 val;
-
-	if (tdx_supports_runtime_update(&tdx_sysinfo) &&
-	    !(ret = read_sys_metadata_field(0x8900000100000000, &val)))
-		sysinfo_handoff->module_hv = val;
-
-	return ret;
+	struct md_sequence *sequence;
+	u32 num_elements;
+	int i = 0;
+
+	pr_info("header: buff_size %u, num_sequences %u\n",
+		list_header->list_buff_size,
+		list_header->num_sequences);
+
+	sequence = (struct md_sequence *)(list_header + 1);
+
+	while (i < list_header->num_sequences) {
+		num_elements = (sequence->sequence_header.last_element_in_field + 1) *
+			       (sequence->sequence_header.last_field_in_sequence + 1);
+		parse_md_sequence(sequence, sysinfo);
+		sequence = (struct md_sequence *)(&sequence->element[num_elements]);
+		i++;
+	}
 }
 
 static int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
 {
+	unsigned long buffer;
+	u64 field_id = -1;
	int ret = 0;
 
-	ret = ret ?: get_tdx_sys_info_version(&sysinfo->version);
-	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);
-	ret = ret ?: get_tdx_sys_info_td_conf(&sysinfo->td_conf);
-	ret = ret ?: get_tdx_sys_info_handoff(&sysinfo->handoff);
+	buffer = __get_free_page(GFP_KERNEL);
+	if (!buffer)
+		return -ENOMEM;
+
+	do {
+		ret = read_sys_metadata_field(&field_id, (void *)buffer);
+		if (ret)
+			break;
+		parse_md_list((void *)buffer, sysinfo);
+	} while (field_id != -1);
+
+	free_page(buffer);
+	if (ret)
+		pr_info("TDX sysinfo version read all failed: %d\n", ret);
 
	return ret;
 }

Re: [PATCH 1/2] x86/virt/tdx: Retrieve TDX module version

[Kiryl Shutsemau]

On Wed, Oct 22, 2025 at 03:54:51PM +0800, Chao Gao wrote:
> On Wed, Oct 01, 2025 at 08:15:46AM -0700, Dave Hansen wrote:
> >On 9/30/25 19:22, Chao Gao wrote:
> >> +	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000003, &val)))
> >> +		sysinfo_version->minor_version = val;
> >> +	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000004, &val)))
> >> +		sysinfo_version->major_version = val;
> >> +	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000005, &val)))
> >> +		sysinfo_version->update_version = val;
> >
> >Heh, how long does this take in practice to get 6 bytes of data out of
> >the module?
> 
> ~8us. And the whole metadata reading process (i.e., get_tdx_sys_info()) takes
> ~113us.
> 
> >When is the point that we move  over to TDH.SYS.RDALL?
> 
> TDH.SYS.RDALL takes ~16us.
> 
> I'm uncertain whether the saved CPU time of ~100us justifies implementing
> TDH.SYS.RDALL.
> 
> TDH.SYS.RDALL returns all metadata as a list of arrays, requiring the kernel to
> parse this structure and iterate through all fields.
> 
> One advantage of TDH.SYS.RDALL is that it eliminates the need to check field
> existence before reading, since it simply dumps all available fields rather
> than targeting specific ones. For example, TDH.SYS.RDALL removes the need for
> the tdx_supports_ABC() check:
> 
> 	in the DPAMT series:
> 
> 	+	if (!ret && tdx_supports_dynamic_pamt(&tdx_sysinfo) &&
> 	+	    !(ret = read_sys_metadata_field(0x9100000100000013, &val)))
> 	+		sysinfo_tdmr->pamt_page_bitmap_entry_bits = val;
> 
> 	in the TDX Module update series:
> 
> 	+	if (tdx_supports_runtime_update(&tdx_sysinfo) &&
> 	+	    !(ret = read_sys_metadata_field(0x8900000100000000, &val)))
> 	+		sysinfo_handoff->module_hv = val;
> 
> While iterating through the array list adds some complexity, this is a one-time
> cost. Once the loop structure is in place, adding new fields only requires
> inserting a "switch-case" clause within the loop.
> 
> Please see the draft code below. If TDH.SYS.RDALL is the right direction, I can
> refine the code and submit a formal patch series.

I don't hate it. Seems more scalable than current approach.

See some comments below.

> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 128e6ffba736..fa9bb6d47a87 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -226,22 +226,23 @@ static int build_tdx_memlist(struct list_head *tmb_list)
> 	return ret;
>  }
>  
> -static int read_sys_metadata_field(u64 field_id, u64 *data)
> +static int read_sys_metadata_field(u64 *field_id, void *ptr)

Keeping the same name for completely different functionality?

>  {
> 	struct tdx_module_args args = {};
> 	int ret;
>  
> 	/*
> -	 * TDH.SYS.RD -- reads one global metadata field
> -	 *  - RDX (in): the field to read
> -	 *  - R8 (out): the field data
> +	 * TDH.SYS.RDALL -- reads all global metadata fields
> +	 *  - RDX (in): the physical address of the buffer to store
> +	 *  - R8 (in/out): the initial field ID to read (in) and
> +	 *		   the next field ID to read (out).
> 	 */
> -	args.rdx = field_id;
> -	ret = seamcall_prerr_ret(TDH_SYS_RD, &args);
> +	args.rdx = __pa(ptr);

Maybe take virtual address (unsigned long) of the buffer as an argument
to the function. And use virt_to_phys() here.

This way there's no need in cast on caller side.

> +	args.r8  = *field_id;
> +	ret = seamcall_prerr_ret(TDH_SYS_RDALL, &args);
> 	if (ret)
> 		return ret;
> -
> -	*data = args.r8;
> +	*field_id = args.r8;
>  
> 	return 0;

Hm. Isn't it buggy?

Caller expects to see field_id == -1 to exit loop, but you never set it
in case of an error. It will result in endless loop if error happens not on
the first iteration.

Drop the branch and always return ret.

>  }
> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> index 1965adb63f1f..44d92047073e 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.h
> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> @@ -42,6 +42,7 @@
>  #define TDH_SYS_RD			34
>  #define TDH_SYS_LP_INIT			35
>  #define TDH_SYS_TDMR_INIT		36
> +#define TDH_SYS_RDALL			37
>  #define TDH_MEM_TRACK			38
>  #define TDH_PHYMEM_CACHE_WB		40
>  #define TDH_PHYMEM_PAGE_WBINVD		41
> diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> index 3fdd5cbc21d8..f4b16367ef2f 100644
> --- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> +++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> @@ -7,121 +7,177 @@
>   * Include this file to other C file instead.
>   */
>  
> -static int get_tdx_sys_info_version(struct tdx_sys_info_version *sysinfo_version)
> +struct md_field_id
>  {
> -	int ret = 0;
> -	u64 val;
> -
> -	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000003, &val)))
> -		sysinfo_version->minor_version = val;
> -	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000004, &val)))
> -		sysinfo_version->major_version = val;
> -	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000005, &val)))
> -		sysinfo_version->update_version = val;
> -
> -	return ret;
> -}
> -
> -static int get_tdx_sys_info_features(struct tdx_sys_info_features *sysinfo_features)
> -{
> -	int ret = 0;
> -	u64 val;
> -
> -	if (!ret && !(ret = read_sys_metadata_field(0x0A00000300000008, &val)))
> -		sysinfo_features->tdx_features0 = val;
> -
> -	return ret;
> -}
> -
> -static int get_tdx_sys_info_tdmr(struct tdx_sys_info_tdmr *sysinfo_tdmr)
> +	union {
> +		struct {
> +			u32 field_code;				// Bits 31:0
> +			u32 element_size_code		: 2;	// Bits 33:32
> +			u32 last_element_in_field	: 4;	// Bits 37:34
> +			u32 last_field_in_sequence	: 9;	// Bits 46:38
> +			u32 reserved_1			: 3;	// Bits 49:47
> +			u32 inc_size			: 1;	// Bit 50
> +			u32 write_mask_valid		: 1;	// Bit 51
> +			u32 context_code		: 3;	// Bits 54:52
> +			u32 reserved_2			: 1;	// Bit 55
> +			u32 class_code			: 6;	// Bits 61:56
> +			u32 reserved_3			: 1;	// Bit 62
> +			u32 non_arch			: 1;	// Bit 63
> +		};
> +		u64 raw;
> +	};
> +};
> +struct md_list_header
>  {
> -	int ret = 0;
> -	u64 val;
> -
> -	if (!ret && !(ret = read_sys_metadata_field(0x9100000100000008, &val)))
> -		sysinfo_tdmr->max_tdmrs = val;
> -	if (!ret && !(ret = read_sys_metadata_field(0x9100000100000009, &val)))
> -		sysinfo_tdmr->max_reserved_per_tdmr = val;
> -	if (!ret && !(ret = read_sys_metadata_field(0x9100000100000010, &val)))
> -		sysinfo_tdmr->pamt_4k_entry_size = val;
> -	if (!ret && !(ret = read_sys_metadata_field(0x9100000100000011, &val)))
> -		sysinfo_tdmr->pamt_2m_entry_size = val;
> -	if (!ret && !(ret = read_sys_metadata_field(0x9100000100000012, &val)))
> -		sysinfo_tdmr->pamt_1g_entry_size = val;
> -
> -	return ret;
> -}
> +	u16 list_buff_size;
> +	u16 num_sequences;
> +	u32 reserved;
> +};
>  
> -static int get_tdx_sys_info_td_ctrl(struct tdx_sys_info_td_ctrl *sysinfo_td_ctrl)
> +struct md_sequence
>  {
> -	int ret = 0;
> -	u64 val;
> +	struct md_field_id sequence_header;
> +	u64 element[0];
> +};
> +
> +#define TDX_MD_ID_MINOR_VERSION			0x0800000100000003
> +#define TDX_MD_ID_MAJOR_VERSION			0x0800000100000004
> +#define TDX_MD_ID_UPDATE_VERSION		0x0800000100000005
> +#define TDX_MD_ID_MODULE_HV			0x8900000100000000
> +#define TDX_MD_ID_TDX_FEATURES0			0x0A00000300000008
> +#define TDX_MD_ID_MAX_TDMRS			0x9100000100000008
> +#define TDX_MD_ID_MAX_RESERVED_PER_TDMR		0x9100000100000009
> +#define TDX_MD_ID_PAMT_4K_ENTRY_SIZE		0x9100000100000010
> +#define TDX_MD_ID_PAMT_2M_ENTRY_SIZE		0x9100000100000011
> +#define TDX_MD_ID_PAMT_1G_ENTRY_SIZE		0x9100000100000012
> +#define TDX_MD_ID_TDR_BASE_SIZE			0x9800000100000000
> +#define TDX_MD_ID_TDCS_BASE_SIZE		0x9800000100000100
> +#define TDX_MD_ID_TDVPS_BASE_SIZE		0x9800000100000200
> +#define TDX_MD_ID_ATTRIBUTES_FIXED0		0x1900000300000000
> +#define TDX_MD_ID_ATTRIBUTES_FIXED1		0x1900000300000001
> +#define TDX_MD_ID_XFAM_FIXED0			0x1900000300000002
> +#define TDX_MD_ID_XFAM_FIXED1			0x1900000300000003
> +#define TDX_MD_ID_NUM_CPUID_CONFIG		0x9900000100000004
> +#define TDX_MD_ID_MAX_VCPUS_PER_TD		0x9900000100000008
> +#define TDX_MD_ID_CPUID_CONFIG_LEAVES		0x9900000300000400
> +#define TDX_MD_ID_CPUID_CONFIG_VALUES		0x9900000300000500
>  
> -	if (!ret && !(ret = read_sys_metadata_field(0x9800000100000000, &val)))
> -		sysinfo_td_ctrl->tdr_base_size = val;
> -	if (!ret && !(ret = read_sys_metadata_field(0x9800000100000100, &val)))
> -		sysinfo_td_ctrl->tdcs_base_size = val;
> -	if (!ret && !(ret = read_sys_metadata_field(0x9800000100000200, &val)))
> -		sysinfo_td_ctrl->tdvps_base_size = val;
> -
> -	return ret;
> -}
> +/*
> + * Extract CLASS_CODE (bits 61:56), CONTEXT_CODE(bits 54:52),
> + * FIELD_CODE(bits 23:0) from metadata IDs. Other bits in metadata IDs
> + * cannot be used for comparison.
> + */
> +#define TDX_MD_ID_MASK				0x3f70000000ffffff
>  
> -static int get_tdx_sys_info_td_conf(struct tdx_sys_info_td_conf *sysinfo_td_conf)
> +static void parse_md_sequence(struct md_sequence *sequence, struct tdx_sys_info *sysinfo)
>  {
> -	int ret = 0;
> -	u64 val;
> +	u32 num_elements = (sequence->sequence_header.last_element_in_field + 1) *
> +			   (sequence->sequence_header.last_field_in_sequence + 1);
> +	u64 base_id = sequence->sequence_header.raw & TDX_MD_ID_MASK;
> +	u64 field_id;
> +	u64 *data;
> 	int i, j;
>  
> -	if (!ret && !(ret = read_sys_metadata_field(0x1900000300000000, &val)))
> -		sysinfo_td_conf->attributes_fixed0 = val;
> -	if (!ret && !(ret = read_sys_metadata_field(0x1900000300000001, &val)))
> -		sysinfo_td_conf->attributes_fixed1 = val;
> -	if (!ret && !(ret = read_sys_metadata_field(0x1900000300000002, &val)))
> -		sysinfo_td_conf->xfam_fixed0 = val;
> -	if (!ret && !(ret = read_sys_metadata_field(0x1900000300000003, &val)))
> -		sysinfo_td_conf->xfam_fixed1 = val;
> -	if (!ret && !(ret = read_sys_metadata_field(0x9900000100000004, &val)))
> -		sysinfo_td_conf->num_cpuid_config = val;
> -	if (!ret && !(ret = read_sys_metadata_field(0x9900000100000008, &val)))
> -		sysinfo_td_conf->max_vcpus_per_td = val;
> -	if (sysinfo_td_conf->num_cpuid_config > ARRAY_SIZE(sysinfo_td_conf->cpuid_config_leaves))
> -		return -EINVAL;
> -	for (i = 0; i < sysinfo_td_conf->num_cpuid_config; i++)
> -		if (!ret && !(ret = read_sys_metadata_field(0x9900000300000400 + i, &val)))
> -			sysinfo_td_conf->cpuid_config_leaves[i] = val;
> -	if (sysinfo_td_conf->num_cpuid_config > ARRAY_SIZE(sysinfo_td_conf->cpuid_config_values))
> -		return -EINVAL;
> -	for (i = 0; i < sysinfo_td_conf->num_cpuid_config; i++)
> -		for (j = 0; j < 2; j++)
> -			if (!ret && !(ret = read_sys_metadata_field(0x9900000300000500 + i * 2 + j, &val)))
> -				sysinfo_td_conf->cpuid_config_values[i][j] = val;
> -
> -	return ret;
> +	pr_info("header 0x%016llx id 0x%016llx elements_in_field %d field %d\n", sequence->sequence_header.raw, base_id,
> +		sequence->sequence_header.last_element_in_field + 1,
> +		sequence->sequence_header.last_field_in_sequence + 1);
> +
> +	/* Iterate over all elements in the sequence */
> +	for (i = 0; i < num_elements; i++) {
> +		field_id = base_id + i;
> +		data = sequence->element + i;
> +
> +		switch (field_id) {
> +		#define READ_TDX_MD(name, field)			\
> +			case (TDX_MD_ID_##name & TDX_MD_ID_MASK):	\
> +				field = *data;				\
> +				break;
> +
> +		READ_TDX_MD(MINOR_VERSION, sysinfo->version.minor_version);
> +		READ_TDX_MD(MAJOR_VERSION, sysinfo->version.major_version);
> +		READ_TDX_MD(UPDATE_VERSION, sysinfo->version.update_version);
> +
> +		READ_TDX_MD(MODULE_HV, sysinfo->handoff.module_hv);
> +
> +		READ_TDX_MD(TDX_FEATURES0, sysinfo->features.tdx_features0);
> +
> +		READ_TDX_MD(MAX_TDMRS, sysinfo->tdmr.max_tdmrs);
> +		READ_TDX_MD(MAX_RESERVED_PER_TDMR, sysinfo->tdmr.max_reserved_per_tdmr);
> +		READ_TDX_MD(PAMT_4K_ENTRY_SIZE, sysinfo->tdmr.pamt_4k_entry_size);
> +		READ_TDX_MD(PAMT_2M_ENTRY_SIZE, sysinfo->tdmr.pamt_2m_entry_size);
> +		READ_TDX_MD(PAMT_1G_ENTRY_SIZE, sysinfo->tdmr.pamt_1g_entry_size);
> +
> +		READ_TDX_MD(TDR_BASE_SIZE, sysinfo->td_ctrl.tdr_base_size);
> +		READ_TDX_MD(TDCS_BASE_SIZE, sysinfo->td_ctrl.tdcs_base_size);
> +		READ_TDX_MD(TDVPS_BASE_SIZE, sysinfo->td_ctrl.tdvps_base_size);
> +
> +		READ_TDX_MD(ATTRIBUTES_FIXED0, sysinfo->td_conf.attributes_fixed0);
> +		READ_TDX_MD(ATTRIBUTES_FIXED1, sysinfo->td_conf.attributes_fixed1);
> +		READ_TDX_MD(XFAM_FIXED0, sysinfo->td_conf.xfam_fixed0);
> +		READ_TDX_MD(XFAM_FIXED1, sysinfo->td_conf.xfam_fixed1);
> +		READ_TDX_MD(NUM_CPUID_CONFIG, sysinfo->td_conf.num_cpuid_config);
> +		READ_TDX_MD(MAX_VCPUS_PER_TD, sysinfo->td_conf.max_vcpus_per_td);
> +		#undef READ_TDX_MD
> +
> +		case TDX_MD_ID_CPUID_CONFIG_LEAVES & TDX_MD_ID_MASK:
> +			for (j = 0; j < sysinfo->td_conf.num_cpuid_config; j++)
> +				sysinfo->td_conf.cpuid_config_leaves[j] = data[j];
> +			/* return as all elements in this sequence are handled */
> +			return;
> +		case TDX_MD_ID_CPUID_CONFIG_VALUES & TDX_MD_ID_MASK:
> +			for (j = 0; j < sysinfo->td_conf.num_cpuid_config; j++) {
> +				sysinfo->td_conf.cpuid_config_values[j][0] = data[2*j];
> +				sysinfo->td_conf.cpuid_config_values[j][1] = data[2*j+1];
> +			}
> +			/* return as all elements in this sequence are handled */
> +			return;
> +		default:
> +			break;
> +		}
> +	}
>  }
>  
> -static int get_tdx_sys_info_handoff(struct tdx_sys_info_handoff *sysinfo_handoff)
> +static void parse_md_list(struct md_list_header *list_header, struct tdx_sys_info *sysinfo)
>  {
> -	int ret = 0;
> -	u64 val;
> -
> -	if (tdx_supports_runtime_update(&tdx_sysinfo) &&
> -	    !(ret = read_sys_metadata_field(0x8900000100000000, &val)))
> -		sysinfo_handoff->module_hv = val;
> -
> -	return ret;
> +	struct md_sequence *sequence;
> +	u32 num_elements;
> +	int i = 0;
> +
> +	pr_info("header: buff_size %u, num_sequences %u\n",
> +		list_header->list_buff_size,
> +		list_header->num_sequences);
> +
> +	sequence = (struct md_sequence *)(list_header + 1);
> +
> +	while (i < list_header->num_sequences) {
> +		num_elements = (sequence->sequence_header.last_element_in_field + 1) *
> +			       (sequence->sequence_header.last_field_in_sequence + 1);
> +		parse_md_sequence(sequence, sysinfo);
> +		sequence = (struct md_sequence *)(&sequence->element[num_elements]);
> +		i++;
> +	}
>  }
>  
>  static int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
>  {
> +	unsigned long buffer;
> +	u64 field_id = -1;
> 	int ret = 0;
>  
> -	ret = ret ?: get_tdx_sys_info_version(&sysinfo->version);
> -	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);
> -	ret = ret ?: get_tdx_sys_info_td_conf(&sysinfo->td_conf);
> -	ret = ret ?: get_tdx_sys_info_handoff(&sysinfo->handoff);
> +	buffer = __get_free_page(GFP_KERNEL);
> +	if (!buffer)
> +		return -ENOMEM;
> +
> +	do {
> +		ret = read_sys_metadata_field(&field_id, (void *)buffer);
> +		if (ret)
> +			break;
> +		parse_md_list((void *)buffer, sysinfo);
> +	} while (field_id != -1);
> +
> +	free_page(buffer);
> +	if (ret)
> +		pr_info("TDX sysinfo version read all failed: %d\n", ret);
>  
> 	return ret;
>  }
> 

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCH 1/2] x86/virt/tdx: Retrieve TDX module version

[Chao Gao]

>I don't hate it. Seems more scalable than current approach.
>
>See some comments below.
>
>> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
>> index 128e6ffba736..fa9bb6d47a87 100644
>> --- a/arch/x86/virt/vmx/tdx/tdx.c
>> +++ b/arch/x86/virt/vmx/tdx/tdx.c
>> @@ -226,22 +226,23 @@ static int build_tdx_memlist(struct list_head *tmb_list)
>> 	return ret;
>>  }
>>  
>> -static int read_sys_metadata_field(u64 field_id, u64 *data)
>> +static int read_sys_metadata_field(u64 *field_id, void *ptr)
>
>Keeping the same name for completely different functionality?

how about read_sys_metadata_fields() or read_sys_metadata_all()?

>
>>  {
>> 	struct tdx_module_args args = {};
>> 	int ret;
>>  
>> 	/*
>> -	 * TDH.SYS.RD -- reads one global metadata field
>> -	 *  - RDX (in): the field to read
>> -	 *  - R8 (out): the field data
>> +	 * TDH.SYS.RDALL -- reads all global metadata fields
>> +	 *  - RDX (in): the physical address of the buffer to store
>> +	 *  - R8 (in/out): the initial field ID to read (in) and
>> +	 *		   the next field ID to read (out).
>> 	 */
>> -	args.rdx = field_id;
>> -	ret = seamcall_prerr_ret(TDH_SYS_RD, &args);
>> +	args.rdx = __pa(ptr);
>
>Maybe take virtual address (unsigned long) of the buffer as an argument
>to the function. And use virt_to_phys() here.
>
>This way there's no need in cast on caller side.

Sure. Will do.

>
>> +	args.r8  = *field_id;
>> +	ret = seamcall_prerr_ret(TDH_SYS_RDALL, &args);
>> 	if (ret)
>> 		return ret;
>> -
>> -	*data = args.r8;
>> +	*field_id = args.r8;
>>  
>> 	return 0;
>
>Hm. Isn't it buggy?
>
>Caller expects to see field_id == -1 to exit loop, but you never set it
>in case of an error. It will result in endless loop if error happens not on
>the first iteration.

The caller checks the return value and bails out if there was an error.

>
>Drop the branch and always return ret.

Setting field_id to -1 on error appears unnecessary since callers must check
the return value anyway. And, even if args.r8 were copied to field_id
on error, this wouldn't guarantee that field_id would be set to -1, as
SEAMCALLs may encounter #GP/#UD exceptions where r8 remains unchanged.

Given this, I prefer to leave field_id as an undefined value on error, and
callers should not read/use it when an error occurs.

What do you think?

Re: [PATCH 1/2] x86/virt/tdx: Retrieve TDX module version

[Kiryl Shutsemau]

On Fri, Oct 24, 2025 at 02:33:23PM +0800, Chao Gao wrote:
> >I don't hate it. Seems more scalable than current approach.
> >
> >See some comments below.
> >
> >> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> >> index 128e6ffba736..fa9bb6d47a87 100644
> >> --- a/arch/x86/virt/vmx/tdx/tdx.c
> >> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> >> @@ -226,22 +226,23 @@ static int build_tdx_memlist(struct list_head *tmb_list)
> >> 	return ret;
> >>  }
> >>  
> >> -static int read_sys_metadata_field(u64 field_id, u64 *data)
> >> +static int read_sys_metadata_field(u64 *field_id, void *ptr)
> >
> >Keeping the same name for completely different functionality?
> 
> how about read_sys_metadata_fields() or read_sys_metadata_all()?

Looks good.

> 
> >
> >>  {
> >> 	struct tdx_module_args args = {};
> >> 	int ret;
> >>  
> >> 	/*
> >> -	 * TDH.SYS.RD -- reads one global metadata field
> >> -	 *  - RDX (in): the field to read
> >> -	 *  - R8 (out): the field data
> >> +	 * TDH.SYS.RDALL -- reads all global metadata fields
> >> +	 *  - RDX (in): the physical address of the buffer to store
> >> +	 *  - R8 (in/out): the initial field ID to read (in) and
> >> +	 *		   the next field ID to read (out).
> >> 	 */
> >> -	args.rdx = field_id;
> >> -	ret = seamcall_prerr_ret(TDH_SYS_RD, &args);
> >> +	args.rdx = __pa(ptr);
> >
> >Maybe take virtual address (unsigned long) of the buffer as an argument
> >to the function. And use virt_to_phys() here.
> >
> >This way there's no need in cast on caller side.
> 
> Sure. Will do.
> 
> >
> >> +	args.r8  = *field_id;
> >> +	ret = seamcall_prerr_ret(TDH_SYS_RDALL, &args);
> >> 	if (ret)
> >> 		return ret;
> >> -
> >> -	*data = args.r8;
> >> +	*field_id = args.r8;
> >>  
> >> 	return 0;
> >
> >Hm. Isn't it buggy?
> >
> >Caller expects to see field_id == -1 to exit loop, but you never set it
> >in case of an error. It will result in endless loop if error happens not on
> >the first iteration.
> 
> The caller checks the return value and bails out if there was an error.

I misread it. Missed the break.

> >
> >Drop the branch and always return ret.
> 
> Setting field_id to -1 on error appears unnecessary since callers must check
> the return value anyway. And, even if args.r8 were copied to field_id
> on error, this wouldn't guarantee that field_id would be set to -1, as
> SEAMCALLs may encounter #GP/#UD exceptions where r8 remains unchanged.
> 
> Given this, I prefer to leave field_id as an undefined value on error, and
> callers should not read/use it when an error occurs.

It is not undefined. TDX module sets R8 to -1 in case of error.

> 
> What do you think?

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCH 1/2] x86/virt/tdx: Retrieve TDX module version

[Chao Gao]

>> >Hm. Isn't it buggy?
>> >
>> >Caller expects to see field_id == -1 to exit loop, but you never set it
>> >in case of an error. It will result in endless loop if error happens not on
>> >the first iteration.
>> 
>> The caller checks the return value and bails out if there was an error.
>
>I misread it. Missed the break.
>
>> >
>> >Drop the branch and always return ret.
>> 
>> Setting field_id to -1 on error appears unnecessary since callers must check
>> the return value anyway. And, even if args.r8 were copied to field_id
>> on error, this wouldn't guarantee that field_id would be set to -1, as
>> SEAMCALLs may encounter #GP/#UD exceptions where r8 remains unchanged.
>> 
>> Given this, I prefer to leave field_id as an undefined value on error, and
>> callers should not read/use it when an error occurs.
>
>It is not undefined. TDX module sets R8 to -1 in case of error.

Yes, I saw it. That's TDX module ABI. It doesn't necessarily have to be the
semantics of this kernel API. So, we have two choices:

1. Follow the TDX module, i.e., set field_id to -1 on error. Then we should do:

	if (ret)
		*field_id = -1;
	else
		*field_id = args.r8;

 to cover #GP/#UD cases.

2. Leave field_id undefined on error, as in this patch.

I don't see the value of setting field_id to -1 on error if callers are
expected to check the return value.

Re: [PATCH 1/2] x86/virt/tdx: Retrieve TDX module version

[Vishal Annapurve]

On Tue, Sep 30, 2025 at 7:24 PM Chao Gao <chao.gao@intel.com> wrote:
>
> Each TDX module is associated with a version in the x.y.z format, where x
> represents the major version, y the minor version, and z the update
> version. Knowing the running TDX module version is valuable for bug
> reporting and debugging.
>
> Retrieve the TDX module version using the existing metadata reading
> interface, in preparation for exposing it to userspace via sysfs.
>

In addition to exposing the TDX module version via sysfs, it would be
helpful to have the TDX module version captured in dmesg logs/console.

Re: [PATCH 1/2] x86/virt/tdx: Retrieve TDX module version

[Chao Gao]

On Wed, Oct 22, 2025 at 03:55:01PM +0800, Chao Gao wrote:
>On Wed, Oct 01, 2025 at 08:15:46AM -0700, Dave Hansen wrote:
>>On 9/30/25 19:22, Chao Gao wrote:
>>> +	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000003, &val)))
>>> +		sysinfo_version->minor_version = val;
>>> +	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000004, &val)))
>>> +		sysinfo_version->major_version = val;
>>> +	if (!ret && !(ret = read_sys_metadata_field(0x0800000100000005, &val)))
>>> +		sysinfo_version->update_version = val;
>>
>>Heh, how long does this take in practice to get 6 bytes of data out of
>>the module?
>
>~8us. And the whole metadata reading process (i.e., get_tdx_sys_info()) takes
>~113us.
>
>>When is the point that we move  over to TDH.SYS.RDALL?
>
>TDH.SYS.RDALL takes ~16us.
>
>I'm uncertain whether the saved CPU time of ~100us justifies implementing
>TDH.SYS.RDALL.

I chatted with Dave and Yilun offline. We think that saving 100us for this
one-off operation isn't worth the code churn.

For the record, the idea of moving over to TDH.SYS.RDALL is:

1. Call TDH.SYS.RDALL to dump all metadata into a data structure
2. Add a tdx_get_one() helper to look up the data structure and get the
   metadata for a given field ID
3. Replace read_sys_metadata_field() with tdx_get_one()