[v3,6/8] x86/virt/tdx: Print TDX module basic information

Commit Message

Huang, Kai Aug. 27, 2024, 7:14 a.m. UTC

Currently the kernel doesn't print any information regarding the TDX
module itself, e.g. module version.  In practice such information is
useful, especially to the developers.

For instance, there are a couple of use cases for dumping module basic
information:

1) When something goes wrong around using TDX, the information like TDX
   module version, supported features etc could be helpful [1][2].

2) For Linux, when the user wants to update the TDX module, one needs to
   replace the old module in a specific location in the EFI partition
   with the new one so that after reboot the BIOS can load it.  However,
   after kernel boots, currently the user has no way to verify it is
   indeed the new module that gets loaded and initialized (e.g., error
   could happen when replacing the old module).  With the module version
   dumped the user can verify this easily.

So dump the basic TDX module information:

 - TDX module version, and the build date.
 - TDX_FEATURES0: Supported TDX features.

And dump the information right after reading global metadata, so that
this information is printed no matter whether module initialization
fails or not.

The actual dmesg will look like:

  virt/tdx: Initializing TDX module: 1.5.00.00.0481 (build_date 20230323), TDX_FEATURES0 0xfbf

Link: https://lore.kernel.org/lkml/[e-mail]/T/#m352829aedf6680d4628c7e40dc40b332eda93355 [1]
Link: https://lore.kernel.org/lkml/[e-mail]/T/#m351ebcbc006d2e5bc3e7650206a087cb2708d451 [2]
Signed-off-by: Kai Huang <[e-mail]>
---

v2 -> v3:
 - 'struct tdx_sysinfo_module_info' -> 'struct tdx_sys_info_features'
 - 'struct tdx_sysinfo_module_version' -> 'struct tdx_sys_info_version'
 - Remove the 'sys_attributes' and the check of debug/production module.

 https://lore.kernel.org/kvm/[e-mail]/T/#md73dd9b02a492acf4a6facae63e8d030e320967d


---
 arch/x86/virt/vmx/tdx/tdx.c | 61 +++++++++++++++++++++++++++++++++++++
 arch/x86/virt/vmx/tdx/tdx.h | 34 ++++++++++++++++++++-
 2 files changed, 94 insertions(+), 1 deletion(-)

Comments

Dan Williams Sept. 6, 2024, 10:46 p.m. UTC | #1

Kai Huang wrote:
> Currently the kernel doesn't print any information regarding the TDX
> module itself, e.g. module version.  In practice such information is
> useful, especially to the developers.
> 
> For instance, there are a couple of use cases for dumping module basic
> information:
> 
> 1) When something goes wrong around using TDX, the information like TDX
>    module version, supported features etc could be helpful [1][2].
> 
> 2) For Linux, when the user wants to update the TDX module, one needs to
>    replace the old module in a specific location in the EFI partition
>    with the new one so that after reboot the BIOS can load it.  However,
>    after kernel boots, currently the user has no way to verify it is
>    indeed the new module that gets loaded and initialized (e.g., error
>    could happen when replacing the old module).  With the module version
>    dumped the user can verify this easily.

For this specific use case the kernel log is less useful then finding
a place to put this in sysfs. This gets back to a proposal to have TDX
instantiate a "tdx_tsm" device which among other things could host this
version data.

The kernel log message is ok, but parsing the kernel log is not
sufficient for this update validation flow concern.

