[PATCH 0/6] TDX: Stop metadata auto-generation, improve readability

[PATCH 0/6] TDX: Stop metadata auto-generation, improve readability

[Xu Yilun]

Hi:

This addresses the common need [1][2] to stop auto-generating metadata
reading code, improve readability, allowing us to manually edit and
review metadata code in a comfortable way. TDX Connect needs to add more
metadata fields based on this series, and I believe also for DPAMT and
TDX Module runtime update.

The main changes derive from previous code & discussions before
auto-generation is introduced, including the usage of
struct field_mapping table, the build-time field size check, the concern
about awkward "ret = ret ?: " code pattern. [3]

Another concern from DPAMT [4] leads to the last patch and I realize all
optional features may face with the same problem - Optional metadata
reading should be skipped when a TDX Module doesn't support, don't fail
the whole TDX Module initialization. I use "TDX Module Extensions" as
the example and test case just because I have the TDX Connect ENV on
hand.

This series is based on Dan's tsm#devsec-phase1 [5] (convenient for my
testing), but is clean to apply to v6.18-rc7.

[1] https://lore.kernel.org/kvm/1e7bcbad-eb26-44b7-97ca-88ab53467212@intel.com/
[2] https://lore.kernel.org/all/89a4e42d-b0fd-49b0-8d51-df7bac0d5e5b@intel.com/
[3] https://lore.kernel.org/kvm/9a06e2cf469cbca2777ac2c4ef70579e6bb934d5.camel@intel.com/
[4] https://lore.kernel.org/kvm/850f7ce0571cb54bc984c79861bdfd104e097eb9.camel@intel.com/
[5] https://git.kernel.org/pub/scm/linux/kernel/git/devsec/tsm.git/log/?h=devsec-phase1


Xu Yilun (6):
  x86/virt/tdx: Move bit definitions of TDX_FEATURES0 to public header
  x86/virt/tdx: Move read_sys_metadata_field() to where it is called
  x86/virt/tdx: Refactor metadata reading with a clearer for loop
  x86/virt/tdx: Sanity check the size of each metadata field
  x86/virt/tdx: Add generic support for reading array-typed metadata
  x86/virt/tdx: Skip unsupported metadata by querying tdx_feature0

 arch/x86/include/asm/tdx.h                  |   5 +
 arch/x86/include/asm/tdx_global_metadata.h  |  12 +-
 arch/x86/virt/vmx/tdx/tdx.h                 |   3 -
 arch/x86/virt/vmx/tdx/tdx.c                 |  20 --
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 194 ++++++++++++--------
 5 files changed, 134 insertions(+), 100 deletions(-)

-- 
2.25.1

[PATCH 1/6] x86/virt/tdx: Move bit definitions of TDX_FEATURES0 to public header

[Xu Yilun]

Move bit definitions of TDX_FEATURES0 to TDX core public header.

Kernel users get TDX_FEATURES0 bitmap via tdx_get_sysinfo(). It is
reasonable to also public the definitions of each bit. TDX Connect (a
new TDX feature to enable Trusted I/O virtualization) will add new bits
and check them in separate kernel modules.

