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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b3af64a8-b8fe-4651-811d-594a206d593f@gmail.com>
Date: Wed, 18 Dec 2024 15:26:42 +0100
From: Klara Modin <klarasmodin@...il.com>
To: Qi Zheng <zhengqi.arch@...edance.com>, peterz@...radead.org,
 tglx@...utronix.de, david@...hat.com, jannh@...gle.com, hughd@...gle.com,
 yuzhao@...gle.com, willy@...radead.org, muchun.song@...ux.dev,
 vbabka@...nel.org, lorenzo.stoakes@...cle.com, akpm@...ux-foundation.org,
 rientjes@...gle.com
Cc: linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 02/12] mm: pgtable: introduce generic p4d_alloc_one() and
 p4d_free()

Hi,

On 2024-12-14 10:02, Qi Zheng wrote:
> Several architectures (arm64, riscv, x86) define p4d_alloc_one() as a
> wrapper for get_zeroed_page() and p4d_free() as a wrapper for free_page().
> 
> For these architectures, provide a generic implementation in
> asm-generic/pgalloc.h and convert them to use it. And like other levels
> of page tables, add statistics for P4D level page table.
> 
> For s390, it also defines p4d_alloc_one() and p4d_free(), but it uses its
> own logic, so skip it.
> 
> Signed-off-by: Qi Zheng <zhengqi.arch@...edance.com>
> ---
>   arch/arm64/include/asm/pgalloc.h | 15 ++++-----
>   arch/riscv/include/asm/pgalloc.h | 25 ++++++---------
>   arch/x86/include/asm/pgalloc.h   | 16 ++++------
>   arch/x86/mm/pgtable.c            |  3 ++
>   include/asm-generic/pgalloc.h    | 55 ++++++++++++++++++++++++++++++++
>   include/linux/mm.h               | 16 ++++++++++
>   6 files changed, 98 insertions(+), 32 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/pgalloc.h b/arch/arm64/include/asm/pgalloc.h
> index e75422864d1bd..679c530549327 100644
> --- a/arch/arm64/include/asm/pgalloc.h
> +++ b/arch/arm64/include/asm/pgalloc.h
> @@ -15,6 +15,8 @@
>   
>   #define __HAVE_ARCH_PGD_FREE
>   #define __HAVE_ARCH_PUD_FREE
> +#define __HAVE_ARCH_P4D_ALLOC_ONE
> +#define __HAVE_ARCH_P4D_FREE
>   #include <asm-generic/pgalloc.h>
>   
>   #define PGD_SIZE	(PTRS_PER_PGD * sizeof(pgd_t))
> @@ -87,19 +89,16 @@ static inline void pgd_populate(struct mm_struct *mm, pgd_t *pgdp, p4d_t *p4dp)
>   
>   static inline p4d_t *p4d_alloc_one(struct mm_struct *mm, unsigned long addr)
>   {
> -	gfp_t gfp = GFP_PGTABLE_USER;
> +	if (!pgtable_l5_enabled())
> +		return NULL;
>   
> -	if (mm == &init_mm)
> -		gfp = GFP_PGTABLE_KERNEL;
> -	return (p4d_t *)get_zeroed_page(gfp);
> +	return __p4d_alloc_one(mm, addr);
>   }
>   
>   static inline void p4d_free(struct mm_struct *mm, p4d_t *p4d)
>   {
> -	if (!pgtable_l5_enabled())
> -		return;
> -	BUG_ON((unsigned long)p4d & (PAGE_SIZE-1));
> -	free_page((unsigned long)p4d);
> +	if (pgtable_l5_enabled())
> +		__p4d_free(mm, p4d);
>   }
>   
>   #define __p4d_free_tlb(tlb, p4d, addr)  p4d_free((tlb)->mm, p4d)
> diff --git a/arch/riscv/include/asm/pgalloc.h b/arch/riscv/include/asm/pgalloc.h
> index f52264304f772..bb6e1c5f1fb19 100644
> --- a/arch/riscv/include/asm/pgalloc.h
> +++ b/arch/riscv/include/asm/pgalloc.h
> @@ -14,6 +14,8 @@
>   #ifdef CONFIG_MMU
>   #define __HAVE_ARCH_PUD_ALLOC_ONE
>   #define __HAVE_ARCH_PUD_FREE
> +#define __HAVE_ARCH_P4D_ALLOC_ONE
> +#define __HAVE_ARCH_P4D_FREE
>   #include <asm-generic/pgalloc.h>
>   
>   static inline void riscv_tlb_remove_ptdesc(struct mmu_gather *tlb, void *pt)
> @@ -118,21 +120,10 @@ static inline void __pud_free_tlb(struct mmu_gather *tlb, pud_t *pud,
>   #define p4d_alloc_one p4d_alloc_one
>   static inline p4d_t *p4d_alloc_one(struct mm_struct *mm, unsigned long addr)
>   {
> -	if (pgtable_l5_enabled) {
> -		gfp_t gfp = GFP_PGTABLE_USER;
> -
> -		if (mm == &init_mm)
> -			gfp = GFP_PGTABLE_KERNEL;
> -		return (p4d_t *)get_zeroed_page(gfp);
> -	}
> +	if (!pgtable_l5_enabled)
> +		return NULL;
>   
> -	return NULL;
> -}
> -
> -static inline void __p4d_free(struct mm_struct *mm, p4d_t *p4d)
> -{
> -	BUG_ON((unsigned long)p4d & (PAGE_SIZE-1));
> -	free_page((unsigned long)p4d);
> +	return __p4d_alloc_one(mm, addr);
>   }
>   
>   #define p4d_free p4d_free
> @@ -145,8 +136,12 @@ static inline void p4d_free(struct mm_struct *mm, p4d_t *p4d)
>   static inline void __p4d_free_tlb(struct mmu_gather *tlb, p4d_t *p4d,
>   				  unsigned long addr)
>   {
> -	if (pgtable_l5_enabled)
> +	if (pgtable_l5_enabled) {
> +		struct ptdesc *ptdesc = virt_to_ptdesc(p4d);
> +
> +		pagetable_p4d_dtor(ptdesc);
>   		riscv_tlb_remove_ptdesc(tlb, virt_to_ptdesc(p4d));
> +	}
>   }
>   #endif /* __PAGETABLE_PMD_FOLDED */
>   
> diff --git a/arch/x86/include/asm/pgalloc.h b/arch/x86/include/asm/pgalloc.h
> index dcd836b59bebd..d9bc6cae77c9e 100644
> --- a/arch/x86/include/asm/pgalloc.h
> +++ b/arch/x86/include/asm/pgalloc.h
> @@ -8,6 +8,8 @@
>   
>   #define __HAVE_ARCH_PTE_ALLOC_ONE
>   #define __HAVE_ARCH_PGD_FREE
> +#define __HAVE_ARCH_P4D_ALLOC_ONE
> +#define __HAVE_ARCH_P4D_FREE
>   #include <asm-generic/pgalloc.h>
>   
>   static inline int  __paravirt_pgd_alloc(struct mm_struct *mm) { return 0; }
> @@ -149,20 +151,16 @@ static inline void pgd_populate_safe(struct mm_struct *mm, pgd_t *pgd, p4d_t *p4
>   
>   static inline p4d_t *p4d_alloc_one(struct mm_struct *mm, unsigned long addr)
>   {
> -	gfp_t gfp = GFP_KERNEL_ACCOUNT;
> +	if (!pgtable_l5_enabled())
> +		return NULL;
>   
> -	if (mm == &init_mm)
> -		gfp &= ~__GFP_ACCOUNT;
> -	return (p4d_t *)get_zeroed_page(gfp);
> +	return __p4d_alloc_one(mm, addr);
>   }
>   
>   static inline void p4d_free(struct mm_struct *mm, p4d_t *p4d)
>   {
> -	if (!pgtable_l5_enabled())
> -		return;
> -
> -	BUG_ON((unsigned long)p4d & (PAGE_SIZE-1));
> -	free_page((unsigned long)p4d);
> +	if (pgtable_l5_enabled())
> +		return __p4d_free(mm, p4d);
>   }
>   
>   extern void ___p4d_free_tlb(struct mmu_gather *tlb, p4d_t *p4d);
> diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
> index 69a357b15974a..3d6e84da45b24 100644
> --- a/arch/x86/mm/pgtable.c
> +++ b/arch/x86/mm/pgtable.c
> @@ -94,6 +94,9 @@ void ___pud_free_tlb(struct mmu_gather *tlb, pud_t *pud)
>   #if CONFIG_PGTABLE_LEVELS > 4
>   void ___p4d_free_tlb(struct mmu_gather *tlb, p4d_t *p4d)
>   {
> +	struct ptdesc *ptdesc = virt_to_ptdesc(p4d);
> +
> +	pagetable_p4d_dtor(ptdesc);
>   	paravirt_release_p4d(__pa(p4d) >> PAGE_SHIFT);
>   	paravirt_tlb_remove_table(tlb, virt_to_page(p4d));
>   }
> diff --git a/include/asm-generic/pgalloc.h b/include/asm-generic/pgalloc.h
> index 7c48f5fbf8aa7..dbf61819b3581 100644
> --- a/include/asm-generic/pgalloc.h
> +++ b/include/asm-generic/pgalloc.h
> @@ -215,6 +215,61 @@ static inline void pud_free(struct mm_struct *mm, pud_t *pud)
>   
>   #endif /* CONFIG_PGTABLE_LEVELS > 3 */
>   
> +#if CONFIG_PGTABLE_LEVELS > 4
> +
> +static inline p4d_t *__p4d_alloc_one_noprof(struct mm_struct *mm, unsigned long addr)
> +{
> +	gfp_t gfp = GFP_PGTABLE_USER;
> +	struct ptdesc *ptdesc;
> +
> +	if (mm == &init_mm)
> +		gfp = GFP_PGTABLE_KERNEL;
> +	gfp &= ~__GFP_HIGHMEM;
> +
> +	ptdesc = pagetable_alloc_noprof(gfp, 0);
> +	if (!ptdesc)
> +		return NULL;
> +
> +	pagetable_p4d_ctor(ptdesc);
> +	return ptdesc_address(ptdesc);
> +}
> +#define __p4d_alloc_one(...)	alloc_hooks(__p4d_alloc_one_noprof(__VA_ARGS__))
> +
> +#ifndef __HAVE_ARCH_P4D_ALLOC_ONE
> +/**
> + * p4d_alloc_one - allocate memory for a P4D-level page table
> + * @mm: the mm_struct of the current context
> + *
> + * Allocate memory for a page table using %GFP_PGTABLE_USER for user context
> + * and %GFP_PGTABLE_KERNEL for kernel context.
> + *
> + * Return: pointer to the allocated memory or %NULL on error
> + */
> +static inline p4d_t *p4d_alloc_one_noprof(struct mm_struct *mm, unsigned long addr)
> +{
> +	return __p4d_alloc_one_noprof(mm, addr);
> +}
> +#define p4d_alloc_one(...)	alloc_hooks(p4d_alloc_one_noprof(__VA_ARGS__))
> +#endif
> +
> +static inline void __p4d_free(struct mm_struct *mm, p4d_t *p4d)
> +{
> +	struct ptdesc *ptdesc = virt_to_ptdesc(p4d);
> +
> +	BUG_ON((unsigned long)p4d & (PAGE_SIZE-1));
> +	pagetable_p4d_dtor(ptdesc);
> +	pagetable_free(ptdesc);
> +}
> +


> +#ifndef __HAVE_ARCH_P4D_FREE
> +static inline void p4d_free(struct mm_struct *mm, pud_t *p4d)

Should this perhaps be p4d_t *p4d rather than pud_t *p4d?

Otherwise I get this build error:

In file included from 
/home/klara/git/linux/arch/riscv/include/asm/kfence.h:8,
                  from /home/klara/git/linux/mm/kfence/core.c:34:
/home/klara/git/linux/include/asm-generic/pgalloc.h: In function ‘p4d_free’:
/home/klara/git/linux/include/asm-generic/pgalloc.h:252:24: error: 
passing argument 2 of ‘__p4d_free’ from incompatible pointer type 
[-Wincompatible-pointer-types]
   252 |         __p4d_free(mm, p4d);
       |                        ^~~
       |                        |
       |                        pud_t *
/home/klara/git/linux/include/asm-generic/pgalloc.h:244:60: note: 
expected ‘p4d_t *’ but argument is of type ‘pud_t *’
   244 | static inline void __p4d_free(struct mm_struct *mm, p4d_t *p4d)
       |                                                     ~~~~~~~^~~
In file included from 
/home/klara/git/linux/arch/riscv/include/asm/kfence.h:8,
                  from /home/klara/git/linux/mm/kfence/report.c:22:
/home/klara/git/linux/include/asm-generic/pgalloc.h: In function ‘p4d_free’:
/home/klara/git/linux/include/asm-generic/pgalloc.h:252:24: error: 
passing argument 2 of ‘__p4d_free’ from incompatible pointer type 
[-Wincompatible-pointer-types]
   252 |         __p4d_free(mm, p4d);
       |                        ^~~
       |                        |
       |                        pud_t *
/home/klara/git/linux/include/asm-generic/pgalloc.h:244:60: note: 
expected ‘p4d_t *’ but argument is of type ‘pud_t *’
   244 | static inline void __p4d_free(struct mm_struct *mm, p4d_t *p4d)
       |                                                     ~~~~~~~^~~

Regards,
Klara Modin

> +{
> +	__p4d_free(mm, p4d);
> +}
> +#endif
> +
> +#endif
> +
>   #ifndef __HAVE_ARCH_PGD_FREE
>   static inline void pgd_free(struct mm_struct *mm, pgd_t *pgd)
>   {
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 5e73e53c34e9e..807a12ed8ec96 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -3237,6 +3237,22 @@ static inline void pagetable_pud_dtor(struct ptdesc *ptdesc)
>   	lruvec_stat_sub_folio(folio, NR_PAGETABLE);
>   }
>   
> +static inline void pagetable_p4d_ctor(struct ptdesc *ptdesc)
> +{
> +	struct folio *folio = ptdesc_folio(ptdesc);
> +
> +	__folio_set_pgtable(folio);
> +	lruvec_stat_add_folio(folio, NR_PAGETABLE);
> +}
> +
> +static inline void pagetable_p4d_dtor(struct ptdesc *ptdesc)
> +{
> +	struct folio *folio = ptdesc_folio(ptdesc);
> +
> +	__folio_clear_pgtable(folio);
> +	lruvec_stat_sub_folio(folio, NR_PAGETABLE);
> +}
> +
>   extern void __init pagecache_init(void);
>   extern void free_initmem(void);
>   


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