From: Michael Ellerman <mpe@ellerman•id.au>
To: "Aneesh Kumar K.V" <aneesh.kumar@linux•ibm.com>,
linuxppc-dev@lists•ozlabs.org
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux•ibm.com>
Subject: Re: [PATCH 2/2] powerpc/mm/books64/pkeys: Rename is_pkey_enabled()
Date: Tue, 30 Jun 2020 22:21:34 +1000 [thread overview]
Message-ID: <87wo3o7nht.fsf@mpe.ellerman.id.au> (raw)
In-Reply-To: <20200627070147.297535-2-aneesh.kumar@linux.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux•ibm.com> writes:
> Rename is_pkey_enabled() to is_pkey_masked() to better indicates that
> this check is to make sure the key is available for userspace usage.
I don't think the new name makes that any clearer. Unless you know that
"masked" means not "available for userspace".
It's also not clear if masked means 00 or 11.
Now that there's only one caller why not just fold it in, that way it
doesn't need a name at all.
> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
> index ca5fcb4bff32..70d760ade922 100644
> --- a/arch/powerpc/mm/book3s64/pkeys.c
> +++ b/arch/powerpc/mm/book3s64/pkeys.c
> @@ -206,18 +206,16 @@ static inline void write_uamor(u64 value)
> mtspr(SPRN_UAMOR, value);
> }
>
> -static bool is_pkey_enabled(int pkey)
> +static bool is_pkey_masked(int pkey)
> {
> u64 uamor = read_uamor();
> u64 pkey_bits = 0x3ul << pkeyshift(pkey);
> u64 uamor_pkey_bits = (uamor & pkey_bits);
>
> /*
> - * Both the bits in UAMOR corresponding to the key should be set or
> - * reset.
> + * Both the bits in UAMOR corresponding to the key should be set
> */
> - WARN_ON(uamor_pkey_bits && (uamor_pkey_bits != pkey_bits));
> - return !!(uamor_pkey_bits);
> + return (uamor_pkey_bits != pkey_bits);
> }
>
> static inline void init_amr(int pkey, u8 init_bits)
> @@ -246,7 +244,7 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
> u64 new_amr_bits = 0x0ul;
> u64 new_iamr_bits = 0x0ul;
>
> - if (!is_pkey_enabled(pkey))
> + if (is_pkey_masked(pkey))
> return -EINVAL;
eg:
u64 pkey_bits = 0x3ul << pkeyshift(pkey);
if ((read_uamor() & pkey_bits) != pkey_bits)
return -EINVAL;
>
> if (init_val & PKEY_DISABLE_EXECUTE) {
cheers
next prev parent reply other threads:[~2020-06-30 12:22 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-27 7:01 [PATCH 1/2] powerpc/mm/book3s54/pkeys: make pkey access check work on execute_only_key Aneesh Kumar K.V
2020-06-27 7:01 ` [PATCH 2/2] powerpc/mm/books64/pkeys: Rename is_pkey_enabled() Aneesh Kumar K.V
2020-06-30 12:21 ` Michael Ellerman [this message]
2020-06-30 12:24 ` [PATCH 1/2] powerpc/mm/book3s54/pkeys: make pkey access check work on execute_only_key Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87wo3o7nht.fsf@mpe.ellerman.id.au \
--to=mpe@ellerman$(echo .)id.au \
--cc=aneesh.kumar@linux$(echo .)ibm.com \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox