Re: [PATCH v2 1/2] powerpc/mm: Move vm_get_page_prot() out of book3s64 code

[Christophe Leroy]

Le 09/08/2022 à 02:55, Russell Currey a écrit :
> On Mon, 2022-08-08 at 14:32 +, 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 
#else
#error "Not supported"
#endif

Christophe

Re: [PATCH v2 1/2] powerpc/mm: Move vm_get_page_prot() out of book3s64 code

[Russell Currey]

On Mon, 2022-08-08 at 14:32 +, 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 
> > ---
> > 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 
> >   #include 
> >   #include 
> > +#include 
> >   
> >   #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 */

Re: [PATCH v2 1/2] powerpc/mm: Move vm_get_page_prot() out of book3s64 code

[Christophe Leroy]

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 
> ---
> 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 
>   #include 
>   #include 
> +#include 
>   
>   #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 */

[PATCH v2 1/2] powerpc/mm: Move vm_get_page_prot() out of book3s64 code

[Russell Currey]

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 
---
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 
 #include 
 #include 
+#include 
 
 #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 */
-- 
2.37.1