Re: [PATCH v2 4/6] powerpc/pkeys: Preallocate execute-only key

[Thiago Jung Bauermann <bauerman@linux.ibm.com>]

Hello,

My understanding is that this patch isn't upstream yet and it's not too
late for bikeshedding. Please ignore if this is not the case.

Ram Pai <linuxram@us.ibm.com> writes:

> @@ -326,48 +330,7 @@ static inline bool pkey_allows_readwrite(int pkey)
>
>  int __execute_only_pkey(struct mm_struct *mm)
>  {
> -	bool need_to_set_mm_pkey = false;
> -	int execute_only_pkey = mm->context.execute_only_pkey;
> -	int ret;
> -
> -	/* Do we need to assign a pkey for mm's execute-only maps? */
> -	if (execute_only_pkey == -1) {
> -		/* Go allocate one to use, which might fail */
> -		execute_only_pkey = mm_pkey_alloc(mm);
> -		if (execute_only_pkey < 0)
> -			return -1;
> -		need_to_set_mm_pkey = true;
> -	}
> -
> -	/*
> -	 * We do not want to go through the relatively costly dance to set AMR
> -	 * if we do not need to. Check it first and assume that if the
> -	 * execute-only pkey is readwrite-disabled than we do not have to set it
> -	 * ourselves.
> -	 */
> -	if (!need_to_set_mm_pkey && !pkey_allows_readwrite(execute_only_pkey))
> -		return execute_only_pkey;
> -
> -	/*
> -	 * Set up AMR so that it denies access for everything other than
> -	 * execution.
> -	 */
> -	ret = __arch_set_user_pkey_access(current, execute_only_pkey,
> -					  PKEY_DISABLE_ACCESS |
> -					  PKEY_DISABLE_WRITE);
> -	/*
> -	 * If the AMR-set operation failed somehow, just return 0 and
> -	 * effectively disable execute-only support.
> -	 */
> -	if (ret) {
> -		mm_pkey_free(mm, execute_only_pkey);
> -		return -1;
> -	}
> -
> -	/* We got one, store it and use it from here on out */
> -	if (need_to_set_mm_pkey)
> -		mm->context.execute_only_pkey = execute_only_pkey;
> -	return execute_only_pkey;
> +	return mm->context.execute_only_pkey;
>  }

There's no reason to have a separate  __execute_only_pkey() function
anymore. Its single line can go directly in execute_only_pkey(), defined
in <asm/pkeys.h>.
-- 
Thiago Jung Bauermann
IBM Linux Technology Center