[PATCH v1 0/8] TDX: Stop auto-generating the global metadata code

[PATCH v1 0/8] TDX: Stop auto-generating the global metadata code

[Chao Gao]

This is a long-overdue cleanup of the TDX global metadata, based on work
from Dave [1].  Patches 1-6 keep Dave's authorship -- I only reworded the
commit messages and comments, with no significant changes to the code.
Note that they are missing Dave's Signed-off-by.

Changes on top of the original:

  - Rebased onto the latest tip/x86/tdx branch
  - Converted the newly added metadata (tdx module handoff)
  - Fixed up the __init annotations. The version and handoff tables must
    not be __initconst
  - Refined the commit messages and comments: dropped forward references
    to later patches and stopped restating what the code already says.
  - Added two patches: cleaning up error handling in get_tdx_sys_info(),
    and turning the runtime size check into a build-time one.

The series is also available at:
  https://github.com/gaochaointel/linux-dev.git tdx-metadata-v1

---
The TDX module exposes its capabilities and limits through "Global Scope
Metadata" fields, defined in the Intel TDX Module ABI spec.  The kernel
mirrors a small subset of those fields in C structures under struct
tdx_sys_info, populated by reading each field via TDH.SYS.RD.

Both the structures and the code that fills them are nominally generated
by an out-of-tree script from a JSON file describing the module's
metadata.  That made it trivial to add a new field, but everything else
suffered for it:

  - The generated files are edited by hand in practice, for different
    reasons.  The VMXON rework added __init annotations to the readers,
    and the handoff metadata is read at module shutdown into a
    caller-local struct rather than into tdx_sys_info.  In-flight series
    will add more: DPAMT and TDX module extension metadata may only be
    read when the corresponding TDX_FEATURES0 bit is set.  Regenerating
    the files would clobber all of these edits.

  - The generated code is opaque to anyone who doesn't have the script
    and the JSON file handy.  Each field is identified by a bare 64-bit
    hex literal, so verifying any one line means cross-referencing the
    JSON file.

  - The script ships outside the tree, so reproducing changes requires
    fetching it from a mailing list link.

  - The structures are short and stable, so the script's value over
    hand-maintained code is small.

So switch to a hand-maintained implementation.  Name each field ID after
the spec, then describe the field-ID-to-C-member mapping as a table: one
row per field, pairing the named spec field ID with the C member that
holds it.  Reading is then a walk over the table.

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/daveh/devel.git/log/?h=tdxtable

Chao Gao (2):
  x86/virt/tdx: Clean up error handling in get_tdx_sys_info()
  x86/virt/tdx: Verify the C member size against the metadata field ID

Dave Hansen (6):
  x86/virt/tdx: Stop treating tdx_global_metadata.h as auto-generated
  x86/virt/tdx: Name the TDX module global metadata field IDs
  x86/virt/tdx: Add a table-driven TDX global metadata reader
  x86/virt/tdx: Convert version/tdmr/td_ctrl/handoff readers
  x86/virt/tdx: Convert td_conf reader
  x86/virt/tdx: Remove the auto-generated tdx_global_metadata.c

 arch/x86/include/asm/tdx_global_metadata.h  |  13 +-
 arch/x86/virt/vmx/tdx/tdx.c                 | 216 +++++++++++++++++++-
 arch/x86/virt/vmx/tdx/tdx.h                 |  66 ++++++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 133 ------------
 4 files changed, 290 insertions(+), 138 deletions(-)
 delete mode 100644 arch/x86/virt/vmx/tdx/tdx_global_metadata.c

-- 
2.52.0

[PATCH v1 1/8] x86/virt/tdx: Stop treating tdx_global_metadata.h as auto-generated

[Chao Gao]

From: Dave Hansen <dave@sr71.net>

The TDX module exposes its capabilities and limits through "Global
Scope Metadata" fields, defined in the Intel TDX Module ABI spec.
The kernel mirrors a small subset of those fields in C structures
(for example, struct tdx_sys_info_version/features). Those structures
are populated by reading each field via the TDH.SYS.RD SEAMCALL.

Today the header that holds these structures is generated by an
out-of-tree script from a JSON file listing all of the TDX module's
metadata.  That made it trivial to add a new field, but everything
else suffered for it:

  - The header is opaque to anyone who doesn't have the script and
    the JSON file handy, and the "Automatically generated" tag tells
    reviewers their edits will be clobbered.
  - The script ships outside the tree, so reproducing changes
    requires fetching it from a mailing list link.
  - The structures are short and stable; the script's value over
    a hand-edited header is small.

In preparation for switching to a hand-maintained implementation,
mark the header as such:

  - Drop the "Automatically generated" comment.
  - Rename the header guard to a non-"AUTO_GENERATED" name following
    the usual asm/ convention.
  - Add a comment describing what the structures are and how they
    are populated.

No functional change intended.

Assisted-by: Claude:claude-opus-5
Not-yet-signed-off-by: Dave Hansen <dave@sr71.net>
Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 arch/x86/include/asm/tdx_global_metadata.h | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/tdx_global_metadata.h b/arch/x86/include/asm/tdx_global_metadata.h
index 41150d546589..a86be9e5ec77 100644
--- a/arch/x86/include/asm/tdx_global_metadata.h
+++ b/arch/x86/include/asm/tdx_global_metadata.h
@@ -1,10 +1,15 @@
 /* SPDX-License-Identifier: GPL-2.0 */
-/* Automatically generated TDX global metadata structures. */
-#ifndef _X86_VIRT_TDX_AUTO_GENERATED_TDX_GLOBAL_METADATA_H
-#define _X86_VIRT_TDX_AUTO_GENERATED_TDX_GLOBAL_METADATA_H
+#ifndef _ASM_X86_TDX_GLOBAL_METADATA_H
+#define _ASM_X86_TDX_GLOBAL_METADATA_H
 
 #include <linux/types.h>
 
