| Message ID | 20220808130109.30738-1-ruscur@russell.cc (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/2] powerpc/mm: Move vm_get_page_prot() out of book3s64 code | expand |
Le 08/08/2022 à 15:01, Russell Currey a écrit : > protection_map is about to be __ro_after_init instead of const, so move > the only non-local function that consumes it to the same file so it can > at least be static. What's the advantage of doing that ? Why does it need to be static ? Christophe > > Signed-off-by: Russell Currey <ruscur@russell.cc> > --- > v2: new > > arch/powerpc/mm/book3s64/pgtable.c | 16 ---------------- > arch/powerpc/mm/pgtable.c | 21 +++++++++++++++++++-- > 2 files changed, 19 insertions(+), 18 deletions(-) > > diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c > index 7b9966402b25..e2a4ea5eb960 100644 > --- a/arch/powerpc/mm/book3s64/pgtable.c > +++ b/arch/powerpc/mm/book3s64/pgtable.c > @@ -550,19 +550,3 @@ unsigned long memremap_compat_align(void) > } > EXPORT_SYMBOL_GPL(memremap_compat_align); > #endif > - > -pgprot_t vm_get_page_prot(unsigned long vm_flags) > -{ > - unsigned long prot = pgprot_val(protection_map[vm_flags & > - (VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]); > - > - if (vm_flags & VM_SAO) > - prot |= _PAGE_SAO; > - > -#ifdef CONFIG_PPC_MEM_KEYS > - prot |= vmflag_to_pte_pkey_bits(vm_flags); > -#endif > - > - return __pgprot(prot); > -} > -EXPORT_SYMBOL(vm_get_page_prot); > diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c > index cb2dcdb18f8e..0b2bbde5fb65 100644 > --- a/arch/powerpc/mm/pgtable.c > +++ b/arch/powerpc/mm/pgtable.c > @@ -27,6 +27,7 @@ > #include <asm/tlb.h> > #include <asm/hugetlb.h> > #include <asm/pte-walk.h> > +#include <asm/pkeys.h> > > #ifdef CONFIG_PPC64 > #define PGD_ALIGN (sizeof(pgd_t) * MAX_PTRS_PER_PGD) > @@ -493,6 +494,22 @@ const pgprot_t protection_map[16] = { > [VM_SHARED | VM_EXEC | VM_WRITE | VM_READ] = PAGE_SHARED_X > }; > > -#ifndef CONFIG_PPC_BOOK3S_64 > -DECLARE_VM_GET_PAGE_PROT > +#ifdef CONFIG_PPC_BOOK3S_64 > +pgprot_t vm_get_page_prot(unsigned long vm_flags) > +{ > + unsigned long prot = pgprot_val(protection_map[vm_flags & > + (VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]); > + > + if (vm_flags & VM_SAO) > + prot |= _PAGE_SAO; > + > +#ifdef CONFIG_PPC_MEM_KEYS > + prot |= vmflag_to_pte_pkey_bits(vm_flags); > #endif > + > + return __pgprot(prot); > +} > +EXPORT_SYMBOL(vm_get_page_prot); > +#else > +DECLARE_VM_GET_PAGE_PROT > +#endif /* CONFIG_PPC_BOOK3S_64 */
On Mon, 2022-08-08 at 14:32 +0000, Christophe Leroy wrote: > > > Le 08/08/2022 à 15:01, Russell Currey a écrit : > > protection_map is about to be __ro_after_init instead of const, so > > move > > the only non-local function that consumes it to the same file so it > > can > > at least be static. > > What's the advantage of doing that ? Why does it need to be static ? > > Christophe It doesn't need to be, I didn't like having it exposed unnecessarily. Aneesh's suggestion lets it stay const so I can drop this patch anyway. - Russell > > > > > Signed-off-by: Russell Currey <ruscur@russell.cc> > > --- > > v2: new > > > > arch/powerpc/mm/book3s64/pgtable.c | 16 ---------------- > > arch/powerpc/mm/pgtable.c | 21 +++++++++++++++++++-- > > 2 files changed, 19 insertions(+), 18 deletions(-) > > > > diff --git a/arch/powerpc/mm/book3s64/pgtable.c > > b/arch/powerpc/mm/book3s64/pgtable.c > > index 7b9966402b25..e2a4ea5eb960 100644 > > --- a/arch/powerpc/mm/book3s64/pgtable.c > > +++ b/arch/powerpc/mm/book3s64/pgtable.c > > @@ -550,19 +550,3 @@ unsigned long memremap_compat_align(void) > > } > > EXPORT_SYMBOL_GPL(memremap_compat_align); > > #endif > > - > > -pgprot_t vm_get_page_prot(unsigned long vm_flags) > > -{ > > - unsigned long prot = pgprot_val(protection_map[vm_flags & > > - > > (VM_READ|VM_WRITE|VM_EXEC|VM_ > > SHARED)]); > > - > > - if (vm_flags & VM_SAO) > > - prot |= _PAGE_SAO; > > - > > -#ifdef CONFIG_PPC_MEM_KEYS > > - prot |= vmflag_to_pte_pkey_bits(vm_flags); > > -#endif > > - > > - return __pgprot(prot); > > -} > > -EXPORT_SYMBOL(vm_get_page_prot); > > diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c > > index cb2dcdb18f8e..0b2bbde5fb65 100644 > > --- a/arch/powerpc/mm/pgtable.c > > +++ b/arch/powerpc/mm/pgtable.c > > @@ -27,6 +27,7 @@ > > #include <asm/tlb.h> > > #include <asm/hugetlb.h> > > #include <asm/pte-walk.h> > > +#include <asm/pkeys.h> > > > > #ifdef CONFIG_PPC64 > > #define PGD_ALIGN (sizeof(pgd_t) * MAX_PTRS_PER_PGD) > > @@ -493,6 +494,22 @@ const pgprot_t protection_map[16] = { > > [VM_SHARED | VM_EXEC | VM_WRITE | VM_READ] = > > PAGE_SHARED_X > > }; > > > > -#ifndef CONFIG_PPC_BOOK3S_64 > > -DECLARE_VM_GET_PAGE_PROT > > +#ifdef CONFIG_PPC_BOOK3S_64 > > +pgprot_t vm_get_page_prot(unsigned long vm_flags) > > +{ > > + unsigned long prot = pgprot_val(protection_map[vm_flags & > > + (VM_READ|VM_WRITE|VM_EXEC|V > > M_SHARED)]); > > + > > + if (vm_flags & VM_SAO) > > + prot |= _PAGE_SAO; > > + > > +#ifdef CONFIG_PPC_MEM_KEYS > > + prot |= vmflag_to_pte_pkey_bits(vm_flags); > > #endif > > + > > + return __pgprot(prot); > > +} > > +EXPORT_SYMBOL(vm_get_page_prot); > > +#else > > +DECLARE_VM_GET_PAGE_PROT > > +#endif /* CONFIG_PPC_BOOK3S_64 */
Le 09/08/2022 à 02:55, Russell Currey a écrit : > On Mon, 2022-08-08 at 14:32 +0000, Christophe Leroy wrote: >> >> >> Le 08/08/2022 à 15:01, Russell Currey a écrit : >>> protection_map is about to be __ro_after_init instead of const, so >>> move >>> the only non-local function that consumes it to the same file so it >>> can >>> at least be static. >> >> What's the advantage of doing that ? Why does it need to be static ? >> >> Christophe > > It doesn't need to be, I didn't like having it exposed unnecessarily. > Aneesh's suggestion lets it stay const so I can drop this patch anyway. Yes I think Aneesh's approach is better as it keeps book3s/64 specific stuff in dedicated file. Also as I probably saw from the robots, including asm/pkeys.h in a non boo3s64 file was a problem, due to the following in that file: #ifdef CONFIG_PPC_BOOK3S_64 #include <asm/book3s/64/pkeys.h> #else #error "Not supported" #endif Christophe
diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c index 7b9966402b25..e2a4ea5eb960 100644 --- a/arch/powerpc/mm/book3s64/pgtable.c +++ b/arch/powerpc/mm/book3s64/pgtable.c @@ -550,19 +550,3 @@ unsigned long memremap_compat_align(void) } EXPORT_SYMBOL_GPL(memremap_compat_align); #endif - -pgprot_t vm_get_page_prot(unsigned long vm_flags) -{ - unsigned long prot = pgprot_val(protection_map[vm_flags & - (VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]); - - if (vm_flags & VM_SAO) - prot |= _PAGE_SAO; - -#ifdef CONFIG_PPC_MEM_KEYS - prot |= vmflag_to_pte_pkey_bits(vm_flags); -#endif - - return __pgprot(prot); -} -EXPORT_SYMBOL(vm_get_page_prot); diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c index cb2dcdb18f8e..0b2bbde5fb65 100644 --- a/arch/powerpc/mm/pgtable.c +++ b/arch/powerpc/mm/pgtable.c @@ -27,6 +27,7 @@ #include <asm/tlb.h> #include <asm/hugetlb.h> #include <asm/pte-walk.h> +#include <asm/pkeys.h> #ifdef CONFIG_PPC64 #define PGD_ALIGN (sizeof(pgd_t) * MAX_PTRS_PER_PGD) @@ -493,6 +494,22 @@ const pgprot_t protection_map[16] = { [VM_SHARED | VM_EXEC | VM_WRITE | VM_READ] = PAGE_SHARED_X }; -#ifndef CONFIG_PPC_BOOK3S_64 -DECLARE_VM_GET_PAGE_PROT +#ifdef CONFIG_PPC_BOOK3S_64 +pgprot_t vm_get_page_prot(unsigned long vm_flags) +{ + unsigned long prot = pgprot_val(protection_map[vm_flags & + (VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]); + + if (vm_flags & VM_SAO) + prot |= _PAGE_SAO; + +#ifdef CONFIG_PPC_MEM_KEYS + prot |= vmflag_to_pte_pkey_bits(vm_flags); #endif + + return __pgprot(prot); +} +EXPORT_SYMBOL(vm_get_page_prot); +#else +DECLARE_VM_GET_PAGE_PROT +#endif /* CONFIG_PPC_BOOK3S_64 */
protection_map is about to be __ro_after_init instead of const, so move the only non-local function that consumes it to the same file so it can at least be static. Signed-off-by: Russell Currey <ruscur@russell.cc> --- v2: new arch/powerpc/mm/book3s64/pgtable.c | 16 ---------------- arch/powerpc/mm/pgtable.c | 21 +++++++++++++++++++-- 2 files changed, 19 insertions(+), 18 deletions(-)