* [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* 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 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 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 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
* [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* 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 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 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
* [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* 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 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
* [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 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