+/*
+ * TDX module "Global Scope Metadata" as documented in the Intel TDX
+ * Module ABI spec. Each sub-structure below corresponds to one TDX
+ * metadata "Class"; its members are populated via TDH.SYS.RD SEAMCALLs.
+ */
+
 struct tdx_sys_info_version {
 	u16 minor_version;
 	u16 major_version;
@@ -52,4 +57,4 @@ struct tdx_sys_info {
 	struct tdx_sys_info_td_conf td_conf;
 };
 
-#endif
+#endif /* _ASM_X86_TDX_GLOBAL_METADATA_H */
-- 
2.52.0

[PATCH v1 2/8] x86/virt/tdx: Name the TDX module global metadata field IDs

[Chao Gao]

From: Dave Hansen <dave@sr71.net>

tdx_global_metadata.c currently identifies each field by a bare 64-bit hex
literal, extracted from the JSON file by an out-of-tree script:

	if (!ret && !(ret = read_sys_metadata_field(0x9100000100000008, &val)))
		sysinfo_tdmr->max_tdmrs = val;

That is unreviewable on its own. Verifying any one line requires the
reviewer to cross-reference the JSON file.

Add MD_FIELD_ID_<NAME> constants for every global metadata field the kernel
currently reads (Version, Features, etc), grouped by class. This follows
how the kernel handles ABI-defined literals elsewhere. The IDs can then be
checked against the spec in one pass, and the read sites become readable
without the JSON file.

The hex literals in tdx_global_metadata.c are left alone because that file
will be replaced by a table-driven reader in the following patches.

No functional change intended.

Assisted-by: Claude:claude-opus-5
Not-yet-signed-off-by: Dave Hansen <dave@sr71.net>
Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 arch/x86/virt/vmx/tdx/tdx.h | 51 +++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index bdfd0e1e337a..5f567cb6c07a 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -58,6 +58,57 @@
  */
 #define TDX_VERSION_SHIFT		16
 
+/*
+ * Global Scope Metadata field IDs.
+ *
+ * See "Global-Scope (TDX Module) Metadata" in the Intel TDX Module ABI
+ * spec.
+ *
+ * A field ID is a 64-bit value that encodes the metadata "Class"
+ * (which Linux mirrors in 'struct tdx_sys_info' sub-structures),
+ * the element size, and a per-class field index. Each ID below
+ * is paired with the C member that holds its value.
+ */
+
+/* Class "TDX Module Version" */
+#define MD_FIELD_ID_MINOR_VERSION		0x0800000100000003ULL
+#define MD_FIELD_ID_MAJOR_VERSION		0x0800000100000004ULL
+#define MD_FIELD_ID_UPDATE_VERSION		0x0800000100000005ULL
+
+/* Class "TDX Features" */
+#define MD_FIELD_ID_TDX_FEATURES0		0x0A00000300000008ULL
+
+/* Class "TDMR Info" */
+#define MD_FIELD_ID_MAX_TDMRS			0x9100000100000008ULL
+#define MD_FIELD_ID_MAX_RESERVED_PER_TDMR	0x9100000100000009ULL
+#define MD_FIELD_ID_PAMT_4K_ENTRY_SIZE		0x9100000100000010ULL
+#define MD_FIELD_ID_PAMT_2M_ENTRY_SIZE		0x9100000100000011ULL
+#define MD_FIELD_ID_PAMT_1G_ENTRY_SIZE		0x9100000100000012ULL
+
+/* Class "TD Control Structures" */
+#define MD_FIELD_ID_TDR_BASE_SIZE		0x9800000100000000ULL
+#define MD_FIELD_ID_TDCS_BASE_SIZE		0x9800000100000100ULL
+#define MD_FIELD_ID_TDVPS_BASE_SIZE		0x9800000100000200ULL
+
+/* Class "TD Configuration" */
+#define MD_FIELD_ID_ATTRIBUTES_FIXED0		0x1900000300000000ULL
+#define MD_FIELD_ID_ATTRIBUTES_FIXED1		0x1900000300000001ULL
+#define MD_FIELD_ID_XFAM_FIXED0			0x1900000300000002ULL
+#define MD_FIELD_ID_XFAM_FIXED1			0x1900000300000003ULL
+#define MD_FIELD_ID_NUM_CPUID_CONFIG		0x9900000100000004ULL
+#define MD_FIELD_ID_MAX_VCPUS_PER_TD		0x9900000100000008ULL
+
+/* Class "TDX Module Handoff" */
+#define MD_FIELD_ID_MODULE_HV			0x8900000100000000ULL
+
+/*
+ * Base IDs for the configurable-CPUID arrays.  The field ID of leaf
+ * index @i is BASE + i; for the values array, sub-entry @j of index
+ * @i is BASE + i*2 + j.
+ */
+#define MD_FIELD_ID_CPUID_CONFIG_LEAVES		0x9900000300000400ULL
+#define MD_FIELD_ID_CPUID_CONFIG_VALUES		0x9900000300000500ULL
+
 /* TDX page types */
 #define	PT_NDA		0x0
 #define	PT_RSVD		0x1
-- 
2.52.0

[PATCH v1 3/8] x86/virt/tdx: Add a table-driven TDX global metadata reader

[Chao Gao]

From: Dave Hansen <dave@sr71.net>

The auto-generated tdx_global_metadata.c populates each member of
'struct tdx_sys_info' with a chain like

	if (!ret && !(ret = read_sys_metadata_field(0x..., &val)))
		sysinfo_xxx->member = val;

repeated once per field, with a bare hex literal for the field ID and an
implicit narrowing assignment.  Reading it requires the JSON file, and
reviewing any change to it requires the out-of-tree script.

Replace it with a small table-driven reader. Each table entry pairs
a named MD_FIELD_ID_* with the C member that holds its value:

	#define MAP_FEATURES(_field_id, _member)	\
		TD_SYSINFO_MAP(_field_id, tdx_sys_info_features, _member)

	static const struct tdx_sys_field features_fields[] __initconst = {
		MAP_FEATURES(TDX_FEATURES0,	tdx_features0),
	};

TD_SYSINFO_MAP() derives the destination offset and width from the
struct via offsetof()/sizeof_field().

Add read_sys_metadata_table() to walk such a table, reading each field
and populating the C struct.

Three things fall out of this table-driven reader, all of which matter now
that the code is hand-maintained rather than auto-generated:

  - The field-to-member pairing becomes data rather than code embedded
    in control flow.
  - The read logic exists once instead of once per field.
  - The width is recorded explicitly instead of being implied by a
    narrowing assignment.

Convert the 'features' class only, as an example. The remaining classes
follow in later patches.

Assisted-by: Claude:claude-opus-5
Not-yet-signed-off-by: Dave Hansen <dave@sr71.net>
Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 arch/x86/virt/vmx/tdx/tdx.c                 | 60 +++++++++++++++++++++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 11 ----
 2 files changed, 60 insertions(+), 11 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 42df8ea464c4..fb21433e7851 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -347,6 +347,66 @@ static int read_sys_metadata_field(u64 field_id, u64 *data)
 	return 0;
 }
 
