powerpc: Memory Protection Keys
Subsystems (5)
v7 cover powerpc: Memory Protection Keys
Memory protection keys enable applications to protect its address space from inadvertent access from or corruption by itself.
The overall idea:
-----------------
A process allocates a key and associates it with
an address range within its address space.
The process then can dynamically set read/write
permissions on the key without involving the
kernel. Any code that violates the permissions
of the address space; as defined by its associated key, will receive a segmentation fault.
This patch series enables the feature on PPC64 HPTE platform.
ISA3.0 section 5.7.13 describes the detailed
specifications.
Highlevel view of the design:
---------------------------
When an application associates a key with a address
address range, program the key in the Linux PTE.
When the MMU detects a page fault, allocate a hash
page and program the key into HPTE. And finally
when the MMU detects a key violation; due to
invalid application access, invoke the registered
signal handler and provide the violated key number
as well as the state of the key register (AMR), at
the time it faulted.
Testing:
-------
This patch series has passed all the protection key
tests available in the selftests directory.The
tests are updated to work on both x86 and powerpc.
NOTE: All the selftest related patches will be part
of a separate patch series.
Outstanding issues:
-------------------
How will the application know if pkey is enabled, if
so how many pkeys are available? IsPKEY_DISABLE_EXECUTE supported? - Ben.
History:
-------
version v7:
(1) refers to device tree property to enable protection keys.
(2) adds 4K PTE support.
(3) fixes a couple of bugs noticed by Thiago
(4) decouples this patch series from arch-
independent code. This patch series can now stand by itself, with one kludge patch(2).
version v6:
(1) selftest changes are broken down into 20 incremental patches.
(2) A separate key allocation mask that
includes PKEY_DISABLE_EXECUTE is
added for powerpc
(3) pkey feature is enabled for 64K HPT case only. RPT and 4k HPT is disabled.
(4) Documentation is updated to better
capture the semantics.
(5) introduced arch_pkeys_enabled() to find
if an arch enables pkeys. Correspond-
ing change the logic that displays
key value in smaps.
(6) code rearranged in many places based on
comments from Dave Hansen, Balbir,
Anshuman.
(7) fixed one bug where a bogus key could be
associated successfully in
pkey_mprotect().
version v5:
(1) reverted back to the old design -- store the key in the pte, instead of bypassing it. The v4 design slowed down the hash page path.
(2) detects key violation when kernel is told to access user pages.
(3) further refined the patches into smaller consumable units
(4) page faults handlers captures the fault-
ing key
from the pte instead of the vma. This
closes a race between where the key update in the vma and a key fault caused by the key programmed in the pte.
(5) a key created with access-denied should
also set it up to deny write. Fixed it.
(6) protection-key number is displayed in
smaps the x86 way.
version v4:
(1) patches no more depend on the pte bits to program the hpte
-- comment by Balbir
(2) documentation updates
(3) fixed a bug in the selftest.
(4) unlike x86, powerpc lets signal handler
change key permission bits; the
change will persist across signal
handler boundaries. Earlier we
allowed the signal handler to
modify a field in the siginfo
structure which would than be used
by the kernel to program the key protection register (AMR)
-- resolves a issue raised by Ben.
"Calls to sys_swapcontext with a
made-up context will end up with a crap AMR if done by code who didn't know about that register".
(5) these changes enable protection keys on 4k-page kernel aswell.
version v3:
(1) split the patches into smaller consumable patches.
(2) added the ability to disable execute
permission on a key at creation.
(3) rename calc_pte_to_hpte_pkey_bits() to
pte_to_hpte_pkey_bits()
-- suggested by Anshuman
(4) some code optimization and clarity in
do_page_fault()
(5) A bug fix while invalidating a hpte slot in __hash_page_4K()
-- noticed by Aneesh
version v2:
(1) documentation and selftest added.
(2) fixed a bug in 4k hpte backed 64k pte
where page invalidation was not
done correctly, and initialization
of second-part-of-the-pte was not
done correctly if the pte was not
yet Hashed with a hpte.
-- Reported by Aneesh.
(3) Fixed ABI breakage caused in siginfo structure.
-- Reported by Anshuman.
version v1: Initial version
Ram Pai (25):
powerpc: define an additional vma bit for protection keys.
powerpc: track allocation status of all pkeys
powerpc: helper function to read,write AMR,IAMR,UAMOR registers
powerpc: helper functions to initialize AMR, IAMR and UAMOR registers
powerpc: cleaup AMR,iAMR when a key is allocated or freed
powerpc: implementation for arch_set_user_pkey_access()
powerpc: sys_pkey_alloc() and sys_pkey_free() system calls
powerpc: ability to create execute-disabled pkeys
powerpc: store and restore the pkey state across context switches
powerpc: introduce execute-only pkey
powerpc: ability to associate pkey to a vma
powerpc: implementation for arch_override_mprotect_pkey()
powerpc: map vma key-protection bits to pte key bits.
powerpc: sys_pkey_mprotect() system call
powerpc: Program HPTE key protection bits
powerpc: helper to validate key-access permissions of a pte
powerpc: check key protection for user page access
powerpc: Macro the mask used for checking DSI exception
powerpc: implementation for arch_vma_access_permitted()
powerpc: Handle exceptions caused by pkey violation
powerpc: capture AMR register content on pkey violation
powerpc: introduce get_pte_pkey() helper
powerpc: capture the violated protection key on fault
powerpc: Deliver SEGV signal on pkey violation
powerpc: Enable pkey subsystem
arch/powerpc/include/asm/book3s/64/mmu-hash.h | 10 + arch/powerpc/include/asm/book3s/64/mmu.h | 10 + arch/powerpc/include/asm/book3s/64/pgtable.h | 69 +++++++- arch/powerpc/include/asm/cputable.h | 8 +- arch/powerpc/include/asm/mman.h | 16 ++- arch/powerpc/include/asm/mmu_context.h | 18 ++- arch/powerpc/include/asm/paca.h | 4 + arch/powerpc/include/asm/pkeys.h | 255 ++++++++++++++++++++++++- arch/powerpc/include/asm/processor.h | 5 + arch/powerpc/include/asm/reg.h | 8 +- arch/powerpc/include/asm/systbl.h | 3 + arch/powerpc/include/asm/unistd.h | 6 +- arch/powerpc/include/uapi/asm/ptrace.h | 1 + arch/powerpc/include/uapi/asm/unistd.h | 3 + arch/powerpc/kernel/asm-offsets.c | 6 + arch/powerpc/kernel/exceptions-64s.S | 2 +- arch/powerpc/kernel/process.c | 25 +++ arch/powerpc/kernel/prom.c | 19 ++ arch/powerpc/kernel/signal_32.c | 5 + arch/powerpc/kernel/signal_64.c | 4 + arch/powerpc/kernel/traps.c | 15 ++ arch/powerpc/mm/fault.c | 31 +++ arch/powerpc/mm/hash_utils_64.c | 26 +++ arch/powerpc/mm/mmu_context_book3s64.c | 2 + arch/powerpc/mm/pkeys.c | 256 +++++++++++++++++++++++++ 25 files changed, 787 insertions(+), 20 deletions(-)
RFC v7 1/25 powerpc: define an additional vma bit for protection keys.
powerpc needs an additional vma bit to support 32 keys. Till the additional vma bit lands in include/linux/mm.h we have to define it in powerpc specific header file. This is needed to get pkeys working on power.
--- arch/powerpc/include/asm/pkeys.h | 18 ++++++++++++++++++ 1 files changed, 18 insertions(+), 0 deletions(-)
arch/powerpc/include/asm/pkeys.h +18 -0
@@ -2,6 +2,24 @@#define _ASM_PPC64_PKEYS_Hextern bool pkey_inited; + +/* +* powerpc needs an additional vma bit to support 32 keys. +* Till the additional vma bit lands in include/linux/mm.h +* we have to carry the hunk below. This is needed to get +* pkeys working on power. -- Ram +*/ +#ifndef VM_HIGH_ARCH_BIT_4 +#define VM_HIGH_ARCH_BIT_4 36 +#define VM_HIGH_ARCH_4 BIT(VM_HIGH_ARCH_BIT_4) +#define VM_PKEY_SHIFT VM_HIGH_ARCH_BIT_0 +#define VM_PKEY_BIT0 VM_HIGH_ARCH_0 +#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 +#endif +#define ARCH_VM_PKEY_FLAGS 0static inline bool mm_pkey_is_allocated (struct mm_struct *mm,int pkey)
Signature · 1 line
1.7.1
RFC v7 2/25 powerpc: track allocation status of all pkeys
In reply to Ram PaiTotal 32 keys are available on power7 and above. However pkey 0,1 are reserved. So effectively we have 30 pkeys.
On 4K kernels, we do not have 5 bits in the PTE to represent all the keys; we only have 3bits.Two of those keys are reserved; pkey 0 and pkey 1. So effectively we have 6 pkeys.
This patch keeps track of reserved keys, allocated keys and keys that are currently free.
Also it adds skeletal functions and macros, that the architecture-independent code expects to be available.
--- arch/powerpc/include/asm/book3s/64/mmu.h | 9 +++ arch/powerpc/include/asm/mmu_context.h | 1 + arch/powerpc/include/asm/pkeys.h | 98 ++++++++++++++++++++++++++++- arch/powerpc/mm/mmu_context_book3s64.c | 2 + arch/powerpc/mm/pkeys.c | 2 + 5 files changed, 108 insertions(+), 4 deletions(-)
arch/powerpc/include/asm/book3s/64/mmu.h +9 -0
@@ -108,6 +108,15 @@ struct patb_entry {#ifdef CONFIG_SPAPR_TCE_IOMMU struct list_head iommu_group_mem_list;#endif + +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +/* +* Each bit represents one protection key. +* bit set -> key allocated +* bit unset -> key available for allocation +*/ +u32 pkey_allocation_map; +#endif } mm_context_t;/*
arch/powerpc/include/asm/mmu_context.h +1 -0
@@ -184,6 +184,7 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,#ifndef CONFIG_PPC64_MEMORY_PROTECTION_KEYS #define pkey_initialize () +#define pkey_mm_init (mm)#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ #endif /* __KERNEL__ */
arch/powerpc/include/asm/pkeys.h +94 -4
133 lines not on this page · raw message
arch/powerpc/mm/mmu_context_book3s64.c +2 -0
@@ -16,6 +16,7 @@#include <linux/string.h> #include <linux/types.h> #include <linux/mm.h> +#include <linux/pkeys.h> #include <linux/spinlock.h> #include <linux/idr.h> #include <linux/export.h>
@@ -120,6 +121,7 @@ static int hash__init_new_context(struct mm_struct *mm)subpage_prot_init_new_context (mm); +pkey_mm_init (mm);return index; }
arch/powerpc/mm/pkeys.c +2 -0
@@ -16,3 +16,5 @@#include <linux/pkeys.h> /* PKEY_* */ bool pkey_inited; +int pkeys_total;/* total pkeys as per device tree */ +u32 initial_allocation_mask;/* bits set for reserved keys */
Signature · 1 line
1.7.1
Re: [RFC v7 02/25] powerpc: track allocation status of all pkeys
Quoted 25 lines · Ram Pai
static inline void
pkey_initialize(void)
{arch/powerpc/include/asm/pkeys.h +18
-0+int os_reserved, i; +/* disable the pkey system till everything * is in place. A patch further down the * line will enable it. */ pkey_inited = false; + +/* Lets assume 32 keys */ + pkeys_total =32 ; + +#ifdef CONFIG_PPC_4K_PAGES +/* +* the OS can manage only 8 pkeys +* due to its inability to represent +* them in the linux 4K-PTE. +*/ + os_reserved = pkeys_total-8 ; +#else + os_reserved =0 ; +#endif +/* +* Bits are in LE format.
arch/powerpc/include/asm/pkeys.h static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey, · line 159
Quoted 10 lines
arch/powerpc/include/asm/pkeys.h +8
-0+* NOTE: 1, 0 are reserved. +* key 0 is the default key, which allows read/write/execute. +* key 1 is recommended not to be used. +* PowerISA(3.0) page 1015, programming note. +*/ + initial_allocation_mask = ~0x0 ; +for (i =2 ; i < (pkeys_total - os_reserved); i++) + initial_allocation_mask &= ~(0x1<<i); }#endif /*_ASM_PPC64_PKEYS_H */
In v6, key 31 was also reserved, but it's not in this version. Is this intentional?
Isn't it better for this function to be in pkeys.c? Ideally, functions should be in .c files not in headers unless they're very small or performance sensitive IMHO.
Signature · 2 lines
Thiago Jung Bauermann IBM Linux Technology Center
Michael Ellerman
reviewerRe: [RFC v7 02/25] powerpc: track allocation status of all pkeys
Quoted 31 lines · Thiago Jung Bauermann
Quoted 30 lines · Ram Pai
static inline void
pkey_initialize(void)
{arch/powerpc/include/asm/pkeys.h +23
-0+int os_reserved, i; +/* disable the pkey system till everything * is in place. A patch further down the * line will enable it. */ pkey_inited = false; + +/* Lets assume 32 keys */ + pkeys_total =32 ; + +#ifdef CONFIG_PPC_4K_PAGES +/* +* the OS can manage only 8 pkeys +* due to its inability to represent +* them in the linux 4K-PTE. +*/ + os_reserved = pkeys_total-8 ; +#else + os_reserved =0 ; +#endif +/* +* Bits are in LE format. +* NOTE: 1, 0 are reserved. +* key 0 is the default key, which allows read/write/execute. +* key 1 is recommended not to be used. +* PowerISA(3.0) page 1015, programming note. +*/
Quoted 8 lines
arch/powerpc/include/asm/pkeys.h static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey, · line 159
Quoted 5 lines
arch/powerpc/include/asm/pkeys.h +3
-0+ initial_allocation_mask = ~0x0 ; +for (i =2 ; i < (pkeys_total - os_reserved); i++) + initial_allocation_mask &= ~(0x1<<i); }#endif /*_ASM_PPC64_PKEYS_H */ In v6, key 31 was also reserved, but it's not in this version. Is this intentional?
That whole thing could be replaced with two constants.
Except it can't, because we can't just hard code the number of keys. It needs to come either from the device tree or be based on the CPU we're running on.
Quoted 3 lines
Isn't it better for this function to be in
pkeys.c? Ideally, functions should be in .c files not in headers unless they're very small or performance sensitive IMHO.
Yes. No reason for that to be in a header AFAICS.
cheers
Re: [RFC v7 02/25] powerpc: track allocation status of all pkeys
Quoted 48 lines · Michael Ellerman
Quoted 31 lines · Thiago Jung Bauermann
Quoted 30 lines · Ram Pai
static inline void
pkey_initialize(void)
{arch/powerpc/include/asm/pkeys.h +23
-0+int os_reserved, i; +/* disable the pkey system till everything * is in place. A patch further down the * line will enable it. */ pkey_inited = false; + +/* Lets assume 32 keys */ + pkeys_total =32 ; + +#ifdef CONFIG_PPC_4K_PAGES +/* +* the OS can manage only 8 pkeys +* due to its inability to represent +* them in the linux 4K-PTE. +*/ + os_reserved = pkeys_total-8 ; +#else + os_reserved =0 ; +#endif +/* +* Bits are in LE format. +* NOTE: 1, 0 are reserved. +* key 0 is the default key, which allows read/write/execute. +* key 1 is recommended not to be used. +* PowerISA(3.0) page 1015, programming note. +*/ Quoted 8 lines
arch/powerpc/include/asm/pkeys.h static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey, · line 159
Quoted 5 lines
arch/powerpc/include/asm/pkeys.h +3
-0+ initial_allocation_mask = ~0x0 ; +for (i =2 ; i < (pkeys_total - os_reserved); i++) + initial_allocation_mask &= ~(0x1<<i); }#endif /*_ASM_PPC64_PKEYS_H */ In v6, key 31 was also reserved, but it's not in this version. Is this intentional?
That whole thing could be replaced with two constants.
Except it can't, because we can't just hard code the number of keys. It needs to come either from the device tree or be based on the CPU we're running on.
Quoted 5 lines
Quoted 3 lines
Isn't it better for this function to be in
pkeys.c? Ideally, functions should be in .c files not in headers unless they're very small or performance sensitive IMHO.Yes. No reason for that to be in a header AFAICS.
Yes can be moved into the pkeys.c file. It was a simple function to begin with....but not so any more.
RP
Re: [RFC v7 02/25] powerpc: track allocation status of all pkeys
In reply to Thiago Jung BauermannQuoted 32 lines · Thiago Jung Bauermann
Quoted 30 lines · Ram Pai
static inline void
pkey_initialize(void)
{arch/powerpc/include/asm/pkeys.h +23
-0+int os_reserved, i; +/* disable the pkey system till everything * is in place. A patch further down the * line will enable it. */ pkey_inited = false; + +/* Lets assume 32 keys */ + pkeys_total =32 ; + +#ifdef CONFIG_PPC_4K_PAGES +/* +* the OS can manage only 8 pkeys +* due to its inability to represent +* them in the linux 4K-PTE. +*/ + os_reserved = pkeys_total-8 ; +#else + os_reserved =0 ; +#endif +/* +* Bits are in LE format. +* NOTE: 1, 0 are reserved. +* key 0 is the default key, which allows read/write/execute. +* key 1 is recommended not to be used. +* PowerISA(3.0) page 1015, programming note. +*/
Quoted 8 lines
arch/powerpc/include/asm/pkeys.h static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey, · line 159
Quoted 5 lines
arch/powerpc/include/asm/pkeys.h +3
-0+ initial_allocation_mask = ~0x0 ; +for (i =2 ; i < (pkeys_total - os_reserved); i++) + initial_allocation_mask &= ~(0x1<<i); }#endif /*_ASM_PPC64_PKEYS_H */ In v6, key 31 was also reserved, but it's not in this version. Is this intentional?
On powernv platform, there is no hypervisor and hence the hypervisor will not reserve key 31 for its own use. Wherease on PAPR guest the hypervisor takes away key 31.
Its not possible to determine at compile time which keys are used or not. Hence the above code. pkeys_total is 32 in this patch, but will be set to whatever value the device tree tells us. That will be done in a subsequent patch.
RP
Re: [RFC v7 02/25] powerpc: track allocation status of all pkeys
Quoted 42 lines · Ram Pai
Quoted 32 lines · Thiago Jung Bauermann
Quoted 30 lines · Ram Pai
static inline void
pkey_initialize(void)
{arch/powerpc/include/asm/pkeys.h +23
-0+int os_reserved, i; +/* disable the pkey system till everything * is in place. A patch further down the * line will enable it. */ pkey_inited = false; + +/* Lets assume 32 keys */ + pkeys_total =32 ; + +#ifdef CONFIG_PPC_4K_PAGES +/* +* the OS can manage only 8 pkeys +* due to its inability to represent +* them in the linux 4K-PTE. +*/ + os_reserved = pkeys_total-8 ; +#else + os_reserved =0 ; +#endif +/* +* Bits are in LE format. +* NOTE: 1, 0 are reserved. +* key 0 is the default key, which allows read/write/execute. +* key 1 is recommended not to be used. +* PowerISA(3.0) page 1015, programming note. +*/ Quoted 8 lines
arch/powerpc/include/asm/pkeys.h static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey, · line 159
Quoted 5 lines
arch/powerpc/include/asm/pkeys.h +3
-0+ initial_allocation_mask = ~0x0 ; +for (i =2 ; i < (pkeys_total - os_reserved); i++) + initial_allocation_mask &= ~(0x1<<i); }#endif /*_ASM_PPC64_PKEYS_H */ In v6, key 31 was also reserved, but it's not in this version. Is this intentional?
Quoted 8 lines
On powernv platform, there is no hypervisor and hence the hypervisor will not reserve key 31 for its own use. Wherease on PAPR guest the hypervisor takes away key 31.
Its not possible to determine at compile time which keys are used or not. Hence the above code.
pkeys_totalis 32 in this patch, but will be set to whatever value the device tree tells us. That will be done in a subsequent patch.
You're right. At the time I made that comment I didn't realize that the hypervisor would subtract its reserved key from the device property.
Signature · 2 lines
Thiago Jung Bauermann IBM Linux Technology Center
Re: [RFC v7 02/25] powerpc: track allocation status of all pkeys
In reply to Ram PaiQuoted 92 lines · On Sun, 30 Jul 2017 17:12:03 -0700 Ram Pai <[email protected]> wrote:
Total 32 keys are available on power7 and above. However pkey 0,1 are reserved. So effectively we have 30 pkeys.
On 4K kernels, we do not have 5 bits in the PTE to represent all the keys; we only have 3bits.Two of those keys are reserved; pkey 0 and pkey 1. So effectively we have 6 pkeys.
This patch keeps track of reserved keys, allocated keys and keys that are currently free.
Also it adds skeletal functions and macros, that the architecture-independent code expects to be available.
Signed-off-by Ram Pai <[email protected]>--- arch/powerpc/include/asm/book3s/64/mmu.h | 9 +++ arch/powerpc/include/asm/mmu_context.h | 1 + arch/powerpc/include/asm/pkeys.h | 98 ++++++++++++++++++++++++++++- arch/powerpc/mm/mmu_context_book3s64.c | 2 + arch/powerpc/mm/pkeys.c | 2 + 5 files changed, 108 insertions(+), 4 deletions(-)arch/powerpc/include/asm/book3s/64/mmu.h +9
-0@@ -108,6 +108,15 @@ struct patb_entry {#ifdef CONFIG_SPAPR_TCE_IOMMU struct list_head iommu_group_mem_list;#endif + +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +/* +* Each bit represents one protection key. +* bit set -> key allocated +* bit unset -> key available for allocation +*/ +u32 pkey_allocation_map; +#endif } mm_context_t;/* arch/powerpc/include/asm/mmu_context.h +1
-0@@ -184,6 +184,7 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,#ifndef CONFIG_PPC64_MEMORY_PROTECTION_KEYS #define pkey_initialize () +#define pkey_mm_init (mm)#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ #endif /* __KERNEL__ */ arch/powerpc/include/asm/pkeys.h +21
-1@@ -2,6 +2,8 @@#define _ASM_PPC64_PKEYS_Hextern bool pkey_inited; +extern int pkeys_total;/* total pkeys as per device tree */ +extern u32 initial_allocation_mask;/* bits set for reserved keys */ /* * powerpc needs an additional vma bit to support 32 keys. @@ -20,21 +22,76 @@#define VM_PKEY_BIT4 VM_HIGH_ARCH_4#endif -#define ARCH_VM_PKEY_FLAGS 0 +#define arch_max_pkey () pkeys_total +#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \ + VM_PKEY_BIT3 | VM_PKEY_BIT4) + +#define pkey_alloc_mask (pkey) (0x1 << pkey) + +#define mm_pkey_allocation_map (mm) (mm->context.pkey_allocation_map) + +#define mm_set_pkey_allocated (mm, pkey) { \ + mm_pkey_allocation_map(mm) |= pkey_alloc_mask(pkey); \ +} + +#define mm_set_pkey_free (mm, pkey) { \ + mm_pkey_allocation_map(mm) &= ~pkey_alloc_mask(pkey); \ +} + +#define mm_set_pkey_is_allocated (mm, pkey) \ + (mm_pkey_allocation_map(mm) & pkey_alloc_mask(pkey)) +
arch/powerpc/include/asm/pkeys.h line 52
Quoted 10 lines
arch/powerpc/include/asm/pkeys.h +6
-1+#define mm_set_pkey_is_reserved (mm, pkey) (initial_allocation_mask & \ + pkey_alloc_mask(pkey))static inline bool mm_pkey_is_allocated (struct mm_struct *mm,int pkey) { -return (pkey ==0 ); +/* a reserved key is never considered as 'explicitly allocated' */ +return ((pkey <arch_max_pkey ()) && + !mm_set_pkey_is_reserved (mm, pkey) && +mm_set_pkey_is_allocated (mm, pkey));
Sounds like this should be called mm_pkey_is_free()
Quoted 14 lines
arch/powerpc/include/asm/pkeys.h +9
-1} +/* +* Returns a positive, 5-bit key on success, or -1 on failure. +*/ static inline int mm_pkey_alloc (struct mm_struct *mm) { -return -1 ; +/* +* Note: this is the one and only place we make sure +* that the pkey is valid as far as the hardware is +* concerned. The rest of the kernel trusts that +* only good, valid pkeys come out of here. +*/
arch/powerpc/include/asm/pkeys.h line 75
Quoted 10 lines
arch/powerpc/include/asm/pkeys.h +10
-0+u32 all_pkeys_mask = (u32)(~(0x0 )); +int ret; + +if (!pkey_inited) +return -1 ; +/* +* Are we out of pkeys? We must handle this specially +* because ffz() behavior is undefined if there are no +* zeros. +*/
Point A
arch/powerpc/include/asm/pkeys.h line 79
Quoted 4 lines
arch/powerpc/include/asm/pkeys.h +4
-0+if (mm_pkey_allocation_map (mm) == all_pkeys_mask) +return -1 ; + + ret =ffz ((u32 )mm_pkey_allocation_map (mm));
Point B
So the allocation occurs from MSB to LSB? I also think you need a preempt disable between points A, B.
Is this code reentrant?
Quoted 29 lines
arch/powerpc/include/asm/pkeys.h +17
-1+mm_set_pkey_allocated (mm, ret); +return ret; }static inline int mm_pkey_free (struct mm_struct *mm,int pkey) { -return -EINVAL ; +if (!pkey_inited) +return -1 ; + +if (!mm_pkey_is_allocated (mm, pkey)) +return -EINVAL ; + +mm_set_pkey_free (mm, pkey); + +return 0 ; }/* @@ -58,12 +115,45 @@ static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,return 0 ; } +static inline void pkey_mm_init (struct mm_struct *mm) +{ +if (!pkey_inited) +return ; +mm_pkey_allocation_map (mm) = initial_allocation_mask; +}
arch/powerpc/include/asm/pkeys.h static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey, · line 133
Quoted 10 lines
arch/powerpc/include/asm/pkeys.h +3
-0+static inline void pkey_initialize (void ) { +int os_reserved, i; +/* disable the pkey system till everything * is in place. A patch further down the * line will enable it. */ pkey_inited = false;
Why are we doing it this way?
Quoted 14 lines
arch/powerpc/include/asm/pkeys.h +14
-0+ +/* Lets assume 32 keys */ + pkeys_total =32 ; + +#ifdef CONFIG_PPC_4K_PAGES +/* +* the OS can manage only 8 pkeys +* due to its inability to represent +* them in the linux 4K-PTE. +*/ + os_reserved = pkeys_total-8 ; +#else + os_reserved =0 ; +#endif
arch/powerpc/include/asm/pkeys.h static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey, · line 157
Quoted 10 lines
arch/powerpc/include/asm/pkeys.h +10
-0+/* +* Bits are in LE format. +* NOTE: 1, 0 are reserved. +* key 0 is the default key, which allows read/write/execute. +* key 1 is recommended not to be used. +* PowerISA(3.0) page 1015, programming note. +*/ + initial_allocation_mask = ~0x0 ; +for (i =2 ; i < (pkeys_total - os_reserved); i++) + initial_allocation_mask &= ~(0x1 <<i);
On radix key 0 is used for supervisor mode to implement SMEP and SMAP. It would be nice to reserve key 0 for the OS for hash as well. Why are the bits in LE format? The limitations of the adjunct partition apply to Linux?
Quoted 27 lines
arch/powerpc/include/asm/pkeys.h +0
-0}#endif /*_ASM_PPC64_PKEYS_H */ arch/powerpc/mm/mmu_context_book3s64.c +2
-0@@ -16,6 +16,7 @@#include <linux/string.h> #include <linux/types.h> #include <linux/mm.h> +#include <linux/pkeys.h> #include <linux/spinlock.h> #include <linux/idr.h> #include <linux/export.h> @@ -120,6 +121,7 @@ static int hash__init_new_context(struct mm_struct *mm)subpage_prot_init_new_context (mm); +pkey_mm_init (mm);return index; }arch/powerpc/mm/pkeys.c +0
-0
arch/powerpc/mm/pkeys.c line 20
Quoted 5 lines
arch/powerpc/mm/pkeys.c +2
-0@@ -16,3 +16,5 @@#include <linux/pkeys.h> /* PKEY_* */ bool pkey_inited; +int pkeys_total;/* total pkeys as per device tree */ +u32 initial_allocation_mask;/* bits set for reserved keys */
Balbir
Re: [RFC v7 02/25] powerpc: track allocation status of all pkeys
Quoted 98 lines · Balbir Singh
Quoted 95 lines · On Sun, 30 Jul 2017 17:12:03 -0700 Ram Pai <[email protected]> wrote:
Total 32 keys are available on power7 and above. However pkey 0,1 are reserved. So effectively we have 30 pkeys.
On 4K kernels, we do not have 5 bits in the PTE to represent all the keys; we only have 3bits.Two of those keys are reserved; pkey 0 and pkey 1. So effectively we have 6 pkeys.
This patch keeps track of reserved keys, allocated keys and keys that are currently free.
Also it adds skeletal functions and macros, that the architecture-independent code expects to be available.
Signed-off-by Ram Pai <[email protected]>--- arch/powerpc/include/asm/book3s/64/mmu.h | 9 +++ arch/powerpc/include/asm/mmu_context.h | 1 + arch/powerpc/include/asm/pkeys.h | 98 ++++++++++++++++++++++++++++- arch/powerpc/mm/mmu_context_book3s64.c | 2 + arch/powerpc/mm/pkeys.c | 2 + 5 files changed, 108 insertions(+), 4 deletions(-)arch/powerpc/include/asm/book3s/64/mmu.h +9
-0@@ -108,6 +108,15 @@ struct patb_entry {#ifdef CONFIG_SPAPR_TCE_IOMMU struct list_head iommu_group_mem_list;#endif + +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +/* +* Each bit represents one protection key. +* bit set -> key allocated +* bit unset -> key available for allocation +*/ +u32 pkey_allocation_map; +#endif } mm_context_t;/* arch/powerpc/include/asm/mmu_context.h +1
-0@@ -184,6 +184,7 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,#ifndef CONFIG_PPC64_MEMORY_PROTECTION_KEYS #define pkey_initialize () +#define pkey_mm_init (mm)#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ #endif /* __KERNEL__ */ arch/powerpc/include/asm/pkeys.h +23
-1@@ -2,6 +2,8 @@#define _ASM_PPC64_PKEYS_Hextern bool pkey_inited; +extern int pkeys_total;/* total pkeys as per device tree */ +extern u32 initial_allocation_mask;/* bits set for reserved keys */ /* * powerpc needs an additional vma bit to support 32 keys. @@ -20,21 +22,76 @@#define VM_PKEY_BIT4 VM_HIGH_ARCH_4#endif -#define ARCH_VM_PKEY_FLAGS 0 +#define arch_max_pkey () pkeys_total +#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \ + VM_PKEY_BIT3 | VM_PKEY_BIT4) + +#define pkey_alloc_mask (pkey) (0x1 << pkey) + +#define mm_pkey_allocation_map (mm) (mm->context.pkey_allocation_map) + +#define mm_set_pkey_allocated (mm, pkey) { \ + mm_pkey_allocation_map(mm) |= pkey_alloc_mask(pkey); \ +} + +#define mm_set_pkey_free (mm, pkey) { \ + mm_pkey_allocation_map(mm) &= ~pkey_alloc_mask(pkey); \ +} + +#define mm_set_pkey_is_allocated (mm, pkey) \ + (mm_pkey_allocation_map(mm) & pkey_alloc_mask(pkey)) + +#define mm_set_pkey_is_reserved (mm, pkey) (initial_allocation_mask & \ + pkey_alloc_mask(pkey))
Quoted 9 lines
arch/powerpc/include/asm/pkeys.h line 52
Quoted 7 lines
arch/powerpc/include/asm/pkeys.h +4
-1static inline bool mm_pkey_is_allocated (struct mm_struct *mm,int pkey) { -return (pkey ==0 ); +/* a reserved key is never considered as 'explicitly allocated' */ +return ((pkey <arch_max_pkey ()) && + !mm_set_pkey_is_reserved (mm, pkey) && +mm_set_pkey_is_allocated (mm, pkey));Sounds like this should be called
mm_pkey_is_free()
hmm. why?
Quoted 27 lines
Quoted 15 lines
arch/powerpc/include/asm/pkeys.h +9
-1} +/* +* Returns a positive, 5-bit key on success, or -1 on failure. +*/ static inline int mm_pkey_alloc (struct mm_struct *mm) { -return -1 ; +/* +* Note: this is the one and only place we make sure +* that the pkey is valid as far as the hardware is +* concerned. The rest of the kernel trusts that +* only good, valid pkeys come out of here. +*/ arch/powerpc/include/asm/pkeys.h line 75
Quoted 10 lines
arch/powerpc/include/asm/pkeys.h +10
-0+u32 all_pkeys_mask = (u32)(~(0x0 )); +int ret; + +if (!pkey_inited) +return -1 ; +/* +* Are we out of pkeys? We must handle this specially +* because ffz() behavior is undefined if there are no +* zeros. +*/ Point A
Quoted 8 lines
arch/powerpc/include/asm/pkeys.h line 79
Quoted 4 lines
arch/powerpc/include/asm/pkeys.h +4
-0+if (mm_pkey_allocation_map (mm) == all_pkeys_mask) +return -1 ; + + ret =ffz ((u32 )mm_pkey_allocation_map (mm));Point B
So the allocation occurs from MSB to LSB?
ffz() allocates bits from right to left. On BE systems it will be from MSB to LSB, and on LE systems it will be from LSB to MSB. right?
Quoted 4 lines
I also think you need
a preempt disable between points A, B.Is this code reentrant?
mm_pkey_alloc() is called holding the mmap_sem. So it cannot race with itself.
Quoted 34 lines
Quoted 34 lines
arch/powerpc/include/asm/pkeys.h +19
-1+mm_set_pkey_allocated (mm, ret); +return ret; }static inline int mm_pkey_free (struct mm_struct *mm,int pkey) { -return -EINVAL ; +if (!pkey_inited) +return -1 ; + +if (!mm_pkey_is_allocated (mm, pkey)) +return -EINVAL ; + +mm_set_pkey_free (mm, pkey); + +return 0 ; }/* @@ -58,12 +115,45 @@ static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,return 0 ; } +static inline void pkey_mm_init (struct mm_struct *mm) +{ +if (!pkey_inited) +return ; +mm_pkey_allocation_map (mm) = initial_allocation_mask; +} +static inline void pkey_initialize (void ) { +int os_reserved, i;
Quoted 8 lines
arch/powerpc/include/asm/pkeys.h static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey, · line 133
Quoted 6 lines
arch/powerpc/include/asm/pkeys.h +1
-0+/* disable the pkey system till everything * is in place. A patch further down the * line will enable it. */ pkey_inited = false;Why are we doing it this way?
many factors .... led to this organization. It could have been done differently, i suppose. but it organically grew to this state.
Running down the memory lane, i recall, I had to introduce many skeletal functions as soon as the PROTECTION_KEY configuration option got enabled. The arch neutral code called into them, and since these skeletal function did nothing, bad thing could happen. So had to disable everything till everything was in place.
Quoted 22 lines
Quoted 22 lines
arch/powerpc/include/asm/pkeys.h +21
-0+ +/* Lets assume 32 keys */ + pkeys_total =32 ; + +#ifdef CONFIG_PPC_4K_PAGES +/* +* the OS can manage only 8 pkeys +* due to its inability to represent +* them in the linux 4K-PTE. +*/ + os_reserved = pkeys_total-8 ; +#else + os_reserved =0 ; +#endif +/* +* Bits are in LE format. +* NOTE: 1, 0 are reserved. +* key 0 is the default key, which allows read/write/execute. +* key 1 is recommended not to be used. +* PowerISA(3.0) page 1015, programming note. +*/
Quoted 8 lines
arch/powerpc/include/asm/pkeys.h static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey, · line 157
Quoted 3 lines
arch/powerpc/include/asm/pkeys.h +3
-0+ initial_allocation_mask = ~0x0 ; +for (i =2 ; i < (pkeys_total - os_reserved); i++) + initial_allocation_mask &= ~(0x1 <<i);On radix key 0 is used for supervisor mode to implement SMEP and SMAP. It would be nice to reserve key 0 for the OS for hash as well. Why are the bits in LE format? The limitations of the adjunct partition apply to Linux?
We dont allocate Key-0 to the application. Key-0 is considered as reserved.
Quoted 30 lines
Quoted 30 lines
arch/powerpc/include/asm/pkeys.h +0
-0}#endif /*_ASM_PPC64_PKEYS_H */ arch/powerpc/mm/mmu_context_book3s64.c +2
-0@@ -16,6 +16,7 @@#include <linux/string.h> #include <linux/types.h> #include <linux/mm.h> +#include <linux/pkeys.h> #include <linux/spinlock.h> #include <linux/idr.h> #include <linux/export.h> @@ -120,6 +121,7 @@ static int hash__init_new_context(struct mm_struct *mm)subpage_prot_init_new_context (mm); +pkey_mm_init (mm);return index; }arch/powerpc/mm/pkeys.c +0
-0@@ -16,3 +16,5 @@#include <linux/pkeys.h> /* PKEY_* */
Quoted 3 lines
arch/powerpc/mm/pkeys.c line 20
Quoted 3 lines
arch/powerpc/mm/pkeys.c +2
-0bool pkey_inited; +int pkeys_total;/* total pkeys as per device tree */ +u32 initial_allocation_mask;/* bits set for reserved keys */
Thanks for the review.
RP
Re: [RFC v7 02/25] powerpc: track allocation status of all pkeys
In reply to Ram PaiHi Ram,
Quoted 7 lines · Ram Pai
Total 32 keys are available on power7 and above. However pkey 0,1 are reserved. So effectively we have 30 pkeys.
On 4K kernels, we do not have 5 bits in the PTE to represent all the keys; we only have 3bits.Two of those keys are reserved; pkey 0 and pkey 1. So effectively we have 6 pkeys.
IIUC, the pkey 0 and 1 are reserved by the hardware, and the kernel PTE has only 5 bits to keep track of the pkey. Why hw pkey 0 and 1 has to be represented in the kernel PTE ?
Quoted 76 lines
This patch keeps track of reserved keys, allocated keys and keys that are currently free.
Also it adds skeletal functions and macros, that the architecture-independent code expects to be available.
Signed-off-by Ram Pai <[email protected]>--- arch/powerpc/include/asm/book3s/64/mmu.h | 9 +++ arch/powerpc/include/asm/mmu_context.h | 1 + arch/powerpc/include/asm/pkeys.h | 98 ++++++++++++++++++++++++++++- arch/powerpc/mm/mmu_context_book3s64.c | 2 + arch/powerpc/mm/pkeys.c | 2 + 5 files changed, 108 insertions(+), 4 deletions(-)arch/powerpc/include/asm/book3s/64/mmu.h +9
-0@@ -108,6 +108,15 @@ struct patb_entry {#ifdef CONFIG_SPAPR_TCE_IOMMU struct list_head iommu_group_mem_list;#endif + +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +/* +* Each bit represents one protection key. +* bit set -> key allocated +* bit unset -> key available for allocation +*/ +u32 pkey_allocation_map; +#endif } mm_context_t;/* arch/powerpc/include/asm/mmu_context.h +1
-0@@ -184,6 +184,7 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,#ifndef CONFIG_PPC64_MEMORY_PROTECTION_KEYS #define pkey_initialize () +#define pkey_mm_init (mm)#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ #endif /* __KERNEL__ */ arch/powerpc/include/asm/pkeys.h +13
-1@@ -2,6 +2,8 @@#define _ASM_PPC64_PKEYS_Hextern bool pkey_inited; +extern int pkeys_total;/* total pkeys as per device tree */ +extern u32 initial_allocation_mask;/* bits set for reserved keys */ /* * powerpc needs an additional vma bit to support 32 keys. @@ -20,21 +22,76 @@#define VM_PKEY_BIT4 VM_HIGH_ARCH_4#endif -#define ARCH_VM_PKEY_FLAGS 0 +#define arch_max_pkey () pkeys_total +#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \ + VM_PKEY_BIT3 | VM_PKEY_BIT4) + +#define pkey_alloc_mask (pkey) (0x1 << pkey) + +#define mm_pkey_allocation_map (mm) (mm->context.pkey_allocation_map) + +#define mm_set_pkey_allocated (mm, pkey) { \ + mm_pkey_allocation_map(mm) |= pkey_alloc_mask(pkey); \ +}
arch/powerpc/include/asm/pkeys.h line 45
Quoted 10 lines
arch/powerpc/include/asm/pkeys.h +10
-0+ +#define mm_set_pkey_free (mm, pkey) { \ + mm_pkey_allocation_map(mm) &= ~pkey_alloc_mask(pkey); \ +} + +#define mm_set_pkey_is_allocated (mm, pkey) \ + (mm_pkey_allocation_map(mm) & pkey_alloc_mask(pkey)) + +#define mm_set_pkey_is_reserved (mm, pkey) (initial_allocation_mask & \ + pkey_alloc_mask(pkey))
This macro doesn't need a 'mm' argument.
arch/powerpc/include/asm/pkeys.h line 56
Quoted 11 lines
arch/powerpc/include/asm/pkeys.h +6
-1static inline bool mm_pkey_is_allocated (struct mm_struct *mm,int pkey) { -return (pkey ==0 ); +/* a reserved key is never considered as 'explicitly allocated' */ +return ((pkey <arch_max_pkey ()) && + !mm_set_pkey_is_reserved (mm, pkey) && +mm_set_pkey_is_allocated (mm, pkey)); } +/* +* Returns a positive, 5-bit key on success, or -1 on failure.
I guess you rely on the mmap_sem to protect against concurrency in mm_pkey_alloc() and mm_pkey_free().
As this is not explicit in the code, it should at least be mentioned in the comment describing the function.
Quoted 120 lines
arch/powerpc/include/asm/pkeys.h +65
-2+*/ static inline int mm_pkey_alloc (struct mm_struct *mm) { -return -1 ; +/* +* Note: this is the one and only place we make sure +* that the pkey is valid as far as the hardware is +* concerned. The rest of the kernel trusts that +* only good, valid pkeys come out of here. +*/ +u32 all_pkeys_mask = (u32)(~(0x0 )); +int ret; + +if (!pkey_inited) +return -1 ; +/* +* Are we out of pkeys? We must handle this specially +* because ffz() behavior is undefined if there are no +* zeros. +*/ +if (mm_pkey_allocation_map (mm) == all_pkeys_mask) +return -1 ; + + ret =ffz ((u32 )mm_pkey_allocation_map (mm)); +mm_set_pkey_allocated (mm, ret); +return ret; }static inline int mm_pkey_free (struct mm_struct *mm,int pkey) { -return -EINVAL ; +if (!pkey_inited) +return -1 ; + +if (!mm_pkey_is_allocated (mm, pkey)) +return -EINVAL ; + +mm_set_pkey_free (mm, pkey); + +return 0 ; }/* @@ -58,12 +115,45 @@ static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey, return 0; } +static inline void pkey_mm_init (struct mm_struct *mm) +{ +if (!pkey_inited) +return ; +mm_pkey_allocation_map (mm) = initial_allocation_mask; +} +static inline void pkey_initialize (void ) { +int os_reserved, i; +/* disable the pkey system till everything * is in place. A patch further down the * line will enable it. */ pkey_inited = false; + +/* Lets assume 32 keys */ + pkeys_total =32 ; + +#ifdef CONFIG_PPC_4K_PAGES +/* +* the OS can manage only 8 pkeys +* due to its inability to represent +* them in the linux 4K-PTE. +*/ + os_reserved = pkeys_total-8 ; +#else + os_reserved =0 ; +#endif +/* +* Bits are in LE format. +* NOTE: 1, 0 are reserved. +* key 0 is the default key, which allows read/write/execute. +* key 1 is recommended not to be used. +* PowerISA(3.0) page 1015, programming note. +*/ + initial_allocation_mask = ~0x0 ; +for (i =2 ; i < (pkeys_total - os_reserved); i++) + initial_allocation_mask &= ~(0x1 <<i); }#endif /*_ASM_PPC64_PKEYS_H */ arch/powerpc/mm/mmu_context_book3s64.c +2
-0@@ -16,6 +16,7 @@#include <linux/string.h> #include <linux/types.h> #include <linux/mm.h> +#include <linux/pkeys.h> #include <linux/spinlock.h> #include <linux/idr.h> #include <linux/export.h> @@ -120,6 +121,7 @@ static int hash__init_new_context(struct mm_struct *mm)subpage_prot_init_new_context (mm); +pkey_mm_init (mm);return index; }arch/powerpc/mm/pkeys.c +2
-0@@ -16,3 +16,5 @@#include <linux/pkeys.h> /* PKEY_* */ bool pkey_inited; +int pkeys_total;/* total pkeys as per device tree */ +u32 initial_allocation_mask;/* bits set for reserved keys */
Re: [RFC v7 02/25] powerpc: track allocation status of all pkeys
Quoted 6 lines · Laurent Dufour
Hi Ram,
Quoted 3 lines · Ram Pai
Total 32 keys are available on power7 and above. However pkey 0,1 are reserved. So effectively we have 30 pkeys.
Quoted 8 lines
Quoted 4 lines
On 4K kernels, we do not have 5 bits in the PTE to represent all the keys; we only have 3bits.Two of those keys are reserved; pkey 0 and pkey 1. So effectively we have 6 pkeys.
IIUC, the pkey 0 and 1 are reserved by the hardware, and the kernel PTE has only 5 bits to keep track of the pkey. Why hw pkey 0 and 1 has to be represented in the kernel PTE ?
Key 0 is the default key. It is reserved in the sense, it cannot be allocated or freed. but its there and will be used as the default. So when no key is associated with a pte, it is infact using key 0.
Good question for the hardware designers. :)
key 1 is suggested not be used because it can be used by hypervisor (powervm) or something else. Since linux kernel does not use it, we dont program the key in the pte.
Quoted 99 lines
Quoted 77 lines
This patch keeps track of reserved keys, allocated keys and keys that are currently free.
Also it adds skeletal functions and macros, that the architecture-independent code expects to be available.
Signed-off-by Ram Pai <[email protected]>--- arch/powerpc/include/asm/book3s/64/mmu.h | 9 +++ arch/powerpc/include/asm/mmu_context.h | 1 + arch/powerpc/include/asm/pkeys.h | 98 ++++++++++++++++++++++++++++- arch/powerpc/mm/mmu_context_book3s64.c | 2 + arch/powerpc/mm/pkeys.c | 2 + 5 files changed, 108 insertions(+), 4 deletions(-)arch/powerpc/include/asm/book3s/64/mmu.h +9
-0@@ -108,6 +108,15 @@ struct patb_entry {#ifdef CONFIG_SPAPR_TCE_IOMMU struct list_head iommu_group_mem_list;#endif + +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +/* +* Each bit represents one protection key. +* bit set -> key allocated +* bit unset -> key available for allocation +*/ +u32 pkey_allocation_map; +#endif } mm_context_t;/* arch/powerpc/include/asm/mmu_context.h +1
-0@@ -184,6 +184,7 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,#ifndef CONFIG_PPC64_MEMORY_PROTECTION_KEYS #define pkey_initialize () +#define pkey_mm_init (mm)#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ #endif /* __KERNEL__ */ arch/powerpc/include/asm/pkeys.h +13
-1@@ -2,6 +2,8 @@#define _ASM_PPC64_PKEYS_Hextern bool pkey_inited; +extern int pkeys_total;/* total pkeys as per device tree */ +extern u32 initial_allocation_mask;/* bits set for reserved keys */ /* * powerpc needs an additional vma bit to support 32 keys. @@ -20,21 +22,76 @@#define VM_PKEY_BIT4 VM_HIGH_ARCH_4#endif -#define ARCH_VM_PKEY_FLAGS 0 +#define arch_max_pkey () pkeys_total +#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \ + VM_PKEY_BIT3 | VM_PKEY_BIT4) + +#define pkey_alloc_mask (pkey) (0x1 << pkey) + +#define mm_pkey_allocation_map (mm) (mm->context.pkey_allocation_map) + +#define mm_set_pkey_allocated (mm, pkey) { \ + mm_pkey_allocation_map(mm) |= pkey_alloc_mask(pkey); \ +}arch/powerpc/include/asm/pkeys.h line 45
Quoted 10 lines
arch/powerpc/include/asm/pkeys.h +10
-0+ +#define mm_set_pkey_free (mm, pkey) { \ + mm_pkey_allocation_map(mm) &= ~pkey_alloc_mask(pkey); \ +} + +#define mm_set_pkey_is_allocated (mm, pkey) \ + (mm_pkey_allocation_map(mm) & pkey_alloc_mask(pkey)) + +#define mm_set_pkey_is_reserved (mm, pkey) (initial_allocation_mask & \ + pkey_alloc_mask(pkey))This macro doesn't need a 'mm' argument.
Quoted 9 lines
arch/powerpc/include/asm/pkeys.h +4
-1static inline bool mm_pkey_is_allocated (struct mm_struct *mm,int pkey) { -return (pkey ==0 ); +/* a reserved key is never considered as 'explicitly allocated' */ +return ((pkey <arch_max_pkey ()) && + !mm_set_pkey_is_reserved (mm, pkey) && +mm_set_pkey_is_allocated (mm, pkey)); }
Quoted 7 lines
arch/powerpc/include/asm/pkeys.h line 56
Quoted 2 lines
arch/powerpc/include/asm/pkeys.h +2
-0+/* +* Returns a positive, 5-bit key on success, or -1 on failure. I guess you rely on the
mmap_semto protect against concurrency inmm_pkey_alloc()andmm_pkey_free().
As this is not explicit in the code, it should at least be mentioned in the comment describing the function.
Yes. will do. good point.
Quoted 121 lines
Quoted 121 lines
arch/powerpc/include/asm/pkeys.h +65
-2+*/ static inline int mm_pkey_alloc (struct mm_struct *mm) { -return -1 ; +/* +* Note: this is the one and only place we make sure +* that the pkey is valid as far as the hardware is +* concerned. The rest of the kernel trusts that +* only good, valid pkeys come out of here. +*/ +u32 all_pkeys_mask = (u32)(~(0x0 )); +int ret; + +if (!pkey_inited) +return -1 ; +/* +* Are we out of pkeys? We must handle this specially +* because ffz() behavior is undefined if there are no +* zeros. +*/ +if (mm_pkey_allocation_map (mm) == all_pkeys_mask) +return -1 ; + + ret =ffz ((u32 )mm_pkey_allocation_map (mm)); +mm_set_pkey_allocated (mm, ret); +return ret; }static inline int mm_pkey_free (struct mm_struct *mm,int pkey) { -return -EINVAL ; +if (!pkey_inited) +return -1 ; + +if (!mm_pkey_is_allocated (mm, pkey)) +return -EINVAL ; + +mm_set_pkey_free (mm, pkey); + +return 0 ; }/* @@ -58,12 +115,45 @@ static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey, return 0; } +static inline void pkey_mm_init (struct mm_struct *mm) +{ +if (!pkey_inited) +return ; +mm_pkey_allocation_map (mm) = initial_allocation_mask; +} +static inline void pkey_initialize (void ) { +int os_reserved, i; +/* disable the pkey system till everything * is in place. A patch further down the * line will enable it. */ pkey_inited = false; + +/* Lets assume 32 keys */ + pkeys_total =32 ; + +#ifdef CONFIG_PPC_4K_PAGES +/* +* the OS can manage only 8 pkeys +* due to its inability to represent +* them in the linux 4K-PTE. +*/ + os_reserved = pkeys_total-8 ; +#else + os_reserved =0 ; +#endif +/* +* Bits are in LE format. +* NOTE: 1, 0 are reserved. +* key 0 is the default key, which allows read/write/execute. +* key 1 is recommended not to be used. +* PowerISA(3.0) page 1015, programming note. +*/ + initial_allocation_mask = ~0x0 ; +for (i =2 ; i < (pkeys_total - os_reserved); i++) + initial_allocation_mask &= ~(0x1 <<i); }#endif /*_ASM_PPC64_PKEYS_H */ arch/powerpc/mm/mmu_context_book3s64.c +2
-0@@ -16,6 +16,7 @@#include <linux/string.h> #include <linux/types.h> #include <linux/mm.h> +#include <linux/pkeys.h> #include <linux/spinlock.h> #include <linux/idr.h> #include <linux/export.h> @@ -120,6 +121,7 @@ static int hash__init_new_context(struct mm_struct *mm)subpage_prot_init_new_context (mm); +pkey_mm_init (mm);return index; }arch/powerpc/mm/pkeys.c +2
-0@@ -16,3 +16,5 @@#include <linux/pkeys.h> /* PKEY_* */ bool pkey_inited; +int pkeys_total;/* total pkeys as per device tree */ +u32 initial_allocation_mask;/* bits set for reserved keys */
Signature · 1 line
Ram Pai
RFC v7 3/25 powerpc: helper function to read, write AMR, IAMR, UAMOR registers
In reply to Ram PaiImplements helper functions to read and write the key related registers; AMR, IAMR, UAMOR.
AMR register tracks the read,write permission of a key IAMR register tracks the execute permission of a key UAMOR register enables and disables a key
--- arch/powerpc/include/asm/book3s/64/pgtable.h | 26 ++++++++++++++++++++++++++ 1 files changed, 26 insertions(+), 0 deletions(-)
arch/powerpc/include/asm/book3s/64/pgtable.h +26 -0
@@ -428,6 +428,32 @@ static inline void huge_ptep_set_wrprotect(struct mm_struct *mm,pte_update (mm, addr, ptep,0 , _PAGE_PRIVILEGED,1 ); } +#include <asm/reg.h> +static inline u64 read_amr (void ) +{ +return mfspr (SPRN_AMR ); +} +static inline void write_amr (u64 value) +{ +mtspr (SPRN_AMR , value); +} +static inline u64 read_iamr (void ) +{ +return mfspr (SPRN_IAMR ); +} +static inline void write_iamr (u64 value) +{ +mtspr (SPRN_IAMR , value); +} +static inline u64 read_uamor (void ) +{ +return mfspr (SPRN_UAMOR ); +} +static inline void write_uamor (u64 value) +{ +mtspr (SPRN_UAMOR , value); +} +#define __HAVE_ARCH_PTEP_GET_AND_CLEARstatic inline pte_t ptep_get_and_clear (struct mm_struct *mm,unsigned long addr,pte_t *ptep)
Signature · 1 line
1.7.1
RFC v7 4/25 powerpc: helper functions to initialize AMR, IAMR and UAMOR registers
In reply to Ram PaiIntroduce helper functions that can initialize the bits in the AMR, IAMR and UAMOR register; the bits that correspond to the given pkey.
--- arch/powerpc/include/asm/pkeys.h | 1 + arch/powerpc/mm/pkeys.c | 46 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 0 deletions(-)
arch/powerpc/include/asm/pkeys.h +1 -0
@@ -25,6 +25,7 @@#define arch_max_pkey () pkeys_total#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \ VM_PKEY_BIT3 | VM_PKEY_BIT4) +#define AMR_BITS_PER_PKEY 2#define pkey_alloc_mask (pkey) (0x1 << pkey)
arch/powerpc/mm/pkeys.c +46 -0
49 lines not on this page · raw message
Signature · 1 line
1.7.1
RFC v7 5/25 powerpc: cleaup AMR, iAMR when a key is allocated or freed
In reply to Ram Paicleanup the bits corresponding to a key in the AMR, and IAMR register, when the key is newly allocated/activated or is freed. We dont want some residual bits cause the hardware enforce unintended behavior when the key is activated or freed.
--- arch/powerpc/include/asm/pkeys.h | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)
arch/powerpc/include/asm/pkeys.h +12 -0
@@ -53,6 +53,8 @@ static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey) mm_set_pkey_is_allocated(mm, pkey)); } +extern void __arch_activate_pkey (int pkey); +extern void __arch_deactivate_pkey (int pkey);/* * Returns a positive, 5-bit key on success, or -1 on failure. */
@@ -79,6 +81,12 @@ static inline int mm_pkey_alloc(struct mm_struct *mm) ret =ffz ((u32 )mm_pkey_allocation_map (mm));mm_set_pkey_allocated (mm, ret); + +/* +* enable the key in the hardware +*/ +if (ret >0 ) +__arch_activate_pkey (ret);return ret; }
@@ -90,6 +98,10 @@ static inline int mm_pkey_free(struct mm_struct *mm, int pkey)if (!mm_pkey_is_allocated (mm, pkey))return -EINVAL ; +/* +* Disable the key in the hardware +*/ +__arch_deactivate_pkey (pkey);mm_set_pkey_free (mm, pkey);return 0 ;
Signature · 1 line
1.7.1
RFC v7 6/25 powerpc: implementation for arch_set_user_pkey_access()
In reply to Ram PaiThis patch provides the detailed implementation for a user to allocate a key and enable it in the hardware.
It provides the plumbing, but it cannot be used till the system call is implemented. The next patch will do so.
--- arch/powerpc/include/asm/pkeys.h | 9 ++++++++- arch/powerpc/mm/pkeys.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletions(-)
arch/powerpc/include/asm/pkeys.h +8 -1
@@ -23,6 +23,9 @@#endif #define arch_max_pkey () pkeys_total +#define AMR_RD_BIT 0x1UL +#define AMR_WR_BIT 0x2UL +#define IAMR_EX_BIT 0x1UL#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \ VM_PKEY_BIT3 | VM_PKEY_BIT4)#define AMR_BITS_PER_PKEY 2
@@ -122,10 +125,14 @@ static inline int arch_override_mprotect_pkey(struct vm_area_struct *vma,return 0 ; } +extern int __arch_set_user_pkey_access (struct task_struct *tsk,int pkey, +unsigned long init_val);static inline int arch_set_user_pkey_access (struct task_struct *tsk,int pkey,unsigned long init_val) { -return 0 ; +if (!pkey_inited) +return -EINVAL ; +return __arch_set_user_pkey_access (tsk, pkey, init_val); }static inline void pkey_mm_init (struct mm_struct *mm)
arch/powerpc/mm/pkeys.c +28 -0
@@ -22,6 +22,11 @@#define PKEY_REG_BITS (sizeof(u64)*8)#define pkeyshift (pkey) (PKEY_REG_BITS - ((pkey+1) * AMR_BITS_PER_PKEY)) +static bool is_pkey_enabled (int pkey) +{ +return !!(read_uamor () & (0x3ul <<pkeyshift (pkey))); +} +static inline void init_amr (int pkey,u8 init_bits) {u64 new_amr_bits = (((u64 )init_bits &0x3UL ) <<pkeyshift (pkey));
@@ -64,3 +69,26 @@ void __arch_deactivate_pkey(int pkey) {pkey_status_change (pkey, false); } + +/* +* set the access right in AMR IAMR and UAMOR register +* for @pkey to that specified in @init_val. +*/ +int __arch_set_user_pkey_access (struct task_struct *tsk,int pkey, +unsigned long init_val) +{ +u64 new_amr_bits =0x0ul ; + +if (!is_pkey_enabled (pkey)) +return -EINVAL ; + +/* Set the bits we need in AMR: */ +if (init_val &PKEY_DISABLE_ACCESS ) + new_amr_bits |=AMR_RD_BIT |AMR_WR_BIT ; +else if (init_val &PKEY_DISABLE_WRITE ) + new_amr_bits |=AMR_WR_BIT ; + +init_amr (pkey, new_amr_bits); + +return 0 ; +}
Signature · 1 line
1.7.1
RFC v7 7/25 powerpc: sys_pkey_alloc() and sys_pkey_free() system calls
In reply to Ram PaiFinally this patch provides the ability for a process to allocate and free a protection key.
--- arch/powerpc/include/asm/systbl.h | 2 ++ arch/powerpc/include/asm/unistd.h | 4 +--- arch/powerpc/include/uapi/asm/unistd.h | 2 ++ 3 files changed, 5 insertions(+), 3 deletions(-)
arch/powerpc/include/asm/systbl.h +2 -0
@@ -388,3 +388,5 @@ COMPAT_SYS_SPU(pwritev2) SYSCALL(kexec_file_load) SYSCALL(statx) +SYSCALL(pkey_alloc) +SYSCALL(pkey_free)
arch/powerpc/include/asm/unistd.h +1 -3
@@ -12,13 +12,11 @@#include <uapi/asm/unistd.h> -#define NR_syscalls 384 +#define NR_syscalls 386#define __NR__exit __NR_exit#define __IGNORE_pkey_mprotect -#define __IGNORE_pkey_alloc -#define __IGNORE_pkey_free#ifndef __ASSEMBLY__
arch/powerpc/include/uapi/asm/unistd.h +2 -0
@@ -394,5 +394,7 @@#define __NR_pwritev2 381#define __NR_kexec_file_load 382#define __NR_statx 383 +#define __NR_pkey_alloc 384 +#define __NR_pkey_free 385#endif /* _UAPI_ASM_POWERPC_UNISTD_H_ */
Signature · 1 line
1.7.1
RFC v7 8/25 powerpc: ability to create execute-disabled pkeys
In reply to Ram Paipowerpc has hardware support to disable execute on a pkey. This patch enables the ability to create execute-disabled keys.
--- arch/powerpc/include/asm/pkeys.h | 12 ++++++++++++ arch/powerpc/mm/pkeys.c | 5 +++++ 2 files changed, 17 insertions(+), 0 deletions(-)
arch/powerpc/include/asm/pkeys.h +12 -0
@@ -22,6 +22,18 @@#define VM_PKEY_BIT4 VM_HIGH_ARCH_4#endif +/* override any generic PKEY Permission defines */ +#undef PKEY_DISABLE_ACCESS +#define PKEY_DISABLE_ACCESS 0x1 +#undef PKEY_DISABLE_WRITE +#define PKEY_DISABLE_WRITE 0x2 +#undef PKEY_DISABLE_EXECUTE +#define PKEY_DISABLE_EXECUTE 0x4 +#undef PKEY_ACCESS_MASK +#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\ + PKEY_DISABLE_WRITE |\ + PKEY_DISABLE_EXECUTE) +#define arch_max_pkey () pkeys_total#define AMR_RD_BIT 0x1UL#define AMR_WR_BIT 0x2UL
arch/powerpc/mm/pkeys.c +5 -0
@@ -78,6 +78,7 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey, unsigned long init_val) {u64 new_amr_bits =0x0ul ; +u64 new_iamr_bits =0x0ul ;if (!is_pkey_enabled (pkey))return -EINVAL ;
@@ -90,5 +91,9 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,init_amr (pkey, new_amr_bits); +if ((init_val &PKEY_DISABLE_EXECUTE )) + new_iamr_bits |=IAMR_EX_BIT ; + +init_iamr (pkey, new_iamr_bits);return 0 ; }
Signature · 1 line
1.7.1
RFC v7 9/25 powerpc: store and restore the pkey state across context switches
In reply to Ram PaiStore and restore the AMR, IAMR and UAMOR register state of the task before scheduling out and after scheduling in, respectively.
--- arch/powerpc/include/asm/pkeys.h | 5 +++++ arch/powerpc/include/asm/processor.h | 5 +++++ arch/powerpc/kernel/process.c | 25 +++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 0 deletions(-)
arch/powerpc/include/asm/pkeys.h +5 -0
@@ -147,6 +147,11 @@ static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,return __arch_set_user_pkey_access (tsk, pkey, init_val); } +static inline bool arch_pkeys_enabled (void ) +{ +return pkey_inited; +} +static inline void pkey_mm_init (struct mm_struct *mm) {if (!pkey_inited)
arch/powerpc/include/asm/processor.h +5 -0
@@ -309,6 +309,11 @@ struct thread_struct {struct thread_vr_state ckvr_state;/* Checkpointed VR state */ unsigned long ckvrsave;/* Checkpointed VRSAVE */ #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */ +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +unsigned long amr; +unsigned long iamr; +unsigned long uamor; +#endif #ifdef CONFIG_KVM_BOOK3S_32_HANDLER void * kvm_shadow_vcpu;/* KVM internal data */ #endif /* CONFIG_KVM_BOOK3S_32_HANDLER */
arch/powerpc/kernel/process.c +25 -0
@@ -42,6 +42,7 @@#include <linux/hw_breakpoint.h> #include <linux/uaccess.h> #include <linux/elf-randomize.h> +#include <linux/pkeys.h> #include <asm/pgtable.h> #include <asm/io.h>
@@ -1096,6 +1097,13 @@ static inline void save_sprs(struct thread_struct *t) t->tar =mfspr (SPRN_TAR ); }#endif +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +if (arch_pkeys_enabled ()) { + t->amr =mfspr (SPRN_AMR ); + t->iamr =mfspr (SPRN_IAMR ); + t->uamor =mfspr (SPRN_UAMOR ); + } +#endif } static inline void restore_sprs(struct thread_struct *old_thread,
@@ -1131,6 +1139,16 @@ static inline void restore_sprs(struct thread_struct *old_thread,mtspr (SPRN_TAR , new_thread->tar); }#endif +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +if (arch_pkeys_enabled ()) { +if (old_thread->amr != new_thread->amr) +mtspr (SPRN_AMR , new_thread->amr); +if (old_thread->iamr != new_thread->iamr) +mtspr (SPRN_IAMR , new_thread->iamr); +if (old_thread->uamor != new_thread->uamor) +mtspr (SPRN_UAMOR , new_thread->uamor); + } +#endif } struct task_struct *__switch_to(struct task_struct *prev,
@@ -1689,6 +1707,13 @@ void start_thread(struct pt_regs *regs, unsigned long start, unsigned long sp) current->thread.tm_tfiar =0 ; current->thread.load_tm =0 ;#endif /* CONFIG_PPC_TRANSACTIONAL_MEM */ +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +if (arch_pkeys_enabled ()) { + current->thread.amr =0x0ul ; + current->thread.iamr =0x0ul ; + current->thread.uamor =0x0ul ; + } +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ }EXPORT_SYMBOL (start_thread);
Signature · 1 line
1.7.1
Re: [RFC v7 09/25] powerpc: store and restore the pkey state across context switches
Quoted 12 lines · Ram Pai
arch/powerpc/kernel/process.c +1
-0@@ -42,6 +42,7 @@#include <linux/hw_breakpoint.h> #include <linux/uaccess.h> #include <linux/elf-randomize.h> +#include <linux/pkeys.h> #include <asm/pgtable.h> #include <asm/io.h> @@ -1096,6 +1097,13 @@ static inline void save_sprs(struct thread_struct *t) t->tar =mfspr (SPRN_TAR );
arch/powerpc/kernel/process.c static inline void save_sprs(struct thread_struct *t) · line 1107
Quoted 10 lines
arch/powerpc/kernel/process.c +7
-0}#endif +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +if (arch_pkeys_enabled ()) { + t->amr =mfspr (SPRN_AMR ); + t->iamr =mfspr (SPRN_IAMR ); + t->uamor =mfspr (SPRN_UAMOR ); + } +#endif }
Is it worth having a flag in thread_struct saying whether it has every called pkey_alloc and only do the mfsprs if it did?
Quoted 30 lines
arch/powerpc/kernel/process.c +17
-0@@ -1131,6 +1139,16 @@ static inline void restore_sprs(struct thread_struct *old_thread,mtspr (SPRN_TAR , new_thread->tar); }#endif +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +if (arch_pkeys_enabled ()) { +if (old_thread->amr != new_thread->amr) +mtspr (SPRN_AMR , new_thread->amr); +if (old_thread->iamr != new_thread->iamr) +mtspr (SPRN_IAMR , new_thread->iamr); +if (old_thread->uamor != new_thread->uamor) +mtspr (SPRN_UAMOR , new_thread->uamor); + } +#endif } struct task_struct *__switch_to(struct task_struct *prev,@@ -1689,6 +1707,13 @@ void start_thread(struct pt_regs *regs, unsigned long start, unsigned long sp) current->thread.tm_tfiar =0 ; current->thread.load_tm =0 ;#endif /* CONFIG_PPC_TRANSACTIONAL_MEM */ +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +if (arch_pkeys_enabled ()) { + current->thread.amr =0x0ul ; + current->thread.iamr =0x0ul ; + current->thread.uamor =0x0ul ; + } +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ }EXPORT_SYMBOL (start_thread);
Signature · 2 lines
Thiago Jung Bauermann IBM Linux Technology Center
Michael Ellerman
reviewerRe: [RFC v7 09/25] powerpc: store and restore the pkey state across context switches
Quoted 18 lines · Thiago Jung Bauermann
Quoted 17 lines · Ram Pai
arch/powerpc/kernel/process.c +4
-0@@ -42,6 +42,7 @@#include <linux/hw_breakpoint.h> #include <linux/uaccess.h> #include <linux/elf-randomize.h> +#include <linux/pkeys.h> #include <asm/pgtable.h> #include <asm/io.h> @@ -1096,6 +1097,13 @@ static inline void save_sprs(struct thread_struct *t) t->tar =mfspr (SPRN_TAR ); }#endif +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +if (arch_pkeys_enabled ()) { + t->amr =mfspr (SPRN_AMR );
Quoted 8 lines
arch/powerpc/kernel/process.c static inline void save_sprs(struct thread_struct *t) · line 1107
Quoted 5 lines
arch/powerpc/kernel/process.c +4
-0+ t->iamr =mfspr (SPRN_IAMR ); + t->uamor = mfspr(SPRN_UAMOR); + } +#endif }Is it worth having a flag in
thread_structsaying whether it has every calledpkey_allocand only do the mfsprs if it did?
Yes, in fact there's a programming note in the UAMOR section of the arch that says exactly that.
On the write side you have to be a bit more careful. You have to make sure you set the UAMOR to 0 when you're switching from a process that has used keys to one that isn't.
cheers
Re: [RFC v7 09/25] powerpc: store and restore the pkey state across context switches
Quoted 20 lines · Michael Ellerman
Quoted 18 lines · Thiago Jung Bauermann
Quoted 17 lines · Ram Pai
arch/powerpc/kernel/process.c +4
-0@@ -42,6 +42,7 @@#include <linux/hw_breakpoint.h> #include <linux/uaccess.h> #include <linux/elf-randomize.h> +#include <linux/pkeys.h> #include <asm/pgtable.h> #include <asm/io.h> @@ -1096,6 +1097,13 @@ static inline void save_sprs(struct thread_struct *t) t->tar =mfspr (SPRN_TAR ); }#endif +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +if (arch_pkeys_enabled ()) { + t->amr =mfspr (SPRN_AMR );
Quoted 8 lines
Quoted 8 lines
arch/powerpc/kernel/process.c static inline void save_sprs(struct thread_struct *t) · line 1107
Quoted 5 lines
arch/powerpc/kernel/process.c +4
-0+ t->iamr =mfspr (SPRN_IAMR ); + t->uamor = mfspr(SPRN_UAMOR); + } +#endif }Is it worth having a flag in
thread_structsaying whether it has every calledpkey_allocand only do the mfsprs if it did?
Yes. This will further optimize the code; a great thing!
Quoted 7 lines
Yes, in fact there's a programming note in the UAMOR section of the arch that says exactly that.
On the write side you have to be a bit more careful. You have to make sure you set the UAMOR to 0 when you're switching from a process that has used keys to one that isn't.
Currently we save and restore AMR/IAMR/UAMOR if the OS has enabled pkeys. This means the UAMOR will get restored to 0 if the application has not used any keys.
But if we do optimize the code further; as suggested by Thiago, we will have to be careful with initializing UAMOR while switching back task that has not used the keys yet.
RP
Quoted 2 lines
cheers
Signature · 1 line
Ram Pai
RFC v7 10/25 powerpc: introduce execute-only pkey
In reply to Ram PaiThis patch provides the implementation of execute-only pkey. The architecture-independent expects the ability to create and manage a special key which has execute-only permission.
--- arch/powerpc/include/asm/book3s/64/mmu.h | 1 + arch/powerpc/include/asm/pkeys.h | 8 ++++- arch/powerpc/mm/pkeys.c | 57 ++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 1 deletions(-)
arch/powerpc/include/asm/book3s/64/mmu.h +1 -0
@@ -116,6 +116,7 @@ struct patb_entry {* bit unset -> key available for allocation */ u32 pkey_allocation_map; +s16 execute_only_pkey;/* key holding execute-only protection */ #endif } mm_context_t;
arch/powerpc/include/asm/pkeys.h +7 -1
@@ -126,11 +126,15 @@ static inline int mm_pkey_free(struct mm_struct *mm, int pkey)* Try to dedicate one of the protection keys to be used as an * execute-only protection key. */ +extern int __execute_only_pkey (struct mm_struct *mm);static inline int execute_only_pkey (struct mm_struct *mm) { -return 0 ; +if (!pkey_inited) +return -1 ; +return __execute_only_pkey (mm); } +static inline int arch_override_mprotect_pkey (struct vm_area_struct *vma,int prot,int pkey) {
@@ -157,6 +161,8 @@ static inline void pkey_mm_init(struct mm_struct *mm)if (!pkey_inited)return ;mm_pkey_allocation_map (mm) = initial_allocation_mask; +/* -1 means unallocated or invalid */ + mm->context.execute_only_pkey =-1 ; }static inline void pkey_initialize (void )
arch/powerpc/mm/pkeys.c +57 -0
60 lines not on this page · raw message
Signature · 1 line
1.7.1
RFC v7 11/25 powerpc: ability to associate pkey to a vma
In reply to Ram Paiarch-independent code expects the arch to map
a pkey into the vma's protection bit setting.
The patch provides that ability.
--- arch/powerpc/include/asm/mman.h | 8 +++++++- arch/powerpc/include/asm/pkeys.h | 12 ++++++++++++ 2 files changed, 19 insertions(+), 1 deletions(-)
arch/powerpc/include/asm/mman.h +7 -1
@@ -13,6 +13,7 @@#include <asm/cputable.h> #include <linux/mm.h> +#include <linux/pkeys.h> #include <asm/cpu_has_feature.h> /*
@@ -22,7 +23,12 @@static inline unsigned long arch_calc_vm_prot_bits (unsigned long prot,unsigned long pkey) { -return (prot &PROT_SAO ) ?VM_SAO :0 ; +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +return (((prot &PROT_SAO ) ?VM_SAO :0 ) | +pkey_to_vmflag_bits (pkey)); +#else +return ((prot &PROT_SAO ) ?VM_SAO :0 ); +#endif }#define arch_calc_vm_prot_bits (prot, pkey) arch_calc_vm_prot_bits(prot, pkey)
arch/powerpc/include/asm/pkeys.h +12 -0
@@ -34,6 +34,18 @@ PKEY_DISABLE_WRITE |\ PKEY_DISABLE_EXECUTE) +static inline u64 pkey_to_vmflag_bits (u16 pkey) +{ +if (!pkey_inited) +return 0x0UL ; + +return (((pkey &0x1UL ) ?VM_PKEY_BIT0 :0x0UL ) | + ((pkey &0x2UL ) ?VM_PKEY_BIT1 :0x0UL ) | + ((pkey &0x4UL ) ?VM_PKEY_BIT2 :0x0UL ) | + ((pkey &0x8UL ) ?VM_PKEY_BIT3 :0x0UL ) | + ((pkey &0x10UL ) ?VM_PKEY_BIT4 :0x0UL )); +} +#define arch_max_pkey () pkeys_total#define AMR_RD_BIT 0x1UL#define AMR_WR_BIT 0x2UL
Signature · 1 line
1.7.1
RFC v7 12/25 powerpc: implementation for arch_override_mprotect_pkey()
In reply to Ram Paiarch independent code calls arch_override_mprotect_pkey() to return a pkey that best matches the requested protection.
This patch provides the implementation.
--- arch/powerpc/include/asm/mmu_context.h | 5 +++ arch/powerpc/include/asm/pkeys.h | 17 ++++++++++- arch/powerpc/mm/pkeys.c | 47 ++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 2 deletions(-)
arch/powerpc/include/asm/mmu_context.h +5 -0
@@ -185,6 +185,11 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,#ifndef CONFIG_PPC64_MEMORY_PROTECTION_KEYS #define pkey_initialize ()#define pkey_mm_init (mm) + +static inline int vma_pkey (struct vm_area_struct *vma) +{ +return 0 ; +}#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ #endif /* __KERNEL__ */
arch/powerpc/include/asm/pkeys.h +15 -2
@@ -46,6 +46,16 @@ static inline u64 pkey_to_vmflag_bits(u16 pkey) ((pkey & 0x10UL) ? VM_PKEY_BIT4 : 0x0UL)); } +#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \ + VM_PKEY_BIT3 | VM_PKEY_BIT4) + +static inline int vma_pkey (struct vm_area_struct *vma) +{ +if (!pkey_inited) +return 0 ; +return (vma->vm_flags &ARCH_VM_PKEY_FLAGS ) >>VM_PKEY_SHIFT ; +} +#define arch_max_pkey () pkeys_total#define AMR_RD_BIT 0x1UL#define AMR_WR_BIT 0x2UL
@@ -146,11 +156,14 @@ static inline int execute_only_pkey(struct mm_struct *mm)return __execute_only_pkey (mm); } - +extern int __arch_override_mprotect_pkey (struct vm_area_struct *vma, +int prot,int pkey);static inline int arch_override_mprotect_pkey (struct vm_area_struct *vma,int prot,int pkey) { -return 0 ; +if (!pkey_inited) +return 0 ; +return __arch_override_mprotect_pkey (vma, prot, pkey); } extern int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
arch/powerpc/mm/pkeys.c +47 -0
50 lines not on this page · raw message
Signature · 1 line
1.7.1
Re: [RFC v7 12/25] powerpc: implementation for arch_override_mprotect_pkey()
Hi Ram,
Quoted 86 lines · Ram Pai
arch independent code calls
arch_override_mprotect_pkey()to return a pkey that best matches the requested protection.This patch provides the implementation.
Signed-off-by Ram Pai <[email protected]>--- arch/powerpc/include/asm/mmu_context.h | 5 +++ arch/powerpc/include/asm/pkeys.h | 17 ++++++++++- arch/powerpc/mm/pkeys.c | 47 ++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 2 deletions(-)arch/powerpc/include/asm/mmu_context.h +5
-0@@ -185,6 +185,11 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,#ifndef CONFIG_PPC64_MEMORY_PROTECTION_KEYS #define pkey_initialize ()#define pkey_mm_init (mm) + +static inline int vma_pkey (struct vm_area_struct *vma) +{ +return 0 ; +}#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ #endif /* __KERNEL__ */ arch/powerpc/include/asm/pkeys.h +15
-2@@ -46,6 +46,16 @@ static inline u64 pkey_to_vmflag_bits(u16 pkey) ((pkey & 0x10UL) ? VM_PKEY_BIT4 : 0x0UL)); } +#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \ + VM_PKEY_BIT3 | VM_PKEY_BIT4) + +static inline int vma_pkey (struct vm_area_struct *vma) +{ +if (!pkey_inited) +return 0 ; +return (vma->vm_flags &ARCH_VM_PKEY_FLAGS ) >>VM_PKEY_SHIFT ; +} +#define arch_max_pkey () pkeys_total#define AMR_RD_BIT 0x1UL#define AMR_WR_BIT 0x2UL@@ -146,11 +156,14 @@ static inline int execute_only_pkey(struct mm_struct *mm)return __execute_only_pkey (mm); } - +extern int __arch_override_mprotect_pkey (struct vm_area_struct *vma, +int prot,int pkey);static inline int arch_override_mprotect_pkey (struct vm_area_struct *vma,int prot,int pkey) { -return 0 ; +if (!pkey_inited) +return 0 ; +return __arch_override_mprotect_pkey (vma, prot, pkey); } extern int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,arch/powerpc/mm/pkeys.c +12
-0@@ -154,3 +154,50 @@ int __execute_only_pkey(struct mm_struct *mm) mm->context.execute_only_pkey = execute_only_pkey;return execute_only_pkey; } + +static inline bool vma_is_pkey_exec_only (struct vm_area_struct *vma) +{ +/* Do this check first since the vm_flags should be hot */ +if ((vma->vm_flags & (VM_READ |VM_WRITE |VM_EXEC )) !=VM_EXEC ) +return false; + +return (vma_pkey (vma) == vma->vm_mm->context.execute_only_pkey); +} + +/* +* This should only be called for *plain* mprotect calls.
arch/powerpc/mm/pkeys.c int __execute_only_pkey(struct mm_struct *mm) · line 178
Quoted 10 lines
arch/powerpc/mm/pkeys.c +10
-0+*/ +int __arch_override_mprotect_pkey (struct vm_area_struct *vma,int prot, +int pkey) +{ +/* +* Is this an mprotect_pkey() call? If so, never +* override the value that came from the user. +*/ +if (pkey !=-1 ) +return pkey;
This check should be moved in arch_override_mprotect_pkey() in arch/powerpc/include/asm/pkeys.h
Quoted 26 lines
arch/powerpc/mm/pkeys.c +25
-0+ +/* +* If the currently associated pkey is execute-only, +* but the requested protection requires read or write, +* move it back to the default pkey. +*/ +if (vma_is_pkey_exec_only (vma) && + (prot & (PROT_READ |PROT_WRITE ))) +return 0 ; + +/* +* the requested protection is execute-only. Hence +* lets use a execute-only pkey. +*/ +if (prot ==PROT_EXEC ) { + pkey =execute_only_pkey (vma->vm_mm); +if (pkey >0 ) +return pkey; + } + +/* +* nothing to override. +*/ +return vma_pkey (vma); +}
Re: [RFC v7 12/25] powerpc: implementation for arch_override_mprotect_pkey()
Quoted 94 lines · Laurent Dufour
Hi Ram,
Quoted 91 lines · Ram Pai
arch independent code calls
arch_override_mprotect_pkey()to return a pkey that best matches the requested protection.This patch provides the implementation.
Signed-off-by Ram Pai <[email protected]>--- arch/powerpc/include/asm/mmu_context.h | 5 +++ arch/powerpc/include/asm/pkeys.h | 17 ++++++++++- arch/powerpc/mm/pkeys.c | 47 ++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 2 deletions(-)arch/powerpc/include/asm/mmu_context.h +5
-0@@ -185,6 +185,11 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,#ifndef CONFIG_PPC64_MEMORY_PROTECTION_KEYS #define pkey_initialize ()#define pkey_mm_init (mm) + +static inline int vma_pkey (struct vm_area_struct *vma) +{ +return 0 ; +}#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ #endif /* __KERNEL__ */ arch/powerpc/include/asm/pkeys.h +15
-2@@ -46,6 +46,16 @@ static inline u64 pkey_to_vmflag_bits(u16 pkey) ((pkey & 0x10UL) ? VM_PKEY_BIT4 : 0x0UL)); } +#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \ + VM_PKEY_BIT3 | VM_PKEY_BIT4) + +static inline int vma_pkey (struct vm_area_struct *vma) +{ +if (!pkey_inited) +return 0 ; +return (vma->vm_flags &ARCH_VM_PKEY_FLAGS ) >>VM_PKEY_SHIFT ; +} +#define arch_max_pkey () pkeys_total#define AMR_RD_BIT 0x1UL#define AMR_WR_BIT 0x2UL@@ -146,11 +156,14 @@ static inline int execute_only_pkey(struct mm_struct *mm)return __execute_only_pkey (mm); } - +extern int __arch_override_mprotect_pkey (struct vm_area_struct *vma, +int prot,int pkey);static inline int arch_override_mprotect_pkey (struct vm_area_struct *vma,int prot,int pkey) { -return 0 ; +if (!pkey_inited) +return 0 ; +return __arch_override_mprotect_pkey (vma, prot, pkey); } extern int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,arch/powerpc/mm/pkeys.c +17
-0@@ -154,3 +154,50 @@ int __execute_only_pkey(struct mm_struct *mm) mm->context.execute_only_pkey = execute_only_pkey;return execute_only_pkey; } + +static inline bool vma_is_pkey_exec_only (struct vm_area_struct *vma) +{ +/* Do this check first since the vm_flags should be hot */ +if ((vma->vm_flags & (VM_READ |VM_WRITE |VM_EXEC )) !=VM_EXEC ) +return false; + +return (vma_pkey (vma) == vma->vm_mm->context.execute_only_pkey); +} + +/* +* This should only be called for *plain* mprotect calls. +*/ +int __arch_override_mprotect_pkey (struct vm_area_struct *vma,int prot, +int pkey) +{ +/*
Quoted 8 lines
arch/powerpc/mm/pkeys.c int __execute_only_pkey(struct mm_struct *mm) · line 178
Quoted 5 lines
arch/powerpc/mm/pkeys.c +5
-0+* Is this an mprotect_pkey() call? If so, never +* override the value that came from the user. +*/ +if (pkey !=-1 ) +return pkey;This check should be moved in
arch_override_mprotect_pkey()inarch/powerpc/include/asm/pkeys.h
ok. will do.
Quoted 19 lines
Quoted 19 lines
arch/powerpc/mm/pkeys.c +18
-0+ +/* +* If the currently associated pkey is execute-only, +* but the requested protection requires read or write, +* move it back to the default pkey. +*/ +if (vma_is_pkey_exec_only (vma) && + (prot & (PROT_READ |PROT_WRITE ))) +return 0 ; + +/* +* the requested protection is execute-only. Hence +* lets use a execute-only pkey. +*/ +if (prot ==PROT_EXEC ) { + pkey =execute_only_pkey (vma->vm_mm); +if (pkey >0 ) +return pkey;
Quoted 8 lines
arch/powerpc/mm/pkeys.c int __execute_only_pkey(struct mm_struct *mm) · line 203
Quoted 8 lines
arch/powerpc/mm/pkeys.c +7
-0+ } + +/* +* nothing to override. +*/ +return vma_pkey (vma); +}
Thanks,
Signature · 1 line
Ram Pai
RFC v7 13/25 powerpc: map vma key-protection bits to pte key bits.
In reply to Ram Paimap the key protection bits of the vma to the pkey bits in the PTE.
The Pte bits used for pkey are 3,4,5,6 and 57. The first four bits are the same four bits that were freed up initially in this patch series. remember? :-) Without those four bits this patch would'nt be possible.
BUT, On 4k kernel, bit 3, and 4 could not be freed up. remember? Hence we have to be satisfied with 5,6 and 7.
--- arch/powerpc/include/asm/book3s/64/pgtable.h | 25 ++++++++++++++++++++++++- arch/powerpc/include/asm/mman.h | 8 ++++++++ arch/powerpc/include/asm/pkeys.h | 12 ++++++++++++ 3 files changed, 44 insertions(+), 1 deletions(-)
arch/powerpc/include/asm/book3s/64/pgtable.h +24 -1
@@ -37,6 +37,7 @@#define _RPAGE_RSV2 0x0800000000000000UL#define _RPAGE_RSV3 0x0400000000000000UL#define _RPAGE_RSV4 0x0200000000000000UL +#define _RPAGE_RSV5 0x00040UL#define _PAGE_PTE 0x4000000000000000UL/* distinguishes PTEs from pointers */ #define _PAGE_PRESENT 0x8000000000000000UL/* pte contains a translation */
@@ -56,6 +57,25 @@/* Max physical address bit as per radix table */ #define _RPAGE_PA_MAX 57 +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +#ifdef CONFIG_PPC_64K_PAGES +#define H_PAGE_PKEY_BIT0 _RPAGE_RSV1 +#define H_PAGE_PKEY_BIT1 _RPAGE_RSV2 +#else /* CONFIG_PPC_64K_PAGES */ +#define H_PAGE_PKEY_BIT0 0/* _RPAGE_RSV1 is not available */ +#define H_PAGE_PKEY_BIT1 0/* _RPAGE_RSV2 is not available */ +#endif /* CONFIG_PPC_64K_PAGES */ +#define H_PAGE_PKEY_BIT2 _RPAGE_RSV3 +#define H_PAGE_PKEY_BIT3 _RPAGE_RSV4 +#define H_PAGE_PKEY_BIT4 _RPAGE_RSV5 +#else /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ +#define H_PAGE_PKEY_BIT0 0 +#define H_PAGE_PKEY_BIT1 0 +#define H_PAGE_PKEY_BIT2 0 +#define H_PAGE_PKEY_BIT3 0 +#define H_PAGE_PKEY_BIT4 0 +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ +/* * Max physical address bit we will use for now. *
@@ -116,13 +136,16 @@#define _PAGE_CHG_MASK (PTE_RPN_MASK | _PAGE_HPTEFLAGS | _PAGE_DIRTY | \ _PAGE_ACCESSED | _PAGE_SPECIAL | _PAGE_PTE | \ _PAGE_SOFT_DIRTY) + +#define H_PAGE_PKEY (H_PAGE_PKEY_BIT0 | H_PAGE_PKEY_BIT1 | H_PAGE_PKEY_BIT2 | \ + H_PAGE_PKEY_BIT3 | H_PAGE_PKEY_BIT4)/* * Mask of bits returned by pte_pgprot() */ #define PAGE_PROT_BITS (_PAGE_SAO | _PAGE_NON_IDEMPOTENT | _PAGE_TOLERANT | \ H_PAGE_4K_PFN | _PAGE_PRIVILEGED | _PAGE_ACCESSED | \ _PAGE_READ | _PAGE_WRITE | _PAGE_DIRTY | _PAGE_EXEC | \ - _PAGE_SOFT_DIRTY) + _PAGE_SOFT_DIRTY | H_PAGE_PKEY)/* * We define 2 sets of base prot bits, one for basic pages (ie, * cacheable kernel and user pages) and one for non cacheable
arch/powerpc/include/asm/mman.h +8 -0
@@ -32,12 +32,20 @@ static inline unsigned long arch_calc_vm_prot_bits(unsigned long prot, }#define arch_calc_vm_prot_bits (prot, pkey) arch_calc_vm_prot_bits(prot, pkey) +static inline pgprot_t arch_vm_get_page_prot (unsigned long vm_flags) { +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +return (vm_flags &VM_SAO ) ? +__pgprot (_PAGE_SAO |vmflag_to_page_pkey_bits (vm_flags)) : +__pgprot (0 |vmflag_to_page_pkey_bits (vm_flags)); +#else return (vm_flags &VM_SAO ) ?__pgprot (_PAGE_SAO) :__pgprot (0 ); +#endif }#define arch_vm_get_page_prot (vm_flags) arch_vm_get_page_prot(vm_flags) +static inline bool arch_validate_prot (unsigned long prot) {if (prot & ~(PROT_READ |PROT_WRITE |PROT_EXEC |PROT_SEM |PROT_SAO ))
arch/powerpc/include/asm/pkeys.h +12 -0
@@ -46,6 +46,18 @@ static inline u64 pkey_to_vmflag_bits(u16 pkey) ((pkey & 0x10UL) ? VM_PKEY_BIT4 : 0x0UL)); } +static inline u64 vmflag_to_page_pkey_bits (u64 vm_flags) +{ +if (!pkey_inited) +return 0x0UL ; + +return (((vm_flags &VM_PKEY_BIT0 ) ?H_PAGE_PKEY_BIT4 :0x0UL ) | + ((vm_flags &VM_PKEY_BIT1 ) ?H_PAGE_PKEY_BIT3 :0x0UL ) | + ((vm_flags &VM_PKEY_BIT2 ) ?H_PAGE_PKEY_BIT2 :0x0UL ) | + ((vm_flags &VM_PKEY_BIT3 ) ?H_PAGE_PKEY_BIT1 :0x0UL ) | + ((vm_flags &VM_PKEY_BIT4 ) ?H_PAGE_PKEY_BIT0 :0x0UL )); +} +#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \ VM_PKEY_BIT3 | VM_PKEY_BIT4)
Signature · 1 line
1.7.1
RFC v7 14/25 powerpc: sys_pkey_mprotect() system call
In reply to Ram PaiPatch provides the ability for a process to
associate a pkey with a address range.
--- arch/powerpc/include/asm/systbl.h | 1 + arch/powerpc/include/asm/unistd.h | 4 +--- arch/powerpc/include/uapi/asm/unistd.h | 1 + 3 files changed, 3 insertions(+), 3 deletions(-)
arch/powerpc/include/asm/systbl.h +1 -0
@@ -390,3 +390,4 @@ SYSCALL(statx) SYSCALL(pkey_alloc) SYSCALL(pkey_free) +SYSCALL(pkey_mprotect)
arch/powerpc/include/asm/unistd.h +1 -3
@@ -12,12 +12,10 @@#include <uapi/asm/unistd.h> -#define NR_syscalls 386 +#define NR_syscalls 387#define __NR__exit __NR_exit -#define __IGNORE_pkey_mprotect -#ifndef __ASSEMBLY__#include <linux/types.h>
arch/powerpc/include/uapi/asm/unistd.h +1 -0
@@ -396,5 +396,6 @@#define __NR_statx 383#define __NR_pkey_alloc 384#define __NR_pkey_free 385 +#define __NR_pkey_mprotect 386#endif /* _UAPI_ASM_POWERPC_UNISTD_H_ */
Signature · 1 line
1.7.1
RFC v7 15/25 powerpc: Program HPTE key protection bits
In reply to Ram PaiMap the PTE protection key bits to the HPTE key protection bits, while creating HPTE entries.
--- arch/powerpc/include/asm/book3s/64/mmu-hash.h | 5 +++++ arch/powerpc/include/asm/mmu_context.h | 6 ++++++ arch/powerpc/include/asm/pkeys.h | 13 +++++++++++++ arch/powerpc/mm/hash_utils_64.c | 1 + 4 files changed, 25 insertions(+), 0 deletions(-)
arch/powerpc/include/asm/book3s/64/mmu-hash.h +5 -0
@@ -90,6 +90,8 @@#define HPTE_R_PP0 ASM_CONST(0x8000000000000000)#define HPTE_R_TS ASM_CONST(0x4000000000000000)#define HPTE_R_KEY_HI ASM_CONST(0x3000000000000000) +#define HPTE_R_KEY_BIT0 ASM_CONST(0x2000000000000000) +#define HPTE_R_KEY_BIT1 ASM_CONST(0x1000000000000000)#define HPTE_R_RPN_SHIFT 12#define HPTE_R_RPN ASM_CONST(0x0ffffffffffff000)#define HPTE_R_RPN_3_0 ASM_CONST(0x01fffffffffff000)
@@ -104,6 +106,9 @@#define HPTE_R_C ASM_CONST(0x0000000000000080)#define HPTE_R_R ASM_CONST(0x0000000000000100)#define HPTE_R_KEY_LO ASM_CONST(0x0000000000000e00) +#define HPTE_R_KEY_BIT2 ASM_CONST(0x0000000000000800) +#define HPTE_R_KEY_BIT3 ASM_CONST(0x0000000000000400) +#define HPTE_R_KEY_BIT4 ASM_CONST(0x0000000000000200)#define HPTE_V_1TB_SEG ASM_CONST(0x4000000000000000)#define HPTE_V_VRMA_MASK ASM_CONST(0x4001ffffff000000)
arch/powerpc/include/asm/mmu_context.h +6 -0
@@ -190,6 +190,12 @@ static inline int vma_pkey(struct vm_area_struct *vma) {return 0 ; } + +static inline u64 pte_to_hpte_pkey_bits (u64 pteflags) +{ +return 0x0UL ; +} +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ #endif /* __KERNEL__ */
arch/powerpc/include/asm/pkeys.h +13 -0
@@ -72,6 +72,19 @@ static inline int vma_pkey(struct vm_area_struct *vma)#define AMR_RD_BIT 0x1UL#define AMR_WR_BIT 0x2UL#define IAMR_EX_BIT 0x1UL + +static inline u64 pte_to_hpte_pkey_bits (u64 pteflags) +{ +if (!pkey_inited) +return 0x0UL ; + +return (((pteflags &H_PAGE_PKEY_BIT0 ) ?HPTE_R_KEY_BIT0 :0x0UL ) | + ((pteflags &H_PAGE_PKEY_BIT1 ) ?HPTE_R_KEY_BIT1 :0x0UL ) | + ((pteflags &H_PAGE_PKEY_BIT2 ) ?HPTE_R_KEY_BIT2 :0x0UL ) | + ((pteflags &H_PAGE_PKEY_BIT3 ) ?HPTE_R_KEY_BIT3 :0x0UL ) | + ((pteflags &H_PAGE_PKEY_BIT4 ) ?HPTE_R_KEY_BIT4 :0x0UL )); +} +#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \ VM_PKEY_BIT3 | VM_PKEY_BIT4)#define AMR_BITS_PER_PKEY 2
arch/powerpc/mm/hash_utils_64.c +1 -0
@@ -231,6 +231,7 @@ unsigned long htab_convert_pte_flags(unsigned long pteflags)*/ rflags |=HPTE_R_M ; + rflags |=pte_to_hpte_pkey_bits (pteflags);return rflags; }
Signature · 1 line
1.7.1
Re: [RFC v7 15/25] powerpc: Program HPTE key protection bits
Quoted 56 lines · Ram Pai
Map the PTE protection key bits to the HPTE key protection bits, while creating HPTE entries.
Signed-off-by Ram Pai <[email protected]>--- arch/powerpc/include/asm/book3s/64/mmu-hash.h | 5 +++++ arch/powerpc/include/asm/mmu_context.h | 6 ++++++ arch/powerpc/include/asm/pkeys.h | 13 +++++++++++++ arch/powerpc/mm/hash_utils_64.c | 1 + 4 files changed, 25 insertions(+), 0 deletions(-)arch/powerpc/include/asm/book3s/64/mmu-hash.h +5
-0@@ -90,6 +90,8 @@#define HPTE_R_PP0 ASM_CONST(0x8000000000000000)#define HPTE_R_TS ASM_CONST(0x4000000000000000)#define HPTE_R_KEY_HI ASM_CONST(0x3000000000000000) +#define HPTE_R_KEY_BIT0 ASM_CONST(0x2000000000000000) +#define HPTE_R_KEY_BIT1 ASM_CONST(0x1000000000000000)#define HPTE_R_RPN_SHIFT 12#define HPTE_R_RPN ASM_CONST(0x0ffffffffffff000)#define HPTE_R_RPN_3_0 ASM_CONST(0x01fffffffffff000)@@ -104,6 +106,9 @@#define HPTE_R_C ASM_CONST(0x0000000000000080)#define HPTE_R_R ASM_CONST(0x0000000000000100)#define HPTE_R_KEY_LO ASM_CONST(0x0000000000000e00) +#define HPTE_R_KEY_BIT2 ASM_CONST(0x0000000000000800) +#define HPTE_R_KEY_BIT3 ASM_CONST(0x0000000000000400) +#define HPTE_R_KEY_BIT4 ASM_CONST(0x0000000000000200)#define HPTE_V_1TB_SEG ASM_CONST(0x4000000000000000)#define HPTE_V_VRMA_MASK ASM_CONST(0x4001ffffff000000)arch/powerpc/include/asm/mmu_context.h +6
-0@@ -190,6 +190,12 @@ static inline int vma_pkey(struct vm_area_struct *vma) {return 0 ; } + +static inline u64 pte_to_hpte_pkey_bits (u64 pteflags) +{ +return 0x0UL ; +} +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ #endif /* __KERNEL__ */ arch/powerpc/include/asm/pkeys.h +0
-0
arch/powerpc/include/asm/pkeys.h static inline int vma_pkey(struct vm_area_struct *vma) · line 79
Quoted 8 lines
arch/powerpc/include/asm/pkeys.h +5
-0@@ -72,6 +72,19 @@ static inline int vma_pkey(struct vm_area_struct *vma)#define AMR_RD_BIT 0x1UL#define AMR_WR_BIT 0x2UL#define IAMR_EX_BIT 0x1UL + +static inline u64 pte_to_hpte_pkey_bits (u64 pteflags) +{ +if (!pkey_inited) +return 0x0UL ;
Why making such a check here, is it to avoid the following check during the boot process only ?
IIUC, there is no way to get H_PAGE_PKEY_BIT* set when pkey_inited is false.
Quoted 23 lines
arch/powerpc/include/asm/pkeys.h +8
-0+ +return (((pteflags &H_PAGE_PKEY_BIT0 ) ?HPTE_R_KEY_BIT0 :0x0UL ) | + ((pteflags &H_PAGE_PKEY_BIT1 ) ?HPTE_R_KEY_BIT1 :0x0UL ) | + ((pteflags &H_PAGE_PKEY_BIT2 ) ?HPTE_R_KEY_BIT2 :0x0UL ) | + ((pteflags &H_PAGE_PKEY_BIT3 ) ?HPTE_R_KEY_BIT3 :0x0UL ) | + ((pteflags &H_PAGE_PKEY_BIT4 ) ?HPTE_R_KEY_BIT4 :0x0UL )); +} +#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \ VM_PKEY_BIT3 | VM_PKEY_BIT4)#define AMR_BITS_PER_PKEY 2arch/powerpc/mm/hash_utils_64.c +1
-0@@ -231,6 +231,7 @@ unsigned long htab_convert_pte_flags(unsigned long pteflags)*/ rflags |=HPTE_R_M ; + rflags |=pte_to_hpte_pkey_bits (pteflags);return rflags; }
Re: [RFC v7 15/25] powerpc: Program HPTE key protection bits
Quoted 63 lines · Laurent Dufour
Quoted 60 lines · Ram Pai
Map the PTE protection key bits to the HPTE key protection bits, while creating HPTE entries.
Signed-off-by Ram Pai <[email protected]>--- arch/powerpc/include/asm/book3s/64/mmu-hash.h | 5 +++++ arch/powerpc/include/asm/mmu_context.h | 6 ++++++ arch/powerpc/include/asm/pkeys.h | 13 +++++++++++++ arch/powerpc/mm/hash_utils_64.c | 1 + 4 files changed, 25 insertions(+), 0 deletions(-)arch/powerpc/include/asm/book3s/64/mmu-hash.h +5
-0@@ -90,6 +90,8 @@#define HPTE_R_PP0 ASM_CONST(0x8000000000000000)#define HPTE_R_TS ASM_CONST(0x4000000000000000)#define HPTE_R_KEY_HI ASM_CONST(0x3000000000000000) +#define HPTE_R_KEY_BIT0 ASM_CONST(0x2000000000000000) +#define HPTE_R_KEY_BIT1 ASM_CONST(0x1000000000000000)#define HPTE_R_RPN_SHIFT 12#define HPTE_R_RPN ASM_CONST(0x0ffffffffffff000)#define HPTE_R_RPN_3_0 ASM_CONST(0x01fffffffffff000)@@ -104,6 +106,9 @@#define HPTE_R_C ASM_CONST(0x0000000000000080)#define HPTE_R_R ASM_CONST(0x0000000000000100)#define HPTE_R_KEY_LO ASM_CONST(0x0000000000000e00) +#define HPTE_R_KEY_BIT2 ASM_CONST(0x0000000000000800) +#define HPTE_R_KEY_BIT3 ASM_CONST(0x0000000000000400) +#define HPTE_R_KEY_BIT4 ASM_CONST(0x0000000000000200)#define HPTE_V_1TB_SEG ASM_CONST(0x4000000000000000)#define HPTE_V_VRMA_MASK ASM_CONST(0x4001ffffff000000)arch/powerpc/include/asm/mmu_context.h +6
-0@@ -190,6 +190,12 @@ static inline int vma_pkey(struct vm_area_struct *vma) {return 0 ; } + +static inline u64 pte_to_hpte_pkey_bits (u64 pteflags) +{ +return 0x0UL ; +} +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ #endif /* __KERNEL__ */ arch/powerpc/include/asm/pkeys.h +1
-0@@ -72,6 +72,19 @@ static inline int vma_pkey(struct vm_area_struct *vma)#define AMR_RD_BIT 0x1UL#define AMR_WR_BIT 0x2UL#define IAMR_EX_BIT 0x1UL +
Quoted 8 lines
arch/powerpc/include/asm/pkeys.h static inline int vma_pkey(struct vm_area_struct *vma) · line 79
Quoted 4 lines
arch/powerpc/include/asm/pkeys.h +4
-0+static inline u64 pte_to_hpte_pkey_bits (u64 pteflags) +{ +if (!pkey_inited) +return 0x0UL ;Why making such a check here, is it to avoid the following check during the boot process only ?
IIUC, there is no way to get H_PAGE_PKEY_BIT* set whenpkey_initedis false.
I know its a little paronia. Trying to avoid a case where the uninitialized pkey bits in the pte are erroneously interpreted as valid by this function. Remember that the caller of this function will use the return value to program the hpte. Nothing really bad should happen since none of the keys are enabled. But ... just playing it safe.
thanks,
RP
Michael Ellerman
reviewerRe: [RFC v7 15/25] powerpc: Program HPTE key protection bits
Quoted 20 lines · Ram Pai
Quoted 10 lines · Laurent Dufour
Quoted 9 lines · Ram Pai
arch/powerpc/include/asm/pkeys.h +1
-0@@ -72,6 +72,19 @@ static inline int vma_pkey(struct vm_area_struct *vma)#define AMR_RD_BIT 0x1UL#define AMR_WR_BIT 0x2UL#define IAMR_EX_BIT 0x1UL +Quoted 8 lines
arch/powerpc/include/asm/pkeys.h static inline int vma_pkey(struct vm_area_struct *vma) · line 79
Quoted 4 lines
arch/powerpc/include/asm/pkeys.h +4
-0+static inline u64 pte_to_hpte_pkey_bits (u64 pteflags) +{ +if (!pkey_inited) +return 0x0UL ;Why making such a check here, is it to avoid the following check during the boot process only ?
IIUC, there is no way to get H_PAGE_PKEY_BIT* set whenpkey_initedis false.
Quoted 5 lines
I know its a little paronia. Trying to avoid a case where the uninitialized pkey bits in the pte are erroneously interpreted as valid by this function. Remember that the caller of this function will use the return value to program the hpte. Nothing really bad should happen since none of the keys are enabled. But ... just playing it safe.
I think that's probably over-paranoid. It's not like it adds much overhead, but it is a hot path, so no need to make it slower than it needs to be.
cheers
RFC v7 16/25 powerpc: helper to validate key-access permissions of a pte
In reply to Ram Paihelper function that checks if the read/write/execute is allowed on the pte.
--- arch/powerpc/include/asm/book3s/64/pgtable.h | 4 +++ arch/powerpc/include/asm/pkeys.h | 12 +++++++++++ arch/powerpc/mm/pkeys.c | 28 ++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 0 deletions(-)
arch/powerpc/include/asm/book3s/64/pgtable.h +4 -0
@@ -477,6 +477,10 @@ static inline void write_uamor(u64 value)mtspr (SPRN_UAMOR , value); } +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +extern bool arch_pte_access_permitted (u64 pte,bool write,bool execute); +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ +#define __HAVE_ARCH_PTEP_GET_AND_CLEARstatic inline pte_t ptep_get_and_clear (struct mm_struct *mm,unsigned long addr,pte_t *ptep)
arch/powerpc/include/asm/pkeys.h +12 -0
@@ -85,6 +85,18 @@ static inline u64 pte_to_hpte_pkey_bits(u64 pteflags) ((pteflags & H_PAGE_PKEY_BIT4) ? HPTE_R_KEY_BIT4 : 0x0UL)); } +static inline u16 pte_to_pkey_bits (u64 pteflags) +{ +if (!pkey_inited) +return 0x0UL ; + +return (((pteflags &H_PAGE_PKEY_BIT0 ) ?0x10 :0x0UL ) | + ((pteflags &H_PAGE_PKEY_BIT1 ) ?0x8 :0x0UL ) | + ((pteflags &H_PAGE_PKEY_BIT2 ) ?0x4 :0x0UL ) | + ((pteflags &H_PAGE_PKEY_BIT3 ) ?0x2 :0x0UL ) | + ((pteflags &H_PAGE_PKEY_BIT4 ) ?0x1 :0x0UL )); +} +#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \ VM_PKEY_BIT3 | VM_PKEY_BIT4)#define AMR_BITS_PER_PKEY 2
arch/powerpc/mm/pkeys.c +28 -0
@@ -201,3 +201,31 @@ int __arch_override_mprotect_pkey(struct vm_area_struct *vma, int prot,*/ return vma_pkey (vma); } + +static bool pkey_access_permitted (int pkey,bool write,bool execute) +{ +int pkey_shift; +u64 amr; + +if (!pkey) +return true; + + pkey_shift =pkeyshift (pkey); +if (!(read_uamor () & (0x3UL << pkey_shift))) +return true; + +if (execute && !(read_iamr () & (IAMR_EX_BIT << pkey_shift))) +return true; + + amr =read_amr ();/* delay reading amr uptil absolutely needed*/ +return ((!write && !(amr & (AMR_RD_BIT << pkey_shift))) || + (write && !(amr & (AMR_WR_BIT << pkey_shift)))); +} + +bool arch_pte_access_permitted (u64 pte,bool write,bool execute) +{ +if (!pkey_inited) +return true; +return pkey_access_permitted (pte_to_pkey_bits (pte), + write, execute); +}
Signature · 1 line
1.7.1
Re: [RFC v7 16/25] powerpc: helper to validate key-access permissions of a pte
Quoted 30 lines · Ram Pai
helper function that checks if the read/write/execute is allowed on the pte.
Signed-off-by Ram Pai <[email protected]>--- arch/powerpc/include/asm/book3s/64/pgtable.h | 4 +++ arch/powerpc/include/asm/pkeys.h | 12 +++++++++++ arch/powerpc/mm/pkeys.c | 28 ++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 0 deletions(-)arch/powerpc/include/asm/book3s/64/pgtable.h +4
-0@@ -477,6 +477,10 @@ static inline void write_uamor(u64 value)mtspr (SPRN_UAMOR , value); } +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +extern bool arch_pte_access_permitted (u64 pte,bool write,bool execute); +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ +#define __HAVE_ARCH_PTEP_GET_AND_CLEARstatic inline pte_t ptep_get_and_clear (struct mm_struct *mm,unsigned long addr,pte_t *ptep)arch/powerpc/include/asm/pkeys.h +0
-0
arch/powerpc/include/asm/pkeys.h static inline u64 pte_to_hpte_pkey_bits(u64 pteflags) · line 91
Quoted 7 lines
arch/powerpc/include/asm/pkeys.h +4
-0@@ -85,6 +85,18 @@ static inline u64 pte_to_hpte_pkey_bits(u64 pteflags) ((pteflags & H_PAGE_PKEY_BIT4) ? HPTE_R_KEY_BIT4 : 0x0UL)); } +static inline u16 pte_to_pkey_bits (u64 pteflags) +{ +if (!pkey_inited) +return 0x0UL ;
Is it really needed to make such a check in this low level function ? The only caller is already checking for pkey_inited before making the call.
Quoted 48 lines
arch/powerpc/include/asm/pkeys.h +8
-0+ +return (((pteflags &H_PAGE_PKEY_BIT0 ) ?0x10 :0x0UL ) | + ((pteflags &H_PAGE_PKEY_BIT1 ) ?0x8 :0x0UL ) | + ((pteflags &H_PAGE_PKEY_BIT2 ) ?0x4 :0x0UL ) | + ((pteflags &H_PAGE_PKEY_BIT3 ) ?0x2 :0x0UL ) | + ((pteflags &H_PAGE_PKEY_BIT4 ) ?0x1 :0x0UL )); +} +#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \ VM_PKEY_BIT3 | VM_PKEY_BIT4)#define AMR_BITS_PER_PKEY 2arch/powerpc/mm/pkeys.c +28
-0@@ -201,3 +201,31 @@ int __arch_override_mprotect_pkey(struct vm_area_struct *vma, int prot,*/ return vma_pkey (vma); } + +static bool pkey_access_permitted (int pkey,bool write,bool execute) +{ +int pkey_shift; +u64 amr; + +if (!pkey) +return true; + + pkey_shift =pkeyshift (pkey); +if (!(read_uamor () & (0x3UL << pkey_shift))) +return true; + +if (execute && !(read_iamr () & (IAMR_EX_BIT << pkey_shift))) +return true; + + amr =read_amr ();/* delay reading amr uptil absolutely needed*/ +return ((!write && !(amr & (AMR_RD_BIT << pkey_shift))) || + (write && !(amr & (AMR_WR_BIT << pkey_shift)))); +} + +bool arch_pte_access_permitted (u64 pte,bool write,bool execute) +{ +if (!pkey_inited) +return true; +return pkey_access_permitted (pte_to_pkey_bits (pte), + write, execute); +}
Re: [RFC v7 16/25] powerpc: helper to validate key-access permissions of a pte
Quoted 36 lines · Laurent Dufour
Quoted 33 lines · Ram Pai
helper function that checks if the read/write/execute is allowed on the pte.
Signed-off-by Ram Pai <[email protected]>--- arch/powerpc/include/asm/book3s/64/pgtable.h | 4 +++ arch/powerpc/include/asm/pkeys.h | 12 +++++++++++ arch/powerpc/mm/pkeys.c | 28 ++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 0 deletions(-)arch/powerpc/include/asm/book3s/64/pgtable.h +4
-0@@ -477,6 +477,10 @@ static inline void write_uamor(u64 value)mtspr (SPRN_UAMOR , value); } +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +extern bool arch_pte_access_permitted (u64 pte,bool write,bool execute); +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ +#define __HAVE_ARCH_PTEP_GET_AND_CLEARstatic inline pte_t ptep_get_and_clear (struct mm_struct *mm,unsigned long addr,pte_t *ptep)arch/powerpc/include/asm/pkeys.h +0
-0@@ -85,6 +85,18 @@ static inline u64 pte_to_hpte_pkey_bits(u64 pteflags) ((pteflags & H_PAGE_PKEY_BIT4) ? HPTE_R_KEY_BIT4 : 0x0UL)); }
Quoted 7 lines
arch/powerpc/include/asm/pkeys.h static inline u64 pte_to_hpte_pkey_bits(u64 pteflags) · line 91
Quoted 4 lines
arch/powerpc/include/asm/pkeys.h +4
-0+static inline u16 pte_to_pkey_bits (u64 pteflags) +{ +if (!pkey_inited) +return 0x0UL ;Is it really needed to make such a check in this low level function ? The only caller is already checking for
pkey_initedbefore making the call.
There are two callers to this function. get_pte_pkey() is one among them and it calls this function ignorant of the status of the pkey-subsystem.
Thanks,
RP
Michael Ellerman
reviewerRe: [RFC v7 16/25] powerpc: helper to validate key-access permissions of a pte
Quoted 18 lines · Ram Pai
Quoted 9 lines · Laurent Dufour
Quoted 8 lines · Ram Pai
arch/powerpc/include/asm/pkeys.h +0
-0@@ -85,6 +85,18 @@ static inline u64 pte_to_hpte_pkey_bits(u64 pteflags) ((pteflags & H_PAGE_PKEY_BIT4) ? HPTE_R_KEY_BIT4 : 0x0UL)); }Quoted 7 lines
arch/powerpc/include/asm/pkeys.h static inline u64 pte_to_hpte_pkey_bits(u64 pteflags) · line 91
Quoted 4 lines
arch/powerpc/include/asm/pkeys.h +4
-0+static inline u16 pte_to_pkey_bits (u64 pteflags) +{ +if (!pkey_inited) +return 0x0UL ;Is it really needed to make such a check in this low level function ? The only caller is already checking for
pkey_initedbefore making the call.
Quoted 3 lines
There are two callers to this function.
get_pte_pkey()is one among them and it calls this function ignorant of the status of the pkey-subsystem.
But if none of the bits are set it will return 0 anyway right?
cheers
RFC v7 17/25 powerpc: check key protection for user page access
In reply to Ram PaiMake sure that the kernel does not access user pages without checking their key-protection.
--- arch/powerpc/include/asm/book3s/64/pgtable.h | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)
arch/powerpc/include/asm/book3s/64/pgtable.h +14 -0
@@ -479,6 +479,20 @@ static inline void write_uamor(u64 value)#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS extern bool arch_pte_access_permitted (u64 pte,bool write,bool execute); + +#define pte_access_permitted (pte, write) \ + (pte_present(pte) && \ + ((!(write) || pte_write(pte)) && \ + arch_pte_access_permitted(pte_val(pte), !!write, 0))) + +/* +* We store key in pmd for huge tlb pages. So need +* to check for key protection. +*/ +#define pmd_access_permitted (pmd, write) \ + (pmd_present(pmd) && \ + ((!(write) || pmd_write(pmd)) && \ + arch_pte_access_permitted(pmd_val(pmd), !!write, 0)))#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ #define __HAVE_ARCH_PTEP_GET_AND_CLEAR
Signature · 1 line
1.7.1
RFC v7 18/25 powerpc: Macro the mask used for checking DSI exception
In reply to Ram PaiReplace the magic number used to check for DSI exception with a meaningful value.
--- arch/powerpc/include/asm/reg.h | 7 ++++++- arch/powerpc/kernel/exceptions-64s.S | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-)
arch/powerpc/include/asm/reg.h +6 -1
@@ -272,16 +272,21 @@#define SPRN_DAR 0x013/* Data Address Register */ #define SPRN_DBCR 0x136/* e300 Data Breakpoint Control Reg */ #define SPRN_DSISR 0x012/* Data Storage Interrupt Status Register */ +#define DSISR_BIT32 0x80000000/* not defined */ #define DSISR_NOHPTE 0x40000000/* no translation found */ +#define DSISR_PAGEATTR_CONFLT 0x20000000/* page attribute conflict */ +#define DSISR_BIT35 0x10000000/* not defined */ #define DSISR_PROTFAULT 0x08000000/* protection fault */ #define DSISR_BADACCESS 0x04000000/* bad access to CI or G */ #define DSISR_ISSTORE 0x02000000/* access was a store */ #define DSISR_DABRMATCH 0x00400000/* hit data breakpoint */ -#define DSISR_NOSEGMENT 0x00200000/* SLB miss */ #define DSISR_KEYFAULT 0x00200000/* Key fault */ +#define DSISR_BIT43 0x00100000/* not defined */ #define DSISR_UNSUPP_MMU 0x00080000/* Unsupported MMU config */ #define DSISR_SET_RC 0x00040000/* Failed setting of R/C bits */ #define DSISR_PGDIRFAULT 0x00020000/* Fault on page directory */ +#define DSISR_PAGE_FAULT_MASK (DSISR_BIT32 | DSISR_PAGEATTR_CONFLT | \ + DSISR_BADACCESS | DSISR_DABRMATCH | DSISR_BIT43)#define SPRN_TBRL 0x10C/* Time Base Read Lower Register (user, R/O) */ #define SPRN_TBRU 0x10D/* Time Base Read Upper Register (user, R/O) */ #define SPRN_CIR 0x11B/* Chip Information Register (hyper, R/0) */
arch/powerpc/kernel/exceptions-64s.S +1 -1
@@ -1411,7 +1411,7 @@ USE_TEXT_SECTION() .balign IFETCH_ALIGN_BYTES do_hash_page: #ifdef CONFIG_PPC_STD_MMU_64 - andis. r0,r4,0xa450 /* weird error? */ + andis. r0,r4,DSISR_PAGE_FAULT_MASK@h bne- handle_page_fault /* if not, try to insert a HPTE */ CURRENT_THREAD_INFO(r11, r1) lwz r0,TI_PREEMPT(r11) /* If we're in an "NMI" */
Signature · 1 line
1.7.1
RFC v7 19/25 powerpc: implementation for arch_vma_access_permitted()
In reply to Ram PaiThis patch provides the implementation for arch_vma_access_permitted(). Returns true if the requested access is allowed by pkey associated with the vma.
--- arch/powerpc/include/asm/mmu_context.h | 5 +++- arch/powerpc/mm/pkeys.c | 43 ++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletions(-)
arch/powerpc/include/asm/mmu_context.h +4 -1
@@ -175,6 +175,10 @@ static inline void arch_bprm_mm_init(struct mm_struct *mm, { } +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +bool arch_vma_access_permitted (struct vm_area_struct *vma, +bool write,bool execute,bool foreign); +#else /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ static inline bool arch_vma_access_permitted (struct vm_area_struct *vma,bool write,bool execute,bool foreign) {
@@ -182,7 +186,6 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,return true; } -#ifndef CONFIG_PPC64_MEMORY_PROTECTION_KEYS #define pkey_initialize ()#define pkey_mm_init (mm)
arch/powerpc/mm/pkeys.c +43 -0
46 lines not on this page · raw message
Signature · 1 line
1.7.1
RFC v7 20/25 powerpc: Handle exceptions caused by pkey violation
In reply to Ram PaiHandle Data and Instruction exceptions caused by memory protection-key.
The CPU will detect the key fault if the HPTE is already programmed with the key.
However if the HPTE is not hashed, a key fault will not
be detected by the hardware. The software will detect
pkey violation in such a case.
--- arch/powerpc/include/asm/reg.h | 3 ++- arch/powerpc/mm/fault.c | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletions(-)
arch/powerpc/include/asm/reg.h +2 -1
@@ -286,7 +286,8 @@#define DSISR_SET_RC 0x00040000/* Failed setting of R/C bits */ #define DSISR_PGDIRFAULT 0x00020000/* Fault on page directory */ #define DSISR_PAGE_FAULT_MASK (DSISR_BIT32 | DSISR_PAGEATTR_CONFLT | \ - DSISR_BADACCESS | DSISR_DABRMATCH | DSISR_BIT43) + DSISR_BADACCESS | DSISR_KEYFAULT | \ + DSISR_DABRMATCH | DSISR_BIT43)#define SPRN_TBRL 0x10C/* Time Base Read Lower Register (user, R/O) */ #define SPRN_TBRU 0x10D/* Time Base Read Upper Register (user, R/O) */ #define SPRN_CIR 0x11B/* Chip Information Register (hyper, R/0) */
arch/powerpc/mm/fault.c +21 -0
@@ -261,6 +261,13 @@ int do_page_fault(struct pt_regs *regs, unsigned long address, }#endif +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +if (error_code &DSISR_KEYFAULT ) { + code =SEGV_PKUERR ; + goto bad_area_nosemaphore; + } +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ +/* We restore the interrupt state now */ if (!arch_irq_disabled_regs (regs))local_irq_enable ();
@@ -441,6 +448,20 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,WARN_ON_ONCE (error_code &DSISR_PROTFAULT );#endif /* CONFIG_PPC_STD_MMU */ +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +if (!arch_vma_access_permitted (vma, flags &FAULT_FLAG_WRITE , + is_exec,0 )) { + code =SEGV_PKUERR ; + goto bad_area; + } +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ + + +/* handle_mm_fault() needs to know if its a instruction access +* fault. +*/ +if (is_exec) + flags |=FAULT_FLAG_INSTRUCTION ;/* * If for any reason at all we couldn't handle the fault, * make sure we exit gracefully rather than endlessly redo
Signature · 1 line
1.7.1
RFC v7 21/25 powerpc: capture AMR register content on pkey violation
In reply to Ram Paicapture AMR register contents, and save it in paca whenever a pkey violation is detected.
This value will be needed to deliver pkey-violation signal to the task.
--- arch/powerpc/include/asm/paca.h | 3 +++ arch/powerpc/kernel/asm-offsets.c | 5 +++++ arch/powerpc/mm/fault.c | 2 ++ 3 files changed, 10 insertions(+), 0 deletions(-)
arch/powerpc/include/asm/paca.h +3 -0
@@ -92,6 +92,9 @@ struct paca_struct {struct dtl_entry *dispatch_log_end;#endif /* CONFIG_PPC_STD_MMU_64 */ u64 dscr_default;/* per-CPU default DSCR */ +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +u64 paca_amr;/* value of amr at exception */ +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ #ifdef CONFIG_PPC_STD_MMU_64 /*
arch/powerpc/kernel/asm-offsets.c +5 -0
@@ -241,6 +241,11 @@ int main(void)OFFSET (PACAHWCPUID , paca_struct, hw_cpu_id);OFFSET (PACAKEXECSTATE , paca_struct, kexec_state);OFFSET (PACA_DSCR_DEFAULT , paca_struct, dscr_default); + +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +OFFSET (PACA_AMR , paca_struct, paca_amr); +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ +OFFSET (ACCOUNT_STARTTIME , paca_struct, accounting.starttime);OFFSET (ACCOUNT_STARTTIME_USER , paca_struct, accounting.starttime_user);OFFSET (ACCOUNT_USER_TIME , paca_struct, accounting.utime);
arch/powerpc/mm/fault.c +2 -0
@@ -264,6 +264,7 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS if (error_code &DSISR_KEYFAULT ) { code =SEGV_PKUERR ; +get_paca ()->paca_amr =read_amr (); goto bad_area_nosemaphore; }#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
@@ -451,6 +452,7 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS if (!arch_vma_access_permitted (vma, flags &FAULT_FLAG_WRITE , is_exec,0 )) { +get_paca ()->paca_amr =read_amr (); code =SEGV_PKUERR ; goto bad_area; }
Signature · 1 line
1.7.1
RFC v7 22/25 powerpc: introduce get_pte_pkey() helper
In reply to Ram Paiget_pte_pkey() helper returns the pkey associated with a address corresponding to a given mm_struct.
--- arch/powerpc/include/asm/book3s/64/mmu-hash.h | 5 +++++ arch/powerpc/mm/hash_utils_64.c | 25 +++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 0 deletions(-)
arch/powerpc/include/asm/book3s/64/mmu-hash.h +5 -0
@@ -450,6 +450,11 @@ extern int hash_page(unsigned long ea, unsigned long access, unsigned long trap,int __hash_page_huge (unsigned long ea,unsigned long access,unsigned long vsid,pte_t *ptep,unsigned long trap,unsigned long flags,int ssize,unsigned int shift,unsigned int mmu_psize); + +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +u16 get_pte_pkey (struct mm_struct *mm,unsigned long address); +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ +#ifdef CONFIG_TRANSPARENT_HUGEPAGE extern int __hash_page_thp(unsigned long ea, unsigned long access, unsigned long vsid, pmd_t *pmdp, unsigned long trap,
arch/powerpc/mm/hash_utils_64.c +25 -0
@@ -1570,6 +1570,31 @@ void hash_preload(struct mm_struct *mm, unsigned long ea,local_irq_restore (flags); } +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +/* +* return the protection key associated with the given address +* and the mm_struct. +*/ +u16 get_pte_pkey (struct mm_struct *mm,unsigned long address) +{ +pte_t *ptep; +u16 pkey =0 ; +unsigned long flags; + +if (!mm || !mm->pgd) +return 0 ; + +local_irq_save (flags); + ptep =find_linux_pte_or_hugepte (mm->pgd, address, +NULL ,NULL ); +if (ptep) + pkey =pte_to_pkey_bits (pte_val (READ_ONCE (*ptep))); +local_irq_restore (flags); + +return pkey; +} +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM static inline void tm_flush_hash_page (int local) {
Signature · 1 line
1.7.1
RFC v7 23/25 powerpc: capture the violated protection key on fault
In reply to Ram PaiCapture the protection key that got violated in paca. This value will be later used to inform the signal handler.
--- arch/powerpc/include/asm/paca.h | 1 + arch/powerpc/kernel/asm-offsets.c | 1 + arch/powerpc/mm/fault.c | 8 ++++++++ 3 files changed, 10 insertions(+), 0 deletions(-)
arch/powerpc/include/asm/paca.h +1 -0
@@ -94,6 +94,7 @@ struct paca_struct {u64 dscr_default;/* per-CPU default DSCR */ #ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS u64 paca_amr;/* value of amr at exception */ +u16 paca_pkey;/* exception causing pkey */ #endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ #ifdef CONFIG_PPC_STD_MMU_64
arch/powerpc/kernel/asm-offsets.c +1 -0
@@ -244,6 +244,7 @@ int main(void)#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS OFFSET (PACA_AMR , paca_struct, paca_amr); +OFFSET (PACA_PKEY , paca_struct, paca_pkey);#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ OFFSET (ACCOUNT_STARTTIME , paca_struct, accounting.starttime);
arch/powerpc/mm/fault.c +8 -0
@@ -265,6 +265,7 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,if (error_code &DSISR_KEYFAULT ) { code =SEGV_PKUERR ;get_paca ()->paca_amr =read_amr (); +get_paca ()->paca_pkey =get_pte_pkey (current->mm, address); goto bad_area_nosemaphore; }#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
@@ -453,6 +454,13 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,if (!arch_vma_access_permitted (vma, flags &FAULT_FLAG_WRITE , is_exec,0 )) {get_paca ()->paca_amr =read_amr (); +/* +* The pgd-pdt...pmd-pte tree may not have been fully setup. +* Hence we cannot walk the tree to locate the pte, to locate +* the key. Hence lets use vma_pkey() to get the key; instead +* of get_pte_pkey(). +*/ +get_paca ()->paca_pkey =vma_pkey (vma); code =SEGV_PKUERR ; goto bad_area; }
Signature · 1 line
1.7.1
RFC v7 24/25 powerpc: Deliver SEGV signal on pkey violation
In reply to Ram PaiThe value of the AMR register at the time of exception is made available in gp_regs[PT_AMR] of the siginfo.
The value of the pkey, whose protection got violated, is made available in si_pkey field of the siginfo structure.
--- arch/powerpc/include/uapi/asm/ptrace.h | 1 + arch/powerpc/kernel/signal_32.c | 5 +++++ arch/powerpc/kernel/signal_64.c | 4 ++++ arch/powerpc/kernel/traps.c | 15 +++++++++++++++ 4 files changed, 25 insertions(+), 0 deletions(-)
arch/powerpc/include/uapi/asm/ptrace.h +1 -0
@@ -110,6 +110,7 @@ struct pt_regs {#define PT_RESULT 43#define PT_DSCR 44#define PT_REGS_COUNT 44 +#define PT_AMR 45#define PT_FPR0 48/* each FP reg occupies 2 slots in this space */
arch/powerpc/kernel/signal_32.c +5 -0
@@ -500,6 +500,11 @@ static int save_user_regs(struct pt_regs *regs, struct mcontext __user *frame, (unsigned long) &frame->tramp[2]); } +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +if (__put_user (get_paca ()->paca_amr, &frame->mc_gregs[PT_AMR ])) +return 1 ; +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ +return 0 ; }
arch/powerpc/kernel/signal_64.c +4 -0
@@ -174,6 +174,10 @@ static long setup_sigcontext(struct sigcontext __user *sc,if (set !=NULL ) err |=__put_user (set->sig[0 ], &sc->oldmask); +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS + err |=__put_user (get_paca ()->paca_amr, &sc->gp_regs[PT_AMR ]); +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ +return err; }
arch/powerpc/kernel/traps.c +15 -0
@@ -20,6 +20,7 @@#include <linux/sched/debug.h> #include <linux/kernel.h> #include <linux/mm.h> +#include <linux/pkeys.h> #include <linux/stddef.h> #include <linux/unistd.h> #include <linux/ptrace.h>
@@ -247,6 +248,15 @@ void user_single_step_siginfo(struct task_struct *tsk, info->si_addr = (void __user *)regs->nip; } +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +static void fill_sig_info_pkey (int si_code,siginfo_t *info,unsigned long addr) +{ +if (si_code !=SEGV_PKUERR ) +return ; + info->si_pkey =get_paca ()->paca_pkey; +} +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ +void _exception (int signr,struct pt_regs *regs,int code,unsigned long addr) {siginfo_t info;
@@ -274,6 +284,11 @@ void _exception(int signr, struct pt_regs *regs, int code, unsigned long addr) info.si_signo = signr; info.si_code = code; info.si_addr = (void __user *) addr; + +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +fill_sig_info_pkey (code, &info, addr); +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ +force_sig_info (signr, &info, current); }
Signature · 1 line
1.7.1
Re: [RFC v7 24/25] powerpc: Deliver SEGV signal on pkey violation
Quoted 5 lines · Ram Pai
The value of the AMR register at the time of exception is made available in gp_regs[PT_AMR] of the siginfo.
The value of the pkey, whose protection got violated, is made available in
si_pkeyfield of the siginfo structure.
Should the IAMR also be made available?
Also, should the AMR and IAMR be accesible to userspace (e.g., to GDB) via ptrace and the core file?
arch/powerpc/kernel/signal_32.c static int save_user_regs(struct pt_regs *regs, struct mcontext __user *frame, · line 509
Quoted 13 lines
arch/powerpc/kernel/signal_32.c +5
-0@@ -500,6 +500,11 @@ static int save_user_regs(struct pt_regs *regs, struct mcontext __user *frame, (unsigned long) &frame->tramp[2]); } +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +if (__put_user (get_paca ()->paca_amr, &frame->mc_gregs[PT_AMR ])) +return 1 ; +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ +return 0 ; }
frame->mc_gregs[PT_AMR] has 32 bits, but paca_amr has 64 bits. Does this work as intended?
Quoted 5 lines
arch/powerpc/kernel/signal_64.c +0
-0
arch/powerpc/kernel/signal_64.c static long setup_sigcontext(struct sigcontext __user *sc, · line 182
Quoted 9 lines
arch/powerpc/kernel/signal_64.c +4
-0@@ -174,6 +174,10 @@ static long setup_sigcontext(struct sigcontext __user *sc,if (set !=NULL ) err |=__put_user (set->sig[0 ], &sc->oldmask); +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS + err |=__put_user (get_paca ()->paca_amr, &sc->gp_regs[PT_AMR ]); +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ +return err; }
Isn't a corresponding change needed in restore_sigcontext? And in the corresponding TM versions setup_tm_sigcontexts and restore_tm_sigcontexts?
Ditto for the equivalent functions in signal_32.c.
Signature · 2 lines
Thiago Jung Bauermann IBM Linux Technology Center
Michael Ellerman
reviewerRe: [RFC v7 24/25] powerpc: Deliver SEGV signal on pkey violation
Quoted 5 lines · Thiago Jung Bauermann
Quoted 3 lines · Ram Pai
The value of the AMR register at the time of exception is made available in gp_regs[PT_AMR] of the siginfo.
Quoted 7 lines
Quoted 2 lines
The value of the pkey, whose protection got violated, is made available in
si_pkeyfield of the siginfo structure.Should the IAMR also be made available?
Also, should the AMR and IAMR be accesible to userspace (e.g., to GDB) via ptrace and the core file?
Yes if they're part of the thread's context they should be accessible via ptrace and in core files.
Quoted 6 lines
Quoted 6 lines
arch/powerpc/kernel/signal_32.c +0
-0@@ -500,6 +500,11 @@ static int save_user_regs(struct pt_regs *regs, struct mcontext __user *frame, (unsigned long) &frame->tramp[2]); }
Quoted 10 lines
arch/powerpc/kernel/signal_32.c static int save_user_regs(struct pt_regs *regs, struct mcontext __user *frame, · line 509
Quoted 7 lines
arch/powerpc/kernel/signal_32.c +5
-0+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +if (__put_user (get_paca ()->paca_amr, &frame->mc_gregs[PT_AMR ])) +return 1 ; +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ +return 0 ; }frame->mc_gregs[PT_AMR] has 32 bits, but
paca_amrhas 64 bits. Does this work as intended?
I don't understand why we are putting it in there at all?
Is there some special handling of the actual register on signals? I haven't seen it. In which case the process can get the value of AMR by reading the register. ??
cheers
Re: [RFC v7 24/25] powerpc: Deliver SEGV signal on pkey violation
Quoted 10 lines · Michael Ellerman
Quoted 8 lines · Thiago Jung Bauermann
Quoted 5 lines · Ram Pai
The value of the AMR register at the time of exception is made available in gp_regs[PT_AMR] of the siginfo.
The value of the pkey, whose protection got violated, is made available in
si_pkeyfield of the siginfo structure.
Quoted 7 lines
Quoted 4 lines
Should the IAMR also be made available?
Also, should the AMR and IAMR be accesible to userspace (e.g., to GDB) via ptrace and the core file?
Yes if they're part of the thread's context they should be accessible via ptrace and in core files.
ok. Some more code needed. :(
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
arch/powerpc/kernel/signal_32.c +0
-0@@ -500,6 +500,11 @@ static int save_user_regs(struct pt_regs *regs, struct mcontext __user *frame, (unsigned long) &frame->tramp[2]); }
Quoted 10 lines
Quoted 10 lines
arch/powerpc/kernel/signal_32.c static int save_user_regs(struct pt_regs *regs, struct mcontext __user *frame, · line 509
Quoted 7 lines
arch/powerpc/kernel/signal_32.c +5
-0+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS +if (__put_user (get_paca ()->paca_amr, &frame->mc_gregs[PT_AMR ])) +return 1 ; +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ +return 0 ; }frame->mc_gregs[PT_AMR] has 32 bits, but
paca_amrhas 64 bits. Does this work as intended?
hmm..i think we should just disable pkey support for 32 bit apps, till we figure out all the edge cases.
Quoted 6 lines
I don't understand why we are putting it in there at all?
Is there some special handling of the actual register on signals? I haven't seen it. In which case the process can get the value of AMR by reading the register. ??
The value of AMR register at the time of the key-exception may not be the same when the signal handler is invoked.
RP
Michael Ellerman
reviewerRe: [RFC v7 24/25] powerpc: Deliver SEGV signal on pkey violation
Quoted 7 lines · Ram Pai
Quoted 6 lines · Michael Ellerman
Quoted 4 lines · Thiago Jung Bauermann
Quoted 2 lines · Ram Pai
The value of the AMR register at the time of exception is made available in gp_regs[PT_AMR] of the siginfo.
...
Quoted 9 lines
Quoted 6 lines
I don't understand why we are putting it in there at all?
Is there some special handling of the actual register on signals? I haven't seen it. In which case the process can get the value of AMR by reading the register. ??
The value of AMR register at the time of the key-exception may not be the same when the signal handler is invoked.
Why not?
cheers
Re: [RFC v7 24/25] powerpc: Deliver SEGV signal on pkey violation
Quoted 12 lines · Michael Ellerman
Quoted 7 lines · Ram Pai
Quoted 6 lines · Michael Ellerman
Quoted 4 lines · Thiago Jung Bauermann
Quoted 2 lines · Ram Pai
The value of the AMR register at the time of exception is made available in gp_regs[PT_AMR] of the siginfo.
...
Quoted 3 lines
Quoted 3 lines
I don't understand why we are putting it in there at all?
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
Is there some special handling of the actual register on signals? I haven't seen it. In which case the process can get the value of AMR by reading the register. ??
The value of AMR register at the time of the key-exception may not be the same when the signal handler is invoked.
Why not?
Assume two threads of a task.
T1: mprotect_key(foo, PAGE_SIZE, pkey=4);
T1: set AMR to disable access for pkey 4;
T1: key fault
T2: set AMR to enable access to pkey 4;T1: fault handler called.
This fault handler will see the new AMR and not the one at the time of the fault.
RP
Re: [RFC v7 24/25] powerpc: Deliver SEGV signal on pkey violation
Quoted 9 lines · Ram Pai
Assume two threads of a task.
T1: mprotect_key(foo, PAGE_SIZE, pkey=4); T1: set AMR to disable access for pkey 4; T1: key fault T2: set AMR to enable access to pkey 4;T1: fault handler called.
This fault handler will see the new AMR and not the one at the time of the fault.
You aren't context switching AMR with the threads ? Ugh... something is very wrong then.
Ben.
Re: [RFC v7 24/25] powerpc: Deliver SEGV signal on pkey violation
Quoted 11 lines · Benjamin Herrenschmidt
Quoted 9 lines · Ram Pai
Assume two threads of a task.
T1: mprotect_key(foo, PAGE_SIZE, pkey=4); T1: set AMR to disable access for pkey 4; T1: key fault T2: set AMR to enable access to pkey 4;T1: fault handler called.
This fault handler will see the new AMR and not the one at the time of the fault.
Quoted 2 lines
You aren't context switching AMR with the threads ? Ugh... something is very wrong then.
I do store and restore AMR accross context switch. So nevermind; the above problem cannot happen.
RP
Re: [RFC v7 24/25] powerpc: Deliver SEGV signal on pkey violation
Quoted 12 lines · On Fri, 18 Aug 2017 15:36:55 -0700 Ram Pai <[email protected]> wrote:
Quoted 11 lines · Benjamin Herrenschmidt
Quoted 9 lines · Ram Pai
Assume two threads of a task.
T1: mprotect_key(foo, PAGE_SIZE, pkey=4); T1: set AMR to disable access for pkey 4; T1: key fault T2: set AMR to enable access to pkey 4;T1: fault handler called.
This fault handler will see the new AMR and not the one at the time of the fault.
Quoted 6 lines
Quoted 2 lines
You aren't context switching AMR with the threads ? Ugh... something is very wrong then.
I do store and restore AMR accross context switch. So nevermind; the above problem cannot happen.
I think the assumption is that pkey_alloc() will do the right thing while allocating keys across threads
Balbir Singh.
Re: [RFC v7 24/25] powerpc: Deliver SEGV signal on pkey violation
Quoted 18 lines · Balbir Singh
Quoted 15 lines · On Fri, 18 Aug 2017 15:36:55 -0700 Ram Pai <[email protected]> wrote:
Quoted 13 lines · Benjamin Herrenschmidt
Quoted 9 lines · Ram Pai
Assume two threads of a task.
T1: mprotect_key(foo, PAGE_SIZE, pkey=4); T1: set AMR to disable access for pkey 4; T1: key fault T2: set AMR to enable access to pkey 4;T1: fault handler called.
This fault handler will see the new AMR and not the one at the time of the fault.You aren't context switching AMR with the threads ? Ugh... something is very wrong then.
Quoted 6 lines
Quoted 3 lines
I do store and restore AMR accross context switch. So nevermind; the above problem cannot happen.
I think the assumption is that
pkey_alloc()will do the right thing while allocating keys across threads
It does. A key allocated to a thread will never be allocated to another thread.
RP
Re: [RFC v7 24/25] powerpc: Deliver SEGV signal on pkey violation
In reply to Ram PaiQuoted 24 lines · Ram Pai
Quoted 12 lines · Michael Ellerman
Quoted 7 lines · Ram Pai
Quoted 6 lines · Michael Ellerman
Quoted 4 lines · Thiago Jung Bauermann
Quoted 2 lines · Ram Pai
The value of the AMR register at the time of exception is made available in gp_regs[PT_AMR] of the siginfo.
...
Quoted 3 lines
Quoted 3 lines
I don't understand why we are putting it in there at all?
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
Is there some special handling of the actual register on signals? I haven't seen it. In which case the process can get the value of AMR by reading the register. ??
The value of AMR register at the time of the key-exception may not be the same when the signal handler is invoked.
Why not?
Assume two threads of a task.
Quoted 7 lines
T1: mprotect_key(foo, PAGE_SIZE, pkey=4); T1: set AMR to disable access for pkey 4; T1: key fault T2: set AMR to enable access to pkey 4;T1: fault handler called.
This fault handler will see the new AMR and not the one at the time of the fault.
Ok. Ben debunked my above reason. So at this point I dont have a solid reason to defend my statement --
"The value of AMR register at the time of the key-exception may not be the same when the signal handler is invoked."
Coming back to the your main question, "why we need to provide the
contents of AMR register to the signal handler?" -- the only reason
i can see is, probably tools like gdb and ptrace may find it useful.
And since it was suggested that content of IAMR is also useful to the application, the value of which cannot be accessed from userspace, it may make sense to provide both the contents.
Please suggest.
RP
Re: [RFC v7 24/25] powerpc: Deliver SEGV signal on pkey violation
Quoted 7 lines · Ram Pai
Coming back to the your main question, "why we need to provide the
contents of AMR register to the signal handler?" -- the only reason
i can see is, probably tools like gdb and ptrace may find it useful.And since it was suggested that content of IAMR is also useful to the application, the value of which cannot be accessed from userspace, it may make sense to provide both the contents.
I'd rather you added ptrace calls to obtain & set it. I don't think we need to touch the signal frame, it's too much of an ABI compatibility burden.
Cheers,
Ben.
RFC v7 25/25 powerpc: Enable pkey subsystem
In reply to Ram PaiPAPR defines 'ibm,processor-storage-keys' property. It exports two values.The first value indicates the number of data-access keys and the second indicates the number of instruction-access keys. Though this alludes that keys can be either data access or instructon access only, that is not the case in reality.Any key can be of any kind. This patch adds all the keys and uses that as the total number of keys available to us.
Non PAPR platforms do not define this property in the device
tree yet. Here, we hardcode CPUs that support pkey by
consulting PowerISA3.0
--- arch/powerpc/include/asm/cputable.h | 8 +++++--- arch/powerpc/include/asm/mmu_context.h | 1 + arch/powerpc/include/asm/pkeys.h | 32 ++++++++++++++++++++++++++++++-- arch/powerpc/kernel/prom.c | 19 +++++++++++++++++++ 4 files changed, 55 insertions(+), 5 deletions(-)
arch/powerpc/include/asm/cputable.h +5 -3
@@ -214,6 +214,7 @@ enum {#define CPU_FTR_DAWR LONG_ASM_CONST(0x0400000000000000)#define CPU_FTR_DABRX LONG_ASM_CONST(0x0800000000000000)#define CPU_FTR_PMAO_BUG LONG_ASM_CONST(0x1000000000000000) +#define CPU_FTR_PKEY LONG_ASM_CONST(0x2000000000000000)#define CPU_FTR_POWER9_DD1 LONG_ASM_CONST(0x4000000000000000)#ifndef __ASSEMBLY__
@@ -452,7 +453,7 @@ enum { CPU_FTR_DSCR | CPU_FTR_SAO | CPU_FTR_ASYM_SMT | \ CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \ CPU_FTR_ICSWX | CPU_FTR_CFAR | CPU_FTR_HVMODE | \ - CPU_FTR_VMX_COPY | CPU_FTR_HAS_PPR | CPU_FTR_DABRX) + CPU_FTR_VMX_COPY | CPU_FTR_HAS_PPR | CPU_FTR_DABRX | CPU_FTR_PKEY)#define CPU_FTRS_POWER8 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \ CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | CPU_FTR_ARCH_206 |\ CPU_FTR_MMCRA | CPU_FTR_SMT | \
@@ -462,7 +463,7 @@ enum { CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \ CPU_FTR_ICSWX | CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \ CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \ - CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP) + CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_PKEY)#define CPU_FTRS_POWER8E (CPU_FTRS_POWER8 | CPU_FTR_PMAO_BUG)#define CPU_FTRS_POWER8_DD1 (CPU_FTRS_POWER8 & ~CPU_FTR_DBELL)#define CPU_FTRS_POWER9 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
@@ -474,7 +475,8 @@ enum { CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \ CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \ CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \ - CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_ARCH_300) + CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_ARCH_300 | \ + CPU_FTR_PKEY)#define CPU_FTRS_POWER9_DD1 ((CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD1) & \ (~CPU_FTR_SAO))#define CPU_FTRS_CELL (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
arch/powerpc/include/asm/mmu_context.h +1 -0
@@ -188,6 +188,7 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,#define pkey_initialize ()#define pkey_mm_init (mm) +#define pkey_mmu_values (total_data, total_execute)static inline int vma_pkey (struct vm_area_struct *vma) {
arch/powerpc/include/asm/pkeys.h +30 -2
@@ -1,6 +1,8 @@#ifndef _ASM_PPC64_PKEYS_H#define _ASM_PPC64_PKEYS_H +#include <asm/firmware.h> +extern bool pkey_inited;extern int pkeys_total;/* total pkeys as per device tree */ extern u32 initial_allocation_mask;/* bits set for reserved keys */
@@ -227,6 +229,24 @@ static inline void pkey_mm_init(struct mm_struct *mm) mm->context.execute_only_pkey =-1 ; } +static inline void pkey_mmu_values (int total_data,int total_execute) +{ +/* +* since any pkey can be used for data or execute, we +* will just treat all keys as equal and track them +* as one entity. +*/ + pkeys_total = total_data + total_execute; +} + +static inline bool pkey_mmu_enabled (void ) +{ +if (firmware_has_feature (FW_FEATURE_LPAR )) +return pkeys_total; +else +return cpu_has_feature (CPU_FTR_PKEY ); +} +static inline void pkey_initialize (void ) {int os_reserved, i;
@@ -236,9 +256,17 @@ static inline void pkey_initialize(void)* line will enable it. */ pkey_inited = false; +if (pkey_mmu_enabled ()) + pkey_inited = !radix_enabled (); + +if (!pkey_inited) +return ; -/* Lets assume 32 keys */ - pkeys_total =32 ; +/* Lets assume 32 keys if we are not told +* the number of pkeys. +*/ +if (!pkeys_total) + pkeys_total =32 ;#ifdef CONFIG_PPC_4K_PAGES /*
arch/powerpc/kernel/prom.c +19 -0
@@ -35,6 +35,7 @@#include <linux/of_fdt.h> #include <linux/libfdt.h> #include <linux/cpu.h> +#include <linux/pkeys.h> #include <asm/prom.h> #include <asm/rtas.h>
@@ -228,6 +229,23 @@ static void __init check_cpu_pa_features(unsigned long node) ibm_pa_features, ARRAY_SIZE(ibm_pa_features)); } +static void __init check_cpu_pkey_feature (unsigned long node) +{ +const __be32 *ftrs; +int len, total_data, total_execute; + + ftrs =of_get_flat_dt_prop (node, +"ibm,processor-storage-keys" , &len); +if (ftrs ==NULL ) +return ; + + len /=sizeof (int ); + total_execute = (len >=2 ) ?be32_to_cpu (ftrs[1 ]) :0 ; + total_data = (len >=1 ) ?be32_to_cpu (ftrs[0 ]) :0 ; +pkey_mmu_values (total_data, total_execute); +} + +#ifdef CONFIG_PPC_STD_MMU_64 static void __init init_mmu_slb_size (unsigned long node) {
@@ -391,6 +409,7 @@ static int __init early_init_dt_scan_cpus(unsigned long node,check_cpu_feature_properties (node);check_cpu_pa_features (node); +check_cpu_pkey_feature (node); }identical_pvr_fixup (node);
Signature · 1 line
1.7.1
Re: [RFC v7 25/25] powerpc: Enable pkey subsystem
Quoted 11 lines · Ram Pai
arch/powerpc/include/asm/cputable.h +1
-0@@ -214,6 +214,7 @@ enum {#define CPU_FTR_DAWR LONG_ASM_CONST(0x0400000000000000)#define CPU_FTR_DABRX LONG_ASM_CONST(0x0800000000000000)#define CPU_FTR_PMAO_BUG LONG_ASM_CONST(0x1000000000000000) +#define CPU_FTR_PKEY LONG_ASM_CONST(0x2000000000000000)#define CPU_FTR_POWER9_DD1 LONG_ASM_CONST(0x4000000000000000)#ifndef __ASSEMBLY__
arch/powerpc/include/asm/cputable.h enum { · line 456
Quoted 5 lines
arch/powerpc/include/asm/cputable.h +1
-1@@ -452,7 +453,7 @@ enum {CPU_FTR_DSCR |CPU_FTR_SAO |CPU_FTR_ASYM_SMT | \CPU_FTR_STCX_CHECKS_ADDRESS |CPU_FTR_POPCNTB |CPU_FTR_POPCNTD | \CPU_FTR_ICSWX |CPU_FTR_CFAR |CPU_FTR_HVMODE | \ - CPU_FTR_VMX_COPY | CPU_FTR_HAS_PPR | CPU_FTR_DABRX) + CPU_FTR_VMX_COPY | CPU_FTR_HAS_PPR | CPU_FTR_DABRX | CPU_FTR_PKEY)
P7 supports protection keys for data access (AMR) but not for instruction access (IAMR), right? There's nothing in the code making this distinction, so either CPU_FTR_PKEY shouldn't be enabled in P7 or separate feature bits for AMR and IAMR should be used and checked before trying to access the IAMR.
Quoted 50 lines
arch/powerpc/include/asm/cputable.h +3
-2#define CPU_FTRS_POWER8 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \ CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | CPU_FTR_ARCH_206 |\ CPU_FTR_MMCRA | CPU_FTR_SMT | \@@ -462,7 +463,7 @@ enum { CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \ CPU_FTR_ICSWX | CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \ CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \ - CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP) + CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_PKEY)#define CPU_FTRS_POWER8E (CPU_FTRS_POWER8 | CPU_FTR_PMAO_BUG)#define CPU_FTRS_POWER8_DD1 (CPU_FTRS_POWER8 & ~CPU_FTR_DBELL)#define CPU_FTRS_POWER9 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \@@ -474,7 +475,8 @@ enum { CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \ CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \ CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \ - CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_ARCH_300) + CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_ARCH_300 | \ + CPU_FTR_PKEY)#define CPU_FTRS_POWER9_DD1 ((CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD1) & \ (~CPU_FTR_SAO))#define CPU_FTRS_CELL (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \arch/powerpc/include/asm/mmu_context.h +1
-0@@ -188,6 +188,7 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,#define pkey_initialize ()#define pkey_mm_init (mm) +#define pkey_mmu_values (total_data, total_execute)static inline int vma_pkey (struct vm_area_struct *vma) {arch/powerpc/include/asm/pkeys.h +2
-0@@ -1,6 +1,8 @@#ifndef _ASM_PPC64_PKEYS_H#define _ASM_PPC64_PKEYS_H +#include <asm/firmware.h> +extern bool pkey_inited;extern int pkeys_total;/* total pkeys as per device tree */ extern u32 initial_allocation_mask;/* bits set for reserved keys */ @@ -227,6 +229,24 @@ static inline void pkey_mm_init(struct mm_struct *mm) mm->context.execute_only_pkey =-1 ; }
arch/powerpc/include/asm/pkeys.h static inline void pkey_mm_init(struct mm_struct *mm) · line 240
Quoted 10 lines
arch/powerpc/include/asm/pkeys.h +9
-0+static inline void pkey_mmu_values (int total_data,int total_execute) +{ +/* +* since any pkey can be used for data or execute, we +* will just treat all keys as equal and track them +* as one entity. +*/ + pkeys_total = total_data + total_execute; +}
Right now this works because the firmware reports 0 execute keys in the device tree, but if (when?) it is fixed to report 32 execute keys as well as 32 data keys (which are the same keys), any place using pkeys_total expecting it to mean the number of keys that are available will be broken. This includes pkey_initialize and mm_pkey_is_allocated.
Perhaps pkeys_total should use total_data as the number of keys supported in the system, and total_execute just as a flag to say whether there's a IAMR? Or, since P8 and later have IAMR and P7 is unlikely to have the firmware fixed, maybe the kernel should just ignore total_execute altogether?
Quoted 21 lines
arch/powerpc/include/asm/pkeys.h +13
-0+static inline bool pkey_mmu_enabled (void ) +{ +if (firmware_has_feature (FW_FEATURE_LPAR )) +return pkeys_total; +else +return cpu_has_feature (CPU_FTR_PKEY ); +} +static inline void pkey_initialize (void ) {int os_reserved, i;@@ -236,9 +256,17 @@ static inline void pkey_initialize(void)* line will enable it. */ pkey_inited = false; +if (pkey_mmu_enabled ()) + pkey_inited = !radix_enabled (); + +if (!pkey_inited) +return ;
arch/powerpc/include/asm/pkeys.h static inline void pkey_initialize(void) · line 272
Quoted 10 lines
arch/powerpc/include/asm/pkeys.h +5
-2-/* Lets assume 32 keys */ - pkeys_total =32 ; +/* Lets assume 32 keys if we are not told +* the number of pkeys. +*/ +if (!pkeys_total) + pkeys_total =32 ;#ifdef CONFIG_PPC_4K_PAGES /*
This patch should remove the comment "disable the pkey system till everything is in place. A patch further down the line will enable it.".
Signature · 2 lines
Thiago Jung Bauermann IBM Linux Technology Center
Re: [RFC v7 25/25] powerpc: Enable pkey subsystem
Quoted 11 lines · Thiago Jung Bauermann
Quoted 9 lines · Ram Pai
arch/powerpc/include/asm/cputable.h +1
-0@@ -214,6 +214,7 @@ enum {#define CPU_FTR_DAWR LONG_ASM_CONST(0x0400000000000000)#define CPU_FTR_DABRX LONG_ASM_CONST(0x0800000000000000)#define CPU_FTR_PMAO_BUG LONG_ASM_CONST(0x1000000000000000) +#define CPU_FTR_PKEY LONG_ASM_CONST(0x2000000000000000)#define CPU_FTR_POWER9_DD1 LONG_ASM_CONST(0x4000000000000000)
Quoted 13 lines
arch/powerpc/include/asm/cputable.h enum { · line 456
Quoted 7 lines
arch/powerpc/include/asm/cputable.h +1
-1#ifndef __ASSEMBLY__@@ -452,7 +453,7 @@ enum {CPU_FTR_DSCR |CPU_FTR_SAO |CPU_FTR_ASYM_SMT | \CPU_FTR_STCX_CHECKS_ADDRESS |CPU_FTR_POPCNTB |CPU_FTR_POPCNTD | \CPU_FTR_ICSWX |CPU_FTR_CFAR |CPU_FTR_HVMODE | \ - CPU_FTR_VMX_COPY | CPU_FTR_HAS_PPR | CPU_FTR_DABRX) + CPU_FTR_VMX_COPY | CPU_FTR_HAS_PPR | CPU_FTR_DABRX | CPU_FTR_PKEY)P7 supports protection keys for data access (AMR) but not for instruction access (IAMR), right? There's nothing in the code making this distinction, so either
CPU_FTR_PKEYshouldn't be enabled in P7 or separate feature bits for AMR and IAMR should be used and checked before trying to access the IAMR.
did'nt David say P7 supports both? P6, i think, only support data. my pkey tests have passed on p7.
Quoted 52 lines
Quoted 52 lines
arch/powerpc/include/asm/cputable.h +3
-2#define CPU_FTRS_POWER8 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \ CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | CPU_FTR_ARCH_206 |\ CPU_FTR_MMCRA | CPU_FTR_SMT | \@@ -462,7 +463,7 @@ enum { CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \ CPU_FTR_ICSWX | CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \ CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \ - CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP) + CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_PKEY)#define CPU_FTRS_POWER8E (CPU_FTRS_POWER8 | CPU_FTR_PMAO_BUG)#define CPU_FTRS_POWER8_DD1 (CPU_FTRS_POWER8 & ~CPU_FTR_DBELL)#define CPU_FTRS_POWER9 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \@@ -474,7 +475,8 @@ enum { CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \ CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \ CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \ - CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_ARCH_300) + CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_ARCH_300 | \ + CPU_FTR_PKEY)#define CPU_FTRS_POWER9_DD1 ((CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD1) & \ (~CPU_FTR_SAO))#define CPU_FTRS_CELL (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \arch/powerpc/include/asm/mmu_context.h +1
-0@@ -188,6 +188,7 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,#define pkey_initialize ()#define pkey_mm_init (mm) +#define pkey_mmu_values (total_data, total_execute)static inline int vma_pkey (struct vm_area_struct *vma) {arch/powerpc/include/asm/pkeys.h +2
-0@@ -1,6 +1,8 @@#ifndef _ASM_PPC64_PKEYS_H#define _ASM_PPC64_PKEYS_H +#include <asm/firmware.h> +extern bool pkey_inited;extern int pkeys_total;/* total pkeys as per device tree */ extern u32 initial_allocation_mask;/* bits set for reserved keys */ @@ -227,6 +229,24 @@ static inline void pkey_mm_init(struct mm_struct *mm) mm->context.execute_only_pkey =-1 ; }
Quoted 15 lines
arch/powerpc/include/asm/pkeys.h static inline void pkey_mm_init(struct mm_struct *mm) · line 240
Quoted 9 lines
arch/powerpc/include/asm/pkeys.h +9
-0+static inline void pkey_mmu_values (int total_data,int total_execute) +{ +/* +* since any pkey can be used for data or execute, we +* will just treat all keys as equal and track them +* as one entity. +*/ + pkeys_total = total_data + total_execute; +}Right now this works because the firmware reports 0 execute keys in the device tree, but if (when?) it is fixed to report 32 execute keys as well as 32 data keys (which are the same keys), any place using
pkeys_totalexpecting it to mean the number of keys that are available will be broken. This includespkey_initializeandmm_pkey_is_allocated.
Good point. we should just ignore total_execute. It should be the same value as total_data on the latest platforms. On older platforms it will continue to be zero.
Quoted 36 lines
Perhaps
pkeys_totalshould usetotal_dataas the number of keys supported in the system, andtotal_executejust as a flag to say whether there's a IAMR? Or, since P8 and later have IAMR and P7 is unlikely to have the firmware fixed, maybe the kernel should just ignoretotal_executealtogether?Quoted 29 lines
arch/powerpc/include/asm/pkeys.h +18
-2+static inline bool pkey_mmu_enabled (void ) +{ +if (firmware_has_feature (FW_FEATURE_LPAR )) +return pkeys_total; +else +return cpu_has_feature (CPU_FTR_PKEY ); +} +static inline void pkey_initialize (void ) {int os_reserved, i;@@ -236,9 +256,17 @@ static inline void pkey_initialize(void)* line will enable it. */ pkey_inited = false; +if (pkey_mmu_enabled ()) + pkey_inited = !radix_enabled (); + +if (!pkey_inited) +return ; -/* Lets assume 32 keys */ - pkeys_total =32 ; +/* Lets assume 32 keys if we are not told +* the number of pkeys. +*/ +if (!pkeys_total) + pkeys_total =32 ;
Quoted 5 lines
arch/powerpc/include/asm/pkeys.h static inline void pkey_initialize(void) · line 272
Quoted 2 lines
arch/powerpc/include/asm/pkeys.h +0
-0#ifdef CONFIG_PPC_4K_PAGES /* This patch should remove the comment "disable the pkey system till everything is in place. A patch further down the line will enable it.".
Yes.
RP
Re: [RFC v7 25/25] powerpc: Enable pkey subsystem
Quoted 26 lines · Ram Pai
Quoted 11 lines · Thiago Jung Bauermann
Quoted 9 lines · Ram Pai
arch/powerpc/include/asm/cputable.h +1
-0@@ -214,6 +214,7 @@ enum {#define CPU_FTR_DAWR LONG_ASM_CONST(0x0400000000000000)#define CPU_FTR_DABRX LONG_ASM_CONST(0x0800000000000000)#define CPU_FTR_PMAO_BUG LONG_ASM_CONST(0x1000000000000000) +#define CPU_FTR_PKEY LONG_ASM_CONST(0x2000000000000000)#define CPU_FTR_POWER9_DD1 LONG_ASM_CONST(0x4000000000000000)Quoted 13 lines
arch/powerpc/include/asm/cputable.h enum { · line 456
Quoted 7 lines
arch/powerpc/include/asm/cputable.h +1
-1#ifndef __ASSEMBLY__@@ -452,7 +453,7 @@ enum {CPU_FTR_DSCR |CPU_FTR_SAO |CPU_FTR_ASYM_SMT | \CPU_FTR_STCX_CHECKS_ADDRESS |CPU_FTR_POPCNTB |CPU_FTR_POPCNTD | \CPU_FTR_ICSWX |CPU_FTR_CFAR |CPU_FTR_HVMODE | \ - CPU_FTR_VMX_COPY | CPU_FTR_HAS_PPR | CPU_FTR_DABRX) + CPU_FTR_VMX_COPY | CPU_FTR_HAS_PPR | CPU_FTR_DABRX | CPU_FTR_PKEY)P7 supports protection keys for data access (AMR) but not for instruction access (IAMR), right? There's nothing in the code making this distinction, so either
CPU_FTR_PKEYshouldn't be enabled in P7 or separate feature bits for AMR and IAMR should be used and checked before trying to access the IAMR.
Quoted 2 lines
did'nt David say P7 supports both? P6, i think, only support data. my pkey tests have passed on p7.
He said that P7 was the first processor to support 32 keys, but if you look at the Virtual Page Class Key Protection section in ISA 2.06, there's no IAMR.
There was a bug in the code where init_iamr was calling write_amr instead of write_iamr, perhaps that's why it worked when you tested on P7?
Quoted 68 lines
Quoted 52 lines
Quoted 52 lines
arch/powerpc/include/asm/cputable.h +3
-2#define CPU_FTRS_POWER8 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \ CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | CPU_FTR_ARCH_206 |\ CPU_FTR_MMCRA | CPU_FTR_SMT | \@@ -462,7 +463,7 @@ enum { CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \ CPU_FTR_ICSWX | CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \ CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \ - CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP) + CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_PKEY)#define CPU_FTRS_POWER8E (CPU_FTRS_POWER8 | CPU_FTR_PMAO_BUG)#define CPU_FTRS_POWER8_DD1 (CPU_FTRS_POWER8 & ~CPU_FTR_DBELL)#define CPU_FTRS_POWER9 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \@@ -474,7 +475,8 @@ enum { CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \ CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \ CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \ - CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_ARCH_300) + CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_ARCH_300 | \ + CPU_FTR_PKEY)#define CPU_FTRS_POWER9_DD1 ((CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD1) & \ (~CPU_FTR_SAO))#define CPU_FTRS_CELL (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \arch/powerpc/include/asm/mmu_context.h +1
-0@@ -188,6 +188,7 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,#define pkey_initialize ()#define pkey_mm_init (mm) +#define pkey_mmu_values (total_data, total_execute)static inline int vma_pkey (struct vm_area_struct *vma) {arch/powerpc/include/asm/pkeys.h +2
-0@@ -1,6 +1,8 @@#ifndef _ASM_PPC64_PKEYS_H#define _ASM_PPC64_PKEYS_H +#include <asm/firmware.h> +extern bool pkey_inited;extern int pkeys_total;/* total pkeys as per device tree */ extern u32 initial_allocation_mask;/* bits set for reserved keys */ @@ -227,6 +229,24 @@ static inline void pkey_mm_init(struct mm_struct *mm) mm->context.execute_only_pkey =-1 ; }Quoted 15 lines
arch/powerpc/include/asm/pkeys.h static inline void pkey_mm_init(struct mm_struct *mm) · line 240
Quoted 9 lines
arch/powerpc/include/asm/pkeys.h +9
-0+static inline void pkey_mmu_values (int total_data,int total_execute) +{ +/* +* since any pkey can be used for data or execute, we +* will just treat all keys as equal and track them +* as one entity. +*/ + pkeys_total = total_data + total_execute; +}Right now this works because the firmware reports 0 execute keys in the device tree, but if (when?) it is fixed to report 32 execute keys as well as 32 data keys (which are the same keys), any place using
pkeys_totalexpecting it to mean the number of keys that are available will be broken. This includespkey_initializeandmm_pkey_is_allocated.
Quoted 3 lines
Good point. we should just ignore
total_execute. It should be the same value astotal_dataon the latest platforms. On older platforms it will continue to be zero.
Indeed. There should just be a special case to disable execute protection for P7.
Quoted 5 lines
Quoted 5 lines
Perhaps
pkeys_totalshould usetotal_dataas the number of keys supported in the system, andtotal_executejust as a flag to say whether there's a IAMR? Or, since P8 and later have IAMR and P7 is unlikely to have the firmware fixed, maybe the kernel should just ignoretotal_executealtogether?
Signature · 2 lines
Thiago Jung Bauermann IBM Linux Technology Center
Re: [RFC v7 25/25] powerpc: Enable pkey subsystem
Quoted 107 lines · Thiago Jung Bauermann
Quoted 26 lines · Ram Pai
Quoted 11 lines · Thiago Jung Bauermann
Quoted 9 lines · Ram Pai
arch/powerpc/include/asm/cputable.h +1
-0@@ -214,6 +214,7 @@ enum {#define CPU_FTR_DAWR LONG_ASM_CONST(0x0400000000000000)#define CPU_FTR_DABRX LONG_ASM_CONST(0x0800000000000000)#define CPU_FTR_PMAO_BUG LONG_ASM_CONST(0x1000000000000000) +#define CPU_FTR_PKEY LONG_ASM_CONST(0x2000000000000000)#define CPU_FTR_POWER9_DD1 LONG_ASM_CONST(0x4000000000000000)Quoted 13 lines
arch/powerpc/include/asm/cputable.h enum { · line 456
Quoted 7 lines
arch/powerpc/include/asm/cputable.h +1
-1#ifndef __ASSEMBLY__@@ -452,7 +453,7 @@ enum {CPU_FTR_DSCR |CPU_FTR_SAO |CPU_FTR_ASYM_SMT | \CPU_FTR_STCX_CHECKS_ADDRESS |CPU_FTR_POPCNTB |CPU_FTR_POPCNTD | \CPU_FTR_ICSWX |CPU_FTR_CFAR |CPU_FTR_HVMODE | \ - CPU_FTR_VMX_COPY | CPU_FTR_HAS_PPR | CPU_FTR_DABRX) + CPU_FTR_VMX_COPY | CPU_FTR_HAS_PPR | CPU_FTR_DABRX | CPU_FTR_PKEY)P7 supports protection keys for data access (AMR) but not for instruction access (IAMR), right? There's nothing in the code making this distinction, so either
CPU_FTR_PKEYshouldn't be enabled in P7 or separate feature bits for AMR and IAMR should be used and checked before trying to access the IAMR.Quoted 2 lines
did'nt David say P7 supports both? P6, i think, only support data. my pkey tests have passed on p7.
He said that P7 was the first processor to support 32 keys, but if you look at the Virtual Page Class Key Protection section in ISA 2.06, there's no IAMR.
There was a bug in the code where
init_iamrwas callingwrite_amrinstead ofwrite_iamr, perhaps that's why it worked when you tested on P7?Quoted 68 lines
Quoted 67 lines
Quoted 51 lines
arch/powerpc/include/asm/cputable.h +3
-2#define CPU_FTRS_POWER8 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \ CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | CPU_FTR_ARCH_206 |\ CPU_FTR_MMCRA | CPU_FTR_SMT | \@@ -462,7 +463,7 @@ enum { CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \ CPU_FTR_ICSWX | CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \ CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \ - CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP) + CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_PKEY)#define CPU_FTRS_POWER8E (CPU_FTRS_POWER8 | CPU_FTR_PMAO_BUG)#define CPU_FTRS_POWER8_DD1 (CPU_FTRS_POWER8 & ~CPU_FTR_DBELL)#define CPU_FTRS_POWER9 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \@@ -474,7 +475,8 @@ enum { CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \ CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \ CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \ - CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_ARCH_300) + CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_ARCH_300 | \ + CPU_FTR_PKEY)#define CPU_FTRS_POWER9_DD1 ((CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD1) & \ (~CPU_FTR_SAO))#define CPU_FTRS_CELL (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \arch/powerpc/include/asm/mmu_context.h +1
-0@@ -188,6 +188,7 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,#define pkey_initialize ()#define pkey_mm_init (mm) +#define pkey_mmu_values (total_data, total_execute)static inline int vma_pkey (struct vm_area_struct *vma) {arch/powerpc/include/asm/pkeys.h +2
-0@@ -1,6 +1,8 @@#ifndef _ASM_PPC64_PKEYS_H#define _ASM_PPC64_PKEYS_H +#include <asm/firmware.h> +extern bool pkey_inited;extern int pkeys_total;/* total pkeys as per device tree */ extern u32 initial_allocation_mask;/* bits set for reserved keys */ @@ -227,6 +229,24 @@ static inline void pkey_mm_init(struct mm_struct *mm) mm->context.execute_only_pkey =-1 ; }arch/powerpc/include/asm/pkeys.h static inline void pkey_mm_init(struct mm_struct *mm) · line 240
Quoted 10 lines
arch/powerpc/include/asm/pkeys.h +9
-0+static inline void pkey_mmu_values (int total_data,int total_execute) +{ +/* +* since any pkey can be used for data or execute, we +* will just treat all keys as equal and track them +* as one entity. +*/ + pkeys_total = total_data + total_execute; +}Right now this works because the firmware reports 0 execute keys in the device tree, but if (when?) it is fixed to report 32 execute keys as well as 32 data keys (which are the same keys), any place using
pkeys_totalexpecting it to mean the number of keys that are available will be broken. This includespkey_initializeandmm_pkey_is_allocated.
Quoted 6 lines
Quoted 3 lines
Good point. we should just ignore
total_execute. It should be the same value astotal_dataon the latest platforms. On older platforms it will continue to be zero.Indeed. There should just be a special case to disable execute protection for P7.
Ok. we should disable execute protection for P7 and earlier generations of CPU.
RP.
Michael Ellerman
reviewerRe: [RFC v7 25/25] powerpc: Enable pkey subsystem
Quoted 24 lines · Ram Pai
Quoted 23 lines · Thiago Jung Bauermann
Quoted 22 lines · Ram Pai
Quoted 20 lines · Thiago Jung Bauermann
arch/powerpc/include/asm/pkeys.h static inline void pkey_mm_init(struct mm_struct *mm) · line 240
Quoted 13 lines · Ram Pai
arch/powerpc/include/asm/pkeys.h +9
-0@@ -227,6 +229,24 @@ static inline void pkey_mm_init(struct mm_struct *mm) mm->context.execute_only_pkey =-1 ; } +static inline void pkey_mmu_values (int total_data,int total_execute) +{ +/* +* since any pkey can be used for data or execute, we +* will just treat all keys as equal and track them +* as one entity. +*/ + pkeys_total = total_data + total_execute; +}Right now this works because the firmware reports 0 execute keys in the device tree, but if (when?) it is fixed to report 32 execute keys as well as 32 data keys (which are the same keys), any place using
pkeys_totalexpecting it to mean the number of keys that are available will be broken. This includespkey_initializeandmm_pkey_is_allocated.
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
Good point. we should just ignore
total_execute. It should be the same value astotal_dataon the latest platforms. On older platforms it will continue to be zero.Indeed. There should just be a special case to disable execute protection for P7.
Ok. we should disable execute protection for P7 and earlier generations of CPU.
You should do what the device tree says you can do.
If it says there are no execute keys then you shouldn't touch the IAMR.
If you don't want to handle the case where there are 0 execute keys but some data keys then you should do:
total_keys = min(data_keys, exec_keys);
cheers
Re: [RFC v7 25/25] powerpc: Enable pkey subsystem
Quoted 29 lines · Michael Ellerman
Quoted 28 lines · Ram Pai
Quoted 27 lines · Thiago Jung Bauermann
Quoted 25 lines · Ram Pai
Quoted 5 lines · Thiago Jung Bauermann
Quoted 4 lines · Ram Pai
arch/powerpc/include/asm/pkeys.h +0
-0@@ -227,6 +229,24 @@ static inline void pkey_mm_init(struct mm_struct *mm) mm->context.execute_only_pkey =-1 ; }Quoted 15 lines
arch/powerpc/include/asm/pkeys.h static inline void pkey_mm_init(struct mm_struct *mm) · line 240
Quoted 9 lines
arch/powerpc/include/asm/pkeys.h +9
-0+static inline void pkey_mmu_values (int total_data,int total_execute) +{ +/* +* since any pkey can be used for data or execute, we +* will just treat all keys as equal and track them +* as one entity. +*/ + pkeys_total = total_data + total_execute; +}Right now this works because the firmware reports 0 execute keys in the device tree, but if (when?) it is fixed to report 32 execute keys as well as 32 data keys (which are the same keys), any place using
pkeys_totalexpecting it to mean the number of keys that are available will be broken. This includespkey_initializeandmm_pkey_is_allocated.Good point. we should just ignore
total_execute. It should be the same value astotal_dataon the latest platforms. On older platforms it will continue to be zero.
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
Indeed. There should just be a special case to disable execute protection for P7.
Ok. we should disable execute protection for P7 and earlier generations of CPU.
You should do what the device tree says you can do.
If it says there are no execute keys then you shouldn't touch the IAMR.
The downside of that approach is that the device tree in P8 LPARs currently says there are no execute keys even though there are. We'd have to require customers to upgrade their firmware to a fixed version if they want to use execute keys.
Signature · 2 lines
Thiago Jung Bauermann IBM Linux Technology Center
Re: [RFC v7 25/25] powerpc: Enable pkey subsystem
Quoted 37 lines · Thiago Jung Bauermann
Quoted 34 lines · Michael Ellerman
Quoted 32 lines · Ram Pai
Quoted 23 lines · Thiago Jung Bauermann
Quoted 22 lines · Ram Pai
Quoted 20 lines · Thiago Jung Bauermann
Review static inline void pkey_mm_init(struct mm_struct *mm) · line 240
Quoted 13 lines · Ram Pai
(quoted diff) +9
-0@@ -227,6 +229,24 @@ static inline void pkey_mm_init(struct mm_struct *mm) mm->context.execute_only_pkey = -1; } +static inline void pkey_mmu_values(int total_data, int total_execute) +{ + /* + * since any pkey can be used for data or execute, we + * will just treat all keys as equal and track them + * as one entity. + */ + pkeys_total = total_data + total_execute; +}Right now this works because the firmware reports 0 execute keys in the device tree, but if (when?) it is fixed to report 32 execute keys as well as 32 data keys (which are the same keys), any place using
pkeys_totalexpecting it to mean the number of keys that are available will be broken. This includespkey_initializeandmm_pkey_is_allocated.Quoted 6 lines
Quoted 3 lines
Good point. we should just ignore
total_execute. It should be the same value astotal_dataon the latest platforms. On older platforms it will continue to be zero.Indeed. There should just be a special case to disable execute protection for P7.
Ok. we should disable execute protection for P7 and earlier generations of CPU.
Quoted 8 lines
Quoted 3 lines
You should do what the device tree says you can do.
If it says there are no execute keys then you shouldn't touch the IAMR.
The downside of that approach is that the device tree in P8 LPARs currently says there are no execute keys even though there are. We'd have to require customers to upgrade their firmware to a fixed version if they want to use execute keys.
Correct. the device tree for this property currently does not correctly capture the number of execute keys.
On skiboot based systems, there is not device tree property to refer to aswell. Thiago has a patch to fix it, but existing systems without the skiboot fix, will not expose that property.
So unfortunately we will have to rely on multiple peices of information to enable the pkey system in the kernel.
RP
Quoted 4 lines
Signature · 3 lines
-- Thiago Jung Bauermann IBM Linux Technology Center
Signature · 1 line
Ram Pai
[RFC v7 26/25] mm/mprotect, powerpc/mm/pkeys, x86/mm/pkeys: Add sysfs interface
In reply to Ram PaiExpose useful information for programs using memory protection keys. Provide implementation for powerpc and x86.
On a powerpc system with pkeys support, here is what is shown:
$ head /sys/kernel/mm/protection_keys/* ==> /sys/kernel/mm/protection_keys/disable_execute_supported <== true
==> /sys/kernel/mm/protection_keys/total_keys <==
32
==> /sys/kernel/mm/protection_keys/usable_keys <== 30
And on an x86 without pkeys support:
$ head /sys/kernel/mm/protection_keys/* ==> /sys/kernel/mm/protection_keys/disable_execute_supported <== false
==> /sys/kernel/mm/protection_keys/total_keys <==
1
==> /sys/kernel/mm/protection_keys/usable_keys <== 0
---
Ram asked me to add a sysfs interface for the memory protection keys feature. Here it is.
If you have suggestions on what should be exposed, please let me know.
arch/powerpc/include/asm/pkeys.h | 2 ++ arch/powerpc/mm/pkeys.c | 12 ++++++++ arch/x86/include/asm/mmu_context.h | 34 +++++++++++----------- arch/x86/include/asm/pkeys.h | 1 + arch/x86/mm/pkeys.c | 5 ++++ mm/mprotect.c | 58 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 96 insertions(+), 16 deletions(-)
arch/powerpc/include/asm/pkeys.h +2 -0
@@ -215,6 +215,8 @@ static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,return __arch_set_user_pkey_access (tsk, pkey, init_val); } +unsigned int arch_usable_pkeys (void ); +static inline bool arch_pkeys_enabled (void ) {return pkey_inited;
arch/powerpc/mm/pkeys.c +12 -0
@@ -272,3 +272,15 @@ bool arch_vma_access_permitted(struct vm_area_struct *vma,return pkey_access_permitted (pkey, write, execute); } + +unsigned int arch_usable_pkeys (void ) +{ +unsigned int reserved; + +if (!pkey_inited) +return 0 ; + + reserved =hweight32 (initial_allocation_mask); + +return (pkeys_total > reserved) ? pkeys_total - reserved :0 ; +}
arch/x86/include/asm/mmu_context.h +18 -16
@@ -105,13 +105,30 @@ static inline void enter_lazy_tlb(struct mm_struct *mm, struct task_struct *tsk)#endif } +#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS +#define PKEY_INITIAL_ALLOCATION_MAP 1 + +static inline int vma_pkey (struct vm_area_struct *vma) +{ +unsigned long vma_pkey_mask =VM_PKEY_BIT0 |VM_PKEY_BIT1 | +VM_PKEY_BIT2 |VM_PKEY_BIT3 ; + +return (vma->vm_flags & vma_pkey_mask) >>VM_PKEY_SHIFT ; +} +#else +static inline int vma_pkey (struct vm_area_struct *vma) +{ +return 0 ; +} +#endif +static inline int init_new_context (struct task_struct *tsk,struct mm_struct *mm) {#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS if (cpu_feature_enabled (X86_FEATURE_OSPKE )) {/* pkey 0 is the default and always allocated */ - mm->context.pkey_allocation_map =0x1 ; + mm->context.pkey_allocation_map =PKEY_INITIAL_ALLOCATION_MAP ;/* -1 means unallocated or invalid */ mm->context.execute_only_pkey =-1 ; }
@@ -205,21 +222,6 @@ static inline void arch_unmap(struct mm_struct *mm, struct vm_area_struct *vma,mpx_notify_unmap (mm, vma, start, end); } -#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS -static inline int vma_pkey (struct vm_area_struct *vma) -{ -unsigned long vma_pkey_mask =VM_PKEY_BIT0 |VM_PKEY_BIT1 | -VM_PKEY_BIT2 |VM_PKEY_BIT3 ; - -return (vma->vm_flags & vma_pkey_mask) >>VM_PKEY_SHIFT ; -} -#else -static inline int vma_pkey (struct vm_area_struct *vma) -{ -return 0 ; -} -#endif -static inline bool __pkru_allows_pkey (u16 pkey,bool write) {u32 pkru =read_pkru ();
arch/x86/include/asm/pkeys.h +1 -0
@@ -105,5 +105,6 @@ extern int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,extern int __arch_set_user_pkey_access (struct task_struct *tsk,int pkey,unsigned long init_val);extern void copy_init_pkru_to_fpregs (void ); +extern unsigned int arch_usable_pkeys (void );#endif /*_ASM_X86_PKEYS_H */
arch/x86/mm/pkeys.c +5 -0
@@ -123,6 +123,11 @@ int __arch_override_mprotect_pkey(struct vm_area_struct *vma, int prot, int pkeyreturn vma_pkey (vma); } +unsigned int arch_usable_pkeys (void ) +{ +return arch_max_pkey () -hweight32 (PKEY_INITIAL_ALLOCATION_MAP ); +} +#define PKRU_AD_KEY (pkey) (PKRU_AD_BIT << ((pkey) * PKRU_BITS_PER_PKEY))/*
mm/mprotect.c +58 -0
62 lines not on this page · raw message
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [RFC v7 26/25] mm/mprotect, powerpc/mm/pkeys, x86/mm/pkeys: Add sysfs interface
Quoted 8 lines · Thiago Jung Bauermann
Expose useful information for programs using memory protection keys. Provide implementation for powerpc and x86.
On a powerpc system with pkeys support, here is what is shown:
$ head /sys/kernel/mm/protection_keys/* ==> /sys/kernel/mm/protection_keys/disable_execute_supported <== true
We should not just call out disable_execute_supported. disable_access_supported and disable_write_supported should also be called out.
Quoted 7 lines
==>
/sys/kernel/mm/protection_keys/total_keys<==
32==>
/sys/kernel/mm/protection_keys/usable_keys<== 30
This is little nebulous. It depends on how we define usable as. Is it the number of keys that are available to the app? If that is the case that value is dynamic. Sometime the OS steals one key for execute-only key. And anything that is dynamic can be inherently racy. So I think we should define 'usable' as guaranteed number of keys available to the app and display a value that is one less than what is available.
in the above example the value should be 29.
RP
Re: [RFC v7 26/25] mm/mprotect, powerpc/mm/pkeys, x86/mm/pkeys: Add sysfs interface
Quoted 6 lines · Ram Pai
Quoted 5 lines · Thiago Jung Bauermann
Expose useful information for programs using memory protection keys. Provide implementation for powerpc and x86.
On a powerpc system with pkeys support, here is what is shown:
Quoted 7 lines
Quoted 3 lines
$ head /sys/kernel/mm/protection_keys/* ==> /sys/kernel/mm/protection_keys/disable_execute_supported <== trueWe should not just call out
disable_execute_supported.disable_access_supportedanddisable_write_supportedshould also be called out.
Ok, will do in the next version.
Quoted 7 lines
Quoted 6 lines
==>
/sys/kernel/mm/protection_keys/total_keys<==
32==>
/sys/kernel/mm/protection_keys/usable_keys<== 30
Quoted 7 lines
This is little nebulous. It depends on how we define usable as. Is it the number of keys that are available to the app? If that is the case that value is dynamic. Sometime the OS steals one key for execute-only key. And anything that is dynamic can be inherently racy. So I think we should define 'usable' as guaranteed number of keys available to the app
Yes, that is how I defined it: the difference between the number of keys provided by the platform and the keys reserved by the OS. I do need to spell it out somewhere inside Documentation/ though.
Quoted 3 lines
and display a value that is one less than what is available.
in the above example the value should be 29.
Good point, I didn't account for the execute-only key. I will make that change in the next version.
Signature · 2 lines
Thiago Jung Bauermann IBM Linux Technology Center