Take the opportunity to change its type to BIT_ULL since TDX_FEATURES0
is explicitly defined as 64-bit in both TDX Module Specification and
TDX core code.

Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
---
 arch/x86/include/asm/tdx.h  | 4 ++++
 arch/x86/virt/vmx/tdx/tdx.h | 3 ---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 6b338d7f01b7..96565f6b69b9 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -148,6 +148,10 @@ static __always_inline u64 sc_retry(sc_func_t func, u64 fn,
 int tdx_cpu_enable(void);
 int tdx_enable(void);
 const char *tdx_dump_mce_info(struct mce *m);
+
+/* Bit definitions of TDX_FEATURES0 metadata field */
+#define TDX_FEATURES0_NO_RBP_MOD	BIT_ULL(18)
+
 const struct tdx_sys_info *tdx_get_sysinfo(void);
 
 int tdx_guest_keyid_alloc(void);
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index 82bb82be8567..c641b4632826 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -84,9 +84,6 @@ struct tdmr_info {
 	DECLARE_FLEX_ARRAY(struct tdmr_reserved_area, reserved_areas);
 } __packed __aligned(TDMR_INFO_ALIGNMENT);
 
-/* Bit definitions of TDX_FEATURES0 metadata field */
-#define TDX_FEATURES0_NO_RBP_MOD	BIT(18)
-
 /*
  * Do not put any hardware-defined TDX structure representations below
  * this comment!
-- 
2.25.1

[PATCH 2/6] x86/virt/tdx: Move read_sys_metadata_field() to where it is called

[Xu Yilun]

Moving read_sys_metadata_field() to tdx_global_metadata.c marks the end
of auto-generating global metadata parsing code. The source of the
auto-generation, the JSON file, is not stable and not authoritative
enough. Switch back to manual editing and improve code readability.

The only possible usage of read_sys_metadata_field() is to cache all
global metadata in system memory on TDX Module initialization. Moving it
alongside other metadata reading code improves readability.

Take the opportunity to remove any description for auto-generation.

Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
---
 arch/x86/include/asm/tdx_global_metadata.h  |  6 +++---
 arch/x86/virt/vmx/tdx/tdx.c                 | 20 -------------------
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 22 ++++++++++++++++++++-
 3 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/arch/x86/include/asm/tdx_global_metadata.h b/arch/x86/include/asm/tdx_global_metadata.h
index 060a2ad744bf..b44f1df013b2 100644
--- a/arch/x86/include/asm/tdx_global_metadata.h
+++ b/arch/x86/include/asm/tdx_global_metadata.h
@@ -1,7 +1,7 @@
 /* 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
+/* TDX global metadata structures. */
+#ifndef _X86_VIRT_TDX_GLOBAL_METADATA_H
+#define _X86_VIRT_TDX_GLOBAL_METADATA_H
 
 #include <linux/types.h>
 
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index eac403248462..0d7f9bdac8a4 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -257,26 +257,6 @@ static int build_tdx_memlist(struct list_head *tmb_list)
 	return ret;
 }
 
-static int read_sys_metadata_field(u64 field_id, u64 *data)
-{
-	struct tdx_module_args args = {};
-	int ret;
-
-	/*
-	 * TDH.SYS.RD -- reads one global metadata field
-	 *  - RDX (in): the field to read
-	 *  - R8 (out): the field data
-	 */
-	args.rdx = field_id;
-	ret = seamcall_prerr_ret(TDH_SYS_RD, &args);
-	if (ret)
-		return ret;
-
-	*data = args.r8;
-
-	return 0;
-}
-
 #include "tdx_global_metadata.c"
 
 static 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 13ad2663488b..0dfb3a9995fe 100644
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
@@ -1,12 +1,32 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * Automatically generated functions to read TDX global metadata.
+ * 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 int read_sys_metadata_field(u64 field_id, u64 *data)
+{
+	struct tdx_module_args args = {};
+	int ret;
+
+	/*
+	 * TDH.SYS.RD -- reads one global metadata field
+	 *  - RDX (in): the field to read
+	 *  - R8 (out): the field data
+	 */
+	args.rdx = field_id;
+	ret = seamcall_prerr_ret(TDH_SYS_RD, &args);
+	if (ret)
+		return ret;
+
+	*data = args.r8;
+
+	return 0;
+}
+
 static int get_tdx_sys_info_features(struct tdx_sys_info_features *sysinfo_features)
 {
 	int ret = 0;
-- 
2.25.1

[PATCH 3/6] x86/virt/tdx: Refactor metadata reading with a clearer for loop

[Xu Yilun]

Replace confusing "ret = ret ?: " code pattern with a normal for loop.

The existing code pattern is compact and friendly to auto-generation.
However, hiding the stop-on-failure logic within the sequential
execution pattern reduces readability. Revive the field mapping table
which lists the field_ids to read and where to store each readout value.
Iterate the table with a normal for loop.

For now this metadata reading process doesn't work well with array typed
fields. Use dedicated method to read these 2 array typed fields. Will
improve in later patches.

Cc: Kai Huang <kai.huang@intel.com>
Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
---
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 172 ++++++++++----------
 1 file changed, 86 insertions(+), 86 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
index 0dfb3a9995fe..3db87c4accd6 100644
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
@@ -7,8 +7,50 @@
  * Include this file to other C file instead.
  */
 
-static int read_sys_metadata_field(u64 field_id, u64 *data)
+struct field_mapping {
+	u64 field_id;
+	int offset;
+	int size;
+};
+
+#define TD_SYSINFO_MAP(_field_id, _member)				\
+	{ .field_id = _field_id,					\
+	  .offset = offsetof(struct tdx_sys_info, _member),		\
+	  .size = sizeof_field(struct tdx_sys_info, _member) }
+
+/* Map TD_SYSINFO fields into 'struct tdx_sys_info': */
+static const struct field_mapping mappings[] = {
+	TD_SYSINFO_MAP(0x0A00000300000008, features.tdx_features0),
+
+	TD_SYSINFO_MAP(0x9100000100000008, tdmr.max_tdmrs),
+	TD_SYSINFO_MAP(0x9100000100000009, tdmr.max_reserved_per_tdmr),
+	TD_SYSINFO_MAP(0x9100000100000010, tdmr.pamt_4k_entry_size),
+	TD_SYSINFO_MAP(0x9100000100000011, tdmr.pamt_2m_entry_size),
+	TD_SYSINFO_MAP(0x9100000100000012, tdmr.pamt_1g_entry_size),
+
+	TD_SYSINFO_MAP(0x9800000100000000, td_ctrl.tdr_base_size),
+	TD_SYSINFO_MAP(0x9800000100000100, td_ctrl.tdcs_base_size),
+	TD_SYSINFO_MAP(0x9800000100000200, td_ctrl.tdvps_base_size),
+
+	TD_SYSINFO_MAP(0x1900000300000000, td_conf.attributes_fixed0),
+	TD_SYSINFO_MAP(0x1900000300000001, td_conf.attributes_fixed1),
+	TD_SYSINFO_MAP(0x1900000300000002, td_conf.xfam_fixed0),
+	TD_SYSINFO_MAP(0x1900000300000003, td_conf.xfam_fixed1),
+	TD_SYSINFO_MAP(0x9900000100000004, td_conf.num_cpuid_config),
+	TD_SYSINFO_MAP(0x9900000100000008, td_conf.max_vcpus_per_td),
+};
+
+/* Populate the following fields in special manner, separate them out. */
+static const struct field_mapping cpuid_config_leaves =
+	TD_SYSINFO_MAP(0x9900000300000400, td_conf.cpuid_config_leaves[0]);
+
+static const struct field_mapping cpuid_config_values =
+	TD_SYSINFO_MAP(0x9900000300000500, td_conf.cpuid_config_values[0][0]);
+
+static int read_sys_metadata_field(u64 field_id, int offset, int size,
+				   struct tdx_sys_info *ts)
 {
+	void *field = ((void *)ts) + offset;
 	struct tdx_module_args args = {};
 	int ret;
 
@@ -22,97 +64,55 @@ static int read_sys_metadata_field(u64 field_id, u64 *data)
 	if (ret)
 		return ret;
 
-	*data = args.r8;
+	memcpy(field, &args.r8, size);
 
 	return 0;
 }
 
-static int get_tdx_sys_info_features(struct tdx_sys_info_features *sysinfo_features)
-{
-	int ret = 0;
-	u64 val;
-
-	if (!ret && !(ret = read_sys_metadata_field(0x0A00000300000008, &val)))
-		sysinfo_features->tdx_features0 = val;
-
-	return ret;
-}
-
-static int get_tdx_sys_info_tdmr(struct tdx_sys_info_tdmr *sysinfo_tdmr)
-{
-	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 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 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 int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
 {
-	int ret = 0;
+	struct tdx_sys_info_td_conf *td_conf = &sysinfo->td_conf;
+	int ret, i;
+
+	/* Populate 'tdx_sys_info' fields using the mapping structure above: */
+	for (i = 0; i < ARRAY_SIZE(mappings); i++) {
+		ret = read_sys_metadata_field(mappings[i].field_id,
+					      mappings[i].offset,
+					      mappings[i].size,
+					      sysinfo);
+		if (ret)
+			return ret;
+	}
+
+	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;
 
-	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);
+	/*
+	 * Populate 2 special fields, td_conf.cpuid_config_leaves[] and
+	 * td_conf.cpuid_config_values[][]
+	 */
+	for (i = 0; i < td_conf->num_cpuid_config; i++) {
+		ret = read_sys_metadata_field(cpuid_config_leaves.field_id + i,
+					      cpuid_config_leaves.offset +
+						cpuid_config_leaves.size * i,
+					      cpuid_config_leaves.size,
+					      sysinfo);
+		if (ret)
+			return ret;
+	}
+
+	for (i = 0;
+	     i < td_conf->num_cpuid_config * ARRAY_SIZE(td_conf->cpuid_config_values[0]);
+	     i++) {
+		ret = read_sys_metadata_field(cpuid_config_values.field_id + i,
+					      cpuid_config_values.offset +
+						cpuid_config_values.size * i,
+					      cpuid_config_values.size,
+					      sysinfo);
+		if (ret)
+			return ret;
+	}
 
-	return ret;
+	return 0;
 }
-- 
2.25.1

[PATCH 4/6] x86/virt/tdx: Sanity check the size of each metadata field

[Xu Yilun]

Add the build-time check to ensure the manually input field mappings
have consistent size definitions. I.e. for each mapping entry, the size
code in field_id should match the corresponding struct member type. This
type safe check prevents wrong interpretation of the readout value.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Kai Huang <kai.huang@intel.com>
Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
---
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 24 ++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
index 3db87c4accd6..836d97166a7a 100644
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
@@ -13,10 +13,32 @@ struct field_mapping {
 	int size;
 };
 
+/*
+ * Size of field abstracted from field ID.
+ *
+ * See Table "MD_FIELD_ID (Metadata Field Identifier / Sequence Header)
+ * Definition", TDX module 1.5 ABI spec.
+ *
+ *  - Bit 33:32: ELEMENT_SIZE_CODE -- size of a single element of metadata
+ *
+ *	0: 8 bits
+ *	1: 16 bits
+ *	2: 32 bits
+ *	3: 64 bits
+ */
+#define MD_FIELD_SIZE_CODE(_field_id)	\
+	(((_field_id) & GENMASK_ULL(33, 32)) >> 32)
+
+#define MD_FIELD_SIZE(_field_id) (1 << MD_FIELD_SIZE_CODE(_field_id))
+
+#define TD_SYSINFO_CHECK_SIZE(_field_id, _size)		\
+	__builtin_choose_expr(MD_FIELD_SIZE(_field_id) == (_size), _size, (void)0)
+
 #define TD_SYSINFO_MAP(_field_id, _member)				\
 	{ .field_id = _field_id,					\
 	  .offset = offsetof(struct tdx_sys_info, _member),		\
-	  .size = sizeof_field(struct tdx_sys_info, _member) }
+	  .size = TD_SYSINFO_CHECK_SIZE(_field_id,			\
+					sizeof_field(struct tdx_sys_info, _member)) }
 
 /* Map TD_SYSINFO fields into 'struct tdx_sys_info': */
 static const struct field_mapping mappings[] = {
-- 
2.25.1

[PATCH 5/6] x86/virt/tdx: Add generic support for reading array-typed metadata

[Xu Yilun]

Rework metadata field reading helper to read array-typed metadata fields
in a unified way. Eliminate the need to read cpuid_config_leaves/values
in special manner.

By adding the array index to the base field_id, the array elements can
be read out. But the actual valid array index boundary should be
retrieved from other metadata fields. Otherwise TDX Module returns error
on an invalid field_id and the whole metadata reading process fails.
That leads to the special handling of these fields.

TDG.SYS.RD provides an output parameter (RDX) for next readable
field_id. Use this return value to validate the next array index and
detect actual array boundary. This makes the array-typed metadata fields
reading self-contained. Remove all special handling for them.

Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
---
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 99 +++++++++------------
 1 file changed, 44 insertions(+), 55 deletions(-)

diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
index 836d97166a7a..c366bace454e 100644
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
@@ -11,6 +11,7 @@ struct field_mapping {
 	u64 field_id;
 	int offset;
 	int size;
+	int count;
 };
 
 /*
@@ -34,11 +35,24 @@ struct field_mapping {
 #define TD_SYSINFO_CHECK_SIZE(_field_id, _size)		\
 	__builtin_choose_expr(MD_FIELD_SIZE(_field_id) == (_size), _size, (void)0)
 
-#define TD_SYSINFO_MAP(_field_id, _member)				\
+#define _TD_SYSINFO_MAP(_field_id, _member, _count)			\
 	{ .field_id = _field_id,					\
 	  .offset = offsetof(struct tdx_sys_info, _member),		\
 	  .size = TD_SYSINFO_CHECK_SIZE(_field_id,			\
-					sizeof_field(struct tdx_sys_info, _member)) }
+					sizeof_field(struct tdx_sys_info, _member)), \
+	  .count = _count }
+
+#define TD_SYSINFO_MAP(_field_id, _member)				\
+	_TD_SYSINFO_MAP(_field_id, _member, 1)
+
+#define TD_SYSINFO_MAP_ARR(_field_id, _arr)				\
+	_TD_SYSINFO_MAP(_field_id, _arr[0],				\
+			ARRAY_SIZE(((struct tdx_sys_info *)0)->_arr))
+
+#define TD_SYSINFO_MAP_2DARR(_field_id, _arr)				\
+	_TD_SYSINFO_MAP(_field_id, _arr[0][0],				\
+			ARRAY_SIZE(((struct tdx_sys_info *)0)->_arr) *	\
+			ARRAY_SIZE(((struct tdx_sys_info *)0)->_arr[0]))
 
 /* Map TD_SYSINFO fields into 'struct tdx_sys_info': */
 static const struct field_mapping mappings[] = {
@@ -60,33 +74,37 @@ static const struct field_mapping mappings[] = {
 	TD_SYSINFO_MAP(0x1900000300000003, td_conf.xfam_fixed1),
 	TD_SYSINFO_MAP(0x9900000100000004, td_conf.num_cpuid_config),
 	TD_SYSINFO_MAP(0x9900000100000008, td_conf.max_vcpus_per_td),
+	TD_SYSINFO_MAP_ARR(0x9900000300000400, td_conf.cpuid_config_leaves),
+	TD_SYSINFO_MAP_2DARR(0x9900000300000500, td_conf.cpuid_config_values),
 };
 
-/* Populate the following fields in special manner, separate them out. */
-static const struct field_mapping cpuid_config_leaves =
-	TD_SYSINFO_MAP(0x9900000300000400, td_conf.cpuid_config_leaves[0]);
-
-static const struct field_mapping cpuid_config_values =
-	TD_SYSINFO_MAP(0x9900000300000500, td_conf.cpuid_config_values[0][0]);
-
-static int read_sys_metadata_field(u64 field_id, int offset, int size,
+static int read_sys_metadata_field(const struct field_mapping *base,
 				   struct tdx_sys_info *ts)
 {
-	void *field = ((void *)ts) + offset;
-	struct tdx_module_args args = {};
-	int ret;
-
-	/*
-	 * TDH.SYS.RD -- reads one global metadata field
-	 *  - RDX (in): the field to read
-	 *  - R8 (out): the field data
-	 */
-	args.rdx = field_id;
-	ret = seamcall_prerr_ret(TDH_SYS_RD, &args);
-	if (ret)
-		return ret;
-
-	memcpy(field, &args.r8, size);
+	int i, ret;
+
+	for (i = 0; i < base->count; i++) {
+		void *field = ((void *)ts) + base->offset + base->size * i;
+		u64 field_id = base->field_id + i;
+		struct tdx_module_args args = {};
+
+		/*
+		 * TDH.SYS.RD -- reads one global metadata field
+		 *  - RDX (in): the field to read
+		 *  - RDX (out) : the next valid field ID
+		 *  - R8 (out): the field data
+		 */
+		args.rdx = field_id;
+		ret = seamcall_prerr_ret(TDH_SYS_RD, &args);
+		if (ret)
+			return ret;
+
+		memcpy(field, &args.r8, base->size);
+
+		/* field_id + 1 is invalid, the metadata array ends */
+		if (args.rdx != field_id + 1)
+			break;
+	}
 
 	return 0;
 }
@@ -98,10 +116,7 @@ static int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
 
 	/* Populate 'tdx_sys_info' fields using the mapping structure above: */
 	for (i = 0; i < ARRAY_SIZE(mappings); i++) {
-		ret = read_sys_metadata_field(mappings[i].field_id,
-					      mappings[i].offset,
-					      mappings[i].size,
-					      sysinfo);
+		ret = read_sys_metadata_field(&mappings[i], sysinfo);
 		if (ret)
 			return ret;
 	}
@@ -110,31 +125,5 @@ static int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
 	    td_conf->num_cpuid_config > ARRAY_SIZE(td_conf->cpuid_config_values))
 		return -EINVAL;
 
-	/*
-	 * Populate 2 special fields, td_conf.cpuid_config_leaves[] and
-	 * td_conf.cpuid_config_values[][]
-	 */
-	for (i = 0; i < td_conf->num_cpuid_config; i++) {
-		ret = read_sys_metadata_field(cpuid_config_leaves.field_id + i,
-					      cpuid_config_leaves.offset +
-						cpuid_config_leaves.size * i,
-					      cpuid_config_leaves.size,
-					      sysinfo);
-		if (ret)
-			return ret;
-	}
-
-	for (i = 0;
-	     i < td_conf->num_cpuid_config * ARRAY_SIZE(td_conf->cpuid_config_values[0]);
-	     i++) {
-		ret = read_sys_metadata_field(cpuid_config_values.field_id + i,
-					      cpuid_config_values.offset +
-						cpuid_config_values.size * i,
-					      cpuid_config_values.size,
-					      sysinfo);
-		if (ret)
-			return ret;
-	}
-
 	return 0;
 }
-- 
2.25.1

[PATCH 6/6] x86/virt/tdx: Skip unsupported metadata by querying tdx_feature0

[Xu Yilun]

TDX Module will add new metadata fields for new features over time.
These new metadata fields are optional and only valid when the
corresponding bits in tdx_feature0 are set. Add new helpers to specify
the required feature bits for each field_id, to avoid reading
unsupported metadata fields and in turn failing the entire metadata
reading process.

Add definitions for the new metadata fields "TDX Module Extensions" as
the example.

Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
---
 arch/x86/include/asm/tdx.h                  |  1 +
 arch/x86/include/asm/tdx_global_metadata.h  |  6 +++++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 25 ++++++++++++++++-----
 3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 96565f6b69b9..886d65ed58c8 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -151,6 +151,7 @@ const char *tdx_dump_mce_info(struct mce *m);
 
 /* Bit definitions of TDX_FEATURES0 metadata field */
 #define TDX_FEATURES0_NO_RBP_MOD	BIT_ULL(18)
+#define TDX_FEATURES0_EXT		BIT_ULL(39)
 
 const struct tdx_sys_info *tdx_get_sysinfo(void);
 
diff --git a/arch/x86/include/asm/tdx_global_metadata.h b/arch/x86/include/asm/tdx_global_metadata.h
index b44f1df013b2..feb9097c9881 100644
--- a/arch/x86/include/asm/tdx_global_metadata.h
+++ b/arch/x86/include/asm/tdx_global_metadata.h
@@ -34,11 +34,17 @@ struct tdx_sys_info_td_conf {
 	u64 cpuid_config_values[128][2];
 };
 
+struct tdx_sys_info_ext {
+	u16 memory_pool_required_pages;
+	u8 ext_required;
+};
+
 struct tdx_sys_info {
 	struct tdx_sys_info_features features;
 	struct tdx_sys_info_tdmr tdmr;
 	struct tdx_sys_info_td_ctrl td_ctrl;
 	struct tdx_sys_info_td_conf td_conf;
+	struct tdx_sys_info_ext ext;
 };
 
 #endif
diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
index c366bace454e..6835af65a5f8 100644
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
@@ -12,6 +12,7 @@ struct field_mapping {
 	int offset;
 	int size;
 	int count;
+	u64 feature0_mask;
 };
 
 /*
@@ -35,24 +36,28 @@ struct field_mapping {
 #define TD_SYSINFO_CHECK_SIZE(_field_id, _size)		\
 	__builtin_choose_expr(MD_FIELD_SIZE(_field_id) == (_size), _size, (void)0)
 
-#define _TD_SYSINFO_MAP(_field_id, _member, _count)			\
+#define _TD_SYSINFO_MAP(_field_id, _member, _count, _feature0_mask)	\
 	{ .field_id = _field_id,					\
 	  .offset = offsetof(struct tdx_sys_info, _member),		\
 	  .size = TD_SYSINFO_CHECK_SIZE(_field_id,			\
 					sizeof_field(struct tdx_sys_info, _member)), \
-	  .count = _count }
+	  .count = _count,						\
+	  .feature0_mask = _feature0_mask }
 
 #define TD_SYSINFO_MAP(_field_id, _member)				\
-	_TD_SYSINFO_MAP(_field_id, _member, 1)
+	_TD_SYSINFO_MAP(_field_id, _member, 1, 0)
 
 #define TD_SYSINFO_MAP_ARR(_field_id, _arr)				\
 	_TD_SYSINFO_MAP(_field_id, _arr[0],				\
-			ARRAY_SIZE(((struct tdx_sys_info *)0)->_arr))
+			ARRAY_SIZE(((struct tdx_sys_info *)0)->_arr), 0)
 
 #define TD_SYSINFO_MAP_2DARR(_field_id, _arr)				\
 	_TD_SYSINFO_MAP(_field_id, _arr[0][0],				\
 			ARRAY_SIZE(((struct tdx_sys_info *)0)->_arr) *	\
-			ARRAY_SIZE(((struct tdx_sys_info *)0)->_arr[0]))
+			ARRAY_SIZE(((struct tdx_sys_info *)0)->_arr[0]), 0)
+
+#define TD_SYSINFO_MAP_FEATURE0(_field_id, _member, _feature0_mask)	\
+	_TD_SYSINFO_MAP(_field_id, _member, 1, _feature0_mask)
 
 /* Map TD_SYSINFO fields into 'struct tdx_sys_info': */
 static const struct field_mapping mappings[] = {
@@ -76,6 +81,12 @@ static const struct field_mapping mappings[] = {
 	TD_SYSINFO_MAP(0x9900000100000008, td_conf.max_vcpus_per_td),
 	TD_SYSINFO_MAP_ARR(0x9900000300000400, td_conf.cpuid_config_leaves),
 	TD_SYSINFO_MAP_2DARR(0x9900000300000500, td_conf.cpuid_config_values),
+
+	TD_SYSINFO_MAP_FEATURE0(0x3100000100000000,
+				ext.memory_pool_required_pages,
+				TDX_FEATURES0_EXT),
+	TD_SYSINFO_MAP_FEATURE0(0x3100000000000001, ext.ext_required,
+				TDX_FEATURES0_EXT),
 };
 
 static int read_sys_metadata_field(const struct field_mapping *base,
@@ -83,6 +94,10 @@ static int read_sys_metadata_field(const struct field_mapping *base,
 {
 	int i, ret;
 
+	if (base->feature0_mask &&
+	    !(ts->features.tdx_features0 & base->feature0_mask))
+		return 0;
+
 	for (i = 0; i < base->count; i++) {
 		void *field = ((void *)ts) + base->offset + base->size * i;
 		u64 field_id = base->field_id + i;
-- 
2.25.1

Re: [PATCH 0/6] TDX: Stop metadata auto-generation, improve readability

[dan.j.williams]

Xu Yilun wrote:
> Hi:
> 
> This addresses the common need [1][2] to stop auto-generating metadata
> reading code, improve readability, allowing us to manually edit and
> review metadata code in a comfortable way. TDX Connect needs to add more
> metadata fields based on this series, and I believe also for DPAMT and
> TDX Module runtime update.

While the writing is on the wall that the autogenerated metadata
infrastructure has become more trouble than it is worth, that work can
come after some of the backlog built on the old way has cleared out.
These in-flight sets of DPAMT, Module Update, and TDX PCIe Linux
Encryption can stay with what they started.

I.e. let us not start injecting new dependencies into in-flight review.

Re: [PATCH 0/6] TDX: Stop metadata auto-generation, improve readability

[Xu Yilun]

On Sat, Dec 06, 2025 at 11:44:49PM -0800, dan.j.williams@intel.com wrote:
> Xu Yilun wrote:
> > Hi:
> > 
> > This addresses the common need [1][2] to stop auto-generating metadata
> > reading code, improve readability, allowing us to manually edit and
> > review metadata code in a comfortable way. TDX Connect needs to add more
> > metadata fields based on this series, and I believe also for DPAMT and
> > TDX Module runtime update.
> 
> While the writing is on the wall that the autogenerated metadata
> infrastructure has become more trouble than it is worth, that work can
> come after some of the backlog built on the old way has cleared out.
> These in-flight sets of DPAMT, Module Update, and TDX PCIe Linux
> Encryption can stay with what they started.
> 
> I.e. let us not start injecting new dependencies into in-flight review.

Yes, I agree. One of the goal is to minimize the manual changes when a
new metadata is to be added, so I expect minor work for the switching
later.

Thanks,
Yilun