[RFC PATCH v2 00/10] TDX: Stop auto-generating the global metadata code

[RFC PATCH v2 00/10] TDX: Stop auto-generating the global metadata code

[Chao Gao]

This series cleans up the TDX global metadata code. It has two goals:

1. Replace the generated code with a table-driven metadata reader.

2. Make the existing code easier to read and maintain, and simplify
   adding new metadata fields.

During the v1 review, Dave raised concerns about signing off on
AI-generated code. I have since rewritten the affected patches based on
my own understanding of the code.

The main goal of this RFC is to agree on whether the table-driven reader is
the right replacement for the generated code. Please raise any concerns or
alternative design ideas.

Feedback on the patch organization is also welcome. In v2, each patch
converts one metadata reader. This should make the individual changes
easier to review, but results in more patches and some repetition in their
changelogs. I am not sure whether this is the best organization for the
series.

Dave, please feel free to ignore this RFC. Kirill, Rick, and other TDX
developers, please take a look.

Changes since v1:
=================
  - Reimplemented the code and rewrote the changelogs for patches
    generated entirely by AI. (Dave)
  - Expanded the problem statement with the history of earlier TDX metadata
    proposals (Rick)
  - Dropped the false claim that literal u64 field IDs are unreviewable
    (Dave)
  - Rebase onto tip/x86/tdx and convert the metadata added by DPAMT.
  - Name field IDs as their readers are converted instead of naming them
    in a separate patch.
  - Convert one metadata reader per patch.
  - v1: https://lore.kernel.org/all/20260804112941.19894-1-chao.gao@intel.com/

The TDX module reports its capabilities and limits through a set of global
metadata fields, each read using a 64-bit field ID via TDH.SYS.RD. The
fields are grouped into classes, and the kernel mirrors each class it
needs in a sub-structure of struct tdx_sys_info.

Both those structures and the code that fills them were generated by an
out-of-tree script from a JSON file.

The script was not the first approach.  Kai's first attempt paired each
field ID with its destination C member in a table and walked the table in a
loop to read every field.  Two pieces of feedback on it drove everything
that followed [1]:

  1. Compile-time type checking. Metadata fields have different sizes (u16
     and u64), so a common helper takes a void * and a size instead of
     a typed destination.

  2. The check that a field ID's encoded size matches its destination
     member ran at runtime, although both sizes are known at build time.

The discussion did not converge after several rounds of review. Dave noted
that, despite the void *, the size check provides the safety that matters:
it catches mismatched field and member widths [2]. That left one problem:
moving the size check from runtime to build time.

Before that was settled, the direction shifted to generating the code with a
script. At the time, the TDX ABI definitions were published as JSON, so
checking a field ID required consulting a machine-readable file by hand.
The script parsed the JSON and generated both the structures and their
readers [3]. Generation also made the size/type check unnecessary. The
field IDs and destination members came from the same input, so a mismatch
could only result from a bug in the script, which is less likely than a
mistake in hand-written code.

Dave concluded that the JSON experiment had failed [4] for two reasons:

  1. The JSON file is not stable. The CPUID config arrays here were once
     sized for a maximum of 32 entries, which has since increased to 128 [5].

  2. The JSON file is not authoritative enough to write code from by itself.

TDX ABI definitions are now available in human-readable PDF specifications
[6]. So, stop relying on the out-of-tree script and maintain the code by
hand.

Yilun later proposed a single table whose entries carry the offset and size
of each mapped member in struct tdx_sys_info [7]. That design does not
cover the new handoff metadata because it is not cached in
struct tdx_sys_info.

This series gives every class its own table: each entry pairs a field ID
with the member that holds it, and a loop walks the table. This also covers
handoff metadata, which is read into a local structure rather than into
struct tdx_sys_info.

The common reader still takes a void *, but each mapping verifies at build
time that the destination member size matches the size encoded in the field
ID. A field/member width mismatch therefore fails the build.

AI usage
========

I used LLM tools to review the patches and refine the wording of the cover
letter and changelogs from my drafts. I reviewed all suggestions and adopted
those I agreed with. For example, AI review suggested the
read_sys_metadata_table() macro, which avoids repeating the table name when
passing both the table and its size.


Testing
=======
Built each patch individually and successfully launched TDs.