[..]
> +static void print_basic_sys_info(struct tdx_sys_info *sysinfo)
> +{
> +	struct tdx_sys_info_features *features = &sysinfo->features;
> +	struct tdx_sys_info_version *version = &sysinfo->version;
> +
> +	/*
> +	 * TDX module version encoding:
> +	 *
> +	 *   <major>.<minor>.<update>.<internal>.<build_num>
> +	 *
> +	 * When printed as text, <major> and <minor> are 1-digit,
> +	 * <update> and <internal> are 2-digits and <build_num>
> +	 * is 4-digits.
> +	 */
> +	pr_info("Initializing TDX module: %u.%u.%02u.%02u.%04u (build_date %u), TDX_FEATURES0 0x%llx\n",
> +			version->major, version->minor,	version->update,
> +			version->internal, version->build_num,
> +			version->build_date, features->tdx_features0);

I do not see the value in dumping a raw features value in the log.
Either parse it or omit it. I would leave it for the tdx_tsm device to
emit.

Huang, Kai Sept. 9, 2024, 10:18 a.m. UTC | #2

On Fri, 2024-09-06 at 15:46 -0700, Dan Williams wrote:
> Kai Huang wrote:
> > Currently the kernel doesn't print any information regarding the TDX
> > module itself, e.g. module version.  In practice such information is
> > useful, especially to the developers.
> > 
> > For instance, there are a couple of use cases for dumping module basic
> > information:
> > 
> > 1) When something goes wrong around using TDX, the information like TDX
> >    module version, supported features etc could be helpful [1][2].
> > 
> > 2) For Linux, when the user wants to update the TDX module, one needs to
> >    replace the old module in a specific location in the EFI partition
> >    with the new one so that after reboot the BIOS can load it.  However,
> >    after kernel boots, currently the user has no way to verify it is
> >    indeed the new module that gets loaded and initialized (e.g., error
> >    could happen when replacing the old module).  With the module version
> >    dumped the user can verify this easily.
> 
> For this specific use case the kernel log is less useful then finding
> a place to put this in sysfs. This gets back to a proposal to have TDX
> instantiate a "tdx_tsm" device which among other things could host this
> version data.
> 
> The kernel log message is ok, but parsing the kernel log is not
> sufficient for this update validation flow concern.

Agree we eventually need /sysfs files for TDX module info like these.  For case
2) the developer who requested this use case to me just wanted to see the module
version dump in the kernel message so that he can verify, so I think this is at
least useful for developers.

Perhaps I can just trim the 2) to below?

  User may want to quickly know TDX module version to see whether the 
  loaded module is the expected one.


> 
> [..]
> > +static void print_basic_sys_info(struct tdx_sys_info *sysinfo)
> > +{
> > +	struct tdx_sys_info_features *features = &sysinfo->features;
> > +	struct tdx_sys_info_version *version = &sysinfo->version;
> > +
> > +	/*
> > +	 * TDX module version encoding:
> > +	 *
> > +	 *   <major>.<minor>.<update>.<internal>.<build_num>
> > +	 *
> > +	 * When printed as text, <major> and <minor> are 1-digit,
> > +	 * <update> and <internal> are 2-digits and <build_num>
> > +	 * is 4-digits.
> > +	 */
> > +	pr_info("Initializing TDX module: %u.%u.%02u.%02u.%04u (build_date %u), TDX_FEATURES0 0x%llx\n",
> > +			version->major, version->minor,	version->update,
> > +			version->internal, version->build_num,
> > +			version->build_date, features->tdx_features0);
> 
> I do not see the value in dumping a raw features value in the log.
> Either parse it or omit it. I would leave it for the tdx_tsm device to
> emit.

The raw features value is mainly for developers.  They can decode the value
based on the spec.  I think it will still be useful.

But sure I can remove it if you want.  With this I'll also defer reading the
tdx_features0 to the patch ("x86/virt/tdx: Don't initialize module that doesn't
support NO_RBP_MOD feature"), because if we don't print it, then we don't need
to read it in this patch.

I think I can move that patch before this patch, so tdx_features0 is always
introduced in that patch.  Then whether we decide to print tdx_features0 in this
patch doesn't impact that patch.

Patch

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 24eb289c80e8..0fb673dd43ed 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -302,6 +302,55 @@  static int __read_sys_metadata_field(u64 field_id, void *val, int size)
 					&_stbuf->_member);			\
 	})
 
