[PATCH v12 0/3] mm, x86, powerpc: Enhancements to Memory Protection Keys.

[PATCH v12 0/3] mm, x86, powerpc: Enhancements to Memory Protection Keys.

[Ram Pai]

This patch series provides arch-neutral enhancements to
enable memory-keys on new architecutes, and the corresponding
changes in x86 and powerpc specific code to support that.

a) Provides ability to support upto 32 keys.  PowerPC
	can handle 32 keys and hence needs this.

b) Arch-neutral code; and not the arch-specific code,
   determines the format of the string, that displays the key
   for each vma in smaps.

History:
-------
version v12:
	(1) fixed compilation errors seen with various x86
		configs.
version v11:
	(1) code that displays key in smaps is not any more
		defined under CONFIG_ARCH_HAS_PKEYS.
       	    - Comment by Eric W. Biederman and Michal Hocko
	(2) merged two patches that implemented (1).
		- comment by Michal Hocko

version prior to v11:
	(1) used one additional bit from VM_HIGH_ARCH_*
       		to support 32 keys.
	    - Suggestion by Dave Hansen.
	(2) powerpc specific changes to support memory keys.

Ram Pai (3):
  mm, powerpc, x86: define VM_PKEY_BITx bits if CONFIG_ARCH_HAS_PKEYS
    is enabled
  mm, powerpc, x86: introduce an additional vma bit for powerpc pkey
  mm, x86, powerpc: display pkey in smaps only if arch supports pkeys

 arch/powerpc/include/asm/mmu_context.h |    5 -----
 arch/x86/include/asm/mmu_context.h     |    5 -----
 arch/x86/include/asm/pkeys.h           |    1 +
 arch/x86/kernel/fpu/xstate.c           |    5 +++++
 arch/x86/kernel/setup.c                |    8 --------
 fs/proc/task_mmu.c                     |   15 ++++++++-------
 include/linux/mm.h                     |   12 +++++++-----
 include/linux/pkeys.h                  |    7 ++++++-
 8 files changed, 27 insertions(+), 31 deletions(-)

[PATCH v12 1/3] mm, powerpc, x86: define VM_PKEY_BITx bits if CONFIG_ARCH_HAS_PKEYS is enabled

[Ram Pai]

VM_PKEY_BITx are defined only if CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
is enabled. Powerpc also needs these bits. Hence lets define the
VM_PKEY_BITx bits for any architecture that enables
CONFIG_ARCH_HAS_PKEYS.

cc: Dave Hansen <dave.hansen@intel.com>
cc: Michael Ellermen <mpe@ellerman.id.au>
cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/proc/task_mmu.c |    4 ++--
 include/linux/mm.h |    9 +++++----
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index ec6d298..6b996d0 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -679,13 +679,13 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
 		[ilog2(VM_MERGEABLE)]	= "mg",
 		[ilog2(VM_UFFD_MISSING)]= "um",
 		[ilog2(VM_UFFD_WP)]	= "uw",
-#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
+#ifdef CONFIG_ARCH_HAS_PKEYS
 		/* These come out via ProtectionKey: */
 		[ilog2(VM_PKEY_BIT0)]	= "",
 		[ilog2(VM_PKEY_BIT1)]	= "",
 		[ilog2(VM_PKEY_BIT2)]	= "",
 		[ilog2(VM_PKEY_BIT3)]	= "",
-#endif
+#endif /* CONFIG_ARCH_HAS_PKEYS */
 	};
 	size_t i;
 
