* [PATCH v2 12/15] powerpc/uaccess: Rename __get/put_user_check/nocheck
From: Christophe Leroy @ 2021-03-10 17:46 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1615398265.git.christophe.leroy@csgroup.eu>
__get_user_check() becomes get_user()
__put_user_check() becomes put_user()
__get_user_nocheck() becomes __get_user()
__put_user_nocheck() becomes __put_user()
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/uaccess.h | 30 ++++++++++--------------------
1 file changed, 10 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 678651a615c3..616a3a7928c2 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -43,16 +43,6 @@ static inline bool __access_ok(unsigned long addr, unsigned long size)
* exception handling means that it's no longer "just"...)
*
*/
-#define get_user(x, ptr) \
- __get_user_check((x), (ptr), sizeof(*(ptr)))
-#define put_user(x, ptr) \
- __put_user_check((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
-
-#define __get_user(x, ptr) \
- __get_user_nocheck((x), (ptr), sizeof(*(ptr)))
-#define __put_user(x, ptr) \
- __put_user_nocheck((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
-
#define __put_user_size(x, ptr, size, retval) \
do { \
__label__ __pu_failed; \
@@ -68,12 +58,12 @@ __pu_failed: \
prevent_write_to_user(ptr, size); \
} while (0)
-#define __put_user_nocheck(x, ptr, size) \
+#define __put_user(x, ptr) \
({ \
long __pu_err; \
__typeof__(*(ptr)) __user *__pu_addr = (ptr); \
- __typeof__(*(ptr)) __pu_val = (x); \
- __typeof__(size) __pu_size = (size); \
+ __typeof__(*(ptr)) __pu_val = (__typeof__(*(ptr)))(x); \
+ __typeof__(sizeof(*(ptr))) __pu_size = sizeof(*(ptr)); \
\
might_fault(); \
__put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \
@@ -81,12 +71,12 @@ __pu_failed: \
__pu_err; \
})
-#define __put_user_check(x, ptr, size) \
+#define put_user(x, ptr) \
({ \
long __pu_err = -EFAULT; \
__typeof__(*(ptr)) __user *__pu_addr = (ptr); \
- __typeof__(*(ptr)) __pu_val = (x); \
- __typeof__(size) __pu_size = (size); \
+ __typeof__(*(ptr)) __pu_val = (__typeof__(*(ptr)))(x); \
+ __typeof__(sizeof(*(ptr))) __pu_size = sizeof(*(ptr)); \
\
might_fault(); \
if (access_ok(__pu_addr, __pu_size)) \
@@ -216,12 +206,12 @@ do { \
#define __long_type(x) \
__typeof__(__builtin_choose_expr(sizeof(x) > sizeof(0UL), 0ULL, 0UL))
-#define __get_user_nocheck(x, ptr, size) \
+#define __get_user(x, ptr) \
({ \
long __gu_err; \
__long_type(*(ptr)) __gu_val; \
__typeof__(*(ptr)) __user *__gu_addr = (ptr); \
- __typeof__(size) __gu_size = (size); \
+ __typeof__(sizeof(*(ptr))) __gu_size = sizeof(*(ptr)); \
\
might_fault(); \
__get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
@@ -230,12 +220,12 @@ do { \
__gu_err; \
})
-#define __get_user_check(x, ptr, size) \
+#define get_user(x, ptr) \
({ \
long __gu_err = -EFAULT; \
__long_type(*(ptr)) __gu_val = 0; \
__typeof__(*(ptr)) __user *__gu_addr = (ptr); \
- __typeof__(size) __gu_size = (size); \
+ __typeof__(sizeof(*(ptr))) __gu_size = sizeof(*(ptr)); \
\
might_fault(); \
if (access_ok(__gu_addr, __gu_size)) \
--
2.25.0
^ permalink raw reply related
* [PATCH v2 13/15] powerpc/uaccess: Refactor get/put_user() and __get/put_user()
From: Christophe Leroy @ 2021-03-10 17:46 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1615398265.git.christophe.leroy@csgroup.eu>
Make get_user() do the access_ok() check then call __get_user().
Make put_user() do the access_ok() check then call __put_user().
Then embed __get_user_size() and __put_user_size() in
__get_user() and __put_user().
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/uaccess.h | 66 +++++++++++-------------------
1 file changed, 23 insertions(+), 43 deletions(-)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 616a3a7928c2..671c083f2f2f 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -43,21 +43,6 @@ static inline bool __access_ok(unsigned long addr, unsigned long size)
* exception handling means that it's no longer "just"...)
*
*/
-#define __put_user_size(x, ptr, size, retval) \
-do { \
- __label__ __pu_failed; \
- \
- retval = 0; \
- allow_write_to_user(ptr, size); \
- __put_user_size_goto(x, ptr, size, __pu_failed); \
- prevent_write_to_user(ptr, size); \
- break; \
- \
-__pu_failed: \
- retval = -EFAULT; \
- prevent_write_to_user(ptr, size); \
-} while (0)
-
#define __put_user(x, ptr) \
({ \
long __pu_err; \
@@ -66,23 +51,29 @@ __pu_failed: \
__typeof__(sizeof(*(ptr))) __pu_size = sizeof(*(ptr)); \
\
might_fault(); \
- __put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \
+ do { \
+ __label__ __pu_failed; \
+ \
+ allow_write_to_user(__pu_addr, __pu_size); \
+ __put_user_size_goto(__pu_val, __pu_addr, __pu_size, __pu_failed); \
+ prevent_write_to_user(__pu_addr, __pu_size); \
+ __pu_err = 0; \
+ break; \
+ \
+__pu_failed: \
+ prevent_write_to_user(__pu_addr, __pu_size); \
+ __pu_err = -EFAULT; \
+ } while (0); \
\
__pu_err; \
})
#define put_user(x, ptr) \
({ \
- long __pu_err = -EFAULT; \
- __typeof__(*(ptr)) __user *__pu_addr = (ptr); \
- __typeof__(*(ptr)) __pu_val = (__typeof__(*(ptr)))(x); \
- __typeof__(sizeof(*(ptr))) __pu_size = sizeof(*(ptr)); \
+ __typeof__(*(ptr)) __user *_pu_addr = (ptr); \
\
- might_fault(); \
- if (access_ok(__pu_addr, __pu_size)) \
- __put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \
- \
- __pu_err; \
+ access_ok(_pu_addr, sizeof(*(ptr))) ? \
+ __put_user(x, _pu_addr) : -EFAULT; \
})
/*
@@ -192,13 +183,6 @@ do { \
} \
} while (0)
-#define __get_user_size(x, ptr, size, retval) \
-do { \
- allow_read_from_user(ptr, size); \
- __get_user_size_allowed(x, ptr, size, retval); \
- prevent_read_from_user(ptr, size); \
-} while (0)
-
/*
* This is a type: either unsigned long, if the argument fits into
* that type, or otherwise unsigned long long.
@@ -214,7 +198,9 @@ do { \
__typeof__(sizeof(*(ptr))) __gu_size = sizeof(*(ptr)); \
\
might_fault(); \
- __get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
+ allow_read_from_user(__gu_addr, __gu_size); \
+ __get_user_size_allowed(__gu_val, __gu_addr, __gu_size, __gu_err); \
+ prevent_read_from_user(__gu_addr, __gu_size); \
(x) = (__typeof__(*(ptr)))__gu_val; \
\
__gu_err; \
@@ -222,17 +208,11 @@ do { \
#define get_user(x, ptr) \
({ \
- long __gu_err = -EFAULT; \
- __long_type(*(ptr)) __gu_val = 0; \
- __typeof__(*(ptr)) __user *__gu_addr = (ptr); \
- __typeof__(sizeof(*(ptr))) __gu_size = sizeof(*(ptr)); \
- \
- might_fault(); \
- if (access_ok(__gu_addr, __gu_size)) \
- __get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
- (x) = (__force __typeof__(*(ptr)))__gu_val; \
+ __typeof__(*(ptr)) __user *_gu_addr = (ptr); \
\
- __gu_err; \
+ access_ok(_gu_addr, sizeof(*(ptr))) ? \
+ __get_user(x, _gu_addr) : \
+ ((x) = (__force __typeof__(*(ptr)))0, -EFAULT); \
})
/* more complex routines */
--
2.25.0
^ permalink raw reply related
* [PATCH v2 15/15] powerpc/uaccess: Use asm goto for get_user when compiler supports it
From: Christophe Leroy @ 2021-03-10 17:46 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1615398265.git.christophe.leroy@csgroup.eu>
clang 11 and future GCC are supporting asm goto with outputs.
Use it to implement get_user in order to get better generated code.
Note that clang requires to set x in the default branch of
__get_user_size_goto() otherwise is compliant about x not being
initialised :puzzled:
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/uaccess.h | 55 ++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index e3e53e88cb26..960ab5c04b11 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -136,6 +136,59 @@ do { \
: "=r" (err) \
: "b" (uaddr), "b" (kaddr), "i" (-EFAULT), "0" (err))
+#ifdef CONFIG_CC_HAS_ASM_GOTO_OUTPUT
+
+#define __get_user_asm_goto(x, addr, label, op) \
+ asm_volatile_goto( \
+ "1: "op"%U1%X1 %0, %1 # get_user\n" \
+ EX_TABLE(1b, %l2) \
+ : "=r" (x) \
+ : "m"UPD_CONSTR (*addr) \
+ : \
+ : label)
+
+#ifdef __powerpc64__
+#define __get_user_asm2_goto(x, addr, label) \
+ __get_user_asm_goto(x, addr, label, "ld")
+#else /* __powerpc64__ */
+#define __get_user_asm2_goto(x, addr, label) \
+ asm_volatile_goto( \
+ "1: lwz%X1 %0, %1\n" \
+ "2: lwz%X1 %L0, %L1\n" \
+ EX_TABLE(1b, %l2) \
+ EX_TABLE(2b, %l2) \
+ : "=r" (x) \
+ : "m" (*addr) \
+ : \
+ : label)
+#endif /* __powerpc64__ */
+
+#define __get_user_size_goto(x, ptr, size, label) \
+do { \
+ BUILD_BUG_ON(size > sizeof(x)); \
+ switch (size) { \
+ case 1: __get_user_asm_goto(x, (u8 __user *)ptr, label, "lbz"); break; \
+ case 2: __get_user_asm_goto(x, (u16 __user *)ptr, label, "lhz"); break; \
+ case 4: __get_user_asm_goto(x, (u32 __user *)ptr, label, "lwz"); break; \
+ case 8: __get_user_asm2_goto(x, (u64 __user *)ptr, label); break; \
+ default: x = 0; BUILD_BUG(); \
+ } \
+} while (0)
+
+#define __get_user_size_allowed(x, ptr, size, retval) \
+do { \
+ __label__ __gus_failed; \
+ \
+ __get_user_size_goto(x, ptr, size, __gus_failed); \
+ retval = 0; \
+ break; \
+__gus_failed: \
+ x = 0; \
+ retval = -EFAULT; \
+} while (0)
+
+#else /* CONFIG_CC_HAS_ASM_GOTO_OUTPUT */
+
#define __get_user_asm(x, addr, err, op) \
__asm__ __volatile__( \
"1: "op"%U2%X2 %1, %2 # get_user\n" \
@@ -192,6 +245,8 @@ do { \
goto label; \
} while (0)
+#endif /* CONFIG_CC_HAS_ASM_GOTO_OUTPUT */
+
/*
* This is a type: either unsigned long, if the argument fits into
* that type, or otherwise unsigned long long.
--
2.25.0
^ permalink raw reply related
* [PATCH v2 14/15] powerpc/uaccess: Introduce __get_user_size_goto()
From: Christophe Leroy @ 2021-03-10 17:46 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1615398265.git.christophe.leroy@csgroup.eu>
We have got two places doing a goto based on the result
of __get_user_size_allowed().
Refactor that into __get_user_size_goto().
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/uaccess.h | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 671c083f2f2f..e3e53e88cb26 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -183,6 +183,15 @@ do { \
} \
} while (0)
+#define __get_user_size_goto(x, ptr, size, label) \
+do { \
+ long __gus_retval; \
+ \
+ __get_user_size_allowed(x, ptr, size, __gus_retval); \
+ if (__gus_retval) \
+ goto label; \
+} while (0)
+
/*
* This is a type: either unsigned long, if the argument fits into
* that type, or otherwise unsigned long long.
@@ -352,13 +361,10 @@ user_write_access_begin(const void __user *ptr, size_t len)
#define user_write_access_end prevent_current_write_to_user
#define unsafe_get_user(x, p, e) do { \
- long __gu_err; \
__long_type(*(p)) __gu_val; \
__typeof__(*(p)) __user *__gu_addr = (p); \
\
- __get_user_size_allowed(__gu_val, __gu_addr, sizeof(*(p)), __gu_err); \
- if (__gu_err) \
- goto e; \
+ __get_user_size_goto(__gu_val, __gu_addr, sizeof(*(p)), e); \
(x) = (__typeof__(*(p)))__gu_val; \
} while (0)
@@ -389,14 +395,8 @@ do { \
#define HAVE_GET_KERNEL_NOFAULT
#define __get_kernel_nofault(dst, src, type, err_label) \
-do { \
- int __kr_err; \
- \
- __get_user_size_allowed(*((type *)(dst)), (__force type __user *)(src),\
- sizeof(type), __kr_err); \
- if (unlikely(__kr_err)) \
- goto err_label; \
-} while (0)
+ __get_user_size_goto(*((type *)(dst)), \
+ (__force type __user *)(src), sizeof(type), err_label)
#define __put_kernel_nofault(dst, src, type, err_label) \
__put_user_size_goto(*((type *)(src)), \
--
2.25.0
^ permalink raw reply related
* [PATCH v1 0/8] Miscellaneous user access improvement
From: Christophe Leroy @ 2021-03-10 17:56 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
Patches 1-3 are cleaning parts of uaccess.h not related
to put_user/get_user
Patch 4 removes some usage of consecutives __get_user
Patches 5 rewrite __patch_instruction to not use uaccess.h internals.
Patches 6-8 switch some parts of code to user_access_begin/end blocks
All patches are independant.
Christophe Leroy (8):
powerpc/uaccess: Also perform 64 bits copies in unsafe_copy_to_user()
on ppc32
powerpc/uaccess: Swap clear_user() and __clear_user()
powerpc/uaccess: Move copy_mc_xxx() functions down
powerpc/syscalls: Use sys_old_select() in ppc_select()
powerpc/lib: Don't use __put_user_asm_goto() outside of uaccess.h
powerpc/net: Switch csum_and_copy_{to/from}_user to user_access block
powerpc/futex: Switch to user_access block
powerpc/ptrace: Convert gpr32_set_common() to user access block
arch/powerpc/include/asm/futex.h | 12 ++--
arch/powerpc/include/asm/ptrace.h | 2 +-
arch/powerpc/include/asm/uaccess.h | 75 ++++++++++++------------
arch/powerpc/include/asm/unistd.h | 1 +
arch/powerpc/kernel/ptrace/ptrace-view.c | 30 ++++++----
arch/powerpc/kernel/syscalls.c | 12 +---
arch/powerpc/lib/checksum_wrappers.c | 15 ++---
arch/powerpc/lib/code-patching.c | 13 ++--
8 files changed, 77 insertions(+), 83 deletions(-)
--
2.25.0
^ permalink raw reply
* [PATCH v1 1/8] powerpc/uaccess: Also perform 64 bits copies in unsafe_copy_to_user() on ppc32
From: Christophe Leroy @ 2021-03-10 17:57 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1615398498.git.christophe.leroy@csgroup.eu>
ppc32 has an efficiant 64 bits __put_user(), so also use it in
order to unroll loops more.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/uaccess.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 78e2a3990eab..2c09cff205ef 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -494,9 +494,9 @@ do { \
size_t _len = (l); \
int _i; \
\
- for (_i = 0; _i < (_len & ~(sizeof(long) - 1)); _i += sizeof(long)) \
- unsafe_put_user(*(long*)(_src + _i), (long __user *)(_dst + _i), e); \
- if (IS_ENABLED(CONFIG_PPC64) && (_len & 4)) { \
+ for (_i = 0; _i < (_len & ~(sizeof(u64) - 1)); _i += sizeof(u64)) \
+ unsafe_put_user(*(u64 *)(_src + _i), (u64 __user *)(_dst + _i), e); \
+ if (_len & 4) { \
unsafe_put_user(*(u32*)(_src + _i), (u32 __user *)(_dst + _i), e); \
_i += 4; \
} \
--
2.25.0
^ permalink raw reply related
* [PATCH v1 3/8] powerpc/uaccess: Move copy_mc_xxx() functions down
From: Christophe Leroy @ 2021-03-10 17:57 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1615398498.git.christophe.leroy@csgroup.eu>
copy_mc_xxx() functions are in the middle of raw_copy functions.
For clarity, move them out of the raw_copy functions block.
They are using access_ok, so they need to be after the general
functions in order to eventually allow the inclusion of
asm-generic/uaccess.h in some future.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/uaccess.h | 52 +++++++++++++++---------------
1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 1c1d404514b1..479cb30eabd7 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -351,32 +351,6 @@ do { \
extern unsigned long __copy_tofrom_user(void __user *to,
const void __user *from, unsigned long size);
-#ifdef CONFIG_ARCH_HAS_COPY_MC
-unsigned long __must_check
-copy_mc_generic(void *to, const void *from, unsigned long size);
-
-static inline unsigned long __must_check
-copy_mc_to_kernel(void *to, const void *from, unsigned long size)
-{
- return copy_mc_generic(to, from, size);
-}
-#define copy_mc_to_kernel copy_mc_to_kernel
-
-static inline unsigned long __must_check
-copy_mc_to_user(void __user *to, const void *from, unsigned long n)
-{
- if (likely(check_copy_size(from, n, true))) {
- if (access_ok(to, n)) {
- allow_write_to_user(to, n);
- n = copy_mc_generic((void *)to, from, n);
- prevent_write_to_user(to, n);
- }
- }
-
- return n;
-}
-#endif
-
#ifdef __powerpc64__
static inline unsigned long
raw_copy_in_user(void __user *to, const void __user *from, unsigned long n)
@@ -433,6 +407,32 @@ static inline unsigned long clear_user(void __user *addr, unsigned long size)
extern long strncpy_from_user(char *dst, const char __user *src, long count);
extern __must_check long strnlen_user(const char __user *str, long n);
+#ifdef CONFIG_ARCH_HAS_COPY_MC
+unsigned long __must_check
+copy_mc_generic(void *to, const void *from, unsigned long size);
+
+static inline unsigned long __must_check
+copy_mc_to_kernel(void *to, const void *from, unsigned long size)
+{
+ return copy_mc_generic(to, from, size);
+}
+#define copy_mc_to_kernel copy_mc_to_kernel
+
+static inline unsigned long __must_check
+copy_mc_to_user(void __user *to, const void *from, unsigned long n)
+{
+ if (likely(check_copy_size(from, n, true))) {
+ if (access_ok(to, n)) {
+ allow_write_to_user(to, n);
+ n = copy_mc_generic((void *)to, from, n);
+ prevent_write_to_user(to, n);
+ }
+ }
+
+ return n;
+}
+#endif
+
extern long __copy_from_user_flushcache(void *dst, const void __user *src,
unsigned size);
extern void memcpy_page_flushcache(char *to, struct page *page, size_t offset,
--
2.25.0
^ permalink raw reply related
* [PATCH v1 4/8] powerpc/syscalls: Use sys_old_select() in ppc_select()
From: Christophe Leroy @ 2021-03-10 17:57 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1615398498.git.christophe.leroy@csgroup.eu>
Instead of opencodying the copy of parameters, use
the generic sys_old_select().
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/unistd.h | 1 +
arch/powerpc/kernel/syscalls.c | 12 ++----------
2 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
index 700fcdac2e3c..b541c690a31c 100644
--- a/arch/powerpc/include/asm/unistd.h
+++ b/arch/powerpc/include/asm/unistd.h
@@ -40,6 +40,7 @@
#define __ARCH_WANT_SYS_SIGPROCMASK
#ifdef CONFIG_PPC32
#define __ARCH_WANT_OLD_STAT
+#define __ARCH_WANT_SYS_OLD_SELECT
#endif
#ifdef CONFIG_PPC64
#define __ARCH_WANT_SYS_TIME
diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
index 078608ec2e92..a552c9e68d7e 100644
--- a/arch/powerpc/kernel/syscalls.c
+++ b/arch/powerpc/kernel/syscalls.c
@@ -82,16 +82,8 @@ int
ppc_select(int n, fd_set __user *inp, fd_set __user *outp, fd_set __user *exp, struct __kernel_old_timeval __user *tvp)
{
if ( (unsigned long)n >= 4096 )
- {
- unsigned long __user *buffer = (unsigned long __user *)n;
- if (!access_ok(buffer, 5*sizeof(unsigned long))
- || __get_user(n, buffer)
- || __get_user(inp, ((fd_set __user * __user *)(buffer+1)))
- || __get_user(outp, ((fd_set __user * __user *)(buffer+2)))
- || __get_user(exp, ((fd_set __user * __user *)(buffer+3)))
- || __get_user(tvp, ((struct __kernel_old_timeval __user * __user *)(buffer+4))))
- return -EFAULT;
- }
+ return sys_old_select((void __user *)n);
+
return sys_select(n, inp, outp, exp, tvp);
}
#endif
--
2.25.0
^ permalink raw reply related
* [PATCH v1 2/8] powerpc/uaccess: Swap clear_user() and __clear_user()
From: Christophe Leroy @ 2021-03-10 17:57 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1615398498.git.christophe.leroy@csgroup.eu>
It is clear_user() which is expected to call __clear_user(),
not the reverse.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/uaccess.h | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 2c09cff205ef..1c1d404514b1 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -414,21 +414,20 @@ raw_copy_to_user(void __user *to, const void *from, unsigned long n)
unsigned long __arch_clear_user(void __user *addr, unsigned long size);
-static inline unsigned long clear_user(void __user *addr, unsigned long size)
+static inline unsigned long __clear_user(void __user *addr, unsigned long size)
{
- unsigned long ret = size;
+ unsigned long ret;
+
might_fault();
- if (likely(access_ok(addr, size))) {
- allow_write_to_user(addr, size);
- ret = __arch_clear_user(addr, size);
- prevent_write_to_user(addr, size);
- }
+ allow_write_to_user(addr, size);
+ ret = __arch_clear_user(addr, size);
+ prevent_write_to_user(addr, size);
return ret;
}
-static inline unsigned long __clear_user(void __user *addr, unsigned long size)
+static inline unsigned long clear_user(void __user *addr, unsigned long size)
{
- return clear_user(addr, size);
+ return likely(access_ok(addr, size)) ? __clear_user(addr, size) : size;
}
extern long strncpy_from_user(char *dst, const char __user *src, long count);
--
2.25.0
^ permalink raw reply related
* [PATCH v1 5/8] powerpc/lib: Don't use __put_user_asm_goto() outside of uaccess.h
From: Christophe Leroy @ 2021-03-10 17:57 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1615398498.git.christophe.leroy@csgroup.eu>
__put_user_asm_goto() is internal to uaccess.h
Use __put_kernel_nofault() instead. The generated code is identical.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/lib/code-patching.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 2333625b5e31..65aec4d6d9ba 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -21,10 +21,15 @@
static int __patch_instruction(struct ppc_inst *exec_addr, struct ppc_inst instr,
struct ppc_inst *patch_addr)
{
- if (!ppc_inst_prefixed(instr))
- __put_user_asm_goto(ppc_inst_val(instr), patch_addr, failed, "stw");
- else
- __put_user_asm_goto(ppc_inst_as_u64(instr), patch_addr, failed, "std");
+ if (!ppc_inst_prefixed(instr)) {
+ u32 val = ppc_inst_val(instr);
+
+ __put_kernel_nofault(patch_addr, &val, u32, failed);
+ } else {
+ u64 val = ppc_inst_as_u64(instr);
+
+ __put_kernel_nofault(patch_addr, &val, u64, failed);
+ }
asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r" (patch_addr),
"r" (exec_addr));
--
2.25.0
^ permalink raw reply related
* [PATCH v1 6/8] powerpc/net: Switch csum_and_copy_{to/from}_user to user_access block
From: Christophe Leroy @ 2021-03-10 17:57 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1615398498.git.christophe.leroy@csgroup.eu>
Use user_access_begin() instead of the
might_sleep/access_ok/allow_access sequence.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/lib/checksum_wrappers.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/lib/checksum_wrappers.c b/arch/powerpc/lib/checksum_wrappers.c
index b895166afc82..f3999cbb2fcc 100644
--- a/arch/powerpc/lib/checksum_wrappers.c
+++ b/arch/powerpc/lib/checksum_wrappers.c
@@ -16,16 +16,12 @@ __wsum csum_and_copy_from_user(const void __user *src, void *dst,
{
__wsum csum;
- might_sleep();
-
- if (unlikely(!access_ok(src, len)))
+ if (unlikely(!user_read_access_begin(src, len)))
return 0;
- allow_read_from_user(src, len);
-
csum = csum_partial_copy_generic((void __force *)src, dst, len);
- prevent_read_from_user(src, len);
+ user_read_access_end();
return csum;
}
EXPORT_SYMBOL(csum_and_copy_from_user);
@@ -34,15 +30,12 @@ __wsum csum_and_copy_to_user(const void *src, void __user *dst, int len)
{
__wsum csum;
- might_sleep();
- if (unlikely(!access_ok(dst, len)))
+ if (unlikely(!user_write_access_begin(dst, len)))
return 0;
- allow_write_to_user(dst, len);
-
csum = csum_partial_copy_generic(src, (void __force *)dst, len);
- prevent_write_to_user(dst, len);
+ user_write_access_end();
return csum;
}
EXPORT_SYMBOL(csum_and_copy_to_user);
--
2.25.0
^ permalink raw reply related
* [PATCH v1 7/8] powerpc/futex: Switch to user_access block
From: Christophe Leroy @ 2021-03-10 17:57 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1615398498.git.christophe.leroy@csgroup.eu>
Use user_access_begin() instead of the access_ok/allow_access sequence.
This brings the missing might_fault() check.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/futex.h | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/futex.h b/arch/powerpc/include/asm/futex.h
index e93ee3202e4c..b3001f8b2c1e 100644
--- a/arch/powerpc/include/asm/futex.h
+++ b/arch/powerpc/include/asm/futex.h
@@ -33,9 +33,8 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
{
int oldval = 0, ret;
- if (!access_ok(uaddr, sizeof(u32)))
+ if (!user_access_begin(uaddr, sizeof(u32)))
return -EFAULT;
- allow_read_write_user(uaddr, uaddr, sizeof(*uaddr));
switch (op) {
case FUTEX_OP_SET:
@@ -56,10 +55,10 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
default:
ret = -ENOSYS;
}
+ user_access_end();
*oval = oldval;
- prevent_read_write_user(uaddr, uaddr, sizeof(*uaddr));
return ret;
}
@@ -70,11 +69,9 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
int ret = 0;
u32 prev;
- if (!access_ok(uaddr, sizeof(u32)))
+ if (!user_access_begin(uaddr, sizeof(u32)))
return -EFAULT;
- allow_read_write_user(uaddr, uaddr, sizeof(*uaddr));
-
__asm__ __volatile__ (
PPC_ATOMIC_ENTRY_BARRIER
"1: lwarx %1,0,%3 # futex_atomic_cmpxchg_inatomic\n\
@@ -93,8 +90,9 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
: "r" (uaddr), "r" (oldval), "r" (newval), "i" (-EFAULT)
: "cc", "memory");
+ user_access_end();
+
*uval = prev;
- prevent_read_write_user(uaddr, uaddr, sizeof(*uaddr));
return ret;
}
--
2.25.0
^ permalink raw reply related
* [PATCH v1 8/8] powerpc/ptrace: Convert gpr32_set_common() to user access block
From: Christophe Leroy @ 2021-03-10 17:57 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1615398498.git.christophe.leroy@csgroup.eu>
Use user access block in gpr32_set_common() instead of
repetitive __get_user() which imply repetitive KUAP open/close.
To get it clean, force inlining of the small set of tiny functions
called inside the block.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/ptrace.h | 2 +-
arch/powerpc/kernel/ptrace/ptrace-view.c | 30 ++++++++++++++----------
2 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index 975ba260006a..cb154fb7b605 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -222,7 +222,7 @@ do { \
} while (0)
#endif /* __powerpc64__ */
-static inline void set_trap(struct pt_regs *regs, unsigned long val)
+static __always_inline void set_trap(struct pt_regs *regs, unsigned long val)
{
regs->trap = (regs->trap & TRAP_FLAGS_MASK) | (val & ~TRAP_FLAGS_MASK);
}
diff --git a/arch/powerpc/kernel/ptrace/ptrace-view.c b/arch/powerpc/kernel/ptrace/ptrace-view.c
index 2bad8068f598..0923c94f684e 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-view.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-view.c
@@ -111,7 +111,7 @@ static unsigned long get_user_msr(struct task_struct *task)
return task->thread.regs->msr | task->thread.fpexc_mode;
}
-static int set_user_msr(struct task_struct *task, unsigned long msr)
+static __always_inline int set_user_msr(struct task_struct *task, unsigned long msr)
{
task->thread.regs->msr &= ~MSR_DEBUGCHANGE;
task->thread.regs->msr |= msr & MSR_DEBUGCHANGE;
@@ -147,7 +147,7 @@ static int set_user_dscr(struct task_struct *task, unsigned long dscr)
* We prevent mucking around with the reserved area of trap
* which are used internally by the kernel.
*/
-static int set_user_trap(struct task_struct *task, unsigned long trap)
+static __always_inline int set_user_trap(struct task_struct *task, unsigned long trap)
{
set_trap(task->thread.regs, trap);
return 0;
@@ -661,6 +661,9 @@ int gpr32_set_common(struct task_struct *target,
const compat_ulong_t __user *u = ubuf;
compat_ulong_t reg;
+ if (!kbuf && !user_read_access_begin(u, count))
+ return -EFAULT;
+
pos /= sizeof(reg);
count /= sizeof(reg);
@@ -669,8 +672,7 @@ int gpr32_set_common(struct task_struct *target,
regs[pos++] = *k++;
else
for (; count > 0 && pos < PT_MSR; --count) {
- if (__get_user(reg, u++))
- return -EFAULT;
+ unsafe_get_user(reg, u++, Efault);
regs[pos++] = reg;
}
@@ -678,8 +680,8 @@ int gpr32_set_common(struct task_struct *target,
if (count > 0 && pos == PT_MSR) {
if (kbuf)
reg = *k++;
- else if (__get_user(reg, u++))
- return -EFAULT;
+ else
+ unsafe_get_user(reg, u++, Efault);
set_user_msr(target, reg);
++pos;
--count;
@@ -692,24 +694,24 @@ int gpr32_set_common(struct task_struct *target,
++k;
} else {
for (; count > 0 && pos <= PT_MAX_PUT_REG; --count) {
- if (__get_user(reg, u++))
- return -EFAULT;
+ unsafe_get_user(reg, u++, Efault);
regs[pos++] = reg;
}
for (; count > 0 && pos < PT_TRAP; --count, ++pos)
- if (__get_user(reg, u++))
- return -EFAULT;
+ unsafe_get_user(reg, u++, Efault);
}
if (count > 0 && pos == PT_TRAP) {
if (kbuf)
reg = *k++;
- else if (__get_user(reg, u++))
- return -EFAULT;
+ else
+ unsafe_get_user(reg, u++, Efault);
set_user_trap(target, reg);
++pos;
--count;
}
+ if (!kbuf)
+ user_read_access_end();
kbuf = k;
ubuf = u;
@@ -717,6 +719,10 @@ int gpr32_set_common(struct task_struct *target,
count *= sizeof(reg);
return user_regset_copyin_ignore(&pos, &count, &kbuf, &ubuf,
(PT_TRAP + 1) * sizeof(reg), -1);
+
+Efault:
+ user_read_access_end();
+ return -EFAULT;
}
static int gpr32_get(struct task_struct *target,
--
2.25.0
^ permalink raw reply related
* Re: [PATCH 14/17] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE
From: Robin Murphy @ 2021-03-10 18:39 UTC (permalink / raw)
To: Christoph Hellwig
Cc: kvm, Will Deacon, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang,
iommu, netdev, linux-arm-kernel, virtualization, freedreno,
David Woodhouse, linux-arm-msm
In-Reply-To: <20210310092533.GA6819@lst.de>
On 2021-03-10 09:25, Christoph Hellwig wrote:
> On Wed, Mar 10, 2021 at 10:15:01AM +0100, Christoph Hellwig wrote:
>> On Thu, Mar 04, 2021 at 03:25:27PM +0000, Robin Murphy wrote:
>>> On 2021-03-01 08:42, Christoph Hellwig wrote:
>>>> Use explicit methods for setting and querying the information instead.
>>>
>>> Now that everyone's using iommu-dma, is there any point in bouncing this
>>> through the drivers at all? Seems like it would make more sense for the x86
>>> drivers to reflect their private options back to iommu_dma_strict (and
>>> allow Intel's caching mode to override it as well), then have
>>> iommu_dma_init_domain just test !iommu_dma_strict &&
>>> domain->ops->flush_iotlb_all.
>>
>> Hmm. I looked at this, and kill off ->dma_enable_flush_queue for
>> the ARM drivers and just looking at iommu_dma_strict seems like a
>> very clear win.
>>
>> OTOH x86 is a little more complicated. AMD and intel defaul to lazy
>> mode, so we'd have to change the global iommu_dma_strict if they are
>> initialized. Also Intel has not only a "static" option to disable
>> lazy mode, but also a "dynamic" one where it iterates structure. So
>> I think on the get side we're stuck with the method, but it still
>> simplifies the whole thing.
>
> Actually... Just mirroring the iommu_dma_strict value into
> struct iommu_domain should solve all of that with very little
> boilerplate code.
Yes, my initial thought was to directly replace the attribute with a
common flag at iommu_domain level, but since in all cases the behaviour
is effectively global rather than actually per-domain, it seemed
reasonable to take it a step further. This passes compile-testing for
arm64 and x86, what do you think?
Robin.
----->8-----
Subject: [PATCH] iommu: Consolidate strict invalidation handling
Now that everyone is using iommu-dma, the global invalidation policy
really doesn't need to be woven through several parts of the core API
and individual drivers, we can just look it up directly at the one point
that we now make the flush queue decision. If the x86 drivers reflect
their internal options and overrides back to iommu_dma_strict, that can
become the canonical source.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/amd/iommu.c | 2 ++
drivers/iommu/dma-iommu.c | 8 +-------
drivers/iommu/intel/iommu.c | 12 ++++++++++++
drivers/iommu/iommu.c | 35 +++++++++++++++++++++++++++--------
include/linux/iommu.h | 2 ++
5 files changed, 44 insertions(+), 15 deletions(-)
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index a69a8b573e40..1db29e59d468 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -1856,6 +1856,8 @@ int __init amd_iommu_init_dma_ops(void)
else
pr_info("Lazy IO/TLB flushing enabled\n");
+ iommu_set_dma_strict(amd_iommu_unmap_flush);
+
return 0;
}
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index af765c813cc8..789a950cc125 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -304,10 +304,6 @@ static void iommu_dma_flush_iotlb_all(struct iova_domain *iovad)
cookie = container_of(iovad, struct iommu_dma_cookie, iovad);
domain = cookie->fq_domain;
- /*
- * The IOMMU driver supporting DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE
- * implies that ops->flush_iotlb_all must be non-NULL.
- */
domain->ops->flush_iotlb_all(domain);
}
@@ -334,7 +330,6 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
struct iommu_dma_cookie *cookie = domain->iova_cookie;
unsigned long order, base_pfn;
struct iova_domain *iovad;
- int attr;
if (!cookie || cookie->type != IOMMU_DMA_IOVA_COOKIE)
return -EINVAL;
@@ -371,8 +366,7 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
init_iova_domain(iovad, 1UL << order, base_pfn);
if (!cookie->fq_domain && (!dev || !dev_is_untrusted(dev)) &&
- !iommu_domain_get_attr(domain, DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE, &attr) &&
- attr) {
+ domain->ops->flush_iotlb_all && !iommu_get_dma_strict()) {
if (init_iova_flush_queue(iovad, iommu_dma_flush_iotlb_all,
iommu_dma_entry_dtor))
pr_warn("iova flush queue initialization failed\n");
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index b5c746f0f63b..f5b452cd1266 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4377,6 +4377,17 @@ int __init intel_iommu_init(void)
down_read(&dmar_global_lock);
for_each_active_iommu(iommu, drhd) {
+ if (!intel_iommu_strict && cap_caching_mode(iommu->cap)) {
+ /*
+ * The flush queue implementation does not perform page-selective
+ * invalidations that are required for efficient TLB flushes in
+ * virtual environments. The benefit of batching is likely to be
+ * much lower than the overhead of synchronizing the virtual and
+ * physical IOMMU page-tables.
+ */
+ pr_warn("IOMMU batching is disabled due to virtualization");
+ intel_iommu_strict = 1;
+ }
iommu_device_sysfs_add(&iommu->iommu, NULL,
intel_iommu_groups,
"%s", iommu->name);
@@ -4384,6 +4395,7 @@ int __init intel_iommu_init(void)
}
up_read(&dmar_global_lock);
+ iommu_set_dma_strict(intel_iommu_strict);
bus_set_iommu(&pci_bus_type, &intel_iommu_ops);
if (si_domain && !hw_pass_through)
register_memory_notifier(&intel_iommu_memory_nb);
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 2f3399203813..80afcf50cd3c 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -69,6 +69,7 @@ static const char * const iommu_group_resv_type_string[] = {
};
#define IOMMU_CMD_LINE_DMA_API BIT(0)
+#define IOMMU_CMD_LINE_STRICT BIT(1)
static void iommu_set_cmd_line_dma_api(void)
{
@@ -80,6 +81,16 @@ static bool iommu_cmd_line_dma_api(void)
return !!(iommu_cmd_line & IOMMU_CMD_LINE_DMA_API);
}
+static void iommu_set_cmd_line_strict(void)
+{
+ iommu_cmd_line |= IOMMU_CMD_LINE_STRICT;
+}
+
+static bool iommu_cmd_line_strict(void)
+{
+ return !!(iommu_cmd_line & IOMMU_CMD_LINE_STRICT);
+}
+
static int iommu_alloc_default_domain(struct iommu_group *group,
struct device *dev);
static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
@@ -337,10 +348,25 @@ early_param("iommu.passthrough", iommu_set_def_domain_type);
static int __init iommu_dma_setup(char *str)
{
- return kstrtobool(str, &iommu_dma_strict);
+ int ret = kstrtobool(str, &iommu_dma_strict);
+
+ if (!ret)
+ iommu_set_cmd_line_strict();
+ return ret;
}
early_param("iommu.strict", iommu_dma_setup);
+void iommu_set_dma_strict(bool val)
+{
+ if (val || !iommu_cmd_line_strict())
+ iommu_dma_strict = val;
+}
+
+bool iommu_get_dma_strict(void)
+{
+ return iommu_dma_strict;
+}
+
static ssize_t iommu_group_attr_show(struct kobject *kobj,
struct attribute *__attr, char *buf)
{
@@ -1520,13 +1546,6 @@ static int iommu_group_alloc_default_domain(struct bus_type *bus,
if (!group->domain)
group->domain = dom;
- if (!iommu_dma_strict) {
- int attr = 1;
- iommu_domain_set_attr(dom,
- DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE,
- &attr);
- }
-
return 0;
}
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index eb5e3f14c5ad..11bbfa273d98 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -495,6 +495,8 @@ extern int iommu_domain_get_attr(struct iommu_domain *domain, enum iommu_attr,
void *data);
extern int iommu_domain_set_attr(struct iommu_domain *domain, enum iommu_attr,
void *data);
+extern void iommu_set_dma_strict(bool val);
+extern bool iommu_get_dma_strict(void);
/* Window handling function prototypes */
extern int iommu_domain_window_enable(struct iommu_domain *domain, u32 wnd_nr,
--
2.21.0.dirty
^ permalink raw reply related
* [PATCH] ALSA: ppc: keywest: remove outdated comment
From: Wolfram Sang @ 2021-03-10 19:32 UTC (permalink / raw)
To: alsa-devel; +Cc: Wolfram Sang, linuxppc-dev
The I2C attach_adapter callback is gone. Remove this reference.
Signed-off-by: Wolfram Sang <wsa@kernel.org>
---
sound/ppc/keywest.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/sound/ppc/keywest.c b/sound/ppc/keywest.c
index a6c1905039de..a8915100d6bb 100644
--- a/sound/ppc/keywest.c
+++ b/sound/ppc/keywest.c
@@ -13,12 +13,7 @@
#include <sound/core.h>
#include "pmac.h"
-/*
- * we have to keep a static variable here since i2c attach_adapter
- * callback cannot pass a private data.
- */
static struct pmac_keywest *keywest_ctx;
-
static bool keywest_probed;
static int keywest_probe(struct i2c_client *client,
--
2.30.0
^ permalink raw reply related
* Re: [PATCH v4 3/6] ASoC: dt-bindings: fsl_rpmsg: Add binding doc for rpmsg cpu dai driver
From: Rob Herring @ 2021-03-10 21:12 UTC (permalink / raw)
To: Shengjiu Wang
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-ALSA, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Takashi Iwai, Liam Girdwood, linux-kernel, Nicolin Chen,
Mark Brown, linuxppc-dev
In-Reply-To: <CAA+D8AM5nH+gwfas_=9gkzaegq4=4q2AfVybBnxM4xU3gOiF4w@mail.gmail.com>
On Wed, Mar 10, 2021 at 6:33 AM Shengjiu Wang <shengjiu.wang@gmail.com> wrote:
>
> Hi Rob
>
> On Wed, Mar 10, 2021 at 10:49 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Mon, Mar 08, 2021 at 09:22:27PM +0800, Shengjiu Wang wrote:
> > > fsl_rpmsg cpu dai driver is driver for rpmsg audio, which is mainly used
> >
> > Bindings describe h/w blocks, not drivers.
>
> I will modify the descriptions. but here it is a virtual device. the
> mapping in real h/w is cortex M core, Cortex M core controls the SAI,
> DMA interface. What we see from Linux side is a audio service
> through rpmsg channel.
It's describing the h/w from the view of the OS. It's not important
that it's a Cortex-M, but how you interface to it whether that's
shared registers, mailbox, etc. And it's what resources the block uses
that the OS controls.
> > > for getting the user's configuration from device tree and configure the
> > > clocks which is used by Cortex-M core. So in this document define the
> > > needed property.
> > >
> > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > ---
> > > .../devicetree/bindings/sound/fsl,rpmsg.yaml | 118 ++++++++++++++++++
> > > 1 file changed, 118 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> > > new file mode 100644
> > > index 000000000000..5731c1fbc0a6
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> > > @@ -0,0 +1,118 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/sound/fsl,rpmsg.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: NXP Audio RPMSG CPU DAI Controller
> > > +
> > > +maintainers:
> > > + - Shengjiu Wang <shengjiu.wang@nxp.com>
> > > +
> > > +description: |
> > > + fsl_rpmsg cpu dai driver is virtual driver for rpmsg audio, which doesn't
> > > + touch hardware. It is mainly used for getting the user's configuration
> > > + from device tree and configure the clocks which is used by Cortex-M core.
> > > + So in this document define the needed property.
> > > +
> > > +properties:
> > > + compatible:
> > > + enum:
> > > + - fsl,imx7ulp-rpmsg
> > > + - fsl,imx8mn-rpmsg
> > > + - fsl,imx8mm-rpmsg
> > > + - fsl,imx8mp-rpmsg
> > > +
> > > + model:
> > > + $ref: /schemas/types.yaml#/definitions/string
> > > + description: User specified audio sound card name
> > > +
> > > + clocks:
> > > + items:
> > > + - description: Peripheral clock for register access
> > > + - description: Master clock
> > > + - description: DMA clock for DMA register access
> > > + - description: Parent clock for multiple of 8kHz sample rates
> > > + - description: Parent clock for multiple of 11kHz sample rates
> > > + minItems: 5
> >
> > If this doesn't touch hardware, what are these clocks for?
>
> When the cortex-M core support audio service, these clock
> needed prepared & enabled by ALSA driver.
>
> >
> > You don't need 'minItems' unless it's less than the number of 'items'.
>
> Ok, I will remove this minItems.
>
> >
> > > +
> > > + clock-names:
> > > + items:
> > > + - const: ipg
> > > + - const: mclk
> > > + - const: dma
> > > + - const: pll8k
> > > + - const: pll11k
> > > + minItems: 5
> > > +
> > > + power-domains:
> > > + maxItems: 1
> > > +
> > > + fsl,audioindex:
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + enum: [0, 1]
> > > + default: 0
> > > + description: Instance index for sound card in
> > > + M core side, which share one rpmsg
> > > + channel.
> >
> > We don't do indexes in DT. What's this numbering tied to?
>
> I will remove it. it is not necessary
>
> >
> > > +
> > > + fsl,version:
> >
> > version of what?
> >
> > This seems odd at best.
> >
>
> I will remove it. it is not necessary
>
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + enum: [1, 2]
> >
> > You're going to update this with every new firmware version?
> >
> > > + default: 2
> > > + description: The version of M core image, which is
> > > + to make driver compatible with different image.
> > > +
> > > + fsl,buffer-size:
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + description: pre allocate dma buffer size
> >
> > How can you have DMA, this doesn't touch h/w?
>
> The DMA is handled by M core image for sound playback
> and capture. we need to allocated buffer in Linux side.
> here just make the buffer size to be configurable.
Do we set audio buffer sizes for other audio devices in DT? If not,
why is this special? If so, why is it not common.
> > > + fsl,enable-lpa:
> > > + $ref: /schemas/types.yaml#/definitions/flag
> > > + description: enable low power audio path.
> > > +
> > > + fsl,rpmsg-out:
> > > + $ref: /schemas/types.yaml#/definitions/flag
> > > + description: |
> > > + This is a boolean property. If present, the transmitting function
> > > + will be enabled.
> > > +
> > > + fsl,rpmsg-in:
> > > + $ref: /schemas/types.yaml#/definitions/flag
> > > + description: |
> > > + This is a boolean property. If present, the receiving function
> > > + will be enabled.
> > > +
> > > + fsl,codec-type:
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + enum: [0, 1, 2]
> > > + default: 0
> > > + description: Sometimes the codec is registered by
> > > + driver not by the device tree, this items
> > > + can be used to distinguish codecs.
> >
> > How does one decide what value to use?
>
> I will add more description:
> 0: dummy codec
> 1: WM8960 codec
> 2: AK4497 codec
I assume the last 2 cases have nodes in DT (pointed to by
'audio-codec'), so this is redundant.
> > > +
> > > + audio-codec:
> > > + $ref: /schemas/types.yaml#/definitions/phandle
> > > + description: The phandle of the audio codec
> >
> > The codec is controlled from the Linux side?
>
> yes.
>
> >
> > > +
> > > + memory-region:
> > > + $ref: /schemas/types.yaml#/definitions/phandle
> > > + description: phandle to the reserved memory nodes
> > > +
> > > +required:
> > > + - compatible
> > > + - fsl,audioindex
> > > + - fsl,version
> > > + - fsl,buffer-size
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > + - |
> > > + rpmsg_audio: rpmsg_audio {
> > > + compatible = "fsl,imx8mn-rpmsg";
> > > + fsl,audioindex = <0> ;
> > > + fsl,version = <2>;
> > > + fsl,buffer-size = <0x6000000>;
> > > + fsl,enable-lpa;
> >
> > How does this work? Don't you need somewhere to put the 'rpmsg' data?
>
> The rpmsg data is not handled in this "rpmsg_audio" device, it is just to
> prepare the resource for rpmsg audio function, the clock, the memory
> the power...
>
> The rpmsg data is handled in imx-pcm-rpmsg.c and imx-audio-rpmsg.c
> These devices is registered by imx remoteproc driver.
Then what is 'memory-region' for? You need that, a mailbox, or ???
somewhere in DT.
Rob
^ permalink raw reply
* Re: [PATCH v4 13/14] dt-bindings: of: Add restricted DMA pool
From: Rob Herring @ 2021-03-10 21:40 UTC (permalink / raw)
To: Will Deacon
Cc: Heikki Krogerus, Peter Zijlstra, Grant Likely, Paul Mackerras,
Frank Rowand, Ingo Molnar, Marek Szyprowski, Stefano Stabellini,
Saravana Kannan, Joerg Roedel, Rafael J . Wysocki,
Christoph Hellwig, Bartosz Golaszewski, xen-devel, Thierry Reding,
linux-devicetree, Konrad Rzeszutek Wilk, Dan Williams,
linuxppc-dev, Nicolas Boichat, Claire Chang, Boris Ostrovsky,
Andy Shevchenko, Juergen Gross, Greg KH, Randy Dunlap, lkml,
list@263.net:IOMMU DRIVERS, Jim Quinlan, Heinrich Schuchardt,
Robin Murphy, Thiago Jung Bauermann
In-Reply-To: <20210310160747.GA29834@willie-the-truck>
On Wed, Mar 10, 2021 at 9:08 AM Will Deacon <will@kernel.org> wrote:
>
> Hi Claire,
>
> On Tue, Feb 09, 2021 at 02:21:30PM +0800, Claire Chang wrote:
> > Introduce the new compatible string, restricted-dma-pool, for restricted
> > DMA. One can specify the address and length of the restricted DMA memory
> > region by restricted-dma-pool in the reserved-memory node.
> >
> > Signed-off-by: Claire Chang <tientzu@chromium.org>
> > ---
> > .../reserved-memory/reserved-memory.txt | 24 +++++++++++++++++++
> > 1 file changed, 24 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > index e8d3096d922c..fc9a12c2f679 100644
> > --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > @@ -51,6 +51,20 @@ compatible (optional) - standard definition
> > used as a shared pool of DMA buffers for a set of devices. It can
> > be used by an operating system to instantiate the necessary pool
> > management subsystem if necessary.
> > + - restricted-dma-pool: This indicates a region of memory meant to be
> > + used as a pool of restricted DMA buffers for a set of devices. The
> > + memory region would be the only region accessible to those devices.
> > + When using this, the no-map and reusable properties must not be set,
> > + so the operating system can create a virtual mapping that will be used
> > + for synchronization. The main purpose for restricted DMA is to
> > + mitigate the lack of DMA access control on systems without an IOMMU,
> > + which could result in the DMA accessing the system memory at
> > + unexpected times and/or unexpected addresses, possibly leading to data
> > + leakage or corruption. The feature on its own provides a basic level
> > + of protection against the DMA overwriting buffer contents at
> > + unexpected times. However, to protect against general data leakage and
> > + system memory corruption, the system needs to provide way to lock down
> > + the memory access, e.g., MPU.
>
> As far as I can tell, these pools work with both static allocations (which
> seem to match your use-case where firmware has preconfigured the DMA ranges)
> but also with dynamic allocations where a 'size' property is present instead
> of the 'reg' property and the kernel is responsible for allocating the
> reservation during boot. Am I right and, if so, is that deliberate?
I believe so. I'm not keen on having size only reservations in DT.
Yes, we allowed that already, but that's back from the days of needing
large CMA carveouts to be reserved early in boot. I've read that the
kernel is much better now at contiguous allocations, so do we really
need this in DT anymore?
> I ask because I think that would potentially be useful to us for the
> Protected KVM work, where we need to bounce virtio memory accesses via
> guest-determined windows because the guest memory is generally inaccessible
> to the host. We've been hacking this using a combination of "swiotlb=force"
> and set_memory_{decrypted,encrypted}() but it would be much better to
> leverage the stuff you have here.
>
> Also:
>
> > +
> > + restricted_dma_mem_reserved: restricted_dma_mem_reserved {
> > + compatible = "restricted-dma-pool";
> > + reg = <0x50000000 0x400000>;
> > + };
> > };
> >
> > /* ... */
> > @@ -138,4 +157,9 @@ one for multimedia processing (named multimedia-memory@77000000, 64MiB).
> > memory-region = <&multimedia_reserved>;
> > /* ... */
> > };
> > +
> > + pcie_device: pcie_device@0,0 {
> > + memory-region = <&restricted_dma_mem_reserved>;
> > + /* ... */
> > + };
>
> I find this example a bit weird, as I didn't think we usually had DT nodes
> for PCI devices; rather they are discovered as a result of probing config
> space. Is the idea that you have one reserved memory region attached to the
> RC and all the PCI devices below that share the region, or is there a need
> for a mapping mechanism?
We can have DT nodes for PCI. AIUI, IBM power systems always do. For
FDT, it's only if there are extra non-discoverable resources. It's
particularly fun when it's resources which need to be enabled for the
PCI device to be discovered. That seems to be a growing problem as PCI
becomes more common on embedded systems.
Rob
^ permalink raw reply
* Re: [PATCH v2 01/15] powerpc/uaccess: Remove __get_user_allowed() and unsafe_op_wrap()
From: Daniel Axtens @ 2021-03-10 21:47 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <439179c5e54c18f2cb8bdf1eea13ea0ef6b98375.1615398265.git.christophe.leroy@csgroup.eu>
Hi Christophe,
Thanks for the answers to my questions on v1.
This all looks good to me.
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
> Those two macros have only one user which is unsafe_get_user().
>
> Put everything in one place and remove them.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/include/asm/uaccess.h | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
> index 78e2a3990eab..8cbf3e3874f1 100644
> --- a/arch/powerpc/include/asm/uaccess.h
> +++ b/arch/powerpc/include/asm/uaccess.h
> @@ -53,9 +53,6 @@ static inline bool __access_ok(unsigned long addr, unsigned long size)
> #define __put_user(x, ptr) \
> __put_user_nocheck((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
>
> -#define __get_user_allowed(x, ptr) \
> - __get_user_nocheck((x), (ptr), sizeof(*(ptr)), false)
> -
> #define __get_user_inatomic(x, ptr) \
> __get_user_nosleep((x), (ptr), sizeof(*(ptr)))
> #define __put_user_inatomic(x, ptr) \
> @@ -482,8 +479,11 @@ user_write_access_begin(const void __user *ptr, size_t len)
> #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_get_user(x, p, e) do { \
> + if (unlikely(__get_user_nocheck((x), (p), sizeof(*(p)), false)))\
> + goto e; \
> +} while (0)
> +
> #define unsafe_put_user(x, p, e) \
> __unsafe_put_user_goto((__typeof__(*(p)))(x), (p), sizeof(*(p)), e)
>
> --
> 2.25.0
^ permalink raw reply
* Re: Errant readings on LM81 with T2080 SoC
From: Chris Packham @ 2021-03-10 21:48 UTC (permalink / raw)
To: Guenter Roeck, jdelvare@suse.com
Cc: linux-hwmon@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org, linux-i2c@vger.kernel.org
In-Reply-To: <d5045879-45aa-db38-e6aa-4c8ea3e62f6c@roeck-us.net>
On 10/03/21 6:06 pm, Guenter Roeck wrote:
> On 3/9/21 6:19 PM, Chris Packham wrote:
>> On 9/03/21 9:27 am, Chris Packham wrote:
>>> On 8/03/21 5:59 pm, Guenter Roeck wrote:
>>>> Other than that, the only other real idea I have would be to monitor
>>>> the i2c bus.
>>> I am in the fortunate position of being able to go into the office and
>>> even happen to have the expensive scope at the moment. Now I just need
>>> to find a tame HW engineer so I don't burn myself trying to attach the
>>> probes.
>> One thing I see on the scope is that when there is a CPU load there
>> appears to be some clock stretching going on (SCL is held low some
>> times). I don't see it without the CPU load. It's hard to correlate a
>> clock stretching event with a bad read or error but it is one area where
>> the SMBUS spec has a maximum that might cause the device to give up waiting.
>>
> Do you have CONFIG_PREEMPT enabled in your kernel ? But even without
> that it is possible that the hot loops at the beginning and end of
> each operation mess up the driver and cause it to sleep longer
> than intended. Did you try usleep_range() ?
I've been running with and without CONFIG_PREEMPT. The failures happen
with both.
I did try usleep_range() and still saw failures.
> On a side note, can you send me a register dump for the lm81 ?
> It would be useful for my module test code.
Here you go this is from a largely unconfigured LM81
0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef
00: 47 47 47 47 47 47 47 47 47 47 47 47 47 47 47 47 GGGGGGGGGGGGGGGG
10: 47 81 24 03 94 00 00 00 00 ff ff ff ff ff ff ff G?$??...........
20: bf cb c1 00 c0 47 ec 24 ff ff 65 ff 00 ff 00 ff ???.?G?$..e.....
30: 00 ff 00 ff 00 ff 00 71 a9 7f 7f ff ff 58 01 04 .......q???..X??
40: 01 08 00 00 00 00 00 50 2f 80 80 01 44 00 00 00 ??.....P/???D...
50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
90: 00 81 24 03 94 00 00 00 00 ff ff ff ff ff ff ff .?$??...........
a0: bf cb c1 00 c0 47 ec 24 ff ff 65 ff 00 ff 00 ff ???.?G?$..e.....
b0: 00 ff 00 ff 00 ff 00 71 a9 7f 7f ff ff 58 01 04 .......q???..X??
c0: 01 00 00 00 00 00 00 50 2f 80 80 01 44 00 00 00 ?......P/???D...
d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
This is from a LM81 that's been configured by our application SW with
limits appropriate for the platform.
0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef
00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................
10: ff 81 24 03 94 00 00 00 00 ff ff ff ff ff ff ff ..$.............
20: bf cc c1 00 c0 47 ec 1c ff ff 65 dc b4 ff c0 d3 .....G....e.....
30: ad ff 00 d3 ad 4e 40 71 a9 4b 46 ff ff 58 01 04 .....N@q.KF..X..
40: 01 08 00 00 00 00 00 f0 2f 80 80 81 44 80 80 80 ......../...D...
50: 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 ................
60: 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 ................
70: 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 ................
80: 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 ................
90: 80 81 24 03 94 00 00 00 00 ff ff ff ff ff ff ff ..$.............
a0: bf cc c1 00 c0 47 ec 1c ff ff 65 dc b4 ff c0 d3 .....G....e.....
b0: ad ff 00 d3 ad 4e 40 71 a9 4b 46 ff ff 58 01 04 .....N@q.KF..X..
c0: 01 00 00 00 00 00 00 f0 2f 80 80 81 44 80 80 80 ......../...D...
d0: 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 ................
e0: 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 ................
f0: 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 ................
^ permalink raw reply
* [PATCH] rpadlpar: fix potential drc_name corruption in store functions
From: Tyrel Datwyler @ 2021-03-10 22:30 UTC (permalink / raw)
To: bhelgaas; +Cc: linux-pci, mmc, linuxppc-dev, linux-kernel, Tyrel Datwyler
Both add_slot_store() and remove_slot_store() try to fix up the drc_name
copied from the store buffer by placing a NULL terminator at nbyte + 1
or in place of a '\n' if present. However, the static buffer that we
copy the drc_name data into is not zeored and can contain anything past
the n-th byte. This is problematic if a '\n' byte appears in that buffer
after nbytes and the string copied into the store buffer was not NULL
terminated to start with as the strchr() search for a '\n' byte will mark
this incorrectly as the end of the drc_name string resulting in a drc_name
string that contains garbage data after the n-th byte. The following
debugging shows an example of the drmgr utility writing "PHB 4543" to
the add_slot sysfs attribute, but add_slot_store logging a corrupted
string value.
[135823.702864] drmgr: drmgr: -c phb -a -s PHB 4543 -d 1
[135823.702879] add_slot_store: drc_name = PHB 4543°|<82>!, rc = -19
Fix this by NULL terminating the string when we copy it into our static
buffer by coping nbytes + 1 of data from the store buffer. The code has
already made sure that nbytes is not >= MAX_DRC_NAME_LEN and the store
buffer is guaranteed to be zeroed beyond the nth-byte of data copied
from the user. Further, since the string is now NULL terminated the code
only needs to change '\n' to '\0' when present.
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/pci/hotplug/rpadlpar_sysfs.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/pci/hotplug/rpadlpar_sysfs.c b/drivers/pci/hotplug/rpadlpar_sysfs.c
index cdbfa5df3a51..375087921284 100644
--- a/drivers/pci/hotplug/rpadlpar_sysfs.c
+++ b/drivers/pci/hotplug/rpadlpar_sysfs.c
@@ -34,12 +34,11 @@ static ssize_t add_slot_store(struct kobject *kobj, struct kobj_attribute *attr,
if (nbytes >= MAX_DRC_NAME_LEN)
return 0;
- memcpy(drc_name, buf, nbytes);
+ memcpy(drc_name, buf, nbytes + 1);
end = strchr(drc_name, '\n');
- if (!end)
- end = &drc_name[nbytes];
- *end = '\0';
+ if (end)
+ *end = '\0';
rc = dlpar_add_slot(drc_name);
if (rc)
@@ -65,12 +64,11 @@ static ssize_t remove_slot_store(struct kobject *kobj,
if (nbytes >= MAX_DRC_NAME_LEN)
return 0;
- memcpy(drc_name, buf, nbytes);
+ memcpy(drc_name, buf, nbytes + 1);
end = strchr(drc_name, '\n');
- if (!end)
- end = &drc_name[nbytes];
- *end = '\0';
+ if (end)
+ *end = '\0';
rc = dlpar_remove_slot(drc_name);
if (rc)
--
2.27.0
^ permalink raw reply related
* Re: [PATCH v2 03/15] powerpc/align: Convert emulate_spe() to user_access_begin
From: Daniel Axtens @ 2021-03-10 22:31 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <0ad4629c2d222019e82fcdfccc70d372beb4adf9.1615398265.git.christophe.leroy@csgroup.eu>
Hi Christophe,
> This patch converts emulate_spe() to using user_access_being
s/being/begin/ :)
> logic.
>
> Since commit 662bbcb2747c ("mm, sched: Allow uaccess in atomic with
> pagefault_disable()"), might_fault() doesn't fire when called from
> sections where pagefaults are disabled, which must be the case
> when using _inatomic variants of __get_user and __put_user. So
> the might_fault() in user_access_begin() is not a problem.
(likewise with the might_fault() in __get_user_nocheck, called from
unsafe_get_user())
> There was a verification of user_mode() together with the access_ok(),
> but the function returns in case !user_mode() immediately after
> the access_ok() verification, so removing that test has no effect.
I agree that removing the test is safe.
> - /* Verify the address of the operand */
> - if (unlikely(user_mode(regs) &&
> - !access_ok(addr, nb)))
> - return -EFAULT;
> -
I found the reasoning a bit confusing: I think it's safe to remove
because:
- we have the usermode check immediately following it:
> /* userland only */
> if (unlikely(!user_mode(regs)))
> return 0;
- and then we have the access_ok() check as part of
user_read_access_begin later on in the function:
> + if (!user_read_access_begin(addr, nb))
> + return -EFAULT;
> +
> switch (nb) {
> case 8:
> - ret |= __get_user_inatomic(temp.v[0], p++);
> - ret |= __get_user_inatomic(temp.v[1], p++);
> - ret |= __get_user_inatomic(temp.v[2], p++);
> - ret |= __get_user_inatomic(temp.v[3], p++);
> + unsafe_get_user(temp.v[0], p++, Efault_read);
> + unsafe_get_user(temp.v[1], p++, Efault_read);
> + unsafe_get_user(temp.v[2], p++, Efault_read);
> + unsafe_get_user(temp.v[3], p++, Efault_read);
This will bail early rather than trying every possible read. I think
that's OK. I can't think of a situation where we could fail to read the
first byte and then successfully read later bytes, for example. Also I
can't think of a sane way userspace could depend on that behaviour. So I
agree with this change (and the change to the write path).
> fallthrough;
> case 4:
> - ret |= __get_user_inatomic(temp.v[4], p++);
> - ret |= __get_user_inatomic(temp.v[5], p++);
> + unsafe_get_user(temp.v[4], p++, Efault_read);
> + unsafe_get_user(temp.v[5], p++, Efault_read);
> fallthrough;
> case 2:
> - ret |= __get_user_inatomic(temp.v[6], p++);
> - ret |= __get_user_inatomic(temp.v[7], p++);
> - if (unlikely(ret))
> - return -EFAULT;
> + unsafe_get_user(temp.v[6], p++, Efault_read);
> + unsafe_get_user(temp.v[7], p++, Efault_read);
> }
> + user_read_access_end();
>
> switch (instr) {
> case EVLDD:
> @@ -255,31 +250,41 @@ static int emulate_spe(struct pt_regs *regs, unsigned int reg,
>
> /* Store result to memory or update registers */
> if (flags & ST) {
> - ret = 0;
> p = addr;
> +
> + if (!user_read_access_begin(addr, nb))
That should be a user_write_access_begin.
> + return -EFAULT;
> +
>
> return 1;
> +
> +Efault_read:
Checkpatch complains that this is CamelCase, which seems like a
checkpatch problem. Efault_{read,write} seem like good labels to me.
(You don't need to change anything, I just like to check the checkpatch
results when reviewing a patch.)
> + user_read_access_end();
> + return -EFAULT;
> +
> +Efault_write:
> + user_write_access_end();
> + return -EFAULT;
> }
> #endif /* CONFIG_SPE */
>
With the user_write_access_begin change:
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
^ permalink raw reply
* Re: [PATCH v2 04/15] powerpc/uaccess: Remove __get/put_user_inatomic()
From: Daniel Axtens @ 2021-03-10 22:37 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1e5c895669e8d54a7810b62dc61eb111f33c2c37.1615398265.git.christophe.leroy@csgroup.eu>
Hi Christophe,
> Powerpc is the only architecture having _inatomic variants of
> __get_user() and __put_user() accessors. They were introduced
> by commit e68c825bb016 ("[POWERPC] Add inatomic versions of __get_user
> and __put_user").
>
> Those variants expand to the _nosleep macros instead of expanding
> to the _nocheck macros. The only difference between the _nocheck
> and the _nosleep macros is the call to might_fault().
>
> Since commit 662bbcb2747c ("mm, sched: Allow uaccess in atomic with
> pagefault_disable()"), __get/put_user() can be used in atomic parts
> of the code, therefore __get/put_user_inatomic() have become useless.
>
> Remove __get_user_inatomic() and __put_user_inatomic().
>
This makes much more sense, thank you!
Simplifying uaccess.h is always good to me :)
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/include/asm/uaccess.h | 37 -------------------
> .../kernel/hw_breakpoint_constraints.c | 2 +-
> arch/powerpc/kernel/traps.c | 2 +-
> 3 files changed, 2 insertions(+), 39 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
> index a08c482b1315..01aea0df4dd0 100644
> --- a/arch/powerpc/include/asm/uaccess.h
> +++ b/arch/powerpc/include/asm/uaccess.h
> @@ -53,11 +53,6 @@ static inline bool __access_ok(unsigned long addr, unsigned long size)
> #define __put_user(x, ptr) \
> __put_user_nocheck((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
>
> -#define __get_user_inatomic(x, ptr) \
> - __get_user_nosleep((x), (ptr), sizeof(*(ptr)))
> -#define __put_user_inatomic(x, ptr) \
> - __put_user_nosleep((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
> -
> #ifdef CONFIG_PPC64
>
> #define ___get_user_instr(gu_op, dest, ptr) \
> @@ -92,9 +87,6 @@ static inline bool __access_ok(unsigned long addr, unsigned long size)
> #define __get_user_instr(x, ptr) \
> ___get_user_instr(__get_user, x, ptr)
>
> -#define __get_user_instr_inatomic(x, ptr) \
> - ___get_user_instr(__get_user_inatomic, x, ptr)
> -
> extern long __put_user_bad(void);
>
> #define __put_user_size(x, ptr, size, retval) \
> @@ -141,20 +133,6 @@ __pu_failed: \
> __pu_err; \
> })
>
> -#define __put_user_nosleep(x, ptr, size) \
> -({ \
> - long __pu_err; \
> - __typeof__(*(ptr)) __user *__pu_addr = (ptr); \
> - __typeof__(*(ptr)) __pu_val = (x); \
> - __typeof__(size) __pu_size = (size); \
> - \
> - __chk_user_ptr(__pu_addr); \
> - __put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \
> - \
> - __pu_err; \
> -})
> -
> -
> /*
> * We don't tell gcc that we are accessing memory, but this is OK
> * because we do not write to any memory gcc knows about, so there
> @@ -320,21 +298,6 @@ do { \
> __gu_err; \
> })
>
> -#define __get_user_nosleep(x, ptr, size) \
> -({ \
> - long __gu_err; \
> - __long_type(*(ptr)) __gu_val; \
> - __typeof__(*(ptr)) __user *__gu_addr = (ptr); \
> - __typeof__(size) __gu_size = (size); \
> - \
> - __chk_user_ptr(__gu_addr); \
> - __get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
> - (x) = (__force __typeof__(*(ptr)))__gu_val; \
> - \
> - __gu_err; \
> -})
> -
> -
> /* more complex routines */
>
> extern unsigned long __copy_tofrom_user(void __user *to,
> diff --git a/arch/powerpc/kernel/hw_breakpoint_constraints.c b/arch/powerpc/kernel/hw_breakpoint_constraints.c
> index 867ee4aa026a..675d1f66ab72 100644
> --- a/arch/powerpc/kernel/hw_breakpoint_constraints.c
> +++ b/arch/powerpc/kernel/hw_breakpoint_constraints.c
> @@ -141,7 +141,7 @@ void wp_get_instr_detail(struct pt_regs *regs, struct ppc_inst *instr,
> {
> struct instruction_op op;
>
> - if (__get_user_instr_inatomic(*instr, (void __user *)regs->nip))
> + if (__get_user_instr(*instr, (void __user *)regs->nip))
> return;
>
> analyse_instr(&op, regs, *instr);
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index 1583fd1c6010..1fa36bd08efe 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -864,7 +864,7 @@ static void p9_hmi_special_emu(struct pt_regs *regs)
> unsigned long ea, msr, msr_mask;
> bool swap;
>
> - if (__get_user_inatomic(instr, (unsigned int __user *)regs->nip))
> + if (__get_user(instr, (unsigned int __user *)regs->nip))
> return;
>
> /*
> --
> 2.25.0
^ permalink raw reply
* Re: PowerPC64 future proof kernel toc, revised for lld
From: Alan Modra @ 2021-03-10 23:41 UTC (permalink / raw)
To: Christophe Leroy; +Cc: alexey, Alexey Kardashevskiy, linuxppc-dev, ellerman
In-Reply-To: <df863fb6-2fd6-00d7-b6f3-94a49c2a5405@csgroup.eu>
On Wed, Mar 10, 2021 at 01:44:57PM +0100, Christophe Leroy wrote:
>
>
> Le 10/03/2021 à 13:25, Alan Modra a écrit :
> > On Wed, Mar 10, 2021 at 08:33:37PM +1100, Alexey Kardashevskiy wrote:
> > > One more question - the older version had a construct "DEFINED (.TOC.) ?
> > > .TOC. : ..." in case .TOC. is not defined (too old ld? too old gcc?) but the
> > > newer patch seems assuming it is always defined, when was it added? I have
> > > the same check in SLOF, for example, do I still need it?
> >
> > .TOC. symbol support was first added 2012-11-06, so you need
> > binutils-2.24 or later to use .TOC. as a symbol.
> >
>
> As of today, minimum requirement to build kernel is binutils 2.23, see https://www.kernel.org/doc/html/latest/process/changes.html#current-minimal-requirements
Yes, and arch/powerpc/Makefile complains about 2.24. So for powerpc
that means you need to go to at least 2.25. Oh the horror of needing
such new tools!
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply
* [PATCH] ibmvfc: free channel_setup_buf during device tear down
From: Tyrel Datwyler @ 2021-03-11 1:22 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
The buffer for negotiating channel setup is DMA allocated at device
probe time. However, the remove path fails to free this allocation which
will prevent the hypervisor from releasing the virtual device in the
case of a hotplug remove.
Fix this issue by freeing the buffer allocation in ibmvfc_free_mem().
Fixes: e95eef3fc0bc ("scsi: ibmvfc: Implement channel enquiry and setup commands")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index e663085a8944..76531eec49de 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -5770,6 +5770,8 @@ static void ibmvfc_free_mem(struct ibmvfc_host *vhost)
vhost->disc_buf_dma);
dma_free_coherent(vhost->dev, sizeof(*vhost->login_buf),
vhost->login_buf, vhost->login_buf_dma);
+ dma_free_coherent(vhost->dev, sizeof(*vhost->channel_setup_buf),
+ vhost->channel_setup_buf, vhost->channel_setup_dma);
dma_pool_destroy(vhost->sg_pool);
ibmvfc_free_queue(vhost, async_q);
LEAVE;
--
2.27.0
^ permalink raw reply related
* [powerpc:fixes-test] BUILD SUCCESS bd73758803c2eedc037c2268b65a19542a832594
From: kernel test robot @ 2021-03-11 1:36 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes-test
branch HEAD: bd73758803c2eedc037c2268b65a19542a832594 powerpc: Fix missing declaration of [en/dis]able_kernel_vsx()
elapsed time: 729m
configs tested: 131
configs skipped: 42
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
sparc allyesconfig
mips db1xxx_defconfig
powerpc tqm8540_defconfig
powerpc ppc44x_defconfig
m68k hp300_defconfig
powerpc adder875_defconfig
powerpc mpc8313_rdb_defconfig
powerpc ppc40x_defconfig
mips pistachio_defconfig
powerpc mpc837x_rdb_defconfig
powerpc pq2fads_defconfig
powerpc obs600_defconfig
powerpc holly_defconfig
powerpc kmeter1_defconfig
riscv rv32_defconfig
arm pxa_defconfig
powerpc tqm8541_defconfig
sh polaris_defconfig
powerpc ep88xc_defconfig
mips rbtx49xx_defconfig
powerpc mpc512x_defconfig
ia64 allmodconfig
sh sh7770_generic_defconfig
i386 defconfig
arm ezx_defconfig
powerpc eiger_defconfig
ia64 zx1_defconfig
sh se7750_defconfig
arm eseries_pxa_defconfig
sh alldefconfig
powerpc katmai_defconfig
m68k m5275evb_defconfig
sh dreamcast_defconfig
microblaze defconfig
powerpc mpc836x_mds_defconfig
sh lboxre2_defconfig
arm xcep_defconfig
arc tb10x_defconfig
arm ep93xx_defconfig
powerpc stx_gp3_defconfig
arm pxa910_defconfig
powerpc icon_defconfig
ia64 defconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
s390 allyesconfig
s390 allmodconfig
parisc allyesconfig
s390 defconfig
i386 allyesconfig
sparc defconfig
i386 tinyconfig
mips allyesconfig
mips allmodconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
i386 randconfig-a005-20210309
i386 randconfig-a003-20210309
i386 randconfig-a002-20210309
i386 randconfig-a006-20210309
i386 randconfig-a004-20210309
i386 randconfig-a001-20210309
x86_64 randconfig-a013-20210309
x86_64 randconfig-a016-20210309
x86_64 randconfig-a015-20210309
x86_64 randconfig-a014-20210309
x86_64 randconfig-a011-20210309
x86_64 randconfig-a012-20210309
x86_64 randconfig-a011-20210310
x86_64 randconfig-a016-20210310
x86_64 randconfig-a013-20210310
x86_64 randconfig-a015-20210310
x86_64 randconfig-a014-20210310
x86_64 randconfig-a012-20210310
i386 randconfig-a016-20210309
i386 randconfig-a012-20210309
i386 randconfig-a014-20210309
i386 randconfig-a013-20210309
i386 randconfig-a011-20210309
i386 randconfig-a015-20210309
x86_64 randconfig-a006-20210308
x86_64 randconfig-a001-20210308
x86_64 randconfig-a004-20210308
x86_64 randconfig-a002-20210308
x86_64 randconfig-a005-20210308
x86_64 randconfig-a003-20210308
riscv nommu_k210_defconfig
riscv allyesconfig
riscv nommu_virt_defconfig
riscv allnoconfig
riscv defconfig
riscv allmodconfig
x86_64 allyesconfig
x86_64 rhel-7.6-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 rhel-8.3-kbuiltin
x86_64 kexec
clang tested configs:
x86_64 randconfig-a013-20210308
x86_64 randconfig-a016-20210308
x86_64 randconfig-a015-20210308
x86_64 randconfig-a014-20210308
x86_64 randconfig-a011-20210308
x86_64 randconfig-a012-20210308
x86_64 randconfig-a006-20210309
x86_64 randconfig-a001-20210309
x86_64 randconfig-a004-20210309
x86_64 randconfig-a002-20210309
x86_64 randconfig-a005-20210309
x86_64 randconfig-a003-20210309
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ 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