* [PATCH V3 0/2] x86/tdx: Skip clearing reclaimed pages unless X86_BUG_TDX_PW_MCE is present
@ 2025-07-22 13:15 Adrian Hunter
2025-07-22 13:15 ` [PATCH V3 1/2] x86/tdx: Eliminate duplicate code in tdx_clear_page() Adrian Hunter
2025-07-22 13:15 ` [PATCH V3 2/2] x86/tdx: Skip clearing reclaimed pages unless X86_BUG_TDX_PW_MCE is present Adrian Hunter
0 siblings, 2 replies; 6+ messages in thread
From: Adrian Hunter @ 2025-07-22 13:15 UTC (permalink / raw)
To: Dave Hansen, pbonzini, seanjc, vannapurve
Cc: Tony Luck, Borislav Petkov, Thomas Gleixner, Ingo Molnar, x86,
H Peter Anvin, linux-kernel, kvm, rick.p.edgecombe, kas,
kai.huang, reinette.chatre, xiaoyao.li, tony.lindgren, binbin.wu,
isaku.yamahata, yan.y.zhao, chao.gao
Hi
Here are 2 small self-explanatory patches related to clearing TDX private
pages.
Patch 1 is a minor tidy-up.
In patch 2, by skipping the clearing step, shutdown time can improve by
up to 40%.
Changes in V3:
x86/tdx: Eliminate duplicate code in tdx_clear_page()
Explain "quirk" rename in commit message (Rick)
Explain mb() change in commit message (Rick)
Add Rev'd-by, Ack'd-by tags
x86/tdx: Skip clearing reclaimed pages unless X86_BUG_TDX_PW_MCE is present
Remove "flush cache" comments (Rick)
Update function comment to better relate to "quirk" naming (Rick)
Add "via MOVDIR64B" to comment (Xiaoyao)
Add Rev'd-by, Ack'd-by tags
Changes in V2 (as requested by Dave):
x86/tdx: Eliminate duplicate code in tdx_clear_page()
Rename reset_tdx_pages() to tdx_quirk_reset_paddr()
Call tdx_quirk_reset_paddr() directly
x86/tdx: Skip clearing reclaimed pages unless X86_BUG_TDX_PW_MCE is present
Improve the comment
Adrian Hunter (2):
x86/tdx: Eliminate duplicate code in tdx_clear_page()
x86/tdx: Skip clearing reclaimed pages unless X86_BUG_TDX_PW_MCE is present
arch/x86/include/asm/tdx.h | 2 ++
arch/x86/kvm/vmx/tdx.c | 25 +++----------------------
arch/x86/virt/vmx/tdx/tdx.c | 15 ++++++++++-----
3 files changed, 15 insertions(+), 27 deletions(-)
Regards
Adrian
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH V3 1/2] x86/tdx: Eliminate duplicate code in tdx_clear_page()
2025-07-22 13:15 [PATCH V3 0/2] x86/tdx: Skip clearing reclaimed pages unless X86_BUG_TDX_PW_MCE is present Adrian Hunter
@ 2025-07-22 13:15 ` Adrian Hunter
2025-07-22 14:11 ` Sean Christopherson
2025-07-22 13:15 ` [PATCH V3 2/2] x86/tdx: Skip clearing reclaimed pages unless X86_BUG_TDX_PW_MCE is present Adrian Hunter
1 sibling, 1 reply; 6+ messages in thread
From: Adrian Hunter @ 2025-07-22 13:15 UTC (permalink / raw)
To: Dave Hansen, pbonzini, seanjc, vannapurve
Cc: Tony Luck, Borislav Petkov, Thomas Gleixner, Ingo Molnar, x86,
H Peter Anvin, linux-kernel, kvm, rick.p.edgecombe, kas,
kai.huang, reinette.chatre, xiaoyao.li, tony.lindgren, binbin.wu,
isaku.yamahata, yan.y.zhao, chao.gao
tdx_clear_page() and reset_tdx_pages() duplicate the TDX page clearing
logic. Rename reset_tdx_pages() to tdx_quirk_reset_paddr() and use it
in place of tdx_clear_page().
The new name reflects that, in fact, the clearing is necessary only for
hardware with a certain quirk. That is dealt with in a subsequent patch
but doing the rename here avoids additional churn.
Note reset_tdx_pages() is slightly different from tdx_clear_page() because,
more appropriately, it uses mb() in place of __mb(). Except when extra
debugging is enabled (kcsan at present), mb() just calls __mb().
Reviewed-by: Kirill A. Shutemov <kas@kernel.org>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
Acked-by: Kai Huang <kai.huang@intel.com>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
Changes in V3:
Explain "quirk" rename in commit message (Rick)
Explain mb() change in commit message (Rick)
Add Rev'd-by, Ack'd-by tags
Changes in V2:
Rename reset_tdx_pages() to tdx_quirk_reset_paddr()
Call tdx_quirk_reset_paddr() directly
arch/x86/include/asm/tdx.h | 2 ++
arch/x86/kvm/vmx/tdx.c | 25 +++----------------------
arch/x86/virt/vmx/tdx/tdx.c | 5 +++--
3 files changed, 8 insertions(+), 24 deletions(-)
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 7ddef3a69866..f66328404724 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -131,6 +131,8 @@ int tdx_guest_keyid_alloc(void);
u32 tdx_get_nr_guest_keyids(void);
void tdx_guest_keyid_free(unsigned int keyid);
+void tdx_quirk_reset_paddr(unsigned long base, unsigned long size);
+
struct tdx_td {
/* TD root structure: */
struct page *tdr_page;
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index 573d6f7d1694..1b549de6da06 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -283,25 +283,6 @@ static inline void tdx_disassociate_vp(struct kvm_vcpu *vcpu)
vcpu->cpu = -1;
}
-static void tdx_clear_page(struct page *page)
-{
- const void *zero_page = (const void *) page_to_virt(ZERO_PAGE(0));
- void *dest = page_to_virt(page);
- unsigned long i;
-
- /*
- * The page could have been poisoned. MOVDIR64B also clears
- * the poison bit so the kernel can safely use the page again.
- */
- for (i = 0; i < PAGE_SIZE; i += 64)
- movdir64b(dest + i, zero_page);
- /*
- * MOVDIR64B store uses WC buffer. Prevent following memory reads
- * from seeing potentially poisoned cache.
- */
- __mb();
-}
-
static void tdx_no_vcpus_enter_start(struct kvm *kvm)
{
struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
@@ -347,7 +328,7 @@ static int tdx_reclaim_page(struct page *page)
r = __tdx_reclaim_page(page);
if (!r)
- tdx_clear_page(page);
+ tdx_quirk_reset_paddr(page_to_phys(page), PAGE_SIZE);
return r;
}
@@ -596,7 +577,7 @@ static void tdx_reclaim_td_control_pages(struct kvm *kvm)
pr_tdx_error(TDH_PHYMEM_PAGE_WBINVD, err);
return;
}
- tdx_clear_page(kvm_tdx->td.tdr_page);
+ tdx_quirk_reset_paddr(page_to_phys(kvm_tdx->td.tdr_page), PAGE_SIZE);
__free_page(kvm_tdx->td.tdr_page);
kvm_tdx->td.tdr_page = NULL;
@@ -1717,7 +1698,7 @@ static int tdx_sept_drop_private_spte(struct kvm *kvm, gfn_t gfn,
pr_tdx_error(TDH_PHYMEM_PAGE_WBINVD, err);
return -EIO;
}
- tdx_clear_page(page);
+ tdx_quirk_reset_paddr(page_to_phys(page), PAGE_SIZE);
tdx_unpin(kvm, page);
return 0;
}
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index c7a9a087ccaf..14d93ed05bd2 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -637,7 +637,7 @@ static int tdmrs_set_up_pamt_all(struct tdmr_info_list *tdmr_list,
* clear these pages. Note this function doesn't flush cache of
* these TDX private pages. The caller should make sure of that.
*/
-static void reset_tdx_pages(unsigned long base, unsigned long size)
+void tdx_quirk_reset_paddr(unsigned long base, unsigned long size)
{
const void *zero_page = (const void *)page_address(ZERO_PAGE(0));
unsigned long phys, end;
@@ -653,10 +653,11 @@ static void reset_tdx_pages(unsigned long base, unsigned long size)
*/
mb();
}
+EXPORT_SYMBOL_GPL(tdx_quirk_reset_paddr);
static void tdmr_reset_pamt(struct tdmr_info *tdmr)
{
- tdmr_do_pamt_func(tdmr, reset_tdx_pages);
+ tdmr_do_pamt_func(tdmr, tdx_quirk_reset_paddr);
}
static void tdmrs_reset_pamt_all(struct tdmr_info_list *tdmr_list)
--
2.48.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH V3 2/2] x86/tdx: Skip clearing reclaimed pages unless X86_BUG_TDX_PW_MCE is present
2025-07-22 13:15 [PATCH V3 0/2] x86/tdx: Skip clearing reclaimed pages unless X86_BUG_TDX_PW_MCE is present Adrian Hunter
2025-07-22 13:15 ` [PATCH V3 1/2] x86/tdx: Eliminate duplicate code in tdx_clear_page() Adrian Hunter
@ 2025-07-22 13:15 ` Adrian Hunter
2025-07-22 15:08 ` Edgecombe, Rick P
1 sibling, 1 reply; 6+ messages in thread
From: Adrian Hunter @ 2025-07-22 13:15 UTC (permalink / raw)
To: Dave Hansen, pbonzini, seanjc, vannapurve
Cc: Tony Luck, Borislav Petkov, Thomas Gleixner, Ingo Molnar, x86,
H Peter Anvin, linux-kernel, kvm, rick.p.edgecombe, kas,
kai.huang, reinette.chatre, xiaoyao.li, tony.lindgren, binbin.wu,
isaku.yamahata, yan.y.zhao, chao.gao
Avoid clearing reclaimed TDX private pages unless the platform is affected
by the X86_BUG_TDX_PW_MCE erratum. This significantly reduces VM shutdown
time on unaffected systems.
Background
KVM currently clears reclaimed TDX private pages using MOVDIR64B, which:
- Clears the TD Owner bit (which identifies TDX private memory) and
integrity metadata without triggering integrity violations.
- Clears poison from cache lines without consuming it, avoiding MCEs on
access (refer TDX Module Base spec. 16.5. Handling Machine Check
Events during Guest TD Operation).
The TDX module also uses MOVDIR64B to initialize private pages before use.
If cache flushing is needed, it sets TDX_FEATURES.CLFLUSH_BEFORE_ALLOC.
However, KVM currently flushes unconditionally, refer commit 94c477a751c7b
("x86/virt/tdx: Add SEAMCALL wrappers to add TD private pages")
In contrast, when private pages are reclaimed, the TDX Module handles
flushing via the TDH.PHYMEM.CACHE.WB SEAMCALL.
Problem
Clearing all private pages during VM shutdown is costly. For guests
with a large amount of memory it can take minutes.
Solution
TDX Module Base Architecture spec. documents that private pages reclaimed
from a TD should be initialized using MOVDIR64B, in order to avoid
integrity violation or TD bit mismatch detection when later being read
using a shared HKID, refer April 2025 spec. "Page Initialization" in
section "8.6.2. Platforms not Using ACT: Required Cache Flush and
Initialization by the Host VMM"
That is an overstatement and will be clarified in coming versions of the
spec. In fact, as outlined in "Table 16.2: Non-ACT Platforms Checks on
Memory" and "Table 16.3: Non-ACT Platforms Checks on Memory Reads in Li
Mode" in the same spec, there is no issue accessing such reclaimed pages
using a shared key that does not have integrity enabled. Linux always uses
KeyID 0 which never has integrity enabled. KeyID 0 is also the TME KeyID
which disallows integrity, refer "TME Policy/Encryption Algorithm" bit
description in "Intel Architecture Memory Encryption Technologies" spec
version 1.6 April 2025. So there is no need to clear pages to avoid
integrity violations.
There remains a risk of poison consumption. However, in the context of
TDX, it is expected that there would be a machine check associated with the
original poisoning. On some platforms that results in a panic. However
platforms may support "SEAM_NR" Machine Check capability, in which case
Linux machine check handler marks the page as poisoned, which prevents it
from being allocated anymore, refer commit 7911f145de5fe ("x86/mce:
Implement recovery for errors in TDX/SEAM non-root mode")
Improvement
By skipping the clearing step on unaffected platforms, shutdown time
can improve by up to 40%.
On platforms with the X86_BUG_TDX_PW_MCE erratum (SPR and EMR), continue
clearing because these platforms may trigger poison on partial writes to
previously-private pages, even with KeyID 0, refer commit 1e536e1068970
("x86/cpu: Detect TDX partial write machine check erratum")
Reviewed-by: Kirill A. Shutemov <kas@kernel.org>
Acked-by: Kai Huang <kai.huang@intel.com>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
Changes in V3:
Remove "flush cache" comments (Rick)
Update function comment to better relate to "quirk" naming (Rick)
Add "via MOVDIR64B" to comment (Xiaoyao)
Add Rev'd-by, Ack'd-by tags
Changes in V2:
Improve the comment
arch/x86/virt/vmx/tdx/tdx.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 14d93ed05bd2..a542e4fbf5a8 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -633,15 +633,19 @@ static int tdmrs_set_up_pamt_all(struct tdmr_info_list *tdmr_list,
}
/*
- * Convert TDX private pages back to normal by using MOVDIR64B to
- * clear these pages. Note this function doesn't flush cache of
- * these TDX private pages. The caller should make sure of that.
+ * Convert TDX private pages back to normal by using MOVDIR64B to clear these
+ * pages. Typically, any write to the page will convert it from TDX private back
+ * to normal kernel memory. Systems with the X86_BUG_TDX_PW_MCE erratum need to
+ * do the conversion explicitly via MOVDIR64B.
*/
void tdx_quirk_reset_paddr(unsigned long base, unsigned long size)
{
const void *zero_page = (const void *)page_address(ZERO_PAGE(0));
unsigned long phys, end;
+ if (!boot_cpu_has_bug(X86_BUG_TDX_PW_MCE))
+ return;
+
end = base + size;
for (phys = base; phys < end; phys += 64)
movdir64b(__va(phys), zero_page);
--
2.48.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH V3 1/2] x86/tdx: Eliminate duplicate code in tdx_clear_page()
2025-07-22 13:15 ` [PATCH V3 1/2] x86/tdx: Eliminate duplicate code in tdx_clear_page() Adrian Hunter
@ 2025-07-22 14:11 ` Sean Christopherson
2025-07-22 14:48 ` Adrian Hunter
0 siblings, 1 reply; 6+ messages in thread
From: Sean Christopherson @ 2025-07-22 14:11 UTC (permalink / raw)
To: Adrian Hunter
Cc: Dave Hansen, pbonzini, vannapurve, Tony Luck, Borislav Petkov,
Thomas Gleixner, Ingo Molnar, x86, H Peter Anvin, linux-kernel,
kvm, rick.p.edgecombe, kas, kai.huang, reinette.chatre,
xiaoyao.li, tony.lindgren, binbin.wu, isaku.yamahata, yan.y.zhao,
chao.gao
On Tue, Jul 22, 2025, Adrian Hunter wrote:
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 7ddef3a69866..f66328404724 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -131,6 +131,8 @@ int tdx_guest_keyid_alloc(void);
> u32 tdx_get_nr_guest_keyids(void);
> void tdx_guest_keyid_free(unsigned int keyid);
>
> +void tdx_quirk_reset_paddr(unsigned long base, unsigned long size);
> +
> struct tdx_td {
> /* TD root structure: */
> struct page *tdr_page;
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index 573d6f7d1694..1b549de6da06 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -283,25 +283,6 @@ static inline void tdx_disassociate_vp(struct kvm_vcpu *vcpu)
> vcpu->cpu = -1;
> }
>
> -static void tdx_clear_page(struct page *page)
> -{
> - const void *zero_page = (const void *) page_to_virt(ZERO_PAGE(0));
> - void *dest = page_to_virt(page);
> - unsigned long i;
> -
> - /*
> - * The page could have been poisoned. MOVDIR64B also clears
> - * the poison bit so the kernel can safely use the page again.
> - */
> - for (i = 0; i < PAGE_SIZE; i += 64)
> - movdir64b(dest + i, zero_page);
> - /*
> - * MOVDIR64B store uses WC buffer. Prevent following memory reads
> - * from seeing potentially poisoned cache.
> - */
> - __mb();
> -}
> -
> static void tdx_no_vcpus_enter_start(struct kvm *kvm)
> {
> struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
> @@ -347,7 +328,7 @@ static int tdx_reclaim_page(struct page *page)
>
> r = __tdx_reclaim_page(page);
> if (!r)
> - tdx_clear_page(page);
> + tdx_quirk_reset_paddr(page_to_phys(page), PAGE_SIZE);
This is silly. Literally every use in KVM is on a struct page. I agree with
Dave that having a wrapper with a completely unrelated name is confusing, but
that's a naming problem, not a code problem.
And FWIW, I find tdx_quirk_reset_paddr() confusing, because it reads like it's
resetting the address itself. But if KVM only ever uses tdx_quirk_reset_page(),
I don't care what you call the inner helper.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V3 1/2] x86/tdx: Eliminate duplicate code in tdx_clear_page()
2025-07-22 14:11 ` Sean Christopherson
@ 2025-07-22 14:48 ` Adrian Hunter
0 siblings, 0 replies; 6+ messages in thread
From: Adrian Hunter @ 2025-07-22 14:48 UTC (permalink / raw)
To: Sean Christopherson
Cc: Dave Hansen, pbonzini, vannapurve, Tony Luck, Borislav Petkov,
Thomas Gleixner, Ingo Molnar, x86, H Peter Anvin, linux-kernel,
kvm, rick.p.edgecombe, kas, kai.huang, reinette.chatre,
xiaoyao.li, tony.lindgren, binbin.wu, isaku.yamahata, yan.y.zhao,
chao.gao
On 22/07/2025 17:11, Sean Christopherson wrote:
> On Tue, Jul 22, 2025, Adrian Hunter wrote:
>> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
>> index 7ddef3a69866..f66328404724 100644
>> --- a/arch/x86/include/asm/tdx.h
>> +++ b/arch/x86/include/asm/tdx.h
>> @@ -131,6 +131,8 @@ int tdx_guest_keyid_alloc(void);
>> u32 tdx_get_nr_guest_keyids(void);
>> void tdx_guest_keyid_free(unsigned int keyid);
>>
>> +void tdx_quirk_reset_paddr(unsigned long base, unsigned long size);
>> +
>> struct tdx_td {
>> /* TD root structure: */
>> struct page *tdr_page;
>> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
>> index 573d6f7d1694..1b549de6da06 100644
>> --- a/arch/x86/kvm/vmx/tdx.c
>> +++ b/arch/x86/kvm/vmx/tdx.c
>> @@ -283,25 +283,6 @@ static inline void tdx_disassociate_vp(struct kvm_vcpu *vcpu)
>> vcpu->cpu = -1;
>> }
>>
>> -static void tdx_clear_page(struct page *page)
>> -{
>> - const void *zero_page = (const void *) page_to_virt(ZERO_PAGE(0));
>> - void *dest = page_to_virt(page);
>> - unsigned long i;
>> -
>> - /*
>> - * The page could have been poisoned. MOVDIR64B also clears
>> - * the poison bit so the kernel can safely use the page again.
>> - */
>> - for (i = 0; i < PAGE_SIZE; i += 64)
>> - movdir64b(dest + i, zero_page);
>> - /*
>> - * MOVDIR64B store uses WC buffer. Prevent following memory reads
>> - * from seeing potentially poisoned cache.
>> - */
>> - __mb();
>> -}
>> -
>> static void tdx_no_vcpus_enter_start(struct kvm *kvm)
>> {
>> struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
>> @@ -347,7 +328,7 @@ static int tdx_reclaim_page(struct page *page)
>>
>> r = __tdx_reclaim_page(page);
>> if (!r)
>> - tdx_clear_page(page);
>> + tdx_quirk_reset_paddr(page_to_phys(page), PAGE_SIZE);
>
> This is silly. Literally every use in KVM is on a struct page. I agree with
> Dave that having a wrapper with a completely unrelated name is confusing, but
> that's a naming problem, not a code problem.
>
> And FWIW, I find tdx_quirk_reset_paddr() confusing, because it reads like it's
> resetting the address itself. But if KVM only ever uses tdx_quirk_reset_page(),
> I don't care what you call the inner helper.
As you say, Dave's second option was:
"The alternative would be to retain a function that keeps the 'struct
page' as an argument. Something like:
tdx_quirk_reset_paddr(unsigned long base, unsigned long size)
and
tdx_quirk_reset_page(struct page *page)"
So I will do that for V4 unless there are further comments.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V3 2/2] x86/tdx: Skip clearing reclaimed pages unless X86_BUG_TDX_PW_MCE is present
2025-07-22 13:15 ` [PATCH V3 2/2] x86/tdx: Skip clearing reclaimed pages unless X86_BUG_TDX_PW_MCE is present Adrian Hunter
@ 2025-07-22 15:08 ` Edgecombe, Rick P
0 siblings, 0 replies; 6+ messages in thread
From: Edgecombe, Rick P @ 2025-07-22 15:08 UTC (permalink / raw)
To: Annapurve, Vishal, pbonzini@redhat.com, Hunter, Adrian,
seanjc@google.com, dave.hansen@linux.intel.com
Cc: kvm@vger.kernel.org, Li, Xiaoyao, Huang, Kai, Zhao, Yan Y,
Luck, Tony, kas@kernel.org, mingo@redhat.com, Chatre, Reinette,
tony.lindgren@linux.intel.com, tglx@linutronix.de,
Yamahata, Isaku, linux-kernel@vger.kernel.org,
binbin.wu@linux.intel.com, hpa@zytor.com, bp@alien8.de, Gao, Chao,
x86@kernel.org
On Tue, 2025-07-22 at 16:15 +0300, Adrian Hunter wrote:
> Avoid clearing reclaimed TDX private pages unless the platform is affected
> by the X86_BUG_TDX_PW_MCE erratum. This significantly reduces VM shutdown
> time on unaffected systems.
>
> Background
>
> KVM currently clears reclaimed TDX private pages using MOVDIR64B, which:
>
> - Clears the TD Owner bit (which identifies TDX private memory) and
> integrity metadata without triggering integrity violations.
> - Clears poison from cache lines without consuming it, avoiding MCEs on
> access (refer TDX Module Base spec. 16.5. Handling Machine Check
> Events during Guest TD Operation).
16.5 could move around. We probably want to put the date, or document version
like (348549-006US).
>
> The TDX module also uses MOVDIR64B to initialize private pages before use.
> If cache flushing is needed, it sets TDX_FEATURES.CLFLUSH_BEFORE_ALLOC.
> However, KVM currently flushes unconditionally, refer commit 94c477a751c7b
> ("x86/virt/tdx: Add SEAMCALL wrappers to add TD private pages")
>
> In contrast, when private pages are reclaimed, the TDX Module handles
> flushing via the TDH.PHYMEM.CACHE.WB SEAMCALL.
>
> Problem
>
> Clearing all private pages during VM shutdown is costly. For guests
> with a large amount of memory it can take minutes.
>
> Solution
>
> TDX Module Base Architecture spec. documents that private pages reclaimed
> from a TD should be initialized using MOVDIR64B, in order to avoid
> integrity violation or TD bit mismatch detection when later being read
> using a shared HKID, refer April 2025 spec. "Page Initialization" in
> section "8.6.2. Platforms not Using ACT: Required Cache Flush and
> Initialization by the Host VMM"
>
> That is an overstatement and will be clarified in coming versions of the
> spec. In fact, as outlined in "Table 16.2: Non-ACT Platforms Checks on
> Memory" and "Table 16.3: Non-ACT Platforms Checks on Memory Reads in Li
> Mode" in the same spec, there is no issue accessing such reclaimed pages
> using a shared key that does not have integrity enabled. Linux always uses
> KeyID 0 which never has integrity enabled. KeyID 0 is also the TME KeyID
> which disallows integrity, refer "TME Policy/Encryption Algorithm" bit
> description in "Intel Architecture Memory Encryption Technologies" spec
> version 1.6 April 2025. So there is no need to clear pages to avoid
> integrity violations.
>
> There remains a risk of poison consumption. However, in the context of
> TDX, it is expected that there would be a machine check associated with the
> original poisoning. On some platforms that results in a panic. However
> platforms may support "SEAM_NR" Machine Check capability, in which case
> Linux machine check handler marks the page as poisoned, which prevents it
> from being allocated anymore, refer commit 7911f145de5fe ("x86/mce:
> Implement recovery for errors in TDX/SEAM non-root mode")
>
> Improvement
>
> By skipping the clearing step on unaffected platforms, shutdown time
> can improve by up to 40%.
>
> On platforms with the X86_BUG_TDX_PW_MCE erratum (SPR and EMR), continue
> clearing because these platforms may trigger poison on partial writes to
> previously-private pages, even with KeyID 0, refer commit 1e536e1068970
> ("x86/cpu: Detect TDX partial write machine check erratum")
>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Reviewed-by: Kirill A. Shutemov <kas@kernel.org>
> Acked-by: Kai Huang <kai.huang@intel.com>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>
>
> Changes in V3:
>
> Remove "flush cache" comments (Rick)
> Update function comment to better relate to "quirk" naming (Rick)
> Add "via MOVDIR64B" to comment (Xiaoyao)
> Add Rev'd-by, Ack'd-by tags
>
> Changes in V2:
>
> Improve the comment
>
>
> arch/x86/virt/vmx/tdx/tdx.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 14d93ed05bd2..a542e4fbf5a8 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -633,15 +633,19 @@ static int tdmrs_set_up_pamt_all(struct tdmr_info_list *tdmr_list,
> }
>
> /*
> - * Convert TDX private pages back to normal by using MOVDIR64B to
> - * clear these pages. Note this function doesn't flush cache of
> - * these TDX private pages. The caller should make sure of that.
> + * Convert TDX private pages back to normal by using MOVDIR64B to clear these
> + * pages. Typically, any write to the page will convert it from TDX private back
> + * to normal kernel memory. Systems with the X86_BUG_TDX_PW_MCE erratum need to
> + * do the conversion explicitly via MOVDIR64B.
> */
> void tdx_quirk_reset_paddr(unsigned long base, unsigned long size)
> {
> const void *zero_page = (const void *)page_address(ZERO_PAGE(0));
> unsigned long phys, end;
>
> + if (!boot_cpu_has_bug(X86_BUG_TDX_PW_MCE))
> + return;
> +
> end = base + size;
> for (phys = base; phys < end; phys += 64)
> movdir64b(__va(phys), zero_page);
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-07-22 15:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-22 13:15 [PATCH V3 0/2] x86/tdx: Skip clearing reclaimed pages unless X86_BUG_TDX_PW_MCE is present Adrian Hunter
2025-07-22 13:15 ` [PATCH V3 1/2] x86/tdx: Eliminate duplicate code in tdx_clear_page() Adrian Hunter
2025-07-22 14:11 ` Sean Christopherson
2025-07-22 14:48 ` Adrian Hunter
2025-07-22 13:15 ` [PATCH V3 2/2] x86/tdx: Skip clearing reclaimed pages unless X86_BUG_TDX_PW_MCE is present Adrian Hunter
2025-07-22 15:08 ` 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;
as well as URLs for NNTP newsgroup(s).