+static int get_tdx_sys_info_features(struct tdx_sys_info_features *sysinfo_features)
+{
+	int ret = 0;
+
+#define TD_SYSINFO_MAP_MOD_FEATURES(_field_id, _member)	\
+	TD_SYSINFO_MAP(_field_id, sysinfo_features, _member)
+
+	TD_SYSINFO_MAP_MOD_FEATURES(TDX_FEATURES0, tdx_features0);
+
+	return ret;
+}
+
+static int get_tdx_sys_info_version(struct tdx_sys_info_version *sysinfo_version)
+{
+	int ret = 0;
+
+#define TD_SYSINFO_MAP_MOD_VERSION(_field_id, _member)	\
+	TD_SYSINFO_MAP(_field_id, sysinfo_version, _member)
+
+	TD_SYSINFO_MAP_MOD_VERSION(MAJOR_VERSION,    major);
+	TD_SYSINFO_MAP_MOD_VERSION(MINOR_VERSION,    minor);
+	TD_SYSINFO_MAP_MOD_VERSION(UPDATE_VERSION,   update);
+	TD_SYSINFO_MAP_MOD_VERSION(INTERNAL_VERSION, internal);
+	TD_SYSINFO_MAP_MOD_VERSION(BUILD_NUM,	     build_num);
+	TD_SYSINFO_MAP_MOD_VERSION(BUILD_DATE,	     build_date);
+
+	return ret;
+}
+
+static void print_basic_sys_info(struct tdx_sys_info *sysinfo)
+{
+	struct tdx_sys_info_features *features = &sysinfo->features;
+	struct tdx_sys_info_version *version = &sysinfo->version;
+
+	/*
+	 * TDX module version encoding:
+	 *
+	 *   <major>.<minor>.<update>.<internal>.<build_num>
+	 *
+	 * When printed as text, <major> and <minor> are 1-digit,
+	 * <update> and <internal> are 2-digits and <build_num>
+	 * is 4-digits.
+	 */
+	pr_info("Initializing TDX module: %u.%u.%02u.%02u.%04u (build_date %u), TDX_FEATURES0 0x%llx\n",
+			version->major, version->minor,	version->update,
+			version->internal, version->build_num,
+			version->build_date, features->tdx_features0);
+}
+
 static int get_tdx_sys_info_tdmr(struct tdx_sys_info_tdmr *sysinfo_tdmr)
 {
 	int ret = 0;
@@ -320,6 +369,16 @@  static int get_tdx_sys_info_tdmr(struct tdx_sys_info_tdmr *sysinfo_tdmr)
 
 static int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
 {
+	int ret;
+
+	ret = get_tdx_sys_info_features(&sysinfo->features);
+	if (ret)
+		return ret;
+
+	ret = get_tdx_sys_info_version(&sysinfo->version);
+	if (ret)
+		return ret;
+
 	return get_tdx_sys_info_tdmr(&sysinfo->tdmr);
 }
 
@@ -1102,6 +1161,8 @@  static int init_tdx_module(void)
 	if (ret)
 		return ret;
 
+	print_basic_sys_info(&sysinfo);
+
 	/*
 	 * To keep things simple, assume that all TDX-protected memory
 	 * will come from the page allocator.  Make sure all pages in the
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index 4cddbb035b9f..b422e8517e01 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -31,6 +31,15 @@ 
  *
  * See the "global_metadata.json" in the "TDX 1.5 ABI definitions".
  */
+#define MD_FIELD_ID_SYS_ATTRIBUTES		0x0A00000200000000ULL
+#define MD_FIELD_ID_TDX_FEATURES0		0x0A00000300000008ULL
+#define MD_FIELD_ID_BUILD_DATE			0x8800000200000001ULL
+#define MD_FIELD_ID_BUILD_NUM			0x8800000100000002ULL
+#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
+
 #define MD_FIELD_ID_MAX_TDMRS			0x9100000100000008ULL
 #define MD_FIELD_ID_MAX_RESERVED_PER_TDMR	0x9100000100000009ULL
 #define MD_FIELD_ID_PAMT_4K_ENTRY_SIZE		0x9100000100000010ULL
@@ -130,6 +139,27 @@  struct tdmr_info_list {
  * those used by the kernel are.
  */
 
+/*
+ * Class "TDX Module Info".
+ *
+ * This class also contains other fields like SYS_ATTRIBUTES and the
+ * NUM_TDX_FEATURES.  For now only TDX_FEATURES0 is needed, but still
+ * keep the structure to follow the spec (and for future extension).
+ */
+struct tdx_sys_info_features {
+	u64 tdx_features0;
+};
+
+/* Class "TDX Module Version" */
+struct tdx_sys_info_version {
+	u16 major;
+	u16 minor;
+	u16 update;
+	u16 internal;
+	u16 build_num;
+	u32 build_date;
+};
+
 /* Class "TDMR info" */
 struct tdx_sys_info_tdmr {
 	u16 max_tdmrs;
@@ -138,7 +168,9 @@  struct tdx_sys_info_tdmr {
 };
 
 struct tdx_sys_info {
-	struct tdx_sys_info_tdmr tdmr;
+	struct tdx_sys_info_features	features;
+	struct tdx_sys_info_version	version;
+	struct tdx_sys_info_tdmr	tdmr;
 };
 
 #endif