| 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
| ||
|
Date: Thu, 11 Apr 2024 23:03:47 +0900
From: Itaru Kitayama <itaru.kitayama@...ux.dev>
To: Ryan Roberts <ryan.roberts@....com>
Cc: Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>, Mark Rutland <mark.rutland@....com>,
Ard Biesheuvel <ardb@...nel.org>,
David Hildenbrand <david@...hat.com>,
Donald Dutile <ddutile@...hat.com>,
Eric Chanudet <echanude@...hat.com>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Itaru Kitayama <itaru.kitayama@...itsu.com>
Subject: Re: [PATCH v3 1/3] arm64: mm: Don't remap pgtables per-cont(pte|pmd)
block
Hi Ryan,
On Fri, Apr 12, 2024 at 02:19:06PM +0100, Ryan Roberts wrote:
> A large part of the kernel boot time is creating the kernel linear map
> page tables. When rodata=full, all memory is mapped by pte. And when
> there is lots of physical ram, there are lots of pte tables to populate.
> The primary cost associated with this is mapping and unmapping the pte
> table memory in the fixmap; at unmap time, the TLB entry must be
> invalidated and this is expensive.
>
> Previously, each pmd and pte table was fixmapped/fixunmapped for each
> cont(pte|pmd) block of mappings (16 entries with 4K granule). This means
> we ended up issuing 32 TLBIs per (pmd|pte) table during the population
> phase.
>
> Let's fix that, and fixmap/fixunmap each page once per population, for a
> saving of 31 TLBIs per (pmd|pte) table. This gives a significant boot
> speedup.
>
> Execution time of map_mem(), which creates the kernel linear map page
> tables, was measured on different machines with different RAM configs:
>
> | Apple M2 VM | Ampere Altra| Ampere Altra| Ampere Altra
> | VM, 16G | VM, 64G | VM, 256G | Metal, 512G
> ---------------|-------------|-------------|-------------|-------------
> | ms (%) | ms (%) | ms (%) | ms (%)
> ---------------|-------------|-------------|-------------|-------------
> before | 168 (0%) | 2198 (0%) | 8644 (0%) | 17447 (0%)
> after | 78 (-53%) | 435 (-80%) | 1723 (-80%) | 3779 (-78%)
>
> Signed-off-by: Ryan Roberts <ryan.roberts@....com>
> Tested-by: Itaru Kitayama <itaru.kitayama@...itsu.com>
> Tested-by: Eric Chanudet <echanude@...hat.com>
> ---
> arch/arm64/mm/mmu.c | 27 ++++++++++++++-------------
> 1 file changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 495b732d5af3..9f1d69b7b494 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -172,12 +172,9 @@ bool pgattr_change_is_safe(u64 old, u64 new)
> return ((old ^ new) & ~mask) == 0;
> }
>
> -static void init_pte(pmd_t *pmdp, unsigned long addr, unsigned long end,
> +static void init_pte(pte_t *ptep, unsigned long addr, unsigned long end,
> phys_addr_t phys, pgprot_t prot)
> {
> - pte_t *ptep;
> -
> - ptep = pte_set_fixmap_offset(pmdp, addr);
> do {
> pte_t old_pte = __ptep_get(ptep);
>
> @@ -192,8 +189,6 @@ static void init_pte(pmd_t *pmdp, unsigned long addr, unsigned long end,
>
> phys += PAGE_SIZE;
> } while (ptep++, addr += PAGE_SIZE, addr != end);
> -
> - pte_clear_fixmap();
> }
>
> static void alloc_init_cont_pte(pmd_t *pmdp, unsigned long addr,
> @@ -204,6 +199,7 @@ static void alloc_init_cont_pte(pmd_t *pmdp, unsigned long addr,
> {
> unsigned long next;
> pmd_t pmd = READ_ONCE(*pmdp);
> + pte_t *ptep;
>
> BUG_ON(pmd_sect(pmd));
> if (pmd_none(pmd)) {
> @@ -219,6 +215,7 @@ static void alloc_init_cont_pte(pmd_t *pmdp, unsigned long addr,
> }
> BUG_ON(pmd_bad(pmd));
>
> + ptep = pte_set_fixmap_offset(pmdp, addr);
> do {
> pgprot_t __prot = prot;
>
> @@ -229,20 +226,21 @@ static void alloc_init_cont_pte(pmd_t *pmdp, unsigned long addr,
> (flags & NO_CONT_MAPPINGS) == 0)
> __prot = __pgprot(pgprot_val(prot) | PTE_CONT);
>
> - init_pte(pmdp, addr, next, phys, __prot);
> + init_pte(ptep, addr, next, phys, __prot);
>
> + ptep += pte_index(next) - pte_index(addr);
> phys += next - addr;
> } while (addr = next, addr != end);
> +
> + pte_clear_fixmap();
> }
>
> -static void init_pmd(pud_t *pudp, unsigned long addr, unsigned long end,
> +static void init_pmd(pmd_t *pmdp, unsigned long addr, unsigned long end,
> phys_addr_t phys, pgprot_t prot,
> phys_addr_t (*pgtable_alloc)(int), int flags)
> {
> unsigned long next;
> - pmd_t *pmdp;
>
> - pmdp = pmd_set_fixmap_offset(pudp, addr);
> do {
> pmd_t old_pmd = READ_ONCE(*pmdp);
>
> @@ -268,8 +266,6 @@ static void init_pmd(pud_t *pudp, unsigned long addr, unsigned long end,
> }
> phys += next - addr;
> } while (pmdp++, addr = next, addr != end);
> -
> - pmd_clear_fixmap();
> }
>
> static void alloc_init_cont_pmd(pud_t *pudp, unsigned long addr,
> @@ -279,6 +275,7 @@ static void alloc_init_cont_pmd(pud_t *pudp, unsigned long addr,
> {
> unsigned long next;
> pud_t pud = READ_ONCE(*pudp);
> + pmd_t *pmdp;
>
> /*
> * Check for initial section mappings in the pgd/pud.
> @@ -297,6 +294,7 @@ static void alloc_init_cont_pmd(pud_t *pudp, unsigned long addr,
> }
> BUG_ON(pud_bad(pud));
>
> + pmdp = pmd_set_fixmap_offset(pudp, addr);
> do {
> pgprot_t __prot = prot;
>
> @@ -307,10 +305,13 @@ static void alloc_init_cont_pmd(pud_t *pudp, unsigned long addr,
> (flags & NO_CONT_MAPPINGS) == 0)
> __prot = __pgprot(pgprot_val(prot) | PTE_CONT);
>
> - init_pmd(pudp, addr, next, phys, __prot, pgtable_alloc, flags);
> + init_pmd(pmdp, addr, next, phys, __prot, pgtable_alloc, flags);
>
> + pmdp += pmd_index(next) - pmd_index(addr);
> phys += next - addr;
> } while (addr = next, addr != end);
> +
> + pmd_clear_fixmap();
> }
>
> static void alloc_init_pud(p4d_t *p4dp, unsigned long addr, unsigned long end,
I looked at this specific patch 1/3 for a while and now make sense the
code changes to reduce down the number of TLBIs respecting the contigous
bit where available at both PMD and PTE levels. I can not finish other 2/3 and 3/3 patches in a timely manner but I'd like to give my
Reviewied-by: Itaru Kitayama <itaru.kitayama@...itsu.com>
on 1/3.
Thanks,
Itaru.
> --
> 2.25.1
>
Powered by blists - more mailing lists