From: Kai Huang kai.huang@intel.com
[ Upstream commit e414b1005891d74bb0c3d27684c58dfbfbd1754b ]
All of the x86 KVM guest types (VMX, SEV and TDX) do some special context tracking when entering guests. This means that the actual guest entry sequence must be noinstr.
Part of entering a TDX guest is passing a physical address to the TDX module. Right now, that physical address is stored as a 'struct page' and converted to a physical address at guest entry. That page=>phys conversion can be complicated, can vary greatly based on kernel config, and it is definitely _not_ a noinstr path today.
There have been a number of tinkering approaches to try and fix this up, but they all fall down due to some part of the page=>phys conversion infrastructure not being noinstr friendly.
Precalculate the page=>phys conversion and store it in the existing 'tdx_vp' structure. Use the new field at every site that needs a tdvpr physical address. Remove the now redundant tdx_tdvpr_pa(). Remove the __flatten remnant from the tinkering.
Note that only one user of the new field is actually noinstr. All others can use page_to_phys(). But, they might as well save the effort since there is a pre-calculated value sitting there for them.
[ dhansen: rewrite all the text ]
Signed-off-by: Kai Huang kai.huang@intel.com Signed-off-by: Dave Hansen dave.hansen@linux.intel.com Reviewed-by: Kiryl Shutsemau kas@kernel.org Tested-by: Farrah Chen farrah.chen@intel.com Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
Rationale - Fixes a real correctness bug in a critical noinstr path. Entering a TDX guest must be noinstr; previously, `tdh_vp_enter()` converted a `struct page` to a physical address at runtime, which is not noinstr- safe. The patch removes the runtime `page_to_phys()` from the guest- entry path and uses a precomputed physical address instead. - Minimal, contained change in the TDX/KVM code. No ABI changes; all updates are internal to TDX vCPU state and seamcall wrappers.
Key Changes - Precompute and store the TDVPR physical address: - Adds `phys_addr_t tdvpr_pa;` to `struct tdx_vp` to hold `page_to_phys(tdvpr_page)` for reuse in noinstr code: arch/x86/include/asm/tdx.h:171. - Computes and assigns the field during vCPU init, with an explicit comment explaining noinstr constraints: arch/x86/kvm/vmx/tdx.c:2936. - Clears the field on free/error paths to avoid stale use: arch/x86/kvm/vmx/tdx.c:855, arch/x86/kvm/vmx/tdx.c:3004. - Make the guest entry truly noinstr: - `tdh_vp_enter()` now uses the precomputed `td->tdvpr_pa` and stays within noinstr constraints: arch/x86/virt/vmx/tdx/tdx.c:1518. - Also removes the `__flatten` remnant and wraps the seamcall with the cache-dirty helper, aligning with other TDX seamcall usage. - Replace page->phys conversions with the precomputed value at all sites that use the TDVPR: - Updated callers pass `vp->tdvpr_pa` instead of recomputing: arch/x86/virt/vmx/tdx/tdx.c:1581, 1650, 1706, 1752, 1769, 1782. - Removes the now-redundant inline helper that did `page_to_phys()` for TDVPR.
Why This Fits Stable - User impact: Fixes potential WARN/BUG and undefined behavior from invoking non-noinstr code in a noinstr entry path for TDX guests. This can affect real deployments using debug/instrumented kernels and is correctness-critical for a guest entry path. - Scope and risk: Small, straightforward refactor; adds one cached field and replaces callers to use it. Memory lifetime is well-defined (page is allocated at init and reclaimed at teardown), and the physical address of a page is stable; zeroing on teardown/error prevents stale usage. - No feature or architectural changes; KVM/TDX only. No user-visible ABI changes. The seamcall helper infrastructure (`__seamcall_dirty_cache`, `__seamcall_saved_ret`) is already present in this subsystem. - Reviewed and tested upstream (Reviewed-by/Tested-by tags), and consistent with prior attempts to fix noinstr issues (this replaces earlier, more fragile approaches like `__flatten`).
Conclusion - This is a low-risk, correctness fix to a critical guest-entry path, improving noinstr compliance. It should be backported to stable kernels that have TDX support.
arch/x86/include/asm/tdx.h | 2 ++ arch/x86/kvm/vmx/tdx.c | 9 +++++++++ arch/x86/virt/vmx/tdx/tdx.c | 21 ++++++++------------- 3 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h index 0922265c6bdcb..17a051d9c9398 100644 --- a/arch/x86/include/asm/tdx.h +++ b/arch/x86/include/asm/tdx.h @@ -169,6 +169,8 @@ struct tdx_td { struct tdx_vp { /* TDVP root page */ struct page *tdvpr_page; + /* precalculated page_to_phys(tdvpr_page) for use in noinstr code */ + phys_addr_t tdvpr_pa;
/* TD vCPU control structure: */ struct page **tdcx_pages; diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c index d91d9d6bb26c1..987c0eb10545c 100644 --- a/arch/x86/kvm/vmx/tdx.c +++ b/arch/x86/kvm/vmx/tdx.c @@ -861,6 +861,7 @@ void tdx_vcpu_free(struct kvm_vcpu *vcpu) if (tdx->vp.tdvpr_page) { tdx_reclaim_control_page(tdx->vp.tdvpr_page); tdx->vp.tdvpr_page = 0; + tdx->vp.tdvpr_pa = 0; }
tdx->state = VCPU_TD_STATE_UNINITIALIZED; @@ -2940,6 +2941,13 @@ static int tdx_td_vcpu_init(struct kvm_vcpu *vcpu, u64 vcpu_rcx) return -ENOMEM; tdx->vp.tdvpr_page = page;
+ /* + * page_to_phys() does not work in 'noinstr' code, like guest + * entry via tdh_vp_enter(). Precalculate and store it instead + * of doing it at runtime later. + */ + tdx->vp.tdvpr_pa = page_to_phys(tdx->vp.tdvpr_page); + tdx->vp.tdcx_pages = kcalloc(kvm_tdx->td.tdcx_nr_pages, sizeof(*tdx->vp.tdcx_pages), GFP_KERNEL); if (!tdx->vp.tdcx_pages) { @@ -3002,6 +3010,7 @@ static int tdx_td_vcpu_init(struct kvm_vcpu *vcpu, u64 vcpu_rcx) if (tdx->vp.tdvpr_page) __free_page(tdx->vp.tdvpr_page); tdx->vp.tdvpr_page = 0; + tdx->vp.tdvpr_pa = 0;
return ret; } diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c index 3ea6f587c81a3..b54581a795f5b 100644 --- a/arch/x86/virt/vmx/tdx/tdx.c +++ b/arch/x86/virt/vmx/tdx/tdx.c @@ -1502,11 +1502,6 @@ static inline u64 tdx_tdr_pa(struct tdx_td *td) return page_to_phys(td->tdr_page); }
-static inline u64 tdx_tdvpr_pa(struct tdx_vp *td) -{ - return page_to_phys(td->tdvpr_page); -} - /* * 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. @@ -1518,9 +1513,9 @@ static void tdx_clflush_page(struct page *page) clflush_cache_range(page_to_virt(page), PAGE_SIZE); }
-noinstr __flatten u64 tdh_vp_enter(struct tdx_vp *td, struct tdx_module_args *args) +noinstr u64 tdh_vp_enter(struct tdx_vp *td, struct tdx_module_args *args) { - args->rcx = tdx_tdvpr_pa(td); + args->rcx = td->tdvpr_pa;
return __seamcall_dirty_cache(__seamcall_saved_ret, TDH_VP_ENTER, args); } @@ -1581,7 +1576,7 @@ u64 tdh_vp_addcx(struct tdx_vp *vp, struct page *tdcx_page) { struct tdx_module_args args = { .rcx = page_to_phys(tdcx_page), - .rdx = tdx_tdvpr_pa(vp), + .rdx = vp->tdvpr_pa, };
tdx_clflush_page(tdcx_page); @@ -1650,7 +1645,7 @@ 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), + .rcx = vp->tdvpr_pa, .rdx = tdx_tdr_pa(td), };
@@ -1706,7 +1701,7 @@ EXPORT_SYMBOL_GPL(tdh_mr_finalize); u64 tdh_vp_flush(struct tdx_vp *vp) { struct tdx_module_args args = { - .rcx = tdx_tdvpr_pa(vp), + .rcx = vp->tdvpr_pa, };
return seamcall(TDH_VP_FLUSH, &args); @@ -1752,7 +1747,7 @@ EXPORT_SYMBOL_GPL(tdh_mng_init); u64 tdh_vp_rd(struct tdx_vp *vp, u64 field, u64 *data) { struct tdx_module_args args = { - .rcx = tdx_tdvpr_pa(vp), + .rcx = vp->tdvpr_pa, .rdx = field, }; u64 ret; @@ -1769,7 +1764,7 @@ 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), + .rcx = vp->tdvpr_pa, .rdx = field, .r8 = data, .r9 = mask, @@ -1782,7 +1777,7 @@ EXPORT_SYMBOL_GPL(tdh_vp_wr); u64 tdh_vp_init(struct tdx_vp *vp, u64 initial_rcx, u32 x2apicid) { struct tdx_module_args args = { - .rcx = tdx_tdvpr_pa(vp), + .rcx = vp->tdvpr_pa, .rdx = initial_rcx, .r8 = x2apicid, };