diff --git a/include/linux/mm.h b/include/linux/mm.h
index ad06d42..ad207ad 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -228,15 +228,16 @@ extern int overcommit_kbytes_handler(struct ctl_table *, int, void __user *,
 #define VM_HIGH_ARCH_4	BIT(VM_HIGH_ARCH_BIT_4)
 #endif /* CONFIG_ARCH_USES_HIGH_VMA_FLAGS */
 
-#if defined(CONFIG_X86)
-# define VM_PAT		VM_ARCH_1	/* PAT reserves whole VMA at once (x86) */
-#if defined (CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS)
+#ifdef CONFIG_ARCH_HAS_PKEYS
 # define VM_PKEY_SHIFT	VM_HIGH_ARCH_BIT_0
 # define VM_PKEY_BIT0	VM_HIGH_ARCH_0	/* A protection key is a 4-bit value */
 # define VM_PKEY_BIT1	VM_HIGH_ARCH_1
 # define VM_PKEY_BIT2	VM_HIGH_ARCH_2
 # define VM_PKEY_BIT3	VM_HIGH_ARCH_3
-#endif
+#endif /* CONFIG_ARCH_HAS_PKEYS */
+
+#if defined(CONFIG_X86)
+# define VM_PAT		VM_ARCH_1	/* PAT reserves whole VMA at once (x86) */
 #elif defined(CONFIG_PPC)
 # define VM_SAO		VM_ARCH_1	/* Strong Access Ordering (powerpc) */
 #elif defined(CONFIG_PARISC)
-- 
1.7.1

Re: [PATCH v12 1/3] mm, powerpc, x86: define VM_PKEY_BITx bits if CONFIG_ARCH_HAS_PKEYS is enabled

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4584 bytes --]

Hi Ram,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.16-rc2 next-20180222]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Ram-Pai/mm-x86-powerpc-Enhancements-to-Memory-Protection-Keys/20180223-042743
config: powerpc-allmodconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

Note: the linux-review/Ram-Pai/mm-x86-powerpc-Enhancements-to-Memory-Protection-Keys/20180223-042743 HEAD c5692bca45543c242ffca15c811923e4c548ed19 builds fine.
      It only hurts bisectibility.

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/pkeys.h:9:0,
                    from arch/powerpc/include/asm/mman.h:16,
                    from include/uapi/linux/mman.h:5,
                    from include/linux/mman.h:9,
                    from arch/powerpc/kernel/asm-offsets.c:22:
   arch/powerpc/include/asm/pkeys.h: In function 'pkey_to_vmflag_bits':
>> arch/powerpc/include/asm/pkeys.h:32:23: error: 'VM_PKEY_BIT4' undeclared (first use in this function); did you mean 'VM_PKEY_BIT0'?
           VM_PKEY_BIT3 | VM_PKEY_BIT4)
                          ^
>> arch/powerpc/include/asm/pkeys.h:42:41: note: in expansion of macro 'ARCH_VM_PKEY_FLAGS'
     return (((u64)pkey << VM_PKEY_SHIFT) & ARCH_VM_PKEY_FLAGS);
                                            ^~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/pkeys.h:32:23: note: each undeclared identifier is reported only once for each function it appears in
           VM_PKEY_BIT3 | VM_PKEY_BIT4)
                          ^
>> arch/powerpc/include/asm/pkeys.h:42:41: note: in expansion of macro 'ARCH_VM_PKEY_FLAGS'
     return (((u64)pkey << VM_PKEY_SHIFT) & ARCH_VM_PKEY_FLAGS);
                                            ^~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/pkeys.h: In function 'vmflag_to_pte_pkey_bits':
   arch/powerpc/include/asm/pkeys.h:54:16: error: 'VM_PKEY_BIT4' undeclared (first use in this function); did you mean 'VM_PKEY_BIT0'?
      ((vm_flags & VM_PKEY_BIT4) ? H_PTE_PKEY_BIT0 : 0x0UL));
                   ^~~~~~~~~~~~
                   VM_PKEY_BIT0
   arch/powerpc/include/asm/pkeys.h: In function 'vma_pkey':
>> arch/powerpc/include/asm/pkeys.h:32:23: error: 'VM_PKEY_BIT4' undeclared (first use in this function); did you mean 'VM_PKEY_BIT0'?
           VM_PKEY_BIT3 | VM_PKEY_BIT4)
                          ^
   arch/powerpc/include/asm/pkeys.h:61:26: note: in expansion of macro 'ARCH_VM_PKEY_FLAGS'
     return (vma->vm_flags & ARCH_VM_PKEY_FLAGS) >> VM_PKEY_SHIFT;
                             ^~~~~~~~~~~~~~~~~~
   make[2]: *** [arch/powerpc/kernel/asm-offsets.s] Error 1
   make[2]: Target '__build' not remade because of errors.
   make[1]: *** [prepare0] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [sub-make] Error 2

