| Message ID | 20200619135850.47155-9-aneesh.kumar@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc/book3s/64/pkeys: Simplify the code | expand |
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes: > Convert the bool to a static key like pkey_disabled. I'm not convinced this is worth the added complexity of a static key. It's not used in any performance critical paths, except for context switch, but that's already guarded by: if (old_thread->iamr != new_thread->iamr) Which should always be false on machines which don't have the execute key enabled. cheers > diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c > index 9e68a08799ee..7d400d5a4076 100644 > --- a/arch/powerpc/mm/book3s64/pkeys.c > +++ b/arch/powerpc/mm/book3s64/pkeys.c > @@ -13,13 +13,13 @@ > #include <linux/of_device.h> > > DEFINE_STATIC_KEY_TRUE(pkey_disabled); > +DEFINE_STATIC_KEY_FALSE(execute_pkey_disabled); > int pkeys_total; /* Total pkeys as per device tree */ > u32 initial_allocation_mask; /* Bits set for the initially allocated keys */ > /* > * Keys marked in the reservation list cannot be allocated by userspace > */ > u32 reserved_allocation_mask; > -static bool pkey_execute_disable_supported; > static u64 default_amr; > static u64 default_iamr; > /* Allow all keys to be modified by default */ > @@ -116,9 +116,7 @@ static int pkey_initialize(void) > * execute_disable support. Instead we use a PVR check. > */ > if (pvr_version_is(PVR_POWER7) || pvr_version_is(PVR_POWER7p)) > - pkey_execute_disable_supported = false; > - else > - pkey_execute_disable_supported = true; > + static_branch_enable(&execute_pkey_disabled); > > #ifdef CONFIG_PPC_4K_PAGES > /* > @@ -214,7 +212,7 @@ static inline void write_amr(u64 value) > > static inline u64 read_iamr(void) > { > - if (!likely(pkey_execute_disable_supported)) > + if (static_branch_unlikely(&execute_pkey_disabled)) > return 0x0UL; > > return mfspr(SPRN_IAMR); > @@ -222,7 +220,7 @@ static inline u64 read_iamr(void) > > static inline void write_iamr(u64 value) > { > - if (!likely(pkey_execute_disable_supported)) > + if (static_branch_unlikely(&execute_pkey_disabled)) > return; > > mtspr(SPRN_IAMR, value); > @@ -282,7 +280,7 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey, > return -EINVAL; > > if (init_val & PKEY_DISABLE_EXECUTE) { > - if (!pkey_execute_disable_supported) > + if (static_branch_unlikely(&execute_pkey_disabled)) > return -EINVAL; > new_iamr_bits |= IAMR_EX_BIT; > } > -- > 2.26.2
On 7/6/20 12:49 PM, Michael Ellerman wrote: > "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes: >> Convert the bool to a static key like pkey_disabled. > > I'm not convinced this is worth the added complexity of a static key. > > It's not used in any performance critical paths, except for context > switch, but that's already guarded by: > > if (old_thread->iamr != new_thread->iamr) > > Which should always be false on machines which don't have the execute > key enabled. > Ok will drop the patch. -aneesh
diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c index 9e68a08799ee..7d400d5a4076 100644 --- a/arch/powerpc/mm/book3s64/pkeys.c +++ b/arch/powerpc/mm/book3s64/pkeys.c @@ -13,13 +13,13 @@ #include <linux/of_device.h> DEFINE_STATIC_KEY_TRUE(pkey_disabled); +DEFINE_STATIC_KEY_FALSE(execute_pkey_disabled); int pkeys_total; /* Total pkeys as per device tree */ u32 initial_allocation_mask; /* Bits set for the initially allocated keys */ /* * Keys marked in the reservation list cannot be allocated by userspace */ u32 reserved_allocation_mask; -static bool pkey_execute_disable_supported; static u64 default_amr; static u64 default_iamr; /* Allow all keys to be modified by default */ @@ -116,9 +116,7 @@ static int pkey_initialize(void) * execute_disable support. Instead we use a PVR check. */ if (pvr_version_is(PVR_POWER7) || pvr_version_is(PVR_POWER7p)) - pkey_execute_disable_supported = false; - else - pkey_execute_disable_supported = true; + static_branch_enable(&execute_pkey_disabled); #ifdef CONFIG_PPC_4K_PAGES /* @@ -214,7 +212,7 @@ static inline void write_amr(u64 value) static inline u64 read_iamr(void) { - if (!likely(pkey_execute_disable_supported)) + if (static_branch_unlikely(&execute_pkey_disabled)) return 0x0UL; return mfspr(SPRN_IAMR); @@ -222,7 +220,7 @@ static inline u64 read_iamr(void) static inline void write_iamr(u64 value) { - if (!likely(pkey_execute_disable_supported)) + if (static_branch_unlikely(&execute_pkey_disabled)) return; mtspr(SPRN_IAMR, value); @@ -282,7 +280,7 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey, return -EINVAL; if (init_val & PKEY_DISABLE_EXECUTE) { - if (!pkey_execute_disable_supported) + if (static_branch_unlikely(&execute_pkey_disabled)) return -EINVAL; new_iamr_bits |= IAMR_EX_BIT; }
Convert the bool to a static key like pkey_disabled. Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> --- arch/powerpc/mm/book3s64/pkeys.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)