On 7/6/20 12:50 PM, Michael Ellerman wrote:
"Aneesh Kumar K.V" <[email protected]> writes:
Use execute_pkey_disabled static key to check for execute key support instead
of pkey_disabled.
Signed-off-by: Aneesh Kumar K.V <[email protected]>
---
arch/powerpc/include/asm/pkeys.h | 10 +---------
arch/powerpc/mm/book3s64/pkeys.c | 5 ++++-
2 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index 47c81d41ea9a..09fbaa409ac4 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -126,15 +126,7 @@ 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)
-{
- if (static_branch_likely(&pkey_disabled))
- return -1;
-
- return __execute_only_pkey(mm);
-}
-
+extern int execute_only_pkey(struct mm_struct *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,
diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
index bbba9c601e14..fed4f159011b 100644
--- a/arch/powerpc/mm/book3s64/pkeys.c
+++ b/arch/powerpc/mm/book3s64/pkeys.c
@@ -345,8 +345,11 @@ void thread_pkey_regs_init(struct thread_struct *thread)
write_uamor(default_uamor);
}
-int __execute_only_pkey(struct mm_struct *mm)
+int execute_only_pkey(struct mm_struct *mm)
{
+ if (static_branch_likely(&execute_pkey_disabled))
+ return -1;
+
return mm->context.execute_only_pkey;
}
That adds the overhead of a function call, but then uses a static_key to
avoid an easy to predict branch, which seems like a bad tradeoff. And
it's not a performance critical path AFAICS.
Anyway this seems unnecessary:
pkey_early_init_devtree()
{
...
if (unlikely(max_pkey <= execute_only_key)) {
/*
* Insufficient number of keys to support
* execute only key. Mark it unavailable.
*/
execute_only_key = -1;
void pkey_mm_init(struct mm_struct *mm)
{
...
mm->context.execute_only_pkey = execute_only_key;
}
ie. Can't it just be:
static inline int execute_only_pkey(struct mm_struct *mm)
{
return mm->context.execute_only_pkey;
}
ok updated with
modified arch/powerpc/mm/book3s64/pkeys.c
@@ -151,7 +151,7 @@ void __init pkey_early_init_devtree(void)
max_pkey = pkeys_total;
#endif
- if (unlikely(max_pkey <= execute_only_key)) {
+ if (unlikely(max_pkey <= execute_only_key) ||
!pkey_execute_disable_supported) {
/*
* Insufficient number of keys to support
* execute only key. Mark it unavailable.
@@ -368,9 +368,6 @@ int __arch_set_user_pkey_access(struct task_struct
*tsk, int pkey,
int execute_only_pkey(struct mm_struct *mm)
{
- if (static_branch_likely(&execute_pkey_disabled))
- return -1;
-
return mm->context.execute_only_pkey;
}
-aneesh
