public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH v2 0/6] SEAMCALL Wrappers
@ 2024-12-03  1:03 Rick Edgecombe
  2024-12-03  1:03 ` [RFC PATCH v2 1/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX KeyID management Rick Edgecombe
                   ` (6 more replies)
  0 siblings, 7 replies; 19+ messages in thread
From: Rick Edgecombe @ 2024-12-03  1:03 UTC (permalink / raw)
  To: kvm, pbonzini, seanjc, dave.hansen
  Cc: isaku.yamahata, kai.huang, linux-kernel, tony.lindgren,
	xiaoyao.li, yan.y.zhao, rick.p.edgecombe, x86, adrian.hunter

Hi,

This is a followup to the "SEAMCALL Wrappers" RFC[0] that spun out of 
Dave’s comments on the SEAMCALL wrappers in the “TDX vCPU/VM creation” 
series [1]. 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.

The last discussion was to use struct pages instead of u64. It is pretty 
much what Dave suggested with a minor tweak to instead include the tdcx 
page count in the TD struct instead of the vCPU one.

This is because it will not vary between vCPUs. Doing it that way 
basically preserves the existing data duplication, but these counts are 
basically "global metadata". The global metadata patches export them as a 
size, but KVM wants to use them as a page count. So we should not be 
including these counts in each TD scoped structure as is currently done. To
address the duplication we need to change the "global metadata patches"
to export the count instead of size.

Otherwise, in the spirit of looking to find better types for the other raw 
u64's, I played around again with the out params of 
tdh_phymem_page_reclaim(). In the end I opted for better names and a 
comment rather than anything fancier.

Here is the branch with the VM/vCPU caller adjustments as the last commit:
https://github.com/intel/tdx/tree/seamcall-rfc-v2

Thanks,

Rick

[0]
https://lore.kernel.org/kvm/20241115202028.1585487-1-rick.p.edgecombe@intel.com/
[1]
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  |  38 ++++++
 arch/x86/virt/vmx/tdx/tdx.c | 240 ++++++++++++++++++++++++++++++++++++
 arch/x86/virt/vmx/tdx/tdx.h |  38 ++++--
 3 files changed, 309 insertions(+), 7 deletions(-)

-- 
2.47.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 1/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX KeyID management
  2024-12-03  1:03 [RFC PATCH v2 0/6] SEAMCALL Wrappers Rick Edgecombe
@ 2024-12-03  1:03 ` Rick Edgecombe
  2024-12-03  1:03 ` [RFC PATCH v2 2/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX TD creation Rick Edgecombe
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 19+ messages in thread
From: Rick Edgecombe @ 2024-12-03  1:03 UTC (permalink / raw)
  To: kvm, pbonzini, seanjc, dave.hansen
  Cc: isaku.yamahata, kai.huang, linux-kernel, tony.lindgren,
	xiaoyao.li, yan.y.zhao, rick.p.edgecombe, x86, adrian.hunter,
	Isaku Yamahata, Binbin Wu, Yuan Yao

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:
 - Use struct page (Dave)

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  | 12 ++++++++++++
 arch/x86/virt/vmx/tdx/tdx.c | 26 ++++++++++++++++++++++++++
 arch/x86/virt/vmx/tdx/tdx.h | 16 +++++++++-------
 3 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index d33e46d53d59..139c003acd1b 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -121,6 +121,18 @@ 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 {
+	/* TD root structure: */
+	struct page *tdr_page;
+
+	int tdcs_nr_pages;
+	/* TD control structure: */
+	struct page **tdcs_pages;
+};
+
+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..2d1cebce8c07 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1562,3 +1562,29 @@ void tdx_guest_keyid_free(unsigned int keyid)
 	ida_free(&tdx_guest_keyid_pool, keyid);
 }
 EXPORT_SYMBOL_GPL(tdx_guest_keyid_free);
+
+static inline u64 tdx_tdr_pa(struct tdx_td *td)
+{
+	return page_to_pfn(td->tdr_page) << PAGE_SHIFT;
+}
+
+u64 tdh_mng_key_config(struct tdx_td *td)
+{
+	struct tdx_module_args args = {
+		.rcx = tdx_tdr_pa(td),
+	};
+
+	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 = tdx_tdr_pa(td),
+	};
+
+	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.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 2/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX TD creation
  2024-12-03  1:03 [RFC PATCH v2 0/6] SEAMCALL Wrappers Rick Edgecombe
  2024-12-03  1:03 ` [RFC PATCH v2 1/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX KeyID management Rick Edgecombe
@ 2024-12-03  1:03 ` Rick Edgecombe
  2024-12-03  2:20   ` Binbin Wu
  2024-12-04 19:54   ` Dave Hansen
  2024-12-03  1:03 ` [RFC PATCH v2 3/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX vCPU creation Rick Edgecombe
                   ` (4 subsequent siblings)
  6 siblings, 2 replies; 19+ messages in thread