vim +32 arch/powerpc/include/asm/pkeys.h

4fb158f6 Ram Pai 2018-01-18  30  
4fb158f6 Ram Pai 2018-01-18  31  #define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
4fb158f6 Ram Pai 2018-01-18 @32  			    VM_PKEY_BIT3 | VM_PKEY_BIT4)
4fb158f6 Ram Pai 2018-01-18  33  
013a91b3 Ram Pai 2018-01-18  34  /* Override any generic PKEY permission defines */
013a91b3 Ram Pai 2018-01-18  35  #define PKEY_DISABLE_EXECUTE   0x4
013a91b3 Ram Pai 2018-01-18  36  #define PKEY_ACCESS_MASK       (PKEY_DISABLE_ACCESS | \
013a91b3 Ram Pai 2018-01-18  37  				PKEY_DISABLE_WRITE  | \
013a91b3 Ram Pai 2018-01-18  38  				PKEY_DISABLE_EXECUTE)
013a91b3 Ram Pai 2018-01-18  39  
013a91b3 Ram Pai 2018-01-18  40  static inline u64 pkey_to_vmflag_bits(u16 pkey)
013a91b3 Ram Pai 2018-01-18  41  {
013a91b3 Ram Pai 2018-01-18 @42  	return (((u64)pkey << VM_PKEY_SHIFT) & ARCH_VM_PKEY_FLAGS);
013a91b3 Ram Pai 2018-01-18  43  }
013a91b3 Ram Pai 2018-01-18  44  

:::::: The code at line 32 was first introduced by commit
:::::: 4fb158f65ac5556b9b4a6f63f38272853ed99b22 powerpc: track allocation status of all pkeys

:::::: TO: Ram Pai <linuxram@us.ibm.com>
:::::: CC: Michael Ellerman <mpe@ellerman.id.au>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 56627 bytes --]

Re: [PATCH v12 1/3] mm, powerpc, x86: define VM_PKEY_BITx bits if CONFIG_ARCH_HAS_PKEYS is enabled

[Ram Pai]

On Fri, Feb 23, 2018 at 03:11:45PM +0800, kbuild test robot wrote:
> Hi Ram,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on linus/master]
> [also build test ERROR on v4.16-rc2 next-20180222]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
>         chmod +x ~/bin/make.cross
...snip..
>         # save the attached .config to linux build tree
>         make.cross ARCH=powerpc 
> 
> Note: the linux-review/Ram-Pai/mm-x86-powerpc-Enhancements-to-Memory-Protection-Keys/20180223-042743 HEAD c5692bca45543c242ffca15c811923e4c548ed19 builds fine.
>       It only hurts bisectibility.

oops, it broke git-bisect on powerpc :-(
The following change will fix it. This should nail it down.

diff --git a/arch/powerpc/include/asm/pkeys.h
b/arch/powerpc/include/asm/pkeys.h
index 0409c80..0b3b669 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -25,6 +25,7 @@
 # define VM_PKEY_BIT1  VM_HIGH_ARCH_1
 # define VM_PKEY_BIT2  VM_HIGH_ARCH_2
 # define VM_PKEY_BIT3  VM_HIGH_ARCH_3
 # define VM_PKEY_BIT4  VM_HIGH_ARCH_4
+#elif !defined(VM_PKEY_BIT4)
+# define VM_PKEY_BIT4  VM_HIGH_ARCH_4
#endif

Re: [PATCH v12 1/3] mm, powerpc, x86: define VM_PKEY_BITx bits if CONFIG_ARCH_HAS_PKEYS is enabled

[Aneesh Kumar K.V]

On 02/24/2018 06:35 AM, Ram Pai wrote:
> On Fri, Feb 23, 2018 at 03:11:45PM +0800, kbuild test robot wrote:
>> Hi Ram,
>>
>> Thank you for the patch! Yet something to improve:
>>
>> [auto build test ERROR on linus/master]
>> [also build test ERROR on v4.16-rc2 next-20180222]
>> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>>
>>          chmod +x ~/bin/make.cross
> ...snip..
>>          # save the attached .config to linux build tree
>>          make.cross ARCH=powerpc
>>
>> Note: the linux-review/Ram-Pai/mm-x86-powerpc-Enhancements-to-Memory-Protection-Keys/20180223-042743 HEAD c5692bca45543c242ffca15c811923e4c548ed19 builds fine.
>>        It only hurts bisectibility.
> 
> oops, it broke git-bisect on powerpc :-(
> The following change will fix it. This should nail it down.
> 
> diff --git a/arch/powerpc/include/asm/pkeys.h
> b/arch/powerpc/include/asm/pkeys.h
> index 0409c80..0b3b669 100644
> --- a/arch/powerpc/include/asm/pkeys.h
> +++ b/arch/powerpc/include/asm/pkeys.h
> @@ -25,6 +25,7 @@
>   # define VM_PKEY_BIT1  VM_HIGH_ARCH_1
>   # define VM_PKEY_BIT2  VM_HIGH_ARCH_2
>   # define VM_PKEY_BIT3  VM_HIGH_ARCH_3
>   # define VM_PKEY_BIT4  VM_HIGH_ARCH_4
> +#elif !defined(VM_PKEY_BIT4)
> +# define VM_PKEY_BIT4  VM_HIGH_ARCH_4
> #endif
> 

Why don't you remove this powerpc definition completely in this patch? 
Also move that comment above #define. The comments are not specific to
BIT1

@@ -231,9 +231,10 @@ extern int overcommit_kbytes_handler(struct 
ctl_table *, int, void __user *,
  #ifdef CONFIG_ARCH_HAS_PKEYS
  # define VM_PKEY_SHIFT	VM_HIGH_ARCH_BIT_0
  # define VM_PKEY_BIT0	VM_HIGH_ARCH_0	/* A protection key is a 4-bit 
value */
-# define VM_PKEY_BIT1	VM_HIGH_ARCH_1
+# define VM_PKEY_BIT1	VM_HIGH_ARCH_1	/* on x86 and 5-bit value on ppc64 
   */
  # define VM_PKEY_BIT2	VM_HIGH_ARCH_2
  # define VM_PKEY_BIT3	VM_HIGH_ARCH_3
+# define VM_PKEY_BIT4	VM_HIGH_ARCH_4
  #endif /* CONFIG_ARCH_HAS_PKEYS */



-aneesh

Re: [PATCH v12 1/3] mm, powerpc, x86: define VM_PKEY_BITx bits if CONFIG_ARCH_HAS_PKEYS is enabled

[Ram Pai]

On Sun, Feb 25, 2018 at 05:27:11PM +0530, Aneesh Kumar K.V wrote:
> 
> 
> On 02/24/2018 06:35 AM, Ram Pai wrote:
> >On Fri, Feb 23, 2018 at 03:11:45PM +0800, kbuild test robot wrote:
> >>Hi Ram,
> >>
> >>Thank you for the patch! Yet something to improve:
> >>
> >>[auto build test ERROR on linus/master]
> >>[also build test ERROR on v4.16-rc2 next-20180222]
> >>[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> >>
> >>         chmod +x ~/bin/make.cross
> >...snip..
> >>         # save the attached .config to linux build tree
> >>         make.cross ARCH=powerpc
> >>
> >>Note: the linux-review/Ram-Pai/mm-x86-powerpc-Enhancements-to-Memory-Protection-Keys/20180223-042743 HEAD c5692bca45543c242ffca15c811923e4c548ed19 builds fine.
> >>       It only hurts bisectibility.
> >
> >oops, it broke git-bisect on powerpc :-(
> >The following change will fix it. This should nail it down.
> >
> >diff --git a/arch/powerpc/include/asm/pkeys.h
> >b/arch/powerpc/include/asm/pkeys.h
> >index 0409c80..0b3b669 100644
> >--- a/arch/powerpc/include/asm/pkeys.h
> >+++ b/arch/powerpc/include/asm/pkeys.h
> >@@ -25,6 +25,7 @@
> >  # define VM_PKEY_BIT1  VM_HIGH_ARCH_1
> >  # define VM_PKEY_BIT2  VM_HIGH_ARCH_2
> >  # define VM_PKEY_BIT3  VM_HIGH_ARCH_3
> >  # define VM_PKEY_BIT4  VM_HIGH_ARCH_4
> >+#elif !defined(VM_PKEY_BIT4)
> >+# define VM_PKEY_BIT4  VM_HIGH_ARCH_4
> >#endif
> >
> 
> Why don't you remove this powerpc definition completely in this
> patch? 

That was my thought too, but refrained from sneaking in the changes into
the patch, to maintain the integrity of all the reviewed-by.

Was planning on sending a seperate patch to remove the
powerpc definition entirely.

RP

Re: [PATCH v12 1/3] mm, powerpc, x86: define VM_PKEY_BITx bits if CONFIG_ARCH_HAS_PKEYS is enabled

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3907 bytes --]

Hi Ram,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.16-rc2 next-20180223]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Ram-Pai/mm-x86-powerpc-Enhancements-to-Memory-Protection-Keys/20180223-042743
config: powerpc-skiroot_defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

Note: the linux-review/Ram-Pai/mm-x86-powerpc-Enhancements-to-Memory-Protection-Keys/20180223-042743 HEAD c5692bca45543c242ffca15c811923e4c548ed19 builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   In file included from include/linux/pkeys.h:9:0,
                    from arch/powerpc/include/asm/mman.h:16,
                    from include/uapi/linux/mman.h:5,
                    from include/linux/mman.h:9,
                    from arch/powerpc/kernel/asm-offsets.c:22:
   arch/powerpc/include/asm/pkeys.h: In function 'pkey_to_vmflag_bits':
>> arch/powerpc/include/asm/pkeys.h:32:23: error: 'VM_PKEY_BIT4' undeclared (first use in this function); did you mean 'VM_PKEY_BIT3'?
           VM_PKEY_BIT3 | VM_PKEY_BIT4)
                          ^
   arch/powerpc/include/asm/pkeys.h:42:41: note: in expansion of macro 'ARCH_VM_PKEY_FLAGS'
     return (((u64)pkey << VM_PKEY_SHIFT) & ARCH_VM_PKEY_FLAGS);
                                            ^~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/pkeys.h:32:23: note: each undeclared identifier is reported only once for each function it appears in
           VM_PKEY_BIT3 | VM_PKEY_BIT4)
                          ^
   arch/powerpc/include/asm/pkeys.h:42:41: note: in expansion of macro 'ARCH_VM_PKEY_FLAGS'
     return (((u64)pkey << VM_PKEY_SHIFT) & ARCH_VM_PKEY_FLAGS);
                                            ^~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/pkeys.h: In function 'vmflag_to_pte_pkey_bits':
   arch/powerpc/include/asm/pkeys.h:54:16: error: 'VM_PKEY_BIT4' undeclared (first use in this function); did you mean 'VM_PKEY_BIT3'?
      ((vm_flags & VM_PKEY_BIT4) ? H_PTE_PKEY_BIT0 : 0x0UL));
                   ^~~~~~~~~~~~
                   VM_PKEY_BIT3
   arch/powerpc/include/asm/pkeys.h: In function 'vma_pkey':
