[RFC PATCH 0/6] SEAMCALL Wrappers

[RFC PATCH 0/6] SEAMCALL Wrappers

[Rick Edgecombe]

Hi,

This is a quick followup to Dave’s comments on the SEAMCALL wrappers in 
the “TDX vCPU/VM creation” series [0]. To try to summarize Dave’s 
comments, he noted that the SEAMCALL wrappers were very thin, not even 
using proper types for things where types exist, like pages. There were a 
couple directions discussed for passing the pages that are handed to TDX 
module for it to use to manage a TD state:
 - Using virtual addresses to host mappings for the page
 - Using structs for each type of physical page, such that there could be
   type checking on all of the similar tdXYZ TDX page types.
 - Pulling in “linux/kvm_types.h” in arch/x86 code to handle types 
   (especially since the later MMU SEAMCALLs take GFN arguments)

There was also some repeated points made that the argument names could use 
some semantic clarity, including possible creating enums for out args.

But overall, I interpreted there to be a wish for the wrappers to do 
something a little more useful than use the EXPORT infrastructure as an 
allow list for approved SEAMCALLs.

I first played around with basically keeping the existing design and using 
KVM’s hpa_t, etc for the types. This really didn’t add much improvement. 
Using virtual addresses simplified some of the code on the KVM side, but 
contrasted with KVM code style that is used to handling physical addresses 
for most things. It also was weird considering that these pages are 
encrypted and can’t be used from the kernel.

The solution in this RFC
------------------------
As we discussed on that series, the SEAMCALL wrappers used to take the KVM 
defined structs that represent TDs and vCPUs, instead of raw tdXYZ page 
references. This was pretty handy and good for avoiding passing the wrong 
type of TDX tdXYZ page, but these structs have a bunch of other stuff that 
is specific to KVM. We don’t want to leak that stuff outside of KVM. But I 
think it is ok for the arch/x86 code to know about TDX arch specific things
that VMMs are generally required to have to manage. So this RFC creates
two structs that represent TDs and vCPUs. They hold references to the tdXYZ
pages that are provided to the TDX module and associated with there 
respective architectural concept (TD and vCPU):
	struct tdx_td {
		hpa_t tdr;
		hpa_t *tdcs;
	};

	struct tdx_vp {
		hpa_t tdvpr;
		hpa_t *tdcx;
	};

I used hpa_t based on that it is commonly used to hold physical addresses 
in KVM, including similar kernel allocated memory like TDP root pages. So 
I thought it fit KVM better, stylistically. It was my best attempt at 
guessing what KVM maintainers would like. Other options could be 
kvm_pfn_t. Or just struct page.

They are passed into the SEAMCALL wrappers like:
	u64 tdh_mng_vpflushdone(struct tdx_td *td)

These new structs are then placed inside KVM’s structs that track TDX 
state for the same concept, where previously those KVM structs held the 
references to the pages directly, like:
struct kvm_tdx {
	struct kvm kvm;

-	u64 tdr;
-	u64 *tdcs;
+	struct tdx_td td;
	...
};

It does get a bit nested though, for example:
	kvm->kvm_arch->(container_of)kvm_tdx->tdx_td->tdr
...but the actual final dereferences in the code don’t need to go that
deep, and look like:
	err = tdh_mng_vpflushdone(&kvm_tdx->td);

Overall, it's not a huge change, but does give the arch/x86 a little more
purpose. Please let me know what you think.

There also was a suggestion from Dave to create a helper to hold a comment 
on the “CLFLUSH_BEFORE_ALLOC” reasoning. This is implemented in this RFC.

Separate from discussions with Dave on the SEAMCALLs, there was some some 
suggestions on how we might remove or combine specific SEAMCALLs. I didn’t 
try this here, because this RFC is more about exploring in general how we 
want to distribute things between KVM and arch/x86 for these SEAMCALL 
wrappers.

So in summary the RFC only has:
 - Use structs to hold tdXYZ fields for TD and vCPUs
 - Make helper to hold CLFLUSH_BEFORE_ALLOC comments
 - Use semantic names for out args
 - (Add Kai's sign-off that should have been in the last version)
  
Patches 1 and 3 contain new commit log verbiage justifying specific design
choices behind the struct definitions.

I didn’t create enums for the out args. Just using proper names for the 
args seemed like a good balance between code clarity and not 
over-engineering. But please correct if this was the wrong judgment.

Here is a branch for seeing the callers. I didn’t squash the caller 
changes into the patches yet either, the caller changes are all just in the
HEAD commit. I also only converted the “VM/vCPU creation” SEAMCALLs to the
approach described above:
https://github.com/intel/tdx/tree/seamcall-rfc

[0] https://lore.kernel.org/kvm/20241030190039.77971-1-rick.p.edgecombe@intel.com/


Rick Edgecombe (6):
  x86/virt/tdx: Add SEAMCALL wrappers for TDX KeyID management
  x86/virt/tdx: Add SEAMCALL wrappers for TDX TD creation
  x86/virt/tdx: Add SEAMCALL wrappers for TDX vCPU creation
  x86/virt/tdx: Add SEAMCALL wrappers for TDX page cache management
  x86/virt/tdx: Add SEAMCALL wrappers for TDX VM/vCPU field access
  x86/virt/tdx: Add SEAMCALL wrappers for TDX flush operations

 arch/x86/include/asm/tdx.h  |  29 +++++
 arch/x86/virt/vmx/tdx/tdx.c | 224 ++++++++++++++++++++++++++++++++++++
 arch/x86/virt/vmx/tdx/tdx.h |  38 ++++--
 3 files changed, 284 insertions(+), 7 deletions(-)

-- 
2.47.0

[RFC PATCH 1/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX KeyID management

[Rick Edgecombe]

Intel TDX protects guest VMs from malicious host and certain physical
attacks. Pre-TDX Intel hardware has support for a memory encryption
architecture called MK-TME, which repurposes several high bits of
physical address as "KeyID". TDX ends up with reserving a sub-range of
MK-TME KeyIDs as "TDX private KeyIDs".

Like MK-TME, these KeyIDs can be associated with an ephemeral key. For TDX
this association is done by the TDX module. It also has its own tracking
for which KeyIDs are in use. To do this ephemeral key setup and manipulate
the TDX module's internal tracking, KVM will use the following SEAMCALLs:
 TDH.MNG.KEY.CONFIG: Mark the KeyID as in use, and initialize its
                     ephemeral key.
 TDH.MNG.KEY.FREEID: Mark the KeyID as not in use.

These SEAMCALLs both operate on TDR structures, which are setup using the
previously added TDH.MNG.CREATE SEAMCALL. KVM's use of these operations
will go like:
 - tdx_guest_keyid_alloc()
 - Initialize TD and TDR page with TDH.MNG.CREATE (not yet-added), passing
   KeyID
 - TDH.MNG.KEY.CONFIG to initialize the key
 - TD runs, teardown is started
 - TDH.MNG.KEY.FREEID
 - tdx_guest_keyid_free()

Don't try to combine the tdx_guest_keyid_alloc() and TDH.MNG.KEY.CONFIG
operations because TDH.MNG.CREATE and some locking need to be done in the
middle. Don't combine TDH.MNG.KEY.FREEID and tdx_guest_keyid_free() so they
are symmetrical with the creation path.

So implement tdh_mng_key_config() and tdh_mng_key_freeid() as separate
functions than tdx_guest_keyid_alloc() and tdx_guest_keyid_free().

The TDX module provides SEAMCALLs to hand pages to the TDX module for
storing TDX controlled state. SEAMCALLs that operate on this state are
directed to the appropriate TD VM using references to the pages originally
provided for managing the TD's state. So the host kernel needs to track
these pages, both as an ID for specifying which TD to operate on, and to
allow them to be eventually reclaimed. The TD VM associated pages are
called TDR (Trust Domain Root) and TDCS (Trust Domain Control Structure).

Introduce "struct tdx_td" for holding references to pages provided to the
TDX module for this TD VM associated state. Don't plan for any TD
associated state that is controlled by KVM to live in this struct. Only
expect it to hold data for concepts specific to the TDX architecture, for
which there can't already be preexisting storage for in KVM.

Add both the TDR page and an array of TDCS pages, even though the SEAMCALL
wrappers will only need to know about the TDR pages for directing the
SEAMCALLs to the right TD. Adding the TDCS pages to this struct will let
all of the TD VM associated pages handed to the TDX module be tracked in
one location. For a type to specify physical pages, use KVM's hpa_t type.
Do this for KVM's benefit This is the common type used to hold physical
addresses in KVM, so will make interoperability easier.

Co-developed-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Signed-off-by: Kai Huang <kai.huang@intel.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Reviewed-by: Yuan Yao <yuan.yao@intel.com>
---
SEAMCALL RFC:
 - Introduce struct tdx_td to use instead of raw TDR u64

uAPI breakout v2:
 - Change to use 'u64' as function parameter to prepare to move
   SEAMCALL wrappers to arch/x86. (Kai)
 - Split to separate patch
 - Move SEAMCALL wrappers from KVM to x86 core;
 - Move TDH_xx macros from KVM to x86 core;
 - Re-write log

uAPI breakout v1:
 - Make argument to C wrapper function struct kvm_tdx * or
   struct vcpu_tdx * .(Sean)
 - Drop unused helpers (Kai)
 - Fix bisectability issues in headers (Kai)
 - Updates from seamcall overhaul (Kai)

v19:
 - Update the commit message to match the patch by Yuan
 - Use seamcall() and seamcall_ret() by paolo

v18:
 - removed stub functions for __seamcall{,_ret}()
 - Added Reviewed-by Binbin
 - Make tdx_seamcall() use struct tdx_module_args instead of taking
   each inputs.
---
 arch/x86/include/asm/tdx.h  |  9 +++++++++
 arch/x86/virt/vmx/tdx/tdx.c | 22 ++++++++++++++++++++++
 arch/x86/virt/vmx/tdx/tdx.h | 16 +++++++++-------
 3 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index d33e46d53d59..ebee4260545f 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -34,6 +34,7 @@
 
 #include <uapi/asm/mce.h>
 #include "tdx_global_metadata.h"
+#include <linux/kvm_types.h>
 
 /*
  * Used by the #VE exception handler to gather the #VE exception
@@ -121,6 +122,14 @@ const struct tdx_sys_info *tdx_get_sysinfo(void);
 
 int tdx_guest_keyid_alloc(void);
 void tdx_guest_keyid_free(unsigned int keyid);
+
+struct tdx_td {
+	hpa_t tdr;
+	hpa_t *tdcs;
+};
+
+u64 tdh_mng_key_config(struct tdx_td *td);
+u64 tdh_mng_key_freeid(struct tdx_td *td);
 #else
 static inline void tdx_init(void) { }
 static inline int tdx_cpu_enable(void) { return -ENODEV; }
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index b883c1a4b002..20eb756b41de 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1562,3 +1562,25 @@ void tdx_guest_keyid_free(unsigned int keyid)
 	ida_free(&tdx_guest_keyid_pool, keyid);
 }
 EXPORT_SYMBOL_GPL(tdx_guest_keyid_free);
+
+u64 tdh_mng_key_config(struct tdx_td *td)
+{
+	struct tdx_module_args args = {
+		.rcx = td->tdr,
+	};
+
+	return seamcall(TDH_MNG_KEY_CONFIG, &args);
+}
+EXPORT_SYMBOL_GPL(tdh_mng_key_config);
+
+
+u64 tdh_mng_key_freeid(struct tdx_td *td)
+{
+	struct tdx_module_args args = {
+		.rcx = td->tdr,
+	};
+
+	return seamcall(TDH_MNG_KEY_FREEID, &args);
+}
+EXPORT_SYMBOL_GPL(tdh_mng_key_freeid);
+
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index 9b708a8fb568..95002e7ff4c5 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -17,13 +17,15 @@
 /*
  * TDX module SEAMCALL leaf functions
  */
-#define TDH_PHYMEM_PAGE_RDMD	24
-#define TDH_SYS_KEY_CONFIG	31
-#define TDH_SYS_INIT		33
-#define TDH_SYS_RD		34
-#define TDH_SYS_LP_INIT		35
-#define TDH_SYS_TDMR_INIT	36
-#define TDH_SYS_CONFIG		45
+#define TDH_MNG_KEY_CONFIG		8
+#define TDH_MNG_KEY_FREEID		20
+#define TDH_PHYMEM_PAGE_RDMD		24
+#define TDH_SYS_KEY_CONFIG		31
+#define TDH_SYS_INIT			33
+#define TDH_SYS_RD			34
+#define TDH_SYS_LP_INIT			35
+#define TDH_SYS_TDMR_INIT		36
+#define TDH_SYS_CONFIG			45
 
 /* TDX page types */
 #define	PT_NDA		0x0
-- 
2.47.0

[RFC PATCH 2/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX TD creation

[Rick Edgecombe]

Intel TDX protects guest VMs from malicious hosts and certain physical
attacks. It defines various control structures that hold state for things
like TDs or vCPUs. These control structures are stored in pages given to
the TDX module and encrypted with either the global KeyID or the guest
KeyIDs.

To manipulate these control structures the TDX module defines a few
SEAMCALLs. KVM will use these during the process of creating a TD as
follows:

1) Allocate a unique TDX KeyID for a new guest.

1) Call TDH.MNG.CREATE to create a "TD Root" (TDR) page, together with
   the new allocated KeyID. Unlike the rest of the TDX guest, the TDR
   page is crypto-protected by the 'global KeyID'.

2) Call the previously added TDH.MNG.KEY.CONFIG on each package to
   configure the KeyID for the guest. After this step, the KeyID to
   protect the guest is ready and the rest of the guest will be protected
   by this KeyID.

3) Call TDH.MNG.ADDCX to add TD Control Structure (TDCS) pages.

4) Call TDH.MNG.INIT to initialize the TDCS.

To reclaim these pages for use by the kernel other SEAMCALLs are needed,
which will be added in future patches.

Add tdh_mng_addcx(), tdh_mng_create() and tdh_mng_init() to export these
SEAMCALLs so that KVM can use them to create TDs.

For SEAMCALLs that give a page to the TDX module to be encrypted, CLFLUSH
the page mapped with KeyID 0, such that any dirty cache lines don't write
back later and clobber TD memory or control structures. Don't worry about
the other MK-TME KeyIDs because the kernel doesn't use them. The TDX docs
specify that this flush is not needed unless the TDX module exposes the
CLFLUSH_BEFORE_ALLOC feature bit. Be conservative and always flush. Add a
helper function to facilitate this.

Co-developed-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Signed-off-by: Kai Huang <kai.huang@intel.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Reviewed-by: Yuan Yao <yuan.yao@intel.com>
---
SEAMCALL RFC:
 - Use struct tdx_td
 - Introduce tdx_clflush_page() to hold CLFLUSH_BEFORE_ALLOC
   explanation

uAPI breakout v2:
 - Change to use 'u64' as function parameter to prepare to move
   SEAMCALL wrappers to arch/x86. (Kai)
 - Split to separate patch
 - Move SEAMCALL wrappers from KVM to x86 core;
 - Move TDH_xx macros from KVM to x86 core;
 - Re-write log

uAPI breakout v1:
 - Make argument to C wrapper function struct kvm_tdx * or
   struct vcpu_tdx * .(Sean)
 - Drop unused helpers (Kai)
 - Fix bisectability issues in headers (Kai)
 - Updates from seamcall overhaul (Kai)

v19:
 - Update the commit message to match the patch by Yuan
 - Use seamcall() and seamcall_ret() by paolo

v18:
 - removed stub functions for __seamcall{,_ret}()
 - Added Reviewed-by Binbin
 - Make tdx_seamcall() use struct tdx_module_args instead of taking
   each inputs.
---
 arch/x86/include/asm/tdx.h  |  3 +++
 arch/x86/virt/vmx/tdx/tdx.c | 51 +++++++++++++++++++++++++++++++++++++
 arch/x86/virt/vmx/tdx/tdx.h |  3 +++
 3 files changed, 57 insertions(+)

diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index ebee4260545f..4c4d092b7c8e 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -128,8 +128,11 @@ struct tdx_td {
 	hpa_t *tdcs;
 };
 
+u64 tdh_mng_addcx(struct tdx_td *td, hpa_t tdcs);
 u64 tdh_mng_key_config(struct tdx_td *td);
+u64 tdh_mng_create(struct tdx_td *td, hpa_t hkid);
 u64 tdh_mng_key_freeid(struct tdx_td *td);
+u64 tdh_mng_init(struct tdx_td *td, u64 td_params, hpa_t *tdr);
 #else
 static inline void tdx_init(void) { }
 static inline int tdx_cpu_enable(void) { return -ENODEV; }
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 20eb756b41de..311f8d85e18d 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1563,6 +1563,29 @@ void tdx_guest_keyid_free(unsigned int keyid)
 }
 EXPORT_SYMBOL_GPL(tdx_guest_keyid_free);
 
+/*
+ * The TDX module exposes a CLFLUSH_BEFORE_ALLOC bit to specify whether
+ * a CLFLUSH of pages is required before handing them to the TDX module.
+ * Be conservative and make the code simpler by doing the CLFLUSH
+ * unconditionally.
+ */
+static void tdx_clflush_page(hpa_t tdr)
+{
+	clflush_cache_range(__va(tdr), PAGE_SIZE);
+}
+
+u64 tdh_mng_addcx(struct tdx_td *td, hpa_t tdcs)
+{
+	struct tdx_module_args args = {
+		.rcx = tdcs,
+		.rdx = td->tdr,
+	};
+
+	tdx_clflush_page(tdcs);
+	return seamcall(TDH_MNG_ADDCX, &args);
+}
+EXPORT_SYMBOL_GPL(tdh_mng_addcx);
+
 u64 tdh_mng_key_config(struct tdx_td *td)
 {
 	struct tdx_module_args args = {
@@ -1573,6 +1596,18 @@ u64 tdh_mng_key_config(struct tdx_td *td)
 }
 EXPORT_SYMBOL_GPL(tdh_mng_key_config);
 
+u64 tdh_mng_create(struct tdx_td *td, hpa_t hkid)
+{
+	struct tdx_module_args args = {
+		.rcx = td->tdr,
+		.rdx = hkid,
+	};
+
+	tdx_clflush_page(td->tdr);
+	return seamcall(TDH_MNG_CREATE, &args);
+}
+EXPORT_SYMBOL_GPL(tdh_mng_create);
+
 
 u64 tdh_mng_key_freeid(struct tdx_td *td)
 {
@@ -1584,3 +1619,19 @@ u64 tdh_mng_key_freeid(struct tdx_td *td)
 }
 EXPORT_SYMBOL_GPL(tdh_mng_key_freeid);
 
+u64 tdh_mng_init(struct tdx_td *td, u64 td_params, hpa_t *tdr)
+{
+	struct tdx_module_args args = {
+		.rcx = td->tdr,
+		.rdx = td_params,
+	};
+	u64 ret;
+
+	ret = seamcall_ret(TDH_MNG_INIT, &args);
+
+	*tdr = args.rcx;
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(tdh_mng_init);
+
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index 95002e7ff4c5..b9287304f372 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -17,8 +17,11 @@
 /*
  * TDX module SEAMCALL leaf functions
  */
+#define TDH_MNG_ADDCX			1
 #define TDH_MNG_KEY_CONFIG		8
+#define TDH_MNG_CREATE			9
 #define TDH_MNG_KEY_FREEID		20
+#define TDH_MNG_INIT			21
 #define TDH_PHYMEM_PAGE_RDMD		24
 #define TDH_SYS_KEY_CONFIG		31
 #define TDH_SYS_INIT			33
-- 
2.47.0

[RFC PATCH 3/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX vCPU creation

[Rick Edgecombe]

Intel TDX protects guest VMs from malicious host and certain physical
attacks. It defines various control structures that hold state for
virtualized components of the TD (i.e. VMs or vCPUs) These control
structures are stored in pages given to the TDX module and encrypted
with either the global KeyID or the guest KeyIDs.

To manipulate these control structures the TDX module defines a few
SEAMCALLs. KVM will use these during the process of creating a vCPU as
follows:

1) Call TDH.VP.CREATE to create a TD vCPU Root (TDVPR) page for each
   vCPU.

2) Call TDH.VP.ADDCX to add per-vCPU control pages (TDCX) for each vCPU.

3) Call TDH.VP.INIT to initialize the TDCX for each vCPU.

To reclaim these pages for use by the kernel other SEAMCALLs are needed,
which will be added in future patches.

Export functions to allow KVM to make these SEAMCALLs. Export two
variants for TDH.VP.CREATE, in order to support the planned logic of KVM
to support TDX modules with and without the ENUM_TOPOLOGY feature. If
KVM can drop support for the !ENUM_TOPOLOGY case, this could go down a
single version. Leave that for later discussion.

The TDX module provides SEAMCALLs to hand pages to the TDX module for
storing TDX controlled state. SEAMCALLs that operate on this state are
directed to the appropriate TD vCPU using references to the pages
originally provided for managing the vCPU's state. So the host kernel
needs to track these pages, both as an ID for specifying which vCPU to
operate on, and to allow them to be eventually reclaimed. The vCPU
associated pages are called TDVPR (Trust Domain Virtual Processor Root)
and TDCX (Trust Domain Control Extension).

Introduce "struct tdx_vp" for holding references to pages provided to the
TDX module for the TD vCPU associated state. Don't plan for any vCPU
associated state that is controlled by KVM to live in this struct. Only
expect it to hold data for concepts specific to the TDX architecture, for
which there can't already be preexisting storage for in KVM.

Add both the TDVPR page and an array of TDCX pages, even though the
SEAMCALL wrappers will only need to know about the TDVPR pages for
directing the SEAMCALLs to the right vCPU. Adding the TDCX pages to this
struct will let all of the vCPU associated pages handed to the TDX module be
tracked in one location. For a type to specify physical pages, use KVM's
hpa_t type. Do this for KVM's benefit This is the common type used to hold
physical addresses in KVM, so will make interoperability easier.

Co-developed-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Signed-off-by: Kai Huang <kai.huang@intel.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Reviewed-by: Yuan Yao <yuan.yao@intel.com>
---
SEAMCALL RFC:
 - Use struct tdx_td
 - Introduce tdx_vp

uAPI breakout v2:
 - Change to use 'u64' as function parameter to prepare to move
   SEAMCALL wrappers to arch/x86. (Kai)
 - Split to separate patch
 - Move SEAMCALL wrappers from KVM to x86 core;
 - Move TDH_xx macros from KVM to x86 core;
 - Re-write log

uAPI breakout v1:
 - Make argument to C wrapper function struct kvm_tdx * or
   struct vcpu_tdx * .(Sean)
 - Drop unused helpers (Kai)
 - Fix bisectability issues in headers (Kai)
 - Updates from seamcall overhaul (Kai)

v19:
 - Update the commit message to match the patch by Yuan
 - Use seamcall() and seamcall_ret() by paolo

v18:
 - removed stub functions for __seamcall{,_ret}()
 - Added Reviewed-by Binbin
 - Make tdx_seamcall() use struct tdx_module_args instead of taking
  each inputs.
---
 arch/x86/include/asm/tdx.h  |  9 ++++++++
 arch/x86/virt/vmx/tdx/tdx.c | 46 +++++++++++++++++++++++++++++++++++++
 arch/x86/virt/vmx/tdx/tdx.h | 11 +++++++++
 3 files changed, 66 insertions(+)

diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 4c4d092b7c8e..83aa2a8a56d3 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -128,11 +128,20 @@ struct tdx_td {
 	hpa_t *tdcs;
 };
 
+struct tdx_vp {
+	hpa_t tdvpr;
+	hpa_t *tdcx;
+};
+
 u64 tdh_mng_addcx(struct tdx_td *td, hpa_t tdcs);
+u64 tdh_vp_addcx(struct tdx_vp *vp, hpa_t tdcx);
 u64 tdh_mng_key_config(struct tdx_td *td);
 u64 tdh_mng_create(struct tdx_td *td, hpa_t hkid);
+u64 tdh_vp_create(struct tdx_td *td, struct tdx_vp *vp);
 u64 tdh_mng_key_freeid(struct tdx_td *td);
 u64 tdh_mng_init(struct tdx_td *td, u64 td_params, hpa_t *tdr);
