Re: [PATCH] x86/kvm/tdx: Save %rbp in TDX_MODULE_CALL
From: Huang, Kai
Date: Thu May 23 2024 - 08:27:34 EST
On Thu, 2024-05-23 at 10:30 +0000, Huang, Kai wrote:
> On Thu, 2024-05-23 at 07:56 +0200, Jürgen Groß wrote:
> > On 20.05.24 13:54, Huang, Kai wrote:
> > > 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)?
> >
> > With that I got:
> >
> > [ 29.328484] virt/tdx: module verson: major 1, minor 5, internal 0
> >
> >
>
> Let me check TDX module guys on this and get back to you.
Hi Jurgen,
I was told the module starting with "1.5.06." has NO_RBP_MOD support.
And I think I was wrong about the <update> part of the version, and we
need that to determine the third part of the module version.
I was also told the 1.5.06 module hasn't been released to public yet, so I
guess your module doesn't support it.
I did another patch (attached) to check NO_RBP_MOD and reject module
initialization if it is not supported, and print out module version:
[ 146.566641] virt/tdx: NO_RBP_MOD feature is not supported
[ 146.572797] virt/tdx: module verson: 1.5.0.0
[ 146.577731] virt/tdx: module initialization failed (-22)
You can have another try to verify at your side, if that helps.
commit e3907269b34e4f488284f35716f231da7bc3e6b3
Author: Kai Huang <kai.huang@xxxxxxxxx>
Date: Thu May 23 23:17:28 2024 +1200
x86/virt/tdx: check NO_RBP_MOD and print module version
Signed-off-by: Kai Huang <kai.huang@xxxxxxxxx>
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 4d6826a76f78..ffc1c27b9134 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1097,11 +1097,60 @@ static int init_tdmrs(struct tdmr_info_list *tdmr_list)
return 0;
}
+static int check_module_compatibility(void)
+{
+ u64 tdx_features0;
+ int ret;
+
+ ret = read_sys_metadata_field(MD_FIELD_ID_TDX_FEATURES0,
+ &tdx_features0);
+
+ if (ret)
+ return ret;
+
+ if (!(tdx_features0 & TDX_FEATURES0_NO_RBP_MOD)) {
+ pr_err("NO_RBP_MOD feature is not supported\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static void print_tdx_version(void)
+{
+ u64 major, minor, update, 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_UPDATE_VERSION, &update);
+ if (ret)
+ return;
+
+ ret = read_sys_metadata_field(MD_FIELD_ID_INTERNAL_VERSION, &internal);
+ if (ret)
+ return;
+
+ pr_info("module verson: %u.%u.%u.%u\n",
+ (u16)major, (u16)minor, (u16)update, (u16)internal);
+}
+
static int init_tdx_module(void)
{
struct tdx_tdmr_sysinfo tdmr_sysinfo;
int ret;
+ /* Make sure TDX module meets kernel's expectation */
+ ret = check_module_compatibility();
+ if (ret)
+ goto out;
+
/*
* To keep things simple, assume that all TDX-protected memory
* will come from the page allocator. Make sure all pages in the
@@ -1155,6 +1204,10 @@ static int init_tdx_module(void)
* Lock out memory hotplug code while building it.
*/
put_online_mems();
+
+out:
+ 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..13ad52352e35 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -37,6 +37,14 @@
#define MD_FIELD_ID_PAMT_2M_ENTRY_SIZE 0x9100000100000011ULL
#define MD_FIELD_ID_PAMT_1G_ENTRY_SIZE 0x9100000100000012ULL
+#define MD_FIELD_ID_TDX_FEATURES0 0x0A00000300000008ULL
+#define TDX_FEATURES0_NO_RBP_MOD BIT_ULL(18)
+
+#define MD_FIELD_ID_MINOR_VERSION 0x0800000100000003ULL
+#define MD_FIELD_ID_MAJOR_VERSION 0x0800000100000004ULL
+#define MD_FIELD_ID_UPDATE_VERSION 0x0800000100000005ULL
+#define MD_FIELD_ID_INTERNAL_VERSION 0x0800000100000006ULL
+
/*
* Sub-field definition of metadata field ID.
*