>> arch/powerpc/include/asm/pkeys.h:32:23: error: 'VM_PKEY_BIT4' undeclared (first use in this function); did you mean 'VM_PKEY_BIT3'?
           VM_PKEY_BIT3 | VM_PKEY_BIT4)
                          ^
   arch/powerpc/include/asm/pkeys.h:61:26: note: in expansion of macro 'ARCH_VM_PKEY_FLAGS'
     return (vma->vm_flags & ARCH_VM_PKEY_FLAGS) >> VM_PKEY_SHIFT;
                             ^~~~~~~~~~~~~~~~~~
   make[2]: *** [arch/powerpc/kernel/asm-offsets.s] Error 1
   make[2]: Target '__build' not remade because of errors.
   make[1]: *** [prepare0] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [sub-make] Error 2

vim +32 arch/powerpc/include/asm/pkeys.h

4fb158f6 Ram Pai 2018-01-18  30  
4fb158f6 Ram Pai 2018-01-18  31  #define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
4fb158f6 Ram Pai 2018-01-18 @32  			    VM_PKEY_BIT3 | VM_PKEY_BIT4)
4fb158f6 Ram Pai 2018-01-18  33  

:::::: The code at line 32 was first introduced by commit
:::::: 4fb158f65ac5556b9b4a6f63f38272853ed99b22 powerpc: track allocation status of all pkeys

