Re: [PATCH RESEND] powerpc: fix usage of _PAGE_RO in hugepage

[Aneesh Kumar K.V]

Christophe Leroy  writes:

> On some CPUs like the 8xx, _PAGE_RW hence _PAGE_WRITE is defined
> as 0 and _PAGE_RO has to be set when a page is not writable
>
> _PAGE_RO is defined by default in pte-common.h, however BOOK3S/64
> doesn't include that file so _PAGE_RO has to be defined explicitly
> in book3s/64/pgtable.h
>
> fixes: a7b9f671f2d14 ("powerpc32: adds handling of _PAGE_RO")
> Signed-off-by: Christophe Leroy 
> ---
> This patch was initially part of the v1 serie of patchs for providing
> hugepage support to the 8xx. As suggested by Aneesh, that serie has
> been splited to focus only on hugepage implementation for 8xx.
> This patch is a fix and is independant of 8xx hugepage implementation,
> allthough it is required to have hugepage support working properly on
> the 8xx.
>
>  arch/powerpc/include/asm/book3s/64/pgtable.h | 2 ++
>  arch/powerpc/mm/hugetlbpage.c| 2 ++
>  2 files changed, 4 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h 
> b/arch/powerpc/include/asm/book3s/64/pgtable.h
> index 8ec8be9..9fd77f8 100644
> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
> @@ -6,6 +6,8 @@
>   */
>  #define _PAGE_BIT_SWAP_TYPE  0
>  
> +#define _PAGE_RO 0
> +
>  #define _PAGE_EXEC   0x1 /* execute permission */
>  #define _PAGE_WRITE  0x2 /* write access allowed */
>  #define _PAGE_READ   0x4 /* read access allowed */
> diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
> index 7372ee1..8a512b1 100644
> --- a/arch/powerpc/mm/hugetlbpage.c
> +++ b/arch/powerpc/mm/hugetlbpage.c
> @@ -1021,6 +1021,8 @@ int gup_hugepte(pte_t *ptep, unsigned long sz, unsigned 
> long addr,
>   mask = _PAGE_PRESENT | _PAGE_READ;
>   if (write)
>   mask |= _PAGE_WRITE;
> + else
> + mask |= _PAGE_RO;
>  
>   if ((pte_val(pte) & mask) != mask)
>   return 0;
> -- 

Can you add the commit message also as code comment ?. It is confusing
to find that we do mask = _PAGE_READ and then mask |= _PAGE_RO.

Otherwise

Reviewed-by: Aneesh Kumar K.V 

-aneesh

[PATCH RESEND] powerpc: fix usage of _PAGE_RO in hugepage

[Christophe Leroy]

On some CPUs like the 8xx, _PAGE_RW hence _PAGE_WRITE is defined
as 0 and _PAGE_RO has to be set when a page is not writable

_PAGE_RO is defined by default in pte-common.h, however BOOK3S/64
doesn't include that file so _PAGE_RO has to be defined explicitly
in book3s/64/pgtable.h

fixes: a7b9f671f2d14 ("powerpc32: adds handling of _PAGE_RO")
Signed-off-by: Christophe Leroy 
---
This patch was initially part of the v1 serie of patchs for providing
hugepage support to the 8xx. As suggested by Aneesh, that serie has
been splited to focus only on hugepage implementation for 8xx.
This patch is a fix and is independant of 8xx hugepage implementation,
allthough it is required to have hugepage support working properly on
the 8xx.

 arch/powerpc/include/asm/book3s/64/pgtable.h | 2 ++
 arch/powerpc/mm/hugetlbpage.c| 2 ++
 2 files changed, 4 insertions(+)

diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h 
b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 8ec8be9..9fd77f8 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -6,6 +6,8 @@
  */
 #define _PAGE_BIT_SWAP_TYPE0
 
+#define _PAGE_RO   0
+
 #define _PAGE_EXEC 0x1 /* execute permission */
 #define _PAGE_WRITE0x2 /* write access allowed */
 #define _PAGE_READ 0x4 /* read access allowed */
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index 7372ee1..8a512b1 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -1021,6 +1021,8 @@ int gup_hugepte(pte_t *ptep, unsigned long sz, unsigned 
long addr,
mask = _PAGE_PRESENT | _PAGE_READ;
if (write)
mask |= _PAGE_WRITE;
+   else
+   mask |= _PAGE_RO;
 
if ((pte_val(pte) & mask) != mask)
return 0;
-- 
2.1.0