| 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: <80ac39fc-3023-4a6c-87ff-05135a4e9abf@arm.com>
Date: Tue, 29 Apr 2025 10:14:12 +0530
From: Dev Jain <dev.jain@....com>
To: Zi Yan <ziy@...dia.com>
Cc: akpm@...ux-foundation.org, 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,
anshuman.khandual@....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
Subject: Re: [PATCH 4/7] arm64: Add batched version of ptep_modify_prot_start
On 28/04/25 11:36 pm, Zi Yan wrote:
> On 28 Apr 2025, at 8:04, 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,
>
> What do you mean by “contpte-edges”? Can you provide an example?
get_and_clear_full_ptes -> contpte_get_and_clear_full_ptes ->
contpte_try_unfold_partial, which unfolds only the start and end
contpte block. Whereas, ptep_get_and_clear -> contpte_try_unfold, which
means it will unfold for every contpte block.
>
>> instead of doing it for every contpte block, which happens for ptep_get_and_clear().
>>
>> 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)
>
> Putting ptes at the end seems to break the naming convention. How about
> ptep_modify_prot_range_start? ptes_modify_prot_start might be OK too.
I was actually following the convention present in
include/linux/pgtable.h, look at all the functions with _ptes suffix.
>
>> {
>> 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)
>> {
>
> Ditto.
>
>> 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
>> --
>> 2.30.2
>
>
> Best Regards,
> Yan, Zi
Powered by blists - more mailing lists