:::::: TO: Ram Pai <linuxram@us.ibm.com>
:::::: CC: Michael Ellerman <mpe@ellerman.id.au>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 19998 bytes --]

Re: [PATCH v12 1/3] mm, powerpc, x86: define VM_PKEY_BITx bits if CONFIG_ARCH_HAS_PKEYS is enabled

[Dave Hansen]

On 02/21/2018 03:52 PM, Ram Pai wrote:
> VM_PKEY_BITx are defined only if CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
> is enabled. Powerpc also needs these bits. Hence lets define the
> VM_PKEY_BITx bits for any architecture that enables
> CONFIG_ARCH_HAS_PKEYS.

Your fixed version looks fine to me.

Reviewed-by: Dave Hansen <dave.hansen@intel.com>

[PATCH v12 2/3] mm, powerpc, x86: introduce an additional vma bit for powerpc pkey

[Ram Pai]

Currently only 4bits are allocated in the vma flags to hold 16
keys. This is sufficient for x86. PowerPC  supports  32  keys,
which needs 5bits. This patch allocates an  additional bit.

cc: Dave Hansen <dave.hansen@intel.com>
cc: Michael Ellermen <mpe@ellerman.id.au>
cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
cc: Andrew Morton <akpm@linux-foundation.org>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Acked-by: Balbir Singh <bsingharora@gmail.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 fs/proc/task_mmu.c |    1 +
 include/linux/mm.h |    3 ++-
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 6b996d0..6d83bb7 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -685,6 +685,7 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
 		[ilog2(VM_PKEY_BIT1)]	= "",
 		[ilog2(VM_PKEY_BIT2)]	= "",
 		[ilog2(VM_PKEY_BIT3)]	= "",