+u64 tdh_vp_init(struct tdx_vp *vp, u64 initial_rcx);
+u64 tdh_vp_init_apicid(struct tdx_vp *vp, u64 initial_rcx, u32 x2apicid);
 #else
 static inline void tdx_init(void) { }
 static inline int tdx_cpu_enable(void) { return -ENODEV; }
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 311f8d85e18d..c125b1519072 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1586,6 +1586,18 @@ u64 tdh_mng_addcx(struct tdx_td *td, hpa_t tdcs)
 }
 EXPORT_SYMBOL_GPL(tdh_mng_addcx);
 
+u64 tdh_vp_addcx(struct tdx_vp *vp, hpa_t tdcx)
+{
+	struct tdx_module_args args = {
+		.rcx = tdcx,
+		.rdx = vp->tdvpr,
+	};
+
+	tdx_clflush_page(tdcx);
+	return seamcall(TDH_VP_ADDCX, &args);
+}
+EXPORT_SYMBOL_GPL(tdh_vp_addcx);
+
 u64 tdh_mng_key_config(struct tdx_td *td)
 {
 	struct tdx_module_args args = {
@@ -1608,6 +1620,17 @@ u64 tdh_mng_create(struct tdx_td *td, hpa_t hkid)
 }
 EXPORT_SYMBOL_GPL(tdh_mng_create);
 
+u64 tdh_vp_create(struct tdx_td *td, struct tdx_vp *vp)
+{
+	struct tdx_module_args args = {
+		.rcx = vp->tdvpr,
+		.rdx = td->tdr,
+	};
+
+	tdx_clflush_page(vp->tdvpr);
+	return seamcall(TDH_VP_CREATE, &args);
+}
+EXPORT_SYMBOL_GPL(tdh_vp_create);
 
 u64 tdh_mng_key_freeid(struct tdx_td *td)
 {
@@ -1635,3 +1658,26 @@ u64 tdh_mng_init(struct tdx_td *td, u64 td_params, hpa_t *tdr)
 }
 EXPORT_SYMBOL_GPL(tdh_mng_init);
 
+u64 tdh_vp_init(struct tdx_vp *vp, u64 initial_rcx)
+{
+	struct tdx_module_args args = {
+		.rcx = vp->tdvpr,
+		.rdx = initial_rcx,
+	};
+
+	return seamcall(TDH_VP_INIT, &args);
+}
+EXPORT_SYMBOL_GPL(tdh_vp_init);
+
+u64 tdh_vp_init_apicid(struct tdx_vp *vp, u64 initial_rcx, u32 x2apicid)
+{
+	struct tdx_module_args args = {
+		.rcx = vp->tdvpr,
+		.rdx = initial_rcx,
+		.r8 = x2apicid,
+	};
+
+	/* apicid requires version == 1. */
+	return seamcall(TDH_VP_INIT | (1ULL << TDX_VERSION_SHIFT), &args);
+}
+EXPORT_SYMBOL_GPL(tdh_vp_init_apicid);
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index b9287304f372..3663971a3669 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -18,10 +18,13 @@
  * TDX module SEAMCALL leaf functions
  */
 #define TDH_MNG_ADDCX			1
+#define TDH_VP_ADDCX			4
 #define TDH_MNG_KEY_CONFIG		8
 #define TDH_MNG_CREATE			9
+#define TDH_VP_CREATE			10
 #define TDH_MNG_KEY_FREEID		20
 #define TDH_MNG_INIT			21
+#define TDH_VP_INIT			22
 #define TDH_PHYMEM_PAGE_RDMD		24
 #define TDH_SYS_KEY_CONFIG		31
 #define TDH_SYS_INIT			33
@@ -30,6 +33,14 @@
 #define TDH_SYS_TDMR_INIT		36
 #define TDH_SYS_CONFIG			45
 
+/*
+ * SEAMCALL leaf:
+ *
+ * Bit 15:0	Leaf number
+ * Bit 23:16	Version number
+ */
+#define TDX_VERSION_SHIFT		16
+
 /* TDX page types */
 #define	PT_NDA		0x0
 #define	PT_RSVD		0x1
-- 
2.47.0

[RFC PATCH 4/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX page cache management

[Rick Edgecombe]

Intel TDX protects guest VMs from malicious host and certain physical
attacks. The TDX module uses pages provided by the host for both control
structures and for TD guest pages. These pages are encrypted using the
MK-TME encryption engine, with its special requirements around cache
invalidation. For its own security, the TDX module ensures pages are
flushed properly and track which usage they are currently assigned. For
creating and tearing down TD VMs and vCPUs KVM will need to use the
TDH.PHYMEM.PAGE.RECLAIM, TDH.PHYMEM.CACHE.WB, and TDH.PHYMEM.PAGE.WBINVD
SEAMCALLs.

Add tdh_phymem_page_reclaim() to enable KVM to call
TDH.PHYMEM.PAGE.RECLAIM to reclaim the page for use by the host kernel.
This effectively resets its state in the TDX module's page tracking
(PAMT), if the page is available to be reclaimed. This will be used by KVM
to reclaim the various types of pages owned by the TDX module. It will
have a small wrapper in KVM that retries in the case of a relevant error
code. Don't implement this wrapper in arch/x86 because KVM's solution
around retrying SEAMCALLs will be better located in a single place.

Add tdh_phymem_cache_wb() to enable KVM to call TDH.PHYMEM.CACHE.WB to do
a cache write back in a way that the TDX module can verify, before it
allows a KeyID to be freed. The KVM code will use this to have a small
wrapper that handles retries. Since the TDH.PHYMEM.CACHE.WB operation is
interruptible, have tdh_phymem_cache_wb() take a resume argument to pass
this info to the TDX module for restarts. It is worth noting that this
SEAMCALL uses a SEAM specific MSR to do the write back in sections. In
this way it does export some new functionality that affects CPU state.

Add tdh_phymem_page_wbinvd_tdr() to enable KVM to call
TDH.PHYMEM.PAGE.WBINVD to do a cache write back and invalidate of a TDR,
using the global KeyID. The underlying TDH.PHYMEM.PAGE.WBINVD SEAMCALL
requires the related KeyID to be encoded into the SEAMCALL args. Since the
global KeyID is not exposed to KVM, a dedicated wrapper is needed for TDR
focused TDH.PHYMEM.PAGE.WBINVD operations.

Co-developed-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Signed-off-by: Kai Huang <kai.huang@intel.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Reviewed-by: Yuan Yao <yuan.yao@intel.com>
---
SEAMCALL RFC:
 - Use struct tdx_td
 - Use arg names with meaning for tdh_phymem_page_reclaim() for out args

uAPI breakout v2:
 - Change to use 'u64' as function parameter to prepare to move
   SEAMCALL wrappers to arch/x86. (Kai)
 - Split to separate patch
 - Move SEAMCALL wrappers from KVM to x86 core;
 - Move TDH_xx macros from KVM to x86 core;
 - Re-write log

uAPI breakout v1:
 - Make argument to C wrapper function struct kvm_tdx * or
   struct vcpu_tdx * .(Sean)
 - Drop unused helpers (Kai)
 - Fix bisectability issues in headers (Kai)
 - Updates from seamcall overhaul (Kai)

v19:
 - Update the commit message to match the patch by Yuan
 - Use seamcall() and seamcall_ret() by paolo

v18:
 - removed stub functions for __seamcall{,_ret}()
 - Added Reviewed-by Binbin
 - Make tdx_seamcall() use struct tdx_module_args instead of taking
   each inputs.
---
 arch/x86/include/asm/tdx.h  |  3 +++
 arch/x86/virt/vmx/tdx/tdx.c | 38 +++++++++++++++++++++++++++++++++++++
 arch/x86/virt/vmx/tdx/tdx.h |  3 +++
 3 files changed, 44 insertions(+)

diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 83aa2a8a56d3..72de04dc242e 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -142,6 +142,9 @@ u64 tdh_mng_key_freeid(struct tdx_td *td);
 u64 tdh_mng_init(struct tdx_td *td, u64 td_params, hpa_t *tdr);
 u64 tdh_vp_init(struct tdx_vp *vp, u64 initial_rcx);
 u64 tdh_vp_init_apicid(struct tdx_vp *vp, u64 initial_rcx, u32 x2apicid);
+u64 tdh_phymem_page_reclaim(hpa_t page, u64 *page_type, u64 *page_owner, u64 *page_size);
+u64 tdh_phymem_cache_wb(bool resume);
+u64 tdh_phymem_page_wbinvd_tdr(struct tdx_td *td);
 #else
 static inline void tdx_init(void) { }
 static inline int tdx_cpu_enable(void) { return -ENODEV; }
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index c125b1519072..6f833d816899 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1681,3 +1681,41 @@ u64 tdh_vp_init_apicid(struct tdx_vp *vp, u64 initial_rcx, u32 x2apicid)
 	return seamcall(TDH_VP_INIT | (1ULL << TDX_VERSION_SHIFT), &args);
 }
 EXPORT_SYMBOL_GPL(tdh_vp_init_apicid);
+
+u64 tdh_phymem_page_reclaim(hpa_t page, u64 *page_type, u64 *page_owner, u64 *page_size)
+{
+	struct tdx_module_args args = {
+		.rcx = page,
+	};
+	u64 ret;
+
+	ret = seamcall_ret(TDH_PHYMEM_PAGE_RECLAIM, &args);
+
+	*page_type = args.rcx;
+	*page_owner = args.rdx;
+	*page_size = args.r8;
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(tdh_phymem_page_reclaim);
+
+u64 tdh_phymem_cache_wb(bool resume)
+{
+	struct tdx_module_args args = {
+		.rcx = resume ? 1 : 0,
+	};
+
+	return seamcall(TDH_PHYMEM_CACHE_WB, &args);
+}
+EXPORT_SYMBOL_GPL(tdh_phymem_cache_wb);
+
+u64 tdh_phymem_page_wbinvd_tdr(struct tdx_td *td)
+{
+	struct tdx_module_args args = {};
+
+	args.rcx = td->tdr | ((u64)tdx_global_keyid << boot_cpu_data.x86_phys_bits);
+
+	return seamcall(TDH_PHYMEM_PAGE_WBINVD, &args);
+}
+EXPORT_SYMBOL_GPL(tdh_phymem_page_wbinvd_tdr);
+
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index 3663971a3669..191bdd1e571d 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -26,11 +26,14 @@
 #define TDH_MNG_INIT			21
 #define TDH_VP_INIT			22
 #define TDH_PHYMEM_PAGE_RDMD		24
+#define TDH_PHYMEM_PAGE_RECLAIM		28
 #define TDH_SYS_KEY_CONFIG		31
 #define TDH_SYS_INIT			33
 #define TDH_SYS_RD			34
 #define TDH_SYS_LP_INIT			35
 #define TDH_SYS_TDMR_INIT		36
+#define TDH_PHYMEM_CACHE_WB		40
+#define TDH_PHYMEM_PAGE_WBINVD		41
 #define TDH_SYS_CONFIG			45
 
 /*
-- 
2.47.0

[RFC PATCH 5/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX VM/vCPU field access

[Rick Edgecombe]

Intel TDX protects guest VMs from malicious host and certain physical
attacks. The TDX module has TD scoped and vCPU scoped "metadata fields".
These fields are a bit like VMCS fields, and stored in data structures
maintained by the TDX module. Export 3 SEAMCALLs for use in reading and
writing these fields:

Make tdh_mng_rd() use MNG.VP.RD to read the TD scoped metadata.

Make tdh_vp_rd()/tdh_vp_wr() use TDH.VP.RD/WR to read/write the vCPU
scoped metadata.

KVM will use these by creating inline helpers that target various metadata
sizes. Export the raw SEAMCALL leaf, to avoid exporting the large number
of various sized helpers.

Co-developed-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Signed-off-by: Kai Huang <kai.huang@intel.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Reviewed-by: Yuan Yao <yuan.yao@intel.com>
---
SEAMCALL RFC:
 - Use struct tdx_td and struct tdx_vp

uAPI breakout v2:
 - Change to use 'u64' as function parameter to prepare to move
   SEAMCALL wrappers to arch/x86. (Kai)
 - Split to separate patch
 - Move SEAMCALL wrappers from KVM to x86 core;
 - Move TDH_xx macros from KVM to x86 core;
 - Re-write log

uAPI breakout v1:
 - Make argument to C wrapper function struct kvm_tdx * or
   struct vcpu_tdx * .(Sean)
 - Drop unused helpers (Kai)
 - Fix bisectability issues in headers (Kai)
 - Updates from seamcall overhaul (Kai)

v19:
 - Update the commit message to match the patch by Yuan
 - Use seamcall() and seamcall_ret() by paolo

v18:
 - removed stub functions for __seamcall{,_ret}()
 - Added Reviewed-by Binbin
 - Make tdx_seamcall() use struct tdx_module_args instead of taking
   each inputs.
---
 arch/x86/include/asm/tdx.h  |  3 +++
 arch/x86/virt/vmx/tdx/tdx.c | 47 +++++++++++++++++++++++++++++++++++++
 arch/x86/virt/vmx/tdx/tdx.h |  3 +++
 3 files changed, 53 insertions(+)

diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 72de04dc242e..6a892727fdc8 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -138,9 +138,12 @@ u64 tdh_vp_addcx(struct tdx_vp *vp, hpa_t tdcx);
 u64 tdh_mng_key_config(struct tdx_td *td);
 u64 tdh_mng_create(struct tdx_td *td, hpa_t hkid);
 u64 tdh_vp_create(struct tdx_td *td, struct tdx_vp *vp);
+u64 tdh_mng_rd(struct tdx_td *td, u64 field, u64 *data);
 u64 tdh_mng_key_freeid(struct tdx_td *td);
 u64 tdh_mng_init(struct tdx_td *td, u64 td_params, hpa_t *tdr);
 u64 tdh_vp_init(struct tdx_vp *vp, u64 initial_rcx);
+u64 tdh_vp_rd(struct tdx_vp *vp, u64 field, u64 *data);
+u64 tdh_vp_wr(struct tdx_vp *vp, u64 field, u64 data, u64 mask);
 u64 tdh_vp_init_apicid(struct tdx_vp *vp, u64 initial_rcx, u32 x2apicid);
 u64 tdh_phymem_page_reclaim(hpa_t page, u64 *page_type, u64 *page_owner, u64 *page_size);
 u64 tdh_phymem_cache_wb(bool resume);
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 6f833d816899..28b3caf5a445 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1632,6 +1632,23 @@ u64 tdh_vp_create(struct tdx_td *td, struct tdx_vp *vp)
 }
 EXPORT_SYMBOL_GPL(tdh_vp_create);
 
+u64 tdh_mng_rd(struct tdx_td *td, u64 field, u64 *data)
+{
+	struct tdx_module_args args = {
+		.rcx = td->tdr,
+		.rdx = field,
+	};
+	u64 ret;
+
+	ret = seamcall_ret(TDH_MNG_RD, &args);
+
+	/* R8: Content of the field, or 0 in case of error. */
+	*data = args.r8;
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(tdh_mng_rd);
+
 u64 tdh_mng_key_freeid(struct tdx_td *td)
 {
 	struct tdx_module_args args = {
@@ -1669,6 +1686,36 @@ u64 tdh_vp_init(struct tdx_vp *vp, u64 initial_rcx)
 }
 EXPORT_SYMBOL_GPL(tdh_vp_init);
 
+u64 tdh_vp_rd(struct tdx_vp *vp, u64 field, u64 *data)
+{
+	struct tdx_module_args args = {
+		.rcx = vp->tdvpr,
+		.rdx = field,
+	};
+	u64 ret;
+
+	ret = seamcall_ret(TDH_VP_RD, &args);
+
+	/* R8: Content of the field, or 0 in case of error. */
+	*data = args.r8;
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(tdh_vp_rd);
+
+u64 tdh_vp_wr(struct tdx_vp *vp, u64 field, u64 data, u64 mask)
+{
+	struct tdx_module_args args = {
+		.rcx = vp->tdvpr,
+		.rdx = field,
+		.r8 = data,
+		.r9 = mask,
+	};
+
+	return seamcall(TDH_VP_WR, &args);
+}
+EXPORT_SYMBOL_GPL(tdh_vp_wr);
+
 u64 tdh_vp_init_apicid(struct tdx_vp *vp, u64 initial_rcx, u32 x2apicid)
 {
 	struct tdx_module_args args = {
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index 191bdd1e571d..5179fc02d109 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -21,11 +21,13 @@
 #define TDH_VP_ADDCX			4
 #define TDH_MNG_KEY_CONFIG		8
 #define TDH_MNG_CREATE			9
+#define TDH_MNG_RD			11
 #define TDH_VP_CREATE			10
 #define TDH_MNG_KEY_FREEID		20
 #define TDH_MNG_INIT			21
 #define TDH_VP_INIT			22
 #define TDH_PHYMEM_PAGE_RDMD		24
+#define TDH_VP_RD			26
 #define TDH_PHYMEM_PAGE_RECLAIM		28
 #define TDH_SYS_KEY_CONFIG		31
 #define TDH_SYS_INIT			33
@@ -34,6 +36,7 @@
 #define TDH_SYS_TDMR_INIT		36
 #define TDH_PHYMEM_CACHE_WB		40
 #define TDH_PHYMEM_PAGE_WBINVD		41
+#define TDH_VP_WR			43
 #define TDH_SYS_CONFIG			45
 
 /*
-- 
2.47.0

[RFC PATCH 6/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX flush operations

[Rick Edgecombe]

Intel TDX protects guest VMs from malicious host and certain physical
attacks. The TDX module has the concept of flushing vCPUs. These flushes
include both a flush of the translation caches and also any other state
internal to the TDX module. Before freeing a KeyID, this flush operation
needs to be done. KVM will need to perform the flush on each pCPU
associated with the TD, and also perform a TD scoped operation that checks
if the flush has been done on all vCPU's associated with the TD.

Add a tdh_vp_flush() function to be used to call TDH.VP.FLUSH on each pCPU
associated with the TD during TD teardown. It will also be called when
disabling TDX and during vCPU migration between pCPUs.

Add tdh_mng_vpflushdone() to be used by KVM to call TDH.MNG.VPFLUSHDONE.
KVM will use this during TD teardown to verify that TDH.VP.FLUSH has been
called sufficiently, and advance the state machine that will allow for
reclaiming the TD's KeyID.

Co-developed-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Signed-off-by: Kai Huang <kai.huang@intel.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Reviewed-by: Yuan Yao <yuan.yao@intel.com>
---
SEAMCALL RFC:
 - Use struct tdx_td and struct tdx_vp

uAPI breakout v2:
 - Change to use 'u64' as function parameter to prepare to move
   SEAMCALL wrappers to arch/x86. (Kai)
 - Split to separate patch
 - Move SEAMCALL wrappers from KVM to x86 core;
 - Move TDH_xx macros from KVM to x86 core;
 - Re-write log

uAPI breakout v1:
 - Make argument to C wrapper function struct kvm_tdx * or
   struct vcpu_tdx * .(Sean)
 - Drop unused helpers (Kai)
 - Fix bisectability issues in headers (Kai)
 - Updates from seamcall overhaul (Kai)

v19:
 - Update the commit message to match the patch by Yuan
 - Use seamcall() and seamcall_ret() by paolo

v18:
 - removed stub functions for __seamcall{,_ret}()
 - Added Reviewed-by Binbin
 - Make tdx_seamcall() use struct tdx_module_args instead of taking
   each inputs.
---
 arch/x86/include/asm/tdx.h  |  2 ++
 arch/x86/virt/vmx/tdx/tdx.c | 20 ++++++++++++++++++++
 arch/x86/virt/vmx/tdx/tdx.h |  2 ++
 3 files changed, 24 insertions(+)

diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 6a892727fdc8..7843a88dc90e 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -139,6 +139,8 @@ u64 tdh_mng_key_config(struct tdx_td *td);
 u64 tdh_mng_create(struct tdx_td *td, hpa_t hkid);
 u64 tdh_vp_create(struct tdx_td *td, struct tdx_vp *vp);
 u64 tdh_mng_rd(struct tdx_td *td, u64 field, u64 *data);
+u64 tdh_vp_flush(struct tdx_vp *vp);
+u64 tdh_mng_vpflushdone(struct tdx_td *td);
 u64 tdh_mng_key_freeid(struct tdx_td *td);
 u64 tdh_mng_init(struct tdx_td *td, u64 td_params, hpa_t *tdr);
 u64 tdh_vp_init(struct tdx_vp *vp, u64 initial_rcx);
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 28b3caf5a445..59cfbd1c91c0 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1649,6 +1649,26 @@ u64 tdh_mng_rd(struct tdx_td *td, u64 field, u64 *data)
 }
 EXPORT_SYMBOL_GPL(tdh_mng_rd);
 
+u64 tdh_vp_flush(struct tdx_vp *vp)
+{
+	struct tdx_module_args args = {
+		.rcx = vp->tdvpr,
+	};
+
+	return seamcall(TDH_VP_FLUSH, &args);
+}
+EXPORT_SYMBOL_GPL(tdh_vp_flush);
+
+u64 tdh_mng_vpflushdone(struct tdx_td *td)
+{
+	struct tdx_module_args args = {
+		.rcx = td->tdr,
+	};
+
+	return seamcall(TDH_MNG_VPFLUSHDONE, &args);
+}
+EXPORT_SYMBOL_GPL(tdh_mng_vpflushdone);
+
 u64 tdh_mng_key_freeid(struct tdx_td *td)
 {
 	struct tdx_module_args args = {
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index 5179fc02d109..08b01b7fe7c2 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -22,6 +22,8 @@
 #define TDH_MNG_KEY_CONFIG		8
 #define TDH_MNG_CREATE			9
 #define TDH_MNG_RD			11
+#define TDH_VP_FLUSH			18
+#define TDH_MNG_VPFLUSHDONE		19
 #define TDH_VP_CREATE			10
 #define TDH_MNG_KEY_FREEID		20
 #define TDH_MNG_INIT			21
-- 
2.47.0

Re: [RFC PATCH 1/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX KeyID management

[Dave Hansen]

On 11/15/24 12:20, Rick Edgecombe wrote:
> +struct tdx_td {
> +	hpa_t tdr;
> +	hpa_t *tdcs;
> +};

This is a step in the right direction because it gives the wrappers some
more type safety.

But an hpa_t is _barely_ better than a u64.  If the 'tdr' is a page,
then it needs to be _stored_ as a page:

	struct page *tdr_page;

Also, please don't forget to spell these things out:

	/* TD root structure: */
	struct page *tdr_page;

And the tdcs is an array of pages, right?  So it should be:

	struct page **tdcs_pages;

Or heck, I _think_ it can theoretically be defined as a variable-length
array:

	struct page *tdcs_pages[];

and use the helpers that we have for that.

Putting it all together, you would have this:

struct tdx_td {
	/* TD root structure: */
	struct page *tdr_page;

	int tdcs_nr_pages;
	/* TD control structure: */
	struct page *tdcs_pages[];
};

That's *MUCH* harder to misuse.  It's 100% obvious that you have a
single page, plus a variable-length array of pages.  This is all from
just looking at the structure definition.

You know that 'tdr' is not just some random physical address.  It's a
whole physical page.  It's page-aligned.  It was allocated, from the
allocator.  It doesn't point to special memory.

Ditto for "hpa_t *tdcs".  It's not obvious from the data structure that
it's an array or if it's an array how it got allocated or how large it is.

Re: [RFC PATCH 1/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX KeyID management

[Sean Christopherson]

On Fri, Nov 22, 2024, Dave Hansen wrote:
> On 11/15/24 12:20, Rick Edgecombe wrote:
> > +struct tdx_td {
> > +	hpa_t tdr;
> > +	hpa_t *tdcs;
> > +};
> 
> This is a step in the right direction because it gives the wrappers some
> more type safety.
> 
> But an hpa_t is _barely_ better than a u64.  If the 'tdr' is a page,
> then it needs to be _stored_ as a page:
> 
> 	struct page *tdr_page;
> 
> Also, please don't forget to spell these things out:
> 
> 	/* TD root structure: */
> 	struct page *tdr_page;
> 
> And the tdcs is an array of pages, right?  So it should be:
> 
> 	struct page **tdcs_pages;
> 
> Or heck, I _think_ it can theoretically be defined as a variable-length
> array:
> 
> 	struct page *tdcs_pages[];
> 
> and use the helpers that we have for that.
> 
> Putting it all together, you would have this:
> 
> struct tdx_td {
> 	/* TD root structure: */
> 	struct page *tdr_page;
> 
> 	int tdcs_nr_pages;
> 	/* TD control structure: */
> 	struct page *tdcs_pages[];
> };
> 
> That's *MUCH* harder to misuse.  It's 100% obvious that you have a
> single page, plus a variable-length array of pages.  This is all from
> just looking at the structure definition.

I don't know the full context, but working with "struct page" is a pain when every
user just wants the physical address.  KVM SVM had a few cases where pointers were
tracked as "struct page", and it was generally unpleasant to read and work with.

I also don't like conflating the kernel's "struct page" with the architecture's
definition of a 4KiB page.

> You know that 'tdr' is not just some random physical address.  It's a
> whole physical page.  It's page-aligned.  It was allocated, from the
> allocator.  It doesn't point to special memory.

Oh, but it does point to special memory.  If it *didn't* point at special memory
that is completely opaque and untouchable, then KVM could use a struct overlay,
which would give contextual information and some amount of type safety.  E.g.
an equivalent without TDX is "struct vmcs *".

Rather than "struct page", what if we add an address_space (in the Sparse sense),
and a typedef for a TDX pages?  Maybe __firmware?  E.g.

  # define __firmware	__attribute__((noderef, address_space(__firmware)))

  typedef u64 __firmware *tdx_page_t;

That doesn't give as much compile-time safety, but in some ways it provides more
type safety since KVM (or whatever else cares) would need to make an explicit and
ugly cast to misuse the pointer.

> Ditto for "hpa_t *tdcs".  It's not obvious from the data structure that
> it's an array or if it's an array how it got allocated or how large it is.

Re: [RFC PATCH 1/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX KeyID management

[Dave Hansen]

On 11/22/24 15:55, Sean Christopherson wrote:
>> You know that 'tdr' is not just some random physical address.  It's a
>> whole physical page.  It's page-aligned.  It was allocated, from the
>> allocator.  It doesn't point to special memory.
> Oh, but it does point to special memory. 

In this specific case I meant "special memory" as not RAM and not
managed by the kernel.

Re: [RFC PATCH 1/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX KeyID management

[Dave Hansen]

On 11/22/24 15:55, Sean Christopherson wrote:
> On Fri, Nov 22, 2024, Dave Hansen wrote:
> I don't know the full context, but working with "struct page" is a pain when every
> user just wants the physical address.  KVM SVM had a few cases where pointers were
> tracked as "struct page", and it was generally unpleasant to read and work with.

I'm not super convinced. page_to_phys(foo) is all it takes

> I also don't like conflating the kernel's "struct page" with the architecture's
> definition of a 4KiB page.

That's fair, although it's pervasively conflated across our entire
codebase. But 'struct page' is substantially better than a hpa_t,
phys_addr_t or u64 that can store a full 64-bits of address. Those
conflate a physical address with a physical page, which is *FAR* worse.

>> You know that 'tdr' is not just some random physical address.  It's a
>> whole physical page.  It's page-aligned.  It was allocated, from the
>> allocator.  It doesn't point to special memory.
> 
> Oh, but it does point to special memory.  If it *didn't* point at special memory
> that is completely opaque and untouchable, then KVM could use a struct overlay,
> which would give contextual information and some amount of type safety.  E.g.
> an equivalent without TDX is "struct vmcs *".
> 
> Rather than "struct page", what if we add an address_space (in the Sparse sense),
> and a typedef for a TDX pages?  Maybe __firmware?  E.g.
> 
>   # define __firmware	__attribute__((noderef, address_space(__firmware)))
> 
>   typedef u64 __firmware *tdx_page_t;
> 
> That doesn't give as much compile-time safety, but in some ways it provides more
> type safety since KVM (or whatever else cares) would need to make an explicit and
> ugly cast to misuse the pointer.

It's better than nothing. But I still vastly prefer to have a type that
tells you that something is physically-allocated out of the buddy, RAM,
and page-aligned.

I'd be better to have:

struct tdx_page {
	u64 page_phys_addr;
};

than depend on sparse, IMNHO.

Do you run sparse every time you compile the kernel, btw? ;)

Re: [RFC PATCH 1/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX KeyID management

[Edgecombe, Rick P]

On Fri, 2024-11-22 at 16:08 -0800, Dave Hansen wrote:
> On 11/22/24 15:55, Sean Christopherson wrote:
> > On Fri, Nov 22, 2024, Dave Hansen wrote:
> > I don't know the full context, but working with "struct page" is a pain when every
> > user just wants the physical address.  KVM SVM had a few cases where pointers were
> > tracked as "struct page", and it was generally unpleasant to read and work with.
> 
> I'm not super convinced. page_to_phys(foo) is all it takes
> 
> > I also don't like conflating the kernel's "struct page" with the architecture's
> > definition of a 4KiB page.
> 
> That's fair, although it's pervasively conflated across our entire
> codebase. But 'struct page' is substantially better than a hpa_t,
> phys_addr_t or u64 that can store a full 64-bits of address. Those
> conflate a physical address with a physical page, which is *FAR* worse.

In the case of tdh_mem_page_aug(), etc the caller only has a kvm_pfn_t passed
from a TDP MMU callback, for the page to be mapped in the guest TD. It is
probably not nice to assume that this kvm_pfn_t will have a struct page. So we
shouldn't always use struct pages for the SEAMCALL wrappers in any case.

What if we just move these members from hpa_t to pfn_t? It keeps us off struct
page, but addresses some of Dave's concerns about hpa_t looking like a specific
address.

> 
> > > You know that 'tdr' is not just some random physical address.  It's a
> > > whole physical page.  It's page-aligned.  It was allocated, from the
> > > allocator.  It doesn't point to special memory.
> > 
> > Oh, but it does point to special memory.  If it *didn't* point at special memory
> > that is completely opaque and untouchable, then KVM could use a struct overlay,
> > which would give contextual information and some amount of type safety.  E.g.
> > an equivalent without TDX is "struct vmcs *".
> > 
> > Rather than "struct page", what if we add an address_space (in the Sparse sense),
> > and a typedef for a TDX pages?  Maybe __firmware?  E.g.
> > 
> >    # define __firmware	__attribute__((noderef, address_space(__firmware)))
> > 
> >    typedef u64 __firmware *tdx_page_t;
> > 
> > That doesn't give as much compile-time safety, but in some ways it provides more
> > type safety since KVM (or whatever else cares) would need to make an explicit and
> > ugly cast to misuse the pointer.
> 
> It's better than nothing. But I still vastly prefer to have a type that
> tells you that something is physically-allocated out of the buddy, RAM,
> and page-aligned.
> 
> I'd be better to have:
> 
> struct tdx_page {
> 	u64 page_phys_addr;
> };
> 
> than depend on sparse, IMNHO.
> 
> Do you run sparse every time you compile the kernel, btw? ;)

Hmm, I'm trying to think of specific scenarios that "tdx page" types could make
big safety difference on.

Sean, do you happen to recall any specific bugs on the SEV side that this would
have helped with?

I hear the intuition, but without specific problems, it doesn't seem worth extra
code to me. Not a strong objection though.

Re: [RFC PATCH 1/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX KeyID management

[Sean Christopherson]

On Fri, Nov 22, 2024, Dave Hansen wrote:
> On 11/22/24 15:55, Sean Christopherson wrote:
> > On Fri, Nov 22, 2024, Dave Hansen wrote:
> > I don't know the full context, but working with "struct page" is a pain when every
> > user just wants the physical address.  KVM SVM had a few cases where pointers were
> > tracked as "struct page", and it was generally unpleasant to read and work with.
> 
> I'm not super convinced. page_to_phys(foo) is all it takes

I looked again at the KVM code that bugs me, and my complaints are with code that
needs both the physical address and the virtual address, i.e. that could/should
use a meaningful pointer to describe the underlying data structure since KVM does
directly access the memory.

But for TDX pages, that obviously doesn't apply, so I withdraw my objection about
using struct page.

> > I also don't like conflating the kernel's "struct page" with the architecture's
> > definition of a 4KiB page.
> 
> That's fair, although it's pervasively conflated across our entire
> codebase. But 'struct page' is substantially better than a hpa_t,
> phys_addr_t or u64 that can store a full 64-bits of address. Those
> conflate a physical address with a physical page, which is *FAR* worse.
> 
> >> You know that 'tdr' is not just some random physical address.  It's a
> >> whole physical page.  It's page-aligned.  It was allocated, from the
> >> allocator.  It doesn't point to special memory.
> > 
> > Oh, but it does point to special memory.  If it *didn't* point at special memory
> > that is completely opaque and untouchable, then KVM could use a struct overlay,
> > which would give contextual information and some amount of type safety.  E.g.
> > an equivalent without TDX is "struct vmcs *".
> > 
> > Rather than "struct page", what if we add an address_space (in the Sparse sense),
> > and a typedef for a TDX pages?  Maybe __firmware?  E.g.
> > 
> >   # define __firmware	__attribute__((noderef, address_space(__firmware)))
> > 
> >   typedef u64 __firmware *tdx_page_t;
> > 
> > That doesn't give as much compile-time safety, but in some ways it provides more
> > type safety since KVM (or whatever else cares) would need to make an explicit and
> > ugly cast to misuse the pointer.
> 
> It's better than nothing. But I still vastly prefer to have a type that
> tells you that something is physically-allocated out of the buddy, RAM,
> and page-aligned.
> 
> I'd be better to have:
> 
> struct tdx_page {
> 	u64 page_phys_addr;
> };
> 
> than depend on sparse, IMNHO.
> 
> Do you run sparse every time you compile the kernel, btw? ;)

Nah, but the 0-day both does such a good job of detecting and reporting new warnings
that I'm generally comfortable relying on sparse for something like this.  Though
as above, I'm ok with using "struct page" for the TDX pages.

Re: [RFC PATCH 1/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX KeyID management

[Dave Hansen]

On 11/25/24 07:44, Sean Christopherson wrote:
> Nah, but the 0-day both does such a good job of detecting and
> reporting new warnings that I'm generally comfortable relying on
> sparse for something like this.  Though as above, I'm ok with using
> "struct page" for the TDX pages.
Cool beans. Thanks for double-checking that KVM code you were concerned
about. It's much appreciated!

Re: [RFC PATCH 1/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX KeyID management

[Paolo Bonzini]

On 11/23/24 03:06, Edgecombe, Rick P wrote:
> On Fri, 2024-11-22 at 16:08 -0800, Dave Hansen wrote:
>> On 11/22/24 15:55, Sean Christopherson wrote:
>>> On Fri, Nov 22, 2024, Dave Hansen wrote:
>>> I don't know the full context, but working with "struct page" is a pain when every
>>> user just wants the physical address.  KVM SVM had a few cases where pointers were
>>> tracked as "struct page", and it was generally unpleasant to read and work with.
>>
>> I'm not super convinced. page_to_phys(foo) is all it takes
>>
>>> I also don't like conflating the kernel's "struct page" with the architecture's
>>> definition of a 4KiB page.
>>
>> That's fair, although it's pervasively conflated across our entire
>> codebase. But 'struct page' is substantially better than a hpa_t,
>> phys_addr_t or u64 that can store a full 64-bits of address. Those
>> conflate a physical address with a physical page, which is *FAR* worse.
> 
> In the case of tdh_mem_page_aug(), etc the caller only has a kvm_pfn_t passed
> from a TDP MMU callback, for the page to be mapped in the guest TD. It is
> probably not nice to assume that this kvm_pfn_t will have a struct page. So we
> shouldn't always use struct pages for the SEAMCALL wrappers in any case.
> 
> What if we just move these members from hpa_t to pfn_t? It keeps us off struct
> page, but addresses some of Dave's concerns about hpa_t looking like a specific
> address.

For tdr I agree with Dave that you probably want a struct which stores 
the struct page*. Currently the code is using __get_free_page(), but 
it's a small change to have it use alloc_page() instead, and 
__free_page() instead of free_page().

The only difference on the arch/x86/virt/ side is a bunch of added 
page_to_phys().

Anyhow, whatever you post I'll take care of adjusting in the KVM patches.

Paolo

Re: [RFC PATCH 1/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX KeyID management

[Edgecombe, Rick P]

On Wed, 2024-11-27 at 19:15 +0100, Paolo Bonzini wrote:
> > What if we just move these members from hpa_t to pfn_t? It keeps us off
> > struct
> > page, but addresses some of Dave's concerns about hpa_t looking like a
> > specific
> > address.
> 
> For tdr I agree with Dave that you probably want a struct which stores 
> the struct page*. Currently the code is using __get_free_page(), but 
> it's a small change to have it use alloc_page() instead, and 
> __free_page() instead of free_page().
> 
> The only difference on the arch/x86/virt/ side is a bunch of added 
> page_to_phys().

Thanks.

> 
> Anyhow, whatever you post I'll take care of adjusting in the KVM patches.

I'm just throwing together a v2. We have the US holidays this week, so it may be
Monday.

Re: [RFC PATCH 0/6] SEAMCALL Wrappers

[Paolo Bonzini]

On Fri, Nov 15, 2024 at 9:20 PM Rick Edgecombe
<rick.p.edgecombe@intel.com> wrote:
> Separate from discussions with Dave on the SEAMCALLs, there was some some
> suggestions on how we might remove or combine specific SEAMCALLs. I didn’t
> try this here, because this RFC is more about exploring in general how we
> want to distribute things between KVM and arch/x86 for these SEAMCALL
> wrappers.
>
> So in summary the RFC only has:
>  - Use structs to hold tdXYZ fields for TD and vCPUs
>  - Make helper to hold CLFLUSH_BEFORE_ALLOC comments
>  - Use semantic names for out args
>  - (Add Kai's sign-off that should have been in the last version)
>
> Patches 1 and 3 contain new commit log verbiage justifying specific design
> choices behind the struct definitions.
>
> I didn’t create enums for the out args. Just using proper names for the
> args seemed like a good balance between code clarity and not
> over-engineering. But please correct if this was the wrong judgment.

Sounds good. I'll also convert

x86/virt/tdx: Add SEAMCALL wrapper tdh_mem_sept_add() to add SEPT pages
x86/virt/tdx: Add SEAMCALL wrappers to add TD private pages
x86/virt/tdx: Add SEAMCALL wrappers to manage TDX TLB tracking
x86/virt/tdx: Add SEAMCALL wrappers to remove a TD private page
x86/virt/tdx: Add SEAMCALL wrappers for TD measurement of initial contents
x86/virt/tdx: Add SEAMCALL wrapper to enter/exit TDX guest

(which I've "extracted" from the TDX-KVM series and placed all at the
top of kvm-coco-queue).

Paolo

> Here is a branch for seeing the callers. I didn’t squash the caller
> changes into the patches yet either, the caller changes are all just in the
> HEAD commit. I also only converted the “VM/vCPU creation” SEAMCALLs to the
> approach described above:
> https://github.com/intel/tdx/tree/seamcall-rfc
>
> [0] https://lore.kernel.org/kvm/20241030190039.77971-1-rick.p.edgecombe@intel.com/
>
>
> Rick Edgecombe (6):
>   x86/virt/tdx: Add SEAMCALL wrappers for TDX KeyID management
>   x86/virt/tdx: Add SEAMCALL wrappers for TDX TD creation
>   x86/virt/tdx: Add SEAMCALL wrappers for TDX vCPU creation
>   x86/virt/tdx: Add SEAMCALL wrappers for TDX page cache management
>   x86/virt/tdx: Add SEAMCALL wrappers for TDX VM/vCPU field access
>   x86/virt/tdx: Add SEAMCALL wrappers for TDX flush operations
>
>  arch/x86/include/asm/tdx.h  |  29 +++++
>  arch/x86/virt/vmx/tdx/tdx.c | 224 ++++++++++++++++++++++++++++++++++++
>  arch/x86/virt/vmx/tdx/tdx.h |  38 ++++--
>  3 files changed, 284 insertions(+), 7 deletions(-)
>
> --
> 2.47.0
>