[PATCH V4 0/7] mm/mmap: Drop arch_vm_get_page_prot() and arch_filter_pgprot()

[PATCH V4 0/7] mm/mmap: Drop arch_vm_get_page_prot() and arch_filter_pgprot()

[Anshuman Khandual]

protection_map[] is an array based construct that translates given vm_flags
combination. This array contains page protection map, which is populated by
the platform via [__S000 .. __S111] and [__P000 .. __P111] exported macros.
Primary usage for protection_map[] is for vm_get_page_prot(), which is used
to determine page protection value for a given vm_flags. vm_get_page_prot()
implementation, could again call platform overrides arch_vm_get_page_prot()
and arch_filter_pgprot(). Some platforms override protection_map[] that was
originally built with __SXXX/__PXXX with different runtime values.

Currently there are multiple layers of abstraction i.e __SXXX/__PXXX macros
, protection_map[], arch_vm_get_page_prot() and arch_filter_pgprot() built
between the platform and generic MM, finally defining vm_get_page_prot().

Hence this series proposes to drop later two abstraction levels and instead
just move the responsibility of defining vm_get_page_prot() to the platform
(still utilizing generic protection_map[] array) itself making it clean and
simple.

This first introduces ARCH_HAS_VM_GET_PAGE_PROT which enables the platforms
to define custom vm_get_page_prot(). This starts converting platforms that
define the overrides arch_filter_pgprot() or arch_vm_get_page_prot() which
enables for those constructs to be dropped off completely.

The series has been inspired from an earlier discuss with Christoph Hellwig

https://lore.kernel.org/all/1632712920-8171-1-git-send-email-anshuman.khandual@arm.com/

This series applies on 5.18-rc1 after the following patch.

https://lore.kernel.org/all/1643004823-16441-1-git-send-email-anshuman.khandual@arm.com/

This series has been cross built for multiple platforms.

- Anshuman

Cc: Christoph Hellwig <hch@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: sparclinux@vger.kernel.org
Cc: linux-mm@kvack.org
Cc: linux-arch@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

Changes in V4:

- ARCH_HAS_VM_GET_PAGE_PROT now excludes generic protection_map[]
- Changed platform's vm_get_page_prot() to use generic protection_map[]
- Dropped all platform changes not enabling either arch_vm_get_page_prot() or arch_filter_pgprot() 
- Dropped all previous tags as code base has changed

Changes in V3:

https://lore.kernel.org/all/1646045273-9343-1-git-send-email-anshuman.khandual@arm.com/

- Dropped variable 'i' from sme_early_init() on x86 platform
- Moved CONFIG_COLDFIRE vm_get_page_prot() inside arch/m68k/mm/mcfmmu.c
- Moved CONFIG_SUN3 vm_get_page_prot() inside arch/m68k/mm/sun3mmu.c
- Dropped cachebits for vm_get_page_prot() inside arch/m68k/mm/motorola.c
- Dropped PAGE_XXX_C definitions from arch/m68k/include/asm/motorola_pgtable.h
- Used PAGE_XXX instead for vm_get_page_prot() inside arch/m68k/mm/motorola.c
- Dropped all references to protection_map[] in the tree
- Replaced s/extensa/xtensa/ on the patch title
- Moved left over comments from pgtable.h into init.c on nios2 platform

Changes in V2:

https://lore.kernel.org/all/1645425519-9034-1-git-send-email-anshuman.khandual@arm.com/

- Dropped the entire comment block in [PATCH 30/30] per Geert
- Replaced __P010 (although commented) with __PAGE_COPY on arm platform
- Replaced __P101 with PAGE_READONLY on um platform

Changes in V1:

https://lore.kernel.org/all/1644805853-21338-1-git-send-email-anshuman.khandual@arm.com/

- Add white spaces around the | operators 
- Moved powerpc_vm_get_page_prot() near vm_get_page_prot() on powerpc
- Moved arm64_vm_get_page_prot() near vm_get_page_prot() on arm64
- Moved sparc_vm_get_page_prot() near vm_get_page_prot() on sparc
- Compacted vm_get_page_prot() switch cases on all platforms
-  _PAGE_CACHE040 inclusion is dependent on CPU_IS_040_OR_060
- VM_SHARED case should return PAGE_NONE (not PAGE_COPY) on SH platform
- Reorganized VM_SHARED, VM_EXEC, VM_WRITE, VM_READ
- Dropped the last patch [RFC V1 31/31] which added macros for vm_flags combinations
  https://lore.kernel.org/all/1643029028-12710-32-git-send-email-anshuman.khandual@arm.com/

Changes in RFC:

https://lore.kernel.org/all/1643029028-12710-1-git-send-email-anshuman.khandual@arm.com/

Anshuman Khandual (6):
  mm/mmap: Add new config ARCH_HAS_VM_GET_PAGE_PROT
  powerpc/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT
  arm64/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT
  sparc/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT
  mm/mmap: Drop arch_filter_pgprot()
  mm/mmap: Drop arch_vm_get_page_pgprot()

Christoph Hellwig (1):
  x86/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT

 arch/arm64/Kconfig               |  1 +
 arch/arm64/include/asm/mman.h    | 24 ----------------------
 arch/arm64/mm/mmap.c             | 33 ++++++++++++++++++++++++++++++
 arch/powerpc/Kconfig             |  1 +
 arch/powerpc/include/asm/mman.h  | 12 -----------
 arch/powerpc/mm/mmap.c           | 26 ++++++++++++++++++++++++
 arch/sparc/Kconfig               |  1 +
 arch/sparc/include/asm/mman.h    |  6 ------
 arch/sparc/mm/init_64.c          | 13 ++++++++++++
 arch/x86/Kconfig                 |  2 +-
 arch/x86/include/asm/pgtable.h   |  5 -----
 arch/x86/include/uapi/asm/mman.h | 14 -------------
 arch/x86/mm/Makefile             |  2 +-
 arch/x86/mm/pgprot.c             | 35 ++++++++++++++++++++++++++++++++
 include/linux/mman.h             |  4 ----
 mm/Kconfig                       |  2 +-
 mm/mmap.c                        | 14 ++++---------
 17 files changed, 117 insertions(+), 78 deletions(-)
 create mode 100644 arch/x86/mm/pgprot.c

-- 
2.25.1

[PATCH V4 1/7] mm/mmap: Add new config ARCH_HAS_VM_GET_PAGE_PROT

[Anshuman Khandual]

Add a new config ARCH_HAS_VM_GET_PAGE_PROT, which when subscribed enables a
given platform to define its own vm_get_page_prot() but still utilizing the
generic protection_map[] array.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Suggested-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 mm/Kconfig | 3 +++
 mm/mmap.c  | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/mm/Kconfig b/mm/Kconfig
index 034d87953600..b1f7624276f8 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -765,6 +765,9 @@ config ARCH_HAS_CURRENT_STACK_POINTER
 config ARCH_HAS_FILTER_PGPROT
 	bool
 
+config ARCH_HAS_VM_GET_PAGE_PROT
+	bool
+
 config ARCH_HAS_PTE_DEVMAP
 	bool
 
diff --git a/mm/mmap.c b/mm/mmap.c
index 3aa839f81e63..87cb2eaf7e1a 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -106,6 +106,7 @@ pgprot_t protection_map[16] __ro_after_init = {
 	__S000, __S001, __S010, __S011, __S100, __S101, __S110, __S111
 };
 
+#ifndef CONFIG_ARCH_HAS_VM_GET_PAGE_PROT
 #ifndef CONFIG_ARCH_HAS_FILTER_PGPROT
 static inline pgprot_t arch_filter_pgprot(pgprot_t prot)
 {
@@ -122,6 +123,7 @@ pgprot_t vm_get_page_prot(unsigned long vm_flags)
 	return arch_filter_pgprot(ret);
 }
 EXPORT_SYMBOL(vm_get_page_prot);
