| 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: <9856f39d-d9d8-45c1-8e5a-514d21a86d51@arm.com>
Date: Wed, 30 Apr 2025 12:02:42 +0530
From: Dev Jain <dev.jain@....com>
To: Anshuman Khandual <anshuman.khandual@....com>, akpm@...ux-foundation.org
Cc: ryan.roberts@....com, david@...hat.com, willy@...radead.org,
linux-mm@...ck.org, linux-kernel@...r.kernel.org, catalin.marinas@....com,
will@...nel.org, Liam.Howlett@...cle.com, lorenzo.stoakes@...cle.com,
vbabka@...e.cz, jannh@...gle.com, peterx@...hat.com, joey.gouly@....com,
ioworker0@...il.com, baohua@...nel.org, kevin.brodsky@....com,
quic_zhenhuah@...cinc.com, christophe.leroy@...roup.eu,
yangyicong@...ilicon.com, linux-arm-kernel@...ts.infradead.org,
namit@...are.com, hughd@...gle.com, yang@...amperecomputing.com,
ziy@...dia.com
Subject: Re: [PATCH v2 4/7] arm64: Add batched version of
ptep_modify_prot_start
On 30/04/25 11:44 am, Anshuman Khandual wrote:
>
>
> On 4/30/25 11:19, Dev Jain wrote:
>>
>>
>> On 30/04/25 11:13 am, Anshuman Khandual wrote:
>>> On 4/29/25 10:53, Dev Jain wrote:
>>>> Override the generic definition to use get_and_clear_full_ptes(), so that
>>>> we do a TLBI possibly only on the "contpte-edges" of the large PTE block,
>>>> instead of doing it for every contpte block, which happens for ptep_get_and_clear().
>>>
>>> Could you please explain what does "contpte-edges" really signify in the
>>> context of large PTE blocks ? Also how TLBI operation only on these edges
>>> will never run into the risk of missing TLB invalidation of some other
>>> mapped areas ?
>>
>> We are doing a TLBI over the whole range already, in the mprotect code:
>> see tlb_flush_pte_range. What the arm64 internal API does, irrespective of the caller, is to do a TLBI for every contpte block in case of unfolding. We don't need that for the intermediate blocks because the caller does that. We do need a TLBI for the start and end contpte block,
>> because in case the range we are invalidating partially covers them, then the caller will not do a TLBI for the non-overlapped PTEs of the block.
>
> But is not splitting the TLBI flush responsibility between the callers
> (intermediate blocks) and the platform API (contpte-edges) - some what
> problematic from a semantics perspective, and will be more susceptible
> for missing TLB flushes etc ?
I seem to agree that this is a semantic problem. Although, we won't ever
miss a TLB flush - we will only have extras. It is the responsibility of
the caller to do the TLBI, the platform API is only checking whether we
need some more TLBIs.
>
>> I'll explain some more in the changelog next version.
>>
>>>
>>>>
>>>> Signed-off-by: Dev Jain <dev.jain@....com>
>>>> ---
>>>> arch/arm64/include/asm/pgtable.h | 5 +++++
>>>> arch/arm64/mm/mmu.c | 12 +++++++++---
>>>> include/linux/pgtable.h | 4 ++++
>>>> mm/pgtable-generic.c | 16 +++++++++++-----
>>>> 4 files changed, 29 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>>>> index 2a77f11b78d5..8872ea5f0642 100644
>>>> --- a/arch/arm64/include/asm/pgtable.h
>>>> +++ b/arch/arm64/include/asm/pgtable.h
>>>> @@ -1553,6 +1553,11 @@ extern void ptep_modify_prot_commit(struct vm_area_struct *vma,
>>>> unsigned long addr, pte_t *ptep,
>>>> pte_t old_pte, pte_t new_pte);
>>>> +#define modify_prot_start_ptes modify_prot_start_ptes
>>>> +extern pte_t modify_prot_start_ptes(struct vm_area_struct *vma,
>>>> + unsigned long addr, pte_t *ptep,
>>>> + unsigned int nr);
>>>> +
>>>> #ifdef CONFIG_ARM64_CONTPTE
>>>> /*
>>>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>>>> index 8fcf59ba39db..fe60be8774f4 100644
>>>> --- a/arch/arm64/mm/mmu.c
>>>> +++ b/arch/arm64/mm/mmu.c
>>>> @@ -1523,7 +1523,8 @@ static int __init prevent_bootmem_remove_init(void)
>>>> early_initcall(prevent_bootmem_remove_init);
>>>> #endif
>>>> -pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
>>>> +pte_t modify_prot_start_ptes(struct vm_area_struct *vma, unsigned long addr,
>>>> + pte_t *ptep, unsigned int nr)
>>>> {
>>>> if (alternative_has_cap_unlikely(ARM64_WORKAROUND_2645198)) {
>>>> /*
>>>> @@ -1532,9 +1533,14 @@ pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte
>>>> * in cases where cpu is affected with errata #2645198.
>>>> */
>>>> if (pte_user_exec(ptep_get(ptep)))
>>>> - return ptep_clear_flush(vma, addr, ptep);
>>>> + return clear_flush_ptes(vma, addr, ptep, nr);
>>>> }
>>>> - return ptep_get_and_clear(vma->vm_mm, addr, ptep);
>>>> + return get_and_clear_full_ptes(vma->vm_mm, addr, ptep, nr, 0);
>>>> +}
>>>> +
>>>> +pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
>>>> +{
>>>> + return modify_prot_start_ptes(vma, addr, ptep, 1);
>>>> }
>>>> void ptep_modify_prot_commit(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep,
>>>> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
>>>> index ed287289335f..10cdb87ccecf 100644
>>>> --- a/include/linux/pgtable.h
>>>> +++ b/include/linux/pgtable.h
>>>> @@ -828,6 +828,10 @@ extern pte_t ptep_clear_flush(struct vm_area_struct *vma,
>>>> pte_t *ptep);
>>>> #endif
>>>> +extern pte_t clear_flush_ptes(struct vm_area_struct *vma,
>>>> + unsigned long address,
>>>> + pte_t *ptep, unsigned int nr);
>>>> +
>>>> #ifndef __HAVE_ARCH_PMDP_HUGE_CLEAR_FLUSH
>>>> extern pmd_t pmdp_huge_clear_flush(struct vm_area_struct *vma,
>>>> unsigned long address,
>>>> diff --git a/mm/pgtable-generic.c b/mm/pgtable-generic.c
>>>> index 5a882f2b10f9..e238f88c3cac 100644
>>>> --- a/mm/pgtable-generic.c
>>>> +++ b/mm/pgtable-generic.c
>>>> @@ -90,17 +90,23 @@ int ptep_clear_flush_young(struct vm_area_struct *vma,
>>>> }
>>>> #endif
>>>> -#ifndef __HAVE_ARCH_PTEP_CLEAR_FLUSH
>>>> -pte_t ptep_clear_flush(struct vm_area_struct *vma, unsigned long address,
>>>> - pte_t *ptep)
>>>> +pte_t clear_flush_ptes(struct vm_area_struct *vma, unsigned long address,
>>>> + pte_t *ptep, unsigned int nr)
>>>> {
>>>> struct mm_struct *mm = (vma)->vm_mm;
>>>> pte_t pte;
>>>> - pte = ptep_get_and_clear(mm, address, ptep);
>>>> + pte = get_and_clear_full_ptes(mm, address, ptep, nr, 0);
>>>> if (pte_accessible(mm, pte))
>>>> - flush_tlb_page(vma, address);
>>>> + flush_tlb_range(vma, address, address + nr * PAGE_SIZE);
>>>> return pte;
>>>> }
>>>> +
>>>> +#ifndef __HAVE_ARCH_PTEP_CLEAR_FLUSH
>>>> +pte_t ptep_clear_flush(struct vm_area_struct *vma, unsigned long address,
>>>> + pte_t *ptep)
>>>> +{
>>>> + return clear_flush_ptes(vma, address, ptep, 1);
>>>> +}
>>>> #endif
>>>> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>>
>
Powered by blists - more mailing lists