From: Rick Edgecombe @ 2024-12-03  1:03 UTC (permalink / raw)
  To: kvm, pbonzini, seanjc, dave.hansen
  Cc: isaku.yamahata, kai.huang, linux-kernel, tony.lindgren,
	xiaoyao.li, yan.y.zhao, rick.p.edgecombe, x86, adrian.hunter,
	Isaku Yamahata, Binbin Wu, Yuan Yao

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 v2:
 - Use struct page (Dave)
 - Fix wrongly named arg in tdh_mng_init()
 - Fix type of hkid in tdh_mng_create()

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 139c003acd1b..a4360a71dbdd 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -131,8 +131,11 @@ struct tdx_td {
 	struct page **tdcs_pages;
 };
 
+u64 tdh_mng_addcx(struct tdx_td *td, struct page *tdcs_page);
 u64 tdh_mng_key_config(struct tdx_td *td);
+u64 tdh_mng_create(struct tdx_td *td, u64 hkid);
 u64 tdh_mng_key_freeid(struct tdx_td *td);
+u64 tdh_mng_init(struct tdx_td *td, u64 td_params, u64 *extended_err);
 #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 2d1cebce8c07..605eb9bd81d3 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1568,6 +1568,29 @@ static inline u64 tdx_tdr_pa(struct tdx_td *td)
 	return page_to_pfn(td->tdr_page) << PAGE_SHIFT;
 }
 
+/*
+ * 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(struct page *tdr)
+{
+	clflush_cache_range(page_to_virt(tdr), PAGE_SIZE);
+}
+
+u64 tdh_mng_addcx(struct tdx_td *td, struct page *tdcs_page)
+{
+	struct tdx_module_args args = {
+		.rcx = page_to_pfn(tdcs_page) << PAGE_SHIFT,
+		.rdx = tdx_tdr_pa(td),
+	};
+
+	tdx_clflush_page(tdcs_page);
+	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 = {
@@ -1578,6 +1601,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, u64 hkid)
+{
+	struct tdx_module_args args = {
+		.rcx = tdx_tdr_pa(td),
+		.rdx = hkid,
+	};
+
+	tdx_clflush_page(td->tdr_page);
+	return seamcall(TDH_MNG_CREATE, &args);
+}
+EXPORT_SYMBOL_GPL(tdh_mng_create);
+
 u64 tdh_mng_key_freeid(struct tdx_td *td)
 {
 	struct tdx_module_args args = {
@@ -1588,3 +1623,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, u64 *extended_err)
+{
+	struct tdx_module_args args = {
+		.rcx = tdx_tdr_pa(td),
+		.rdx = td_params,
+	};
+	u64 ret;
+
+	ret = seamcall_ret(TDH_MNG_INIT, &args);
+
+	*extended_err = 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.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 3/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX vCPU creation
  2024-12-03  1:03 [RFC PATCH v2 0/6] SEAMCALL Wrappers Rick Edgecombe
  2024-12-03  1:03 ` [RFC PATCH v2 1/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX KeyID management Rick Edgecombe
  2024-12-03  1:03 ` [RFC PATCH v2 2/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX TD creation Rick Edgecombe
@ 2024-12-03  1:03 ` Rick Edgecombe
  2024-12-03  1:03 ` [RFC PATCH v2 4/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX page cache management Rick Edgecombe
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 19+ messages in thread
From: Rick Edgecombe @ 2024-12-03  1:03 UTC (permalink / raw)
  To: kvm, pbonzini, seanjc, dave.hansen
  Cc: isaku.yamahata, kai.huang, linux-kernel, tony.lindgren,
	xiaoyao.li, yan.y.zhao, rick.p.edgecombe, x86, adrian.hunter,
	Isaku Yamahata, Binbin Wu, Yuan Yao

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 v2:
 - Use struct page (Dave)

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  | 15 +++++++++++
 arch/x86/virt/vmx/tdx/tdx.c | 53 +++++++++++++++++++++++++++++++++++++
 arch/x86/virt/vmx/tdx/tdx.h | 11 ++++++++
 3 files changed, 79 insertions(+)

diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index a4360a71dbdd..018bbabf8639 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -129,13 +129,28 @@ struct tdx_td {
 	int tdcs_nr_pages;
 	/* TD control structure: */
 	struct page **tdcs_pages;
+
+	/* Size of `tdcx_pages` in struct tdx_vp */
+	int tdcx_nr_pages;
+};
+
+struct tdx_vp {
+	/* TDVP root page */
+	struct page *tdvpr_page;
+
+	/* TD vCPU control structure: */
+	struct page **tdcx_pages;
 };
 
 u64 tdh_mng_addcx(struct tdx_td *td, struct page *tdcs_page);
+u64 tdh_vp_addcx(struct tdx_vp *vp, struct page *tdcx_page);
 u64 tdh_mng_key_config(struct tdx_td *td);
 u64 tdh_mng_create(struct tdx_td *td, u64 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, u64 *extended_err);
+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 605eb9bd81d3..d71656868fe4 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -5,6 +5,7 @@
  * Intel Trusted Domain Extensions (TDX) support
  */
 
+#include "asm/page_types.h"
 #define pr_fmt(fmt)	"virt/tdx: " fmt
 
 #include <linux/types.h>