+#endif	/* CONFIG_ARCH_HAS_VM_GET_PAGE_PROT */
 
 static pgprot_t vm_pgprot_modify(pgprot_t oldprot, unsigned long vm_flags)
 {
-- 
2.25.1

Re: [PATCH V4 1/7] mm/mmap: Add new config ARCH_HAS_VM_GET_PAGE_PROT

[Catalin Marinas]

On Thu, Apr 07, 2022 at 04:02:45PM +0530, Anshuman Khandual wrote:
> Add a new config ARCH_HAS_VM_GET_PAGE_PROT, which when subscribed enables a
> given platform to define its own vm_get_page_prot() but still utilizing the
> generic protection_map[] array.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Suggested-by: Christoph Hellwig <hch@infradead.org>
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

[PATCH V4 2/7] powerpc/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT

[Anshuman Khandual]

This defines and exports a platform specific custom vm_get_page_prot() via
subscribing ARCH_HAS_VM_GET_PAGE_PROT. While here, this also localizes
arch_vm_get_page_prot() as powerpc_vm_get_page_prot() and moves it near
vm_get_page_prot().

Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/powerpc/Kconfig            |  1 +
 arch/powerpc/include/asm/mman.h | 12 ------------
 arch/powerpc/mm/mmap.c          | 26 ++++++++++++++++++++++++++
 3 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 174edabb74fa..eb9b6ddbf92f 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -140,6 +140,7 @@ config PPC
 	select ARCH_HAS_TICK_BROADCAST		if GENERIC_CLOCKEVENTS_BROADCAST
 	select ARCH_HAS_UACCESS_FLUSHCACHE
 	select ARCH_HAS_UBSAN_SANITIZE_ALL
+	select ARCH_HAS_VM_GET_PAGE_PROT
 	select ARCH_HAVE_NMI_SAFE_CMPXCHG
 	select ARCH_KEEP_MEMBLOCK
 	select ARCH_MIGHT_HAVE_PC_PARPORT
diff --git a/arch/powerpc/include/asm/mman.h b/arch/powerpc/include/asm/mman.h
index 7cb6d18f5cd6..1b024e64c8ec 100644
--- a/arch/powerpc/include/asm/mman.h
+++ b/arch/powerpc/include/asm/mman.h
@@ -24,18 +24,6 @@ static inline unsigned long arch_calc_vm_prot_bits(unsigned long prot,
 }
 #define arch_calc_vm_prot_bits(prot, pkey) arch_calc_vm_prot_bits(prot, pkey)
 
-static inline pgprot_t arch_vm_get_page_prot(unsigned long vm_flags)
-{
-#ifdef CONFIG_PPC_MEM_KEYS
-	return (vm_flags & VM_SAO) ?
-		__pgprot(_PAGE_SAO | vmflag_to_pte_pkey_bits(vm_flags)) :
-		__pgprot(0 | vmflag_to_pte_pkey_bits(vm_flags));
-#else
-	return (vm_flags & VM_SAO) ? __pgprot(_PAGE_SAO) : __pgprot(0);
-#endif
-}
-#define arch_vm_get_page_prot(vm_flags) arch_vm_get_page_prot(vm_flags)
-
 static inline bool arch_validate_prot(unsigned long prot, unsigned long addr)
 {
 	if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM | PROT_SAO))
diff --git a/arch/powerpc/mm/mmap.c b/arch/powerpc/mm/mmap.c
index c475cf810aa8..cd17bd6fa36b 100644
--- a/arch/powerpc/mm/mmap.c
+++ b/arch/powerpc/mm/mmap.c
@@ -254,3 +254,29 @@ void arch_pick_mmap_layout(struct mm_struct *mm, struct rlimit *rlim_stack)
 		mm->get_unmapped_area = arch_get_unmapped_area_topdown;
 	}
 }
+
+#ifdef CONFIG_PPC64
+static pgprot_t powerpc_vm_get_page_prot(unsigned long vm_flags)
+{
+#ifdef CONFIG_PPC_MEM_KEYS
+	return (vm_flags & VM_SAO) ?
+		__pgprot(_PAGE_SAO | vmflag_to_pte_pkey_bits(vm_flags)) :
+		__pgprot(0 | vmflag_to_pte_pkey_bits(vm_flags));
+#else
+	return (vm_flags & VM_SAO) ? __pgprot(_PAGE_SAO) : __pgprot(0);
+#endif
+}
+#else
+static pgprot_t powerpc_vm_get_page_prot(unsigned long vm_flags)
+{
+	return __pgprot(0);
+}
+#endif /* CONFIG_PPC64 */
+
+pgprot_t vm_get_page_prot(unsigned long vm_flags)
+{
+	return __pgprot(pgprot_val(protection_map[vm_flags &
+			(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]) |
+	       pgprot_val(powerpc_vm_get_page_prot(vm_flags)));
+}
+EXPORT_SYMBOL(vm_get_page_prot);
-- 
2.25.1

Re: [PATCH V4 2/7] powerpc/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT

[Christophe Leroy]

Le 07/04/2022 à 12:32, Anshuman Khandual a écrit :
> This defines and exports a platform specific custom vm_get_page_prot() via
> subscribing ARCH_HAS_VM_GET_PAGE_PROT. While here, this also localizes
> arch_vm_get_page_prot() as powerpc_vm_get_page_prot() and moves it near
> vm_get_page_prot().
> 
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>   arch/powerpc/Kconfig            |  1 +
>   arch/powerpc/include/asm/mman.h | 12 ------------
>   arch/powerpc/mm/mmap.c          | 26 ++++++++++++++++++++++++++
>   3 files changed, 27 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 174edabb74fa..eb9b6ddbf92f 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -140,6 +140,7 @@ config PPC
>   	select ARCH_HAS_TICK_BROADCAST		if GENERIC_CLOCKEVENTS_BROADCAST
>   	select ARCH_HAS_UACCESS_FLUSHCACHE
>   	select ARCH_HAS_UBSAN_SANITIZE_ALL
> +	select ARCH_HAS_VM_GET_PAGE_PROT
>   	select ARCH_HAVE_NMI_SAFE_CMPXCHG
>   	select ARCH_KEEP_MEMBLOCK
>   	select ARCH_MIGHT_HAVE_PC_PARPORT
> diff --git a/arch/powerpc/include/asm/mman.h b/arch/powerpc/include/asm/mman.h
> index 7cb6d18f5cd6..1b024e64c8ec 100644
> --- a/arch/powerpc/include/asm/mman.h
> +++ b/arch/powerpc/include/asm/mman.h
> @@ -24,18 +24,6 @@ static inline unsigned long arch_calc_vm_prot_bits(unsigned long prot,
>   }
>   #define arch_calc_vm_prot_bits(prot, pkey) arch_calc_vm_prot_bits(prot, pkey)
>   
> -static inline pgprot_t arch_vm_get_page_prot(unsigned long vm_flags)
> -{
> -#ifdef CONFIG_PPC_MEM_KEYS
> -	return (vm_flags & VM_SAO) ?
> -		__pgprot(_PAGE_SAO | vmflag_to_pte_pkey_bits(vm_flags)) :
> -		__pgprot(0 | vmflag_to_pte_pkey_bits(vm_flags));
> -#else
> -	return (vm_flags & VM_SAO) ? __pgprot(_PAGE_SAO) : __pgprot(0);
> -#endif
> -}
> -#define arch_vm_get_page_prot(vm_flags) arch_vm_get_page_prot(vm_flags)
> -
>   static inline bool arch_validate_prot(unsigned long prot, unsigned long addr)
>   {
>   	if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM | PROT_SAO))
> diff --git a/arch/powerpc/mm/mmap.c b/arch/powerpc/mm/mmap.c
> index c475cf810aa8..cd17bd6fa36b 100644
> --- a/arch/powerpc/mm/mmap.c
> +++ b/arch/powerpc/mm/mmap.c
> @@ -254,3 +254,29 @@ void arch_pick_mmap_layout(struct mm_struct *mm, struct rlimit *rlim_stack)
>   		mm->get_unmapped_area = arch_get_unmapped_area_topdown;
>   	}
>   }
> +
> +#ifdef CONFIG_PPC64
> +static pgprot_t powerpc_vm_get_page_prot(unsigned long vm_flags)
> +{
> +#ifdef CONFIG_PPC_MEM_KEYS
> +	return (vm_flags & VM_SAO) ?
> +		__pgprot(_PAGE_SAO | vmflag_to_pte_pkey_bits(vm_flags)) :
> +		__pgprot(0 | vmflag_to_pte_pkey_bits(vm_flags));
> +#else
> +	return (vm_flags & VM_SAO) ? __pgprot(_PAGE_SAO) : __pgprot(0);
> +#endif
> +}
> +#else
> +static pgprot_t powerpc_vm_get_page_prot(unsigned long vm_flags)
> +{
> +	return __pgprot(0);
> +}
> +#endif /* CONFIG_PPC64 */

Can we reduce this forest of #ifdefs and make it more readable ?

mm/mmap.c is going away with patch 
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/d6d849621f821af253e777a24eda4c648814a76e.1646847562.git.christophe.leroy@csgroup.eu/

So it would be better to add two versions of vm_get_page_prot(), for 
instance one in mm/pgtable_64.c and one in mm/pgtable_32.c