+/*
+ * Mapping between a TDX global metadata field and the C member that
+ * holds its value. Use TD_SYSINFO_MAP() to populate entries.
+ */
+struct tdx_sys_field {
+	u64 field_id;
+	u16 offset;
+	u8  size;
+};
+
+#define TD_SYSINFO_MAP(_field_id, _struct, _member)				\
+	{									\
+		.field_id = MD_FIELD_ID_##_field_id,				\
+		.offset   = offsetof(struct _struct, _member),			\
+		.size     = sizeof_field(struct _struct, _member),		\
+	}
+
+/*
+ * Walk a table of TDX global metadata fields, read each via TDH.SYS.RD,
+ * and store the result into the matching C member of *@base.
+ */
+static int read_sys_metadata_table(const struct tdx_sys_field *fields,
+				   int nr_fields, void *base)
+{
+	int i, ret;
+	u64 val;
+
+	for (i = 0; i < nr_fields; i++) {
+		const struct tdx_sys_field *f = &fields[i];
+
+		ret = read_sys_metadata_field(f->field_id, &val);
+		if (ret)
+			return ret;
+
+		switch (f->size) {
+		case 1: *(u8  *)(base + f->offset) = val; break;
+		case 2: *(u16 *)(base + f->offset) = val; break;
+		case 4: *(u32 *)(base + f->offset) = val; break;
+		case 8: *(u64 *)(base + f->offset) = val; break;
+		default:
+			return -EINVAL;
+		}
+	}
+	return 0;
+}
+
+#define MAP_FEATURES(_field_id, _member)	\
+	TD_SYSINFO_MAP(_field_id, tdx_sys_info_features, _member)
+
+static const struct tdx_sys_field features_fields[] __initconst = {
+	MAP_FEATURES(TDX_FEATURES0,	tdx_features0),
+};
+
+static __init int get_tdx_sys_info_features(struct tdx_sys_info_features *sysinfo_features)
+{
+	return read_sys_metadata_table(features_fields,
+				       ARRAY_SIZE(features_fields),
+				       sysinfo_features);
+}
+
 #include "tdx_global_metadata.c"
 
 static __init int check_features(struct tdx_sys_info *sysinfo)
diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
index e49c300f23d4..e69c655a91a0 100644
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
@@ -22,17 +22,6 @@ static int get_tdx_sys_info_version(struct tdx_sys_info_version *sysinfo_version
 	return ret;
 }
 
-static __init 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 __init int get_tdx_sys_info_tdmr(struct tdx_sys_info_tdmr *sysinfo_tdmr)
 {
 	int ret = 0;
-- 
2.52.0

[PATCH v1 4/8] x86/virt/tdx: Convert version/tdmr/td_ctrl/handoff readers

[Chao Gao]

From: Dave Hansen <dave@sr71.net>

Convert the simple-scalar global-metadata classes to the table-driven
reader.

Drop the corresponding functions from the auto-generated file.

Note that the version and handoff tables are not tagged __initconst,
as their readers run at runtime.

Assisted-by: Claude:claude-opus-5
Not-yet-signed-off-by: Dave Hansen <dave@sr71.net>
Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 arch/x86/virt/vmx/tdx/tdx.c                 | 64 +++++++++++++++++++++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 62 --------------------
 2 files changed, 64 insertions(+), 62 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index fb21433e7851..89055afeef68 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -393,6 +393,22 @@ static int read_sys_metadata_table(const struct tdx_sys_field *fields,
 	return 0;
 }
 
+#define MAP_VERSION(_field_id, _member)	\
+	TD_SYSINFO_MAP(_field_id, tdx_sys_info_version, _member)
+
+static const struct tdx_sys_field version_fields[] = {
+	MAP_VERSION(MINOR_VERSION,	minor_version),
+	MAP_VERSION(MAJOR_VERSION,	major_version),
+	MAP_VERSION(UPDATE_VERSION,	update_version),
+};
+
+static int get_tdx_sys_info_version(struct tdx_sys_info_version *sysinfo_version)
+{
+	return read_sys_metadata_table(version_fields,
+				       ARRAY_SIZE(version_fields),
+				       sysinfo_version);
+}
+
 #define MAP_FEATURES(_field_id, _member)	\
 	TD_SYSINFO_MAP(_field_id, tdx_sys_info_features, _member)
 
@@ -407,6 +423,54 @@ static __init int get_tdx_sys_info_features(struct tdx_sys_info_features *sysinf
 				       sysinfo_features);
 }
 
+#define MAP_TDMR(_field_id, _member)	\
+	TD_SYSINFO_MAP(_field_id, tdx_sys_info_tdmr, _member)
+
+static const struct tdx_sys_field tdmr_fields[] __initconst = {
+	MAP_TDMR(MAX_TDMRS,		max_tdmrs),
+	MAP_TDMR(MAX_RESERVED_PER_TDMR,	max_reserved_per_tdmr),
+	MAP_TDMR(PAMT_4K_ENTRY_SIZE,	pamt_4k_entry_size),
+	MAP_TDMR(PAMT_2M_ENTRY_SIZE,	pamt_2m_entry_size),
+	MAP_TDMR(PAMT_1G_ENTRY_SIZE,	pamt_1g_entry_size),
+};
+
+static __init int get_tdx_sys_info_tdmr(struct tdx_sys_info_tdmr *sysinfo_tdmr)
+{
+	return read_sys_metadata_table(tdmr_fields,
+				       ARRAY_SIZE(tdmr_fields),
+				       sysinfo_tdmr);
+}
+
+#define MAP_TD_CTRL(_field_id, _member)	\
+	TD_SYSINFO_MAP(_field_id, tdx_sys_info_td_ctrl, _member)
+
+static const struct tdx_sys_field td_ctrl_fields[] __initconst = {
+	MAP_TD_CTRL(TDR_BASE_SIZE,	tdr_base_size),
+	MAP_TD_CTRL(TDCS_BASE_SIZE,	tdcs_base_size),
+	MAP_TD_CTRL(TDVPS_BASE_SIZE,	tdvps_base_size),
+};
+
+static __init int get_tdx_sys_info_td_ctrl(struct tdx_sys_info_td_ctrl *sysinfo_td_ctrl)
+{
+	return read_sys_metadata_table(td_ctrl_fields,
+				       ARRAY_SIZE(td_ctrl_fields),
+				       sysinfo_td_ctrl);
+}
+
+#define MAP_HANDOFF(_field_id, _member)	\
+	TD_SYSINFO_MAP(_field_id, tdx_sys_info_handoff, _member)
+
+static const struct tdx_sys_field handoff_fields[] = {
+	MAP_HANDOFF(MODULE_HV,	module_hv),
+};
+
+static int get_tdx_sys_info_handoff(struct tdx_sys_info_handoff *sysinfo_handoff)
+{
+	return read_sys_metadata_table(handoff_fields,
+				       ARRAY_SIZE(handoff_fields),
+				       sysinfo_handoff);
+}
+
 #include "tdx_global_metadata.c"
 
 static __init int check_features(struct tdx_sys_info *sysinfo)
diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
index e69c655a91a0..0c2cc99f1af1 100644
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
@@ -7,55 +7,6 @@
  * 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 __init int get_tdx_sys_info_tdmr(struct tdx_sys_info_tdmr *sysinfo_tdmr)
-{
-	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;
-}
-
-static __init int get_tdx_sys_info_td_ctrl(struct tdx_sys_info_td_ctrl *sysinfo_td_ctrl)
-{
-	int ret = 0;
-	u64 val;
-
-	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;
-}
-
 static __init int get_tdx_sys_info_td_conf(struct tdx_sys_info_td_conf *sysinfo_td_conf)
 {
 	int ret = 0;
@@ -89,19 +40,6 @@ static __init int get_tdx_sys_info_td_conf(struct tdx_sys_info_td_conf *sysinfo_
 	return ret;
 }
 
-static int get_tdx_sys_info_handoff(struct tdx_sys_info_handoff *sysinfo_handoff)
-{
-	int ret;
-	u64 val;
-
-	ret = read_sys_metadata_field(0x8900000100000000, &val);
-	if (ret)
-		return ret;
-
-	sysinfo_handoff->module_hv = val;
-	return 0;
-}
-
 static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
 {
 	int ret = 0;
-- 
2.52.0

[PATCH v1 5/8] x86/virt/tdx: Convert td_conf reader

[Chao Gao]

From: Dave Hansen <dave@sr71.net>

Convert the "TD Configuration" class to the table-driven reader.

This class has two parts:

  - Six scalar fields (attributes_fixed{0,1}, xfam_fixed{0,1},
    num_cpuid_config, max_vcpus_per_td) that fit straight into a
    TD_SYSINFO_MAP table.

  - Two arrays (cpuid_config_leaves[] and cpuid_config_values[][])
    whose lengths come from num_cpuid_config and whose field IDs
    are computed from a base announced by the spec:

	field_id(leaves[i])     = MD_FIELD_ID_CPUID_CONFIG_LEAVES + i
	field_id(values[i][j])  = MD_FIELD_ID_CPUID_CONFIG_VALUES + i*2 + j

    The arrays can't be expressed as a static table, so read them
    explicitly after the scalar block has populated num_cpuid_config.

Bounds-check num_cpuid_config against the C array sizes before
indexing, matching the prior generated code.

Drop the corresponding function from the auto-generated file.

Assisted-by: Claude:claude-opus-5
Not-yet-signed-off-by: Dave Hansen <dave@sr71.net>
Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 arch/x86/virt/vmx/tdx/tdx.c                 | 56 +++++++++++++++++++++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 33 ------------
 2 files changed, 56 insertions(+), 33 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 89055afeef68..98534e702144 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -471,6 +471,62 @@ static int get_tdx_sys_info_handoff(struct tdx_sys_info_handoff *sysinfo_handoff
 				       sysinfo_handoff);
 }
 
+#define MAP_TD_CONF(_field_id, _member)	\
+	TD_SYSINFO_MAP(_field_id, tdx_sys_info_td_conf, _member)
+
+/*
+ * Scalar fields of the "TD Configuration" class.  num_cpuid_config
+ * must be present here (and must be read before the CPUID arrays
+ * below) because it sizes them.
+ */
+static const struct tdx_sys_field td_conf_fields[] __initconst = {
+	MAP_TD_CONF(ATTRIBUTES_FIXED0,	attributes_fixed0),
+	MAP_TD_CONF(ATTRIBUTES_FIXED1,	attributes_fixed1),
+	MAP_TD_CONF(XFAM_FIXED0,	xfam_fixed0),
+	MAP_TD_CONF(XFAM_FIXED1,	xfam_fixed1),
+	MAP_TD_CONF(NUM_CPUID_CONFIG,	num_cpuid_config),
+	MAP_TD_CONF(MAX_VCPUS_PER_TD,	max_vcpus_per_td),
+};
+
+static __init int get_tdx_sys_info_td_conf(struct tdx_sys_info_td_conf *td_conf)
+{
+	int ret, i, j;
+
+	ret = read_sys_metadata_table(td_conf_fields,
+				      ARRAY_SIZE(td_conf_fields),
+				      td_conf);
+	if (ret)
+		return ret;
+
+	/*
+	 * The configurable-CPUID arrays are sized at runtime by
+	 * num_cpuid_config, so they can't be expressed in a static
+	 * TD_SYSINFO_MAP table.  Their field IDs are contiguous from
+	 * the bases announced by the spec.
+	 */
+	if (td_conf->num_cpuid_config > ARRAY_SIZE(td_conf->cpuid_config_leaves) ||
+	    td_conf->num_cpuid_config > ARRAY_SIZE(td_conf->cpuid_config_values))
+		return -EINVAL;
+
+	for (i = 0; i < td_conf->num_cpuid_config; i++) {
+		ret = read_sys_metadata_field(MD_FIELD_ID_CPUID_CONFIG_LEAVES + i,
+					      &td_conf->cpuid_config_leaves[i]);
+		if (ret)
+			return ret;
+
+		for (j = 0; j < 2; j++) {
+			u64 fid = MD_FIELD_ID_CPUID_CONFIG_VALUES + i * 2 + j;
+
+			ret = read_sys_metadata_field(fid,
+						      &td_conf->cpuid_config_values[i][j]);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
 #include "tdx_global_metadata.c"
 
 static __init int check_features(struct tdx_sys_info *sysinfo)
diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
index 0c2cc99f1af1..4d673cac0976 100644
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
@@ -7,39 +7,6 @@
  * Include this file to other C file instead.
  */
 
-static __init int get_tdx_sys_info_td_conf(struct tdx_sys_info_td_conf *sysinfo_td_conf)
-{
-	int ret = 0;
-	u64 val;
-	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;
-}
-
 static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
 {
 	int ret = 0;
-- 
2.52.0

[PATCH v1 6/8] x86/virt/tdx: Remove the auto-generated tdx_global_metadata.c

[Chao Gao]

From: Dave Hansen <dave@sr71.net>

With all metadata classes converted, the only thing left in the
auto-generated tdx_global_metadata.c is get_tdx_sys_info().

Move it into tdx.c so that the auto-generated file can be deleted, along
with the unusual `#include "tdx_global_metadata.c"`.

No functional change intended.

Assisted-by: Claude:claude-opus-5
Not-yet-signed-off-by: Dave Hansen <dave@sr71.net>
Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 arch/x86/virt/vmx/tdx/tdx.c                 | 19 ++++++++++++++-
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 27 ---------------------
 2 files changed, 18 insertions(+), 28 deletions(-)
 delete mode 100644 arch/x86/virt/vmx/tdx/tdx_global_metadata.c

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 98534e702144..1b8cd7656f36 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -527,7 +527,24 @@ static __init int get_tdx_sys_info_td_conf(struct tdx_sys_info_td_conf *td_conf)
 	return 0;
 }
 
-#include "tdx_global_metadata.c"
+static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
+{
+	int ret = 0;
+
+	ret = ret ?: get_tdx_sys_info_version(&sysinfo->version);
+
+	pr_info("Module version: " TDX_VERSION_FMT "\n",
+		sysinfo->version.major_version,
+		sysinfo->version.minor_version,
+		sysinfo->version.update_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);
+
+	return ret;
+}
 
 static __init int check_features(struct tdx_sys_info *sysinfo)
 {
diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
deleted file mode 100644
index 4d673cac0976..000000000000
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ /dev/null
@@ -1,27 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Automatically generated functions to read TDX global metadata.
- *
- * This file doesn't compile on its own as it lacks of inclusion
- * of SEAMCALL wrapper primitive which reads global metadata.
- * Include this file to other C file instead.
- */
-
-static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
-{
-	int ret = 0;
-
-	ret = ret ?: get_tdx_sys_info_version(&sysinfo->version);
-
-	pr_info("Module version: " TDX_VERSION_FMT "\n",
-		sysinfo->version.major_version,
-		sysinfo->version.minor_version,
-		sysinfo->version.update_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);
-
-	return ret;
-}
-- 
2.52.0

[PATCH v1 7/8] x86/virt/tdx: Clean up error handling in get_tdx_sys_info()

[Chao Gao]

get_tdx_sys_info() chains its calls with

	ret = ret ?: get_tdx_sys_info_foo(...);

so that the remaining reads are skipped once one fails. That is an artifact
of the code generator rather than the usual kernel idiom. Use plain early
returns instead.

The version is still printed before the error from reading it is checked,
so a failed read is reported with whatever the print shows.

No functional change intended.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 arch/x86/virt/vmx/tdx/tdx.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 1b8cd7656f36..4bf21848df62 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -529,21 +529,31 @@ static __init int get_tdx_sys_info_td_conf(struct tdx_sys_info_td_conf *td_conf)
 
 static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
 {
-	int ret = 0;
+	int ret;
 
-	ret = ret ?: get_tdx_sys_info_version(&sysinfo->version);
+	ret = get_tdx_sys_info_version(&sysinfo->version);
 
 	pr_info("Module version: " TDX_VERSION_FMT "\n",
 		sysinfo->version.major_version,
 		sysinfo->version.minor_version,
 		sysinfo->version.update_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);
+	if (ret)
+		return ret;
 
-	return ret;
+	ret = get_tdx_sys_info_features(&sysinfo->features);
+	if (ret)
+		return ret;
+
+	ret = get_tdx_sys_info_tdmr(&sysinfo->tdmr);
+	if (ret)
+		return ret;
+
+	ret = get_tdx_sys_info_td_ctrl(&sysinfo->td_ctrl);
+	if (ret)
+		return ret;
+
+	return get_tdx_sys_info_td_conf(&sysinfo->td_conf);
 }
 
 static __init int check_features(struct tdx_sys_info *sysinfo)
-- 
2.52.0

[PATCH v1 8/8] x86/virt/tdx: Verify the C member size against the metadata field ID

[Chao Gao]

Each TDX global metadata field ID encodes the size of that field.
read_sys_metadata_table() stores each value at the width recorded in
the table, which TD_SYSINFO_MAP() derives from the destination C member.
Nothing checks that the two agree.

A table entry naming the wrong field ID, or a struct member declared at
the wrong width, would silently truncate the value read from the TDX
module. That is a kernel-side bug rather than a TDX module problem.

Add macros to extract the encoded size from a field ID, and use them in
TD_SYSINFO_MAP() to assert that it matches the member size.  Both are
compile-time constants, so the check costs nothing at runtime.

Note that BUILD_BUG_ON() cannot be used in a structure initializer; use
BUILD_BUG_ON_ZERO() instead, which yields 0 and so can be folded into the
.size initializer without changing its value.

No functional change intended.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 arch/x86/virt/vmx/tdx/tdx.c |  9 ++++++++-
 arch/x86/virt/vmx/tdx/tdx.h | 15 +++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 4bf21848df62..59099cc15f7a 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -357,11 +357,18 @@ struct tdx_sys_field {
 	u8  size;
 };
 
+/*
+ * The size encoded in the field ID and the size of the destination C
+ * member must agree; BUILD_BUG_ON_ZERO() enforces this at compile time.
+ */
 #define TD_SYSINFO_MAP(_field_id, _struct, _member)				\
 	{									\
 		.field_id = MD_FIELD_ID_##_field_id,				\
 		.offset   = offsetof(struct _struct, _member),			\
-		.size     = sizeof_field(struct _struct, _member),		\
+		.size     = sizeof_field(struct _struct, _member) +		\
+			    BUILD_BUG_ON_ZERO(					\
+				sizeof_field(struct _struct, _member) !=	\
+				MD_FIELD_ID_ELE_SIZE(MD_FIELD_ID_##_field_id)),	\
 	}
 
 /*
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index 5f567cb6c07a..c612b1cf7c14 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -109,6 +109,21 @@
 #define MD_FIELD_ID_CPUID_CONFIG_LEAVES		0x9900000300000400ULL
 #define MD_FIELD_ID_CPUID_CONFIG_VALUES		0x9900000300000500ULL
 
+/*
+ * Sub-field definitions of MD_FIELD_ID.
+ *
+ * See "MD_FIELD_ID (Metadata Field Identifier / Sequence Header)
+ * Definition" in the Intel TDX Module ABI spec.
+ *
+ *  - Bit 33:32: ELEMENT_SIZE_CODE -- log2 of a single metadata
+ *                                    element's size in bytes
+ */
+#define MD_FIELD_ID_ELE_SIZE_CODE(field_id)	\
+	(((field_id) & GENMASK_ULL(33, 32)) >> 32)
+
+#define MD_FIELD_ID_ELE_SIZE(field_id)		\
+	(1 << MD_FIELD_ID_ELE_SIZE_CODE(field_id))
+
 /* TDX page types */
 #define	PT_NDA		0x0
 #define	PT_RSVD		0x1
-- 
2.52.0

Re: [PATCH v1 0/8] TDX: Stop auto-generating the global metadata code

[Dave Hansen]

On 8/4/26 04:29, Chao Gao wrote:
> This is a long-overdue cleanup of the TDX global metadata, based on work
> from Dave [1].

I wouldn't say "from Dave". At best, I'd say "AI slop in response to
Dave's prompt" or "work vibe coded by Dave".

But seriously, all I wanted to do was show my fellow humans that it
wasn't an insurmountable task.

> Patches 1-6 keep Dave's authorship -- I only reworded the
> commit messages and comments, with no significant changes to the code.
> Note that they are missing Dave's Signed-off-by.

Yes, and that was intentional.

Re: [PATCH v1 1/8] x86/virt/tdx: Stop treating tdx_global_metadata.h as auto-generated

[Dave Hansen]

On 8/4/26 04:29, Chao Gao wrote:
> From: Dave Hansen <dave@sr71.net>
> 
> The TDX module exposes its capabilities and limits through "Global
> Scope Metadata" fields, defined in the Intel TDX Module ABI spec.
> The kernel mirrors a small subset of those fields in C structures
> (for example, struct tdx_sys_info_version/features). Those structures
> are populated by reading each field via the TDH.SYS.RD SEAMCALL.
> 
> Today the header that holds these structures is generated by an
> out-of-tree script from a JSON file listing all of the TDX module's
> metadata.  That made it trivial to add a new field, but everything
> else suffered for it:
> 
>   - The header is opaque to anyone who doesn't have the script and
>     the JSON file handy, and the "Automatically generated" tag tells
>     reviewers their edits will be clobbered.
>   - The script ships outside the tree, so reproducing changes
>     requires fetching it from a mailing list link.
>   - The structures are short and stable; the script's value over
>     a hand-edited header is small.

Well, and the big one: after we started doing this, the "ABI Definitions
for Intel® TDX" was declared to be not an ABI. So, even if we code to
the JSON, there's no guarantee the JSON will be stable.

<sigh>

Honestly that's what matters. The script and all the other fluff is just
noise.

> Signed-off-by: Chao Gao <chao.gao@intel.com>

So, I'm curious: What made you feel OK to sign-off on this? I'm not
judging. I'm open-minded on this. All I know is *I* wasn't ready to
sign-off on this.

I really want to know what your thought process was.

Re: [PATCH v1 2/8] x86/virt/tdx: Name the TDX module global metadata field IDs

[Dave Hansen]

On 8/4/26 04:29, Chao Gao wrote:
> From: Dave Hansen <dave@sr71.net>
> 
> tdx_global_metadata.c currently identifies each field by a bare 64-bit hex
> literal, extracted from the JSON file by an out-of-tree script:
> 
> 	if (!ret && !(ret = read_sys_metadata_field(0x9100000100000008, &val)))
> 		sysinfo_tdmr->max_tdmrs = val;
> 
> That is unreviewable on its own. Verifying any one line requires the
> reviewer to cross-reference the JSON file.

I'm looking at "global_metadata.pdf" from this[1]. I see a line with
"MAX_TDMRS" and "0x9100000100000008". That matches the lines above. That
seems *FAR* from unreviewable. There's also no JSON in sight.

What am I missing?

Listen, I don't like how this turned out. I'm asking for it to be
changed. But let's not justify it with things that just aren't true.

> The hex literals in tdx_global_metadata.c are left alone because that file
> will be replaced by a table-driven reader in the following patches.

This part of the changelog is good.

> No functional change intended.

This is a bit much. :)

> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> index bdfd0e1e337a..5f567cb6c07a 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.h
> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> @@ -58,6 +58,57 @@
>   */
>  #define TDX_VERSION_SHIFT		16
>  
> +/*
> + * Global Scope Metadata field IDs.
> + *
> + * See "Global-Scope (TDX Module) Metadata" in the Intel TDX Module ABI
> + * spec.
> + *
> + * A field ID is a 64-bit value that encodes the metadata "Class"
> + * (which Linux mirrors in 'struct tdx_sys_info' sub-structures),
> + * the element size, and a per-class field index. Each ID below
> + * is paired with the C member that holds its value.
> + */

This is a bit verbose for my taste.

> +/* Class "TDX Module Version" */
> +#define MD_FIELD_ID_MINOR_VERSION		0x0800000100000003ULL
> +#define MD_FIELD_ID_MAJOR_VERSION		0x0800000100000004ULL
> +#define MD_FIELD_ID_UPDATE_VERSION		0x0800000100000005ULL
> +
> +/* Class "TDX Features" */
> +#define MD_FIELD_ID_TDX_FEATURES0		0x0A00000300000008ULL
> +
> +/* Class "TDMR Info" */
> +#define MD_FIELD_ID_MAX_TDMRS			0x9100000100000008ULL
> +#define MD_FIELD_ID_MAX_RESERVED_PER_TDMR	0x9100000100000009ULL
> +#define MD_FIELD_ID_PAMT_4K_ENTRY_SIZE		0x9100000100000010ULL
> +#define MD_FIELD_ID_PAMT_2M_ENTRY_SIZE		0x9100000100000011ULL
> +#define MD_FIELD_ID_PAMT_1G_ENTRY_SIZE		0x9100000100000012ULL
> +
> +/* Class "TD Control Structures" */
> +#define MD_FIELD_ID_TDR_BASE_SIZE		0x9800000100000000ULL
> +#define MD_FIELD_ID_TDCS_BASE_SIZE		0x9800000100000100ULL
> +#define MD_FIELD_ID_TDVPS_BASE_SIZE		0x9800000100000200ULL
> +
> +/* Class "TD Configuration" */
> +#define MD_FIELD_ID_ATTRIBUTES_FIXED0		0x1900000300000000ULL
> +#define MD_FIELD_ID_ATTRIBUTES_FIXED1		0x1900000300000001ULL
> +#define MD_FIELD_ID_XFAM_FIXED0			0x1900000300000002ULL
> +#define MD_FIELD_ID_XFAM_FIXED1			0x1900000300000003ULL
> +#define MD_FIELD_ID_NUM_CPUID_CONFIG		0x9900000100000004ULL
> +#define MD_FIELD_ID_MAX_VCPUS_PER_TD		0x9900000100000008ULL
> +
> +/* Class "TDX Module Handoff" */
> +#define MD_FIELD_ID_MODULE_HV			0x8900000100000000ULL

These are fine on their own.

> +/*
> + * Base IDs for the configurable-CPUID arrays.  The field ID of leaf
> + * index @i is BASE + i; for the values array, sub-entry @j of index
> + * @i is BASE + i*2 + j.
> + */
> +#define MD_FIELD_ID_CPUID_CONFIG_LEAVES		0x9900000300000400ULL
> +#define MD_FIELD_ID_CPUID_CONFIG_VALUES		0x9900000300000500ULL

This is complete jibberish without more context.

1,
https://www.intel.com/content/www/us/en/content-details/865803/abi-definitions-for-intel-tdx.html

Re: [PATCH v1 1/8] x86/virt/tdx: Stop treating tdx_global_metadata.h as auto-generated

[Chao Gao]

>> Today the header that holds these structures is generated by an
>> out-of-tree script from a JSON file listing all of the TDX module's
>> metadata.  That made it trivial to add a new field, but everything
>> else suffered for it:
>> 
>>   - The header is opaque to anyone who doesn't have the script and
>>     the JSON file handy, and the "Automatically generated" tag tells
>>     reviewers their edits will be clobbered.
>>   - The script ships outside the tree, so reproducing changes
>>     requires fetching it from a mailing list link.
>>   - The structures are short and stable; the script's value over
>>     a hand-edited header is small.
>
>Well, and the big one: after we started doing this, the "ABI Definitions
>for Intel® TDX" was declared to be not an ABI. So, even if we code to
>the JSON, there's no guarantee the JSON will be stable.
>
><sigh>
>
>Honestly that's what matters. The script and all the other fluff is just
>noise.

Thanks for this. So the key point is that auto-generating rests on a
premise: the JSON is the ABI and stays stable. Then regenerating gives
you something you can trust without re-checking. That premise isn't true
now, so we should stop auto-generating.

One thing I'd like to confirm: is the concern that the JSON could be wrong
about what modules actually implement, or just that there's no commitment
to keep the JSON stable? or both?

>
>> Signed-off-by: Chao Gao <chao.gao@intel.com>
>
>So, I'm curious: What made you feel OK to sign-off on this? I'm not
>judging. I'm open-minded on this. All I know is *I* wasn't ready to
>sign-off on this.
>
>I really want to know what your thought process was.

The idea and the implementation looks good to me, and I thought they were
ready for on-list review.  As the person who modified and posted the
series, my reading of submitting-patches.rst is that I need to sign off
_any_ patches I posted.

Re: [PATCH v1 0/8] TDX: Stop auto-generating the global metadata code

[Chao Gao]

On Tue, Aug 04, 2026 at 04:38:44PM -0700, Dave Hansen wrote:
>On 8/4/26 04:29, Chao Gao wrote:
>> This is a long-overdue cleanup of the TDX global metadata, based on work
>> from Dave [1].
>
>I wouldn't say "from Dave". At best, I'd say "AI slop in response to
>Dave's prompt" or "work vibe coded by Dave".
>
>But seriously, all I wanted to do was show my fellow humans that it
>wasn't an insurmountable task.

Ok. Given this, I'll take authorship of the whole series and credit you in
the cover letter, rather than keeping you as the author on patches 1-6. Let
me know if this doesn't work.

>
>> Patches 1-6 keep Dave's authorship -- I only reworded the
>> commit messages and comments, with no significant changes to the code.
>> Note that they are missing Dave's Signed-off-by.
>
>Yes, and that was intentional.

Re: [PATCH v1 2/8] x86/virt/tdx: Name the TDX module global metadata field IDs

[Chao Gao]

On Tue, Aug 04, 2026 at 04:52:26PM -0700, Dave Hansen wrote:
>On 8/4/26 04:29, Chao Gao wrote:
>> From: Dave Hansen <dave@sr71.net>
>> 
>> tdx_global_metadata.c currently identifies each field by a bare 64-bit hex
>> literal, extracted from the JSON file by an out-of-tree script:
>> 
>> 	if (!ret && !(ret = read_sys_metadata_field(0x9100000100000008, &val)))
>> 		sysinfo_tdmr->max_tdmrs = val;
>> 
>> That is unreviewable on its own. Verifying any one line requires the
>> reviewer to cross-reference the JSON file.
>
>I'm looking at "global_metadata.pdf" from this[1]. I see a line with
>"MAX_TDMRS" and "0x9100000100000008". That matches the lines above. That
>seems *FAR* from unreviewable. There's also no JSON in sight.
>
>What am I missing?
>
>Listen, I don't like how this turned out. I'm asking for it to be
>changed. But let's not justify it with things that just aren't true.

Sure. The only reason is to follow kernel convention for ABI constants
(for example, SEAMCALL leaf functions in the same header) and make the
use sites a bit more readable.

>
>> The hex literals in tdx_global_metadata.c are left alone because that file
>> will be replaced by a table-driven reader in the following patches.
>
>This part of the changelog is good.
>
>> No functional change intended.
>
>This is a bit much. :)

Ok. Will remove it.

>
>> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
>> index bdfd0e1e337a..5f567cb6c07a 100644
>> --- a/arch/x86/virt/vmx/tdx/tdx.h
>> +++ b/arch/x86/virt/vmx/tdx/tdx.h
>> @@ -58,6 +58,57 @@
>>   */
>>  #define TDX_VERSION_SHIFT		16
>>  
>> +/*
>> + * Global Scope Metadata field IDs.
>> + *
>> + * See "Global-Scope (TDX Module) Metadata" in the Intel TDX Module ABI
>> + * spec.
>> + *
>> + * A field ID is a 64-bit value that encodes the metadata "Class"
>> + * (which Linux mirrors in 'struct tdx_sys_info' sub-structures),
>> + * the element size, and a per-class field index. Each ID below
>> + * is paired with the C member that holds its value.
>> + */
>
>This is a bit verbose for my taste.

I will reduce this to:

/*
 * Global Scope Metadata field IDs.
 *
 * See "Global-Scope (TDX Module) Metadata" in the Intel TDX Module ABI
 * spec.
 */


>> +/*
>> + * Base IDs for the configurable-CPUID arrays.  The field ID of leaf
>> + * index @i is BASE + i; for the values array, sub-entry @j of index
>> + * @i is BASE + i*2 + j.
>> + */
>> +#define MD_FIELD_ID_CPUID_CONFIG_LEAVES		0x9900000300000400ULL
>> +#define MD_FIELD_ID_CPUID_CONFIG_VALUES		0x9900000300000500ULL
>
>This is complete jibberish without more context.
>

I will drop this comment and instead explain the indexing around the loop
that reads the configurable CPUID leaves and values.

Re: [PATCH v1 1/8] x86/virt/tdx: Stop treating tdx_global_metadata.h as auto-generated

[Dave Hansen]

On 8/5/26 05:06, Chao Gao wrote:
...
> One thing I'd like to confirm: is the concern that the JSON could be wrong
> about what modules actually implement, or just that there's no commitment
> to keep the JSON stable? or both?

Actually, the JSON from what I understand is _quite_ closely tied to the
implementation. It is always "right".

The problem is that the JSON is *too* closely tied. There is no separate
implementation and ABI. The published "ABI" really is just an artifact
of the implementation. It changes as the implementation changes, and
that's not something the kernel can use in practice.

Really "ABI" to TDX means something very different from what it means to
the kernel.

Re: [PATCH v1 2/8] x86/virt/tdx: Name the TDX module global metadata field IDs

[Edgecombe, Rick P]

On Tue, 2026-08-04 at 04:29 -0700, Chao Gao wrote:
> +/*
> + * Global Scope Metadata field IDs.
> + *
> + * See "Global-Scope (TDX Module) Metadata" in the Intel TDX Module ABI
> + * spec.
> + *
> + * A field ID is a 64-bit value that encodes the metadata "Class"
> + * (which Linux mirrors in 'struct tdx_sys_info' sub-structures),
> + * the element size, and a per-class field index. Each ID below
> + * is paired with the C member that holds its value.
> + */

I wonder about having a macro to construct these. See "Table 3.55: MD_FIELD_ID
(Metadata Field Identifier / Sequence Header) Definition" in the TDX ABI spec
for the meaning of the bits set in these defines.

But I'd also think it would nice to have an easy way to cross reference the
fields between kernel and TDX docs. We could probably consider adjustments to
the docs as part of the solution for that. Of if the macro makes that cross
reference harder, then not do it.

Re: [PATCH v1 0/8] TDX: Stop auto-generating the global metadata code

[Edgecombe, Rick P]

On Tue, 2026-08-04 at 04:29 -0700, Chao Gao wrote:
> The TDX module exposes its capabilities and limits through "Global Scope
> Metadata" fields, defined in the Intel TDX Module ABI spec.  The kernel
> mirrors a small subset of those fields in C structures under struct
> tdx_sys_info, populated by reading each field via TDH.SYS.RD.
> 
> Both the structures and the code that fills them are nominally generated
> by an out-of-tree script from a JSON file describing the module's
> metadata.  That made it trivial to add a new field, but everything else
> suffered for it:
> 
>   - The generated files are edited by hand in practice, for different
>     reasons.  The VMXON rework added __init annotations to the readers,
>     and the handoff metadata is read at module shutdown into a
>     caller-local struct rather than into tdx_sys_info.  In-flight series
>     will add more: DPAMT and TDX module extension metadata may only be
>     read when the corresponding TDX_FEATURES0 bit is set.  Regenerating
>     the files would clobber all of these edits.
> 
>   - The generated code is opaque to anyone who doesn't have the script
>     and the JSON file handy.  Each field is identified by a bare 64-bit
>     hex literal, so verifying any one line means cross-referencing the
>     JSON file.
> 
>   - The script ships outside the tree, so reproducing changes requires
>     fetching it from a mailing list link.
> 
>   - The structures are short and stable, so the script's value over
>     hand-maintained code is small.
> 
> So switch to a hand-maintained implementation.  Name each field ID after
> the spec, then describe the field-ID-to-C-member mapping as a table: one
> row per field, pairing the named spec field ID with the C member that
> holds it.  Reading is then a walk over the table.

It might help to fill out this problem statement a bit more. The script is
already dead because of problems. But it came about due to other problems. And
we still have problems without the script.

Also, there were previous attempts at a macro based solution that failed to make
it upstream. It would be good to highlight how it avoids those problems.

Re: [PATCH v1 3/8] x86/virt/tdx: Add a table-driven TDX global metadata reader

[Edgecombe, Rick P]

On Tue, 2026-08-04 at 04:29 -0700, Chao Gao wrote:
> +/*
> + * Mapping between a TDX global metadata field and the C member that
> + * holds its value. Use TD_SYSINFO_MAP() to populate entries.
> + */
> +struct tdx_sys_field {
> +	u64 field_id;
> +	u16 offset;
> +	u8  size;

We are supposed to be able to extract the size from the field id too. We could
leave the bits out of the field id and have it constructed on the fly, or we
could do a static assert based on the size matching between field id size and
struct size. It depends on how we want to have the field ids defines I guess.