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>,
linuxram@us•ibm.com, bauerman@linux•ibm.com
Subject: Re: [PATCH v5 10/26] powerpc/book3s64/pkeys: Convert pkey_total to max_pkey
Date: Mon, 06 Jul 2020 17:04:39 +1000 [thread overview]
Message-ID: <87tuyl5dko.fsf@mpe.ellerman.id.au> (raw)
In-Reply-To: <20200619135850.47155-11-aneesh.kumar@linux.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux•ibm.com> writes:
> max_pkey now represents max key value that userspace can allocate.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux•ibm.com>
> ---
> arch/powerpc/include/asm/pkeys.h | 7 +++++--
> arch/powerpc/mm/book3s64/pkeys.c | 14 +++++++-------
> 2 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> index 75d2a2c19c04..652bad7334f3 100644
> --- a/arch/powerpc/include/asm/pkeys.h
> +++ b/arch/powerpc/include/asm/pkeys.h
> @@ -12,7 +12,7 @@
> #include <asm/firmware.h>
>
> DECLARE_STATIC_KEY_FALSE(pkey_disabled);
> -extern int pkeys_total; /* total pkeys as per device tree */
> +extern int max_pkey;
> extern u32 initial_allocation_mask; /* bits set for the initially allocated keys */
> extern u32 reserved_allocation_mask; /* bits set for reserved keys */
>
> @@ -44,7 +44,10 @@ static inline int vma_pkey(struct vm_area_struct *vma)
> return (vma->vm_flags & ARCH_VM_PKEY_FLAGS) >> VM_PKEY_SHIFT;
> }
>
> -#define arch_max_pkey() pkeys_total
> +static inline int arch_max_pkey(void)
> +{
> + return max_pkey;
> +}
If pkeys_total = 32 then max_pkey = 31.
So we can't just substitute one for the other. ie. arch_max_pkey() must
have been wrong, or it is wrong now.
> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
> index 87d882a9aaf2..a4d7287082a8 100644
> --- a/arch/powerpc/mm/book3s64/pkeys.c
> +++ b/arch/powerpc/mm/book3s64/pkeys.c
> @@ -14,7 +14,7 @@
>
> DEFINE_STATIC_KEY_FALSE(pkey_disabled);
> DEFINE_STATIC_KEY_FALSE(execute_pkey_disabled);
> -int pkeys_total; /* Total pkeys as per device tree */
> +int max_pkey; /* Maximum key value supported */
> u32 initial_allocation_mask; /* Bits set for the initially allocated keys */
> /*
> * Keys marked in the reservation list cannot be allocated by userspace
> @@ -84,7 +84,7 @@ static int scan_pkey_feature(void)
>
> static int pkey_initialize(void)
> {
> - int os_reserved, i;
> + int pkeys_total, i;
>
> /*
> * We define PKEY_DISABLE_EXECUTE in addition to the arch-neutral
> @@ -122,12 +122,12 @@ static int pkey_initialize(void)
> * The OS can manage only 8 pkeys due to its inability to represent them
> * in the Linux 4K PTE. Mark all other keys reserved.
> */
> - os_reserved = pkeys_total - 8;
> + max_pkey = min(8, pkeys_total);
Shouldn't it be 7 ?
> #else
> - os_reserved = 0;
> + max_pkey = pkeys_total;
> #endif
>
> - if (unlikely((pkeys_total - os_reserved) <= execute_only_key)) {
> + if (unlikely(max_pkey <= execute_only_key)) {
Isn't that an off-by-one now?
This is one-off boot time code, there's no need to clutter it with
unlikely.
> /*
> * Insufficient number of keys to support
> * execute only key. Mark it unavailable.
> @@ -174,10 +174,10 @@ static int pkey_initialize(void)
> default_uamor &= ~(0x3ul << pkeyshift(1));
>
> /*
> - * Prevent the usage of OS reserved the keys. Update UAMOR
> + * Prevent the usage of OS reserved keys. Update UAMOR
> * for those keys.
> */
> - for (i = (pkeys_total - os_reserved); i < pkeys_total; i++) {
> + for (i = max_pkey; i < pkeys_total; i++) {
Another off-by-one? Shouldn't we start from max_pkey + 1 ?
> reserved_allocation_mask |= (0x1 << i);
> default_uamor &= ~(0x3ul << pkeyshift(i));
> }
cheers
next prev parent reply other threads:[~2020-07-06 7:04 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-19 13:58 [PATCH v5 00/26] powerpc/book3s/64/pkeys: Simplify the code Aneesh Kumar K.V
2020-06-19 13:58 ` [PATCH v5 01/26] powerpc/book3s64/pkeys: Fixup bit numbering Aneesh Kumar K.V
2020-06-19 13:58 ` [PATCH v5 02/26] powerpc/book3s64/pkeys: pkeys are supported only on hash on book3s Aneesh Kumar K.V
2020-06-19 13:58 ` [PATCH v5 03/26] powerpc/book3s64/pkeys: Move pkey related bits in the linux page table Aneesh Kumar K.V
2020-06-19 13:58 ` [PATCH v5 04/26] powerpc/book3s64/pkeys: Explain key 1 reservation details Aneesh Kumar K.V
2020-06-19 13:58 ` [PATCH v5 05/26] powerpc/book3s64/pkeys: Simplify the key initialization Aneesh Kumar K.V
2020-06-19 13:58 ` [PATCH v5 06/26] powerpc/book3s64/pkeys: Prevent key 1 modification from userspace Aneesh Kumar K.V
2020-06-19 13:58 ` [PATCH v5 07/26] powerpc/book3s64/pkeys: kill cpu feature key CPU_FTR_PKEY Aneesh Kumar K.V
2020-06-19 13:58 ` [PATCH v5 08/26] powerpc/book3s64/pkeys: Convert execute key support to static key Aneesh Kumar K.V
2020-07-06 7:19 ` Michael Ellerman
2020-07-06 8:47 ` Aneesh Kumar K.V
2020-06-19 13:58 ` [PATCH v5 09/26] powerpc/book3s64/pkeys: Simplify pkey disable branch Aneesh Kumar K.V
2020-06-19 13:58 ` [PATCH v5 10/26] powerpc/book3s64/pkeys: Convert pkey_total to max_pkey Aneesh Kumar K.V
2020-07-06 7:04 ` Michael Ellerman [this message]
2020-07-06 7:20 ` Aneesh Kumar K.V
2020-07-07 1:26 ` Michael Ellerman
2020-06-19 13:58 ` [PATCH v5 11/26] powerpc/book3s64/pkeys: Make initial_allocation_mask static Aneesh Kumar K.V
2020-07-06 7:04 ` Michael Ellerman
2020-07-06 8:48 ` Aneesh Kumar K.V
2020-06-19 13:58 ` [PATCH v5 12/26] powerpc/book3s64/pkeys: Mark all the pkeys above max pkey as reserved Aneesh Kumar K.V
2020-06-19 13:58 ` [PATCH v5 13/26] powerpc/book3s64/pkeys: Enable MMU_FTR_PKEY Aneesh Kumar K.V
2020-07-06 13:10 ` Michael Ellerman
2020-07-06 14:09 ` Aneesh Kumar K.V
2020-07-06 17:17 ` Aneesh Kumar K.V
2020-07-07 1:02 ` Michael Ellerman
2020-06-19 13:58 ` [PATCH v5 14/26] powerpc/book3s64/kuep: Add MMU_FTR_KUEP Aneesh Kumar K.V
2020-06-19 13:58 ` [PATCH v5 15/26] powerpc/book3s64/pkeys: Use execute_pkey_disable static key Aneesh Kumar K.V
2020-07-06 7:20 ` Michael Ellerman
2020-07-06 8:49 ` Aneesh Kumar K.V
2020-06-19 13:58 ` [PATCH v5 16/26] powerpc/book3s64/pkeys: Use MMU_FTR_PKEY instead of pkey_disabled " Aneesh Kumar K.V
2020-06-19 13:58 ` [PATCH v5 17/26] powerpc/book3s64/keys: Print information during boot Aneesh Kumar K.V
2020-07-06 7:52 ` Michael Ellerman
2020-06-19 13:58 ` [PATCH v5 18/26] powerpc/book3s64/keys/kuap: Reset AMR/IAMR values on kexec Aneesh Kumar K.V
2020-07-06 12:29 ` Michael Ellerman
2020-07-06 14:39 ` Aneesh Kumar K.V
2020-07-07 1:07 ` Michael Ellerman
2020-06-19 13:58 ` [PATCH v5 19/26] powerpc/book3s64/kuap: Move KUAP related function outside radix Aneesh Kumar K.V
2020-07-06 12:41 ` Michael Ellerman
2020-07-06 14:41 ` Aneesh Kumar K.V
2020-07-07 1:22 ` Michael Ellerman
2020-06-19 13:58 ` [PATCH v5 20/26] powerpc/book3s64/kuep: Move KUEP " Aneesh Kumar K.V
2020-06-19 13:58 ` [PATCH v5 21/26] powerpc/book3s64/kuap: Rename MMU_FTR_RADIX_KUAP to MMU_FTR_KUAP Aneesh Kumar K.V
2020-06-19 13:58 ` [PATCH v5 22/26] powerpc/book3s64/kuap/kuep: Make KUAP and KUEP a subfeature of PPC_MEM_KEYS Aneesh Kumar K.V
2020-06-19 13:58 ` [PATCH v5 23/26] powerpc/book3s64/kuap: Move UAMOR setup to key init function Aneesh Kumar K.V
2020-07-07 6:05 ` Michael Ellerman
2020-07-07 11:25 ` Aneesh Kumar K.V
2020-06-19 13:58 ` [PATCH v5 24/26] powerpc/selftest/ptrave-pkey: Rename variables to make it easier to follow code Aneesh Kumar K.V
2020-06-19 13:58 ` [PATCH v5 25/26] powerpc/selftest/ptrace-pkey: Update the test to mark an invalid pkey correctly Aneesh Kumar K.V
2020-06-19 13:58 ` [PATCH v5 26/26] powerpc/selftest/ptrace-pkey: IAMR and uamor cannot be updated by ptrace Aneesh Kumar K.V
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=87tuyl5dko.fsf@mpe.ellerman.id.au \
--to=mpe@ellerman$(echo .)id.au \
--cc=aneesh.kumar@linux$(echo .)ibm.com \
--cc=bauerman@linux$(echo .)ibm.com \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=linuxram@us$(echo .)ibm.com \
/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