> +
> +pgprot_t vm_get_page_prot(unsigned long vm_flags)
> +{
> +	return __pgprot(pgprot_val(protection_map[vm_flags &
> +			(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]) |
> +	       pgprot_val(powerpc_vm_get_page_prot(vm_flags)));
> +}
> +EXPORT_SYMBOL(vm_get_page_prot);

By the way I'm a bit puzzled with the name powerpc_vm_get_page_prot(), 
the name suggests that it's just a powerpc replacement of 
vm_get_page_prot(). I'd prefer if it was named __vm_get_page_prot(), it 
would be clearer that it is a complement to vm_get_page_prot() and not a 
remplacement.

Christophe

Re: [PATCH V4 2/7] powerpc/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT

[Christophe Leroy]

Le 08/04/2022 à 14:53, Christophe Leroy a écrit :
> 
> 
> Le 07/04/2022 à 12:32, Anshuman Khandual a écrit :
>> This defines and exports a platform specific custom vm_get_page_prot() 
>> via
>> subscribing ARCH_HAS_VM_GET_PAGE_PROT. While here, this also localizes
>> arch_vm_get_page_prot() as powerpc_vm_get_page_prot() and moves it near
>> vm_get_page_prot().
>>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>   arch/powerpc/Kconfig            |  1 +
>>   arch/powerpc/include/asm/mman.h | 12 ------------
>>   arch/powerpc/mm/mmap.c          | 26 ++++++++++++++++++++++++++
>>   3 files changed, 27 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index 174edabb74fa..eb9b6ddbf92f 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -140,6 +140,7 @@ config PPC
>>       select ARCH_HAS_TICK_BROADCAST        if 
>> GENERIC_CLOCKEVENTS_BROADCAST
>>       select ARCH_HAS_UACCESS_FLUSHCACHE
>>       select ARCH_HAS_UBSAN_SANITIZE_ALL
>> +    select ARCH_HAS_VM_GET_PAGE_PROT
>>       select ARCH_HAVE_NMI_SAFE_CMPXCHG
>>       select ARCH_KEEP_MEMBLOCK
>>       select ARCH_MIGHT_HAVE_PC_PARPORT
>> diff --git a/arch/powerpc/include/asm/mman.h 
>> b/arch/powerpc/include/asm/mman.h
>> index 7cb6d18f5cd6..1b024e64c8ec 100644
>> --- a/arch/powerpc/include/asm/mman.h
>> +++ b/arch/powerpc/include/asm/mman.h
>> @@ -24,18 +24,6 @@ static inline unsigned long 
>> arch_calc_vm_prot_bits(unsigned long prot,
>>   }
>>   #define arch_calc_vm_prot_bits(prot, pkey) 
>> arch_calc_vm_prot_bits(prot, pkey)
>> -static inline pgprot_t arch_vm_get_page_prot(unsigned long vm_flags)
>> -{
>> -#ifdef CONFIG_PPC_MEM_KEYS
>> -    return (vm_flags & VM_SAO) ?
>> -        __pgprot(_PAGE_SAO | vmflag_to_pte_pkey_bits(vm_flags)) :
>> -        __pgprot(0 | vmflag_to_pte_pkey_bits(vm_flags));
>> -#else
>> -    return (vm_flags & VM_SAO) ? __pgprot(_PAGE_SAO) : __pgprot(0);
>> -#endif
>> -}
>> -#define arch_vm_get_page_prot(vm_flags) arch_vm_get_page_prot(vm_flags)
>> -
>>   static inline bool arch_validate_prot(unsigned long prot, unsigned 
>> long addr)
>>   {
>>       if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM | 
>> PROT_SAO))
>> diff --git a/arch/powerpc/mm/mmap.c b/arch/powerpc/mm/mmap.c
>> index c475cf810aa8..cd17bd6fa36b 100644
>> --- a/arch/powerpc/mm/mmap.c
>> +++ b/arch/powerpc/mm/mmap.c
>> @@ -254,3 +254,29 @@ void arch_pick_mmap_layout(struct mm_struct *mm, 
>> struct rlimit *rlim_stack)
>>           mm->get_unmapped_area = arch_get_unmapped_area_topdown;
>>       }
>>   }
>> +
>> +#ifdef CONFIG_PPC64
>> +static pgprot_t powerpc_vm_get_page_prot(unsigned long vm_flags)
>> +{
>> +#ifdef CONFIG_PPC_MEM_KEYS
>> +    return (vm_flags & VM_SAO) ?
>> +        __pgprot(_PAGE_SAO | vmflag_to_pte_pkey_bits(vm_flags)) :
>> +        __pgprot(0 | vmflag_to_pte_pkey_bits(vm_flags));
>> +#else
>> +    return (vm_flags & VM_SAO) ? __pgprot(_PAGE_SAO) : __pgprot(0);
>> +#endif
>> +}
>> +#else
>> +static pgprot_t powerpc_vm_get_page_prot(unsigned long vm_flags)
>> +{
>> +    return __pgprot(0);
>> +}
>> +#endif /* CONFIG_PPC64 */
> 
> Can we reduce this forest of #ifdefs and make it more readable ?
> 
> mm/mmap.c is going away with patch 
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/d6d849621f821af253e777a24eda4c648814a76e.1646847562.git.christophe.leroy@csgroup.eu/ 
> 
> 
> So it would be better to add two versions of vm_get_page_prot(), for 
> instance one in mm/pgtable_64.c and one in mm/pgtable_32.c

Indeed, you don't need anything at all for PPC32. All you need to do is

	select ARCH_HAS_VM_GET_PAGE_PROT if PPC64

And in fact it could even be PPC_BOOK3S_64 instead of PPC64 because 
CONFIG_PPC_MEM_KEYS depends on PPC_BOOK3S_64 and _PAGE_SAO is 0 on 
nohash/64.

So you can then put it into arch/powerpc/mm/book3s64/pgtable.c


> 
> 
>> +
>> +pgprot_t vm_get_page_prot(unsigned long vm_flags)
>> +{
>> +    return __pgprot(pgprot_val(protection_map[vm_flags &
>> +            (VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]) |
>> +           pgprot_val(powerpc_vm_get_page_prot(vm_flags)));
>> +}
>> +EXPORT_SYMBOL(vm_get_page_prot);
> 
> By the way I'm a bit puzzled with the name powerpc_vm_get_page_prot(), 
> the name suggests that it's just a powerpc replacement of 
> vm_get_page_prot(). I'd prefer if it was named __vm_get_page_prot(), it 
> would be clearer that it is a complement to vm_get_page_prot() and not a 
> remplacement.
> 
> Christophe

Re: [PATCH V4 2/7] powerpc/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT

[Anshuman Khandual]

On 4/9/22 17:06, Christophe Leroy wrote:
> 
> 
> Le 08/04/2022 à 14:53, Christophe Leroy a écrit :
>>
>>
>> Le 07/04/2022 à 12:32, Anshuman Khandual a écrit :
>>> This defines and exports a platform specific custom vm_get_page_prot() via
>>> subscribing ARCH_HAS_VM_GET_PAGE_PROT. While here, this also localizes
>>> arch_vm_get_page_prot() as powerpc_vm_get_page_prot() and moves it near
>>> vm_get_page_prot().
>>>
>>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>>> Cc: Paul Mackerras <paulus@samba.org>
>>> Cc: linuxppc-dev@lists.ozlabs.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>> ---
>>>   arch/powerpc/Kconfig            |  1 +
>>>   arch/powerpc/include/asm/mman.h | 12 ------------
>>>   arch/powerpc/mm/mmap.c          | 26 ++++++++++++++++++++++++++
>>>   3 files changed, 27 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>>> index 174edabb74fa..eb9b6ddbf92f 100644
>>> --- a/arch/powerpc/Kconfig
>>> +++ b/arch/powerpc/Kconfig
>>> @@ -140,6 +140,7 @@ config PPC
>>>       select ARCH_HAS_TICK_BROADCAST        if GENERIC_CLOCKEVENTS_BROADCAST
>>>       select ARCH_HAS_UACCESS_FLUSHCACHE
>>>       select ARCH_HAS_UBSAN_SANITIZE_ALL
>>> +    select ARCH_HAS_VM_GET_PAGE_PROT
>>>       select ARCH_HAVE_NMI_SAFE_CMPXCHG
>>>       select ARCH_KEEP_MEMBLOCK
>>>       select ARCH_MIGHT_HAVE_PC_PARPORT
>>> diff --git a/arch/powerpc/include/asm/mman.h b/arch/powerpc/include/asm/mman.h
>>> index 7cb6d18f5cd6..1b024e64c8ec 100644
>>> --- a/arch/powerpc/include/asm/mman.h
>>> +++ b/arch/powerpc/include/asm/mman.h
>>> @@ -24,18 +24,6 @@ static inline unsigned long arch_calc_vm_prot_bits(unsigned long prot,
>>>   }
>>>   #define arch_calc_vm_prot_bits(prot, pkey) arch_calc_vm_prot_bits(prot, pkey)
>>> -static inline pgprot_t arch_vm_get_page_prot(unsigned long vm_flags)
>>> -{
>>> -#ifdef CONFIG_PPC_MEM_KEYS
>>> -    return (vm_flags & VM_SAO) ?
>>> -        __pgprot(_PAGE_SAO | vmflag_to_pte_pkey_bits(vm_flags)) :
>>> -        __pgprot(0 | vmflag_to_pte_pkey_bits(vm_flags));
>>> -#else
>>> -    return (vm_flags & VM_SAO) ? __pgprot(_PAGE_SAO) : __pgprot(0);
>>> -#endif
>>> -}
>>> -#define arch_vm_get_page_prot(vm_flags) arch_vm_get_page_prot(vm_flags)
>>> -
>>>   static inline bool arch_validate_prot(unsigned long prot, unsigned long addr)
>>>   {
>>>       if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM | PROT_SAO))
>>> diff --git a/arch/powerpc/mm/mmap.c b/arch/powerpc/mm/mmap.c
>>> index c475cf810aa8..cd17bd6fa36b 100644
>>> --- a/arch/powerpc/mm/mmap.c
>>> +++ b/arch/powerpc/mm/mmap.c
>>> @@ -254,3 +254,29 @@ void arch_pick_mmap_layout(struct mm_struct *mm, struct rlimit *rlim_stack)
>>>           mm->get_unmapped_area = arch_get_unmapped_area_topdown;
>>>       }
>>>   }
>>> +
>>> +#ifdef CONFIG_PPC64
>>> +static pgprot_t powerpc_vm_get_page_prot(unsigned long vm_flags)
>>> +{
>>> +#ifdef CONFIG_PPC_MEM_KEYS
>>> +    return (vm_flags & VM_SAO) ?
>>> +        __pgprot(_PAGE_SAO | vmflag_to_pte_pkey_bits(vm_flags)) :
>>> +        __pgprot(0 | vmflag_to_pte_pkey_bits(vm_flags));
>>> +#else
>>> +    return (vm_flags & VM_SAO) ? __pgprot(_PAGE_SAO) : __pgprot(0);
>>> +#endif
>>> +}
>>> +#else
>>> +static pgprot_t powerpc_vm_get_page_prot(unsigned long vm_flags)
>>> +{
>>> +    return __pgprot(0);
>>> +}
>>> +#endif /* CONFIG_PPC64 */
>>
>> Can we reduce this forest of #ifdefs and make it more readable ?
>>
>> mm/mmap.c is going away with patch https://patchwork.ozlabs.org/project/linuxppc-dev/patch/d6d849621f821af253e777a24eda4c648814a76e.1646847562.git.christophe.leroy@csgroup.eu/
>>
>> So it would be better to add two versions of vm_get_page_prot(), for instance one in mm/pgtable_64.c and one in mm/pgtable_32.c
> 
> Indeed, you don't need anything at all for PPC32. All you need to do is
> 
>     select ARCH_HAS_VM_GET_PAGE_PROT if PPC64
> 
> And in fact it could even be PPC_BOOK3S_64 instead of PPC64 because CONFIG_PPC_MEM_KEYS depends on PPC_BOOK3S_64 and _PAGE_SAO is 0 on nohash/64.
> 
> So you can then put it into arch/powerpc/mm/book3s64/pgtable.c

Would something like the following change work ?

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index eb9b6ddbf92f..69e44358a235 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -140,7 +140,7 @@ config PPC
        select ARCH_HAS_TICK_BROADCAST          if GENERIC_CLOCKEVENTS_BROADCAST
        select ARCH_HAS_UACCESS_FLUSHCACHE
        select ARCH_HAS_UBSAN_SANITIZE_ALL
-       select ARCH_HAS_VM_GET_PAGE_PROT
+       select ARCH_HAS_VM_GET_PAGE_PROT        if PPC_BOOK3S_64
        select ARCH_HAVE_NMI_SAFE_CMPXCHG
        select ARCH_KEEP_MEMBLOCK
        select ARCH_MIGHT_HAVE_PC_PARPORT
diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c
index 052e6590f84f..59d235519b44 100644
--- a/arch/powerpc/mm/book3s64/pgtable.c
+++ b/arch/powerpc/mm/book3s64/pgtable.c
@@ -7,6 +7,7 @@
 #include <linux/mm_types.h>
 #include <linux/memblock.h>
 #include <linux/memremap.h>
+#include <linux/pkeys.h>
 #include <linux/debugfs.h>
 #include <misc/cxl-base.h>
 
@@ -549,3 +550,22 @@ unsigned long memremap_compat_align(void)
 }
 EXPORT_SYMBOL_GPL(memremap_compat_align);
 #endif
+
+static pgprot_t __vm_get_page_prot(unsigned long vm_flags)
+{
+#ifdef CONFIG_PPC_MEM_KEYS
+       return (vm_flags & VM_SAO) ?
+               __pgprot(_PAGE_SAO | vmflag_to_pte_pkey_bits(vm_flags)) :
+               __pgprot(0 | vmflag_to_pte_pkey_bits(vm_flags));
+#else
+       return (vm_flags & VM_SAO) ? __pgprot(_PAGE_SAO) : __pgprot(0);
+#endif
+}
+
+pgprot_t vm_get_page_prot(unsigned long vm_flags)
+{
+       return __pgprot(pgprot_val(protection_map[vm_flags &
+                       (VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]) |
+              pgprot_val(__vm_get_page_prot(vm_flags)));
+}
+EXPORT_SYMBOL(vm_get_page_prot);

Re: [PATCH V4 2/7] powerpc/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT

[Christophe Leroy]

Le 11/04/2022 à 07:31, Anshuman Khandual a écrit :
> 
> 
> On 4/9/22 17:06, Christophe Leroy wrote:
>>
>>
>> Le 08/04/2022 à 14:53, Christophe Leroy a écrit :
>>>
>>>
>>> Le 07/04/2022 à 12:32, Anshuman Khandual a écrit :
>>>> This defines and exports a platform specific custom vm_get_page_prot() via
>>>> subscribing ARCH_HAS_VM_GET_PAGE_PROT. While here, this also localizes
>>>> arch_vm_get_page_prot() as powerpc_vm_get_page_prot() and moves it near
>>>> vm_get_page_prot().
>>>>
>>>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>>>> Cc: Paul Mackerras <paulus@samba.org>
>>>> Cc: linuxppc-dev@lists.ozlabs.org
>>>> Cc: linux-kernel@vger.kernel.org
>>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>>> ---
>>>>    arch/powerpc/Kconfig            |  1 +
>>>>    arch/powerpc/include/asm/mman.h | 12 ------------
>>>>    arch/powerpc/mm/mmap.c          | 26 ++++++++++++++++++++++++++
>>>>    3 files changed, 27 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>>>> index 174edabb74fa..eb9b6ddbf92f 100644
>>>> --- a/arch/powerpc/Kconfig
>>>> +++ b/arch/powerpc/Kconfig
>>>> @@ -140,6 +140,7 @@ config PPC
>>>>        select ARCH_HAS_TICK_BROADCAST        if GENERIC_CLOCKEVENTS_BROADCAST
>>>>        select ARCH_HAS_UACCESS_FLUSHCACHE
>>>>        select ARCH_HAS_UBSAN_SANITIZE_ALL
>>>> +    select ARCH_HAS_VM_GET_PAGE_PROT
>>>>        select ARCH_HAVE_NMI_SAFE_CMPXCHG
>>>>        select ARCH_KEEP_MEMBLOCK
>>>>        select ARCH_MIGHT_HAVE_PC_PARPORT
>>>> diff --git a/arch/powerpc/include/asm/mman.h b/arch/powerpc/include/asm/mman.h
>>>> index 7cb6d18f5cd6..1b024e64c8ec 100644
>>>> --- a/arch/powerpc/include/asm/mman.h
>>>> +++ b/arch/powerpc/include/asm/mman.h
>>>> @@ -24,18 +24,6 @@ static inline unsigned long arch_calc_vm_prot_bits(unsigned long prot,
>>>>    }
>>>>    #define arch_calc_vm_prot_bits(prot, pkey) arch_calc_vm_prot_bits(prot, pkey)
>>>> -static inline pgprot_t arch_vm_get_page_prot(unsigned long vm_flags)
>>>> -{
>>>> -#ifdef CONFIG_PPC_MEM_KEYS
>>>> -    return (vm_flags & VM_SAO) ?
>>>> -        __pgprot(_PAGE_SAO | vmflag_to_pte_pkey_bits(vm_flags)) :
>>>> -        __pgprot(0 | vmflag_to_pte_pkey_bits(vm_flags));
>>>> -#else
>>>> -    return (vm_flags & VM_SAO) ? __pgprot(_PAGE_SAO) : __pgprot(0);
>>>> -#endif
>>>> -}
>>>> -#define arch_vm_get_page_prot(vm_flags) arch_vm_get_page_prot(vm_flags)
>>>> -
>>>>    static inline bool arch_validate_prot(unsigned long prot, unsigned long addr)
>>>>    {
>>>>        if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM | PROT_SAO))
>>>> diff --git a/arch/powerpc/mm/mmap.c b/arch/powerpc/mm/mmap.c
>>>> index c475cf810aa8..cd17bd6fa36b 100644
>>>> --- a/arch/powerpc/mm/mmap.c
>>>> +++ b/arch/powerpc/mm/mmap.c
>>>> @@ -254,3 +254,29 @@ void arch_pick_mmap_layout(struct mm_struct *mm, struct rlimit *rlim_stack)
>>>>            mm->get_unmapped_area = arch_get_unmapped_area_topdown;
>>>>        }
>>>>    }
>>>> +
>>>> +#ifdef CONFIG_PPC64
>>>> +static pgprot_t powerpc_vm_get_page_prot(unsigned long vm_flags)
>>>> +{
>>>> +#ifdef CONFIG_PPC_MEM_KEYS
>>>> +    return (vm_flags & VM_SAO) ?
>>>> +        __pgprot(_PAGE_SAO | vmflag_to_pte_pkey_bits(vm_flags)) :
>>>> +        __pgprot(0 | vmflag_to_pte_pkey_bits(vm_flags));
>>>> +#else
>>>> +    return (vm_flags & VM_SAO) ? __pgprot(_PAGE_SAO) : __pgprot(0);
>>>> +#endif
>>>> +}
>>>> +#else
>>>> +static pgprot_t powerpc_vm_get_page_prot(unsigned long vm_flags)
>>>> +{
>>>> +    return __pgprot(0);
>>>> +}
>>>> +#endif /* CONFIG_PPC64 */
>>>
>>> Can we reduce this forest of #ifdefs and make it more readable ?
>>>
>>> mm/mmap.c is going away with patch https://patchwork.ozlabs.org/project/linuxppc-dev/patch/d6d849621f821af253e777a24eda4c648814a76e.1646847562.git.christophe.leroy@csgroup.eu/
>>>
>>> So it would be better to add two versions of vm_get_page_prot(), for instance one in mm/pgtable_64.c and one in mm/pgtable_32.c
>>
>> Indeed, you don't need anything at all for PPC32. All you need to do is
>>
>>      select ARCH_HAS_VM_GET_PAGE_PROT if PPC64
>>
>> And in fact it could even be PPC_BOOK3S_64 instead of PPC64 because CONFIG_PPC_MEM_KEYS depends on PPC_BOOK3S_64 and _PAGE_SAO is 0 on nohash/64.
>>
>> So you can then put it into arch/powerpc/mm/book3s64/pgtable.c
> 
> Would something like the following change work ?

Yes that looks good to me.

Thanks
Christophe

> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index eb9b6ddbf92f..69e44358a235 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -140,7 +140,7 @@ config PPC
>          select ARCH_HAS_TICK_BROADCAST          if GENERIC_CLOCKEVENTS_BROADCAST
>          select ARCH_HAS_UACCESS_FLUSHCACHE
>          select ARCH_HAS_UBSAN_SANITIZE_ALL
> -       select ARCH_HAS_VM_GET_PAGE_PROT
> +       select ARCH_HAS_VM_GET_PAGE_PROT        if PPC_BOOK3S_64
>          select ARCH_HAVE_NMI_SAFE_CMPXCHG
>          select ARCH_KEEP_MEMBLOCK
>          select ARCH_MIGHT_HAVE_PC_PARPORT
> diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c
> index 052e6590f84f..59d235519b44 100644
> --- a/arch/powerpc/mm/book3s64/pgtable.c
> +++ b/arch/powerpc/mm/book3s64/pgtable.c
> @@ -7,6 +7,7 @@
>   #include <linux/mm_types.h>
>   #include <linux/memblock.h>
>   #include <linux/memremap.h>
> +#include <linux/pkeys.h>
>   #include <linux/debugfs.h>
>   #include <misc/cxl-base.h>
>   
> @@ -549,3 +550,22 @@ unsigned long memremap_compat_align(void)
>   }
>   EXPORT_SYMBOL_GPL(memremap_compat_align);
>   #endif
> +
> +static pgprot_t __vm_get_page_prot(unsigned long vm_flags)
> +{
> +#ifdef CONFIG_PPC_MEM_KEYS
> +       return (vm_flags & VM_SAO) ?
> +               __pgprot(_PAGE_SAO | vmflag_to_pte_pkey_bits(vm_flags)) :
> +               __pgprot(0 | vmflag_to_pte_pkey_bits(vm_flags));
> +#else
> +       return (vm_flags & VM_SAO) ? __pgprot(_PAGE_SAO) : __pgprot(0);
> +#endif
> +}
> +
> +pgprot_t vm_get_page_prot(unsigned long vm_flags)
> +{
> +       return __pgprot(pgprot_val(protection_map[vm_flags &
> +                       (VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]) |
> +              pgprot_val(__vm_get_page_prot(vm_flags)));
> +}
> +EXPORT_SYMBOL(vm_get_page_prot);

[PATCH V4 3/7] arm64/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT

[Anshuman Khandual]

This defines and exports a platform specific custom vm_get_page_prot() via
subscribing ARCH_HAS_VM_GET_PAGE_PROT. It localizes arch_vm_get_page_prot()
and moves it near vm_get_page_prot().

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/arm64/Kconfig            |  1 +
 arch/arm64/include/asm/mman.h | 24 ------------------------
 arch/arm64/mm/mmap.c          | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 57c4c995965f..dd0b15162bb3 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -45,6 +45,7 @@ config ARM64
 	select ARCH_HAS_SYSCALL_WRAPPER
 	select ARCH_HAS_TEARDOWN_DMA_OPS if IOMMU_SUPPORT
 	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
+	select ARCH_HAS_VM_GET_PAGE_PROT
 	select ARCH_HAS_ZONE_DMA_SET if EXPERT
 	select ARCH_HAVE_ELF_PROT
 	select ARCH_HAVE_NMI_SAFE_CMPXCHG
diff --git a/arch/arm64/include/asm/mman.h b/arch/arm64/include/asm/mman.h
index e3e28f7daf62..5966ee4a6154 100644
--- a/arch/arm64/include/asm/mman.h
+++ b/arch/arm64/include/asm/mman.h
@@ -35,30 +35,6 @@ static inline unsigned long arch_calc_vm_flag_bits(unsigned long flags)
 }
 #define arch_calc_vm_flag_bits(flags) arch_calc_vm_flag_bits(flags)
 
-static inline pgprot_t arch_vm_get_page_prot(unsigned long vm_flags)
-{
-	pteval_t prot = 0;
-
-	if (vm_flags & VM_ARM64_BTI)
-		prot |= PTE_GP;
-
-	/*
-	 * There are two conditions required for returning a Normal Tagged
-	 * memory type: (1) the user requested it via PROT_MTE passed to
-	 * mmap() or mprotect() and (2) the corresponding vma supports MTE. We
-	 * register (1) as VM_MTE in the vma->vm_flags and (2) as
-	 * VM_MTE_ALLOWED. Note that the latter can only be set during the
-	 * mmap() call since mprotect() does not accept MAP_* flags.
-	 * Checking for VM_MTE only is sufficient since arch_validate_flags()
-	 * does not permit (VM_MTE & !VM_MTE_ALLOWED).
-	 */
-	if (vm_flags & VM_MTE)
-		prot |= PTE_ATTRINDX(MT_NORMAL_TAGGED);
-
-	return __pgprot(prot);
-}
-#define arch_vm_get_page_prot(vm_flags) arch_vm_get_page_prot(vm_flags)
-
 static inline bool arch_validate_prot(unsigned long prot,
 	unsigned long addr __always_unused)
 {
diff --git a/arch/arm64/mm/mmap.c b/arch/arm64/mm/mmap.c
index 77ada00280d9..307534fcec00 100644
--- a/arch/arm64/mm/mmap.c
+++ b/arch/arm64/mm/mmap.c
@@ -55,3 +55,36 @@ static int __init adjust_protection_map(void)
 	return 0;
 }
 arch_initcall(adjust_protection_map);
+
+static pgprot_t arm64_arch_vm_get_page_prot(unsigned long vm_flags)
+{
+	pteval_t prot = 0;
+
+	if (vm_flags & VM_ARM64_BTI)
+		prot |= PTE_GP;
+
+	/*
+	 * There are two conditions required for returning a Normal Tagged
+	 * memory type: (1) the user requested it via PROT_MTE passed to
+	 * mmap() or mprotect() and (2) the corresponding vma supports MTE. We
+	 * register (1) as VM_MTE in the vma->vm_flags and (2) as
+	 * VM_MTE_ALLOWED. Note that the latter can only be set during the
+	 * mmap() call since mprotect() does not accept MAP_* flags.
+	 * Checking for VM_MTE only is sufficient since arch_validate_flags()
+	 * does not permit (VM_MTE & !VM_MTE_ALLOWED).
+	 */
+	if (vm_flags & VM_MTE)
+		prot |= PTE_ATTRINDX(MT_NORMAL_TAGGED);
+
+	return __pgprot(prot);
+}
+
+pgprot_t vm_get_page_prot(unsigned long vm_flags)
+{
+	pgprot_t ret = __pgprot(pgprot_val(protection_map[vm_flags &
+				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]) |
+				pgprot_val(arm64_arch_vm_get_page_prot(vm_flags)));
+
+	return ret;
+}
+EXPORT_SYMBOL(vm_get_page_prot);
-- 
2.25.1

Re: [PATCH V4 3/7] arm64/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT

[Catalin Marinas]

On Thu, Apr 07, 2022 at 04:02:47PM +0530, Anshuman Khandual wrote:
> diff --git a/arch/arm64/mm/mmap.c b/arch/arm64/mm/mmap.c
> index 77ada00280d9..307534fcec00 100644
> --- a/arch/arm64/mm/mmap.c
> +++ b/arch/arm64/mm/mmap.c
> @@ -55,3 +55,36 @@ static int __init adjust_protection_map(void)
>  	return 0;
>  }
>  arch_initcall(adjust_protection_map);
> +
> +static pgprot_t arm64_arch_vm_get_page_prot(unsigned long vm_flags)
> +{
> +	pteval_t prot = 0;
> +
> +	if (vm_flags & VM_ARM64_BTI)
> +		prot |= PTE_GP;
> +
> +	/*
> +	 * There are two conditions required for returning a Normal Tagged
> +	 * memory type: (1) the user requested it via PROT_MTE passed to
> +	 * mmap() or mprotect() and (2) the corresponding vma supports MTE. We
> +	 * register (1) as VM_MTE in the vma->vm_flags and (2) as
> +	 * VM_MTE_ALLOWED. Note that the latter can only be set during the
> +	 * mmap() call since mprotect() does not accept MAP_* flags.
> +	 * Checking for VM_MTE only is sufficient since arch_validate_flags()
> +	 * does not permit (VM_MTE & !VM_MTE_ALLOWED).
> +	 */
> +	if (vm_flags & VM_MTE)
> +		prot |= PTE_ATTRINDX(MT_NORMAL_TAGGED);
> +
> +	return __pgprot(prot);
> +}
> +
> +pgprot_t vm_get_page_prot(unsigned long vm_flags)
> +{
> +	pgprot_t ret = __pgprot(pgprot_val(protection_map[vm_flags &
> +				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]) |
> +				pgprot_val(arm64_arch_vm_get_page_prot(vm_flags)));
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(vm_get_page_prot);

Could you write all this in a single function? I think I mentioned it in
a previous series (untested):

pgprot_t vm_get_page_prot(unsigned long vm_flags)
{
	pteval_t prot = pgprot_val(protection_map[vm_flags &
				   (VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]);

	if (vm_flags & VM_ARM64_BTI)
		prot |= PTE_GP;

	/*
	 * There are two conditions required for returning a Normal Tagged
	 * memory type: (1) the user requested it via PROT_MTE passed to
	 * mmap() or mprotect() and (2) the corresponding vma supports MTE. We
	 * register (1) as VM_MTE in the vma->vm_flags and (2) as
	 * VM_MTE_ALLOWED. Note that the latter can only be set during the
	 * mmap() call since mprotect() does not accept MAP_* flags.
	 * Checking for VM_MTE only is sufficient since arch_validate_flags()
	 * does not permit (VM_MTE & !VM_MTE_ALLOWED).
	 */
	if (vm_flags & VM_MTE)
		prot |= PTE_ATTRINDX(MT_NORMAL_TAGGED);

	return __pgprot(prot);
}
EXPORT_SYMBOL(vm_get_page_prot);

With that:

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

Re: [PATCH V4 3/7] arm64/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT

[Anshuman Khandual]

On 4/8/22 15:58, Catalin Marinas wrote:
> On Thu, Apr 07, 2022 at 04:02:47PM +0530, Anshuman Khandual wrote:
>> diff --git a/arch/arm64/mm/mmap.c b/arch/arm64/mm/mmap.c
>> index 77ada00280d9..307534fcec00 100644
>> --- a/arch/arm64/mm/mmap.c
>> +++ b/arch/arm64/mm/mmap.c
>> @@ -55,3 +55,36 @@ static int __init adjust_protection_map(void)
>>  	return 0;
>>  }
>>  arch_initcall(adjust_protection_map);
>> +
>> +static pgprot_t arm64_arch_vm_get_page_prot(unsigned long vm_flags)
>> +{
>> +	pteval_t prot = 0;
>> +
>> +	if (vm_flags & VM_ARM64_BTI)
>> +		prot |= PTE_GP;
>> +
>> +	/*
>> +	 * There are two conditions required for returning a Normal Tagged
>> +	 * memory type: (1) the user requested it via PROT_MTE passed to
>> +	 * mmap() or mprotect() and (2) the corresponding vma supports MTE. We
>> +	 * register (1) as VM_MTE in the vma->vm_flags and (2) as
>> +	 * VM_MTE_ALLOWED. Note that the latter can only be set during the
>> +	 * mmap() call since mprotect() does not accept MAP_* flags.
>> +	 * Checking for VM_MTE only is sufficient since arch_validate_flags()
>> +	 * does not permit (VM_MTE & !VM_MTE_ALLOWED).
>> +	 */
>> +	if (vm_flags & VM_MTE)
>> +		prot |= PTE_ATTRINDX(MT_NORMAL_TAGGED);
>> +
>> +	return __pgprot(prot);
>> +}
>> +
>> +pgprot_t vm_get_page_prot(unsigned long vm_flags)
>> +{
>> +	pgprot_t ret = __pgprot(pgprot_val(protection_map[vm_flags &
>> +				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]) |
>> +				pgprot_val(arm64_arch_vm_get_page_prot(vm_flags)));
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL(vm_get_page_prot);
> 
> Could you write all this in a single function? I think I mentioned it in
> a previous series (untested):

Right, missed that.

> 
> pgprot_t vm_get_page_prot(unsigned long vm_flags)
> {
> 	pteval_t prot = pgprot_val(protection_map[vm_flags &
> 				   (VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]);
> 
> 	if (vm_flags & VM_ARM64_BTI)
> 		prot |= PTE_GP;
> 
> 	/*
> 	 * There are two conditions required for returning a Normal Tagged
> 	 * memory type: (1) the user requested it via PROT_MTE passed to
> 	 * mmap() or mprotect() and (2) the corresponding vma supports MTE. We
> 	 * register (1) as VM_MTE in the vma->vm_flags and (2) as
> 	 * VM_MTE_ALLOWED. Note that the latter can only be set during the
> 	 * mmap() call since mprotect() does not accept MAP_* flags.
> 	 * Checking for VM_MTE only is sufficient since arch_validate_flags()
> 	 * does not permit (VM_MTE & !VM_MTE_ALLOWED).
> 	 */
> 	if (vm_flags & VM_MTE)
> 		prot |= PTE_ATTRINDX(MT_NORMAL_TAGGED);
> 
> 	return __pgprot(prot);
> }
> EXPORT_SYMBOL(vm_get_page_prot);
> 
> With that:

Sure, will change them into a single function.

> 
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> 

[PATCH V4 4/7] sparc/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT

[Anshuman Khandual]

This defines and exports a platform specific custom vm_get_page_prot() via
subscribing ARCH_HAS_VM_GET_PAGE_PROT. It localizes arch_vm_get_page_prot()
as sparc_vm_get_page_prot() and moves near vm_get_page_prot().

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Khalid Aziz <khalid.aziz@oracle.com>
Cc: sparclinux@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/sparc/Kconfig            |  1 +
 arch/sparc/include/asm/mman.h |  6 ------
 arch/sparc/mm/init_64.c       | 13 +++++++++++++
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 9200bc04701c..85b573643af6 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -84,6 +84,7 @@ config SPARC64
 	select PERF_USE_VMALLOC
 	select ARCH_HAVE_NMI_SAFE_CMPXCHG
 	select HAVE_C_RECORDMCOUNT
+	select ARCH_HAS_VM_GET_PAGE_PROT
 	select HAVE_ARCH_AUDITSYSCALL
 	select ARCH_SUPPORTS_ATOMIC_RMW
 	select ARCH_SUPPORTS_DEBUG_PAGEALLOC
diff --git a/arch/sparc/include/asm/mman.h b/arch/sparc/include/asm/mman.h
index 274217e7ed70..af9c10c83dc5 100644
--- a/arch/sparc/include/asm/mman.h
+++ b/arch/sparc/include/asm/mman.h
@@ -46,12 +46,6 @@ static inline unsigned long sparc_calc_vm_prot_bits(unsigned long prot)
 	}
 }
 
-#define arch_vm_get_page_prot(vm_flags) sparc_vm_get_page_prot(vm_flags)
-static inline pgprot_t sparc_vm_get_page_prot(unsigned long vm_flags)
-{
-	return (vm_flags & VM_SPARC_ADI) ? __pgprot(_PAGE_MCD_4V) : __pgprot(0);
-}
-
 #define arch_validate_prot(prot, addr) sparc_validate_prot(prot, addr)
 static inline int sparc_validate_prot(unsigned long prot, unsigned long addr)
 {
diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
index 8b1911591581..dcb17763c1f2 100644
--- a/arch/sparc/mm/init_64.c
+++ b/arch/sparc/mm/init_64.c
@@ -3184,3 +3184,16 @@ void copy_highpage(struct page *to, struct page *from)
 	}
 }
 EXPORT_SYMBOL(copy_highpage);
+
+static pgprot_t sparc_vm_get_page_prot(unsigned long vm_flags)
+{
+	return (vm_flags & VM_SPARC_ADI) ? __pgprot(_PAGE_MCD_4V) : __pgprot(0);
+}
+
+pgprot_t vm_get_page_prot(unsigned long vm_flags)
+{
+	return __pgprot(pgprot_val(protection_map[vm_flags &
+			(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]) |
+			pgprot_val(sparc_vm_get_page_prot(vm_flags)));
+}
+EXPORT_SYMBOL(vm_get_page_prot);
-- 
2.25.1

Re: [PATCH V4 4/7] sparc/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT

[Khalid Aziz]

On 4/7/22 04:32, Anshuman Khandual wrote:
> This defines and exports a platform specific custom vm_get_page_prot() via
> subscribing ARCH_HAS_VM_GET_PAGE_PROT. It localizes arch_vm_get_page_prot()
> as sparc_vm_get_page_prot() and moves near vm_get_page_prot().
> 
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Khalid Aziz <khalid.aziz@oracle.com>
> Cc: sparclinux@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>   arch/sparc/Kconfig            |  1 +
>   arch/sparc/include/asm/mman.h |  6 ------
>   arch/sparc/mm/init_64.c       | 13 +++++++++++++
>   3 files changed, 14 insertions(+), 6 deletions(-)
> 

Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com>


> diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
> index 9200bc04701c..85b573643af6 100644
> --- a/arch/sparc/Kconfig
> +++ b/arch/sparc/Kconfig
> @@ -84,6 +84,7 @@ config SPARC64
>   	select PERF_USE_VMALLOC
>   	select ARCH_HAVE_NMI_SAFE_CMPXCHG
>   	select HAVE_C_RECORDMCOUNT
> +	select ARCH_HAS_VM_GET_PAGE_PROT
>   	select HAVE_ARCH_AUDITSYSCALL
>   	select ARCH_SUPPORTS_ATOMIC_RMW
>   	select ARCH_SUPPORTS_DEBUG_PAGEALLOC
> diff --git a/arch/sparc/include/asm/mman.h b/arch/sparc/include/asm/mman.h
> index 274217e7ed70..af9c10c83dc5 100644
> --- a/arch/sparc/include/asm/mman.h
> +++ b/arch/sparc/include/asm/mman.h
> @@ -46,12 +46,6 @@ static inline unsigned long sparc_calc_vm_prot_bits(unsigned long prot)
>   	}
>   }
>   
> -#define arch_vm_get_page_prot(vm_flags) sparc_vm_get_page_prot(vm_flags)
> -static inline pgprot_t sparc_vm_get_page_prot(unsigned long vm_flags)
> -{
> -	return (vm_flags & VM_SPARC_ADI) ? __pgprot(_PAGE_MCD_4V) : __pgprot(0);
> -}
> -
>   #define arch_validate_prot(prot, addr) sparc_validate_prot(prot, addr)
>   static inline int sparc_validate_prot(unsigned long prot, unsigned long addr)
>   {
> diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
> index 8b1911591581..dcb17763c1f2 100644
> --- a/arch/sparc/mm/init_64.c
> +++ b/arch/sparc/mm/init_64.c
> @@ -3184,3 +3184,16 @@ void copy_highpage(struct page *to, struct page *from)
>   	}
>   }
>   EXPORT_SYMBOL(copy_highpage);
> +
> +static pgprot_t sparc_vm_get_page_prot(unsigned long vm_flags)
> +{
> +	return (vm_flags & VM_SPARC_ADI) ? __pgprot(_PAGE_MCD_4V) : __pgprot(0);
> +}
> +
> +pgprot_t vm_get_page_prot(unsigned long vm_flags)
> +{
> +	return __pgprot(pgprot_val(protection_map[vm_flags &
> +			(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]) |
> +			pgprot_val(sparc_vm_get_page_prot(vm_flags)));
> +}
> +EXPORT_SYMBOL(vm_get_page_prot);

[PATCH V4 5/7] x86/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT

[Anshuman Khandual]

From: Christoph Hellwig <hch@infradead.org>

This defines and exports a platform specific custom vm_get_page_prot() via
subscribing ARCH_HAS_VM_GET_PAGE_PROT. This also unsubscribes from config
ARCH_HAS_FILTER_PGPROT, after dropping off arch_filter_pgprot() and
arch_vm_get_page_prot().

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/x86/Kconfig                 |  2 +-
 arch/x86/include/asm/pgtable.h   |  5 -----
 arch/x86/include/uapi/asm/mman.h | 14 -------------
 arch/x86/mm/Makefile             |  2 +-
 arch/x86/mm/pgprot.c             | 35 ++++++++++++++++++++++++++++++++
 5 files changed, 37 insertions(+), 21 deletions(-)
 create mode 100644 arch/x86/mm/pgprot.c

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index b0142e01002e..c355c420150e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -76,7 +76,6 @@ config X86
 	select ARCH_HAS_EARLY_DEBUG		if KGDB
 	select ARCH_HAS_ELF_RANDOMIZE
 	select ARCH_HAS_FAST_MULTIPLIER
-	select ARCH_HAS_FILTER_PGPROT
 	select ARCH_HAS_FORTIFY_SOURCE
 	select ARCH_HAS_GCOV_PROFILE_ALL
 	select ARCH_HAS_KCOV			if X86_64
@@ -95,6 +94,7 @@ config X86
 	select ARCH_HAS_SYNC_CORE_BEFORE_USERMODE
 	select ARCH_HAS_SYSCALL_WRAPPER
 	select ARCH_HAS_UBSAN_SANITIZE_ALL
+	select ARCH_HAS_VM_GET_PAGE_PROT
 	select ARCH_HAS_DEBUG_WX
 	select ARCH_HAS_ZONE_DMA_SET if EXPERT
 	select ARCH_HAVE_NMI_SAFE_CMPXCHG
diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index 62ab07e24aef..3563f4645fa1 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -649,11 +649,6 @@ static inline pgprot_t pgprot_modify(pgprot_t oldprot, pgprot_t newprot)
 
 #define canon_pgprot(p) __pgprot(massage_pgprot(p))
 
-static inline pgprot_t arch_filter_pgprot(pgprot_t prot)
-{
-	return canon_pgprot(prot);
-}
-
 static inline int is_new_memtype_allowed(u64 paddr, unsigned long size,
 					 enum page_cache_mode pcm,
 					 enum page_cache_mode new_pcm)
diff --git a/arch/x86/include/uapi/asm/mman.h b/arch/x86/include/uapi/asm/mman.h
index d4a8d0424bfb..775dbd3aff73 100644
--- a/arch/x86/include/uapi/asm/mman.h
+++ b/arch/x86/include/uapi/asm/mman.h
@@ -5,20 +5,6 @@
 #define MAP_32BIT	0x40		/* only give out 32bit addresses */
 
 #ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
-/*
- * Take the 4 protection key bits out of the vma->vm_flags
- * value and turn them in to the bits that we can put in
- * to a pte.
- *
- * Only override these if Protection Keys are available
- * (which is only on 64-bit).
- */
-#define arch_vm_get_page_prot(vm_flags)	__pgprot(	\
-		((vm_flags) & VM_PKEY_BIT0 ? _PAGE_PKEY_BIT0 : 0) |	\
-		((vm_flags) & VM_PKEY_BIT1 ? _PAGE_PKEY_BIT1 : 0) |	\
-		((vm_flags) & VM_PKEY_BIT2 ? _PAGE_PKEY_BIT2 : 0) |	\
-		((vm_flags) & VM_PKEY_BIT3 ? _PAGE_PKEY_BIT3 : 0))
-
 #define arch_calc_vm_prot_bits(prot, key) (		\
 		((key) & 0x1 ? VM_PKEY_BIT0 : 0) |      \
 		((key) & 0x2 ? VM_PKEY_BIT1 : 0) |      \
diff --git a/arch/x86/mm/Makefile b/arch/x86/mm/Makefile
index fe3d3061fc11..fb6b41a48ae5 100644
--- a/arch/x86/mm/Makefile
+++ b/arch/x86/mm/Makefile
@@ -20,7 +20,7 @@ CFLAGS_REMOVE_mem_encrypt_identity.o	= -pg
 endif
 
 obj-y				:=  init.o init_$(BITS).o fault.o ioremap.o extable.o mmap.o \
-				    pgtable.o physaddr.o setup_nx.o tlb.o cpu_entry_area.o maccess.o
+				    pgtable.o physaddr.o setup_nx.o tlb.o cpu_entry_area.o maccess.o pgprot.o
 
 obj-y				+= pat/
 
diff --git a/arch/x86/mm/pgprot.c b/arch/x86/mm/pgprot.c
new file mode 100644
index 000000000000..763742782286
--- /dev/null
+++ b/arch/x86/mm/pgprot.c
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/export.h>
+#include <linux/mm.h>
+#include <asm/pgtable.h>
+
+pgprot_t vm_get_page_prot(unsigned long vm_flags)
+{
+	unsigned long val = pgprot_val(protection_map[vm_flags &
+				      (VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]);
+
+#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
+	/*
+	 * Take the 4 protection key bits out of the vma->vm_flags value and
+	 * turn them in to the bits that we can put in to a pte.
+	 *
+	 * Only override these if Protection Keys are available (which is only
+	 * on 64-bit).
+	 */
+	if (vm_flags & VM_PKEY_BIT0)
+		val |= _PAGE_PKEY_BIT0;
+	if (vm_flags & VM_PKEY_BIT1)
+		val |= _PAGE_PKEY_BIT1;
+	if (vm_flags & VM_PKEY_BIT2)
+		val |= _PAGE_PKEY_BIT2;
+	if (vm_flags & VM_PKEY_BIT3)
+		val |= _PAGE_PKEY_BIT3;
+#endif
+
+	val = __sme_set(val);
+	if (val & _PAGE_PRESENT)
+		val &= __supported_pte_mask;
+	return __pgprot(val);
+}
+EXPORT_SYMBOL(vm_get_page_prot);
-- 
2.25.1

[PATCH V4 6/7] mm/mmap: Drop arch_filter_pgprot()

[Anshuman Khandual]

There are no platforms left which subscribe ARCH_HAS_FILTER_PGPROT. Hence
drop generic arch_filter_pgprot() and also config ARCH_HAS_FILTER_PGPROT.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 mm/Kconfig | 3 ---
 mm/mmap.c  | 9 +--------
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/mm/Kconfig b/mm/Kconfig
index b1f7624276f8..3f7b6d7b69df 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -762,9 +762,6 @@ config ARCH_HAS_CURRENT_STACK_POINTER
 	  register alias named "current_stack_pointer", this config can be
 	  selected.
 
-config ARCH_HAS_FILTER_PGPROT
-	bool
-
 config ARCH_HAS_VM_GET_PAGE_PROT
 	bool
 
diff --git a/mm/mmap.c b/mm/mmap.c
index 87cb2eaf7e1a..edf2a5e38f4d 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -107,20 +107,13 @@ pgprot_t protection_map[16] __ro_after_init = {
 };
 
 #ifndef CONFIG_ARCH_HAS_VM_GET_PAGE_PROT
-#ifndef CONFIG_ARCH_HAS_FILTER_PGPROT
-static inline pgprot_t arch_filter_pgprot(pgprot_t prot)
-{
-	return prot;
-}
-#endif
-
 pgprot_t vm_get_page_prot(unsigned long vm_flags)
 {
 	pgprot_t ret = __pgprot(pgprot_val(protection_map[vm_flags &
 				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]) |
 			pgprot_val(arch_vm_get_page_prot(vm_flags)));
 
-	return arch_filter_pgprot(ret);
+	return ret;
 }
 EXPORT_SYMBOL(vm_get_page_prot);
 #endif	/* CONFIG_ARCH_HAS_VM_GET_PAGE_PROT */
-- 
2.25.1

Re: [PATCH V4 6/7] mm/mmap: Drop arch_filter_pgprot()

[Catalin Marinas]

On Thu, Apr 07, 2022 at 04:02:50PM +0530, Anshuman Khandual wrote:
> There are no platforms left which subscribe ARCH_HAS_FILTER_PGPROT. Hence
> drop generic arch_filter_pgprot() and also config ARCH_HAS_FILTER_PGPROT.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

[PATCH V4 7/7] mm/mmap: Drop arch_vm_get_page_pgprot()

[Anshuman Khandual]

There are no platforms left which use arch_vm_get_page_prot(). Just drop
generic arch_vm_get_page_prot().

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 include/linux/mman.h | 4 ----
 mm/mmap.c            | 3 +--
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/include/linux/mman.h b/include/linux/mman.h
index b66e91b8176c..58b3abd457a3 100644
--- a/include/linux/mman.h
+++ b/include/linux/mman.h
@@ -93,10 +93,6 @@ static inline void vm_unacct_memory(long pages)
 #define arch_calc_vm_flag_bits(flags) 0
 #endif
 
-#ifndef arch_vm_get_page_prot
-#define arch_vm_get_page_prot(vm_flags) __pgprot(0)
-#endif
-
 #ifndef arch_validate_prot
 /*
  * This is called from mprotect().  PROT_GROWSDOWN and PROT_GROWSUP have
diff --git a/mm/mmap.c b/mm/mmap.c
index edf2a5e38f4d..db7f33154206 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -110,8 +110,7 @@ pgprot_t protection_map[16] __ro_after_init = {
 pgprot_t vm_get_page_prot(unsigned long vm_flags)
 {
 	pgprot_t ret = __pgprot(pgprot_val(protection_map[vm_flags &
-				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]) |
-			pgprot_val(arch_vm_get_page_prot(vm_flags)));
+				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]));
 
 	return ret;
 }
-- 
2.25.1

Re: [PATCH V4 7/7] mm/mmap: Drop arch_vm_get_page_pgprot()

[Catalin Marinas]

On Thu, Apr 07, 2022 at 04:02:51PM +0530, Anshuman Khandual wrote:
> There are no platforms left which use arch_vm_get_page_prot(). Just drop
> generic arch_vm_get_page_prot().
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

Re: [PATCH V4 0/7] mm/mmap: Drop arch_vm_get_page_prot() and arch_filter_pgprot()

[Andrew Morton]

On Thu,  7 Apr 2022 16:02:44 +0530 Anshuman Khandual <anshuman.khandual@arm.com> wrote:

> protection_map[] is an array based construct that translates given vm_flags
> combination. This array contains page protection map, which is populated by
> the platform via [__S000 .. __S111] and [__P000 .. __P111] exported macros.
> Primary usage for protection_map[] is for vm_get_page_prot(), which is used
> to determine page protection value for a given vm_flags. vm_get_page_prot()
> implementation, could again call platform overrides arch_vm_get_page_prot()
> and arch_filter_pgprot(). Some platforms override protection_map[] that was
> originally built with __SXXX/__PXXX with different runtime values.
> 
> Currently there are multiple layers of abstraction i.e __SXXX/__PXXX macros
> , protection_map[], arch_vm_get_page_prot() and arch_filter_pgprot() built
> between the platform and generic MM, finally defining vm_get_page_prot().
> 
> Hence this series proposes to drop later two abstraction levels and instead
> just move the responsibility of defining vm_get_page_prot() to the platform
> (still utilizing generic protection_map[] array) itself making it clean and
> simple.
> 
> This first introduces ARCH_HAS_VM_GET_PAGE_PROT which enables the platforms
> to define custom vm_get_page_prot(). This starts converting platforms that
> define the overrides arch_filter_pgprot() or arch_vm_get_page_prot() which
> enables for those constructs to be dropped off completely.
> 
> The series has been inspired from an earlier discuss with Christoph Hellwig
> 
> https://lore.kernel.org/all/1632712920-8171-1-git-send-email-anshuman.khandual@arm.com/
> 
> This series applies on 5.18-rc1 after the following patch.
> 
> https://lore.kernel.org/all/1643004823-16441-1-git-send-email-anshuman.khandual@arm.com/

Confusing.  That patch is already in 5.18-rc1.

But the version which was merged (24e988c7fd1ee701e) lacked the change
to arch/arm64/Kconfig.  I seem to recall that this patch went through a
few issues and perhaps the arm64 change was dropped.  Can you please
check?

(It would be easier for me to track all this down if the original patch
had had cc:linux-mm.  Please cc linux-mm!)

Re: [PATCH V4 0/7] mm/mmap: Drop arch_vm_get_page_prot() and arch_filter_pgprot()

[Anshuman Khandual]

On 4/8/22 04:50, Andrew Morton wrote:
> On Thu,  7 Apr 2022 16:02:44 +0530 Anshuman Khandual <anshuman.khandual@arm.com> wrote:
> 
>> protection_map[] is an array based construct that translates given vm_flags
>> combination. This array contains page protection map, which is populated by
>> the platform via [__S000 .. __S111] and [__P000 .. __P111] exported macros.
>> Primary usage for protection_map[] is for vm_get_page_prot(), which is used
>> to determine page protection value for a given vm_flags. vm_get_page_prot()
>> implementation, could again call platform overrides arch_vm_get_page_prot()
>> and arch_filter_pgprot(). Some platforms override protection_map[] that was
>> originally built with __SXXX/__PXXX with different runtime values.
>>
>> Currently there are multiple layers of abstraction i.e __SXXX/__PXXX macros
>> , protection_map[], arch_vm_get_page_prot() and arch_filter_pgprot() built
>> between the platform and generic MM, finally defining vm_get_page_prot().
>>
>> Hence this series proposes to drop later two abstraction levels and instead
>> just move the responsibility of defining vm_get_page_prot() to the platform
>> (still utilizing generic protection_map[] array) itself making it clean and
>> simple.
>>
>> This first introduces ARCH_HAS_VM_GET_PAGE_PROT which enables the platforms
>> to define custom vm_get_page_prot(). This starts converting platforms that
>> define the overrides arch_filter_pgprot() or arch_vm_get_page_prot() which
>> enables for those constructs to be dropped off completely.
>>
>> The series has been inspired from an earlier discuss with Christoph Hellwig
>>
>> https://lore.kernel.org/all/1632712920-8171-1-git-send-email-anshuman.khandual@arm.com/
>>
>> This series applies on 5.18-rc1 after the following patch.
>>
>> https://lore.kernel.org/all/1643004823-16441-1-git-send-email-anshuman.khandual@arm.com/
> 
> Confusing.  That patch is already in 5.18-rc1.
Ahh, my bad, forgot to delete these lines here in the cover letter.
This series just applies cleanly on 5.18-rc1 without dependency.

> But the version which was merged (24e988c7fd1ee701e) lacked the change
> to arch/arm64/Kconfig.  I seem to recall that this patch went through a
> few issues and perhaps the arm64 change was dropped.  Can you please
> check?

ARCH_HAS_FILTER_PGPROT on arm64 got dropped off via another commit i.e
6e2edd6371a4 ("arm64: Ensure execute-only permissions are not allowed
without EPAN").

> 
> (It would be easier for me to track all this down if the original patch
> had had cc:linux-mm.  Please cc linux-mm!
Sure, will do. Please do let me know if there is anything else that needs
to be taken care.