Re: [PATCH] x86/kvm/tdx: Save %rbp in TDX_MODULE_CALL

[Huang, Kai]

On Fri, 2024-05-17 at 09:48 -0700, Dave Hansen wrote:
> On 5/17/24 08:58, Juergen Gross wrote:
> > On 17.05.24 17:52, Dave Hansen wrote:
> ..
> > > Once we have the specific TDX module version, we can go ask the folks
> > > who write it if there were any RBP clobbering bugs.
> > 
> > Okay, how to get the TDX module version?
> 
> You need something like this:
> 
> > https://lore.kernel.org/all/20231012134136.1310650-1-yi.sun@xxxxxxxxx/

This one prints TDX version info in the TDX guest, but not host.

The attached diff prints the TDX version (something like below) during
module initialization, and should meet Juergen's needs for temporary use:

[  113.543538] virt/tdx: module verson: major 1, minor 5, internal 0

> 
> .. and yeah, this needs to be upstream.
> 

From this thread I think it makes sense to add code to the TDX host code
to print the TDX version during module initialization.  I'll start to work
on this.

One thing is from the spec TDX has "4 versions": major, minor, update,
internal.  They are all 16-bit, and the overall version can be written in:

	<Major>.<Minor>.<Update>.<Internal>, e.g., 1.5.05.01

(see TDX module 1.5 API spec, section 3.3.2 "TDX Module Version".)

The attached diff only prints major, minor and internal, but leaves the
update out because I believe it is for module runtime update (yet to
confirm).

Given there are 4 versions, I think it makes sense to implement reading
them based on this patchset ...

https://lore.kernel.org/kvm/6940c326-bfca-4c67-badf-ab5c086bf492@xxxxxxxxx/T/

... which extends the global metadata reading code to support any
arbitrary struct and all element sizes (although all 4 versions are 16-
bit)?




diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 4d6826a76f78..f105214e36a3 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1097,6 +1097,27 @@ static int init_tdmrs(struct tdmr_info_list *tdmr_list)
 	return 0;
 }
 
+static void print_tdx_version(void)
+{
+	u64 major, minor, internal;
+	int ret;
+
+	ret = read_sys_metadata_field(MD_FIELD_ID_MAJOR_VERSION, &major);
+	if (ret)
+		return;
+
+	ret = read_sys_metadata_field(MD_FIELD_ID_MINOR_VERSION, &minor);
+	if (ret)
+		return;
+
+	ret = read_sys_metadata_field(MD_FIELD_ID_INTERNAL_VERSION, &internal);
+	if (ret)
+		return;
+
+	pr_info("module verson: major %u, minor %u, internal %u\n",
+			(u16)major, (u16)minor, (u16)internal);
+}
+
 static int init_tdx_module(void)
 {
 	struct tdx_tdmr_sysinfo tdmr_sysinfo;
@@ -1155,6 +1176,9 @@ static int init_tdx_module(void)
 	 * Lock out memory hotplug code while building it.
 	 */
 	put_online_mems();
+
+	print_tdx_version();
+
 	return ret;
 
 err_reset_pamts:
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index b701f69485d3..ae8a96e0f53c 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -37,6 +37,10 @@
 #define MD_FIELD_ID_PAMT_2M_ENTRY_SIZE		0x9100000100000011ULL
 #define MD_FIELD_ID_PAMT_1G_ENTRY_SIZE		0x9100000100000012ULL
 
+#define MD_FIELD_ID_MINOR_VERSION		0x0800000100000003ULL
+#define MD_FIELD_ID_MAJOR_VERSION		0x0800000100000004ULL
+#define MD_FIELD_ID_INTERNAL_VERSION		0x0800000100000006ULL
+
 /*
  * Sub-field definition of metadata field ID.
  *