+		[ilog2(VM_PKEY_BIT4)]	= "",
 #endif /* CONFIG_ARCH_HAS_PKEYS */
 	};
 	size_t i;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index ad207ad..d534f46 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -231,9 +231,10 @@ extern int overcommit_kbytes_handler(struct ctl_table *, int, void __user *,
 #ifdef CONFIG_ARCH_HAS_PKEYS
 # define VM_PKEY_SHIFT	VM_HIGH_ARCH_BIT_0
 # define VM_PKEY_BIT0	VM_HIGH_ARCH_0	/* A protection key is a 4-bit value */
-# define VM_PKEY_BIT1	VM_HIGH_ARCH_1
+# define VM_PKEY_BIT1	VM_HIGH_ARCH_1	/* on x86 and 5-bit value on ppc64   */
 # define VM_PKEY_BIT2	VM_HIGH_ARCH_2
 # define VM_PKEY_BIT3	VM_HIGH_ARCH_3
+# define VM_PKEY_BIT4	VM_HIGH_ARCH_4
 #endif /* CONFIG_ARCH_HAS_PKEYS */
 
 #if defined(CONFIG_X86)
-- 
1.7.1

Re: [PATCH v12 2/3] mm, powerpc, x86: introduce an additional vma bit for powerpc pkey

[Dave Hansen]

On 02/21/2018 03:52 PM, Ram Pai wrote:
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index ad207ad..d534f46 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -231,9 +231,10 @@ extern int overcommit_kbytes_handler(struct ctl_table *, int, void __user *,
>  #ifdef CONFIG_ARCH_HAS_PKEYS
>  # define VM_PKEY_SHIFT	VM_HIGH_ARCH_BIT_0
>  # define VM_PKEY_BIT0	VM_HIGH_ARCH_0	/* A protection key is a 4-bit value */
> -# define VM_PKEY_BIT1	VM_HIGH_ARCH_1
> +# define VM_PKEY_BIT1	VM_HIGH_ARCH_1	/* on x86 and 5-bit value on ppc64   */
>  # define VM_PKEY_BIT2	VM_HIGH_ARCH_2
>  # define VM_PKEY_BIT3	VM_HIGH_ARCH_3
> +# define VM_PKEY_BIT4	VM_HIGH_ARCH_4
>  #endif /* CONFIG_ARCH_HAS_PKEYS */

I think I would prefer if VM_PKEY_BIT4 was unusable on x86, or #defined
to 0.  We don't want folks using a bit that can not be programmed into
the hardware.

[PATCH v12 3/3] mm, x86, powerpc: display pkey in smaps only if arch supports pkeys

[Ram Pai]

Currently the  architecture  specific code is expected to
display  the  protection  keys  in  smap  for a given vma.
This can lead to redundant code and possibly to divergent
formats in which the key gets displayed.

This  patch  changes  the implementation. It displays the
pkey only if the architecture support pkeys, i.e
arch_pkeys_enabled() returns true.  This patch
provides x86 implementation for arch_pkeys_enabled().

x86 arch_show_smap() function is not needed anymore.
Deleting it.

cc: Dave Hansen <dave.hansen@intel.com>
cc: Michael Ellermen <mpe@ellerman.id.au>
cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
(fixed compilation errors for x86 configs)
Acked-by: Michal Hocko <mhocko@suse.com>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/mmu_context.h |    5 -----
 arch/x86/include/asm/mmu_context.h     |    5 -----
 arch/x86/include/asm/pkeys.h           |    1 +
 arch/x86/kernel/fpu/xstate.c           |    5 +++++
 arch/x86/kernel/setup.c                |    8 --------
 fs/proc/task_mmu.c                     |   10 +++++-----
 include/linux/pkeys.h                  |    7 ++++++-
 7 files changed, 17 insertions(+), 24 deletions(-)

diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
index 051b3d6..566b3c2 100644
--- a/arch/powerpc/include/asm/mmu_context.h
+++ b/arch/powerpc/include/asm/mmu_context.h
@@ -203,11 +203,6 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
 #define thread_pkey_regs_restore(new_thread, old_thread)
 #define thread_pkey_regs_init(thread)
 
-static inline int vma_pkey(struct vm_area_struct *vma)
-{
-	return 0;
-}
-
 static inline u64 pte_to_hpte_pkey_bits(u64 pteflags)
 {
 	return 0x0UL;
diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h
index c931b88..134f3a0 100644
--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -294,11 +294,6 @@ static inline int vma_pkey(struct vm_area_struct *vma)
 
 	return (vma->vm_flags & vma_pkey_mask) >> VM_PKEY_SHIFT;
 }
-#else
-static inline int vma_pkey(struct vm_area_struct *vma)
-{
-	return 0;
-}
 #endif
 
 /*
diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h
index a0ba1ff..f6c287b 100644
--- a/arch/x86/include/asm/pkeys.h
+++ b/arch/x86/include/asm/pkeys.h
@@ -6,6 +6,7 @@
 
 extern int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
 		unsigned long init_val);
+extern bool arch_pkeys_enabled(void);
 
 /*
  * Try to dedicate one of the protection keys to be used as an
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 87a57b7..4f566e9 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -945,6 +945,11 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
 
 	return 0;
 }
+
+bool arch_pkeys_enabled(void)
+{
+	return boot_cpu_has(X86_FEATURE_OSPKE);
+}
 #endif /* ! CONFIG_ARCH_HAS_PKEYS */
 
 /*
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 1ae67e9..d0539e3 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1314,11 +1314,3 @@ static int __init register_kernel_offset_dumper(void)
 	return 0;
 }
 __initcall(register_kernel_offset_dumper);
-
-void arch_show_smap(struct seq_file *m, struct vm_area_struct *vma)
-{
-	if (!boot_cpu_has(X86_FEATURE_OSPKE))
-		return;
-
-	seq_printf(m, "ProtectionKey:  %8u\n", vma_pkey(vma));
-}
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 6d83bb7..70aa912 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -18,10 +18,12 @@
 #include <linux/page_idle.h>
 #include <linux/shmem_fs.h>
 #include <linux/uaccess.h>
+#include <linux/pkeys.h>
 
 #include <asm/elf.h>
 #include <asm/tlb.h>
 #include <asm/tlbflush.h>
+#include <asm/mmu_context.h>
 #include "internal.h"
 
 void task_mem(struct seq_file *m, struct mm_struct *mm)
@@ -733,10 +735,6 @@ static int smaps_hugetlb_range(pte_t *pte, unsigned long hmask,
 }
 #endif /* HUGETLB_PAGE */
 
-void __weak arch_show_smap(struct seq_file *m, struct vm_area_struct *vma)
-{
-}
-
 static int show_smap(struct seq_file *m, void *v, int is_pid)
 {
 	struct proc_maps_private *priv = m->private;
@@ -856,9 +854,11 @@ static int show_smap(struct seq_file *m, void *v, int is_pid)
 			   (unsigned long)(mss->pss >> (10 + PSS_SHIFT)));
 
 	if (!rollup_mode) {
-		arch_show_smap(m, vma);
+		if (arch_pkeys_enabled())
+			seq_printf(m, "ProtectionKey:  %8u\n", vma_pkey(vma));
 		show_smap_vma_flags(m, vma);
 	}
+
 	m_cache_vma(m, vma);
 	return ret;
 }
diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h
index 0794ca7..49dff15 100644
--- a/include/linux/pkeys.h
+++ b/include/linux/pkeys.h
@@ -3,7 +3,6 @@
 #define _LINUX_PKEYS_H
 
 #include <linux/mm_types.h>
-#include <asm/mmu_context.h>
 
 #ifdef CONFIG_ARCH_HAS_PKEYS
 #include <asm/pkeys.h>
@@ -13,6 +12,7 @@
 #define arch_override_mprotect_pkey(vma, prot, pkey) (0)
 #define PKEY_DEDICATED_EXECUTE_ONLY 0
 #define ARCH_VM_PKEY_FLAGS 0
+#define vma_pkey(vma) 0
 
 static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
 {
@@ -35,6 +35,11 @@ static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
 	return 0;
 }
 
+static inline bool arch_pkeys_enabled(void)
+{
+	return false;
+}
+
 static inline void copy_init_pkru_to_fpregs(void)
 {
 }
-- 
1.7.1

Re: [PATCH v12 3/3] mm, x86, powerpc: display pkey in smaps only if arch supports pkeys

[Dave Hansen]

On 02/21/2018 03:52 PM, Ram Pai wrote:
> Currently the  architecture  specific code is expected to
> display  the  protection  keys  in  smap  for a given vma.
> This can lead to redundant code and possibly to divergent
> formats in which the key gets displayed.
> 
> This  patch  changes  the implementation. It displays the
> pkey only if the architecture support pkeys, i.e
> arch_pkeys_enabled() returns true.  This patch
> provides x86 implementation for arch_pkeys_enabled().
> 
> x86 arch_show_smap() function is not needed anymore.
> Deleting it.

This looks fine to me.  Thanks for doing these, Ram.

Reviewed-by: Dave Hansen <dave.hansen@intel.com>

mm, x86, powerpc: pkey semantics for key-0 ?

[Ram Pai]

Dave,

Is there a reason why the default key; key-0, is not allowed to be
explicitly associated with pages using pkey_mprotect()?

I see valid usecases where an application may initially want to
associate an address-range with some key and latter choose to revert to
its initial state, by associating key-0.  However our implementation
(both x86 and power) do not allow pkey_mprotect() to be called with
key-0.

I do not see a reason why it must be blocked.

Thoughts?
RP

Re: mm, x86, powerpc: pkey semantics for key-0 ?

[Dave Hansen]

On 03/08/2018 10:25 AM, Ram Pai wrote:
> Is there a reason why the default key; key-0, is not allowed to be
> explicitly associated with pages using pkey_mprotect()?

No, it's a bug if it is not permitted.  I have a vague recollection of
knowing about this and having a patch.