| Message ID | 1643029028-12710-1-git-send-email-anshuman.khandual@arm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | mm/mmap: Drop protection_map[] and platform's __SXXX/__PXXX requirements | expand |
Hi Anshuman, On Mon, Jan 24, 2022 at 06:26:37PM +0530, Anshuman Khandual 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 all these abstraction levels and instead > just move the responsibility of defining vm_get_page_prot() to the platform > 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 > either change protection_map[] or define the overrides arch_filter_pgprot() > or arch_vm_get_page_prot() which enables for those constructs to be dropped > off completely. This series then converts remaining platforms which enables > for __SXXX/__PXXX constructs to be dropped off completely. Finally it drops > the generic vm_get_page_prot() and then ARCH_HAS_VM_GET_PAGE_PROT as every > platform now defines their own vm_get_page_prot(). I generally like the idea, I just think the conversion can be more straight forward. Rather than adding ARCH_HAS_VM_GET_PAGE_PROT and then dropping it, why won't me make the generic vm_get_page_prot() __weak, then add per-arch implementation and in the end drop the generic one? > The last patch demonstrates how vm_flags combination indices can be defined > as macros and be replaces across all platforms (if required, not done yet). > > 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.17-rc1 after the following patch. > > https://lore.kernel.org/all/1643004823-16441-1-git-send-email-anshuman.khandual@arm.com/ > > This has been cross built for multiple platforms. I would like to get some > early feed back on this proposal. All reviews and suggestions welcome. > > Hello Christoph, > > I have taken the liberty to preserve your authorship on the x86 patch which > is borrowed almost as is from our earlier discussion. I have also added you > as 'Suggested-by:' on the patch that adds config ARCH_HAS_VM_GET_PAGE_PROT. > Nonetheless please feel free to correct me for any other missing authorship > attributes I should have added. Thank you. > > - Anshuman > > Cc: Christoph Hellwig <hch@infradead.org> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: linux-mm@kvack.org > Cc: linux-kernel@vger.kernel.org > > Anshuman Khandual (30): > mm/debug_vm_pgtable: Directly use vm_get_page_prot() > mm/mmap: Clarify protection_map[] indices > 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 > mips/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT > m68k/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT > arm/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT > mm/mmap: Drop protection_map[] > mm/mmap: Drop arch_filter_pgprot() > mm/mmap: Drop arch_vm_get_page_pgprot() > s390/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT > riscv/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT > alpha/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT > sh/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT > arc/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT > csky/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT > extensa/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT > parisc/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT > openrisc/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT > um/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT > microblaze/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT > nios2/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT > hexagon/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT > nds32/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT > ia64/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT > mm/mmap: Drop generic vm_get_page_prot() > mm/mmap: Drop ARCH_HAS_VM_GET_PAGE_PROT > mm/mmap: Define macros for vm_flags access permission combinations > > Christoph Hellwig (1): > x86/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT > > arch/alpha/include/asm/pgtable.h | 17 ----- > arch/alpha/mm/init.c | 41 +++++++++++ > arch/arc/include/asm/pgtable-bits-arcv2.h | 17 ----- > arch/arc/mm/mmap.c | 41 +++++++++++ > arch/arm/include/asm/pgtable.h | 18 ----- > arch/arm/mm/mmu.c | 50 +++++++++++-- > arch/arm64/Kconfig | 1 - > arch/arm64/include/asm/mman.h | 3 +- > arch/arm64/include/asm/pgtable-prot.h | 18 ----- > arch/arm64/include/asm/pgtable.h | 2 +- > arch/arm64/mm/mmap.c | 50 +++++++++++++ > arch/csky/include/asm/pgtable.h | 18 ----- > arch/csky/mm/init.c | 41 +++++++++++ > arch/hexagon/include/asm/pgtable.h | 24 ------- > arch/hexagon/mm/init.c | 42 +++++++++++ > arch/ia64/include/asm/pgtable.h | 17 ----- > arch/ia64/mm/init.c | 43 ++++++++++- > arch/m68k/include/asm/mcf_pgtable.h | 59 --------------- > arch/m68k/include/asm/motorola_pgtable.h | 22 ------ > arch/m68k/include/asm/sun3_pgtable.h | 22 ------ > arch/m68k/mm/init.c | 87 +++++++++++++++++++++++ > arch/m68k/mm/motorola.c | 44 +++++++++++- > arch/microblaze/include/asm/pgtable.h | 17 ----- > arch/microblaze/mm/init.c | 41 +++++++++++ > arch/mips/include/asm/pgtable.h | 22 ------ > arch/mips/mm/cache.c | 65 ++++++++++------- > arch/nds32/include/asm/pgtable.h | 17 ----- > arch/nds32/mm/mmap.c | 41 +++++++++++ > arch/nios2/include/asm/pgtable.h | 16 ----- > arch/nios2/mm/init.c | 41 +++++++++++ > arch/openrisc/include/asm/pgtable.h | 18 ----- > arch/openrisc/mm/init.c | 41 +++++++++++ > arch/parisc/include/asm/pgtable.h | 20 ------ > arch/parisc/mm/init.c | 41 +++++++++++ > arch/powerpc/include/asm/mman.h | 3 +- > arch/powerpc/include/asm/pgtable.h | 19 ----- > arch/powerpc/mm/mmap.c | 47 ++++++++++++ > arch/riscv/include/asm/pgtable.h | 16 ----- > arch/riscv/mm/init.c | 41 +++++++++++ > arch/s390/include/asm/pgtable.h | 17 ----- > arch/s390/mm/mmap.c | 41 +++++++++++ > arch/sh/include/asm/pgtable.h | 17 ----- > arch/sh/mm/mmap.c | 43 +++++++++++ > arch/sparc/include/asm/mman.h | 1 - > arch/sparc/include/asm/pgtable_32.h | 19 ----- > arch/sparc/include/asm/pgtable_64.h | 19 ----- > arch/sparc/mm/init_32.c | 41 +++++++++++ > arch/sparc/mm/init_64.c | 71 +++++++++++++----- > arch/um/include/asm/pgtable.h | 17 ----- > arch/um/kernel/mem.c | 41 +++++++++++ > arch/x86/Kconfig | 1 - > arch/x86/include/asm/pgtable.h | 5 -- > arch/x86/include/asm/pgtable_types.h | 19 ----- > arch/x86/include/uapi/asm/mman.h | 14 ---- > arch/x86/mm/Makefile | 2 +- > arch/x86/mm/mem_encrypt_amd.c | 4 -- > arch/x86/mm/pgprot.c | 71 ++++++++++++++++++ > arch/xtensa/include/asm/pgtable.h | 18 ----- > arch/xtensa/mm/init.c | 41 +++++++++++ > include/linux/mm.h | 45 ++++++++++-- > include/linux/mman.h | 4 -- > mm/Kconfig | 3 - > mm/debug_vm_pgtable.c | 27 +++---- > mm/mmap.c | 22 ------ > 64 files changed, 1150 insertions(+), 636 deletions(-) > create mode 100644 arch/x86/mm/pgprot.c > > -- > 2.25.1 > >
On 1/27/22 6:08 PM, Mike Rapoport wrote: > Hi Anshuman, > > On Mon, Jan 24, 2022 at 06:26:37PM +0530, Anshuman Khandual 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 all these abstraction levels and instead >> just move the responsibility of defining vm_get_page_prot() to the platform >> 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 >> either change protection_map[] or define the overrides arch_filter_pgprot() >> or arch_vm_get_page_prot() which enables for those constructs to be dropped >> off completely. This series then converts remaining platforms which enables >> for __SXXX/__PXXX constructs to be dropped off completely. Finally it drops >> the generic vm_get_page_prot() and then ARCH_HAS_VM_GET_PAGE_PROT as every >> platform now defines their own vm_get_page_prot(). > > I generally like the idea, I just think the conversion can be more straight > forward. Rather than adding ARCH_HAS_VM_GET_PAGE_PROT and then dropping it, > why won't me make the generic vm_get_page_prot() __weak, then add per-arch > implementation and in the end drop the generic one? > Is not the ARCH_HAS_ config based switch over a relatively better method ? IIUC some existing platform overrides could have been implemented via __weak identifier, although ARCH_HAS_ method was preferred. But I might be missing something here.