| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20251222031919.41964-1-ioworker0@gmail.com>
Date: Mon, 22 Dec 2025 11:19:19 +0800
From: Lance Yang <ioworker0@...il.com>
To: david@...nel.org
Cc: Liam.Howlett@...cle.com,
akpm@...ux-foundation.org,
aneesh.kumar@...nel.org,
arnd@...db.de,
baohua@...nel.org,
baolin.wang@...ux.alibaba.com,
bp@...en8.de,
dave.hansen@...ux.intel.com,
dev.jain@....com,
hpa@...or.com,
ioworker0@...il.com,
jannh@...gle.com,
lance.yang@...ux.dev,
linux-arch@...r.kernel.org,
linux-kernel@...r.kernel.org,
linux-mm@...ck.org,
lorenzo.stoakes@...cle.com,
mingo@...hat.com,
npache@...hat.com,
npiggin@...il.com,
peterz@...radead.org,
riel@...riel.com,
ryan.roberts@....com,
shy828301@...il.com,
tglx@...utronix.de,
will@...nel.org,
x86@...nel.org,
ziy@...dia.com
Subject: Re: [PATCH RFC 2/3] x86/mm: implement redundant IPI elimination for
From: Lance Yang <lance.yang@...ux.dev>
On Thu, 18 Dec 2025 14:08:07 +0100, David Hildenbrand (Red Hat) wrote:
> On 12/13/25 09:00, Lance Yang wrote:
> > From: Lance Yang <lance.yang@...ux.dev>
> >
> > Pass both freed_tables and unshared_tables to flush_tlb_mm_range() to
> > ensure lazy-TLB CPUs receive IPIs and flush their paging-structure caches:
> >
> > flush_tlb_mm_range(..., freed_tables || unshared_tables);
> >
> > Implement tlb_table_flush_implies_ipi_broadcast() for x86: on native x86
> > without paravirt or INVLPGB, the TLB flush IPI already provides necessary
> > synchronization, allowing the second IPI to be skipped. For paravirt with
> > non-native flush_tlb_multi and for INVLPGB, conservatively keep both IPIs.
> >
> > Suggested-by: David Hildenbrand (Red Hat) <david@...nel.org>
> > Signed-off-by: Lance Yang <lance.yang@...ux.dev>
> > ---
> > arch/x86/include/asm/tlb.h | 17 ++++++++++++++++-
> > 1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/include/asm/tlb.h b/arch/x86/include/asm/tlb.h
> > index 866ea78ba156..96602b7b7210 100644
> > --- a/arch/x86/include/asm/tlb.h
> > +++ b/arch/x86/include/asm/tlb.h
> > @@ -5,10 +5,24 @@
> > #define tlb_flush tlb_flush
> > static inline void tlb_flush(struct mmu_gather *tlb);
> >
> > +#define tlb_table_flush_implies_ipi_broadcast tlb_table_flush_implies_ipi_broadcast
> > +static inline bool tlb_table_flush_implies_ipi_broadcast(void);
> > +
> > #include <asm-generic/tlb.h>
> > #include <linux/kernel.h>
> > #include <vdso/bits.h>
> > #include <vdso/page.h>
> > +#include <asm/paravirt.h>
> > +
> > +static inline bool tlb_table_flush_implies_ipi_broadcast(void)
> > +{
> > +#ifdef CONFIG_PARAVIRT
> > + /* Paravirt may use hypercalls that don't send real IPIs. */
> > + if (pv_ops.mmu.flush_tlb_multi != native_flush_tlb_multi)
> > + return false;
> > +#endif
> > + return !cpu_feature_enabled(X86_FEATURE_INVLPGB);
>
> Right, here I was wondering whether we should have a new pv_ops callback
> to indicate that instead.
>
> pv_ops.mmu.tlb_table_flush_implies_ipi_broadcast()
>
> Or a simple boolean property that pv init code properly sets.
Cool!
>
> Something for x86 folks to give suggestions for. :)
I prefer to use a boolean property instead of comparing function pointers.
Something like this:
----8<----
diff --git a/arch/x86/hyperv/mmu.c b/arch/x86/hyperv/mmu.c
index cfcb60468b01..90e9da33f2c7 100644
--- a/arch/x86/hyperv/mmu.c
+++ b/arch/x86/hyperv/mmu.c
@@ -243,4 +243,5 @@ void hyperv_setup_mmu_ops(void)
pr_info("Using hypercall for remote TLB flush\n");
pv_ops.mmu.flush_tlb_multi = hyperv_flush_tlb_multi;
+ pv_ops.mmu.tlb_flush_implies_ipi_broadcast = false;
}
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 3502939415ad..f9756df6f3f6 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -133,6 +133,19 @@ struct pv_mmu_ops {
void (*flush_tlb_multi)(const struct cpumask *cpus,
const struct flush_tlb_info *info);
+ /*
+ * Indicates whether TLB flush IPIs provide sufficient synchronization
+ * for GUP-fast when freeing or unsharing page tables.
+ *
+ * Set to true only when the TLB flush guarantees:
+ * - IPIs reach all CPUs with potentially stale paging-structure caches
+ * - Synchronization with IRQ-disabled code like GUP-fast
+ *
+ * Paravirt implementations that use hypercalls (which may not send
+ * real IPIs) should set this to false.
+ */
+ bool tlb_flush_implies_ipi_broadcast;
+
/* Hook for intercepting the destruction of an mm_struct. */
void (*exit_mmap)(struct mm_struct *mm);
void (*notify_page_enc_status_changed)(unsigned long pfn, int npages, bool enc);
diff --git a/arch/x86/include/asm/tlb.h b/arch/x86/include/asm/tlb.h
index 96602b7b7210..9d20ad4786cc 100644
--- a/arch/x86/include/asm/tlb.h
+++ b/arch/x86/include/asm/tlb.h
@@ -18,7 +18,7 @@ static inline bool tlb_table_flush_implies_ipi_broadcast(void)
{
#ifdef CONFIG_PARAVIRT
/* Paravirt may use hypercalls that don't send real IPIs. */
- if (pv_ops.mmu.flush_tlb_multi != native_flush_tlb_multi)
+ if (!pv_ops.mmu.tlb_flush_implies_ipi_broadcast)
return false;
#endif
return !cpu_feature_enabled(X86_FEATURE_INVLPGB);
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index df78ddee0abb..aaea83100105 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -843,6 +843,7 @@ static void __init kvm_guest_init(void)
#ifdef CONFIG_SMP
if (pv_tlb_flush_supported()) {
pv_ops.mmu.flush_tlb_multi = kvm_flush_tlb_multi;
+ pv_ops.mmu.tlb_flush_implies_ipi_broadcast = false;
pr_info("KVM setup pv remote TLB flush\n");
}
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index ab3e172dcc69..625fe93e138a 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -173,6 +173,7 @@ struct paravirt_patch_template pv_ops = {
.mmu.flush_tlb_kernel = native_flush_tlb_global,
.mmu.flush_tlb_one_user = native_flush_tlb_one_user,
.mmu.flush_tlb_multi = native_flush_tlb_multi,
+ .mmu.tlb_flush_implies_ipi_broadcast = true,
.mmu.exit_mmap = paravirt_nop,
.mmu.notify_page_enc_status_changed = paravirt_nop,
diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
index 7a35c3393df4..06eb80cfb4da 100644
--- a/arch/x86/xen/mmu_pv.c
+++ b/arch/x86/xen/mmu_pv.c
@@ -2185,6 +2185,7 @@ static const typeof(pv_ops) xen_mmu_ops __initconst = {
.flush_tlb_kernel = xen_flush_tlb,
.flush_tlb_one_user = xen_flush_tlb_one_user,
.flush_tlb_multi = xen_flush_tlb_multi,
+ .tlb_flush_implies_ipi_broadcast = false,
.pgd_alloc = xen_pgd_alloc,
.pgd_free = xen_pgd_free,
---
Native x86 sets it to true, paravirt guests (Xen/KVM/Hyper-V) set it to
false. Making the intent explicit :)
Hopefully x86 folks can give me some suggestions!
Thanks,
Lance
Powered by blists - more mailing lists