Re: [PATCH V5 08/31] powerpc/mm: Drop pte-common.h from BOOK3S 64

[Anshuman Khandual <khandual@linux.vnet.ibm.com>]

On 11/23/2015 03:52 PM, Aneesh Kumar K.V wrote:
> We copy only needed PTE bits define from pte-common.h to respective
> hash related header. This should greatly simply later patches in which
                     
           Will it be simplify            ^^^^^^^^

> we are going to change the pte format for hash config
> 
> Acked-by: Scott Wood <scottwood@freescale.com>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/book3s/64/hash-4k.h |   1 +
>  arch/powerpc/include/asm/book3s/64/hash.h    |   2 +
>  arch/powerpc/include/asm/book3s/64/pgtable.h | 106 ++++++++++++++++++++++++++-
>  arch/powerpc/include/asm/book3s/pgtable.h    |  16 ++--
>  4 files changed, 113 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/book3s/64/hash-4k.h b/arch/powerpc/include/asm/book3s/64/hash-4k.h
> index f2c51cd61f69..15518b620f5a 100644
> --- a/arch/powerpc/include/asm/book3s/64/hash-4k.h
> +++ b/arch/powerpc/include/asm/book3s/64/hash-4k.h
> @@ -62,6 +62,7 @@
>  /* shift to put page number into pte */
>  #define PTE_RPN_SHIFT	(17)
>  
> +#define _PAGE_4K_PFN		0
>  #ifndef __ASSEMBLY__
>  /*
>   * 4-level page tables related bits
> diff --git a/arch/powerpc/include/asm/book3s/64/hash.h b/arch/powerpc/include/asm/book3s/64/hash.h
> index 8e60d4fa434d..7deb5063ff8c 100644
> --- a/arch/powerpc/include/asm/book3s/64/hash.h
> +++ b/arch/powerpc/include/asm/book3s/64/hash.h
> @@ -20,6 +20,7 @@
>  #define _PAGE_EXEC		0x0004 /* No execute on POWER4 and newer (we invert) */
>  #define _PAGE_GUARDED		0x0008
>  /* We can derive Memory coherence from _PAGE_NO_CACHE */
> +#define _PAGE_COHERENT		0x0
>  #define _PAGE_NO_CACHE		0x0020 /* I: cache inhibit */
>  #define _PAGE_WRITETHRU		0x0040 /* W: cache write-through */
>  #define _PAGE_DIRTY		0x0080 /* C: page changed */
> @@ -30,6 +31,7 @@
>  /* No separate kernel read-only */
>  #define _PAGE_KERNEL_RW		(_PAGE_RW | _PAGE_DIRTY) /* user access blocked by key */
>  #define _PAGE_KERNEL_RO		 _PAGE_KERNEL_RW
> +#define _PAGE_KERNEL_RWX	(_PAGE_DIRTY | _PAGE_RW | _PAGE_EXEC)
>  
>  /* Strong Access Ordering */
>  #define _PAGE_SAO		(_PAGE_WRITETHRU | _PAGE_NO_CACHE | _PAGE_COHERENT)
> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
> index ddc08bf22709..e41b9d47cc32 100644
> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
> @@ -94,11 +94,111 @@
>  #define _HPAGE_CHG_MASK (PTE_RPN_MASK | _PAGE_HPTEFLAGS |		\
>  			 _PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_SPLITTING | \
>  			 _PAGE_THP_HUGE)
> +#define _PTE_NONE_MASK	_PAGE_HPTEFLAGS
>  /*
> - * Default defines for things which we don't use.
> - * We should get this removed.
> + * The mask convered by the RPN must be a ULL on 32-bit platforms with
> + * 64-bit PTEs
> + * FIXME!! double check the RPN_MAX May be not used
>   */
> -#include <asm/pte-common.h>
> +//#define PTE_RPN_MAX	(1UL << (32 - PTE_RPN_SHIFT))

Why this commented definition, this was not part of original PTE
definitions in pte-common.h

>  /*
> diff --git a/arch/powerpc/include/asm/book3s/pgtable.h b/arch/powerpc/include/asm/book3s/pgtable.h
> index fa270cfcf30a..87333618af3b 100644
> --- a/arch/powerpc/include/asm/book3s/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/pgtable.h
> @@ -11,10 +11,7 @@
>  #ifndef __ASSEMBLY__
>  
>  /* Generic accessors to PTE bits */
> -static inline int pte_write(pte_t pte)
> -{
> -	return (pte_val(pte) & (_PAGE_RW | _PAGE_RO)) != _PAGE_RO;
> -}
> +static inline int pte_write(pte_t pte)		{ return !!(pte_val(pte) & _PAGE_RW);}


>  /* Generic modifiers for PTE bits */
> -static inline pte_t pte_wrprotect(pte_t pte) {
> -	pte_val(pte) &= ~(_PAGE_RW | _PAGE_HWWRITE);
> -	pte_val(pte) |= _PAGE_RO; return pte; }
> +static inline pte_t pte_wrprotect(pte_t pte)
> +{
> +	pte_val(pte) &= ~_PAGE_RW;
> +	return pte;
> +}
>  static inline pte_t pte_mkclean(pte_t pte) {
> -	pte_val(pte) &= ~(_PAGE_DIRTY | _PAGE_HWWRITE); return pte; }
> +	pte_val(pte) &= ~_PAGE_DIRTY; return pte; }


Should not these changes in function definitions happen in a separate patch.
The commit message here does not speak about these changes happening.