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

[PATCH v3 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.

I dropped the RFC tag from this posting, as nobody seemed to disagree
with the approach in v2. Tony's suggestions, splitting the version
fields out so that tdx_sys_info can be __ro_after_init and not caching
init-only metadata, are left for a separate series.

Dave, please feel free to ignore this for now. Kirill, Rick, and other TDX
developers, please take a look. Reviews and tags are welcome.

Changes
=======

The biggest change is that the get_tdx_sys_info_foo() wrappers are
gone. Rick questioned whether a single-use wrapper around one
read_sys_metadata_table() call earns its keep, and Dave agreed it
is an unnecessary layer of abstraction.

Other changes:
  - Pass the field ID to TDX_SYSINFO_MAP() instead of pasting the
    TDX_MD_FIELD_ID_ prefix onto a suffix, and rename the
    defines to TDX_FIELD_*, so the names can be grepped (Rick)
  - Patch 8: Pick up Rick's Reviewed-by
  - Patch 9: Print the module version only after reading it succeeds.
    (Rick)
  - Patch 10: rewrite the changelog to add more background about the
    size bits in field IDs and also add an alternative discussion.
    [Rick]
  - Patch 10: Squash TDX_MD_FIELD_ELE_SIZE_CODE() and
    TDX_MD_FIELD_ELE_SIZE() into a single TDX_FIELD_SIZE() (Rick)
  - v2: https://lore.kernel.org/kvm/20260918132946.76533-1-chao.gao@intel.com/


Problem & Solution
==================

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.


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                 | 198 +++++++++++++++++++-
 arch/x86/virt/vmx/tdx/tdx.h                 |  46 +++++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 154 ---------------
 3 files changed, 241 insertions(+), 157 deletions(-)
 delete mode 100644 arch/x86/virt/vmx/tdx/tdx_global_metadata.c


base-commit: a49e2d257594931772ab8f0024708c3076f3aa1d
-- 
2.52.0

[PATCH v3 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.

Prepare to 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, a structure type and a member name.
Add __read_sys_metadata_table() helper to read every field in a table.
Annotate that 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. It
suggested adding read_sys_metadata_table() macro, which avoids repeating
the table name when passing both the table and its size.

Signed-off-by: Chao Gao <chao.gao@intel.com>
Link: https://lore.kernel.org/kvm/1e7bcbad-eb26-44b7-97ca-88ab53467212@intel.com/ # [1]
---
v3:
 - Pass the field ID to TDX_SYSINFO_MAP() instead of pasting the
   TDX_MD_FIELD_ID_ prefix onto a suffix inside the macro [Rick]
 - Call out that this patch only prepares to eliminate the two patterns.
   [Rick]
 - Define @offset in struct field_mapping as size_t [Binbin]
---
 arch/x86/virt/vmx/tdx/tdx.c | 39 +++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 96ced0494b68..67a80ed86bd3 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -395,6 +395,45 @@ 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;
+	size_t 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 read_sys_metadata_table(_mappings, _data) \
+	__read_sys_metadata_table(_mappings, ARRAY_SIZE(_mappings), _data)
+
+#define TDX_SYSINFO_MAP(_field, _type, _member)			\
+{								\
+	.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

[PATCH v3 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.

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

Signed-off-by: Chao Gao <chao.gao@intel.com>
---
v3:
 - Drop the get_tdx_sys_info_*() wrappers to remove an unnecessary layer
   of abstraction. [Rick, Dave]
 - Shorten the field ID defines from TDX_MD_FIELD_ID_* to TDX_FIELD_*
   [Rick]
---
 arch/x86/virt/vmx/tdx/tdx.c                 | 15 ++++++++++++---
 arch/x86/virt/vmx/tdx/tdx.h                 | 10 ++++++++++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 17 +----------------
 3 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 67a80ed86bd3..9d8a55a5da31 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;
@@ -434,6 +434,15 @@ 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(TDX_FIELD_MINOR_VERSION,  minor_version),
+	TDX_SYSINFO_MAP_VERSION(TDX_FIELD_MAJOR_VERSION,  major_version),
+	TDX_SYSINFO_MAP_VERSION(TDX_FIELD_UPDATE_VERSION, update_version),
+};
+
 #include "tdx_global_metadata.c"
 
 static __init int check_features(struct tdx_sys_info *sysinfo)
@@ -1401,7 +1410,7 @@ int tdx_module_run_update(void)
 	if (ret)
 		return ret;
 
-	ret = get_tdx_sys_info_version(&tdx_sysinfo.version);
+	ret = read_sys_metadata_table(version_mappings, &tdx_sysinfo.version);
 	/*
 	 * Only fails if there is something unexpected
 	 * and severely wrong with the module.
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index db209541d3cd..10cbc2d77a5a 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -52,6 +52,16 @@
 #define TDH_PHYMEM_PAMT_REMOVE		59
 #define TDH_SYS_DISABLE			69
 
+/*
+ * TDX global metadata field IDs.
+ *
+ * See "global_metadata.pdf" in Intel TDX Module ABI Definitions.
+ */
+/* Class "TDX Module Version" */
+#define TDX_FIELD_MINOR_VERSION			0x0800000100000003ULL
+#define TDX_FIELD_MAJOR_VERSION			0x0800000100000004ULL
+#define TDX_FIELD_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..c55674cd4ce6 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;
@@ -129,7 +114,7 @@ static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
 {
 	int ret = 0;
 
-	ret = ret ?: get_tdx_sys_info_version(&sysinfo->version);
+	ret = ret ?: read_sys_metadata_table(version_mappings, &sysinfo->version);
 
 	pr_info("Module version: " TDX_VERSION_FMT "\n",
 		sysinfo->version.major_version,
-- 
2.52.0

[PATCH v3 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                 |  7 +++++++
 arch/x86/virt/vmx/tdx/tdx.h                 |  3 +++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 13 +------------
 3 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 9d8a55a5da31..c78bb3be6303 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -443,6 +443,13 @@ static const struct field_mapping version_mappings[] = {
 	TDX_SYSINFO_MAP_VERSION(TDX_FIELD_UPDATE_VERSION, update_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_FIELD_TDX_FEATURES0, tdx_features0),
+};
+
 #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 10cbc2d77a5a..87e055a2546c 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -62,6 +62,9 @@
 #define TDX_FIELD_MAJOR_VERSION			0x0800000100000004ULL
 #define TDX_FIELD_UPDATE_VERSION		0x0800000100000005ULL
 
+/* Class "TDX Module Info" */
+#define TDX_FIELD_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 c55674cd4ce6..3996af787ff8 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;
@@ -121,7 +110,7 @@ static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
 		sysinfo->version.minor_version,
 		sysinfo->version.update_version);
 
-	ret = ret ?: get_tdx_sys_info_features(&sysinfo->features);
+	ret = ret ?: read_sys_metadata_table(feature_mappings, &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);
-- 
2.52.0

[PATCH v3 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                 | 16 ++++++++++
 arch/x86/virt/vmx/tdx/tdx.h                 |  8 +++++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 35 ++-------------------
 3 files changed, 26 insertions(+), 33 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index c78bb3be6303..24d22d8a2016 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -450,6 +450,22 @@ static const struct field_mapping feature_mappings[] __initconst = {
 	TDX_SYSINFO_MAP_FEATURES(TDX_FIELD_TDX_FEATURES0, tdx_features0),
 };
 
+#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(TDX_FIELD_MAX_TDMRS,		max_tdmrs),
+	TDX_SYSINFO_MAP_TDMR(TDX_FIELD_MAX_RESERVED_PER_TDMR,	max_reserved_per_tdmr),
+	TDX_SYSINFO_MAP_TDMR(TDX_FIELD_PAMT_4K_ENTRY_SIZE,	pamt_4k_entry_size),
+	TDX_SYSINFO_MAP_TDMR(TDX_FIELD_PAMT_2M_ENTRY_SIZE,	pamt_2m_entry_size),
+	TDX_SYSINFO_MAP_TDMR(TDX_FIELD_PAMT_1G_ENTRY_SIZE,	pamt_1g_entry_size),
+};
+
+static const struct field_mapping dpamt_mappings[] __initconst = {
+	TDX_SYSINFO_MAP_TDMR(TDX_FIELD_PAMT_PAGE_BITMAP_ENTRY_BITS,
+			     pamt_page_bitmap_entry_bits),
+};
+
 #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 87e055a2546c..4faf07925dc0 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -65,6 +65,14 @@
 /* Class "TDX Module Info" */
 #define TDX_FIELD_TDX_FEATURES0			0x0A00000300000008ULL
 
+/* Class "TDMR Info" */
+#define TDX_FIELD_MAX_TDMRS			0x9100000100000008ULL
+#define TDX_FIELD_MAX_RESERVED_PER_TDMR		0x9100000100000009ULL
+#define TDX_FIELD_PAMT_4K_ENTRY_SIZE		0x9100000100000010ULL
+#define TDX_FIELD_PAMT_2M_ENTRY_SIZE		0x9100000100000011ULL
+#define TDX_FIELD_PAMT_1G_ENTRY_SIZE		0x9100000100000012ULL
+#define TDX_FIELD_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 3996af787ff8..870caac6a585 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;
@@ -111,7 +80,7 @@ static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
 		sysinfo->version.update_version);
 
 	ret = ret ?: read_sys_metadata_table(feature_mappings, &sysinfo->features);
-	ret = ret ?: get_tdx_sys_info_tdmr(&sysinfo->tdmr);
+	ret = ret ?: read_sys_metadata_table(tdmr_mappings, &sysinfo->tdmr);
 	ret = ret ?: get_tdx_sys_info_td_ctrl(&sysinfo->td_ctrl);
 	ret = ret ?: get_tdx_sys_info_td_conf(&sysinfo->td_conf);
 
@@ -122,7 +91,7 @@ static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
 	 * DPAMT but don't expose the metadata.
 	 */
 	if (!ret && tdx_supports_dynamic_pamt(sysinfo))
-		ret = get_tdx_sys_info_tdmr_dpamt(&sysinfo->tdmr);
+		ret = read_sys_metadata_table(dpamt_mappings, &sysinfo->tdmr);
 
 	return ret;
 }
-- 
2.52.0

[PATCH v3 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                 |  9 +++++++++
 arch/x86/virt/vmx/tdx/tdx.h                 |  5 +++++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 17 +----------------
 3 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 24d22d8a2016..af4cafdd4c4e 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -466,6 +466,15 @@ static const struct field_mapping dpamt_mappings[] __initconst = {
 			     pamt_page_bitmap_entry_bits),
 };
 
+#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(TDX_FIELD_TDR_BASE_SIZE,	tdr_base_size),
+	TDX_SYSINFO_MAP_TD_CTRL(TDX_FIELD_TDCS_BASE_SIZE,	tdcs_base_size),
+	TDX_SYSINFO_MAP_TD_CTRL(TDX_FIELD_TDVPS_BASE_SIZE,	tdvps_base_size),
+};
+
 #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 4faf07925dc0..8bd8928c1802 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -73,6 +73,11 @@
 #define TDX_FIELD_PAMT_1G_ENTRY_SIZE		0x9100000100000012ULL
 #define TDX_FIELD_PAMT_PAGE_BITMAP_ENTRY_BITS	0x9100000000000013ULL
 
+/* Class "TD Control Structures" */
+#define TDX_FIELD_TDR_BASE_SIZE			0x9800000100000000ULL
+#define TDX_FIELD_TDCS_BASE_SIZE		0x9800000100000100ULL
+#define TDX_FIELD_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 870caac6a585..30834ecdccab 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;
@@ -81,7 +66,7 @@ static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
 
 	ret = ret ?: read_sys_metadata_table(feature_mappings, &sysinfo->features);
 	ret = ret ?: read_sys_metadata_table(tdmr_mappings, &sysinfo->tdmr);
-	ret = ret ?: get_tdx_sys_info_td_ctrl(&sysinfo->td_ctrl);
+	ret = ret ?: read_sys_metadata_table(td_ctrl_mappings, &sysinfo->td_ctrl);
 	ret = ret ?: get_tdx_sys_info_td_conf(&sysinfo->td_conf);
 
 	/*
-- 
2.52.0

[PATCH v3 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                 |  9 ++++++++-
 arch/x86/virt/vmx/tdx/tdx.h                 |  3 +++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 13 -------------
 3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index af4cafdd4c4e..79561312ebfd 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -475,6 +475,13 @@ static const struct field_mapping td_ctrl_mappings[] __initconst = {
 	TDX_SYSINFO_MAP_TD_CTRL(TDX_FIELD_TDVPS_BASE_SIZE,	tdvps_base_size),
 };
 
+#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(TDX_FIELD_MODULE_HV, module_hv),
+};
+
 #include "tdx_global_metadata.c"
 
 static __init int check_features(struct tdx_sys_info *sysinfo)
@@ -1402,7 +1409,7 @@ int tdx_module_shutdown(void)
 	int ret;
 	int cpu;
 
-	ret = get_tdx_sys_info_handoff(&handoff);
+	ret = read_sys_metadata_table(handoff_mappings, &handoff);
 	/*
 	 * Handoff information is required for proper
 	 * shutdown. Refuse to shut down without it.
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index 8bd8928c1802..720e301f599e 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -78,6 +78,9 @@
 #define TDX_FIELD_TDCS_BASE_SIZE		0x9800000100000100ULL
 #define TDX_FIELD_TDVPS_BASE_SIZE		0x9800000100000200ULL
 
+/* Class "TDX Module Handoff" */
+#define TDX_FIELD_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 30834ecdccab..8ca1cdcf55fe 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

[PATCH v3 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>
---
v3:
 - Reword the comment above the CPUID loops
---
 arch/x86/virt/vmx/tdx/tdx.c                 | 53 +++++++++++++++++++++
 arch/x86/virt/vmx/tdx/tdx.h                 | 10 ++++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 33 -------------
 3 files changed, 63 insertions(+), 33 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 79561312ebfd..e821ab76a140 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -482,6 +482,59 @@ static const struct field_mapping handoff_mappings[] = {
 	TDX_SYSINFO_MAP_HANDOFF(TDX_FIELD_MODULE_HV, module_hv),
 };
 
+#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(TDX_FIELD_ATTRIBUTES_FIXED0,	attributes_fixed0),
+	TDX_SYSINFO_MAP_TD_CONF(TDX_FIELD_ATTRIBUTES_FIXED1,	attributes_fixed1),
+	TDX_SYSINFO_MAP_TD_CONF(TDX_FIELD_XFAM_FIXED0,		xfam_fixed0),
+	TDX_SYSINFO_MAP_TD_CONF(TDX_FIELD_XFAM_FIXED1,		xfam_fixed1),
+	TDX_SYSINFO_MAP_TD_CONF(TDX_FIELD_NUM_CPUID_CONFIG,	num_cpuid_config),
+	TDX_SYSINFO_MAP_TD_CONF(TDX_FIELD_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_FIELD_CPUID_CONFIG_LEAVES and TDX_FIELD_CPUID_CONFIG_VALUES
+	 * 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_FIELD_CPUID_CONFIG_LEAVES + i,
+					      &td_conf->cpuid_config_leaves[i]);
+		if (ret)
+			return ret;
+
+		/* Each config has two u64s of CPUID values. */
+		for (j = 0; j < 2; j++) {
+			ret = read_sys_metadata_field(
+				TDX_FIELD_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 720e301f599e..e41fc5e4925e 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -78,6 +78,16 @@
 #define TDX_FIELD_TDCS_BASE_SIZE		0x9800000100000100ULL
 #define TDX_FIELD_TDVPS_BASE_SIZE		0x9800000100000200ULL
 
+/* Class "TD Configurability" */
+#define TDX_FIELD_ATTRIBUTES_FIXED0		0x1900000300000000ULL
+#define TDX_FIELD_ATTRIBUTES_FIXED1		0x1900000300000001ULL
+#define TDX_FIELD_XFAM_FIXED0			0x1900000300000002ULL
+#define TDX_FIELD_XFAM_FIXED1			0x1900000300000003ULL
+#define TDX_FIELD_NUM_CPUID_CONFIG		0x9900000100000004ULL
+#define TDX_FIELD_MAX_VCPUS_PER_TD		0x9900000100000008ULL
+#define TDX_FIELD_CPUID_CONFIG_LEAVES		0x9900000300000400ULL
+#define TDX_FIELD_CPUID_CONFIG_VALUES		0x9900000300000500ULL
+
 /* Class "TDX Module Handoff" */
 #define TDX_FIELD_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 8ca1cdcf55fe..f248c962fd5d 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 v3 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>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@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 e821ab76a140..1a69beb62ec2 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -535,7 +535,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 ?: read_sys_metadata_table(version_mappings, &sysinfo->version);
+
+	pr_info("Module version: " TDX_VERSION_FMT "\n",
+		sysinfo->version.major_version,
+		sysinfo->version.minor_version,
+		sysinfo->version.update_version);
+
+	ret = ret ?: read_sys_metadata_table(feature_mappings, &sysinfo->features);
+	ret = ret ?: read_sys_metadata_table(tdmr_mappings, &sysinfo->tdmr);
+	ret = ret ?: read_sys_metadata_table(td_ctrl_mappings, &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 = read_sys_metadata_table(dpamt_mappings, &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 f248c962fd5d..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 ?: read_sys_metadata_table(version_mappings, &sysinfo->version);
-
-	pr_info("Module version: " TDX_VERSION_FMT "\n",
-		sysinfo->version.major_version,
-		sysinfo->version.minor_version,
-		sysinfo->version.update_version);
-
-	ret = ret ?: read_sys_metadata_table(feature_mappings, &sysinfo->features);
-	ret = ret ?: read_sys_metadata_table(tdmr_mappings, &sysinfo->tdmr);
-	ret = ret ?: read_sys_metadata_table(td_ctrl_mappings, &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 = read_sys_metadata_table(dpamt_mappings, &sysinfo->tdmr);
-
-	return ret;
-}
-- 
2.52.0

[PATCH v3 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.

Print the module version only after its metadata has been read
successfully. The "ret ?:" chain had no early exit, so the pr_info()
between the reads ran even when the version read itself failed, printing a
partially read or all-zero version. That isn't considered a big problem,
but early returns make it easy to avoid.

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

Signed-off-by: Chao Gao <chao.gao@intel.com>
---
v3:
 - Print the module version only after reading its metadata succeeds [Rick]
---
 arch/x86/virt/vmx/tdx/tdx.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 1a69beb62ec2..e7d4fc3f350f 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -537,19 +537,32 @@ 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 ?: read_sys_metadata_table(version_mappings, &sysinfo->version);
+	ret = read_sys_metadata_table(version_mappings, &sysinfo->version);
+	if (ret)
+		return ret;
 
 	pr_info("Module version: " TDX_VERSION_FMT "\n",
 		sysinfo->version.major_version,
 		sysinfo->version.minor_version,
 		sysinfo->version.update_version);
 
-	ret = ret ?: read_sys_metadata_table(feature_mappings, &sysinfo->features);
-	ret = ret ?: read_sys_metadata_table(tdmr_mappings, &sysinfo->tdmr);
-	ret = ret ?: read_sys_metadata_table(td_ctrl_mappings, &sysinfo->td_ctrl);
-	ret = ret ?: get_tdx_sys_info_td_conf(&sysinfo->td_conf);
+	ret = read_sys_metadata_table(feature_mappings, &sysinfo->features);
+	if (ret)
+		return ret;
+
+	ret = read_sys_metadata_table(tdmr_mappings, &sysinfo->tdmr);
+	if (ret)
+		return ret;
+
+	ret = read_sys_metadata_table(td_ctrl_mappings, &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
@@ -557,10 +570,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 = read_sys_metadata_table(dpamt_mappings, &sysinfo->tdmr);
+		if (ret)
+			return ret;
+	}
 
-	return ret;
+	return 0;
 }
 
 static __init int check_features(struct tdx_sys_info *sysinfo)
-- 
2.52.0

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

[Chao Gao]

The TDX module reports its capabilities and limits as a set of metadata
fields, each identified by a field ID, and TDH.SYS.RD reads one field by
its ID. TDH.SYS.RD returns the value as a u64, but metadata fields are not
all 64 bits wide. A field ID therefore encodes the field's size into
bits 33:32.

The kernel mirrors these fields in C structures, and each member's size
must match the size encoded in the field ID. TDX_SYSINFO_MAP() stores the
member size and uses it to decide how many bytes to copy from the value
returned by TDH.SYS.RD. The size in the field ID is never consulted, so any
mismatch goes unnoticed. Declaring a u32 member for a 64-bit field, for
example, would silently store only its low 4 bytes.

Add macros to extract the size encoded in a field ID and check it against
the member size. The check happens at build time, so it catches a wrongly
typed member with no runtime cost.

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.

An alternative would be to leave the size bits out of the field ID
definitions and construct the IDs from the member sizes, which makes a
mismatch impossible. But the TDX module ABI definitions list the full field
IDs, and definitions with the size bits stripped would match nothing in the
docs, making them harder to verify. Keep the IDs exactly as documented and
check the size they encode against the C type instead.

AI was used under supervision to review code and workshop logs. It
suggested extracting TDX_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>
---
v3:
 - Add background on the size bits encoded in a field ID. [Rick]
 - Add rationale for checking the size in the field ID instead of
   building the ID from the member size. [Rick]
 - Squash TDX_MD_FIELD_ELE_SIZE_CODE() and TDX_MD_FIELD_ELE_SIZE() into a
   single TDX_FIELD_SIZE(). [Rick]
---
 arch/x86/virt/vmx/tdx/tdx.c | 12 +++++++++++-
 arch/x86/virt/vmx/tdx/tdx.h |  7 +++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index e7d4fc3f350f..360875efb263 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -427,11 +427,21 @@ static int __read_sys_metadata_table(const struct field_mapping *mappings,
 #define read_sys_metadata_table(_mappings, _data) \
 	__read_sys_metadata_table(_mappings, ARRAY_SIZE(_mappings), _data)
 
+/*
+ * The size encoded in the field ID and the size of the destination C
+ * member must agree.
+ */
+#define TDX_FIELD_SIZE_CHECK(_field, _type, _member)		\
+	BUILD_BUG_ON_ZERO(sizeof_field(_type, _member) !=	\
+			  TDX_FIELD_SIZE(_field))
+
 #define TDX_SYSINFO_MAP(_field, _type, _member)			\
 {								\
 	.field_id	= _field,				\
 	.offset		= offsetof(_type, _member),		\
-	.size		= sizeof_field(_type, _member),		\
+	.size		= sizeof_field(_type, _member) +	\
+			  TDX_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 e41fc5e4925e..b3694a80a0c8 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -91,6 +91,13 @@
 /* Class "TDX Module Handoff" */
 #define TDX_FIELD_MODULE_HV			0x8900000100000000ULL
 
+/*
+ * Bits 33:32 of a field ID hold the log2 of the metadata field size in
+ * bytes. See "Metadata Field Identifier" in the Intel TDX Module ABI
+ * Specification.
+ */
+#define TDX_FIELD_SIZE(field_id)	(1 << (((field_id) >> 32) & 0x3))
+
 /* TDX page types */
 #define	PT_NDA		0x0
 #define	PT_RSVD		0x1
-- 
2.52.0