* [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 11/15] powerpc/uaccess: Split out __get_user_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>
One part of __get_user_nocheck() is used for __get_user(),
the other part for unsafe_get_user().
Move the part dedicated to unsafe_get_user() in it.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/uaccess.h | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index a8c683695ec7..678651a615c3 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -49,7 +49,7 @@ static inline bool __access_ok(unsigned long addr, unsigned long size)
__put_user_check((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
#define __get_user(x, ptr) \
- __get_user_nocheck((x), (ptr), sizeof(*(ptr)), true)
+ __get_user_nocheck((x), (ptr), sizeof(*(ptr)))
#define __put_user(x, ptr) \
__put_user_nocheck((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
@@ -216,19 +216,15 @@ do { \
#define __long_type(x) \
__typeof__(__builtin_choose_expr(sizeof(x) > sizeof(0UL), 0ULL, 0UL))
-#define __get_user_nocheck(x, ptr, size, do_allow) \
+#define __get_user_nocheck(x, ptr, size) \
({ \
long __gu_err; \
__long_type(*(ptr)) __gu_val; \
__typeof__(*(ptr)) __user *__gu_addr = (ptr); \
__typeof__(size) __gu_size = (size); \
\
- if (do_allow) { \
- might_fault(); \
- __get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
- } else { \
- __get_user_size_allowed(__gu_val, __gu_addr, __gu_size, __gu_err); \
- } \
+ might_fault(); \
+ __get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
(x) = (__typeof__(*(ptr)))__gu_val; \
\
__gu_err; \
@@ -386,8 +382,14 @@ 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 { \
- if (unlikely(__get_user_nocheck((x), (p), sizeof(*(p)), false)))\
- goto e; \
+ 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; \
+ (x) = (__typeof__(*(p)))__gu_val; \
} while (0)
#define unsafe_put_user(x, p, e) \
--
2.25.0
^ permalink raw reply related
* [PATCH v2 10/15] powerpc/uaccess: Remove calls to __get_user_bad() and __put_user_bad()
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_bad() and __put_user_bad() are functions that are
declared but not defined, in order to make the link fail in
case they are called.
Nowadays, we have BUILD_BUG() and BUILD_BUG_ON() for that, and
they have the advantage to break the build earlier as it breaks
it at compile time instead of link time.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/uaccess.h | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index a9f2639ca3a8..a8c683695ec7 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -53,8 +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)))
-extern long __put_user_bad(void);
-
#define __put_user_size(x, ptr, size, retval) \
do { \
__label__ __pu_failed; \
@@ -136,12 +134,10 @@ do { \
case 2: __put_user_asm_goto(x, __pus_addr, label, "sth"); break; \
case 4: __put_user_asm_goto(x, __pus_addr, label, "stw"); break; \
case 8: __put_user_asm2_goto(x, __pus_addr, label); break; \
- default: __put_user_bad(); \
+ default: BUILD_BUG(); \
} \
} while (0)
-extern long __get_user_bad(void);
-
/*
* This does an atomic 128 byte aligned load from userspace.
* Upto caller to do enable_kernel_vmx() before calling!
@@ -196,14 +192,13 @@ extern long __get_user_bad(void);
#define __get_user_size_allowed(x, ptr, size, retval) \
do { \
retval = 0; \
- if (size > sizeof(x)) \
- (x) = __get_user_bad(); \
+ BUILD_BUG_ON(size > sizeof(x)); \
switch (size) { \
case 1: __get_user_asm(x, (u8 __user *)ptr, retval, "lbz"); break; \
case 2: __get_user_asm(x, (u16 __user *)ptr, retval, "lhz"); break; \
case 4: __get_user_asm(x, (u32 __user *)ptr, retval, "lwz"); break; \
case 8: __get_user_asm2(x, (u64 __user *)ptr, retval); break; \
- default: (x) = __get_user_bad(); \
+ default: BUILD_BUG(); \
} \
} while (0)
--
2.25.0
^ permalink raw reply related
* [PATCH v2 09/15] powerpc/uaccess: Remove __chk_user_ptr() in __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>
Commit d02f6b7dab82 ("powerpc/uaccess: Evaluate macro arguments once,
before user access is allowed") changed the __chk_user_ptr()
argument from the passed ptr pointer to the locally
declared __gu_addr. But __gu_addr is locally defined as __user
so the check is pointless.
During kernel build __chk_user_ptr() voids and is only evaluated
during sparse checks so it should have been armless to leave the
original pointer check there.
Nevertheless, this check is indeed redundant with the assignment
above which casts the ptr pointer to the local __user __gu_addr.
In case of mismatch, sparse will detect it there, so the
__check_user_ptr() is not needed anywhere else than in access_ok().
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/uaccess.h | 3 ---
1 file changed, 3 deletions(-)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index a6d3563cf3ee..a9f2639ca3a8 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -78,7 +78,6 @@ __pu_failed: \
__typeof__(size) __pu_size = (size); \
\
might_fault(); \
- __chk_user_ptr(__pu_addr); \
__put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \
\
__pu_err; \
@@ -197,7 +196,6 @@ extern long __get_user_bad(void);
#define __get_user_size_allowed(x, ptr, size, retval) \
do { \
retval = 0; \
- __chk_user_ptr(ptr); \
if (size > sizeof(x)) \
(x) = __get_user_bad(); \
switch (size) { \
@@ -230,7 +228,6 @@ do { \
__typeof__(*(ptr)) __user *__gu_addr = (ptr); \
__typeof__(size) __gu_size = (size); \
\
- __chk_user_ptr(__gu_addr); \
if (do_allow) { \
might_fault(); \
__get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
--
2.25.0
^ permalink raw reply related
* [PATCH v2 08/15] powerpc/uaccess: Remove __unsafe_put_user_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>
__unsafe_put_user_goto() is just an intermediate layer to
__put_user_size_goto() without added value other than doing
the __user pointer type checking.
Do the __user pointer type checking in __put_user_size_goto()
and remove __unsafe_put_user_goto().
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/uaccess.h | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index c4bbc64758a0..a6d3563cf3ee 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -130,23 +130,17 @@ __pu_failed: \
#define __put_user_size_goto(x, ptr, size, label) \
do { \
+ __typeof__(*(ptr)) __user *__pus_addr = (ptr); \
+ \
switch (size) { \
- case 1: __put_user_asm_goto(x, ptr, label, "stb"); break; \
- case 2: __put_user_asm_goto(x, ptr, label, "sth"); break; \
- case 4: __put_user_asm_goto(x, ptr, label, "stw"); break; \
- case 8: __put_user_asm2_goto(x, ptr, label); break; \
+ case 1: __put_user_asm_goto(x, __pus_addr, label, "stb"); break; \
+ case 2: __put_user_asm_goto(x, __pus_addr, label, "sth"); break; \
+ case 4: __put_user_asm_goto(x, __pus_addr, label, "stw"); break; \
+ case 8: __put_user_asm2_goto(x, __pus_addr, label); break; \
default: __put_user_bad(); \
} \
} while (0)
-#define __unsafe_put_user_goto(x, ptr, size, label) \
-do { \
- __typeof__(*(ptr)) __user *__pu_addr = (ptr); \
- __chk_user_ptr(ptr); \
- __put_user_size_goto((x), __pu_addr, (size), label); \
-} while (0)
-
-
extern long __get_user_bad(void);
/*
@@ -405,7 +399,7 @@ user_write_access_begin(const void __user *ptr, size_t len)
} while (0)
#define unsafe_put_user(x, p, e) \
- __unsafe_put_user_goto((__typeof__(*(p)))(x), (p), sizeof(*(p)), e)
+ __put_user_size_goto((__typeof__(*(p)))(x), (p), sizeof(*(p)), e)
#define unsafe_copy_to_user(d, s, l, e) \
do { \
--
2.25.0
^ permalink raw reply related
* [PATCH v2 05/15] powerpc/uaccess: Move get_user_instr helpers in asm/inst.h
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>
Those helpers use get_user helpers but they don't participate
in their implementation, so they do not belong to asm/uaccess.h
Move them in asm/inst.h
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/inst.h | 34 ++++++++++++++++++++++++++++++
arch/powerpc/include/asm/uaccess.h | 34 ------------------------------
2 files changed, 34 insertions(+), 34 deletions(-)
diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
index cc73c1267572..19e18af2fac9 100644
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -4,6 +4,40 @@
#include <asm/ppc-opcode.h>
+#ifdef CONFIG_PPC64
+
+#define ___get_user_instr(gu_op, dest, ptr) \
+({ \
+ long __gui_ret = 0; \
+ unsigned long __gui_ptr = (unsigned long)ptr; \
+ struct ppc_inst __gui_inst; \
+ unsigned int __prefix, __suffix; \
+ __gui_ret = gu_op(__prefix, (unsigned int __user *)__gui_ptr); \
+ if (__gui_ret == 0) { \
+ if ((__prefix >> 26) == OP_PREFIX) { \
+ __gui_ret = gu_op(__suffix, \
+ (unsigned int __user *)__gui_ptr + 1); \
+ __gui_inst = ppc_inst_prefix(__prefix, \
+ __suffix); \
+ } else { \
+ __gui_inst = ppc_inst(__prefix); \
+ } \
+ if (__gui_ret == 0) \
+ (dest) = __gui_inst; \
+ } \
+ __gui_ret; \
+})
+#else /* !CONFIG_PPC64 */
+#define ___get_user_instr(gu_op, dest, ptr) \
+ gu_op((dest).val, (u32 __user *)(ptr))
+#endif /* CONFIG_PPC64 */
+
+#define get_user_instr(x, ptr) \
+ ___get_user_instr(get_user, x, ptr)
+
+#define __get_user_instr(x, ptr) \
+ ___get_user_instr(__get_user, x, ptr)
+
/*
* Instruction data type for POWER
*/
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 01aea0df4dd0..eaa828a6a419 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -53,40 +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)))
-#ifdef CONFIG_PPC64
-
-#define ___get_user_instr(gu_op, dest, ptr) \
-({ \
- long __gui_ret = 0; \
- unsigned long __gui_ptr = (unsigned long)ptr; \
- struct ppc_inst __gui_inst; \
- unsigned int __prefix, __suffix; \
- __gui_ret = gu_op(__prefix, (unsigned int __user *)__gui_ptr); \
- if (__gui_ret == 0) { \
- if ((__prefix >> 26) == OP_PREFIX) { \
- __gui_ret = gu_op(__suffix, \
- (unsigned int __user *)__gui_ptr + 1); \
- __gui_inst = ppc_inst_prefix(__prefix, \
- __suffix); \
- } else { \
- __gui_inst = ppc_inst(__prefix); \
- } \
- if (__gui_ret == 0) \
- (dest) = __gui_inst; \
- } \
- __gui_ret; \
-})
-#else /* !CONFIG_PPC64 */
-#define ___get_user_instr(gu_op, dest, ptr) \
- gu_op((dest).val, (u32 __user *)(ptr))
-#endif /* CONFIG_PPC64 */
-
-#define get_user_instr(x, ptr) \
- ___get_user_instr(get_user, x, ptr)
-
-#define __get_user_instr(x, ptr) \
- ___get_user_instr(__get_user, x, ptr)
-
extern long __put_user_bad(void);
#define __put_user_size(x, ptr, size, retval) \
--
2.25.0
^ permalink raw reply related
* [PATCH v2 07/15] powerpc/uaccess: Call might_fault() inconditionaly
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>
Commit 6bfd93c32a50 ("powerpc: Fix incorrect might_sleep in
__get_user/__put_user on kernel addresses") added a check to not call
might_sleep() on kernel addresses. This was to enable the use of
__get_user() in the alignment exception handler for any address.
Then commit 95156f0051cb ("lockdep, mm: fix might_fault() annotation")
added a check of the address space in might_fault(), based on
set_fs() logic. But this didn't solve the powerpc alignment exception
case as it didn't call set_fs(KERNEL_DS).
Nowadays, set_fs() is gone, previous patch fixed the alignment
exception handler and __get_user/__put_user are not supposed to be
used anymore to read kernel memory.
Therefore the is_kernel_addr() check has become useless and can be
removed.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/uaccess.h | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index eaa828a6a419..c4bbc64758a0 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -77,8 +77,7 @@ __pu_failed: \
__typeof__(*(ptr)) __pu_val = (x); \
__typeof__(size) __pu_size = (size); \
\
- if (!is_kernel_addr((unsigned long)__pu_addr)) \
- might_fault(); \
+ might_fault(); \
__chk_user_ptr(__pu_addr); \
__put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \
\
@@ -238,12 +237,12 @@ do { \
__typeof__(size) __gu_size = (size); \
\
__chk_user_ptr(__gu_addr); \
- if (do_allow && !is_kernel_addr((unsigned long)__gu_addr)) \
+ if (do_allow) { \
might_fault(); \
- if (do_allow) \
__get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
- else \
+ } else { \
__get_user_size_allowed(__gu_val, __gu_addr, __gu_size, __gu_err); \
+ } \
(x) = (__typeof__(*(ptr)))__gu_val; \
\
__gu_err; \
--
2.25.0
^ permalink raw reply related
* [PATCH v2 06/15] powerpc/align: Don't use __get_user_instr() on kernel addresses
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>
In the old days, when we didn't have kernel userspace access
protection and had set_fs(), it was wise to use __get_user()
and friends to read kernel memory.
Nowadays, get_user() is granting userspace access and is exclusively
for userspace access.
In alignment exception handler, use probe_kernel_read_inst()
instead of __get_user_instr() for reading instructions in kernel.
This will allow to remove the is_kernel_addr() check in
__get/put_user() in a following patch.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/kernel/align.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
index c4d7b445b459..8d4c7af262e2 100644
--- a/arch/powerpc/kernel/align.c
+++ b/arch/powerpc/kernel/align.c
@@ -310,7 +310,11 @@ int fix_alignment(struct pt_regs *regs)
*/
CHECK_FULL_REGS(regs);
- if (unlikely(__get_user_instr(instr, (void __user *)regs->nip)))
+ if (is_kernel_addr(regs->nip))
+ r = probe_kernel_read_inst(&instr, (void *)regs->nip);
+ else
+ r = __get_user_instr(instr, (void __user *)regs->nip);
+ if (unlikely(r))
return -EFAULT;
if ((regs->msr & MSR_LE) != (MSR_KERNEL & MSR_LE)) {
/* We don't handle PPC little-endian any more... */
--
2.25.0
^ permalink raw reply related
* [PATCH v2 03/15] powerpc/align: Convert emulate_spe() to user_access_begin
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>
This patch converts emulate_spe() to using user_access_being
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.
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.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/kernel/align.c | 61 ++++++++++++++++++++-----------------
1 file changed, 33 insertions(+), 28 deletions(-)
diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
index c7797eb958c7..c4d7b445b459 100644
--- a/arch/powerpc/kernel/align.c
+++ b/arch/powerpc/kernel/align.c
@@ -107,7 +107,6 @@ static struct aligninfo spe_aligninfo[32] = {
static int emulate_spe(struct pt_regs *regs, unsigned int reg,
struct ppc_inst ppc_instr)
{
- int ret;
union {
u64 ll;
u32 w[2];
@@ -127,11 +126,6 @@ static int emulate_spe(struct pt_regs *regs, unsigned int reg,
nb = spe_aligninfo[instr].len;
flags = spe_aligninfo[instr].flags;
- /* Verify the address of the operand */
- if (unlikely(user_mode(regs) &&
- !access_ok(addr, nb)))
- return -EFAULT;
-
/* userland only */
if (unlikely(!user_mode(regs)))
return 0;
@@ -169,26 +163,27 @@ static int emulate_spe(struct pt_regs *regs, unsigned int reg,
}
} else {
temp.ll = data.ll = 0;
- ret = 0;
p = addr;
+ 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);
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))
+ return -EFAULT;
+
switch (nb) {
case 8:
- ret |= __put_user_inatomic(data.v[0], p++);
- ret |= __put_user_inatomic(data.v[1], p++);
- ret |= __put_user_inatomic(data.v[2], p++);
- ret |= __put_user_inatomic(data.v[3], p++);
+ unsafe_put_user(data.v[0], p++, Efault_write);
+ unsafe_put_user(data.v[1], p++, Efault_write);
+ unsafe_put_user(data.v[2], p++, Efault_write);
+ unsafe_put_user(data.v[3], p++, Efault_write);
fallthrough;
case 4:
- ret |= __put_user_inatomic(data.v[4], p++);
- ret |= __put_user_inatomic(data.v[5], p++);
+ unsafe_put_user(data.v[4], p++, Efault_write);
+ unsafe_put_user(data.v[5], p++, Efault_write);
fallthrough;
case 2:
- ret |= __put_user_inatomic(data.v[6], p++);
- ret |= __put_user_inatomic(data.v[7], p++);
+ unsafe_put_user(data.v[6], p++, Efault_write);
+ unsafe_put_user(data.v[7], p++, Efault_write);
}
- if (unlikely(ret))
- return -EFAULT;
+ user_write_access_end();
} else {
*evr = data.w[0];
regs->gpr[reg] = data.w[1];
}
return 1;
+
+Efault_read:
+ user_read_access_end();
+ return -EFAULT;
+
+Efault_write:
+ user_write_access_end();
+ return -EFAULT;
}
#endif /* CONFIG_SPE */
--
2.25.0
^ permalink raw reply related
* [PATCH v2 04/15] powerpc/uaccess: Remove __get/put_user_inatomic()
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>
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().
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 related
* [PATCH v2 02/15] powerpc/uaccess: Define ___get_user_instr() for ppc32
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>
Define simple ___get_user_instr() for ppc32 instead of
defining ppc32 versions of the three get_user_instr()
helpers.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Reviewed-by: Daniel Axtens <dja@axtens.net>
---
arch/powerpc/include/asm/uaccess.h | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 8cbf3e3874f1..a08c482b1315 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -81,6 +81,10 @@ static inline bool __access_ok(unsigned long addr, unsigned long size)
} \
__gui_ret; \
})
+#else /* !CONFIG_PPC64 */
+#define ___get_user_instr(gu_op, dest, ptr) \
+ gu_op((dest).val, (u32 __user *)(ptr))
+#endif /* CONFIG_PPC64 */
#define get_user_instr(x, ptr) \
___get_user_instr(get_user, x, ptr)
@@ -91,18 +95,6 @@ static inline bool __access_ok(unsigned long addr, unsigned long size)
#define __get_user_instr_inatomic(x, ptr) \
___get_user_instr(__get_user_inatomic, x, ptr)
-#else /* !CONFIG_PPC64 */
-#define get_user_instr(x, ptr) \
- get_user((x).val, (u32 __user *)(ptr))
-
-#define __get_user_instr(x, ptr) \
- __get_user_nocheck((x).val, (u32 __user *)(ptr), sizeof(u32), true)
-
-#define __get_user_instr_inatomic(x, ptr) \
- __get_user_nosleep((x).val, (u32 __user *)(ptr), sizeof(u32))
-
-#endif /* CONFIG_PPC64 */
-
extern long __put_user_bad(void);
#define __put_user_size(x, ptr, size, retval) \
--
2.25.0
^ permalink raw reply related
* [PATCH v2 00/15] powerpc: Cleanup of uaccess.h and adding asm goto for get_user()
From: Christophe Leroy @ 2021-03-10 17:46 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
This series cleans up uaccess.h and adds asm goto for get_user()
v2:
- Further clean ups
- asm goto for get_user()
- Move a few patches unrelated to put_user/get_user into another misc series.
Christophe Leroy (15):
powerpc/uaccess: Remove __get_user_allowed() and unsafe_op_wrap()
powerpc/uaccess: Define ___get_user_instr() for ppc32
powerpc/align: Convert emulate_spe() to user_access_begin
powerpc/uaccess: Remove __get/put_user_inatomic()
powerpc/uaccess: Move get_user_instr helpers in asm/inst.h
powerpc/align: Don't use __get_user_instr() on kernel addresses
powerpc/uaccess: Call might_fault() inconditionaly
powerpc/uaccess: Remove __unsafe_put_user_goto()
powerpc/uaccess: Remove __chk_user_ptr() in __get/put_user
powerpc/uaccess: Remove calls to __get_user_bad() and __put_user_bad()
powerpc/uaccess: Split out __get_user_nocheck()
powerpc/uaccess: Rename __get/put_user_check/nocheck
powerpc/uaccess: Refactor get/put_user() and __get/put_user()
powerpc/uaccess: Introduce __get_user_size_goto()
powerpc/uaccess: Use asm goto for get_user when compiler supports it
arch/powerpc/include/asm/inst.h | 34 ++
arch/powerpc/include/asm/uaccess.h | 293 +++++++-----------
arch/powerpc/kernel/align.c | 67 ++--
.../kernel/hw_breakpoint_constraints.c | 2 +-
arch/powerpc/kernel/traps.c | 2 +-
5 files changed, 187 insertions(+), 211 deletions(-)
--
2.25.0
^ permalink raw reply
* [PATCH v2 01/15] powerpc/uaccess: Remove __get_user_allowed() and unsafe_op_wrap()
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>
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 related
* [PATCH] cxl: don't manipulate the mm.mm_users field directly
From: Laurent Dufour @ 2021-03-10 17:44 UTC (permalink / raw)
To: fbarrat, ajd, arnd, gregkh; +Cc: clombard, linuxppc-dev, linux-kernel
It is better to rely on the API provided by the MM layer instead of
directly manipulating the mm_users field.
Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
---
drivers/misc/cxl/fault.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/misc/cxl/fault.c b/drivers/misc/cxl/fault.c
index 01153b74334a..60c829113299 100644
--- a/drivers/misc/cxl/fault.c
+++ b/drivers/misc/cxl/fault.c
@@ -200,7 +200,7 @@ static struct mm_struct *get_mem_context(struct cxl_context *ctx)
if (ctx->mm == NULL)
return NULL;
- if (!atomic_inc_not_zero(&ctx->mm->mm_users))
+ if (!mmget_not_zero(ctx->mm))
return NULL;
return ctx->mm;
--
2.30.1
^ permalink raw reply related
* Re: [PATCH v1] powerpc: Include running function as first entry in save_stack_trace() and friends
From: Segher Boessenkool @ 2021-03-10 17:37 UTC (permalink / raw)
To: Mark Rutland
Cc: Marco Elver, Catalin Marinas, linuxppc-dev, LKML, kasan-dev,
broonie, Paul Mackerras, Will Deacon, linux-arm-kernel
In-Reply-To: <20210310112441.GA19619@C02TD0UTHF1T.local>
Hi!
On Wed, Mar 10, 2021 at 11:32:20AM +0000, Mark Rutland wrote:
> On Tue, Mar 09, 2021 at 04:05:32PM -0600, Segher Boessenkool wrote:
> > On Tue, Mar 09, 2021 at 04:05:23PM +0000, Mark Rutland wrote:
> > > On Thu, Mar 04, 2021 at 03:54:48PM -0600, Segher Boessenkool wrote:
> > > > On Thu, Mar 04, 2021 at 02:57:30PM +0000, Mark Rutland wrote:
> > > > > It looks like GCC is happy to give us the function-entry-time FP if we use
> > > > > __builtin_frame_address(1),
> > > >
> > > > From the GCC manual:
> > > > Calling this function with a nonzero argument can have
> > > > unpredictable effects, including crashing the calling program. As
> > > > a result, calls that are considered unsafe are diagnosed when the
> > > > '-Wframe-address' option is in effect. Such calls should only be
> > > > made in debugging situations.
> > > >
> > > > It *does* warn (the warning is in -Wall btw), on both powerpc and
> > > > aarch64. Furthermore, using this builtin causes lousy code (it forces
> > > > the use of a frame pointer, which we normally try very hard to optimise
> > > > away, for good reason).
> > > >
> > > > And, that warning is not an idle warning. Non-zero arguments to
> > > > __builtin_frame_address can crash the program. It won't on simpler
> > > > functions, but there is no real definition of what a simpler function
> > > > *is*. It is meant for debugging, not for production use (this is also
> > > > why no one has bothered to make it faster).
> > > >
> > > > On Power it should work, but on pretty much any other arch it won't.
> > >
> > > I understand this is true generally, and cannot be relied upon in
> > > portable code. However as you hint here for Power, I believe that on
> > > arm64 __builtin_frame_address(1) shouldn't crash the program due to the
> > > way frame records work on arm64, but I'll go check with some local
> > > compiler folk. I agree that __builtin_frame_address(2) and beyond
> > > certainly can, e.g. by NULL dereference and similar.
> >
> > I still do not know the aarch64 ABI well enough. If only I had time!
> >
> > > For context, why do you think this would work on power specifically? I
> > > wonder if our rationale is similar.
> >
> > On most 64-bit Power ABIs all stack frames are connected together as a
> > linked list (which is updated atomically, importantly). This makes it
> > possible to always find all previous stack frames.
>
> We have something similar on arm64, where the kernel depends on being
> built with a frame pointer following the AAPCS frame pointer rules.
The huge difference is on Power this is about the stack itself: you do
not need a frame pointer at all for it (there is no specific register
named as frame pointer, even).
> Every stack frame contains a "frame record" *somewhere* within that
> stack frame, and the frame records are chained together as a linked
> list. The frame pointer points at the most recent frame record (and this
> is what __builtin_frame_address(0) returns).
> > See gcc.gnu.org/PR60109 for example.
>
> Sure; I see that being true generally (and Ramana noted that on 32-bit
> arm a frame pointer wasn't mandated), but I think in this case we have a
> stronger target (and configuration) specific guarantee.
It sounds like it, yes. You need to have a frame pointer in the ABI,
with pretty strong rules, and have everything follow those rules.
> > Is the frame pointer required?!
>
> The arm64 Linux port mandates frame pointers for kernel code. It is
> generally possible to build code without frame pointers (e.g. userspace),
> but doing that for kernel code would be a bug.
I see. And it even is less expensive to do this than on most machines,
because of register pair load/store instructions :-)
> > > > The real way forward is to bite the bullet and to no longer pretend you
> > > > can do a full backtrace from just the stack contents. You cannot.
> > >
> > > I think what you mean here is that there's no reliable way to handle the
> > > current/leaf function, right? If so I do agree.
> >
> > No, I meant what I said.
> >
> > There is the separate issue that you do not know where the return
> > address (etc.) is stored in a function that has not yet done a call
> > itself, sure. You cannot assume anything the ABI does not tell you you
> > can depend on.
>
> This is in the frame record per the AAPCS.
But you do not know where in the function it will store that. It often
can be optimised by the compiler to only store the LR and FP on paths
where a call will happen later, and there is no way (without DWARF info
or similar) to know whether that has happened yet or not.
This is a well-known problem of course. For the current function you
cannot know in general if there is an activation frame yet or not.
Segher
^ permalink raw reply
* Re: [PATCH 3/9] powerpc/pseries: remove the ppc-cmm file system
From: Al Viro @ 2021-03-10 16:29 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jason Gunthorpe, David Hildenbrand, VMware, Inc.,
Michael S. Tsirkin, linux-kernel, dri-devel, virtualization,
linux-mm, Minchan Kim, Alex Williamson, Nadav Amit, Daniel Vetter,
linux-fsdevel, Andrew Morton, linuxppc-dev, Nitin Gupta
In-Reply-To: <20210309155348.974875-4-hch@lst.de>
On Tue, Mar 09, 2021 at 04:53:42PM +0100, Christoph Hellwig wrote:
> Just use the generic anon_inode file system.
Umm... The only problem I see here is the lifetime rules for
that module, and that's not something introduced in this patchset.
Said that, looks like the logics around that place is duplicated in
cmm.c, vmw_balloon.c and virtion_balloon.c and I wonder if it would
be better off with a helper in mm/balloon.c to be used for that setup...
^ permalink raw reply
* Re: [PATCH 4/9] drm: remove the drm file system
From: Al Viro @ 2021-03-10 16:32 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jason Gunthorpe, David Hildenbrand, VMware, Inc.,
Michael S. Tsirkin, linux-kernel, dri-devel, virtualization,
linux-mm, Minchan Kim, Alex Williamson, Nadav Amit, Daniel Vetter,
linux-fsdevel, Andrew Morton, linuxppc-dev, Nitin Gupta
In-Reply-To: <20210309155348.974875-5-hch@lst.de>
On Tue, Mar 09, 2021 at 04:53:43PM +0100, Christoph Hellwig wrote:
> Just use the generic anon_inode file system.
Are you changing the lifetime rules for that module?
^ permalink raw reply
* Re: PowerPC64 future proof kernel toc, revised for lld
From: Segher Boessenkool @ 2021-03-10 16:17 UTC (permalink / raw)
To: Christophe Leroy
Cc: alexey, Alexey Kardashevskiy, linuxppc-dev, ellerman, Alan Modra
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
The minimum GCC version required is 4.9, released April 2014, so it
would make sense to require binutils 2.24 at least as well: that was the
last binutils release before the GCC 4.9 release (it was end of 2013).
Generally you should make sure to always have a binutils at least as new
as your GCC (and newer almost always works just fine).
Segher
^ permalink raw reply
* Re: [PATCH v4 13/14] dt-bindings: of: Add restricted DMA pool
From: Will Deacon @ 2021-03-10 16:07 UTC (permalink / raw)
To: Claire Chang
Cc: heikki.krogerus, peterz, grant.likely, paulus, Frank Rowand,
mingo, Marek Szyprowski, sstabellini, Saravana Kannan,
Joerg Roedel, Rafael J . Wysocki, Christoph Hellwig,
Bartosz Golaszewski, xen-devel, Thierry Reding, linux-devicetree,
Konrad Rzeszutek Wilk, Dan Williams, linuxppc-dev, Rob Herring,
boris.ostrovsky, Andy Shevchenko, jgross, Nicolas Boichat,
Greg KH, Randy Dunlap, lkml, list@263.net:IOMMU DRIVERS,
Jim Quinlan, xypron.glpk, Robin Murphy, bauerman
In-Reply-To: <20210209062131.2300005-14-tientzu@chromium.org>
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 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?
Will
^ permalink raw reply
* Re: [PATCH V2 2/2] powerpc/perf: Add platform specific check_attr_config
From: Segher Boessenkool @ 2021-03-10 15:19 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: Madhavan Srinivasan, linuxppc-dev
In-Reply-To: <dde47591-db00-4c4f-ed24-a8ab5a7a4c6a@ozlabs.ru>
On Thu, Mar 11, 2021 at 12:16:25AM +1100, Alexey Kardashevskiy wrote:
> On 26/02/2021 17:50, Madhavan Srinivasan wrote:
> >+int isa3_X_check_attr_config(struct perf_event *ev)
>
> "isa300" is used everywhere else to refer to ISA 3.00.
And that itself is a misspelling, there is nothing called "ISA 3.00" (it
is called "ISA 3.0").
Segher
^ permalink raw reply
* Re: [PATCH] powerpc: Fix missing declaration of [en/dis]able_kernel_vsx()
From: Christophe Leroy @ 2021-03-10 13:33 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linux Kernel Mailing List, amd-gfx list, Paul Mackerras,
Alex Deucher, linuxppc-dev, Christian König
In-Reply-To: <CAMuHMdUQcE7+O9NWH4Xxxv+r7ZFnTGqtHuteOMiSPY_gK5xkZw@mail.gmail.com>
Hi Geert,
Le 09/03/2021 à 11:55, Geert Uytterhoeven a écrit :
> Hi Christophe,
>
> On Tue, Mar 9, 2021 at 10:58 AM Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>> Le 09/03/2021 à 10:16, Geert Uytterhoeven a écrit :
>>> On Tue, Mar 9, 2021 at 9:52 AM Christophe Leroy
>>> <christophe.leroy@csgroup.eu> wrote:
>>>> Le 09/03/2021 à 09:45, Geert Uytterhoeven a écrit :
>>>>> On Tue, Mar 9, 2021 at 9:39 AM Christophe Leroy
>>>>> <christophe.leroy@csgroup.eu> wrote:
>>>>>> Add stub instances of enable_kernel_vsx() and disable_kernel_vsx()
>>>>>> when CONFIG_VSX is not set, to avoid following build failure.
>>>>>>
>>>>>> CC [M] drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o
>>>>>> In file included from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services_types.h:29,
>>>>>> from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:37,
>>>>>> from drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:27:
>>>>>> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c: In function 'dcn_bw_apply_registry_override':
>>>>>> ./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:64:3: error: implicit declaration of function 'enable_kernel_vsx'; did you mean 'enable_kernel_fp'? [-Werror=implicit-function-declaration]
>>>>>> 64 | enable_kernel_vsx(); \
>>>>>> | ^~~~~~~~~~~~~~~~~
>>>>>> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:640:2: note: in expansion of macro 'DC_FP_START'
>>>>>> 640 | DC_FP_START();
>>>>>> | ^~~~~~~~~~~
>>>>>> ./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:75:3: error: implicit declaration of function 'disable_kernel_vsx'; did you mean 'disable_kernel_fp'? [-Werror=implicit-function-declaration]
>>>>>> 75 | disable_kernel_vsx(); \
>>>>>> | ^~~~~~~~~~~~~~~~~~
>>>>>> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:676:2: note: in expansion of macro 'DC_FP_END'
>>>>>> 676 | DC_FP_END();
>>>>>> | ^~~~~~~~~
>>>>>> cc1: some warnings being treated as errors
>>>>>> make[5]: *** [drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o] Error 1
>>>>>>
>>>>>> Fixes: 16a9dea110a6 ("amdgpu: Enable initial DCN support on POWER")
>>>>>> Cc: stable@vger.kernel.org
>>>>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>>>>>
>>>>> Thanks for your patch!
>>>>>
>>>>>> --- a/arch/powerpc/include/asm/switch_to.h
>>>>>> +++ b/arch/powerpc/include/asm/switch_to.h
>>>>>> @@ -71,6 +71,16 @@ static inline void disable_kernel_vsx(void)
>>>>>> {
>>>>>> msr_check_and_clear(MSR_FP|MSR_VEC|MSR_VSX);
>>>>>> }
>>>>>> +#else
>>>>>> +static inline void enable_kernel_vsx(void)
>>>>>> +{
>>>>>> + BUILD_BUG();
>>>>>> +}
>>>>>> +
>>>>>> +static inline void disable_kernel_vsx(void)
>>>>>> +{
>>>>>> + BUILD_BUG();
>>>>>> +}
>>>>>> #endif
>>>>>
>>>>> I'm wondering how this is any better than the current situation: using
>>>>> BUILD_BUG() will still cause a build failure?
>>>>
>>>> No it won't cause a failure. In drivers/gpu/drm/amd/display/dc/os_types.h you have:
>>>>
>>>> #define DC_FP_START() { \
>>>> if (cpu_has_feature(CPU_FTR_VSX_COMP)) { \
>>>> preempt_disable(); \
>>>> enable_kernel_vsx(); \
>>>> } else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) { \
>>>> preempt_disable(); \
>>>> enable_kernel_altivec(); \
>>>> } else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) { \
>>>> preempt_disable(); \
>>>> enable_kernel_fp(); \
>>>> } \
>>>>
>>>> When CONFIG_VSX is not selected, cpu_has_feature(CPU_FTR_VSX_COMP) constant folds to 'false' so the
>>>> call to enable_kernel_vsx() is discarded and the build succeeds.
>>>
>>> IC. So you might as well have an empty (dummy) function instead?
>>>
>>
>> But with an empty function, you take the risk that one day, someone calls it without checking that
>> CONFIG_VSX is selected. Here if someone does that, build will fail.
>
> OK, convinced.
>
Note that following build test performed on kisskb, with gcc 4.9 the following change is required in
addition:
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/b231dfa040ce4cc37f702f5c3a595fdeabfe0462.1615378209.git.christophe.leroy@csgroup.eu/
Christophe
^ permalink raw reply
* Re: [PATCH v4 3/6] ASoC: dt-bindings: fsl_rpmsg: Add binding doc for rpmsg cpu dai driver
From: Shengjiu Wang @ 2021-03-10 13:33 UTC (permalink / raw)
To: Rob Herring
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
alsa-devel, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Takashi Iwai, Liam Girdwood, linux-kernel, Nicolin Chen,
Mark Brown, linuxppc-dev
In-Reply-To: <20210310024834.GA1623179@robh.at.kernel.org>
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.
>
> > 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.
>
> > +
> > + 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
>
> > +
> > + 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.
I will update this document in v5
Best regards
Wang Shengjiu
^ permalink raw reply
* Re: [PATCH V2 2/2] powerpc/perf: Add platform specific check_attr_config
From: Alexey Kardashevskiy @ 2021-03-10 13:16 UTC (permalink / raw)
To: Madhavan Srinivasan, mpe; +Cc: linuxppc-dev
In-Reply-To: <20210226065025.1254973-2-maddy@linux.ibm.com>
On 26/02/2021 17:50, Madhavan Srinivasan wrote:
> Add platform specific attr.config value checks. Patch
> includes checks for both power9 and power10.
>
> Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com>
> ---
> Changelog v1:
> - No changes.
>
> arch/powerpc/perf/isa207-common.c | 41 +++++++++++++++++++++++++++++++
> arch/powerpc/perf/isa207-common.h | 2 ++
> arch/powerpc/perf/power10-pmu.c | 13 ++++++++++
> arch/powerpc/perf/power9-pmu.c | 13 ++++++++++
> 4 files changed, 69 insertions(+)
>
> diff --git a/arch/powerpc/perf/isa207-common.c b/arch/powerpc/perf/isa207-common.c
> index e4f577da33d8..b255799f5b51 100644
> --- a/arch/powerpc/perf/isa207-common.c
> +++ b/arch/powerpc/perf/isa207-common.c
> @@ -694,3 +694,44 @@ int isa207_get_alternatives(u64 event, u64 alt[], int size, unsigned int flags,
>
> return num_alt;
> }
> +
> +int isa3_X_check_attr_config(struct perf_event *ev)
"isa300" is used everywhere else to refer to ISA 3.00.
> +{
> + u64 val, sample_mode;
> + u64 event = ev->attr.config;
> +
> + val = (event >> EVENT_SAMPLE_SHIFT) & EVENT_SAMPLE_MASK;
I am not familiar with the code - "Raw event encoding for Power9" from
arch/powerpc/perf/power9-pmu.c - where is this from? Is this how linux
defines encoding or it is P9 UM or something?
> + sample_mode = val & 0x3;
> +
> + /*
> + * MMCRA[61:62] is Randome Sampling Mode (SM).
> + * value of 0b11 is reserved.
> + */
> + if (sample_mode == 0x3)
> + return -1;
> +
> + /*
> + * Check for all reserved value
> + */
> + switch (val) {
> + case 0x5:
> + case 0x9:
> + case 0xD:
> + case 0x19:
> + case 0x1D:
> + case 0x1A:
> + case 0x1E:
What spec did these numbers come from?
> + return -1;
> + }
> +
> + /*
> + * MMCRA[48:51]/[52:55]) Threshold Start/Stop
> + * Events Selection.
> + * 0b11110000/0b00001111 is reserved.
The mapping between the event and MMCRA is very unclear :) But there are
more reserved values in MMCRA in PowerISA_public.v3.0B.pdf:
===
0000 Reserved
Problem state access (SPR 770)
1000 - 1111 - ReservedPrivileged access (SPR 770 or 786)
1000 - 1111 - Implementation-dependent
===
Do not you need to filter these too?
> + */
> + val = (event >> EVENT_THR_CTL_SHIFT) & EVENT_THR_CTL_MASK;
> + if (((val & 0xF0) == 0xF0) || ((val & 0xF) == 0xF))
> + return -1;
Since the filters may differ for problem and privileged, may be make
these check_attr_config() hooks return EINVAL or EPERM and pass it on in
the caller? Not sure there is much value in it though.
> +
> + return 0;
> +}
> diff --git a/arch/powerpc/perf/isa207-common.h b/arch/powerpc/perf/isa207-common.h
> index 1af0e8c97ac7..ae8eaf05efd1 100644
> --- a/arch/powerpc/perf/isa207-common.h
> +++ b/arch/powerpc/perf/isa207-common.h
> @@ -280,4 +280,6 @@ void isa207_get_mem_data_src(union perf_mem_data_src *dsrc, u32 flags,
> struct pt_regs *regs);
> void isa207_get_mem_weight(u64 *weight);
>
> +int isa3_X_check_attr_config(struct perf_event *ev);
> +
> #endif
> diff --git a/arch/powerpc/perf/power10-pmu.c b/arch/powerpc/perf/power10-pmu.c
> index a901c1348cad..bc64354cab6a 100644
> --- a/arch/powerpc/perf/power10-pmu.c
> +++ b/arch/powerpc/perf/power10-pmu.c
> @@ -106,6 +106,18 @@ static int power10_get_alternatives(u64 event, unsigned int flags, u64 alt[])
> return num_alt;
> }
>
> +static int power10_check_attr_config(struct perf_event *ev)
> +{
> + u64 val;
> + u64 event = ev->attr.config;
> +
> + val = (event >> EVENT_SAMPLE_SHIFT) & EVENT_SAMPLE_MASK;
> + if (val == 0x10 || isa3_X_check_attr_config(ev))
> + return -1;
> +
> + return 0;
> +}
> +
> GENERIC_EVENT_ATTR(cpu-cycles, PM_RUN_CYC);
> GENERIC_EVENT_ATTR(instructions, PM_RUN_INST_CMPL);
> GENERIC_EVENT_ATTR(branch-instructions, PM_BR_CMPL);
> @@ -559,6 +571,7 @@ static struct power_pmu power10_pmu = {
> .attr_groups = power10_pmu_attr_groups,
> .bhrb_nr = 32,
> .capabilities = PERF_PMU_CAP_EXTENDED_REGS,
> + .check_attr_config = power10_check_attr_config,
> };
>
> int init_power10_pmu(void)
> diff --git a/arch/powerpc/perf/power9-pmu.c b/arch/powerpc/perf/power9-pmu.c
> index 2a57e93a79dc..b3b9b226d053 100644
> --- a/arch/powerpc/perf/power9-pmu.c
> +++ b/arch/powerpc/perf/power9-pmu.c
> @@ -151,6 +151,18 @@ static int power9_get_alternatives(u64 event, unsigned int flags, u64 alt[])
> return num_alt;
> }
>
> +static int power9_check_attr_config(struct perf_event *ev)
> +{
> + u64 val;
> + u64 event = ev->attr.config;
> +
> + val = (event >> EVENT_SAMPLE_SHIFT) & EVENT_SAMPLE_MASK;
> + if (val == 0xC || isa3_X_check_attr_config(ev))
> + return -1;
> +
> + return 0;
> +}
> +
> GENERIC_EVENT_ATTR(cpu-cycles, PM_CYC);
> GENERIC_EVENT_ATTR(stalled-cycles-frontend, PM_ICT_NOSLOT_CYC);
> GENERIC_EVENT_ATTR(stalled-cycles-backend, PM_CMPLU_STALL);
> @@ -437,6 +449,7 @@ static struct power_pmu power9_pmu = {
> .attr_groups = power9_pmu_attr_groups,
> .bhrb_nr = 32,
> .capabilities = PERF_PMU_CAP_EXTENDED_REGS,
> + .check_attr_config = power9_check_attr_config,
> };
>
> int init_power9_pmu(void)
>
--
Alexey
^ permalink raw reply
* Re: PowerPC64 future proof kernel toc, revised for lld
From: Christophe Leroy @ 2021-03-10 12:44 UTC (permalink / raw)
To: Alan Modra, Alexey Kardashevskiy; +Cc: alexey, linuxppc-dev, ellerman
In-Reply-To: <20210310122513.GB29645@bubble.grove.modra.org>
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
Christophe
^ permalink raw reply
* [PATCH] powerpc/Makefile: Remove workaround for gcc versions below 4.9
From: Christophe Leroy @ 2021-03-10 12:43 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
Commit 6ec4476ac825 ("Raise gcc version requirement to 4.9")
made it impossible to build with gcc 4.8 and under.
Remove related workaround.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/Makefile | 6 ------
1 file changed, 6 deletions(-)
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 5f8544cf724a..32dd693b4e42 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -181,12 +181,6 @@ CC_FLAGS_FTRACE := -pg
ifdef CONFIG_MPROFILE_KERNEL
CC_FLAGS_FTRACE += -mprofile-kernel
endif
-# Work around gcc code-gen bugs with -pg / -fno-omit-frame-pointer in gcc <= 4.8
-# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199
-# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52828
-ifndef CONFIG_CC_IS_CLANG
-CC_FLAGS_FTRACE += $(call cc-ifversion, -lt, 0409, -mno-sched-epilog)
-endif
endif
CFLAGS-$(CONFIG_TARGET_CPU_BOOL) += $(call cc-option,-mcpu=$(CONFIG_TARGET_CPU))
--
2.25.0
^ permalink raw reply related
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