[PATCH v6 7/7] powerpc: mm: Support page table check

[Christophe Leroy]

Le 14/02/2023 à 02:59, Rohan McLure a écrit :
> On creation and clearing of a page table mapping, instrument such calls
> by invoking page_table_check_pte_set and page_table_check_pte_clear
> respectively. These calls serve as a sanity check against illegal
> mappings.

Please also explaing the changes around set_pte_at() versus set_pte().

> 
> Enable ARCH_SUPPORTS_PAGE_TABLE_CHECK for all ppc64, and 32-bit
> platforms implementing Book3S.

As far as I can see below, it is implemented for all plateforms, 
including nohash/32.

> 
> Change pud_pfn to be a runtime bug rather than a build bug as it is
> consumed by page_table_check_pud_{clear,set} which are not called.

Isn't this done in another patch ?

> 
> See also:
> 
> riscv support in commit 3fee229a8eb9 ("riscv/mm: enable
> ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> arm64 in commit 42b2547137f5 ("arm64/mm: enable
> ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> x86_64 in commit d283d422c6c4 ("x86: mm: add x86_64 support for page table
> check")
> 
> Signed-off-by: Rohan McLure <rmclure at linux.ibm.com>
> ---
> V2: Update spacing and types assigned to pte_update calls.
> V3: Update one last pte_update call to remove __pte invocation.
> V5: Fix 32-bit nohash double set
> V6: Omit __set_pte_at instrumentation - should be instrumented by
> set_pte_at, with set_pte in between, performing all prior checks.
> Instrument pmds. Use set_pte where needed.
> ---
>   arch/powerpc/Kconfig                         |  1 +
>   arch/powerpc/include/asm/book3s/32/pgtable.h |  8 +++-
>   arch/powerpc/include/asm/book3s/64/pgtable.h | 44 ++++++++++++++++----
>   arch/powerpc/include/asm/nohash/32/pgtable.h |  7 +++-
>   arch/powerpc/include/asm/nohash/64/pgtable.h |  8 +++-
>   arch/powerpc/include/asm/pgtable.h           | 11 ++++-
>   arch/powerpc/mm/book3s64/hash_pgtable.c      |  2 +-
>   arch/powerpc/mm/book3s64/pgtable.c           | 16 ++++---
>   arch/powerpc/mm/book3s64/radix_pgtable.c     | 10 ++---
>   arch/powerpc/mm/nohash/book3e_pgtable.c      |  2 +-
>   arch/powerpc/mm/pgtable_32.c                 |  2 +-
>   11 files changed, 84 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 2c9cdf1d8761..2474e2699037 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -154,6 +154,7 @@ config PPC
>   	select ARCH_STACKWALK
>   	select ARCH_SUPPORTS_ATOMIC_RMW
>   	select ARCH_SUPPORTS_DEBUG_PAGEALLOC	if PPC_BOOK3S || PPC_8xx || 40x
> +	select ARCH_SUPPORTS_PAGE_TABLE_CHECK
>   	select ARCH_USE_BUILTIN_BSWAP
>   	select ARCH_USE_CMPXCHG_LOCKREF		if PPC64
>   	select ARCH_USE_MEMTEST

> diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
> index b76fdb80b6c9..df016a0a3135 100644
> --- a/arch/powerpc/include/asm/pgtable.h
> +++ b/arch/powerpc/include/asm/pgtable.h
> @@ -48,7 +48,16 @@ struct mm_struct;
>   /* Keep these as a macros to avoid include dependency mess */
>   #define pte_page(x)		pfn_to_page(pte_pfn(x))
>   #define mk_pte(page, pgprot)	pfn_pte(page_to_pfn(page), (pgprot))
> -#define set_pte_at  		set_pte
> +#define set_pte_at(mm, addr, ptep, pte)				\
> +({								\
> +	struct mm_struct *_mm = (mm);				\
> +	unsigned long _addr = (addr);				\
> +	pte_t *_ptep = (ptep), _pte = (pte);			\
> +								\
> +	page_table_check_pte_set(_mm, _addr, _ptep, _pte);	\
> +	set_pte(_mm, _addr, _ptep, _pte);			\
> +})

Can you make it a static inline function instead of a macro ?

> +
>   /*
>    * Select all bits except the pfn
>    */