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(-)
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 <[email protected]>
cc: Michael Ellermen <[email protected]>
cc: Benjamin Herrenschmidt <[email protected]>
cc: Andrew Morton <[email protected]>
Reviewed-by: Ingo Molnar <[email protected]>
Acked-by: Balbir Singh <[email protected]>
Signed-off-by: Ram Pai <[email protected]>
---
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
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 <[email protected]>
cc: Michael Ellermen <[email protected]>
cc: Benjamin Herrenschmidt <[email protected]>
cc: Andrew Morton <[email protected]>
Signed-off-by: Thiago Jung Bauermann <[email protected]>
(fixed compilation errors for x86 configs)
Acked-by: Michal Hocko <[email protected]>
Reviewed-by: Ingo Molnar <[email protected]>
Signed-off-by: Ram Pai <[email protected]>
---
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
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 <[email protected]>
cc: Michael Ellermen <[email protected]>
cc: Benjamin Herrenschmidt <[email protected]>
cc: Andrew Morton <[email protected]>
Signed-off-by: Ram Pai <[email protected]>
Reviewed-by: Ingo Molnar <[email protected]>
Reviewed-by: Aneesh Kumar K.V <[email protected]>
---
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
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 <[email protected]>
:::::: CC: Michael Ellerman <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
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 <[email protected]>
:::::: CC: Michael Ellerman <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
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
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
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
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 <[email protected]>
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.
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 <[email protected]>
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
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.