| 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: <5071efdf-8260-43dc-8042-69414b124009@linux.dev>
Date: Tue, 23 Dec 2025 19:13:11 +0800
From: Lance Yang <lance.yang@...ux.dev>
To: "David Hildenbrand (Red Hat)" <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, jannh@...gle.com,
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,
Lance Yang <ioworker0@...il.com>
Subject: Re: [PATCH RFC 2/3] x86/mm: implement redundant IPI elimination for
On 2025/12/23 17:44, David Hildenbrand (Red Hat) wrote:
> On 12/22/25 04:19, Lance Yang wrote:
>> 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);
>
> I'd have thought that the X86_FEATURE_INVLPGB heck should then also be
> taken care of by whoever sets tlb_flush_implies_ipi_broadcast.
Makes sense!
Let's have the INVLPGB check happen at setup time, not at use time :P
Cheers,
Lance
Powered by blists - more mailing lists