Subject: Re: [PATCH v5 08/26] powerpc/book3s64/pkeys: Convert execute key support to static key

From: Michael Ellerman <[email-masked]>
Date: Mon, 06 Jul 2020 17:19:58 +1000

"Aneesh Kumar K.V" <[email-masked]> 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