All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Ram Pai <linuxram@us.ibm.com>
To: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	Michael Ellerman <mpe@ellerman.id.au>
Subject: Re: [PATCH] powerpc/pkeys: Fix reading of ibm,processor-storage-keys property
Date: Thu, 20 Sep 2018 09:11:47 -0700	[thread overview]
Message-ID: <20180920161147.GE5809@ram.oc3035372033.ibm.com> (raw)
In-Reply-To: <20180920043858.28773-1-bauerman@linux.ibm.com>

On Thu, Sep 20, 2018 at 01:38:58AM -0300, Thiago Jung Bauermann wrote:
> scan_pkey_feature() uses of_property_read_u32_array() to read the
> ibm,processor-storage-keys property and calls be32_to_cpu() on the
> value it gets. The problem is that of_property_read_u32_array() already
> returns the value converted to the CPU byte order.
> 
> The value of pkeys_total ends up more or less sane because there's a min()
> call in pkey_initialize() which reduces pkeys_total to 32. So in practice
> the kernel ignores the fact that the hypervisor reserved one key for
> itself (the device tree advertises 31 keys in my test VM).
> 
> This is wrong, but the effect in practice is that when a process tries to
> allocate the 32nd key, it gets an -EINVAL error instead of -ENOSPC which
> would indicate that there aren't any keys available

Thanks for the fix.

Reviewed-by: Ram Pai <linuxram@us.ibm.com>

> 
> Fixes: cf43d3b26452 ("powerpc: Enable pkey subsystem")
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> ---
>  arch/powerpc/mm/pkeys.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
> index 333b1f80c435..b271b283c785 100644
> --- a/arch/powerpc/mm/pkeys.c
> +++ b/arch/powerpc/mm/pkeys.c
> @@ -45,7 +45,7 @@ static void scan_pkey_feature(void)
>  	 * Since any pkey can be used for data or execute, we will just treat
>  	 * all keys as equal and track them as one entity.
>  	 */
> -	pkeys_total = be32_to_cpu(vals[0]);
> +	pkeys_total = vals[0];
>  	pkeys_devtree_defined = true;
>  }

-- 
Ram Pai


WARNING: multiple messages have this Message-ID (diff)
From: Ram Pai <linuxram@us.ibm.com>
To: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	Michael Ellerman <mpe@ellerman.id.au>
Subject: Re: [PATCH] powerpc/pkeys: Fix reading of ibm, processor-storage-keys property
Date: Thu, 20 Sep 2018 09:11:47 -0700	[thread overview]
Message-ID: <20180920161147.GE5809@ram.oc3035372033.ibm.com> (raw)
In-Reply-To: <20180920043858.28773-1-bauerman@linux.ibm.com>

On Thu, Sep 20, 2018 at 01:38:58AM -0300, Thiago Jung Bauermann wrote:
> scan_pkey_feature() uses of_property_read_u32_array() to read the
> ibm,processor-storage-keys property and calls be32_to_cpu() on the
> value it gets. The problem is that of_property_read_u32_array() already
> returns the value converted to the CPU byte order.
> 
> The value of pkeys_total ends up more or less sane because there's a min()
> call in pkey_initialize() which reduces pkeys_total to 32. So in practice
> the kernel ignores the fact that the hypervisor reserved one key for
> itself (the device tree advertises 31 keys in my test VM).
> 
> This is wrong, but the effect in practice is that when a process tries to
> allocate the 32nd key, it gets an -EINVAL error instead of -ENOSPC which
> would indicate that there aren't any keys available

Thanks for the fix.

Reviewed-by: Ram Pai <linuxram@us.ibm.com>

> 
> Fixes: cf43d3b26452 ("powerpc: Enable pkey subsystem")
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> ---
>  arch/powerpc/mm/pkeys.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
> index 333b1f80c435..b271b283c785 100644
> --- a/arch/powerpc/mm/pkeys.c
> +++ b/arch/powerpc/mm/pkeys.c
> @@ -45,7 +45,7 @@ static void scan_pkey_feature(void)
>  	 * Since any pkey can be used for data or execute, we will just treat
>  	 * all keys as equal and track them as one entity.
>  	 */
> -	pkeys_total = be32_to_cpu(vals[0]);
> +	pkeys_total = vals[0];
>  	pkeys_devtree_defined = true;
>  }

-- 
Ram Pai

  reply	other threads:[~2018-09-20 16:12 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-20  4:38 [PATCH] powerpc/pkeys: Fix reading of ibm,processor-storage-keys property Thiago Jung Bauermann
2018-09-20  4:38 ` [PATCH] powerpc/pkeys: Fix reading of ibm, processor-storage-keys property Thiago Jung Bauermann
2018-09-20 16:11 ` Ram Pai [this message]
2018-09-20 16:11   ` Ram Pai
2018-09-21 11:59 ` 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=20180920161147.GE5809@ram.oc3035372033.ibm.com \
    --to=linuxram@us.ibm.com \
    --cc=bauerman@linux.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.