[1]: https://lore.kernel.org/kvm/66b16121c48f4_4fc729424@dwillia2-xfh.jf.intel.com.notmuch/
[2]: https://lore.kernel.org/kvm/c3b1e743-6d34-49ce-8e60-a41038f27c61@intel.com/
[3]: https://lore.kernel.org/kvm/f25673ea-08c5-474b-a841-095656820b67@intel.com/
[4]: https://lore.kernel.org/kvm/1e7bcbad-eb26-44b7-97ca-88ab53467212@intel.com/
[5]: https://lore.kernel.org/lkml/55f97ca1-8f32-4e33-96fb-d82ed2109f9a@intel.com/
[6]: https://www.intel.com/content/www/us/en/content-details/865803/abi-definitions-for-intel-tdx.html
[7]: https://lore.kernel.org/kvm/20251202050844.2520762-4-yilun.xu@linux.intel.com/



Chao Gao (10):
  x86/virt/tdx: Add a helper to read a table of metadata fields
  x86/virt/tdx: Convert the version metadata reader
  x86/virt/tdx: Convert the features metadata reader
  x86/virt/tdx: Convert the tdmr metadata reader
  x86/virt/tdx: Convert the td_ctrl metadata reader
  x86/virt/tdx: Convert the handoff metadata reader
  x86/virt/tdx: Convert the td_conf metadata reader
  x86/virt/tdx: Remove tdx_global_metadata.c
  x86/virt/tdx: Use early returns in get_tdx_sys_info()
  x86/virt/tdx: Verify structure member sizes against metadata field IDs

 arch/x86/virt/vmx/tdx/tdx.c                 | 222 +++++++++++++++++++-
 arch/x86/virt/vmx/tdx/tdx.h                 |  54 +++++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 154 --------------
 3 files changed, 275 insertions(+), 155 deletions(-)
 delete mode 100644 arch/x86/virt/vmx/tdx/tdx_global_metadata.c

-- 
2.52.0

[RFC PATCH v2 01/10] x86/virt/tdx: Add a helper to read a table of metadata fields

[Chao Gao]

The metadata field readers get_tdx_sys_info_<class>() in
tdx_global_metadata.c were generated by an out-of-tree script. That has
not worked out: the JSON file they were generated from is neither stable
nor authoritative enough [1].  The goal now is to maintain the readers by
hand and to establish one standard way of adding a metadata field.

Take get_tdx_sys_info_version() as an example:

      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;

Two patterns stand out: the read-check-store sequence repeats once per
field, and the error of each read is chained into the reads that follow.
Neither is common in hand-written code.

Eliminate both with a loop that reads each field, stores the value into
its structure member, and returns on the first error.

Add 'struct field_mapping' to describe one field as its ID plus the
offset and size of the member that receives its value.  Add
TDX_SYSINFO_MAP() to build such an entry from a field ID name, a
structure type and a member name.  Add __read_sys_metadata_table() to
read every field in a table. Annotate the helper __maybe_unused as there
is no caller right now.

Following changes will convert the existing readers to use the new helper.

AI was used under supervision to review code and workshop logs.

Signed-off-by: Chao Gao <chao.gao@intel.com>
Link: https://lore.kernel.org/kvm/1e7bcbad-eb26-44b7-97ca-88ab53467212@intel.com/ # [1]
---
 arch/x86/virt/vmx/tdx/tdx.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 063574ed8625..5cd9c6eb98c2 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -395,6 +395,42 @@ static int read_sys_metadata_field(u64 field_id, u64 *data)
 	return 0;
 }
 
+/*
+ * Map a TDX global metadata field to a structure member.
+ * @field_id: The TDX global metadata field ID.
+ * @size: The size of the structure member.
+ * @offset: The member's offset within its containing structure.
+ */
+struct field_mapping {
+	u64 field_id;
+	size_t size;
+	int offset;
+};
+
+/* Read each metadata field listed in @mappings[] into @data. */
+static int __maybe_unused __read_sys_metadata_table(const struct field_mapping *mappings,
+						    int num_mappings, void *data)
+{
+	int i, ret;
+	u64 val;
+
+	for (i = 0; i < num_mappings; i++) {
+		ret = read_sys_metadata_field(mappings[i].field_id, &val);
+		if (ret)
+			return ret;
+		memcpy((char *)data + mappings[i].offset, &val, mappings[i].size);
+	}
+
+	return 0;
+}
+
+#define TDX_SYSINFO_MAP(_field, _type, _member)			\
+{								\
+	.field_id	= TDX_MD_FIELD_ID_##_field,		\
+	.offset		= offsetof(_type, _member),		\
+	.size		= sizeof_field(_type, _member),		\
+}
+
 #include "tdx_global_metadata.c"
 
 static __init int check_features(struct tdx_sys_info *sysinfo)