@@ -1568,6 +1569,11 @@ static inline u64 tdx_tdr_pa(struct tdx_td *td)
 	return page_to_pfn(td->tdr_page) << PAGE_SHIFT;
 }
 
+static inline u64 tdx_tdvpr_pa(struct tdx_vp *td)
+{
+	return page_to_pfn(td->tdvpr_page) << PAGE_SHIFT;
+}
+
 /*
  * 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.
@@ -1591,6 +1597,18 @@ u64 tdh_mng_addcx(struct tdx_td *td, struct page *tdcs_page)
 }
 EXPORT_SYMBOL_GPL(tdh_mng_addcx);
 
+u64 tdh_vp_addcx(struct tdx_vp *vp, struct page *tdcx_page)
+{
+	struct tdx_module_args args = {
+		.rcx = page_to_pfn(tdcx_page) << PAGE_SHIFT,
+		.rdx = tdx_tdvpr_pa(vp),
+	};
+
+	tdx_clflush_page(tdcx_page);
+	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 = {
@@ -1613,6 +1631,18 @@ u64 tdh_mng_create(struct tdx_td *td, u64 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 = tdx_tdvpr_pa(vp),
+		.rdx = tdx_tdr_pa(td),
+	};
+
+	tdx_clflush_page(vp->tdvpr_page);
+	return seamcall(TDH_VP_CREATE, &args);
+}
+EXPORT_SYMBOL_GPL(tdh_vp_create);
+
 u64 tdh_mng_key_freeid(struct tdx_td *td)
 {
 	struct tdx_module_args args = {
@@ -1639,3 +1669,26 @@ u64 tdh_mng_init(struct tdx_td *td, u64 td_params, u64 *extended_err)
 }
 EXPORT_SYMBOL_GPL(tdh_mng_init);
 
+u64 tdh_vp_init(struct tdx_vp *vp, u64 initial_rcx)
+{
+	struct tdx_module_args args = {
+		.rcx = tdx_tdvpr_pa(vp),
+		.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 = tdx_tdvpr_pa(vp),
+		.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.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 4/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX page cache management
  2024-12-03  1:03 [RFC PATCH v2 0/6] SEAMCALL Wrappers Rick Edgecombe
                   ` (2 preceding siblings ...)
  2024-12-03  1:03 ` [RFC PATCH v2 3/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX vCPU creation Rick Edgecombe
@ 2024-12-03  1:03 ` Rick Edgecombe
  2024-12-03  2:33   ` Binbin Wu
  2024-12-11  1:23   ` Yan Zhao
  2024-12-03  1:03 ` [RFC PATCH v2 5/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX VM/vCPU field access Rick Edgecombe
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 19+ messages in thread
From: Rick Edgecombe @ 2024-12-03  1:03 UTC (permalink / raw)
  To: kvm, pbonzini, seanjc, dave.hansen
  Cc: isaku.yamahata, kai.huang, linux-kernel, tony.lindgren,
	xiaoyao.li, yan.y.zhao, rick.p.edgecombe, x86, adrian.hunter,
	Isaku Yamahata, Binbin Wu, Yuan Yao

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 v2:
 - Use struct page (Dave)
 - Rename out args for tdh_phymem_page_reclaim() to make it clear these
   are TDX specific values, and not to be interpreted normally. Also,
   add a comment about this.

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 | 43 +++++++++++++++++++++++++++++++++++++
 arch/x86/virt/vmx/tdx/tdx.h |  3 +++
 3 files changed, 49 insertions(+)

diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 018bbabf8639..f6ab8a9ea46b 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -151,6 +151,9 @@ u64 tdh_mng_key_freeid(struct tdx_td *td);
 u64 tdh_mng_init(struct tdx_td *td, u64 td_params, u64 *extended_err);
 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(struct page *page, u64 *tdx_pt, u64 *tdx_owner, u64 *tdx_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 d71656868fe4..b2a1ed13d0da 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1692,3 +1692,46 @@ 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);
+
+/*
+ * TDX ABI defines output operands as PT, OWNER and SIZE. These are TDX defined fomats.
+ * So despite the names, they must be interpted specially as described by the spec. Return
+ * them only for error reporting purposes.
+ */
+u64 tdh_phymem_page_reclaim(struct page *page, u64 *tdx_pt, u64 *tdx_owner, u64 *tdx_size)
+{
+	struct tdx_module_args args = {
+		.rcx = page_to_pfn(page) << PAGE_SHIFT,
+	};
+	u64 ret;
+
+	ret = seamcall_ret(TDH_PHYMEM_PAGE_RECLAIM, &args);
+
+	*tdx_pt = args.rcx;
+	*tdx_owner = args.rdx;
+	*tdx_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 = tdx_tdr_pa(td) | ((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.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 5/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX VM/vCPU field access
  2024-12-03  1:03 [RFC PATCH v2 0/6] SEAMCALL Wrappers Rick Edgecombe
                   ` (3 preceding siblings ...)
  2024-12-03  1:03 ` [RFC PATCH v2 4/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX page cache management Rick Edgecombe
@ 2024-12-03  1:03 ` Rick Edgecombe
  2024-12-04 19:57   ` Dave Hansen
  2024-12-03  1:03 ` [RFC PATCH v2 6/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX flush operations Rick Edgecombe
  2024-12-04  1:24 ` [RFC PATCH v2 0/6] SEAMCALL Wrappers Huang, Kai
  6 siblings, 1 reply; 19+ messages in thread
From: Rick Edgecombe @ 2024-12-03  1:03 UTC (permalink / raw)
  To: kvm, pbonzini, seanjc, dave.hansen
  Cc: isaku.yamahata, kai.huang, linux-kernel, tony.lindgren,
	xiaoyao.li, yan.y.zhao, rick.p.edgecombe, x86, adrian.hunter,
	Isaku Yamahata, Binbin Wu, Yuan Yao

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 f6ab8a9ea46b..9bc3c1160d43 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -147,9 +147,12 @@ u64 tdh_vp_addcx(struct tdx_vp *vp, struct page *tdcx_page);
 u64 tdh_mng_key_config(struct tdx_td *td);
 u64 tdh_mng_create(struct tdx_td *td, u64 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, u64 *extended_err);
 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(struct page *page, u64 *tdx_pt, u64 *tdx_owner, u64 *tdx_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 b2a1ed13d0da..6d35ea5b238f 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1643,6 +1643,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 = tdx_tdr_pa(td),
+		.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 = {
@@ -1680,6 +1697,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 = tdx_tdvpr_pa(vp),
+		.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 = tdx_tdvpr_pa(vp),
+		.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.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 6/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX flush operations
  2024-12-03  1:03 [RFC PATCH v2 0/6] SEAMCALL Wrappers Rick Edgecombe
                   ` (4 preceding siblings ...)
  2024-12-03  1:03 ` [RFC PATCH v2 5/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX VM/vCPU field access Rick Edgecombe
@ 2024-12-03  1:03 ` Rick Edgecombe
  2024-12-04  1:24 ` [RFC PATCH v2 0/6] SEAMCALL Wrappers Huang, Kai
  6 siblings, 0 replies; 19+ messages in thread
From: Rick Edgecombe @ 2024-12-03  1:03 UTC (permalink / raw)
  To: kvm, pbonzini, seanjc, dave.hansen
  Cc: isaku.yamahata, kai.huang, linux-kernel, tony.lindgren,
	xiaoyao.li, yan.y.zhao, rick.p.edgecombe, x86, adrian.hunter,
	Isaku Yamahata, Binbin Wu, Yuan Yao

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 9bc3c1160d43..bbb8f0bae9ba 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -148,6 +148,8 @@ u64 tdh_mng_key_config(struct tdx_td *td);
 u64 tdh_mng_create(struct tdx_td *td, u64 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, u64 *extended_err);
 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 6d35ea5b238f..b30ee1cff22f 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1660,6 +1660,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 = tdx_tdvpr_pa(vp),
+	};
+
+	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 = tdx_tdr_pa(td),
+	};
+
+	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.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 2/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX TD creation
  2024-12-03  1:03 ` [RFC PATCH v2 2/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX TD creation Rick Edgecombe
@ 2024-12-03  2:20   ` Binbin Wu
  2024-12-04  1:58     ` Edgecombe, Rick P
  2024-12-04 19:54   ` Dave Hansen
  1 sibling, 1 reply; 19+ messages in thread
From: Binbin Wu @ 2024-12-03  2:20 UTC (permalink / raw)
  To: Rick Edgecombe
  Cc: kvm, pbonzini, seanjc, dave.hansen, isaku.yamahata, kai.huang,
	linux-kernel, tony.lindgren, xiaoyao.li, yan.y.zhao, x86,
	adrian.hunter, Isaku Yamahata, Yuan Yao




On 12/3/2024 9:03 AM, Rick Edgecombe wrote:
[...]
>   
> +/*
> + * 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(struct page *tdr)
The argument should have a generic name instead of tdr, because it's not
limited to TDR.

> +{
> +	clflush_cache_range(page_to_virt(tdr), PAGE_SIZE);
> +}
> +
> +u64 tdh_mng_addcx(struct tdx_td *td, struct page *tdcs_page)
> +{
> +	struct tdx_module_args args = {
> +		.rcx = page_to_pfn(tdcs_page) << PAGE_SHIFT,
> +		.rdx = tdx_tdr_pa(td),
> +	};
> +
> +	tdx_clflush_page(tdcs_page);
> +	return seamcall(TDH_MNG_ADDCX, &args);
> +}
> +EXPORT_SYMBOL_GPL(tdh_mng_addcx);
> +
[...]


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 4/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX page cache management
  2024-12-03  1:03 ` [RFC PATCH v2 4/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX page cache management Rick Edgecombe
@ 2024-12-03  2:33   ` Binbin Wu
  2024-12-04  1:58     ` Edgecombe, Rick P
  2024-12-11  1:23   ` Yan Zhao
  1 sibling, 1 reply; 19+ messages in thread
From: Binbin Wu @ 2024-12-03  2:33 UTC (permalink / raw)
  To: Rick Edgecombe
  Cc: kvm, pbonzini, seanjc, dave.hansen, isaku.yamahata, kai.huang,
	linux-kernel, tony.lindgren, xiaoyao.li, yan.y.zhao, x86,
	adrian.hunter, Isaku Yamahata, Yuan Yao




On 12/3/2024 9:03 AM, Rick Edgecombe wrote:
[...]
> +
> +/*
> + * TDX ABI defines output operands as PT, OWNER and SIZE. These are TDX defined fomats.
fomats -> formats

> + * So despite the names, they must be interpted specially as described by the spec. Return
interpted -> interpreted

> + * them only for error reporting purposes.
> + */
> +u64 tdh_phymem_page_reclaim(struct page *page, u64 *tdx_pt, u64 *tdx_owner, u64 *tdx_size)
> +{
> +	struct tdx_module_args args = {
> +		.rcx = page_to_pfn(page) << PAGE_SHIFT,
> +	};
> +	u64 ret;
> +
> +	ret = seamcall_ret(TDH_PHYMEM_PAGE_RECLAIM, &args);
> +
> +	*tdx_pt = args.rcx;
> +	*tdx_owner = args.rdx;
> +	*tdx_size = args.r8;
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(tdh_phymem_page_reclaim);
> +
[...]


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 0/6] SEAMCALL Wrappers
  2024-12-03  1:03 [RFC PATCH v2 0/6] SEAMCALL Wrappers Rick Edgecombe
                   ` (5 preceding siblings ...)
  2024-12-03  1:03 ` [RFC PATCH v2 6/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX flush operations Rick Edgecombe
@ 2024-12-04  1:24 ` Huang, Kai
  2024-12-04  1:57   ` Edgecombe, Rick P
  6 siblings, 1 reply; 19+ messages in thread
From: Huang, Kai @ 2024-12-04  1:24 UTC (permalink / raw)
  To: kvm@vger.kernel.org, pbonzini@redhat.com, Hansen, Dave,
	seanjc@google.com, Edgecombe, Rick P
  Cc: Li, Xiaoyao, x86@kernel.org, Hunter, Adrian,
	linux-kernel@vger.kernel.org, isaku.yamahata@gmail.com,
	Zhao, Yan Y, tony.lindgren@linux.intel.com

> 
> This is because it will not vary between vCPUs. Doing it that way 
> basically preserves the existing data duplication, but these counts are 
> basically "global metadata". The global metadata patches export them as a 
> size, but KVM wants to use them as a page count. So we should not be 
> including these counts in each TD scoped structure as is currently done. To
> address the duplication we need to change the "global metadata patches"
> to export the count instead of size.
> 

Currently the global metadata reading script generates the struct member based
on the "field name" of the JSON file.  The JSON file stores them as "size":

  "TDR_BASE_SIZE", "TDCS_BASE_SIZE", "TDVPS_BASE_SIZE"

We will need to tweak the script to map "metadata field name" to "kernel
structure member name", and more "special handling for specific fields" when
auto generating the code.

It's feasible but I am not sure whether it's worth to do, since we are basically
talking about converting size to page count.

Also, from global metadata's point of view, perhaps it is also good to just
provide a metadata which is consistent with what module reports.  How kernel
uses the metadata is another layer on top of it.

Btw, perhaps we don't need to keep 'tdcs_nr_pages' and 'tdcx_nr_pages' in
'struct tdx_td', i.e., as per-TD variables.  They are constants for all TDX
guests.

E.g., assuming KVM is still going to use them, it can just access them using the
metadata structure:

	static inline int tdx_tdcs_nr_pages(void)
	{
		return tdx_sysinfo->td_ctrl.tdcx_base_size >> PAGE_SHIFT;
	}

AFAICT they are only used when creating/destroying TD for a couple of times, so
I assume doing ">> PAGE_SHIFT" a couple of times won't really matter. 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 0/6] SEAMCALL Wrappers
  2024-12-04  1:24 ` [RFC PATCH v2 0/6] SEAMCALL Wrappers Huang, Kai
@ 2024-12-04  1:57   ` Edgecombe, Rick P
  0 siblings, 0 replies; 19+ messages in thread
From: Edgecombe, Rick P @ 2024-12-04  1:57 UTC (permalink / raw)
  To: kvm@vger.kernel.org, pbonzini@redhat.com, Hansen, Dave,
	seanjc@google.com, Huang, Kai
  Cc: Li, Xiaoyao, tony.lindgren@linux.intel.com, Hunter, Adrian,
	linux-kernel@vger.kernel.org, isaku.yamahata@gmail.com,
	x86@kernel.org, Zhao, Yan Y

On Wed, 2024-12-04 at 01:24 +0000, Huang, Kai wrote:
> Currently the global metadata reading script generates the struct member based
> on the "field name" of the JSON file.  The JSON file stores them as "size":
> 
>   "TDR_BASE_SIZE", "TDCS_BASE_SIZE", "TDVPS_BASE_SIZE"
> 
> We will need to tweak the script to map "metadata field name" to "kernel
> structure member name", and more "special handling for specific fields" when
> auto generating the code.
> 
> It's feasible but I am not sure whether it's worth to do, since we are basically
> talking about converting size to page count.

Ah, right. So given that this is generated code, we should probably just add the
wrappers like you suggest. In any case, we should remove the counts from the new
arch/x86 structs.

> 
> Also, from global metadata's point of view, perhaps it is also good to just
> provide a metadata which is consistent with what module reports.  How kernel
> uses the metadata is another layer on top of it.

I'm not sure I buy this one though. The exported arch/x86 interface shouldn't
have to match the HW directly.

> 
> Btw, perhaps we don't need to keep 'tdcs_nr_pages' and 'tdcx_nr_pages' in
> 'struct tdx_td', i.e., as per-TD variables.  They are constants for all TDX
> guests.
> 
> E.g., assuming KVM is still going to use them, it can just access them using the
> metadata structure:
> 
> 	static inline int tdx_tdcs_nr_pages(void)
> 	{
> 		return tdx_sysinfo->td_ctrl.tdcx_base_size >> PAGE_SHIFT;
> 	}
> 
> AFAICT they are only used when creating/destroying TD for a couple of times, so
> I assume doing ">> PAGE_SHIFT" a couple of times won't really matter.

None of the users are in fast paths. Using page count directly would be more
about reducing wrapper clutter.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 2/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX TD creation
  2024-12-03  2:20   ` Binbin Wu
@ 2024-12-04  1:58     ` Edgecombe, Rick P
  0 siblings, 0 replies; 19+ messages in thread
From: Edgecombe, Rick P @ 2024-12-04  1:58 UTC (permalink / raw)
  To: binbin.wu@linux.intel.com
  Cc: Hansen, Dave, seanjc@google.com, Huang, Kai, x86@kernel.org,
	yuan.yao@intel.com, Li, Xiaoyao, isaku.yamahata@gmail.com,
	linux-kernel@vger.kernel.org, tony.lindgren@linux.intel.com,
	kvm@vger.kernel.org, pbonzini@redhat.com, Yamahata, Isaku,
	Hunter, Adrian, Zhao, Yan Y

On Tue, 2024-12-03 at 10:20 +0800, Binbin Wu wrote:
> > +/*
> > + * 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(struct page *tdr)
> The argument should have a generic name instead of tdr, because it's not
> limited to TDR.

Doh, yes. Thanks.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 4/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX page cache management
  2024-12-03  2:33   ` Binbin Wu
@ 2024-12-04  1:58     ` Edgecombe, Rick P
  0 siblings, 0 replies; 19+ messages in thread
From: Edgecombe, Rick P @ 2024-12-04  1:58 UTC (permalink / raw)
  To: binbin.wu@linux.intel.com
  Cc: Hansen, Dave, seanjc@google.com, Huang, Kai, x86@kernel.org,
	yuan.yao@intel.com, Li, Xiaoyao, isaku.yamahata@gmail.com,
	linux-kernel@vger.kernel.org, tony.lindgren@linux.intel.com,
	kvm@vger.kernel.org, pbonzini@redhat.com, Yamahata, Isaku,
	Hunter, Adrian, Zhao, Yan Y

On Tue, 2024-12-03 at 10:33 +0800, Binbin Wu wrote:
> > + * TDX ABI defines output operands as PT, OWNER and SIZE. These are TDX
> > defined fomats.
> fomats -> formats
> 
> > + * So despite the names, they must be interpted specially as described by
> > the spec. Return
> interpted -> interpreted

Oof, thanks.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 2/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX TD creation
  2024-12-03  1:03 ` [RFC PATCH v2 2/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX TD creation Rick Edgecombe
  2024-12-03  2:20   ` Binbin Wu
@ 2024-12-04 19:54   ` Dave Hansen
  2024-12-05 17:25     ` Edgecombe, Rick P
  1 sibling, 1 reply; 19+ messages in thread
From: Dave Hansen @ 2024-12-04 19:54 UTC (permalink / raw)
  To: Rick Edgecombe, kvm, pbonzini, seanjc
  Cc: isaku.yamahata, kai.huang, linux-kernel, tony.lindgren,
	xiaoyao.li, yan.y.zhao, x86, adrian.hunter, Isaku Yamahata,
	Binbin Wu, Yuan Yao

On 12/2/24 17:03, Rick Edgecombe wrote:
> +u64 tdh_mng_addcx(struct tdx_td *td, struct page *tdcs_page)
> +{
> +	struct tdx_module_args args = {
> +		.rcx = page_to_pfn(tdcs_page) << PAGE_SHIFT,

This is a nit, but there is a page_to_phys().

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 5/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX VM/vCPU field access
  2024-12-03  1:03 ` [RFC PATCH v2 5/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX VM/vCPU field access Rick Edgecombe
@ 2024-12-04 19:57   ` Dave Hansen
  2024-12-05 17:33     ` Edgecombe, Rick P
  0 siblings, 1 reply; 19+ messages in thread
From: Dave Hansen @ 2024-12-04 19:57 UTC (permalink / raw)
  To: Rick Edgecombe, kvm, pbonzini, seanjc
  Cc: isaku.yamahata, kai.huang, linux-kernel, tony.lindgren,
	xiaoyao.li, yan.y.zhao, x86, adrian.hunter, Isaku Yamahata,
	Binbin Wu, Yuan Yao

On 12/2/24 17:03, Rick Edgecombe wrote:
> +u64 tdh_vp_wr(struct tdx_vp *vp, u64 field, u64 data, u64 mask)
> +{
> +	struct tdx_module_args args = {
> +		.rcx = tdx_tdvpr_pa(vp),
> +		.rdx = field,
> +		.r8 = data,
> +		.r9 = mask,
> +	};
> +
> +	return seamcall(TDH_VP_WR, &args);
> +}
> +EXPORT_SYMBOL_GPL(tdh_vp_wr);

There's a bit more tweaking you could _probably_ do here like giving
'field' a real type that means something, probably an enum. But that's
well into nitpicky territory and might not buy anything in practice.

Overall this set looks fine to me. The types are much more safe and
helpers are much more self-explanatory.  So, for the series:

Acked-by: Dave Hansen <dave.hansen@linux.intel.com>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 2/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX TD creation
  2024-12-04 19:54   ` Dave Hansen
@ 2024-12-05 17:25     ` Edgecombe, Rick P
  0 siblings, 0 replies; 19+ messages in thread
From: Edgecombe, Rick P @ 2024-12-05 17:25 UTC (permalink / raw)
  To: kvm@vger.kernel.org, pbonzini@redhat.com, Hansen, Dave,
	seanjc@google.com
  Cc: yuan.yao@intel.com, Huang, Kai, x86@kernel.org,
	binbin.wu@linux.intel.com, Li, Xiaoyao,
	linux-kernel@vger.kernel.org, Zhao, Yan Y,
	tony.lindgren@linux.intel.com, isaku.yamahata@gmail.com,
	Hunter, Adrian, Yamahata, Isaku

On Wed, 2024-12-04 at 11:54 -0800, Dave Hansen wrote:
> On 12/2/24 17:03, Rick Edgecombe wrote:
> > +u64 tdh_mng_addcx(struct tdx_td *td, struct page *tdcs_page)
> > +{
> > +	struct tdx_module_args args = {
> > +		.rcx = page_to_pfn(tdcs_page) << PAGE_SHIFT,
> 
> This is a nit, but there is a page_to_phys().

I almost used that, but the macro casts to dma_addr_t which didn't quite fit
these callers. Seems ok though.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 5/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX VM/vCPU field access
  2024-12-04 19:57   ` Dave Hansen
@ 2024-12-05 17:33     ` Edgecombe, Rick P
  0 siblings, 0 replies; 19+ messages in thread
From: Edgecombe, Rick P @ 2024-12-05 17:33 UTC (permalink / raw)
  To: kvm@vger.kernel.org, pbonzini@redhat.com, Hansen, Dave,
	seanjc@google.com
  Cc: Huang, Kai, x86@kernel.org, binbin.wu@linux.intel.com,
	Li, Xiaoyao, linux-kernel@vger.kernel.org, Zhao, Yan Y,
	tony.lindgren@linux.intel.com, isaku.yamahata@gmail.com,
	Hunter, Adrian, Yamahata, Isaku

On Wed, 2024-12-04 at 11:57 -0800, Dave Hansen wrote:
> On 12/2/24 17:03, Rick Edgecombe wrote:
> > +u64 tdh_vp_wr(struct tdx_vp *vp, u64 field, u64 data, u64 mask)
> > +{
> > +	struct tdx_module_args args = {
> > +		.rcx = tdx_tdvpr_pa(vp),
> > +		.rdx = field,
> > +		.r8 = data,
> > +		.r9 = mask,
> > +	};
> > +
> > +	return seamcall(TDH_VP_WR, &args);
> > +}
> > +EXPORT_SYMBOL_GPL(tdh_vp_wr);
> 
> There's a bit more tweaking you could _probably_ do here like giving
> 'field' a real type that means something, probably an enum. But that's
> well into nitpicky territory and might not buy anything in practice.
> 
> Overall this set looks fine to me. The types are much more safe and
> helpers are much more self-explanatory.  So, for the series:
> 
> Acked-by: Dave Hansen <dave.hansen@linux.intel.com>

Thanks Dave!

We have two more batches of SEAMCALLs required for base support. A bunch for
managing the S-EPT, and the single multiplexed TDH.VP.ENTER one.

Next, Yan is going to take this general scheme and post another RFC series with
the S-EPT ones for review.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 4/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX page cache management
  2024-12-03  1:03 ` [RFC PATCH v2 4/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX page cache management Rick Edgecombe
  2024-12-03  2:33   ` Binbin Wu
@ 2024-12-11  1:23   ` Yan Zhao
  2024-12-11  1:33     ` Edgecombe, Rick P
  1 sibling, 1 reply; 19+ messages in thread
From: Yan Zhao @ 2024-12-11  1:23 UTC (permalink / raw)
  To: Rick Edgecombe
  Cc: kvm, pbonzini, seanjc, dave.hansen, isaku.yamahata, kai.huang,
	linux-kernel, tony.lindgren, xiaoyao.li, x86, adrian.hunter,
	Isaku Yamahata, Binbin Wu, Yuan Yao

On Mon, Dec 02, 2024 at 05:03:14PM -0800, Rick Edgecombe wrote:
...
> +u64 tdh_phymem_page_wbinvd_tdr(struct tdx_td *td)
> +{
> +	struct tdx_module_args args = {};
> +
> +	args.rcx = tdx_tdr_pa(td) | ((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);
The tdx_global_keyid is of type u16 in TDX spec and TDX module.
As Reinette pointed out, u64 could cause overflow.

Do we need to change all keyids to u16, including those in
tdh.mng.create() in patch 2,
the global_keyid, tdx_guest_keyid_start in arch/x86/virt/vmx/tdx/tdx.c
and kvm_tdx->hkid in arch/x86/kvm/vmx/tdx.c ?

BTW, is it a good idea to move set_hkid_to_hpa() from KVM TDX to x86 common
header?

static __always_inline hpa_t set_hkid_to_hpa(hpa_t pa, u16 hkid)
{
        return pa | ((hpa_t)hkid << boot_cpu_data.x86_phys_bits);
}

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 4/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX page cache management
  2024-12-11  1:23   ` Yan Zhao
@ 2024-12-11  1:33     ` Edgecombe, Rick P
  0 siblings, 0 replies; 19+ messages in thread
From: Edgecombe, Rick P @ 2024-12-11  1:33 UTC (permalink / raw)
  To: Zhao, Yan Y
  Cc: Hansen, Dave, seanjc@google.com, Huang, Kai, x86@kernel.org,
	binbin.wu@linux.intel.com, Li, Xiaoyao, isaku.yamahata@gmail.com,
	linux-kernel@vger.kernel.org, tony.lindgren@linux.intel.com,
	kvm@vger.kernel.org, pbonzini@redhat.com, Yamahata, Isaku,
	Hunter, Adrian, yuan.yao@intel.com

On Wed, 2024-12-11 at 09:23 +0800, Yan Zhao wrote:
> On Mon, Dec 02, 2024 at 05:03:14PM -0800, Rick Edgecombe wrote:
> ...
> > +u64 tdh_phymem_page_wbinvd_tdr(struct tdx_td *td)
> > +{
> > +	struct tdx_module_args args = {};
> > +
> > +	args.rcx = tdx_tdr_pa(td) | ((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);
> The tdx_global_keyid is of type u16 in TDX spec and TDX module.
> As Reinette pointed out, u64 could cause overflow.
> 
> Do we need to change all keyids to u16, including those in
> tdh.mng.create() in patch 2,
> the global_keyid, tdx_guest_keyid_start in arch/x86/virt/vmx/tdx/tdx.c
> and kvm_tdx->hkid in arch/x86/kvm/vmx/tdx.c ?

It seems like a good idea.

> 
> BTW, is it a good idea to move set_hkid_to_hpa() from KVM TDX to x86 common
> header?
> 
> static __always_inline hpa_t set_hkid_to_hpa(hpa_t pa, u16 hkid)
> {
>         return pa | ((hpa_t)hkid << boot_cpu_data.x86_phys_bits);
> }

Ah, yep.

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2024-12-11  1:33 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-03  1:03 [RFC PATCH v2 0/6] SEAMCALL Wrappers Rick Edgecombe
2024-12-03  1:03 ` [RFC PATCH v2 1/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX KeyID management Rick Edgecombe
2024-12-03  1:03 ` [RFC PATCH v2 2/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX TD creation Rick Edgecombe
2024-12-03  2:20   ` Binbin Wu
2024-12-04  1:58     ` Edgecombe, Rick P
2024-12-04 19:54   ` Dave Hansen
2024-12-05 17:25     ` Edgecombe, Rick P
2024-12-03  1:03 ` [RFC PATCH v2 3/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX vCPU creation Rick Edgecombe
2024-12-03  1:03 ` [RFC PATCH v2 4/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX page cache management Rick Edgecombe
2024-12-03  2:33   ` Binbin Wu
2024-12-04  1:58     ` Edgecombe, Rick P
2024-12-11  1:23   ` Yan Zhao
2024-12-11  1:33     ` Edgecombe, Rick P
2024-12-03  1:03 ` [RFC PATCH v2 5/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX VM/vCPU field access Rick Edgecombe
2024-12-04 19:57   ` Dave Hansen
2024-12-05 17:33     ` Edgecombe, Rick P
2024-12-03  1:03 ` [RFC PATCH v2 6/6] x86/virt/tdx: Add SEAMCALL wrappers for TDX flush operations Rick Edgecombe
2024-12-04  1:24 ` [RFC PATCH v2 0/6] SEAMCALL Wrappers Huang, Kai
2024-12-04  1:57   ` Edgecombe, Rick P

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox