* Re: [PATCH RESEND 4/4] powerpc/uaccess: Implement user_read_access_begin and user_write_access_begin
From: Kees Cook @ 2020-04-02 7:52 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-arch, linuxppc-dev, airlied, hpa, linux-kernel, linux-mm,
Paul Mackerras, viro, daniel, akpm, torvalds
In-Reply-To: <ebcf8256e02a7dffb292f3d800e264dce263cac5.1585811416.git.christophe.leroy@c-s.fr>
On Thu, Apr 02, 2020 at 07:34:19AM +0000, Christophe Leroy wrote:
> Add support for selective read or write user access with
> user_read_access_begin/end and user_write_access_begin/end.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> arch/powerpc/include/asm/book3s/32/kup.h | 4 ++--
> arch/powerpc/include/asm/kup.h | 14 +++++++++++++-
> arch/powerpc/include/asm/uaccess.h | 22 ++++++++++++++++++++++
> 3 files changed, 37 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/32/kup.h b/arch/powerpc/include/asm/book3s/32/kup.h
> index 3c0ba22dc360..1617e73bee30 100644
> --- a/arch/powerpc/include/asm/book3s/32/kup.h
> +++ b/arch/powerpc/include/asm/book3s/32/kup.h
> @@ -108,7 +108,7 @@ static __always_inline void allow_user_access(void __user *to, const void __user
> u32 addr, end;
>
> BUILD_BUG_ON(!__builtin_constant_p(dir));
> - BUILD_BUG_ON(dir == KUAP_CURRENT);
> + BUILD_BUG_ON(dir & ~KUAP_READ_WRITE);
>
> if (!(dir & KUAP_WRITE))
> return;
> @@ -131,7 +131,7 @@ static __always_inline void prevent_user_access(void __user *to, const void __us
>
> BUILD_BUG_ON(!__builtin_constant_p(dir));
>
> - if (dir == KUAP_CURRENT) {
> + if (dir & KUAP_CURRENT_WRITE) {
> u32 kuap = current->thread.kuap;
>
> if (unlikely(!kuap))
> diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/kup.h
> index 92bcd1a26d73..c745ee41ad66 100644
> --- a/arch/powerpc/include/asm/kup.h
> +++ b/arch/powerpc/include/asm/kup.h
> @@ -10,7 +10,9 @@
> * Use the current saved situation instead of the to/from/size params.
> * Used on book3s/32
> */
> -#define KUAP_CURRENT 4
> +#define KUAP_CURRENT_READ 4
> +#define KUAP_CURRENT_WRITE 8
> +#define KUAP_CURRENT (KUAP_CURRENT_READ | KUAP_CURRENT_WRITE)
>
> #ifdef CONFIG_PPC64
> #include <asm/book3s/64/kup-radix.h>
> @@ -101,6 +103,16 @@ static inline void prevent_current_access_user(void)
> prevent_user_access(NULL, NULL, ~0UL, KUAP_CURRENT);
> }
>
> +static inline void prevent_current_read_from_user(void)
> +{
> + prevent_user_access(NULL, NULL, ~0UL, KUAP_CURRENT_READ);
> +}
> +
> +static inline void prevent_current_write_to_user(void)
> +{
> + prevent_user_access(NULL, NULL, ~0UL, KUAP_CURRENT_WRITE);
> +}
> +
> #endif /* !__ASSEMBLY__ */
>
> #endif /* _ASM_POWERPC_KUAP_H_ */
> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
> index 2f500debae21..4427d419eb1d 100644
> --- a/arch/powerpc/include/asm/uaccess.h
> +++ b/arch/powerpc/include/asm/uaccess.h
> @@ -468,6 +468,28 @@ static __must_check inline bool user_access_begin(const void __user *ptr, size_t
> #define user_access_save prevent_user_access_return
> #define user_access_restore restore_user_access
>
> +static __must_check inline bool
> +user_read_access_begin(const void __user *ptr, size_t len)
> +{
> + if (unlikely(!access_ok(ptr, len)))
> + return false;
> + allow_read_from_user(ptr, len);
> + return true;
> +}
> +#define user_read_access_begin user_read_access_begin
> +#define user_read_access_end prevent_current_read_from_user
> +
> +static __must_check inline bool
> +user_write_access_begin(const void __user *ptr, size_t len)
> +{
> + if (unlikely(!access_ok(ptr, len)))
> + return false;
> + allow_write_to_user((void __user *)ptr, len);
> + return true;
> +}
> +#define user_write_access_begin user_write_access_begin
> +#define user_write_access_end prevent_current_write_to_user
> +
> #define unsafe_op_wrap(op, err) do { if (unlikely(op)) goto err; } while (0)
> #define unsafe_get_user(x, p, e) unsafe_op_wrap(__get_user_allowed(x, p), e)
> #define unsafe_put_user(x, p, e) unsafe_op_wrap(__put_user_allowed(x, p), e)
> --
> 2.25.0
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH RESEND 3/4] drm/i915/gem: Replace user_access_begin by user_write_access_begin
From: Kees Cook @ 2020-04-02 7:52 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-arch, linuxppc-dev, airlied, hpa, linux-kernel, linux-mm,
Paul Mackerras, viro, daniel, akpm, torvalds
In-Reply-To: <6da6fa391c0d6344cc9ff99a69fcaa65666f3947.1585811416.git.christophe.leroy@c-s.fr>
On Thu, Apr 02, 2020 at 07:34:18AM +0000, Christophe Leroy wrote:
> When i915_gem_execbuffer2_ioctl() is using user_access_begin(),
> that's only to perform unsafe_put_user() so use
> user_write_access_begin() in order to only open write access.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Why is this split from the other conversions?
Reviewed-by: Kees Cook <keescook@chromium.org>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index 7643a30ba4cd..4be8205a70b6 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -1611,14 +1611,14 @@ static int eb_copy_relocations(const struct i915_execbuffer *eb)
> * happened we would make the mistake of assuming that the
> * relocations were valid.
> */
> - if (!user_access_begin(urelocs, size))
> + if (!user_write_access_begin(urelocs, size))
> goto end;
>
> for (copied = 0; copied < nreloc; copied++)
> unsafe_put_user(-1,
> &urelocs[copied].presumed_offset,
> end_user);
> - user_access_end();
> + user_write_access_end();
>
> eb->exec[i].relocs_ptr = (uintptr_t)relocs;
> }
> @@ -1626,7 +1626,7 @@ static int eb_copy_relocations(const struct i915_execbuffer *eb)
> return 0;
>
> end_user:
> - user_access_end();
> + user_write_access_end();
> end:
> kvfree(relocs);
> err = -EFAULT;
> @@ -2991,7 +2991,8 @@ i915_gem_execbuffer2_ioctl(struct drm_device *dev, void *data,
> * And this range already got effectively checked earlier
> * when we did the "copy_from_user()" above.
> */
> - if (!user_access_begin(user_exec_list, count * sizeof(*user_exec_list)))
> + if (!user_write_access_begin(user_exec_list,
> + count * sizeof(*user_exec_list)))
> goto end;
>
> for (i = 0; i < args->buffer_count; i++) {
> @@ -3005,7 +3006,7 @@ i915_gem_execbuffer2_ioctl(struct drm_device *dev, void *data,
> end_user);
> }
> end_user:
> - user_access_end();
> + user_write_access_end();
> end:;
> }
>
> --
> 2.25.0
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH RESEND 2/4] uaccess: Selectively open read or write user access
From: Kees Cook @ 2020-04-02 7:51 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-arch, linuxppc-dev, airlied, hpa, linux-kernel, linux-mm,
Paul Mackerras, viro, daniel, akpm, torvalds
In-Reply-To: <25040ad2d2a2cef45a2442b0e934141987e11b71.1585811416.git.christophe.leroy@c-s.fr>
On Thu, Apr 02, 2020 at 07:34:17AM +0000, Christophe Leroy wrote:
> [...]
> diff --git a/kernel/compat.c b/kernel/compat.c
> index 843dd17e6078..705ca7e418c6 100644
> --- a/kernel/compat.c
> +++ b/kernel/compat.c
> @@ -199,7 +199,7 @@ long compat_get_bitmap(unsigned long *mask, const compat_ulong_t __user *umask,
> bitmap_size = ALIGN(bitmap_size, BITS_PER_COMPAT_LONG);
> nr_compat_longs = BITS_TO_COMPAT_LONGS(bitmap_size);
>
> - if (!user_access_begin(umask, bitmap_size / 8))
> + if (!user_write_access_begin(umask, bitmap_size / 8))
This looks mismatched: should be user_read_access_begin()?
> return -EFAULT;
>
> while (nr_compat_longs > 1) {
> @@ -211,11 +211,11 @@ long compat_get_bitmap(unsigned long *mask, const compat_ulong_t __user *umask,
> }
> if (nr_compat_longs)
> unsafe_get_user(*mask, umask++, Efault);
> - user_access_end();
> + user_read_access_end();
> return 0;
>
> Efault:
> - user_access_end();
> + user_read_access_end();
> return -EFAULT;
> }
(These correctly end read access.)
>
> @@ -228,7 +228,7 @@ long compat_put_bitmap(compat_ulong_t __user *umask, unsigned long *mask,
> bitmap_size = ALIGN(bitmap_size, BITS_PER_COMPAT_LONG);
> nr_compat_longs = BITS_TO_COMPAT_LONGS(bitmap_size);
>
> - if (!user_access_begin(umask, bitmap_size / 8))
> + if (!user_read_access_begin(umask, bitmap_size / 8))
And ..._write_... here?
> return -EFAULT;
>
> while (nr_compat_longs > 1) {
> @@ -239,10 +239,10 @@ long compat_put_bitmap(compat_ulong_t __user *umask, unsigned long *mask,
> }
> if (nr_compat_longs)
> unsafe_put_user((compat_ulong_t)*mask, umask++, Efault);
> - user_access_end();
> + user_write_access_end();
> return 0;
> Efault:
> - user_access_end();
> + user_write_access_end();
> return -EFAULT;
> }
(These correctly end write access.)
All the others look correct. With the above fixed:
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v4 6/6] pseries/sysfs: Minimise IPI noise while reading [idle_][s]purr
From: Naveen N. Rao @ 2020-04-02 7:34 UTC (permalink / raw)
To: ego, Michael Ellerman
Cc: Tyrel Datwyler, Vaidyanathan Srinivasan, linuxppc-dev,
linux-kernel, Kamalesh Babulal
In-Reply-To: <20200401120127.GC17237@in.ibm.com>
Gautham R Shenoy wrote:
> Hello Naveen,
>
>
> On Wed, Apr 01, 2020 at 03:28:48PM +0530, Naveen N. Rao wrote:
>> Gautham R. Shenoy wrote:
>> >From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
>> >
> [..snip..]
>
>> >+
>> >+static ssize_t show_purr(struct device *dev,
>> >+ struct device_attribute *attr, char *buf)
>> > {
>> >- u64 *ret = val;
>> >+ struct cpu *cpu = container_of(dev, struct cpu, dev);
>> >+ struct util_acct_stats *stats;
>> >
>> >- *ret = read_this_idle_purr();
>> >+ stats = get_util_stats_ptr(cpu->dev.id);
>> >+ return sprintf(buf, "%llx\n", stats->latest_purr);
>>
>> This alters the behavior of the current sysfs purr file. I am not sure if it
>> is reasonable to return the same PURR value across a 10ms window.
>
>
> It does reduce it to 10ms window. I am not sure if anyone samples PURR
> etc faster than that rate.
>
> I measured how much time it takes to read the purr, spurr, idle_purr,
> idle_spurr files back-to-back. It takes not more than 150us. From
> lparstat will these values be read back-to-back ? If so, we can reduce
> the staleness_tolerance to something like 500us and still avoid extra
> IPIs. If not, what is the maximum delay between the first sysfs file
> read and the last sysfs file read ?
Oh, for lparstat usage, this is perfectly fine.
I meant that there could be other users of [s]purr who might care. I
don't know of one, but since this is an existing sysfs interface, I
wanted to point out that the behavior might change.
>
>>
>> I wonder if we should introduce a sysctl interface to control thresholding.
>> It can default to 0, which disables thresholding so that the existing
>> behavior continues. Applications (lparstat) can optionally set it to suit
>> their use.
>
> We would be introducing 3 new sysfs interfaces that way instead of
> two.
>
> /sys/devices/system/cpu/purr_spurr_staleness
> /sys/devices/system/cpu/cpuX/idle_purr
> /sys/devices/system/cpu/cpuX/idle_spurr
>
> I don't have a problem with this. Nathan, Michael, thoughts on this?
>
>
> The alternative is to have a procfs interface, something like
> /proc/powerpc/resource_util_stats
>
> which gives a listing similar to /proc/stat, i.e
>
> CPUX <purr> <idle_purr> <spurr> <idle_spurr>
>
> Even in this case, the values can be obtained in one-shot with a
> single IPI and be printed in the row corresponding to the CPU.
Right -- and that would be optimal requiring a single system call, at
the cost of using a legacy interface.
The other option would be to drop this patch and to just go with patches
1-5 introducing the new sysfs interfaces for idle_[s]purr. It isn't
entirely clear how often this would be used, or its actual impact. We
can perhaps consider this optimization if and when this causes
problems...
Thanks,
Naveen
^ permalink raw reply
* [PATCH RESEND 3/4] drm/i915/gem: Replace user_access_begin by user_write_access_begin
From: Christophe Leroy @ 2020-04-02 7:34 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, airlied,
daniel, torvalds, viro, akpm, keescook, hpa
Cc: linux-arch, linux-mm, linuxppc-dev, linux-kernel
In-Reply-To: <27106d62fdbd4ffb47796236050e418131cb837f.1585811416.git.christophe.leroy@c-s.fr>
When i915_gem_execbuffer2_ioctl() is using user_access_begin(),
that's only to perform unsafe_put_user() so use
user_write_access_begin() in order to only open write access.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 7643a30ba4cd..4be8205a70b6 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1611,14 +1611,14 @@ static int eb_copy_relocations(const struct i915_execbuffer *eb)
* happened we would make the mistake of assuming that the
* relocations were valid.
*/
- if (!user_access_begin(urelocs, size))
+ if (!user_write_access_begin(urelocs, size))
goto end;
for (copied = 0; copied < nreloc; copied++)
unsafe_put_user(-1,
&urelocs[copied].presumed_offset,
end_user);
- user_access_end();
+ user_write_access_end();
eb->exec[i].relocs_ptr = (uintptr_t)relocs;
}
@@ -1626,7 +1626,7 @@ static int eb_copy_relocations(const struct i915_execbuffer *eb)
return 0;
end_user:
- user_access_end();
+ user_write_access_end();
end:
kvfree(relocs);
err = -EFAULT;
@@ -2991,7 +2991,8 @@ i915_gem_execbuffer2_ioctl(struct drm_device *dev, void *data,
* And this range already got effectively checked earlier
* when we did the "copy_from_user()" above.
*/
- if (!user_access_begin(user_exec_list, count * sizeof(*user_exec_list)))
+ if (!user_write_access_begin(user_exec_list,
+ count * sizeof(*user_exec_list)))
goto end;
for (i = 0; i < args->buffer_count; i++) {
@@ -3005,7 +3006,7 @@ i915_gem_execbuffer2_ioctl(struct drm_device *dev, void *data,
end_user);
}
end_user:
- user_access_end();
+ user_write_access_end();
end:;
}
--
2.25.0
^ permalink raw reply related
* [PATCH RESEND 1/4] uaccess: Add user_read_access_begin/end and user_write_access_begin/end
From: Christophe Leroy @ 2020-04-02 7:34 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, airlied,
daniel, torvalds, viro, akpm, keescook, hpa
Cc: linux-arch, linux-mm, linuxppc-dev, linux-kernel
Some architectures like powerpc64 have the capability to separate
read access and write access protection.
For get_user() and copy_from_user(), powerpc64 only open read access.
For put_user() and copy_to_user(), powerpc64 only open write access.
But when using unsafe_get_user() or unsafe_put_user(),
user_access_begin open both read and write.
Other architectures like powerpc book3s 32 bits only allow write
access protection. And on this architecture protection is an heavy
operation as it requires locking/unlocking per segment of 256Mbytes.
On those architecture it is therefore desirable to do the unlocking
only for write access. (Note that book3s/32 ranges from very old
powermac from the 90's with powerpc 601 processor, till modern
ADSL boxes with PowerQuicc II modern processors for instance so it
is still worth considering)
In order to avoid any risk based of hacking some variable parameters
passed to user_access_begin/end that would allow hacking and
leaving user access open or opening too much, it is preferable to
use dedicated static functions that can't be overridden.
Add a user_read_access_begin and user_read_access_end to only open
read access.
Add a user_write_access_begin and user_write_access_end to only open
write access.
By default, when undefined, those new access helpers default on the
existing user_access_begin and user_access_end.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Link: https://patchwork.ozlabs.org/patch/1227926/
---
Resending this series as I mistakenly only sent it to powerpc list
begining of February (https://patchwork.ozlabs.org/patch/1233172/)
This series is based on the discussion we had in January, see
https://patchwork.ozlabs.org/patch/1227926/ . I tried to
take into account all remarks, especially @hpa 's remark to use
a fixed API on not base the relocking on a magic id returned at
unlocking.
This series is awaited for implementing selective lkdtm test to
test powerpc64 independant read and write protection, see
https://patchwork.ozlabs.org/patch/1231765/
include/linux/uaccess.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index 67f016010aad..9861c89f93be 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -378,6 +378,14 @@ extern long strnlen_unsafe_user(const void __user *unsafe_addr, long count);
static inline unsigned long user_access_save(void) { return 0UL; }
static inline void user_access_restore(unsigned long flags) { }
#endif
+#ifndef user_write_access_begin
+#define user_write_access_begin user_access_begin
+#define user_write_access_end user_access_end
+#endif
+#ifndef user_read_access_begin
+#define user_read_access_begin user_access_begin
+#define user_read_access_end user_access_end
+#endif
#ifdef CONFIG_HARDENED_USERCOPY
void usercopy_warn(const char *name, const char *detail, bool to_user,
--
2.25.0
^ permalink raw reply related
* [PATCH RESEND 4/4] powerpc/uaccess: Implement user_read_access_begin and user_write_access_begin
From: Christophe Leroy @ 2020-04-02 7:34 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, airlied,
daniel, torvalds, viro, akpm, keescook, hpa
Cc: linux-arch, linux-mm, linuxppc-dev, linux-kernel
In-Reply-To: <27106d62fdbd4ffb47796236050e418131cb837f.1585811416.git.christophe.leroy@c-s.fr>
Add support for selective read or write user access with
user_read_access_begin/end and user_write_access_begin/end.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/book3s/32/kup.h | 4 ++--
arch/powerpc/include/asm/kup.h | 14 +++++++++++++-
arch/powerpc/include/asm/uaccess.h | 22 ++++++++++++++++++++++
3 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/kup.h b/arch/powerpc/include/asm/book3s/32/kup.h
index 3c0ba22dc360..1617e73bee30 100644
--- a/arch/powerpc/include/asm/book3s/32/kup.h
+++ b/arch/powerpc/include/asm/book3s/32/kup.h
@@ -108,7 +108,7 @@ static __always_inline void allow_user_access(void __user *to, const void __user
u32 addr, end;
BUILD_BUG_ON(!__builtin_constant_p(dir));
- BUILD_BUG_ON(dir == KUAP_CURRENT);
+ BUILD_BUG_ON(dir & ~KUAP_READ_WRITE);
if (!(dir & KUAP_WRITE))
return;
@@ -131,7 +131,7 @@ static __always_inline void prevent_user_access(void __user *to, const void __us
BUILD_BUG_ON(!__builtin_constant_p(dir));
- if (dir == KUAP_CURRENT) {
+ if (dir & KUAP_CURRENT_WRITE) {
u32 kuap = current->thread.kuap;
if (unlikely(!kuap))
diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/kup.h
index 92bcd1a26d73..c745ee41ad66 100644
--- a/arch/powerpc/include/asm/kup.h
+++ b/arch/powerpc/include/asm/kup.h
@@ -10,7 +10,9 @@
* Use the current saved situation instead of the to/from/size params.
* Used on book3s/32
*/
-#define KUAP_CURRENT 4
+#define KUAP_CURRENT_READ 4
+#define KUAP_CURRENT_WRITE 8
+#define KUAP_CURRENT (KUAP_CURRENT_READ | KUAP_CURRENT_WRITE)
#ifdef CONFIG_PPC64
#include <asm/book3s/64/kup-radix.h>
@@ -101,6 +103,16 @@ static inline void prevent_current_access_user(void)
prevent_user_access(NULL, NULL, ~0UL, KUAP_CURRENT);
}
+static inline void prevent_current_read_from_user(void)
+{
+ prevent_user_access(NULL, NULL, ~0UL, KUAP_CURRENT_READ);
+}
+
+static inline void prevent_current_write_to_user(void)
+{
+ prevent_user_access(NULL, NULL, ~0UL, KUAP_CURRENT_WRITE);
+}
+
#endif /* !__ASSEMBLY__ */
#endif /* _ASM_POWERPC_KUAP_H_ */
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 2f500debae21..4427d419eb1d 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -468,6 +468,28 @@ static __must_check inline bool user_access_begin(const void __user *ptr, size_t
#define user_access_save prevent_user_access_return
#define user_access_restore restore_user_access
+static __must_check inline bool
+user_read_access_begin(const void __user *ptr, size_t len)
+{
+ if (unlikely(!access_ok(ptr, len)))
+ return false;
+ allow_read_from_user(ptr, len);
+ return true;
+}
+#define user_read_access_begin user_read_access_begin
+#define user_read_access_end prevent_current_read_from_user
+
+static __must_check inline bool
+user_write_access_begin(const void __user *ptr, size_t len)
+{
+ if (unlikely(!access_ok(ptr, len)))
+ return false;
+ allow_write_to_user((void __user *)ptr, len);
+ return true;
+}
+#define user_write_access_begin user_write_access_begin
+#define user_write_access_end prevent_current_write_to_user
+
#define unsafe_op_wrap(op, err) do { if (unlikely(op)) goto err; } while (0)
#define unsafe_get_user(x, p, e) unsafe_op_wrap(__get_user_allowed(x, p), e)
#define unsafe_put_user(x, p, e) unsafe_op_wrap(__put_user_allowed(x, p), e)
--
2.25.0
^ permalink raw reply related
* [PATCH RESEND 2/4] uaccess: Selectively open read or write user access
From: Christophe Leroy @ 2020-04-02 7:34 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, airlied,
daniel, torvalds, viro, akpm, keescook, hpa
Cc: linux-arch, linux-mm, linuxppc-dev, linux-kernel
In-Reply-To: <27106d62fdbd4ffb47796236050e418131cb837f.1585811416.git.christophe.leroy@c-s.fr>
When opening user access to only perform reads, only open read access.
When opening user access to only perform writes, only open write
access.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
fs/readdir.c | 12 ++++++------
kernel/compat.c | 12 ++++++------
kernel/exit.c | 12 ++++++------
lib/strncpy_from_user.c | 4 ++--
lib/strnlen_user.c | 4 ++--
lib/usercopy.c | 6 +++---
6 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/fs/readdir.c b/fs/readdir.c
index de2eceffdee8..ed6aaad451aa 100644
--- a/fs/readdir.c
+++ b/fs/readdir.c
@@ -242,7 +242,7 @@ static int filldir(struct dir_context *ctx, const char *name, int namlen,
return -EINTR;
dirent = buf->current_dir;
prev = (void __user *) dirent - prev_reclen;
- if (!user_access_begin(prev, reclen + prev_reclen))
+ if (!user_write_access_begin(prev, reclen + prev_reclen))
goto efault;
/* This might be 'dirent->d_off', but if so it will get overwritten */
@@ -251,14 +251,14 @@ static int filldir(struct dir_context *ctx, const char *name, int namlen,
unsafe_put_user(reclen, &dirent->d_reclen, efault_end);
unsafe_put_user(d_type, (char __user *) dirent + reclen - 1, efault_end);
unsafe_copy_dirent_name(dirent->d_name, name, namlen, efault_end);
- user_access_end();
+ user_write_access_end();
buf->current_dir = (void __user *)dirent + reclen;
buf->prev_reclen = reclen;
buf->count -= reclen;
return 0;
efault_end:
- user_access_end();
+ user_write_access_end();
efault:
buf->error = -EFAULT;
return -EFAULT;
@@ -327,7 +327,7 @@ static int filldir64(struct dir_context *ctx, const char *name, int namlen,
return -EINTR;
dirent = buf->current_dir;
prev = (void __user *)dirent - prev_reclen;
- if (!user_access_begin(prev, reclen + prev_reclen))
+ if (!user_write_access_begin(prev, reclen + prev_reclen))
goto efault;
/* This might be 'dirent->d_off', but if so it will get overwritten */
@@ -336,7 +336,7 @@ static int filldir64(struct dir_context *ctx, const char *name, int namlen,
unsafe_put_user(reclen, &dirent->d_reclen, efault_end);
unsafe_put_user(d_type, &dirent->d_type, efault_end);
unsafe_copy_dirent_name(dirent->d_name, name, namlen, efault_end);
- user_access_end();
+ user_write_access_end();
buf->prev_reclen = reclen;
buf->current_dir = (void __user *)dirent + reclen;
@@ -344,7 +344,7 @@ static int filldir64(struct dir_context *ctx, const char *name, int namlen,
return 0;
efault_end:
- user_access_end();
+ user_write_access_end();
efault:
buf->error = -EFAULT;
return -EFAULT;
diff --git a/kernel/compat.c b/kernel/compat.c
index 843dd17e6078..705ca7e418c6 100644
--- a/kernel/compat.c
+++ b/kernel/compat.c
@@ -199,7 +199,7 @@ long compat_get_bitmap(unsigned long *mask, const compat_ulong_t __user *umask,
bitmap_size = ALIGN(bitmap_size, BITS_PER_COMPAT_LONG);
nr_compat_longs = BITS_TO_COMPAT_LONGS(bitmap_size);
- if (!user_access_begin(umask, bitmap_size / 8))
+ if (!user_write_access_begin(umask, bitmap_size / 8))
return -EFAULT;
while (nr_compat_longs > 1) {
@@ -211,11 +211,11 @@ long compat_get_bitmap(unsigned long *mask, const compat_ulong_t __user *umask,
}
if (nr_compat_longs)
unsafe_get_user(*mask, umask++, Efault);
- user_access_end();
+ user_read_access_end();
return 0;
Efault:
- user_access_end();
+ user_read_access_end();
return -EFAULT;
}
@@ -228,7 +228,7 @@ long compat_put_bitmap(compat_ulong_t __user *umask, unsigned long *mask,
bitmap_size = ALIGN(bitmap_size, BITS_PER_COMPAT_LONG);
nr_compat_longs = BITS_TO_COMPAT_LONGS(bitmap_size);
- if (!user_access_begin(umask, bitmap_size / 8))
+ if (!user_read_access_begin(umask, bitmap_size / 8))
return -EFAULT;
while (nr_compat_longs > 1) {
@@ -239,10 +239,10 @@ long compat_put_bitmap(compat_ulong_t __user *umask, unsigned long *mask,
}
if (nr_compat_longs)
unsafe_put_user((compat_ulong_t)*mask, umask++, Efault);
- user_access_end();
+ user_write_access_end();
return 0;
Efault:
- user_access_end();
+ user_write_access_end();
return -EFAULT;
}
diff --git a/kernel/exit.c b/kernel/exit.c
index d70d47159640..61b2f7a85079 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1555,7 +1555,7 @@ SYSCALL_DEFINE5(waitid, int, which, pid_t, upid, struct siginfo __user *,
if (!infop)
return err;
- if (!user_access_begin(infop, sizeof(*infop)))
+ if (!user_write_access_begin(infop, sizeof(*infop)))
return -EFAULT;
unsafe_put_user(signo, &infop->si_signo, Efault);
@@ -1564,10 +1564,10 @@ SYSCALL_DEFINE5(waitid, int, which, pid_t, upid, struct siginfo __user *,
unsafe_put_user(info.pid, &infop->si_pid, Efault);
unsafe_put_user(info.uid, &infop->si_uid, Efault);
unsafe_put_user(info.status, &infop->si_status, Efault);
- user_access_end();
+ user_write_access_end();
return err;
Efault:
- user_access_end();
+ user_write_access_end();
return -EFAULT;
}
@@ -1682,7 +1682,7 @@ COMPAT_SYSCALL_DEFINE5(waitid,
if (!infop)
return err;
- if (!user_access_begin(infop, sizeof(*infop)))
+ if (!user_write_access_begin(infop, sizeof(*infop)))
return -EFAULT;
unsafe_put_user(signo, &infop->si_signo, Efault);
@@ -1691,10 +1691,10 @@ COMPAT_SYSCALL_DEFINE5(waitid,
unsafe_put_user(info.pid, &infop->si_pid, Efault);
unsafe_put_user(info.uid, &infop->si_uid, Efault);
unsafe_put_user(info.status, &infop->si_status, Efault);
- user_access_end();
+ user_write_access_end();
return err;
Efault:
- user_access_end();
+ user_write_access_end();
return -EFAULT;
}
#endif
diff --git a/lib/strncpy_from_user.c b/lib/strncpy_from_user.c
index 706020b06617..b90ec550183a 100644
--- a/lib/strncpy_from_user.c
+++ b/lib/strncpy_from_user.c
@@ -116,9 +116,9 @@ long strncpy_from_user(char *dst, const char __user *src, long count)
kasan_check_write(dst, count);
check_object_size(dst, count, false);
- if (user_access_begin(src, max)) {
+ if (user_read_access_begin(src, max)) {
retval = do_strncpy_from_user(dst, src, count, max);
- user_access_end();
+ user_read_access_end();
return retval;
}
}
diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c
index 41670d4a5816..1616710b8a82 100644
--- a/lib/strnlen_user.c
+++ b/lib/strnlen_user.c
@@ -109,9 +109,9 @@ long strnlen_user(const char __user *str, long count)
if (max > count)
max = count;
- if (user_access_begin(str, max)) {
+ if (user_read_access_begin(str, max)) {
retval = do_strnlen_user(str, count, max);
- user_access_end();
+ user_read_access_end();
return retval;
}
}
diff --git a/lib/usercopy.c b/lib/usercopy.c
index cbb4d9ec00f2..ca2a697a2061 100644
--- a/lib/usercopy.c
+++ b/lib/usercopy.c
@@ -58,7 +58,7 @@ int check_zeroed_user(const void __user *from, size_t size)
from -= align;
size += align;
- if (!user_access_begin(from, size))
+ if (!user_read_access_begin(from, size))
return -EFAULT;
unsafe_get_user(val, (unsigned long __user *) from, err_fault);
@@ -79,10 +79,10 @@ int check_zeroed_user(const void __user *from, size_t size)
val &= aligned_byte_mask(size);
done:
- user_access_end();
+ user_read_access_end();
return (val == 0);
err_fault:
- user_access_end();
+ user_read_access_end();
return -EFAULT;
}
EXPORT_SYMBOL(check_zeroed_user);
--
2.25.0
^ permalink raw reply related
* [PATCH v10 14/14] powerpc: Use mm_context vas_windows counter to issue CP_ABORT
From: Haren Myneni @ 2020-04-02 7:20 UTC (permalink / raw)
To: mpe
Cc: mikey, ajd, frederic.barrat, linux-kernel, npiggin, hch, oohall,
clg, herbert, sukadev, linuxppc-dev, srikar
In-Reply-To: <1585810846.2275.23.camel@hbabu-laptop>
set_thread_uses_vas() sets used_vas flag for a process that opened VAS
window and issue CP_ABORT during context switch for only that process.
In multi-thread application, windows can be shared. For example Thread A
can open a window and Thread B can run COPY/PASTE instructions to send
NX request which may cause corruption or snooping or a covert channel.
Also once this flag is set, continue to run CP_ABORT even the VAS window
is closed.
So define vas-windows counter in process mm_context, increment this
counter for each window open and decrement it for window close. If
vas-windows is set, issue CP_ABORT during context switch. It means
clear the foreign real address mapping only if the process / thread uses
COPY/PASTE. Then disable it for that process if windows are not open.
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
Reported-by: Nicholas Piggin <npiggin@gmail.com>
Suggested-by: Milton Miller <miltonm@us.ibm.com>
Suggested-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/book3s/64/mmu.h | 3 +++
arch/powerpc/include/asm/mmu_context.h | 22 ++++++++++++++++++++++
arch/powerpc/include/asm/processor.h | 1 -
arch/powerpc/kernel/process.c | 8 ++++++--
arch/powerpc/platforms/powernv/vas-window.c | 1 +
5 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h
index bb3deb7..f0a9ff6 100644
--- a/arch/powerpc/include/asm/book3s/64/mmu.h
+++ b/arch/powerpc/include/asm/book3s/64/mmu.h
@@ -116,6 +116,9 @@ struct patb_entry {
/* Number of users of the external (Nest) MMU */
atomic_t copros;
+ /* Number of user space windows opened in process mm_context */
+ atomic_t vas_windows;
+
struct hash_mm_context *hash_context;
unsigned long vdso_base;
diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
index 360367c..7fd249498 100644
--- a/arch/powerpc/include/asm/mmu_context.h
+++ b/arch/powerpc/include/asm/mmu_context.h
@@ -185,11 +185,33 @@ static inline void mm_context_remove_copro(struct mm_struct *mm)
dec_mm_active_cpus(mm);
}
}
+
+/*
+ * vas_windows counter shows number of open windows in the mm
+ * context. During context switch, use this counter to clear the
+ * foreign real address mapping (CP_ABORT) for the thread / process
+ * that intend to use COPY/PASTE. When a process closes all windows,
+ * disable CP_ABORT which is expensive to run.
+ */
+static inline void mm_context_add_vas_windows(struct mm_struct *mm)
+{
+ atomic_inc(&mm->context.vas_windows);
+}
+
+static inline void mm_context_remove_vas_windows(struct mm_struct *mm)
+{
+ int c = atomic_dec_if_positive(&mm->context.vas_windows);
+
+ /* Detect imbalance between add and remove */
+ WARN_ON(c < 0);
+}
#else
static inline void inc_mm_active_cpus(struct mm_struct *mm) { }
static inline void dec_mm_active_cpus(struct mm_struct *mm) { }
static inline void mm_context_add_copro(struct mm_struct *mm) { }
static inline void mm_context_remove_copro(struct mm_struct *mm) { }
+static inline void mm_context_add_vas_windows(struct mm_struct *mm) { }
+static inline void mm_context_remove_vas_windows(struct mm_struct *mm) { }
#endif
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index eedcbfb..bfa336f 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -272,7 +272,6 @@ struct thread_struct {
unsigned mmcr0;
unsigned used_ebb;
- unsigned int used_vas;
#endif
};
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index fad50db..a3ecaf9 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1221,7 +1221,8 @@ struct task_struct *__switch_to(struct task_struct *prev,
* mappings, we must issue a cp_abort to clear any state and
* prevent snooping, corruption or a covert channel.
*/
- if (current->thread.used_vas)
+ if (current->mm &&
+ atomic_read(¤t->mm->context.vas_windows))
asm volatile(PPC_CP_ABORT);
}
#endif /* CONFIG_PPC_BOOK3S_64 */
@@ -1466,7 +1467,10 @@ int set_thread_uses_vas(void)
if (!cpu_has_feature(CPU_FTR_ARCH_300))
return -EINVAL;
- current->thread.used_vas = 1;
+ if (!current->mm)
+ return -EINVAL;
+
+ mm_context_add_vas_windows(current->mm);
/*
* Even a process that has no foreign real address mapping can use
diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
index 3ffad5a..33dfbbf 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -1333,6 +1333,7 @@ int vas_win_close(struct vas_window *window)
put_pid(window->pid);
if (window->mm) {
mm_context_remove_copro(window->mm);
+ mm_context_remove_vas_windows(window->mm);
mmdrop(window->mm);
}
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH v10 13/14] powerpc/vas: Free send window in VAS instance after credits returned
From: Haren Myneni @ 2020-04-02 7:19 UTC (permalink / raw)
To: mpe
Cc: mikey, ajd, frederic.barrat, linux-kernel, npiggin, hch, oohall,
clg, herbert, sukadev, linuxppc-dev, srikar
In-Reply-To: <1585810846.2275.23.camel@hbabu-laptop>
NX may be processing requests while trying to close window. Wait until
all credits are returned and then free send window from VAS instance.
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
arch/powerpc/platforms/powernv/vas-window.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
index 3d80f37..3ffad5a 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -1316,14 +1316,14 @@ int vas_win_close(struct vas_window *window)
unmap_paste_region(window);
- clear_vinst_win(window);
-
poll_window_busy_state(window);
unpin_close_window(window);
poll_window_credits(window);
+ clear_vinst_win(window);
+
poll_window_castout(window);
/* if send window, drop reference to matching receive window */
--
1.8.3.1
^ permalink raw reply related
* [PATCH v10 12/14] powerpc/vas: Display process stuck message
From: Haren Myneni @ 2020-04-02 7:18 UTC (permalink / raw)
To: mpe
Cc: mikey, ajd, frederic.barrat, linux-kernel, npiggin, hch, oohall,
clg, herbert, sukadev, linuxppc-dev, srikar
In-Reply-To: <1585810846.2275.23.camel@hbabu-laptop>
Process can not close send window until all requests are processed.
Means wait until window state is not busy and send credits are
returned. Display debug messages in case taking longer to close the
window.
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
arch/powerpc/platforms/powernv/vas-window.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
index 4b5adf5..3d80f37 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -1181,6 +1181,7 @@ static void poll_window_credits(struct vas_window *window)
{
u64 val;
int creds, mode;
+ int count = 0;
val = read_hvwc_reg(window, VREG(WINCTL));
if (window->tx_win)
@@ -1199,10 +1200,27 @@ static void poll_window_credits(struct vas_window *window)
creds = GET_FIELD(VAS_LRX_WCRED, val);
}
+ /*
+ * Takes around few milliseconds to complete all pending requests
+ * and return credits.
+ * TODO: Scan fault FIFO and invalidate CRBs points to this window
+ * and issue CRB Kill to stop all pending requests. Need only
+ * if there is a bug in NX or fault handling in kernel.
+ */
if (creds < window->wcreds_max) {
val = 0;
set_current_state(TASK_UNINTERRUPTIBLE);
schedule_timeout(msecs_to_jiffies(10));
+ count++;
+ /*
+ * Process can not close send window until all credits are
+ * returned.
+ */
+ if (!(count % 10000))
+ pr_debug("VAS: pid %d stuck. Waiting for credits returned for Window(%d). creds %d, Retries %d\n",
+ vas_window_pid(window), window->winid,
+ creds, count);
+
goto retry;
}
}
@@ -1216,6 +1234,7 @@ static void poll_window_busy_state(struct vas_window *window)
{
int busy;
u64 val;
+ int count = 0;
retry:
val = read_hvwc_reg(window, VREG(WIN_STATUS));
@@ -1224,6 +1243,15 @@ static void poll_window_busy_state(struct vas_window *window)
val = 0;
set_current_state(TASK_UNINTERRUPTIBLE);
schedule_timeout(msecs_to_jiffies(5));
+ count++;
+ /*
+ * Takes around few milliseconds to process all pending
+ * requests.
+ */
+ if (!(count % 10000))
+ pr_debug("VAS: pid %d stuck. Window (ID=%d) is in busy state. Retries %d\n",
+ vas_window_pid(window), window->winid, count);
+
goto retry;
}
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH v10 11/14] powerpc/vas: Do not use default credits for receive window
From: Haren Myneni @ 2020-04-02 7:17 UTC (permalink / raw)
To: mpe
Cc: mikey, ajd, frederic.barrat, linux-kernel, npiggin, hch, oohall,
clg, herbert, sukadev, linuxppc-dev, srikar
In-Reply-To: <1585810846.2275.23.camel@hbabu-laptop>
System checkstops if RxFIFO overruns with more requests than the
maximum possible number of CRBs allowed in FIFO at any time. So
max credits value (rxattr.wcreds_max) is set and is passed to
vas_rx_win_open() by the the driver.
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
arch/powerpc/platforms/powernv/vas-window.c | 4 ++--
arch/powerpc/platforms/powernv/vas.h | 2 --
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
index 3ef7120..4b5adf5 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -772,7 +772,7 @@ static bool rx_win_args_valid(enum vas_cop_type cop,
if (attr->rx_fifo_size > VAS_RX_FIFO_SIZE_MAX)
return false;
- if (attr->wcreds_max > VAS_RX_WCREDS_MAX)
+ if (!attr->wcreds_max)
return false;
if (attr->nx_win) {
@@ -877,7 +877,7 @@ struct vas_window *vas_rx_win_open(int vasid, enum vas_cop_type cop,
rxwin->nx_win = rxattr->nx_win;
rxwin->user_win = rxattr->user_win;
rxwin->cop = cop;
- rxwin->wcreds_max = rxattr->wcreds_max ?: VAS_WCREDS_DEFAULT;
+ rxwin->wcreds_max = rxattr->wcreds_max;
init_winctx_for_rxwin(rxwin, rxattr, &winctx);
init_winctx_regs(rxwin, &winctx);
diff --git a/arch/powerpc/platforms/powernv/vas.h b/arch/powerpc/platforms/powernv/vas.h
index efdaa28..32b5261 100644
--- a/arch/powerpc/platforms/powernv/vas.h
+++ b/arch/powerpc/platforms/powernv/vas.h
@@ -101,11 +101,9 @@
/*
* Initial per-process credits.
* Max send window credits: 4K-1 (12-bits in VAS_TX_WCRED)
- * Max receive window credits: 64K-1 (16 bits in VAS_LRX_WCRED)
*
* TODO: Needs tuning for per-process credits
*/
-#define VAS_RX_WCREDS_MAX ((64 << 10) - 1)
#define VAS_TX_WCREDS_MAX ((4 << 10) - 1)
#define VAS_WCREDS_DEFAULT (1 << 10)
--
1.8.3.1
^ permalink raw reply related
* [PATCH v10 10/14] powerpc/vas: Print CRB and FIFO values
From: Haren Myneni @ 2020-04-02 7:16 UTC (permalink / raw)
To: mpe
Cc: mikey, ajd, frederic.barrat, linux-kernel, npiggin, hch, oohall,
clg, herbert, sukadev, linuxppc-dev, srikar
In-Reply-To: <1585810846.2275.23.camel@hbabu-laptop>
Dump FIFO entries if could not find send window and print CRB
for debugging.
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
arch/powerpc/platforms/powernv/vas-fault.c | 41 ++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/vas-fault.c b/arch/powerpc/platforms/powernv/vas-fault.c
index b6bec64..25db70b 100644
--- a/arch/powerpc/platforms/powernv/vas-fault.c
+++ b/arch/powerpc/platforms/powernv/vas-fault.c
@@ -26,6 +26,28 @@
*/
#define VAS_FAULT_WIN_FIFO_SIZE (4 << 20)
+static void dump_crb(struct coprocessor_request_block *crb)
+{
+ struct data_descriptor_entry *dde;
+ struct nx_fault_stamp *nx;
+
+ dde = &crb->source;
+ pr_devel("SrcDDE: addr 0x%llx, len %d, count %d, idx %d, flags %d\n",
+ be64_to_cpu(dde->address), be32_to_cpu(dde->length),
+ dde->count, dde->index, dde->flags);
+
+ dde = &crb->target;
+ pr_devel("TgtDDE: addr 0x%llx, len %d, count %d, idx %d, flags %d\n",
+ be64_to_cpu(dde->address), be32_to_cpu(dde->length),
+ dde->count, dde->index, dde->flags);
+
+ nx = &crb->stamp.nx;
+ pr_devel("NX Stamp: PSWID 0x%x, FSA 0x%llx, flags 0x%x, FS 0x%x\n",
+ be32_to_cpu(nx->pswid),
+ be64_to_cpu(crb->stamp.nx.fault_storage_addr),
+ nx->flags, nx->fault_status);
+}
+
/*
* Update the CSB to indicate a translation error.
*
@@ -148,6 +170,23 @@ static void update_csb(struct vas_window *window,
pid_vnr(pid), rc);
}
+static void dump_fifo(struct vas_instance *vinst, void *entry)
+{
+ unsigned long *end = vinst->fault_fifo + vinst->fault_fifo_size;
+ unsigned long *fifo = entry;
+ int i;
+
+ pr_err("Fault fifo size %d, Max crbs %d\n", vinst->fault_fifo_size,
+ vinst->fault_fifo_size / CRB_SIZE);
+
+ /* Dump 10 CRB entries or until end of FIFO */
+ pr_err("Fault FIFO Dump:\n");
+ for (i = 0; i < 10*(CRB_SIZE/8) && fifo < end; i += 4, fifo += 4) {
+ pr_err("[%.3d, %p]: 0x%.16lx 0x%.16lx 0x%.16lx 0x%.16lx\n",
+ i, fifo, *fifo, *(fifo+1), *(fifo+2), *(fifo+3));
+ }
+}
+
/*
* Process valid CRBs in fault FIFO.
* NX process user space requests, return credit and update the status
@@ -233,6 +272,7 @@ irqreturn_t vas_fault_thread_fn(int irq, void *data)
vinst->vas_id, vinst->fault_fifo, fifo,
vinst->fault_crbs);
+ dump_crb(crb);
window = vas_pswid_to_window(vinst,
be32_to_cpu(crb->stamp.nx.pswid));
@@ -245,6 +285,7 @@ irqreturn_t vas_fault_thread_fn(int irq, void *data)
* But we should not get here.
* TODO: Disable IRQ.
*/
+ dump_fifo(vinst, (void *)entry);
pr_err("VAS[%d] fault_fifo %p, fifo %p, pswid 0x%x, fault_crbs %d bad CRB?\n",
vinst->vas_id, vinst->fault_fifo, fifo,
be32_to_cpu(crb->stamp.nx.pswid),
--
1.8.3.1
^ permalink raw reply related
* [PATCH v10 09/14] powerpc/vas: Return credits after handling fault
From: Haren Myneni @ 2020-04-02 7:15 UTC (permalink / raw)
To: mpe
Cc: mikey, ajd, frederic.barrat, linux-kernel, npiggin, hch, oohall,
clg, herbert, sukadev, linuxppc-dev, srikar
In-Reply-To: <1585810846.2275.23.camel@hbabu-laptop>
NX uses credit mechanism to control the number of requests issued on
a specific window at any point of time. Only send windows and fault
window are used credits. When the request is issued on a given window,
a credit is taken. This credit will be returned after that request is
processed. If credits are not available, returns RMA_Busy for send
window and RMA_Reject for fault window.
NX expects OS to return credit for send window after processing fault
CRB. Also credit has to be returned for fault window after handling
the fault.
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
arch/powerpc/platforms/powernv/vas-fault.c | 9 ++++++++
arch/powerpc/platforms/powernv/vas-window.c | 36 +++++++++++++++++++++++++++++
arch/powerpc/platforms/powernv/vas.h | 1 +
3 files changed, 46 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/vas-fault.c b/arch/powerpc/platforms/powernv/vas-fault.c
index 354577d..b6bec64 100644
--- a/arch/powerpc/platforms/powernv/vas-fault.c
+++ b/arch/powerpc/platforms/powernv/vas-fault.c
@@ -224,6 +224,10 @@ irqreturn_t vas_fault_thread_fn(int irq, void *data)
memcpy(crb, fifo, CRB_SIZE);
entry->stamp.nx.pswid = cpu_to_be32(FIFO_INVALID_ENTRY);
entry->ccw |= cpu_to_be32(CCW0_INVALID);
+ /*
+ * Return credit for the fault window.
+ */
+ vas_return_credit(vinst->fault_win, false);
pr_devel("VAS[%d] fault_fifo %p, fifo %p, fault_crbs %d\n",
vinst->vas_id, vinst->fault_fifo, fifo,
@@ -249,6 +253,11 @@ irqreturn_t vas_fault_thread_fn(int irq, void *data)
WARN_ON_ONCE(1);
} else {
update_csb(window, crb);
+ /*
+ * Return credit for send window after processing
+ * fault CRB.
+ */
+ vas_return_credit(window, true);
}
}
}
diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
index f12f7eb..3ef7120 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -1317,6 +1317,42 @@ int vas_win_close(struct vas_window *window)
}
EXPORT_SYMBOL_GPL(vas_win_close);
+/*
+ * Return credit for the given window.
+ * Send windows and fault window uses credit mechanism as follows:
+ *
+ * Send windows:
+ * - The default number of credits available for each send window is
+ * 1024. It means 1024 requests can be issued asynchronously at the
+ * same time. If the credit is not available, that request will be
+ * returned with RMA_Busy.
+ * - One credit is taken when NX request is issued.
+ * - This credit is returned after NX processed that request.
+ * - If NX encounters translation error, kernel will return the
+ * credit on the specific send window after processing the fault CRB.
+ *
+ * Fault window:
+ * - The total number credits available is FIFO_SIZE/CRB_SIZE.
+ * Means 4MB/128 in the current implementation. If credit is not
+ * available, RMA_Reject is returned.
+ * - A credit is taken when NX pastes CRB in fault FIFO.
+ * - The kernel with return credit on fault window after reading entry
+ * from fault FIFO.
+ */
+void vas_return_credit(struct vas_window *window, bool tx)
+{
+ uint64_t val;
+
+ val = 0ULL;
+ if (tx) { /* send window */
+ val = SET_FIELD(VAS_TX_WCRED, val, 1);
+ write_hvwc_reg(window, VREG(TX_WCRED_ADDER), val);
+ } else {
+ val = SET_FIELD(VAS_LRX_WCRED, val, 1);
+ write_hvwc_reg(window, VREG(LRX_WCRED_ADDER), val);
+ }
+}
+
struct vas_window *vas_pswid_to_window(struct vas_instance *vinst,
uint32_t pswid)
{
diff --git a/arch/powerpc/platforms/powernv/vas.h b/arch/powerpc/platforms/powernv/vas.h
index 0af7912..efdaa28 100644
--- a/arch/powerpc/platforms/powernv/vas.h
+++ b/arch/powerpc/platforms/powernv/vas.h
@@ -436,6 +436,7 @@ struct vas_winctx {
extern int vas_setup_fault_window(struct vas_instance *vinst);
extern irqreturn_t vas_fault_thread_fn(int irq, void *data);
extern irqreturn_t vas_fault_handler(int irq, void *dev_id);
+extern void vas_return_credit(struct vas_window *window, bool tx);
extern struct vas_window *vas_pswid_to_window(struct vas_instance *vinst,
uint32_t pswid);
--
1.8.3.1
^ permalink raw reply related
* [PATCH v10 08/14] powerpc/vas: Update CSB and notify process for fault CRBs
From: Haren Myneni @ 2020-04-02 7:14 UTC (permalink / raw)
To: mpe
Cc: mikey, ajd, frederic.barrat, linux-kernel, npiggin, hch, oohall,
clg, herbert, sukadev, linuxppc-dev, srikar
In-Reply-To: <1585810846.2275.23.camel@hbabu-laptop>
Applications polls on CSB for the status update after requests are
issued. NX process these requests and update the CSB with the status.
If it encounters translation error, pastes CRB in fault FIFO and
raises an interrupt. The kernel handles fault by reading CRB from
fault FIFO and process the fault CRB.
For each fault CRB, update fault address in CRB (fault_storage_addr)
and translation error status in CSB so that user space can touch the
fault address and resend the request. If the user space passed invalid
CSB address send signal to process with SIGSEGV.
In the case of multi-thread applications, child thread may not be
available. So if the task is not running, send signal to tgid.
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
arch/powerpc/platforms/powernv/vas-fault.c | 126 ++++++++++++++++++++++++++++-
1 file changed, 125 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/powernv/vas-fault.c b/arch/powerpc/platforms/powernv/vas-fault.c
index 0da8358..354577d 100644
--- a/arch/powerpc/platforms/powernv/vas-fault.c
+++ b/arch/powerpc/platforms/powernv/vas-fault.c
@@ -11,6 +11,7 @@
#include <linux/slab.h>
#include <linux/uaccess.h>
#include <linux/kthread.h>
+#include <linux/sched/signal.h>
#include <linux/mmu_context.h>
#include <asm/icswx.h>
@@ -26,6 +27,128 @@
#define VAS_FAULT_WIN_FIFO_SIZE (4 << 20)
/*
+ * Update the CSB to indicate a translation error.
+ *
+ * User space will be polling on CSB after the request is issued.
+ * If NX can handle the request without any issues, it updates CSB.
+ * Whereas if NX encounters page fault, the kernel will handle the
+ * fault and update CSB with translation error.
+ *
+ * If we are unable to update the CSB means copy_to_user failed due to
+ * invalid csb_addr, send a signal to the process.
+ */
+static void update_csb(struct vas_window *window,
+ struct coprocessor_request_block *crb)
+{
+ struct coprocessor_status_block csb;
+ struct kernel_siginfo info;
+ struct task_struct *tsk;
+ void __user *csb_addr;
+ struct pid *pid;
+ int rc;
+
+ /*
+ * NX user space windows can not be opened for task->mm=NULL
+ * and faults will not be generated for kernel requests.
+ */
+ if (WARN_ON_ONCE(!window->mm || !window->user_win))
+ return;
+
+ csb_addr = (void __user *)be64_to_cpu(crb->csb_addr);
+
+ memset(&csb, 0, sizeof(csb));
+ csb.cc = CSB_CC_TRANSLATION;
+ csb.ce = CSB_CE_TERMINATION;
+ csb.cs = 0;
+ csb.count = 0;
+
+ /*
+ * NX operates and returns in BE format as defined CRB struct.
+ * So saves fault_storage_addr in BE as NX pastes in FIFO and
+ * expects user space to convert to CPU format.
+ */
+ csb.address = crb->stamp.nx.fault_storage_addr;
+ csb.flags = 0;
+
+ pid = window->pid;
+ tsk = get_pid_task(pid, PIDTYPE_PID);
+ /*
+ * Process closes send window after all pending NX requests are
+ * completed. In multi-thread applications, a child thread can
+ * open a window and can exit without closing it. May be some
+ * requests are pending or this window can be used by other
+ * threads later. We should handle faults if NX encounters
+ * pages faults on these requests. Update CSB with translation
+ * error and fault address. If csb_addr passed by user space is
+ * invalid, send SEGV signal to pid saved in window. If the
+ * child thread is not running, send the signal to tgid.
+ * Parent thread (tgid) will close this window upon its exit.
+ *
+ * pid and mm references are taken when window is opened by
+ * process (pid). So tgid is used only when child thread opens
+ * a window and exits without closing it.
+ */
+ if (!tsk) {
+ pid = window->tgid;
+ tsk = get_pid_task(pid, PIDTYPE_PID);
+ /*
+ * Parent thread (tgid) will be closing window when it
+ * exits. So should not get here.
+ */
+ if (WARN_ON_ONCE(!tsk))
+ return;
+ }
+
+ /* Return if the task is exiting. */
+ if (tsk->flags & PF_EXITING) {
+ put_task_struct(tsk);
+ return;
+ }
+
+ use_mm(window->mm);
+ rc = copy_to_user(csb_addr, &csb, sizeof(csb));
+ /*
+ * User space polls on csb.flags (first byte). So add barrier
+ * then copy first byte with csb flags update.
+ */
+ if (!rc) {
+ csb.flags = CSB_V;
+ /* Make sure update to csb.flags is visible now */
+ smp_mb();
+ rc = copy_to_user(csb_addr, &csb, sizeof(u8));
+ }
+ unuse_mm(window->mm);
+ put_task_struct(tsk);
+
+ /* Success */
+ if (!rc)
+ return;
+
+ pr_debug("Invalid CSB address 0x%p signalling pid(%d)\n",
+ csb_addr, pid_vnr(pid));
+
+ clear_siginfo(&info);
+ info.si_signo = SIGSEGV;
+ info.si_errno = EFAULT;
+ info.si_code = SEGV_MAPERR;
+ info.si_addr = csb_addr;
+
+ /*
+ * process will be polling on csb.flags after request is sent to
+ * NX. So generally CSB update should not fail except when an
+ * application passes invalid csb_addr. So an error message will
+ * be displayed and leave it to user space whether to ignore or
+ * handle this signal.
+ */
+ rcu_read_lock();
+ rc = kill_pid_info(SIGSEGV, &info, pid);
+ rcu_read_unlock();
+
+ pr_devel("%s(): pid %d kill_proc_info() rc %d\n", __func__,
+ pid_vnr(pid), rc);
+}
+
+/*
* Process valid CRBs in fault FIFO.
* NX process user space requests, return credit and update the status
* in CRB. If it encounters transalation error when accessing CRB or
@@ -124,8 +247,9 @@ irqreturn_t vas_fault_thread_fn(int irq, void *data)
vinst->fault_crbs);
WARN_ON_ONCE(1);
+ } else {
+ update_csb(window, crb);
}
-
}
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH v10 07/14] powerpc/vas: Setup thread IRQ handler per VAS instance
From: Haren Myneni @ 2020-04-02 7:13 UTC (permalink / raw)
To: mpe
Cc: mikey, ajd, frederic.barrat, linux-kernel, npiggin, hch, oohall,
clg, herbert, sukadev, linuxppc-dev, srikar
In-Reply-To: <1585810846.2275.23.camel@hbabu-laptop>
When NX encounters translation error on CRB and any request buffer,
raises an interrupt on the CPU to handle the fault. It can raise one
interrupt for multiple faults. Expects OS to handle these faults and
return credits for fault window after processing faults.
Setup thread IRQ handler and IRQ thread function per each VAS instance.
IRQ handler checks if the thread is already woken up and can handle new
faults. If so returns with IRQ_HANDLED, otherwise wake up thread to
process new faults.
The thread functions reads each CRB entry from fault FIFO until sees
invalid entry. After reading each CRB, determine the corresponding
send window using pswid (from CRB) and process fault CRB. Then
invalidate the entry and return credit. Processing fault CRB and
return credit is described in subsequent patches.
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
arch/powerpc/platforms/powernv/vas-fault.c | 131 ++++++++++++++++++++++++++++
arch/powerpc/platforms/powernv/vas-window.c | 60 +++++++++++++
arch/powerpc/platforms/powernv/vas.c | 23 ++++-
arch/powerpc/platforms/powernv/vas.h | 7 ++
4 files changed, 220 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/powernv/vas-fault.c b/arch/powerpc/platforms/powernv/vas-fault.c
index 4044998..0da8358 100644
--- a/arch/powerpc/platforms/powernv/vas-fault.c
+++ b/arch/powerpc/platforms/powernv/vas-fault.c
@@ -11,6 +11,7 @@
#include <linux/slab.h>
#include <linux/uaccess.h>
#include <linux/kthread.h>
+#include <linux/mmu_context.h>
#include <asm/icswx.h>
#include "vas.h"
@@ -25,6 +26,136 @@
#define VAS_FAULT_WIN_FIFO_SIZE (4 << 20)
/*
+ * Process valid CRBs in fault FIFO.
+ * NX process user space requests, return credit and update the status
+ * in CRB. If it encounters transalation error when accessing CRB or
+ * request buffers, raises interrupt on the CPU to handle the fault.
+ * It takes credit on fault window, updates nx_fault_stamp in CRB with
+ * the following information and pastes CRB in fault FIFO.
+ *
+ * pswid - window ID of the window on which the request is sent.
+ * fault_storage_addr - fault address
+ *
+ * It can raise a single interrupt for multiple faults. Expects OS to
+ * process all valid faults and return credit for each fault on user
+ * space and fault windows. This fault FIFO control will be done with
+ * credit mechanism. NX can continuously paste CRBs until credits are not
+ * available on fault window. Otherwise, returns with RMA_reject.
+ *
+ * Total credits available on fault window: FIFO_SIZE(4MB)/CRBS_SIZE(128)
+ *
+ */
+irqreturn_t vas_fault_thread_fn(int irq, void *data)
+{
+ struct vas_instance *vinst = data;
+ struct coprocessor_request_block *crb, *entry;
+ struct coprocessor_request_block buf;
+ struct vas_window *window;
+ unsigned long flags;
+ void *fifo;
+
+ crb = &buf;
+
+ /*
+ * VAS can interrupt with multiple page faults. So process all
+ * valid CRBs within fault FIFO until reaches invalid CRB.
+ * We use CCW[0] and pswid to validate validate CRBs:
+ *
+ * CCW[0] Reserved bit. When NX pastes CRB, CCW[0]=0
+ * OS sets this bit to 1 after reading CRB.
+ * pswid NX assigns window ID. Set pswid to -1 after
+ * reading CRB from fault FIFO.
+ *
+ * We exit this function if no valid CRBs are available to process.
+ * So acquire fault_lock and reset fifo_in_progress to 0 before
+ * exit.
+ * In case kernel receives another interrupt with different page
+ * fault, interrupt handler returns with IRQ_HANDLED if
+ * fifo_in_progress is set. Means these new faults will be
+ * handled by the current thread. Otherwise set fifo_in_progress
+ * and return IRQ_WAKE_THREAD to wake up thread.
+ */
+ while (true) {
+ spin_lock_irqsave(&vinst->fault_lock, flags);
+ /*
+ * Advance the fault fifo pointer to next CRB.
+ * Use CRB_SIZE rather than sizeof(*crb) since the latter is
+ * aligned to CRB_ALIGN (256) but the CRB written to by VAS is
+ * only CRB_SIZE in len.
+ */
+ fifo = vinst->fault_fifo + (vinst->fault_crbs * CRB_SIZE);
+ entry = fifo;
+
+ if ((entry->stamp.nx.pswid == cpu_to_be32(FIFO_INVALID_ENTRY))
+ || (entry->ccw & cpu_to_be32(CCW0_INVALID))) {
+ vinst->fifo_in_progress = 0;
+ spin_unlock_irqrestore(&vinst->fault_lock, flags);
+ return IRQ_HANDLED;
+ }
+
+ spin_unlock_irqrestore(&vinst->fault_lock, flags);
+ vinst->fault_crbs++;
+ if (vinst->fault_crbs == (vinst->fault_fifo_size / CRB_SIZE))
+ vinst->fault_crbs = 0;
+
+ memcpy(crb, fifo, CRB_SIZE);
+ entry->stamp.nx.pswid = cpu_to_be32(FIFO_INVALID_ENTRY);
+ entry->ccw |= cpu_to_be32(CCW0_INVALID);
+
+ pr_devel("VAS[%d] fault_fifo %p, fifo %p, fault_crbs %d\n",
+ vinst->vas_id, vinst->fault_fifo, fifo,
+ vinst->fault_crbs);
+
+ window = vas_pswid_to_window(vinst,
+ be32_to_cpu(crb->stamp.nx.pswid));
+
+ if (IS_ERR(window)) {
+ /*
+ * We got an interrupt about a specific send
+ * window but we can't find that window and we can't
+ * even clean it up (return credit on user space
+ * window).
+ * But we should not get here.
+ * TODO: Disable IRQ.
+ */
+ pr_err("VAS[%d] fault_fifo %p, fifo %p, pswid 0x%x, fault_crbs %d bad CRB?\n",
+ vinst->vas_id, vinst->fault_fifo, fifo,
+ be32_to_cpu(crb->stamp.nx.pswid),
+ vinst->fault_crbs);
+
+ WARN_ON_ONCE(1);
+ }
+
+ }
+}
+
+irqreturn_t vas_fault_handler(int irq, void *dev_id)
+{
+ struct vas_instance *vinst = dev_id;
+ irqreturn_t ret = IRQ_WAKE_THREAD;
+ unsigned long flags;
+
+ /*
+ * NX can generate an interrupt for multiple faults. So the
+ * fault handler thread process all CRBs until finds invalid
+ * entry. In case if NX sees continuous faults, it is possible
+ * that the thread function entered with the first interrupt
+ * can execute and process all valid CRBs.
+ * So wake up thread only if the fault thread is not in progress.
+ */
+ spin_lock_irqsave(&vinst->fault_lock, flags);
+
+ if (vinst->fifo_in_progress)
+ ret = IRQ_HANDLED;
+ else
+ vinst->fifo_in_progress = 1;
+
+ spin_unlock_irqrestore(&vinst->fault_lock, flags);
+
+ return ret;
+}
+
+/*
* Fault window is opened per VAS instance. NX pastes fault CRB in fault
* FIFO upon page faults.
*/
diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
index 063cda2..f12f7eb 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -1050,6 +1050,15 @@ struct vas_window *vas_tx_win_open(int vasid, enum vas_cop_type cop,
}
} else {
/*
+ * Interrupt hanlder or fault window setup failed. Means
+ * NX can not generate fault for page fault. So not
+ * opening for user space tx window.
+ */
+ if (!vinst->virq) {
+ rc = -ENODEV;
+ goto free_window;
+ }
+ /*
* A user mapping must ensure that context switch issues
* CP_ABORT for this thread.
*/
@@ -1307,3 +1316,54 @@ int vas_win_close(struct vas_window *window)
return 0;
}
EXPORT_SYMBOL_GPL(vas_win_close);
+
+struct vas_window *vas_pswid_to_window(struct vas_instance *vinst,
+ uint32_t pswid)
+{
+ struct vas_window *window;
+ int winid;
+
+ if (!pswid) {
+ pr_devel("%s: called for pswid 0!\n", __func__);
+ return ERR_PTR(-ESRCH);
+ }
+
+ decode_pswid(pswid, NULL, &winid);
+
+ if (winid >= VAS_WINDOWS_PER_CHIP)
+ return ERR_PTR(-ESRCH);
+
+ /*
+ * If application closes the window before the hardware
+ * returns the fault CRB, we should wait in vas_win_close()
+ * for the pending requests. so the window must be active
+ * and the process alive.
+ *
+ * If its a kernel process, we should not get any faults and
+ * should not get here.
+ */
+ window = vinst->windows[winid];
+
+ if (!window) {
+ pr_err("PSWID decode: Could not find window for winid %d pswid %d vinst 0x%p\n",
+ winid, pswid, vinst);
+ return NULL;
+ }
+
+ /*
+ * Do some sanity checks on the decoded window. Window should be
+ * NX GZIP user send window. FTW windows should not incur faults
+ * since their CRBs are ignored (not queued on FIFO or processed
+ * by NX).
+ */
+ if (!window->tx_win || !window->user_win || !window->nx_win ||
+ window->cop == VAS_COP_TYPE_FAULT ||
+ window->cop == VAS_COP_TYPE_FTW) {
+ pr_err("PSWID decode: id %d, tx %d, user %d, nx %d, cop %d\n",
+ winid, window->tx_win, window->user_win,
+ window->nx_win, window->cop);
+ WARN_ON(1);
+ }
+
+ return window;
+}
diff --git a/arch/powerpc/platforms/powernv/vas.c b/arch/powerpc/platforms/powernv/vas.c
index 9013a63..598e4cd 100644
--- a/arch/powerpc/platforms/powernv/vas.c
+++ b/arch/powerpc/platforms/powernv/vas.c
@@ -14,6 +14,8 @@
#include <linux/of_platform.h>
#include <linux/of_address.h>
#include <linux/of.h>
+#include <linux/irqdomain.h>
+#include <linux/interrupt.h>
#include <asm/prom.h>
#include <asm/xive.h>
@@ -26,7 +28,25 @@
static int vas_irq_fault_window_setup(struct vas_instance *vinst)
{
- return vas_setup_fault_window(vinst);
+ char devname[64];
+ int rc = 0;
+
+ snprintf(devname, sizeof(devname), "vas-%d", vinst->vas_id);
+ rc = request_threaded_irq(vinst->virq, vas_fault_handler,
+ vas_fault_thread_fn, 0, devname, vinst);
+
+ if (rc) {
+ pr_err("VAS[%d]: Request IRQ(%d) failed with %d\n",
+ vinst->vas_id, vinst->virq, rc);
+ goto out;
+ }
+
+ rc = vas_setup_fault_window(vinst);
+ if (rc)
+ free_irq(vinst->virq, vinst);
+
+out:
+ return rc;
}
static int init_vas_instance(struct platform_device *pdev)
@@ -119,6 +139,7 @@ static int init_vas_instance(struct platform_device *pdev)
list_add(&vinst->node, &vas_instances);
mutex_unlock(&vas_mutex);
+ spin_lock_init(&vinst->fault_lock);
/*
* IRQ and fault handling setup is needed only for user space
* send windows.
diff --git a/arch/powerpc/platforms/powernv/vas.h b/arch/powerpc/platforms/powernv/vas.h
index 2a04072..0af7912 100644
--- a/arch/powerpc/platforms/powernv/vas.h
+++ b/arch/powerpc/platforms/powernv/vas.h
@@ -331,7 +331,10 @@ struct vas_instance {
u64 irq_port;
int virq;
+ int fault_crbs;
int fault_fifo_size;
+ int fifo_in_progress;
+ spinlock_t fault_lock;
void *fault_fifo;
struct vas_window *fault_win; /* Fault window */
@@ -431,6 +434,10 @@ struct vas_winctx {
extern void vas_window_init_dbgdir(struct vas_window *win);
extern void vas_window_free_dbgdir(struct vas_window *win);
extern int vas_setup_fault_window(struct vas_instance *vinst);
+extern irqreturn_t vas_fault_thread_fn(int irq, void *data);
+extern irqreturn_t vas_fault_handler(int irq, void *dev_id);
+extern struct vas_window *vas_pswid_to_window(struct vas_instance *vinst,
+ uint32_t pswid);
static inline int vas_window_pid(struct vas_window *window)
{
--
1.8.3.1
^ permalink raw reply related
* [PATCH v10 06/14] powerpc/vas: Take reference to PID and mm for user space windows
From: Haren Myneni @ 2020-04-02 7:13 UTC (permalink / raw)
To: mpe
Cc: mikey, ajd, frederic.barrat, linux-kernel, npiggin, hch, oohall,
clg, herbert, sukadev, linuxppc-dev, srikar
In-Reply-To: <1585810846.2275.23.camel@hbabu-laptop>
When process opens a window, its pid and tgid will be saved in the
vas_window struct. This window will be closed when the process exits.
The kernel handles NX faults by updating CSB or send SEGV signal to pid
of the process if the user space csb addr is invalid.
In multi-thread applications, a window can be opened by a child thread,
but it will not be closed when this thread exits. It is expected that
the parent will clean up all resources including NX windows opened by
child threads. A child thread can send NX requests using this window
and could be killed before completion is reported. If the pid assigned
to this thread is reused while requests are pending, a failure SEGV
would be directed to the wrong place.
To prevent reusing the pid, take references to pid and mm when the window
is opened and release them when when the window is closed. Then if child
thread is not running, SEGV signal will be sent to thread group leader
(tgid).
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
arch/powerpc/platforms/powernv/vas-debug.c | 2 +-
arch/powerpc/platforms/powernv/vas-window.c | 50 ++++++++++++++++++++++++++---
arch/powerpc/platforms/powernv/vas.h | 9 +++++-
3 files changed, 55 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/vas-debug.c b/arch/powerpc/platforms/powernv/vas-debug.c
index 09e63df..ef9a717 100644
--- a/arch/powerpc/platforms/powernv/vas-debug.c
+++ b/arch/powerpc/platforms/powernv/vas-debug.c
@@ -38,7 +38,7 @@ static int info_show(struct seq_file *s, void *private)
seq_printf(s, "Type: %s, %s\n", cop_to_str(window->cop),
window->tx_win ? "Send" : "Receive");
- seq_printf(s, "Pid : %d\n", window->pid);
+ seq_printf(s, "Pid : %d\n", vas_window_pid(window));
unlock:
mutex_unlock(&vas_mutex);
diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
index dc46bf6..063cda2 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -12,6 +12,8 @@
#include <linux/log2.h>
#include <linux/rcupdate.h>
#include <linux/cred.h>
+#include <linux/sched/mm.h>
+#include <linux/mmu_context.h>
#include <asm/switch_to.h>
#include <asm/ppc-opcode.h>
#include "vas.h"
@@ -876,8 +878,6 @@ struct vas_window *vas_rx_win_open(int vasid, enum vas_cop_type cop,
rxwin->user_win = rxattr->user_win;
rxwin->cop = cop;
rxwin->wcreds_max = rxattr->wcreds_max ?: VAS_WCREDS_DEFAULT;
- if (rxattr->user_win)
- rxwin->pid = task_pid_vnr(current);
init_winctx_for_rxwin(rxwin, rxattr, &winctx);
init_winctx_regs(rxwin, &winctx);
@@ -1027,7 +1027,6 @@ struct vas_window *vas_tx_win_open(int vasid, enum vas_cop_type cop,
txwin->tx_win = 1;
txwin->rxwin = rxwin;
txwin->nx_win = txwin->rxwin->nx_win;
- txwin->pid = attr->pid;
txwin->user_win = attr->user_win;
txwin->wcreds_max = attr->wcreds_max ?: VAS_WCREDS_DEFAULT;
@@ -1057,6 +1056,40 @@ struct vas_window *vas_tx_win_open(int vasid, enum vas_cop_type cop,
rc = set_thread_uses_vas();
if (rc)
goto free_window;
+
+ /*
+ * Window opened by a child thread may not be closed when
+ * it exits. So take reference to its pid and release it
+ * when the window is free by parent thread.
+ * Acquire a reference to the task's pid to make sure
+ * pid will not be re-used - needed only for multithread
+ * applications.
+ */
+ txwin->pid = get_task_pid(current, PIDTYPE_PID);
+ /*
+ * Acquire a reference to the task's mm.
+ */
+ txwin->mm = get_task_mm(current);
+
+ if (!txwin->mm) {
+ put_pid(txwin->pid);
+ pr_err("VAS: pid(%d): mm_struct is not found\n",
+ current->pid);
+ rc = -EPERM;
+ goto free_window;
+ }
+
+ mmgrab(txwin->mm);
+ mmput(txwin->mm);
+ mm_context_add_copro(txwin->mm);
+ /*
+ * Process closes window during exit. In the case of
+ * multithread application, the child thread can open
+ * window and can exit without closing it. Expects parent
+ * thread to use and close the window. So do not need
+ * to take pid reference for parent thread.
+ */
+ txwin->tgid = find_get_pid(task_tgid_vnr(current));
}
set_vinst_win(vinst, txwin);
@@ -1257,8 +1290,17 @@ int vas_win_close(struct vas_window *window)
poll_window_castout(window);
/* if send window, drop reference to matching receive window */
- if (window->tx_win)
+ if (window->tx_win) {
+ if (window->user_win) {
+ /* Drop references to pid and mm */
+ put_pid(window->pid);
+ if (window->mm) {
+ mm_context_remove_copro(window->mm);
+ mmdrop(window->mm);
+ }
+ }
put_rx_win(window->rxwin);
+ }
vas_window_free(window);
diff --git a/arch/powerpc/platforms/powernv/vas.h b/arch/powerpc/platforms/powernv/vas.h
index 88d084d..2a04072 100644
--- a/arch/powerpc/platforms/powernv/vas.h
+++ b/arch/powerpc/platforms/powernv/vas.h
@@ -355,7 +355,9 @@ struct vas_window {
bool user_win; /* True if user space window */
void *hvwc_map; /* HV window context */
void *uwc_map; /* OS/User window context */
- pid_t pid; /* Linux process id of owner */
+ struct pid *pid; /* Linux process id of owner */
+ struct pid *tgid; /* Thread group ID of owner */
+ struct mm_struct *mm; /* Linux process mm_struct */
int wcreds_max; /* Window credits */
char *dbgname;
@@ -430,6 +432,11 @@ struct vas_winctx {
extern void vas_window_free_dbgdir(struct vas_window *win);
extern int vas_setup_fault_window(struct vas_instance *vinst);
+static inline int vas_window_pid(struct vas_window *window)
+{
+ return pid_vnr(window->pid);
+}
+
static inline void vas_log_write(struct vas_window *win, char *name,
void *regptr, u64 val)
{
--
1.8.3.1
^ permalink raw reply related
* [PATCH v10 05/14] powerpc/vas: Register NX with fault window ID and IRQ port value
From: Haren Myneni @ 2020-04-02 7:11 UTC (permalink / raw)
To: mpe
Cc: mikey, ajd, frederic.barrat, linux-kernel, npiggin, hch, oohall,
clg, herbert, sukadev, linuxppc-dev, srikar
In-Reply-To: <1585810846.2275.23.camel@hbabu-laptop>
For each user space send window, register NX with fault window ID
and port value so that NX paste CRBs in this fault FIFO when it
sees fault on the request buffer.
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
arch/powerpc/platforms/powernv/vas-window.c | 15 +++++++++++++--
arch/powerpc/platforms/powernv/vas.h | 15 +++++++++++++++
2 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
index 1783fa9..dc46bf6 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -373,7 +373,7 @@ int init_winctx_regs(struct vas_window *window, struct vas_winctx *winctx)
init_xlate_regs(window, winctx->user_win);
val = 0ULL;
- val = SET_FIELD(VAS_FAULT_TX_WIN, val, 0);
+ val = SET_FIELD(VAS_FAULT_TX_WIN, val, winctx->fault_win_id);
write_hvwc_reg(window, VREG(FAULT_TX_WIN), val);
/* In PowerNV, interrupts go to HV. */
@@ -748,6 +748,8 @@ static void init_winctx_for_rxwin(struct vas_window *rxwin,
winctx->min_scope = VAS_SCOPE_LOCAL;
winctx->max_scope = VAS_SCOPE_VECTORED_GROUP;
+ if (rxwin->vinst->virq)
+ winctx->irq_port = rxwin->vinst->irq_port;
}
static bool rx_win_args_valid(enum vas_cop_type cop,
@@ -944,13 +946,22 @@ static void init_winctx_for_txwin(struct vas_window *txwin,
winctx->lpid = txattr->lpid;
winctx->pidr = txattr->pidr;
winctx->rx_win_id = txwin->rxwin->winid;
+ /*
+ * IRQ and fault window setup is successful. Set fault window
+ * for the send window so that ready to handle faults.
+ */
+ if (txwin->vinst->virq)
+ winctx->fault_win_id = txwin->vinst->fault_win->winid;
winctx->dma_type = VAS_DMA_TYPE_INJECT;
winctx->tc_mode = txattr->tc_mode;
winctx->min_scope = VAS_SCOPE_LOCAL;
winctx->max_scope = VAS_SCOPE_VECTORED_GROUP;
+ if (txwin->vinst->virq)
+ winctx->irq_port = txwin->vinst->irq_port;
- winctx->pswid = 0;
+ winctx->pswid = txattr->pswid ? txattr->pswid :
+ encode_pswid(txwin->vinst->vas_id, txwin->winid);
}
static bool tx_win_args_valid(enum vas_cop_type cop,
diff --git a/arch/powerpc/platforms/powernv/vas.h b/arch/powerpc/platforms/powernv/vas.h
index 9c8e3f5..88d084d 100644
--- a/arch/powerpc/platforms/powernv/vas.h
+++ b/arch/powerpc/platforms/powernv/vas.h
@@ -467,6 +467,21 @@ static inline u64 read_hvwc_reg(struct vas_window *win,
return in_be64(win->hvwc_map+reg);
}
+/*
+ * Encode/decode the Partition Send Window ID (PSWID) for a window in
+ * a way that we can uniquely identify any window in the system. i.e.
+ * we should be able to locate the 'struct vas_window' given the PSWID.
+ *
+ * Bits Usage
+ * 0:7 VAS id (8 bits)
+ * 8:15 Unused, 0 (3 bits)
+ * 16:31 Window id (16 bits)
+ */
+static inline u32 encode_pswid(int vasid, int winid)
+{
+ return ((u32)winid | (vasid << (31 - 7)));
+}
+
static inline void decode_pswid(u32 pswid, int *vasid, int *winid)
{
if (vasid)
--
1.8.3.1
^ permalink raw reply related
* [PATCH v10 04/14] powerpc/vas: Setup fault window per VAS instance
From: Haren Myneni @ 2020-04-02 7:11 UTC (permalink / raw)
To: mpe
Cc: mikey, ajd, frederic.barrat, linux-kernel, npiggin, hch, oohall,
clg, herbert, sukadev, linuxppc-dev, srikar
In-Reply-To: <1585810846.2275.23.camel@hbabu-laptop>
Setup fault window for each VAS instance. When NX gets a fault on
request buffer, pastes fault CRB in the corresponding fault FIFO and
then raises an interrupt to the OS. The kernel handles this fault
and process faults CRB from this FIFO.
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
arch/powerpc/platforms/powernv/Makefile | 2 +-
arch/powerpc/platforms/powernv/vas-fault.c | 77 +++++++++++++++++++++++++++++
arch/powerpc/platforms/powernv/vas-window.c | 4 +-
arch/powerpc/platforms/powernv/vas.c | 20 ++++++++
arch/powerpc/platforms/powernv/vas.h | 21 ++++++++
5 files changed, 121 insertions(+), 3 deletions(-)
create mode 100644 arch/powerpc/platforms/powernv/vas-fault.c
diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
index c0f8120..395789f 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -17,7 +17,7 @@ obj-$(CONFIG_MEMORY_FAILURE) += opal-memory-errors.o
obj-$(CONFIG_OPAL_PRD) += opal-prd.o
obj-$(CONFIG_PERF_EVENTS) += opal-imc.o
obj-$(CONFIG_PPC_MEMTRACE) += memtrace.o
-obj-$(CONFIG_PPC_VAS) += vas.o vas-window.o vas-debug.o
+obj-$(CONFIG_PPC_VAS) += vas.o vas-window.o vas-debug.o vas-fault.o
obj-$(CONFIG_OCXL_BASE) += ocxl.o
obj-$(CONFIG_SCOM_DEBUGFS) += opal-xscom.o
obj-$(CONFIG_PPC_SECURE_BOOT) += opal-secvar.o
diff --git a/arch/powerpc/platforms/powernv/vas-fault.c b/arch/powerpc/platforms/powernv/vas-fault.c
new file mode 100644
index 0000000..4044998
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/vas-fault.c
@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * VAS Fault handling.
+ * Copyright 2019, IBM Corporation
+ */
+
+#define pr_fmt(fmt) "vas: " fmt
+
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/kthread.h>
+#include <asm/icswx.h>
+
+#include "vas.h"
+
+/*
+ * The maximum FIFO size for fault window can be 8MB
+ * (VAS_RX_FIFO_SIZE_MAX). Using 4MB FIFO since each VAS
+ * instance will be having fault window.
+ * 8MB FIFO can be used if expects more faults for each VAS
+ * instance.
+ */
+#define VAS_FAULT_WIN_FIFO_SIZE (4 << 20)
+
+/*
+ * Fault window is opened per VAS instance. NX pastes fault CRB in fault
+ * FIFO upon page faults.
+ */
+int vas_setup_fault_window(struct vas_instance *vinst)
+{
+ struct vas_rx_win_attr attr;
+
+ vinst->fault_fifo_size = VAS_FAULT_WIN_FIFO_SIZE;
+ vinst->fault_fifo = kzalloc(vinst->fault_fifo_size, GFP_KERNEL);
+ if (!vinst->fault_fifo) {
+ pr_err("Unable to alloc %d bytes for fault_fifo\n",
+ vinst->fault_fifo_size);
+ return -ENOMEM;
+ }
+
+ /*
+ * Invalidate all CRB entries. NX pastes valid entry for each fault.
+ */
+ memset(vinst->fault_fifo, FIFO_INVALID_ENTRY, vinst->fault_fifo_size);
+ vas_init_rx_win_attr(&attr, VAS_COP_TYPE_FAULT);
+
+ attr.rx_fifo_size = vinst->fault_fifo_size;
+ attr.rx_fifo = vinst->fault_fifo;
+
+ /*
+ * Max creds is based on number of CRBs can fit in the FIFO.
+ * (fault_fifo_size/CRB_SIZE). If 8MB FIFO is used, max creds
+ * will be 0xffff since the receive creds field is 16bits wide.
+ */
+ attr.wcreds_max = vinst->fault_fifo_size / CRB_SIZE;
+ attr.lnotify_lpid = 0;
+ attr.lnotify_pid = mfspr(SPRN_PID);
+ attr.lnotify_tid = mfspr(SPRN_PID);
+
+ vinst->fault_win = vas_rx_win_open(vinst->vas_id, VAS_COP_TYPE_FAULT,
+ &attr);
+
+ if (IS_ERR(vinst->fault_win)) {
+ pr_err("VAS: Error %ld opening FaultWin\n",
+ PTR_ERR(vinst->fault_win));
+ kfree(vinst->fault_fifo);
+ return PTR_ERR(vinst->fault_win);
+ }
+
+ pr_devel("VAS: Created FaultWin %d, LPID/PID/TID [%d/%d/%d]\n",
+ vinst->fault_win->winid, attr.lnotify_lpid,
+ attr.lnotify_pid, attr.lnotify_tid);
+
+ return 0;
+}
diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
index 0c0d27d..1783fa9 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -827,9 +827,9 @@ void vas_init_rx_win_attr(struct vas_rx_win_attr *rxattr, enum vas_cop_type cop)
rxattr->fault_win = true;
rxattr->notify_disable = true;
rxattr->rx_wcred_mode = true;
- rxattr->tx_wcred_mode = true;
rxattr->rx_win_ord_mode = true;
- rxattr->tx_win_ord_mode = true;
+ rxattr->rej_no_credit = true;
+ rxattr->tc_mode = VAS_THRESH_DISABLED;
} else if (cop == VAS_COP_TYPE_FTW) {
rxattr->user_win = true;
rxattr->intr_disable = true;
diff --git a/arch/powerpc/platforms/powernv/vas.c b/arch/powerpc/platforms/powernv/vas.c
index 3303cfe..9013a63 100644
--- a/arch/powerpc/platforms/powernv/vas.c
+++ b/arch/powerpc/platforms/powernv/vas.c
@@ -24,6 +24,11 @@
static DEFINE_PER_CPU(int, cpu_vas_id);
+static int vas_irq_fault_window_setup(struct vas_instance *vinst)
+{
+ return vas_setup_fault_window(vinst);
+}
+
static int init_vas_instance(struct platform_device *pdev)
{
struct device_node *dn = pdev->dev.of_node;
@@ -114,6 +119,21 @@ static int init_vas_instance(struct platform_device *pdev)
list_add(&vinst->node, &vas_instances);
mutex_unlock(&vas_mutex);
+ /*
+ * IRQ and fault handling setup is needed only for user space
+ * send windows.
+ */
+ if (vinst->virq) {
+ rc = vas_irq_fault_window_setup(vinst);
+ /*
+ * Fault window is used only for user space send windows.
+ * So if vinst->virq is NULL, tx_win_open returns -ENODEV
+ * for user space.
+ */
+ if (rc)
+ vinst->virq = 0;
+ }
+
vas_instance_init_dbgdir(vinst);
dev_set_drvdata(&pdev->dev, vinst);
diff --git a/arch/powerpc/platforms/powernv/vas.h b/arch/powerpc/platforms/powernv/vas.h
index 598608b..9c8e3f5 100644
--- a/arch/powerpc/platforms/powernv/vas.h
+++ b/arch/powerpc/platforms/powernv/vas.h
@@ -296,6 +296,22 @@ enum vas_notify_after_count {
};
/*
+ * NX can generate an interrupt for multiple faults and expects kernel
+ * to process all of them. So read all valid CRB entries until find the
+ * invalid one. So use pswid which is pasted by NX and ccw[0] (reserved
+ * bit in BE) to check valid CRB. CCW[0] will not be touched by user
+ * space. Application gets CRB formt error if it updates this bit.
+ *
+ * Invalidate FIFO during allocation and process all entries from last
+ * successful read until finds invalid pswid and ccw[0] values.
+ * After reading each CRB entry from fault FIFO, the kernel invalidate
+ * it by updating pswid with FIFO_INVALID_ENTRY and CCW[0] with
+ * CCW0_INVALID.
+ */
+#define FIFO_INVALID_ENTRY 0xffffffff
+#define CCW0_INVALID 1
+
+/*
* One per instance of VAS. Each instance will have a separate set of
* receive windows, one per coprocessor type.
*
@@ -315,6 +331,10 @@ struct vas_instance {
u64 irq_port;
int virq;
+ int fault_fifo_size;
+ void *fault_fifo;
+ struct vas_window *fault_win; /* Fault window */
+
struct mutex mutex;
struct vas_window *rxwin[VAS_COP_TYPE_MAX];
struct vas_window *windows[VAS_WINDOWS_PER_CHIP];
@@ -408,6 +428,7 @@ struct vas_winctx {
extern void vas_instance_init_dbgdir(struct vas_instance *vinst);
extern void vas_window_init_dbgdir(struct vas_window *win);
extern void vas_window_free_dbgdir(struct vas_window *win);
+extern int vas_setup_fault_window(struct vas_instance *vinst);
static inline void vas_log_write(struct vas_window *win, char *name,
void *regptr, u64 val)
--
1.8.3.1
^ permalink raw reply related
* [PATCH v10 03/14] powerpc/vas: Alloc and setup IRQ and trigger port address
From: Haren Myneni @ 2020-04-02 7:10 UTC (permalink / raw)
To: mpe
Cc: mikey, ajd, frederic.barrat, linux-kernel, npiggin, hch, oohall,
clg, herbert, sukadev, linuxppc-dev, srikar
In-Reply-To: <1585810846.2275.23.camel@hbabu-laptop>
Allocate a xive irq on each chip with a vas instance. The NX coprocessor
raises a host CPU interrupt via vas if it encounters page fault on user
space request buffer. Subsequent patches register the trigger port with
the NX coprocessor, and create a vas fault handler for this interrupt
mapping.
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
arch/powerpc/platforms/powernv/vas.c | 44 +++++++++++++++++++++++++++++++-----
arch/powerpc/platforms/powernv/vas.h | 2 ++
2 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/vas.c b/arch/powerpc/platforms/powernv/vas.c
index ed9cc6d..3303cfe 100644
--- a/arch/powerpc/platforms/powernv/vas.c
+++ b/arch/powerpc/platforms/powernv/vas.c
@@ -15,6 +15,7 @@
#include <linux/of_address.h>
#include <linux/of.h>
#include <asm/prom.h>
+#include <asm/xive.h>
#include "vas.h"
@@ -25,10 +26,12 @@
static int init_vas_instance(struct platform_device *pdev)
{
- int rc, cpu, vasid;
- struct resource *res;
- struct vas_instance *vinst;
struct device_node *dn = pdev->dev.of_node;
+ struct vas_instance *vinst;
+ struct xive_irq_data *xd;
+ uint32_t chipid, hwirq;
+ struct resource *res;
+ int rc, cpu, vasid;
rc = of_property_read_u32(dn, "ibm,vas-id", &vasid);
if (rc) {
@@ -36,6 +39,12 @@ static int init_vas_instance(struct platform_device *pdev)
return -ENODEV;
}
+ rc = of_property_read_u32(dn, "ibm,chip-id", &chipid);
+ if (rc) {
+ pr_err("No ibm,chip-id property for %s?\n", pdev->name);
+ return -ENODEV;
+ }
+
if (pdev->num_resources != 4) {
pr_err("Unexpected DT configuration for [%s, %d]\n",
pdev->name, vasid);
@@ -69,9 +78,32 @@ static int init_vas_instance(struct platform_device *pdev)
vinst->paste_win_id_shift = 63 - res->end;
- pr_devel("Initialized instance [%s, %d], paste_base 0x%llx, "
- "paste_win_id_shift 0x%llx\n", pdev->name, vasid,
- vinst->paste_base_addr, vinst->paste_win_id_shift);
+ hwirq = xive_native_alloc_irq_on_chip(chipid);
+ if (!hwirq) {
+ pr_err("Inst%d: Unable to allocate global irq for chip %d\n",
+ vinst->vas_id, chipid);
+ return -ENOENT;
+ }
+
+ vinst->virq = irq_create_mapping(NULL, hwirq);
+ if (!vinst->virq) {
+ pr_err("Inst%d: Unable to map global irq %d\n",
+ vinst->vas_id, hwirq);
+ return -EINVAL;
+ }
+
+ xd = irq_get_handler_data(vinst->virq);
+ if (!xd) {
+ pr_err("Inst%d: Invalid virq %d\n",
+ vinst->vas_id, vinst->virq);
+ return -EINVAL;
+ }
+
+ vinst->irq_port = xd->trig_page;
+ pr_devel("Initialized instance [%s, %d] paste_base 0x%llx paste_win_id_shift 0x%llx IRQ %d Port 0x%llx\n",
+ pdev->name, vasid, vinst->paste_base_addr,
+ vinst->paste_win_id_shift, vinst->virq,
+ vinst->irq_port);
for_each_possible_cpu(cpu) {
if (cpu_to_chip_id(cpu) == of_get_ibm_chip_id(dn))
diff --git a/arch/powerpc/platforms/powernv/vas.h b/arch/powerpc/platforms/powernv/vas.h
index 5574aec..598608b 100644
--- a/arch/powerpc/platforms/powernv/vas.h
+++ b/arch/powerpc/platforms/powernv/vas.h
@@ -313,6 +313,8 @@ struct vas_instance {
u64 paste_base_addr;
u64 paste_win_id_shift;
+ u64 irq_port;
+ int virq;
struct mutex mutex;
struct vas_window *rxwin[VAS_COP_TYPE_MAX];
struct vas_window *windows[VAS_WINDOWS_PER_CHIP];
--
1.8.3.1
^ permalink raw reply related
* [PATCH v10 02/14] powerpc/vas: Define nx_fault_stamp in coprocessor_request_block
From: Haren Myneni @ 2020-04-02 7:09 UTC (permalink / raw)
To: mpe
Cc: mikey, ajd, frederic.barrat, linux-kernel, npiggin, hch, oohall,
clg, herbert, sukadev, linuxppc-dev, srikar
In-Reply-To: <1585810846.2275.23.camel@hbabu-laptop>
Kernel sets fault address and status in CRB for NX page fault on user
space address after processing page fault. User space gets the signal
and handles the fault mentioned in CRB by bringing the page in to
memory and send NX request again.
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
arch/powerpc/include/asm/icswx.h | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/icswx.h b/arch/powerpc/include/asm/icswx.h
index 9872f85..965b1f3 100644
--- a/arch/powerpc/include/asm/icswx.h
+++ b/arch/powerpc/include/asm/icswx.h
@@ -108,6 +108,17 @@ struct data_descriptor_entry {
__be64 address;
} __packed __aligned(DDE_ALIGN);
+/* 4.3.2 NX-stamped Fault CRB */
+
+#define NX_STAMP_ALIGN (0x10)
+
+struct nx_fault_stamp {
+ __be64 fault_storage_addr;
+ __be16 reserved;
+ __u8 flags;
+ __u8 fault_status;
+ __be32 pswid;
+} __packed __aligned(NX_STAMP_ALIGN);
/* Chapter 6.5.2 Coprocessor-Request Block (CRB) */
@@ -135,10 +146,15 @@ struct coprocessor_request_block {
struct coprocessor_completion_block ccb;
- u8 reserved[48];
+ union {
+ struct nx_fault_stamp nx;
+ u8 reserved[16];
+ } stamp;
+
+ u8 reserved[32];
struct coprocessor_status_block csb;
-} __packed __aligned(CRB_ALIGN);
+} __packed;
/* RFC02167 Initiate Coprocessor Instructions document
--
1.8.3.1
^ permalink raw reply related
* [PATCH v10 01/14] powerpc/xive: Define xive_native_alloc_irq_on_chip()
From: Haren Myneni @ 2020-04-02 7:09 UTC (permalink / raw)
To: mpe
Cc: mikey, ajd, frederic.barrat, linux-kernel, npiggin, hch, oohall,
clg, herbert, sukadev, linuxppc-dev, srikar
In-Reply-To: <1585810846.2275.23.camel@hbabu-laptop>
This function allocates IRQ on a specific chip. VAS needs per chip
IRQ allocation and will have IRQ handler per VAS instance.
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/include/asm/xive.h | 9 ++++++++-
arch/powerpc/sysdev/xive/native.c | 6 +++---
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h
index 93f982db..d08ea11 100644
--- a/arch/powerpc/include/asm/xive.h
+++ b/arch/powerpc/include/asm/xive.h
@@ -5,6 +5,8 @@
#ifndef _ASM_POWERPC_XIVE_H
#define _ASM_POWERPC_XIVE_H
+#include <asm/opal-api.h>
+
#define XIVE_INVALID_VP 0xffffffff
#ifdef CONFIG_PPC_XIVE
@@ -108,7 +110,6 @@ struct xive_q {
int xive_native_populate_irq_data(u32 hw_irq,
struct xive_irq_data *data);
void xive_cleanup_irq_data(struct xive_irq_data *xd);
-u32 xive_native_alloc_irq(void);
void xive_native_free_irq(u32 irq);
int xive_native_configure_irq(u32 hw_irq, u32 target, u8 prio, u32 sw_irq);
@@ -137,6 +138,12 @@ int xive_native_set_queue_state(u32 vp_id, uint32_t prio, u32 qtoggle,
u32 qindex);
int xive_native_get_vp_state(u32 vp_id, u64 *out_state);
bool xive_native_has_queue_state_support(void);
+extern u32 xive_native_alloc_irq_on_chip(u32 chip_id);
+
+static inline u32 xive_native_alloc_irq(void)
+{
+ return xive_native_alloc_irq_on_chip(OPAL_XIVE_ANY_CHIP);
+}
#else
diff --git a/arch/powerpc/sysdev/xive/native.c b/arch/powerpc/sysdev/xive/native.c
index 0ff6b73..14d4406 100644
--- a/arch/powerpc/sysdev/xive/native.c
+++ b/arch/powerpc/sysdev/xive/native.c
@@ -279,12 +279,12 @@ static int xive_native_get_ipi(unsigned int cpu, struct xive_cpu *xc)
}
#endif /* CONFIG_SMP */
-u32 xive_native_alloc_irq(void)
+u32 xive_native_alloc_irq_on_chip(u32 chip_id)
{
s64 rc;
for (;;) {
- rc = opal_xive_allocate_irq(OPAL_XIVE_ANY_CHIP);
+ rc = opal_xive_allocate_irq(chip_id);
if (rc != OPAL_BUSY)
break;
msleep(OPAL_BUSY_DELAY_MS);
@@ -293,7 +293,7 @@ u32 xive_native_alloc_irq(void)
return 0;
return rc;
}
-EXPORT_SYMBOL_GPL(xive_native_alloc_irq);
+EXPORT_SYMBOL_GPL(xive_native_alloc_irq_on_chip);
void xive_native_free_irq(u32 irq)
{
--
1.8.3.1
^ permalink raw reply related
* Re: [RFC PATCH 3/4] powerpc ppc-opcode: move ppc instuction encoding from test_emulate_step
From: Naveen N. Rao @ 2020-04-02 7:04 UTC (permalink / raw)
To: Balamuruhan S, Michael Ellerman
Cc: jniethe5, linuxppc-dev, sandipan, paulus, ravi.bangoria
In-Reply-To: <87ftdmtsy9.fsf@mpe.ellerman.id.au>
Michael Ellerman wrote:
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
>> Balamuruhan S wrote:
>>> Few ppc instructions are encoded in test_emulate_step.c, consolidate them to
>>> ppc-opcode.h, fix redefintion errors in bpf_jit caused due to this consolidation.
>>> Reuse the macros from ppc-opcode.h
> ...
>>> diff --git a/arch/powerpc/net/bpf_jit32.h b/arch/powerpc/net/bpf_jit32.h
>>> index 4ec2a9f14f84..8a9f16a7262e 100644
>>> --- a/arch/powerpc/net/bpf_jit32.h
>>> +++ b/arch/powerpc/net/bpf_jit32.h
>>> @@ -76,13 +76,13 @@ DECLARE_LOAD_FUNC(sk_load_byte_msh);
>>> else { PPC_ADDIS(r, base, IMM_HA(i)); \
>>> PPC_LBZ(r, r, IMM_L(i)); } } while(0)
>>>
>>> -#define PPC_LD_OFFS(r, base, i) do { if ((i) < 32768) PPC_LD(r, base, i); \
>>> +#define _OFFS(r, base, i) do { if ((i) < 32768) EMIT(PPC_ENCODE_LD(r, base, i)); \
>> ^^^^^
>> Should be PPC_LD_OFFS. For the next version, please also build ppc32 and
>> booke codebase to confirm that your changes in those areas are fine.
>>
>> PPC_ENCODE_* also looks quite verbose, so perhaps PPC_ENC_* might be
>> better. Otherwise, this patchset looks good to me and should help reuse
>> some of those macros, especially from the eBPF codebase.
>>
>> Michael,
>> Can you let us know if this looks ok to you? Based on your feedback, we
>> will also update the eBPF codebase.
>
> I didn't really like the first patch which does the mass renaming. It
> creates a huge amount of churn.
>
> I think I'd be happier if this series just did what it needs, and then
> maybe at the end there's a patch to update all the existing names, which
> I may or may not take.
Ok.
>
> As far as the naming, currently we have:
>
> PPC_INST_FOO - just the opcode
>
> PPC_FOO(x) - macro to encode the opcode with x and (usually) also emit a
> .long and stringify.
>
> And you need an in-between that gives you the full instruction but
> without the .long and stringify, right?
Yes.
>
> So how about PPC_RAW_FOO() for just the numeric value, without the .long
> and stringify.
Sure, thanks for the feedback -- that makes sense.
>
> We also seem to have a lot of PPC_INST_FOO's that are only ever used in
> the PPC_INST macro. I'm inclined to fold those into the PPC_INST macro,
> to avoid people accidentally using the PPC_INST version when they don't
> mean to. But that's a separate issue.
Good point -- I do see many uses of PPC_INST_FOO that can be replaced
with PPC_RAW_FOO once we introduce that. We will take a stab at doing
this cleanup as a separate patch at the end.
Thanks,
Naveen
^ permalink raw reply
* [PATCH v10 00/14] powerpc/vas: Page fault handling for user space NX requests
From: Haren Myneni @ 2020-04-02 7:00 UTC (permalink / raw)
To: mpe
Cc: mikey, srikar, frederic.barrat, ajd, linux-kernel, npiggin, hch,
oohall, clg, sukadev, linuxppc-dev, herbert
On power9, Virtual Accelerator Switchboard (VAS) allows user space or
kernel to communicate with Nest Accelerator (NX) directly using COPY/PASTE
instructions. NX provides various functionalities such as compression,
encryption and etc. But only compression (842 and GZIP formats) is
supported in Linux kernel on power9.
842 compression driver (drivers/crypto/nx/nx-842-powernv.c)
is already included in Linux. Only GZIP support will be available from
user space.
Applications can issue GZIP compression / decompression requests to NX with
COPY/PASTE instructions. When NX is processing these requests, can hit
fault on the request buffer (not in memory). It issues an interrupt and
pastes fault CRB in fault FIFO. Expects kernel to handle this fault and
return credits for both send and fault windows after processing.
This patch series adds IRQ and fault window setup, and NX fault handling:
- Alloc IRQ and trigger port address, and configure IRQ per VAS instance.
- Set port# for each window to generate an interrupt when noticed fault.
- Set fault window and FIFO on which NX paste fault CRB.
- Setup IRQ thread fault handler per VAS instance.
- When receiving an interrupt, Read CRBs from fault FIFO and update
coprocessor_status_block (CSB) in the corresponding CRB with translation
failure (CSB_CC_TRANSLATION). After issuing NX requests, process polls
on CSB address. When it sees translation error, can touch the request
buffer to bring the page in to memory and reissue NX request.
- If copy_to_user fails on user space CSB address, OS sends SEGV signal.
Tested these patches with NX-GZIP enable patches and posted them as separate
patch series.
Patch 1: Define alloc IRQ per chip which is needed to alloc IRQ per VAS
instance.
Patch 2: Define nx_fault_stamp on which NX writes fault status for the fault
CRB
Patch 3: Alloc and setup IRQ and trigger port address for each VAS instance
Patches 4 & 5: Setup fault window and register NX per each VAS instance. This
window is used for NX to paste fault CRB in FIFO.
Patch 6: Reference to pid and mm so that pid is not used until window closed.
Needed for multi thread application where child can open a window
and can be used by parent it later.
Patch 7: Setup threaded IRQ handler per VAS
Patch 8: Process CRBs from fault FIFO and notify tasks by updating CSB or
through signals.
Patches 9 & 11: Return credits for send and fault windows after handling
faults.
Patches 10 & 12: Dump FIFO / CRB data and messages for error conditions
Patch 13: Fix closing send window after all credits are returned. This issue
happens only for user space requests. No page faults on kernel
request buffer.
Patch 14: For each process / thread, use mm_context->vas_windows counter to
clear foreign address mapping and disable it.
Changelog:
V2:
- Use threaded IRQ instead of own kernel thread handler
- Use pswid instead of user space CSB address to find valid CRB
- Removed unused macros and other changes as suggested by Christoph Hellwig
V3:
- Rebased to 5.5-rc2
- Use struct pid * instead of pid_t for vas_window tgid
- Code cleanup as suggested by Christoph Hellwig
V4:
- Define xive alloc and get IRQ info based on chip ID and use these
functions for IRQ setup per VAS instance. It eliminates skiboot
dependency as suggested by Oliver.
V5:
- Do not update CSB if the process is exiting (patch8)
V6:
- Add interrupt handler instead of default one and return IRQ_HANDLED
if the fault handling thread is already in progress. (Patch7)
- Use platform send window ID and CCW[0] bit to find valid CRB in
fault FIFO (Patch7).
- Return fault address to user space in BE and other changes as
suggested by Michael Neuling. (patch8)
- Rebased to 5.6-rc4
V7:
- Fixed sparse warnings (patches 4, 9 and 10)
V8:
- Moved mm_context_remove_copro() before mmdrop() (patch6)
- Moved barrier before csb.flags store and add WARN_ON_ONCE() checks (patch8)
V9:
- Rebased to 5.6
- Changes based on Cedric's comments
- Removed "Define xive_native_alloc_get_irq_info()" patch and used
irq_get_handler_data() (patch3)
- Changes based on comments from Nicholas Piggin
- Moved "Taking PID reference" patch before setting VAS fault handler
patch
- Removed mutex_lock/unlock (patch7)
- Other cleanup changes
V10:
- Include patch to enable and disable CP_ABORT execution using
mm_context->vas_windows counter.
- Remove 'if (txwin)' line which is covered with 'else' before (patch6)
Haren Myneni (14):
powerpc/xive: Define xive_native_alloc_irq_on_chip()
powerpc/vas: Define nx_fault_stamp in coprocessor_request_block
powerpc/vas: Alloc and setup IRQ and trigger port address
powerpc/vas: Setup fault window per VAS instance
powerpc/vas: Register NX with fault window ID and IRQ port value
powerpc/vas: Take reference to PID and mm for user space windows
powerpc/vas: Setup thread IRQ handler per VAS instance
powerpc/vas: Update CSB and notify process for fault CRBs
powerpc/vas: Return credits after handling fault
powerpc/vas: Print CRB and FIFO values
powerpc/vas: Do not use default credits for receive window
powerpc/vas: Display process stuck message
powerpc/vas: Free send window in VAS instance after credits returned
powerpc: Use mm_context vas_windows counter to issue CP_ABORT
arch/powerpc/include/asm/book3s/64/mmu.h | 3 +
arch/powerpc/include/asm/icswx.h | 20 +-
arch/powerpc/include/asm/mmu_context.h | 22 ++
arch/powerpc/include/asm/processor.h | 1 -
arch/powerpc/include/asm/xive.h | 9 +-
arch/powerpc/kernel/process.c | 8 +-
arch/powerpc/platforms/powernv/Makefile | 2 +-
arch/powerpc/platforms/powernv/vas-debug.c | 2 +-
arch/powerpc/platforms/powernv/vas-fault.c | 382 ++++++++++++++++++++++++++++
arch/powerpc/platforms/powernv/vas-window.c | 202 ++++++++++++++-
arch/powerpc/platforms/powernv/vas.c | 85 ++++++-
arch/powerpc/platforms/powernv/vas.h | 57 ++++-
arch/powerpc/sysdev/xive/native.c | 6 +-
13 files changed, 767 insertions(+), 32 deletions(-)
create mode 100644 arch/powerpc/platforms/powernv/vas-fault.c
--
1.8.3.1
^ permalink raw reply
* Re: [PATCH v4 14/25] nvdimm/ocxl: Add support for Admin commands
From: Dan Williams @ 2020-04-02 6:41 UTC (permalink / raw)
To: Alastair D'Silva
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Rob Herring, Dave Jiang, linux-nvdimm, Aneesh Kumar K . V,
Krzysztof Kozlowski, Anju T Sudhakar, Mahesh Salgaonkar,
Andrew Donnellan, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
Cédric Le Goater, Thomas Gleixner, Hari Bathini, Linux MM,
Greg Kroah-Hartman, Linux Kernel Mailing List, Vishal Verma,
Frederic Barrat, Paul Mackerras, Andrew Morton, linuxppc-dev,
David S. Miller
In-Reply-To: <20200327071202.2159885-15-alastair@d-silva.org>
On Sun, Mar 29, 2020 at 10:23 PM Alastair D'Silva <alastair@d-silva.org> wrote:
>
> Admin commands for these devices are the primary means of interacting
> with the device controller to provide functionality beyond the load/store
> capabilities offered via the NPU.
>
> For example, SMART data, firmware update, and device error logs are
> implemented via admin commands.
>
> This patch requests the metadata required to issue admin commands, as well
> as some helper functions to construct and check the completion of the
> commands.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
> drivers/nvdimm/ocxl/main.c | 65 ++++++
> drivers/nvdimm/ocxl/ocxlpmem.h | 50 ++++-
> drivers/nvdimm/ocxl/ocxlpmem_internal.c | 261 ++++++++++++++++++++++++
> 3 files changed, 375 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/nvdimm/ocxl/main.c b/drivers/nvdimm/ocxl/main.c
> index be76acd33d74..8db573036423 100644
> --- a/drivers/nvdimm/ocxl/main.c
> +++ b/drivers/nvdimm/ocxl/main.c
> @@ -217,6 +217,58 @@ static int register_lpc_mem(struct ocxlpmem *ocxlpmem)
> return 0;
> }
>
> +/**
> + * extract_command_metadata() - Extract command data from MMIO & save it for further use
> + * @ocxlpmem: the device metadata
> + * @offset: The base address of the command data structures (address of CREQO)
> + * @command_metadata: A pointer to the command metadata to populate
> + * Return: 0 on success, negative on failure
> + */
> +static int extract_command_metadata(struct ocxlpmem *ocxlpmem, u32 offset,
> + struct command_metadata *command_metadata)
How about "struct ocxlpmem *ocp" throughout all these patches? The
full duplication of the type name as the local variable name makes
this look like non-idiomatic Linux code to me. It had not quite hit me
until I saw "struct command_metadata *command_metadata" that just
strikes me as too literal and the person that gets to maintain this
code later will appreciate a smaller amount of typing.
Also, is it really the case that the layout of the admin command
metadata needs to be programmatically determined at runtime? I would
expect it to be a static command definition in the spec.
> +{
> + int rc;
> + u64 tmp;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu, offset,
> + OCXL_LITTLE_ENDIAN, &tmp);
> + if (rc)
> + return rc;
> +
> + command_metadata->request_offset = tmp >> 32;
> + command_metadata->response_offset = tmp & 0xFFFFFFFF;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu, offset + 8,
> + OCXL_LITTLE_ENDIAN, &tmp);
> + if (rc)
> + return rc;
> +
> + command_metadata->data_offset = tmp >> 32;
> + command_metadata->data_size = tmp & 0xFFFFFFFF;
> +
> + command_metadata->id = 0;
> +
> + return 0;
> +}
> +
> +/**
> + * setup_command_metadata() - Set up the command metadata
> + * @ocxlpmem: the device metadata
> + */
> +static int setup_command_metadata(struct ocxlpmem *ocxlpmem)
> +{
> + int rc;
> +
> + mutex_init(&ocxlpmem->admin_command.lock);
> +
> + rc = extract_command_metadata(ocxlpmem, GLOBAL_MMIO_ACMA_CREQO,
> + &ocxlpmem->admin_command);
> + if (rc)
> + return rc;
> +
> + return 0;
> +}
> +
> /**
> * allocate_minor() - Allocate a minor number to use for an OpenCAPI pmem device
> * @ocxlpmem: the device metadata
> @@ -421,6 +473,14 @@ static int probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>
> ocxlpmem->pdev = pci_dev_get(pdev);
>
> + ocxlpmem->timeouts[ADMIN_COMMAND_ERRLOG] = 2000; // ms
> + ocxlpmem->timeouts[ADMIN_COMMAND_HEARTBEAT] = 100; // ms
> + ocxlpmem->timeouts[ADMIN_COMMAND_SMART] = 100; // ms
> + ocxlpmem->timeouts[ADMIN_COMMAND_CONTROLLER_DUMP] = 1000; // ms
> + ocxlpmem->timeouts[ADMIN_COMMAND_CONTROLLER_STATS] = 100; // ms
> + ocxlpmem->timeouts[ADMIN_COMMAND_SHUTDOWN] = 1000; // ms
> + ocxlpmem->timeouts[ADMIN_COMMAND_FW_UPDATE] = 16000; // ms
> +
> pci_set_drvdata(pdev, ocxlpmem);
>
> ocxlpmem->ocxl_fn = ocxl_function_open(pdev);
> @@ -467,6 +527,11 @@ static int probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> goto err;
> }
>
> + if (setup_command_metadata(ocxlpmem)) {
> + dev_err(&pdev->dev, "Could not read command metadata\n");
> + goto err;
> + }
> +
> elapsed = 0;
> timeout = ocxlpmem->readiness_timeout +
> ocxlpmem->memory_available_timeout;
> diff --git a/drivers/nvdimm/ocxl/ocxlpmem.h b/drivers/nvdimm/ocxl/ocxlpmem.h
> index 3eadbe19f6d0..b72b3f909fc3 100644
> --- a/drivers/nvdimm/ocxl/ocxlpmem.h
> +++ b/drivers/nvdimm/ocxl/ocxlpmem.h
> @@ -7,6 +7,7 @@
> #include <linux/mm.h>
>
> #define LABEL_AREA_SIZE BIT_ULL(PA_SECTION_SHIFT)
> +#define DEFAULT_TIMEOUT 100
>
> #define GLOBAL_MMIO_CHI 0x000
> #define GLOBAL_MMIO_CHIC 0x008
> @@ -57,6 +58,7 @@
> #define GLOBAL_MMIO_HCI_CONTROLLER_DUMP_COLLECTED BIT_ULL(5) // CDC
> #define GLOBAL_MMIO_HCI_REQ_HEALTH_PERF BIT_ULL(6) // CHPD
>
> +// must be maintained with admin_command_names in ocxlpmem_internal.c
> #define ADMIN_COMMAND_HEARTBEAT 0x00u
> #define ADMIN_COMMAND_SHUTDOWN 0x01u
> #define ADMIN_COMMAND_FW_UPDATE 0x02u
> @@ -68,6 +70,13 @@
> #define ADMIN_COMMAND_CMD_CAPS 0x08u
> #define ADMIN_COMMAND_MAX 0x08u
>
> +#define NS_COMMAND_SECURE_ERASE 0x20ull
> +
> +#define NS_RESPONSE_SECURE_ERASE_ACCESSIBLE_SUCCESS 0x20
> +#define NS_RESPONSE_SECURE_ERASE_ACCESSIBLE_ATTEMPTED 0x28
> +#define NS_RESPONSE_SECURE_ERASE_DEFECTIVE_SUCCESS 0x30
> +#define NS_RESPONSE_SECURE_ERASE_DEFECTIVE_ATTEMPTED 0x38
> +
> #define STATUS_SUCCESS 0x00
> #define STATUS_MEM_UNAVAILABLE 0x20
> #define STATUS_BLOCKED_BG_TASK 0x21
> @@ -81,6 +90,16 @@
> #define STATUS_FW_ARG_INVALID STATUS_BAD_REQUEST_PARM
> #define STATUS_FW_INVALID STATUS_BAD_DATA_PARM
>
> +struct command_metadata {
> + u32 request_offset;
> + u32 response_offset;
> + u32 data_offset;
> + u32 data_size;
> + struct mutex lock; /* locks access to this command */
> + u16 id;
> + u8 op_code;
> +};
> +
> struct ocxlpmem {
> struct device dev;
> struct pci_dev *pdev;
> @@ -91,10 +110,11 @@ struct ocxlpmem {
> struct ocxl_afu *ocxl_afu;
> struct ocxl_context *ocxl_context;
> void *metadata_addr;
> + struct command_metadata admin_command;
> struct resource pmem_res;
> struct nd_region *nd_region;
> char fw_version[8 + 1];
> -
> + u32 timeouts[ADMIN_COMMAND_MAX + 1];
> u32 max_controller_dump_size;
> u16 scm_revision; // major/minor
> u8 readiness_timeout; /* The worst case time (in seconds) that the host
> @@ -123,3 +143,31 @@ struct ocxlpmem {
> * Returns 0 on success, negative on error
> */
> int ocxlpmem_chi(const struct ocxlpmem *ocxlpmem, u64 *chi);
> +
> +/**
> + * admin_command_execute() - Execute an admin command and wait for completion
> + *
> + * Additional MMIO registers (dependent on the command) may
> + * need to be initialized
> + *
> + * @ocxlpmem: the device metadata
> + * @op_code: the code for the admin command
> + * Returns 0 on success, -EINVAL for a bad op code, -EBUSY on timeout
> + */
> +int admin_command_execute(struct ocxlpmem *ocxlpmem, u8 op_code);
> +
> +/**
> + * admin_response_handled() - Notify the controller that the admin response has been handled
> + * @ocxlpmem: the device metadata
> + * Returns 0 on success, negative on failure
> + */
> +int admin_response_handled(const struct ocxlpmem *ocxlpmem);
> +
> +/**
> + * warn_status() - Emit a kernel warning showing a command status.
> + * @ocxlpmem: the device metadata
> + * @message: A message to accompany the warning
> + * @status: The command status
> + */
> +void warn_status(const struct ocxlpmem *ocxlpmem, const char *message,
> + u8 status);
> diff --git a/drivers/nvdimm/ocxl/ocxlpmem_internal.c b/drivers/nvdimm/ocxl/ocxlpmem_internal.c
> index 5578169b7515..7470a6ab3b08 100644
> --- a/drivers/nvdimm/ocxl/ocxlpmem_internal.c
> +++ b/drivers/nvdimm/ocxl/ocxlpmem_internal.c
> @@ -17,3 +17,264 @@ int ocxlpmem_chi(const struct ocxlpmem *ocxlpmem, u64 *chi)
>
> return 0;
> }
> +
> +#define COMMAND_REQUEST_SIZE (8 * sizeof(u64))
> +/**
> + * scm_command_request() - Set up a command request
> + * @cmd: The metadata for the type of command to be issued
> + * @op_code: the op code for the command
> + * @valid_bytes: the number of bytes in the header to preserve (these must be set before calling)
> + */
> +static int scm_command_request(const struct ocxlpmem *ocxlpmem,
> + struct command_metadata *cmd, u8 op_code,
> + u8 valid_bytes)
Ah, here's an abbreviation for command metadata.
> +{
> + u64 val = op_code;
> + int rc;
> + u8 i;
> +
> + cmd->op_code = op_code;
> + cmd->id++;
> +
> + val |= ((u64)cmd->id) << 16;
> +
> + rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu, cmd->request_offset,
> + OCXL_LITTLE_ENDIAN, val);
> + if (rc)
> + return rc;
> +
> + for (i = valid_bytes; i < COMMAND_REQUEST_SIZE; i += sizeof(u64)) {
> + rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu,
> + cmd->request_offset + i,
> + OCXL_LITTLE_ENDIAN, 0);
> + if (rc)
> + return rc;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * admin_command_request() - Issue an admin command request
> + * @ocxlpmem: the device metadata
> + * @op_code: The op-code for the command
> + *
> + * Returns an identifier for the command, or negative on error
> + */
> +static int admin_command_request(struct ocxlpmem *ocxlpmem, u8 op_code)
> +{
> + u8 valid_bytes = sizeof(u64);
> +
> + switch (op_code) {
> + case ADMIN_COMMAND_HEARTBEAT:
> + case ADMIN_COMMAND_SHUTDOWN:
> + case ADMIN_COMMAND_ERRLOG:
> + case ADMIN_COMMAND_CMD_CAPS:
> + valid_bytes += 0;
> + break;
> + case ADMIN_COMMAND_FW_UPDATE:
> + case ADMIN_COMMAND_SMART:
> + case ADMIN_COMMAND_CONTROLLER_STATS:
> + case ADMIN_COMMAND_CONTROLLER_DUMP:
> + valid_bytes += sizeof(u64);
> + break;
> + case ADMIN_COMMAND_FW_DEBUG:
> + valid_bytes += 3 * sizeof(u64);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return scm_command_request(ocxlpmem, &ocxlpmem->admin_command, op_code,
> + valid_bytes);
Why isn't this just:
ocp_command_request(ocp, op_code, valid_bytes);
...because it's not clear to me why this switches from a generic name
like admin_command_request() and then switches to scm_. The
"command_metadata" is implied by the ocp context and the op_code,
right?
> +}
> +
> +static int command_response(const struct ocxlpmem *ocxlpmem,
> + const struct command_metadata *cmd)
> +{
> + u64 val;
> + u16 id;
> + u8 status;
> + int rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + cmd->response_offset,
> + OCXL_LITTLE_ENDIAN, &val);
> + if (rc)
> + return rc;
> +
> + status = val & 0xff;
> + id = (val >> 16) & 0xffff;
> +
> + if (id != cmd->id) {
> + dev_err(&ocxlpmem->dev,
> + "Expected response for command %d, but received response for command %d instead.\n",
> + cmd->id, id);
> + return -EBUSY;
> + }
> +
> + return status;
> +}
> +
> +/**
> + * admin_response() - Validate an admin response
> + * @ocxlpmem: the device metadata
> + * Returns the status code of the command, or negative on error
> + */
> +static int admin_response(const struct ocxlpmem *ocxlpmem)
> +{
> + return command_response(ocxlpmem, &ocxlpmem->admin_command);
> +}
> +
> +/**
> + * admin_command_exec() - Notify the controller to start processing a pending admin command
> + * @ocxlpmem: the device metadata
> + * Returns 0 on success, negative on error
> + */
> +static int admin_command_exec(const struct ocxlpmem *ocxlpmem)
> +{
> + return ocxl_global_mmio_set64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_HCI,
> + OCXL_LITTLE_ENDIAN, GLOBAL_MMIO_HCI_ACRW);
> +}
> +
> +static bool admin_command_complete(const struct ocxlpmem *ocxlpmem)
> +{
> + u64 val = 0;
> +
> + int rc = ocxlpmem_chi(ocxlpmem, &val);
> +
> + WARN_ON(rc);
> +
> + return (val & GLOBAL_MMIO_CHI_ACRA) != 0;
> +}
> +
> +/**
> + * admin_command_complete_timeout() - Wait for an admin command to finish executing
> + * @ocxlpmem: the device metadata
> + * @command: the admin command to wait for completion (determines the timeout)
> + * Returns 0 on success, -EBUSY on timeout
> + */
> +static int admin_command_complete_timeout(const struct ocxlpmem *ocxlpmem,
> + int command)
> +{
> + unsigned long timeout = jiffies +
> + msecs_to_jiffies(ocxlpmem->timeouts[command]);
> +
> + // 32 is the next power of 2 greater than the 20ms minimum for msleep
> +#define TIMEOUT_SLEEP_MILLIS 32
> + do {
> + if (admin_command_complete(ocxlpmem))
> + return 0;
> + msleep(TIMEOUT_SLEEP_MILLIS);
> + } while (time_before(jiffies, timeout));
> +
> + if (admin_command_complete(ocxlpmem))
> + return 0;
> +
> + return -EBUSY;
> +}
> +
> +// Must be maintained with ADMIN_COMMAND_* in ocxlpmem.h
> +static const char * const admin_command_names[] = {
> + "heartbeat",
> + "shutdown",
> + "firmware update",
> + "firmware debug",
> + "retrieve error log",
> + "retrieve SMART data",
> + "controller statistics",
> + "controller dump",
> + "command capabilities",
> +};
> +
> +/**
> + * admin_command_name() - get the name of an admin command
> + * @ocxlpmem: the device metadata
> + * @op_code: the code for the admin command
> + * Returns a string representing the name of the command
> + */
> +static const char *admin_command_name(u8 op_code)
> +{
> + if (op_code > ADMIN_COMMAND_MAX)
> + return "unknown command";
> +
> + return admin_command_names[op_code];
> +}
> +
> +/**
> + * admin_command_execute() - Execute an admin command and wait for completion
> + *
> + * Additional MMIO registers (dependent on the command) may
> + * need to be initialized
> + *
> + * @ocxlpmem: the device metadata
> + * @op_code: the code for the admin command
> + * Returns 0 on success, -EINVAL for a bad op code, -EBUSY on timeout
> + */
> +int admin_command_execute(struct ocxlpmem *ocxlpmem, u8 op_code)
> +{
> + int rc;
> +
> + if (op_code > ADMIN_COMMAND_MAX)
> + return -EINVAL;
> +
> + rc = admin_command_request(ocxlpmem, op_code);
> + if (rc)
> + return rc;
> +
> + rc = admin_command_exec(ocxlpmem);
> + if (rc)
> + return rc;
> +
> + rc = admin_command_complete_timeout(ocxlpmem, op_code);
> + if (rc < 0) {
> + dev_warn(&ocxlpmem->dev, "%s timed out\n",
> + admin_command_name(op_code));
> + return rc;
> + }
> +
> + return admin_response(ocxlpmem);
> +}
> +
> +int admin_response_handled(const struct ocxlpmem *ocxlpmem)
> +{
> + // writing to the CHIC register clears the bit in CHI
> + return ocxl_global_mmio_set64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_CHIC,
> + OCXL_LITTLE_ENDIAN, GLOBAL_MMIO_CHI_ACRA);
> +}
> +
> +void warn_status(const struct ocxlpmem *ocxlpmem, const char *message,
> + u8 status)
> +{
> + const char *text = "Unknown";
> +
> + switch (status) {
> + case STATUS_SUCCESS:
> + text = "Success";
> + break;
> +
> + case STATUS_MEM_UNAVAILABLE:
> + text = "Persistent memory unavailable";
> + break;
> +
> + case STATUS_BAD_OPCODE:
> + text = "Bad opcode";
> + break;
> +
> + case STATUS_BAD_REQUEST_PARM:
> + text = "Bad request parameter";
> + break;
> +
> + case STATUS_BAD_DATA_PARM:
> + text = "Bad data parameter";
> + break;
> +
> + case STATUS_DEBUG_BLOCKED:
> + text = "Debug action blocked";
> + break;
> +
> + case STATUS_FAIL:
> + text = "Failed";
> + break;
> + }
> +
> + dev_warn(&ocxlpmem->dev, "%s: %s (%x)\n", message, text, status);
> +}
> --
> 2.24.1
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox