| Message ID | 3656d47c53bff577739dac536dbae31fff52f6d8.1708078640.git.christophe.leroy@csgroup.eu (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 3c8016e681c5e0f5f3ad15edb4569727cd32eaff |
| Headers | show |
| Series | [1/2] powerpc: Refactor __kernel_map_pages() | expand |
Christophe Leroy <christophe.leroy@csgroup.eu> writes: > __kernel_map_pages() is almost identical for PPC32 and RADIX. > > Refactor it. > > On PPC32 it is not needed for KFENCE, but to keep it simple > just make it similar to PPC64. > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > arch/powerpc/include/asm/book3s/64/pgtable.h | 10 ---------- > arch/powerpc/include/asm/book3s/64/radix.h | 2 -- > arch/powerpc/mm/book3s64/radix_pgtable.c | 14 -------------- > arch/powerpc/mm/pageattr.c | 19 +++++++++++++++++++ > arch/powerpc/mm/pgtable_32.c | 15 --------------- > 5 files changed, 19 insertions(+), 41 deletions(-) > > diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c > index 421db7c4f2a4..16b8d20d6ca8 100644 > --- a/arch/powerpc/mm/pageattr.c > +++ b/arch/powerpc/mm/pageattr.c > @@ -101,3 +101,22 @@ int change_memory_attr(unsigned long addr, int numpages, long action) > return apply_to_existing_page_range(&init_mm, start, size, > change_page_attr, (void *)action); > } > + > +#if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_KFENCE) > +#ifdef CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC > +void __kernel_map_pages(struct page *page, int numpages, int enable) > +{ > + unsigned long addr = (unsigned long)page_address(page); > + > + if (PageHighMem(page)) > + return; > + > + if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && !radix_enabled()) > + hash__kernel_map_pages(page, numpages, enable); > + else if (enable) > + set_memory_p(addr, numpages); > + else > + set_memory_np(addr, numpages); > +} This doesn't build on 32-bit, eg. ppc32_allmodconfig: ../arch/powerpc/mm/pageattr.c: In function '__kernel_map_pages': ../arch/powerpc/mm/pageattr.c:116:23: error: implicit declaration of function 'hash__kernel_map_pages' [-Werror=implicit-function-declaration] 116 | err = hash__kernel_map_pages(page, numpages, enable); | ^~~~~~~~~~~~~~~~~~~~~~ I couldn't see a nice way to get around it, so ended up with: void __kernel_map_pages(struct page *page, int numpages, int enable) { int err; unsigned long addr = (unsigned long)page_address(page); if (PageHighMem(page)) return; #ifdef CONFIG_PPC_BOOK3S_64 if (!radix_enabled()) err = hash__kernel_map_pages(page, numpages, enable); else #endif if (enable) err = set_memory_p(addr, numpages); else err = set_memory_np(addr, numpages); cheers
Le 22/02/2024 à 06:32, Michael Ellerman a écrit : > Christophe Leroy <christophe.leroy@csgroup.eu> writes: >> __kernel_map_pages() is almost identical for PPC32 and RADIX. >> >> Refactor it. >> >> On PPC32 it is not needed for KFENCE, but to keep it simple >> just make it similar to PPC64. >> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> --- >> arch/powerpc/include/asm/book3s/64/pgtable.h | 10 ---------- >> arch/powerpc/include/asm/book3s/64/radix.h | 2 -- >> arch/powerpc/mm/book3s64/radix_pgtable.c | 14 -------------- >> arch/powerpc/mm/pageattr.c | 19 +++++++++++++++++++ >> arch/powerpc/mm/pgtable_32.c | 15 --------------- >> 5 files changed, 19 insertions(+), 41 deletions(-) >> >> diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c >> index 421db7c4f2a4..16b8d20d6ca8 100644 >> --- a/arch/powerpc/mm/pageattr.c >> +++ b/arch/powerpc/mm/pageattr.c >> @@ -101,3 +101,22 @@ int change_memory_attr(unsigned long addr, int numpages, long action) >> return apply_to_existing_page_range(&init_mm, start, size, >> change_page_attr, (void *)action); >> } >> + >> +#if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_KFENCE) >> +#ifdef CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC >> +void __kernel_map_pages(struct page *page, int numpages, int enable) >> +{ >> + unsigned long addr = (unsigned long)page_address(page); >> + >> + if (PageHighMem(page)) >> + return; >> + >> + if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && !radix_enabled()) >> + hash__kernel_map_pages(page, numpages, enable); >> + else if (enable) >> + set_memory_p(addr, numpages); >> + else >> + set_memory_np(addr, numpages); >> +} > > This doesn't build on 32-bit, eg. ppc32_allmodconfig: > > ../arch/powerpc/mm/pageattr.c: In function '__kernel_map_pages': > ../arch/powerpc/mm/pageattr.c:116:23: error: implicit declaration of function 'hash__kernel_map_pages' [-Werror=implicit-function-declaration] > 116 | err = hash__kernel_map_pages(page, numpages, enable); > | ^~~~~~~~~~~~~~~~~~~~~~ > > I couldn't see a nice way to get around it, so ended up with: > > void __kernel_map_pages(struct page *page, int numpages, int enable) > { > int err; > unsigned long addr = (unsigned long)page_address(page); > > if (PageHighMem(page)) > return; > > #ifdef CONFIG_PPC_BOOK3S_64 > if (!radix_enabled()) > err = hash__kernel_map_pages(page, numpages, enable); > else > #endif > if (enable) > err = set_memory_p(addr, numpages); > else > err = set_memory_np(addr, numpages); > I missed something it seems. Not good to leave something unterminated when you leave for vacation and think it was finished when you come back. The best solution I see is to move hash__kernel_map_pages() prototype somewhere else. $ git grep -e hash__ -e radix__ -- arch/powerpc/include/asm/*.h arch/powerpc/include/asm/bug.h:void hash__do_page_fault(struct pt_regs *); arch/powerpc/include/asm/mmu.h:extern void radix__mmu_cleanup_all(void); arch/powerpc/include/asm/mmu_context.h:extern void radix__switch_mmu_context(struct mm_struct *prev, arch/powerpc/include/asm/mmu_context.h: return radix__switch_mmu_context(prev, next); arch/powerpc/include/asm/mmu_context.h:extern int hash__alloc_context_id(void); arch/powerpc/include/asm/mmu_context.h:void __init hash__reserve_context_id(int id); arch/powerpc/include/asm/mmu_context.h: context_id = hash__alloc_context_id(); arch/powerpc/include/asm/mmu_context.h: * radix__flush_all_mm() to determine the scope (local/global) arch/powerpc/include/asm/mmu_context.h: radix__flush_all_mm(mm); Maybe asm/mmu.h ? Or mm/mmu_decl.h ? Christophe
Christophe Leroy <christophe.leroy@csgroup.eu> writes: > Le 22/02/2024 à 06:32, Michael Ellerman a écrit : >> Christophe Leroy <christophe.leroy@csgroup.eu> writes: >>> __kernel_map_pages() is almost identical for PPC32 and RADIX. >>> >>> Refactor it. >>> >>> On PPC32 it is not needed for KFENCE, but to keep it simple >>> just make it similar to PPC64. >>> >>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >>> --- >>> arch/powerpc/include/asm/book3s/64/pgtable.h | 10 ---------- >>> arch/powerpc/include/asm/book3s/64/radix.h | 2 -- >>> arch/powerpc/mm/book3s64/radix_pgtable.c | 14 -------------- >>> arch/powerpc/mm/pageattr.c | 19 +++++++++++++++++++ >>> arch/powerpc/mm/pgtable_32.c | 15 --------------- >>> 5 files changed, 19 insertions(+), 41 deletions(-) >>> >>> diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c >>> index 421db7c4f2a4..16b8d20d6ca8 100644 >>> --- a/arch/powerpc/mm/pageattr.c >>> +++ b/arch/powerpc/mm/pageattr.c >>> @@ -101,3 +101,22 @@ int change_memory_attr(unsigned long addr, int numpages, long action) >>> return apply_to_existing_page_range(&init_mm, start, size, >>> change_page_attr, (void *)action); >>> } >>> + >>> +#if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_KFENCE) >>> +#ifdef CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC >>> +void __kernel_map_pages(struct page *page, int numpages, int enable) >>> +{ >>> + unsigned long addr = (unsigned long)page_address(page); >>> + >>> + if (PageHighMem(page)) >>> + return; >>> + >>> + if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && !radix_enabled()) >>> + hash__kernel_map_pages(page, numpages, enable); >>> + else if (enable) >>> + set_memory_p(addr, numpages); >>> + else >>> + set_memory_np(addr, numpages); >>> +} >> >> This doesn't build on 32-bit, eg. ppc32_allmodconfig: >> >> ../arch/powerpc/mm/pageattr.c: In function '__kernel_map_pages': >> ../arch/powerpc/mm/pageattr.c:116:23: error: implicit declaration of function 'hash__kernel_map_pages' [-Werror=implicit-function-declaration] >> 116 | err = hash__kernel_map_pages(page, numpages, enable); >> | ^~~~~~~~~~~~~~~~~~~~~~ >> >> I couldn't see a nice way to get around it, so ended up with: >> >> void __kernel_map_pages(struct page *page, int numpages, int enable) >> { >> int err; >> unsigned long addr = (unsigned long)page_address(page); >> >> if (PageHighMem(page)) >> return; >> >> #ifdef CONFIG_PPC_BOOK3S_64 >> if (!radix_enabled()) >> err = hash__kernel_map_pages(page, numpages, enable); >> else >> #endif >> if (enable) >> err = set_memory_p(addr, numpages); >> else >> err = set_memory_np(addr, numpages); >> > > > I missed something it seems. Not good to leave something unterminated > when you leave for vacation and think it was finished when you come back. > > The best solution I see is to move hash__kernel_map_pages() prototype > somewhere else. > $ git grep -e hash__ -e radix__ -- arch/powerpc/include/asm/*.h > arch/powerpc/include/asm/bug.h:void hash__do_page_fault(struct pt_regs *); > arch/powerpc/include/asm/mmu.h:extern void radix__mmu_cleanup_all(void); > arch/powerpc/include/asm/mmu_context.h:extern void radix__switch_mmu_context(struct mm_struct *prev, > arch/powerpc/include/asm/mmu_context.h: return radix__switch_mmu_context(prev, next); > arch/powerpc/include/asm/mmu_context.h:extern int hash__alloc_context_id(void); > arch/powerpc/include/asm/mmu_context.h:void __init hash__reserve_context_id(int id); > arch/powerpc/include/asm/mmu_context.h: context_id = hash__alloc_context_id(); > arch/powerpc/include/asm/mmu_context.h: * radix__flush_all_mm() to determine the scope (local/global) > arch/powerpc/include/asm/mmu_context.h: radix__flush_all_mm(mm); If anything I'd prefer to move those out of there into the book3s/64/ headers :) > Maybe asm/mmu.h ? > > Or mm/mmu_decl.h ? Yeah I'll do that. It's a bit of a dumping ground, but at least it's internal to arch code, not exported to the rest of the kernel. cheers
On Fri, 16 Feb 2024 11:17:33 +0100, Christophe Leroy wrote: > __kernel_map_pages() is almost identical for PPC32 and RADIX. > > Refactor it. > > On PPC32 it is not needed for KFENCE, but to keep it simple > just make it similar to PPC64. > > [...] Applied to powerpc/next. [1/2] powerpc: Refactor __kernel_map_pages() https://git.kernel.org/powerpc/c/3c8016e681c5e0f5f3ad15edb4569727cd32eaff [2/2] powerpc: Don't ignore errors from set_memory_{n}p() in __kernel_map_pages() https://git.kernel.org/powerpc/c/9cbacb834b4afcb55eb8ac5115fa82fc7ede5c83 cheers
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h index 927d585652bc..62c43d3d80ec 100644 --- a/arch/powerpc/include/asm/book3s/64/pgtable.h +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h @@ -1027,16 +1027,6 @@ static inline void vmemmap_remove_mapping(unsigned long start, } #endif -#if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_KFENCE) -static inline void __kernel_map_pages(struct page *page, int numpages, int enable) -{ - if (radix_enabled()) - radix__kernel_map_pages(page, numpages, enable); - else - hash__kernel_map_pages(page, numpages, enable); -} -#endif - static inline pte_t pmd_pte(pmd_t pmd) { return __pte_raw(pmd_raw(pmd)); diff --git a/arch/powerpc/include/asm/book3s/64/radix.h b/arch/powerpc/include/asm/book3s/64/radix.h index 357e23a403d3..8f55ff74bb68 100644 --- a/arch/powerpc/include/asm/book3s/64/radix.h +++ b/arch/powerpc/include/asm/book3s/64/radix.h @@ -362,8 +362,6 @@ int radix__create_section_mapping(unsigned long start, unsigned long end, int radix__remove_section_mapping(unsigned long start, unsigned long end); #endif /* CONFIG_MEMORY_HOTPLUG */ -void radix__kernel_map_pages(struct page *page, int numpages, int enable); - #ifdef CONFIG_ARCH_WANT_OPTIMIZE_DAX_VMEMMAP #define vmemmap_can_optimize vmemmap_can_optimize bool vmemmap_can_optimize(struct vmem_altmap *altmap, struct dev_pagemap *pgmap); diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c index c6a4ac766b2b..e16e2fd104c5 100644 --- a/arch/powerpc/mm/book3s64/radix_pgtable.c +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c @@ -1339,20 +1339,6 @@ void __ref radix__vmemmap_free(unsigned long start, unsigned long end, #endif #endif -#if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_KFENCE) -void radix__kernel_map_pages(struct page *page, int numpages, int enable) -{ - unsigned long addr; - - addr = (unsigned long)page_address(page); - - if (enable) - set_memory_p(addr, numpages); - else - set_memory_np(addr, numpages); -} -#endif - #ifdef CONFIG_TRANSPARENT_HUGEPAGE unsigned long radix__pmd_hugepage_update(struct mm_struct *mm, unsigned long addr, diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c index 421db7c4f2a4..16b8d20d6ca8 100644 --- a/arch/powerpc/mm/pageattr.c +++ b/arch/powerpc/mm/pageattr.c @@ -101,3 +101,22 @@ int change_memory_attr(unsigned long addr, int numpages, long action) return apply_to_existing_page_range(&init_mm, start, size, change_page_attr, (void *)action); } + +#if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_KFENCE) +#ifdef CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC +void __kernel_map_pages(struct page *page, int numpages, int enable) +{ + unsigned long addr = (unsigned long)page_address(page); + + if (PageHighMem(page)) + return; + + if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && !radix_enabled()) + hash__kernel_map_pages(page, numpages, enable); + else if (enable) + set_memory_p(addr, numpages); + else + set_memory_np(addr, numpages); +} +#endif +#endif diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c index e94919853ca3..4be97b4a44f9 100644 --- a/arch/powerpc/mm/pgtable_32.c +++ b/arch/powerpc/mm/pgtable_32.c @@ -188,18 +188,3 @@ void mark_rodata_ro(void) ptdump_check_wx(); } #endif - -#if defined(CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC) && defined(CONFIG_DEBUG_PAGEALLOC) -void __kernel_map_pages(struct page *page, int numpages, int enable) -{ - unsigned long addr = (unsigned long)page_address(page); - - if (PageHighMem(page)) - return; - - if (enable) - set_memory_p(addr, numpages); - else - set_memory_np(addr, numpages); -} -#endif /* CONFIG_DEBUG_PAGEALLOC */
__kernel_map_pages() is almost identical for PPC32 and RADIX. Refactor it. On PPC32 it is not needed for KFENCE, but to keep it simple just make it similar to PPC64. Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> --- arch/powerpc/include/asm/book3s/64/pgtable.h | 10 ---------- arch/powerpc/include/asm/book3s/64/radix.h | 2 -- arch/powerpc/mm/book3s64/radix_pgtable.c | 14 -------------- arch/powerpc/mm/pageattr.c | 19 +++++++++++++++++++ arch/powerpc/mm/pgtable_32.c | 15 --------------- 5 files changed, 19 insertions(+), 41 deletions(-)