-- 
2.52.0

[RFC PATCH v2 02/10] x86/virt/tdx: Convert the version metadata reader

[Chao Gao]

With the helper to read a table of metadata fields in place, the
existing metadata readers can be standardized on it.

Convert the version metadata reader: add a table that pairs each field ID
with the 'struct tdx_sys_info_version' member that holds its value, and
read all version fields by walking that table.

Name the field IDs for readability, so the table entries don't carry raw
hex literals.

Also add read_sys_metadata_table() macro to derive the number of entries
with ARRAY_SIZE() so that callers don't need to pass it separately.

AI was used under supervision to review code and workshop logs. It suggests
adding read_sys_metadata_table() macro.

Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 arch/x86/virt/vmx/tdx/tdx.c                 | 21 +++++++++++++++++++--
 arch/x86/virt/vmx/tdx/tdx.h                 | 10 ++++++++++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 15 ---------------
 3 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 5cd9c6eb98c2..0c8b571f5186 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -408,8 +408,8 @@ struct field_mapping {
 };
 
 /* Read each metadata field listed in @mappings[] into @data. */
-static int __maybe_unused __read_sys_metadata_table(const struct field_mapping *mappings,
-						    int num_mappings, void *data)
+static int __read_sys_metadata_table(const struct field_mapping *mappings,
+				     int num_mappings, void *data)
 {
 	int i, ret;
 	u64 val;
@@ -431,6 +431,23 @@ static int __maybe_unused __read_sys_metadata_table(const struct field_mapping *
 	.size		= sizeof_field(_type, _member),		\
 }
 
+#define TDX_SYSINFO_MAP_VERSION(_field_id, _member) \
+	TDX_SYSINFO_MAP(_field_id, struct tdx_sys_info_version, _member)
+
+static const struct field_mapping version_mappings[] = {
+	TDX_SYSINFO_MAP_VERSION(MINOR_VERSION,  minor_version),
+	TDX_SYSINFO_MAP_VERSION(MAJOR_VERSION,  major_version),
+	TDX_SYSINFO_MAP_VERSION(UPDATE_VERSION, update_version),
+};
+
+#define read_sys_metadata_table(_table, _data) \
+	__read_sys_metadata_table(_table, ARRAY_SIZE(_table), _data)
+
+static int get_tdx_sys_info_version(struct tdx_sys_info_version *version)
+{
+	return read_sys_metadata_table(version_mappings, version);
+}
+
 #include "tdx_global_metadata.c"
 
 static __init int check_features(struct tdx_sys_info *sysinfo)
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index a886c54decaa..44124f399703 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -60,6 +60,16 @@
  */
 #define TDX_VERSION_SHIFT		16
 
+/*
+ * TDX global metadata field IDs.
+ *
+ * See "global_metadata.pdf" in Intel TDX Module ABI Definitions.
+ */
+/* Class "TDX Module Version" */
+#define TDX_MD_FIELD_ID_MINOR_VERSION			0x0800000100000003ULL
+#define TDX_MD_FIELD_ID_MAJOR_VERSION			0x0800000100000004ULL
+#define TDX_MD_FIELD_ID_UPDATE_VERSION			0x0800000100000005ULL
+
 /* TDX page types */
 #define	PT_NDA		0x0
 #define	PT_RSVD		0x1
diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
index 98ebf17aab1c..9510bf5f6e7d 100644
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
@@ -7,21 +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_features(struct tdx_sys_info_features *sysinfo_features)
 {
 	int ret = 0;
-- 
2.52.0

[RFC PATCH v2 03/10] x86/virt/tdx: Convert the features metadata reader

[Chao Gao]

Continue converting the metadata readers to the table-driven helper.

Add a table that pairs each field ID with the 'struct
tdx_sys_info_features' member that holds its value, and read all fields by
walking that table.

Even though the structure has only one field, add a table anyway for
symmetry with the other classes. Adding a field later then becomes a
one-line change.

Annotate the table as __initconst as it is referenced only during init.

AI was used under supervision to review code and workshop logs.

Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 arch/x86/virt/vmx/tdx/tdx.c                 | 12 ++++++++++++
 arch/x86/virt/vmx/tdx/tdx.h                 |  3 +++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 11 -----------
 3 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 0c8b571f5186..59e2396fea39 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -448,6 +448,18 @@ static int get_tdx_sys_info_version(struct tdx_sys_info_version *version)
 	return read_sys_metadata_table(version_mappings, version);
 }
 
+#define TDX_SYSINFO_MAP_FEATURES(_field_id, _member) \
+	TDX_SYSINFO_MAP(_field_id, struct tdx_sys_info_features, _member)
+
+static const struct field_mapping feature_mappings[] __initconst = {
+	TDX_SYSINFO_MAP_FEATURES(TDX_FEATURES0, tdx_features0),
+};
+
+static __init int get_tdx_sys_info_features(struct tdx_sys_info_features *features)
+{
+	return read_sys_metadata_table(feature_mappings, 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.h b/arch/x86/virt/vmx/tdx/tdx.h
index 44124f399703..582b6ff9a0bb 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -70,6 +70,9 @@
 #define TDX_MD_FIELD_ID_MAJOR_VERSION			0x0800000100000004ULL
 #define TDX_MD_FIELD_ID_UPDATE_VERSION			0x0800000100000005ULL
 
+/* Class "TDX Module Info" */
+#define TDX_MD_FIELD_ID_TDX_FEATURES0			0x0A00000300000008ULL
+
 /* TDX page types */
 #define	PT_NDA		0x0
 #define	PT_RSVD		0x1
diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
index 9510bf5f6e7d..615f4d23c5d8 100644
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
@@ -7,17 +7,6 @@
  * Include this file to other C file instead.
  */
 
-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_dpamt(struct tdx_sys_info_tdmr *sysinfo_tdmr)
 {
 	int ret;
-- 
2.52.0

[RFC PATCH v2 04/10] x86/virt/tdx: Convert the tdmr metadata reader

[Chao Gao]

Continue converting the metadata readers to the table-driven helper.

The "TDMR info" metadata class has two readers: one for the fields that are
always present, and one for the fields that exist only when the module
supports Dynamic PAMT.

Add a table for each, both pairing field IDs with the 'struct
tdx_sys_info_tdmr' members that hold their values, and read all fields by
walking the tables.

Annotate both tables as __initconst since they are referenced only during
init.

AI was used under supervision to review the code and workshop the
changelog.

Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 arch/x86/virt/vmx/tdx/tdx.c                 | 25 +++++++++++++++++
 arch/x86/virt/vmx/tdx/tdx.h                 |  8 ++++++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 31 ---------------------
 3 files changed, 33 insertions(+), 31 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 59e2396fea39..72aed3fc2b25 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -460,6 +460,31 @@ static __init int get_tdx_sys_info_features(struct tdx_sys_info_features *featur
 	return read_sys_metadata_table(feature_mappings, features);
 }
 
+#define TDX_SYSINFO_MAP_TDMR(_field_id, _member) \
+	TDX_SYSINFO_MAP(_field_id, struct tdx_sys_info_tdmr, _member)
+
+static const struct field_mapping tdmr_mappings[] __initconst = {
+	TDX_SYSINFO_MAP_TDMR(MAX_TDMRS,			max_tdmrs),
+	TDX_SYSINFO_MAP_TDMR(MAX_RESERVED_PER_TDMR,	max_reserved_per_tdmr),
+	TDX_SYSINFO_MAP_TDMR(PAMT_4K_ENTRY_SIZE,	pamt_4k_entry_size),
+	TDX_SYSINFO_MAP_TDMR(PAMT_2M_ENTRY_SIZE,	pamt_2m_entry_size),
+	TDX_SYSINFO_MAP_TDMR(PAMT_1G_ENTRY_SIZE,	pamt_1g_entry_size),
+};
+
+static __init int get_tdx_sys_info_tdmr(struct tdx_sys_info_tdmr *tdmr)
+{
+	return read_sys_metadata_table(tdmr_mappings, tdmr);
+}
+
+static const struct field_mapping dpamt_mappings[] __initconst = {
+	TDX_SYSINFO_MAP_TDMR(PAMT_PAGE_BITMAP_ENTRY_BITS, pamt_page_bitmap_entry_bits),
+};
+
+static __init int get_tdx_sys_info_tdmr_dpamt(struct tdx_sys_info_tdmr *tdmr)
+{
+	return read_sys_metadata_table(dpamt_mappings, tdmr);
+}
+
 #include "tdx_global_metadata.c"
 
 static __init int check_features(struct tdx_sys_info *sysinfo)
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index 582b6ff9a0bb..9a8c32b180e1 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -73,6 +73,14 @@
 /* Class "TDX Module Info" */
 #define TDX_MD_FIELD_ID_TDX_FEATURES0			0x0A00000300000008ULL
 
+/* Class "TDMR Info" */
+#define TDX_MD_FIELD_ID_MAX_TDMRS			0x9100000100000008ULL
+#define TDX_MD_FIELD_ID_MAX_RESERVED_PER_TDMR		0x9100000100000009ULL
+#define TDX_MD_FIELD_ID_PAMT_4K_ENTRY_SIZE		0x9100000100000010ULL
+#define TDX_MD_FIELD_ID_PAMT_2M_ENTRY_SIZE		0x9100000100000011ULL
+#define TDX_MD_FIELD_ID_PAMT_1G_ENTRY_SIZE		0x9100000100000012ULL
+#define TDX_MD_FIELD_ID_PAMT_PAGE_BITMAP_ENTRY_BITS	0x9100000000000013ULL
+
 /* TDX page types */
 #define	PT_NDA		0x0
 #define	PT_RSVD		0x1
diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
index 615f4d23c5d8..f0673dc28cb9 100644
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
@@ -7,37 +7,6 @@
  * Include this file to other C file instead.
  */
 
-static __init int get_tdx_sys_info_tdmr_dpamt(struct tdx_sys_info_tdmr *sysinfo_tdmr)
-{
-	int ret;
-	u64 val;
-
-	ret = read_sys_metadata_field(0x9100000000000013, &val);
-	if (!ret)
-		sysinfo_tdmr->pamt_page_bitmap_entry_bits = 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;
-- 
2.52.0

[RFC PATCH v2 05/10] x86/virt/tdx: Convert the td_ctrl metadata reader

[Chao Gao]

Continue converting the metadata readers to the table-driven helper.

Add a table that pairs each field ID with the 'struct
tdx_sys_info_td_ctrl' member that holds its value, and read all fields by
walking that table.

Annotate the table as __initconst as it is referenced only during init.

AI was used under supervision to review code and workshop logs

Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 arch/x86/virt/vmx/tdx/tdx.c                 | 14 ++++++++++++++
 arch/x86/virt/vmx/tdx/tdx.h                 |  5 +++++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 15 ---------------
 3 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 72aed3fc2b25..7b6b1ee075a3 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -485,6 +485,20 @@ static __init int get_tdx_sys_info_tdmr_dpamt(struct tdx_sys_info_tdmr *tdmr)
 	return read_sys_metadata_table(dpamt_mappings, tdmr);
 }
 
+#define TDX_SYSINFO_MAP_TD_CTRL(_field_id, _member) \
+	TDX_SYSINFO_MAP(_field_id, struct tdx_sys_info_td_ctrl, _member)
+
+static const struct field_mapping td_ctrl_mappings[] __initconst = {
+	TDX_SYSINFO_MAP_TD_CTRL(TDR_BASE_SIZE,		tdr_base_size),
+	TDX_SYSINFO_MAP_TD_CTRL(TDCS_BASE_SIZE,		tdcs_base_size),
+	TDX_SYSINFO_MAP_TD_CTRL(TDVPS_BASE_SIZE,	tdvps_base_size),
+};
+
+static __init int get_tdx_sys_info_td_ctrl(struct tdx_sys_info_td_ctrl *td_ctrl)
+{
+	return read_sys_metadata_table(td_ctrl_mappings, td_ctrl);
+}
+
 #include "tdx_global_metadata.c"
 
 static __init int check_features(struct tdx_sys_info *sysinfo)
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index 9a8c32b180e1..f33eb7dc7b45 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -81,6 +81,11 @@
 #define TDX_MD_FIELD_ID_PAMT_1G_ENTRY_SIZE		0x9100000100000012ULL
 #define TDX_MD_FIELD_ID_PAMT_PAGE_BITMAP_ENTRY_BITS	0x9100000000000013ULL
 
+/* Class "TD Control Structures" */
+#define TDX_MD_FIELD_ID_TDR_BASE_SIZE			0x9800000100000000ULL
+#define TDX_MD_FIELD_ID_TDCS_BASE_SIZE			0x9800000100000100ULL
+#define TDX_MD_FIELD_ID_TDVPS_BASE_SIZE			0x9800000100000200ULL
+
 /* TDX page types */
 #define	PT_NDA		0x0
 #define	PT_RSVD		0x1
diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
index f0673dc28cb9..c6b375771265 100644
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
@@ -7,21 +7,6 @@
  * Include this file to other C file instead.
  */
 
-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;
-- 
2.52.0

[RFC PATCH v2 06/10] x86/virt/tdx: Convert the handoff metadata reader

[Chao Gao]

Continue converting the metadata readers to the table-driven helper.

Add a table that pairs each field ID with the 'struct
tdx_sys_info_handoff' member that holds its value, and read all fields by
walking that table.

AI was used under supervision to review code and workshop logs

Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 arch/x86/virt/vmx/tdx/tdx.c                 | 12 ++++++++++++
 arch/x86/virt/vmx/tdx/tdx.h                 |  3 +++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 13 -------------
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 7b6b1ee075a3..2bd7a2c9432b 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -499,6 +499,18 @@ static __init int get_tdx_sys_info_td_ctrl(struct tdx_sys_info_td_ctrl *td_ctrl)
 	return read_sys_metadata_table(td_ctrl_mappings, td_ctrl);
 }
 
+#define TDX_SYSINFO_MAP_HANDOFF(_field_id, _member) \
+	TDX_SYSINFO_MAP(_field_id, struct tdx_sys_info_handoff, _member)
+
+static const struct field_mapping handoff_mappings[] = {
+	TDX_SYSINFO_MAP_HANDOFF(MODULE_HV, module_hv),
+};
+
+static int get_tdx_sys_info_handoff(struct tdx_sys_info_handoff *handoff)
+{
+	return read_sys_metadata_table(handoff_mappings, 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.h b/arch/x86/virt/vmx/tdx/tdx.h
index f33eb7dc7b45..6f99e4cdcc19 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -86,6 +86,9 @@
 #define TDX_MD_FIELD_ID_TDCS_BASE_SIZE			0x9800000100000100ULL
 #define TDX_MD_FIELD_ID_TDVPS_BASE_SIZE			0x9800000100000200ULL
 
+/* Class "TDX Module Handoff" */
+#define TDX_MD_FIELD_ID_MODULE_HV			0x8900000100000000ULL
+
 /* TDX page types */
 #define	PT_NDA		0x0
 #define	PT_RSVD		0x1
diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
index c6b375771265..36bbf42dead9 100644
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
@@ -40,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

[RFC PATCH v2 07/10] x86/virt/tdx: Convert the td_conf metadata reader

[Chao Gao]

Continue converting the metadata readers to the table-driven helper.

The "TD Configurability" class holds several scalar fields plus two CPUID
arrays. The arrays have a fixed capacity, but the number of entries to
read is variable and reported by the num_cpuid_config scalar field.

Add a table that pairs each scalar field ID with the 'struct
tdx_sys_info_td_conf' member that holds its value, and read those fields
by walking that table.

Annotate the table as __initconst as it is referenced only during init.

Leave the two arrays open coded. 'struct field_mapping' pairs one field ID
with one structure member, so describing an array would require a field ID
per element plus an entry count that is unknown until num_cpuid_config has
been read. That is not worth building for the only two arrays the kernel
reads.

Read the arrays with explicit loops as the generated code did, but store
each value directly into its array member instead of into a temporary u64
first. The members are u64 already, so the extra copies serve no purpose.

AI was used under supervision to review code and workshop logs.

Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 arch/x86/virt/vmx/tdx/tdx.c                 | 51 +++++++++++++++++++++
 arch/x86/virt/vmx/tdx/tdx.h                 | 10 ++++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 33 -------------
 3 files changed, 61 insertions(+), 33 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 2bd7a2c9432b..571c3bb50532 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -511,6 +511,57 @@ static int get_tdx_sys_info_handoff(struct tdx_sys_info_handoff *handoff)
 	return read_sys_metadata_table(handoff_mappings, handoff);
 }
 
+#define TDX_SYSINFO_MAP_TD_CONF(_field_id, _member) \
+	TDX_SYSINFO_MAP(_field_id, struct tdx_sys_info_td_conf, _member)
+
+static const struct field_mapping td_conf_mappings[] __initconst = {
+	TDX_SYSINFO_MAP_TD_CONF(ATTRIBUTES_FIXED0,	attributes_fixed0),
+	TDX_SYSINFO_MAP_TD_CONF(ATTRIBUTES_FIXED1,	attributes_fixed1),
+	TDX_SYSINFO_MAP_TD_CONF(XFAM_FIXED0,		xfam_fixed0),
+	TDX_SYSINFO_MAP_TD_CONF(XFAM_FIXED1,		xfam_fixed1),
+	TDX_SYSINFO_MAP_TD_CONF(NUM_CPUID_CONFIG,	num_cpuid_config),
+	TDX_SYSINFO_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_mappings, td_conf);
+	if (ret)
+		return ret;
+
+	/*
+	 * The number of CPUID config entries must not exceed the array
+	 * sizes.
+	 */
+	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;
+
+	/*
+	 * TDX_MD_FIELD_ID_CPUID_CONFIG_* give the field ID of each array's
+	 * first element. The remaining elements follow consecutively, in
+	 * the order they appear in the structure.
+	 */
+	for (i = 0; i < td_conf->num_cpuid_config; i++) {
+		ret = read_sys_metadata_field(TDX_MD_FIELD_ID_CPUID_CONFIG_LEAVES + i,
+					      &td_conf->cpuid_config_leaves[i]);
+		if (ret)
+			return ret;
+
+		for (j = 0; j < 2; j++) {
+			ret = read_sys_metadata_field(
+				TDX_MD_FIELD_ID_CPUID_CONFIG_VALUES + i * 2 + j,
+				&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.h b/arch/x86/virt/vmx/tdx/tdx.h
index 6f99e4cdcc19..407aded3137a 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -86,6 +86,16 @@
 #define TDX_MD_FIELD_ID_TDCS_BASE_SIZE			0x9800000100000100ULL
 #define TDX_MD_FIELD_ID_TDVPS_BASE_SIZE			0x9800000100000200ULL
 
+/* Class "TD Configurability" */
+#define TDX_MD_FIELD_ID_ATTRIBUTES_FIXED0		0x1900000300000000ULL
+#define TDX_MD_FIELD_ID_ATTRIBUTES_FIXED1		0x1900000300000001ULL
+#define TDX_MD_FIELD_ID_XFAM_FIXED0			0x1900000300000002ULL
+#define TDX_MD_FIELD_ID_XFAM_FIXED1			0x1900000300000003ULL
+#define TDX_MD_FIELD_ID_NUM_CPUID_CONFIG		0x9900000100000004ULL
+#define TDX_MD_FIELD_ID_MAX_VCPUS_PER_TD		0x9900000100000008ULL
+#define TDX_MD_FIELD_ID_CPUID_CONFIG_LEAVES		0x9900000300000400ULL
+#define TDX_MD_FIELD_ID_CPUID_CONFIG_VALUES		0x9900000300000500ULL
+
 /* Class "TDX Module Handoff" */
 #define TDX_MD_FIELD_ID_MODULE_HV			0x8900000100000000ULL
 
diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
index 36bbf42dead9..779c26e68f02 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

[RFC PATCH v2 08/10] x86/virt/tdx: Remove tdx_global_metadata.c

[Chao Gao]

tdx_global_metadata.c held the script-generated metadata readers,
deliberately kept in their own file so that generated code stayed separate
from hand-written code. It cannot be compiled on its own because it lacks
the low-level SEAMCALL wrappers, so tdx.c #includes it directly.

Including one C file into another is unusual, and now that the readers are
maintained by hand there is nothing left to isolate. get_tdx_sys_info() is
the only function still in the file.

Move get_tdx_sys_info() verbatim into tdx.c and delete
tdx_global_metadata.c along with its #include.

AI was used under supervision to review code and workshop logs.

Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 arch/x86/virt/vmx/tdx/tdx.c                 | 28 +++++++++++++++-
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 36 ---------------------
 2 files changed, 27 insertions(+), 37 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 571c3bb50532..9f9f2ef80f55 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -562,7 +562,33 @@ 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);
+
+	/*
+	 * The kernel supports using TDX without DPAMT, so
+	 * avoid reporting failure if it's not supported. Don't
+	 * try to support buggy TDX modules that advertise
+	 * DPAMT but don't expose the metadata.
+	 */
+	if (!ret && tdx_supports_dynamic_pamt(sysinfo))
+		ret = get_tdx_sys_info_tdmr_dpamt(&sysinfo->tdmr);
+
+	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 779c26e68f02..000000000000
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ /dev/null
@@ -1,36 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * 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);
-
-	/*
-	 * The kernel supports using TDX without DPAMT, so
-	 * avoid reporting failure if it's not supported. Don't
-	 * try to support buggy TDX modules that advertise
-	 * DPAMT but don't expose the metadata.
-	 */
-	if (!ret && tdx_supports_dynamic_pamt(sysinfo))
-		ret = get_tdx_sys_info_tdmr_dpamt(&sysinfo->tdmr);
-
-	return ret;
-}
-- 
2.52.0

[RFC PATCH v2 09/10] x86/virt/tdx: Use early returns in get_tdx_sys_info()

[Chao Gao]

get_tdx_sys_info() was generated by a script. It chains its metadata reads
with:

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

The function is maintained by hand now. Use conventional early returns
instead.

AI was used under supervision to review code and workshop logs.

Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 arch/x86/virt/vmx/tdx/tdx.c | 33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 9f9f2ef80f55..9d5a3296d3c0 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -564,19 +564,33 @@ 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;
+
+	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;
+
+	ret = get_tdx_sys_info_td_conf(&sysinfo->td_conf);
+	if (ret)
+		return ret;
 
 	/*
 	 * The kernel supports using TDX without DPAMT, so
@@ -584,10 +598,13 @@ static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
 	 * try to support buggy TDX modules that advertise
 	 * DPAMT but don't expose the metadata.
 	 */
-	if (!ret && tdx_supports_dynamic_pamt(sysinfo))
+	if (tdx_supports_dynamic_pamt(sysinfo)) {
 		ret = get_tdx_sys_info_tdmr_dpamt(&sysinfo->tdmr);
+		if (ret)
+			return ret;
+	}
 
-	return ret;
+	return 0;
 }
 
 static __init int check_features(struct tdx_sys_info *sysinfo)
-- 
2.52.0

[RFC PATCH v2 10/10] x86/virt/tdx: Verify structure member sizes against metadata field IDs

[Chao Gao]

A metadata field ID encodes the size of a single element. TDX_SYSINFO_MAP()
instead derives the copy size from the destination member, and nothing
verifies that the two sizes agree.

A wrongly typed member is a kernel bug: declaring a u32 for an 8-byte
metadata field would silently store only its low 4 bytes.

Add macros to extract the element size encoded in a field ID and verify it
against the destination member size at build time.

BUILD_BUG_ON() cannot be used in a structure initializer, so use
BUILD_BUG_ON_ZERO() and add its zero result to the .size initializer. This
performs the build-time check without changing the stored size.

AI was used under supervision to review code and workshop logs. It
suggested extracting TDX_MD_FIELD_SIZE_CHECK() instead of open coding the
check in TDX_SYSINFO_MAP(), to keep the .size line from being too long.

Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 arch/x86/virt/vmx/tdx/tdx.c | 12 +++++++++++-
 arch/x86/virt/vmx/tdx/tdx.h | 15 +++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 9d5a3296d3c0..6dc328561009 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -424,11 +424,21 @@ static int __read_sys_metadata_table(const struct field_mapping *mappings,
 	return 0;
 }
 
+/*
+ * The size encoded in the field ID and the size of the destination C
+ * member must agree.
+ */
+#define TDX_MD_FIELD_SIZE_CHECK(_field, _type, _member)	\
+	BUILD_BUG_ON_ZERO(sizeof_field(_type, _member) !=	\
+			  TDX_MD_FIELD_ELE_SIZE(TDX_MD_FIELD_ID_##_field))
+
 #define TDX_SYSINFO_MAP(_field, _type, _member)			\
 {								\
 	.field_id	= TDX_MD_FIELD_ID_##_field,		\
 	.offset		= offsetof(_type, _member),		\
-	.size		= sizeof_field(_type, _member),		\
+	.size		= sizeof_field(_type, _member) +	\
+			  TDX_MD_FIELD_SIZE_CHECK(		\
+				_field, _type, _member),	\
 }
 
 #define TDX_SYSINFO_MAP_VERSION(_field_id, _member) \
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index 407aded3137a..17fdb682410e 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -99,6 +99,21 @@
 /* Class "TDX Module Handoff" */
 #define TDX_MD_FIELD_ID_MODULE_HV			0x8900000100000000ULL
 
+/*
+ * Sub-field definitions of TDX global metadata field IDs.
+ *
+ * See "Metadata Field Identifier" in the Intel TDX Module ABI
+ * Specification.
+ *
+ *  - Bit 33:32: ELEMENT_SIZE_CODE -- log2 of a single metadata
+ *                                    element's size in bytes
+ */
+#define TDX_MD_FIELD_ELE_SIZE_CODE(field_id)	\
+	(((field_id) & GENMASK_ULL(33, 32)) >> 32)
+
+#define TDX_MD_FIELD_ELE_SIZE(field_id)	\
+	(1 << TDX_MD_FIELD_ELE_SIZE_CODE(field_id))
+
 /* TDX page types */
 #define	PT_NDA		0x0
 #define	PT_RSVD		0x1
-- 
2.52.0