Re: [PATCH v1 08/26] x86/virt/tdx: Add tdx_enable_ext() to enable of TDX Module Extensions

[Xu Yilun <yilun.xu@linux.intel.com>]

> >>> --- a/arch/x86/virt/vmx/tdx/tdx.h
> >>> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> >>> @@ -46,6 +46,7 @@
> >>>  #define TDH_PHYMEM_PAGE_WBINVD         41
> >>>  #define TDH_VP_WR                      43
> >>>  #define TDH_SYS_CONFIG                 45
> >>> +#define TDH_SYS_CONFIG_V1              (TDH_SYS_CONFIG | (1ULL << TDX_VERSION_SHIFT))
> >>>
> >>> And if a SEAMCALL needs export, add new tdh_foobar() helper. Anyway
> >>> the parameter list should be different.
> >>
> >> I'd need quite a bit of convincing that this is the right way.
> >>
> >> What is the scenario where there's a:
> >>
> >> 	TDH_SYS_CONFIG_V1
> >> and
> >> 	TDH_SYS_CONFIG_V2
> >>
> >> in the tree at the same time?
> > 
> > I assume you mean TDH_SYS_CONFIG & TDH_SYS_CONFIG_V1.
> 
> Sure. But I wasn't being that literal about it. My point was whether we
> need two macros for two simultaneous uses of the same seamcall.
> 
> > If you want to enable optional features via this seamcall, you must use
> > v1, otherwise v0 & v1 are all good. Mm... I suddenly don't see usecase
> > they must co-exist. Unconditionally use v1 is fine. So does TDH_VP_INIT.
> > 
> > Does that mean we don't have to keep versions, always use the latest is
> > good? (Proper Macro to be used...)
> > 
> >  -#define TDH_SYS_CONFIG                 45
> >  +#define TDH_SYS_CONFIG                 (45 | (1ULL << TDX_VERSION_SHIFT))
> 
> That's my theory: we don't need to keep versions.

Sorry, I found we have to keep versions for backward compatibility. The
old TDX Modules reject v1.

Here is my change, including TDH_SYS_CONFIG & TDH_VP_INIT. Will break
them down in formal patches.

-----8<-------

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 3b4d7cb25164..1e0a174dfb57 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1330,6 +1330,7 @@ static __init int config_tdx_module(struct tdmr_info_list *tdmr_list,
                                    u64 global_keyid)
 {
        struct tdx_module_args args = {};
+       u64 seamcall_fn = TDH_SYS_CONFIG;
        u64 *tdmr_pa_array;
        size_t array_sz;
        int i, ret;
@@ -1354,7 +1355,14 @@ static __init int config_tdx_module(struct tdmr_info_list *tdmr_list,
        args.rcx = __pa(tdmr_pa_array);
        args.rdx = tdmr_list->nr_consumed_tdmrs;
        args.r8 = global_keyid;
-       ret = seamcall_prerr(TDH_SYS_CONFIG, &args);
+
+       if (tdx_sysinfo.features.tdx_features0 & TDX_FEATURES0_TDXCONNECT) {
+               args.r9 |= TDX_FEATURES0_TDXCONNECT;
+               args.r11 = ktime_get_real_seconds();
+               seamcall_fn = TDH_SYS_CONFIG_V1;
+       }
+
+       ret = seamcall_prerr(seamcall_fn, &args);

        /* Free the array as it is not required anymore. */
        kfree(tdmr_pa_array);
@@ -2153,7 +2166,7 @@ u64 tdh_vp_init(struct tdx_vp *vp, u64 initial_rcx, u32 x2apicid)
        };

        /* apicid requires version == 1. */
-       return seamcall(TDH_VP_INIT | (1ULL << TDX_VERSION_SHIFT), &args);
+       return seamcall(TDH_VP_INIT_V1, &args);
 }
 EXPORT_SYMBOL_GPL(tdh_vp_init);

diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index 4370d3d177f6..835ea2f08fe2 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -2,6 +2,7 @@
 #ifndef _X86_VIRT_TDX_H
 #define _X86_VIRT_TDX_H

+#include <linux/bitfield.h>
 #include <linux/bits.h>

 /*
@@ -11,6 +12,18 @@
  * architectural definitions come first.
  */

+/*
+ * SEAMCALL leaf:
+ *
+ * Bit 15:0    Leaf number
+ * Bit 23:16   Version number
+ */
+#define SEAMCALL_LEAF                  GENMASK(15, 0)
+#define SEAMCALL_VER                   GENMASK(23, 16)
+
+#define SEAMCALL_LEAF_VER(l, v)                (FIELD_PREP(SEAMCALL_LEAF, l) | \
+                                        FIELD_PREP(SEAMCALL_VER, v))
+
 /*
  * TDX module SEAMCALL leaf functions
  */
@@ -31,7 +44,7 @@
 #define TDH_VP_CREATE                  10
 #define TDH_MNG_KEY_FREEID             20
 #define TDH_MNG_INIT                   21
-#define TDH_VP_INIT                    22
+#define TDH_VP_INIT_V1                 SEAMCALL_LEAF_VER(22, 1)
 #define TDH_PHYMEM_PAGE_RDMD           24
 #define TDH_VP_RD                      26
 #define TDH_PHYMEM_PAGE_RECLAIM                28
@@ -46,14 +59,7 @@
 #define TDH_PHYMEM_PAGE_WBINVD         41
 #define TDH_VP_WR                      43
 #define TDH_SYS_CONFIG                 45
-
-/*
- * SEAMCALL leaf:
- *
- * Bit 15:0    Leaf number
- * Bit 23:16   Version number
- */
-#define TDX_VERSION_SHIFT              16
+#define TDH_SYS_CONFIG_V1              SEAMCALL_LEAF_VER(TDH_SYS_CONFIG, 1)

 /* TDX page types */
 #define        PT_NDA          0x0