* [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()
@ 2020-04-16 12:39 Christophe Leroy
2020-06-29 6:52 ` Christophe Leroy
0 siblings, 1 reply; 14+ messages in thread
From: Christophe Leroy @ 2020-04-16 12:39 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
segher
Cc: linux-kernel, linuxppc-dev
At the time being, __put_user()/__get_user() and friends only use
D-form addressing, with 0 offset. Ex:
lwz reg1, 0(reg2)
Give the compiler the opportunity to use other adressing modes
whenever possible, to get more optimised code.
Hereunder is a small exemple:
struct test {
u32 item1;
u16 item2;
u8 item3;
u64 item4;
};
int set_test_user(struct test __user *from, struct test __user *to)
{
int err;
u32 item1;
u16 item2;
u8 item3;
u64 item4;
err = __get_user(item1, &from->item1);
err |= __get_user(item2, &from->item2);
err |= __get_user(item3, &from->item3);
err |= __get_user(item4, &from->item4);
err |= __put_user(item1, &to->item1);
err |= __put_user(item2, &to->item2);
err |= __put_user(item3, &to->item3);
err |= __put_user(item4, &to->item4);
return err;
}
Before the patch:
00000df0 <set_test_user>:
df0: 94 21 ff f0 stwu r1,-16(r1)
df4: 39 40 00 00 li r10,0
df8: 93 c1 00 08 stw r30,8(r1)
dfc: 93 e1 00 0c stw r31,12(r1)
e00: 7d 49 53 78 mr r9,r10
e04: 80 a3 00 00 lwz r5,0(r3)
e08: 38 e3 00 04 addi r7,r3,4
e0c: 7d 46 53 78 mr r6,r10
e10: a0 e7 00 00 lhz r7,0(r7)
e14: 7d 29 33 78 or r9,r9,r6
e18: 39 03 00 06 addi r8,r3,6
e1c: 7d 46 53 78 mr r6,r10
e20: 89 08 00 00 lbz r8,0(r8)
e24: 7d 29 33 78 or r9,r9,r6
e28: 38 63 00 08 addi r3,r3,8
e2c: 7d 46 53 78 mr r6,r10
e30: 83 c3 00 00 lwz r30,0(r3)
e34: 83 e3 00 04 lwz r31,4(r3)
e38: 7d 29 33 78 or r9,r9,r6
e3c: 7d 43 53 78 mr r3,r10
e40: 90 a4 00 00 stw r5,0(r4)
e44: 7d 29 1b 78 or r9,r9,r3
e48: 38 c4 00 04 addi r6,r4,4
e4c: 7d 43 53 78 mr r3,r10
e50: b0 e6 00 00 sth r7,0(r6)
e54: 7d 29 1b 78 or r9,r9,r3
e58: 38 e4 00 06 addi r7,r4,6
e5c: 7d 43 53 78 mr r3,r10
e60: 99 07 00 00 stb r8,0(r7)
e64: 7d 23 1b 78 or r3,r9,r3
e68: 38 84 00 08 addi r4,r4,8
e6c: 93 c4 00 00 stw r30,0(r4)
e70: 93 e4 00 04 stw r31,4(r4)
e74: 7c 63 53 78 or r3,r3,r10
e78: 83 c1 00 08 lwz r30,8(r1)
e7c: 83 e1 00 0c lwz r31,12(r1)
e80: 38 21 00 10 addi r1,r1,16
e84: 4e 80 00 20 blr
After the patch:
00000dbc <set_test_user>:
dbc: 39 40 00 00 li r10,0
dc0: 7d 49 53 78 mr r9,r10
dc4: 80 03 00 00 lwz r0,0(r3)
dc8: 7d 48 53 78 mr r8,r10
dcc: a1 63 00 04 lhz r11,4(r3)
dd0: 7d 29 43 78 or r9,r9,r8
dd4: 7d 48 53 78 mr r8,r10
dd8: 88 a3 00 06 lbz r5,6(r3)
ddc: 7d 29 43 78 or r9,r9,r8
de0: 7d 48 53 78 mr r8,r10
de4: 80 c3 00 08 lwz r6,8(r3)
de8: 80 e3 00 0c lwz r7,12(r3)
dec: 7d 29 43 78 or r9,r9,r8
df0: 7d 43 53 78 mr r3,r10
df4: 90 04 00 00 stw r0,0(r4)
df8: 7d 29 1b 78 or r9,r9,r3
dfc: 7d 43 53 78 mr r3,r10
e00: b1 64 00 04 sth r11,4(r4)
e04: 7d 29 1b 78 or r9,r9,r3
e08: 7d 43 53 78 mr r3,r10
e0c: 98 a4 00 06 stb r5,6(r4)
e10: 7d 23 1b 78 or r3,r9,r3
e14: 90 c4 00 08 stw r6,8(r4)
e18: 90 e4 00 0c stw r7,12(r4)
e1c: 7c 63 53 78 or r3,r3,r10
e20: 4e 80 00 20 blr
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Reviewed-by: Segher Boessenkool <segher@kernel.crashing.org>
---
v2:
- Added <> modifier in __put_user_asm() and __get_user_asm()
- Removed %U2 in __put_user_asm2() and __get_user_asm2()
- Reworded the commit log
---
arch/powerpc/include/asm/uaccess.h | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 7c811442b607..9365b59495a2 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -114,7 +114,7 @@ extern long __put_user_bad(void);
*/
#define __put_user_asm(x, addr, err, op) \
__asm__ __volatile__( \
- "1: " op " %1,0(%2) # put_user\n" \
+ "1: " op "%U2%X2 %1,%2 # put_user\n" \
"2:\n" \
".section .fixup,\"ax\"\n" \
"3: li %0,%3\n" \
@@ -122,7 +122,7 @@ extern long __put_user_bad(void);
".previous\n" \
EX_TABLE(1b, 3b) \
: "=r" (err) \
- : "r" (x), "b" (addr), "i" (-EFAULT), "0" (err))
+ : "r" (x), "m<>" (*addr), "i" (-EFAULT), "0" (err))
#ifdef __powerpc64__
#define __put_user_asm2(x, ptr, retval) \
@@ -130,8 +130,8 @@ extern long __put_user_bad(void);
#else /* __powerpc64__ */
#define __put_user_asm2(x, addr, err) \
__asm__ __volatile__( \
- "1: stw %1,0(%2)\n" \
- "2: stw %1+1,4(%2)\n" \
+ "1: stw%X2 %1,%2\n" \
+ "2: stw%X2 %L1,%L2\n" \
"3:\n" \
".section .fixup,\"ax\"\n" \
"4: li %0,%3\n" \
@@ -140,7 +140,7 @@ extern long __put_user_bad(void);
EX_TABLE(1b, 4b) \
EX_TABLE(2b, 4b) \
: "=r" (err) \
- : "r" (x), "b" (addr), "i" (-EFAULT), "0" (err))
+ : "r" (x), "m" (*addr), "i" (-EFAULT), "0" (err))
#endif /* __powerpc64__ */
#define __put_user_size_allowed(x, ptr, size, retval) \
@@ -260,7 +260,7 @@ extern long __get_user_bad(void);
#define __get_user_asm(x, addr, err, op) \
__asm__ __volatile__( \
- "1: "op" %1,0(%2) # get_user\n" \
+ "1: "op"%U2%X2 %1, %2 # get_user\n" \
"2:\n" \
".section .fixup,\"ax\"\n" \
"3: li %0,%3\n" \
@@ -269,7 +269,7 @@ extern long __get_user_bad(void);
".previous\n" \
EX_TABLE(1b, 3b) \
: "=r" (err), "=r" (x) \
- : "b" (addr), "i" (-EFAULT), "0" (err))
+ : "m<>" (*addr), "i" (-EFAULT), "0" (err))
#ifdef __powerpc64__
#define __get_user_asm2(x, addr, err) \
@@ -277,8 +277,8 @@ extern long __get_user_bad(void);
#else /* __powerpc64__ */
#define __get_user_asm2(x, addr, err) \
__asm__ __volatile__( \
- "1: lwz %1,0(%2)\n" \
- "2: lwz %1+1,4(%2)\n" \
+ "1: lwz%X2 %1, %2\n" \
+ "2: lwz%X2 %L1, %L2\n" \
"3:\n" \
".section .fixup,\"ax\"\n" \
"4: li %0,%3\n" \
@@ -289,7 +289,7 @@ extern long __get_user_bad(void);
EX_TABLE(1b, 4b) \
EX_TABLE(2b, 4b) \
: "=r" (err), "=&r" (x) \
- : "b" (addr), "i" (-EFAULT), "0" (err))
+ : "m" (*addr), "i" (-EFAULT), "0" (err))
#endif /* __powerpc64__ */
#define __get_user_size_allowed(x, ptr, size, retval) \
@@ -299,10 +299,10 @@ do { \
if (size > sizeof(x)) \
(x) = __get_user_bad(); \
switch (size) { \
- case 1: __get_user_asm(x, ptr, retval, "lbz"); break; \
- case 2: __get_user_asm(x, ptr, retval, "lhz"); break; \
- case 4: __get_user_asm(x, ptr, retval, "lwz"); break; \
- case 8: __get_user_asm2(x, ptr, retval); break; \
+ 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(); \
} \
} while (0)
--
2.25.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user() 2020-04-16 12:39 [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user() Christophe Leroy @ 2020-06-29 6:52 ` Christophe Leroy 2020-06-29 11:27 ` Michael Ellerman 0 siblings, 1 reply; 14+ messages in thread From: Christophe Leroy @ 2020-06-29 6:52 UTC (permalink / raw) To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin, segher Cc: linuxppc-dev, linux-kernel Hi Michael, I see this patch is marked as "defered" in patchwork, but I can't see any related discussion. Is it normal ? Christophe Le 16/04/2020 à 14:39, Christophe Leroy a écrit : > At the time being, __put_user()/__get_user() and friends only use > D-form addressing, with 0 offset. Ex: > > lwz reg1, 0(reg2) > > Give the compiler the opportunity to use other adressing modes > whenever possible, to get more optimised code. > > Hereunder is a small exemple: > > struct test { > u32 item1; > u16 item2; > u8 item3; > u64 item4; > }; > > int set_test_user(struct test __user *from, struct test __user *to) > { > int err; > u32 item1; > u16 item2; > u8 item3; > u64 item4; > > err = __get_user(item1, &from->item1); > err |= __get_user(item2, &from->item2); > err |= __get_user(item3, &from->item3); > err |= __get_user(item4, &from->item4); > > err |= __put_user(item1, &to->item1); > err |= __put_user(item2, &to->item2); > err |= __put_user(item3, &to->item3); > err |= __put_user(item4, &to->item4); > > return err; > } > > Before the patch: > > 00000df0 <set_test_user>: > df0: 94 21 ff f0 stwu r1,-16(r1) > df4: 39 40 00 00 li r10,0 > df8: 93 c1 00 08 stw r30,8(r1) > dfc: 93 e1 00 0c stw r31,12(r1) > e00: 7d 49 53 78 mr r9,r10 > e04: 80 a3 00 00 lwz r5,0(r3) > e08: 38 e3 00 04 addi r7,r3,4 > e0c: 7d 46 53 78 mr r6,r10 > e10: a0 e7 00 00 lhz r7,0(r7) > e14: 7d 29 33 78 or r9,r9,r6 > e18: 39 03 00 06 addi r8,r3,6 > e1c: 7d 46 53 78 mr r6,r10 > e20: 89 08 00 00 lbz r8,0(r8) > e24: 7d 29 33 78 or r9,r9,r6 > e28: 38 63 00 08 addi r3,r3,8 > e2c: 7d 46 53 78 mr r6,r10 > e30: 83 c3 00 00 lwz r30,0(r3) > e34: 83 e3 00 04 lwz r31,4(r3) > e38: 7d 29 33 78 or r9,r9,r6 > e3c: 7d 43 53 78 mr r3,r10 > e40: 90 a4 00 00 stw r5,0(r4) > e44: 7d 29 1b 78 or r9,r9,r3 > e48: 38 c4 00 04 addi r6,r4,4 > e4c: 7d 43 53 78 mr r3,r10 > e50: b0 e6 00 00 sth r7,0(r6) > e54: 7d 29 1b 78 or r9,r9,r3 > e58: 38 e4 00 06 addi r7,r4,6 > e5c: 7d 43 53 78 mr r3,r10 > e60: 99 07 00 00 stb r8,0(r7) > e64: 7d 23 1b 78 or r3,r9,r3 > e68: 38 84 00 08 addi r4,r4,8 > e6c: 93 c4 00 00 stw r30,0(r4) > e70: 93 e4 00 04 stw r31,4(r4) > e74: 7c 63 53 78 or r3,r3,r10 > e78: 83 c1 00 08 lwz r30,8(r1) > e7c: 83 e1 00 0c lwz r31,12(r1) > e80: 38 21 00 10 addi r1,r1,16 > e84: 4e 80 00 20 blr > > After the patch: > > 00000dbc <set_test_user>: > dbc: 39 40 00 00 li r10,0 > dc0: 7d 49 53 78 mr r9,r10 > dc4: 80 03 00 00 lwz r0,0(r3) > dc8: 7d 48 53 78 mr r8,r10 > dcc: a1 63 00 04 lhz r11,4(r3) > dd0: 7d 29 43 78 or r9,r9,r8 > dd4: 7d 48 53 78 mr r8,r10 > dd8: 88 a3 00 06 lbz r5,6(r3) > ddc: 7d 29 43 78 or r9,r9,r8 > de0: 7d 48 53 78 mr r8,r10 > de4: 80 c3 00 08 lwz r6,8(r3) > de8: 80 e3 00 0c lwz r7,12(r3) > dec: 7d 29 43 78 or r9,r9,r8 > df0: 7d 43 53 78 mr r3,r10 > df4: 90 04 00 00 stw r0,0(r4) > df8: 7d 29 1b 78 or r9,r9,r3 > dfc: 7d 43 53 78 mr r3,r10 > e00: b1 64 00 04 sth r11,4(r4) > e04: 7d 29 1b 78 or r9,r9,r3 > e08: 7d 43 53 78 mr r3,r10 > e0c: 98 a4 00 06 stb r5,6(r4) > e10: 7d 23 1b 78 or r3,r9,r3 > e14: 90 c4 00 08 stw r6,8(r4) > e18: 90 e4 00 0c stw r7,12(r4) > e1c: 7c 63 53 78 or r3,r3,r10 > e20: 4e 80 00 20 blr > > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> > Reviewed-by: Segher Boessenkool <segher@kernel.crashing.org> > --- > v2: > - Added <> modifier in __put_user_asm() and __get_user_asm() > - Removed %U2 in __put_user_asm2() and __get_user_asm2() > - Reworded the commit log > --- > arch/powerpc/include/asm/uaccess.h | 28 ++++++++++++++-------------- > 1 file changed, 14 insertions(+), 14 deletions(-) > > diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h > index 7c811442b607..9365b59495a2 100644 > --- a/arch/powerpc/include/asm/uaccess.h > +++ b/arch/powerpc/include/asm/uaccess.h > @@ -114,7 +114,7 @@ extern long __put_user_bad(void); > */ > #define __put_user_asm(x, addr, err, op) \ > __asm__ __volatile__( \ > - "1: " op " %1,0(%2) # put_user\n" \ > + "1: " op "%U2%X2 %1,%2 # put_user\n" \ > "2:\n" \ > ".section .fixup,\"ax\"\n" \ > "3: li %0,%3\n" \ > @@ -122,7 +122,7 @@ extern long __put_user_bad(void); > ".previous\n" \ > EX_TABLE(1b, 3b) \ > : "=r" (err) \ > - : "r" (x), "b" (addr), "i" (-EFAULT), "0" (err)) > + : "r" (x), "m<>" (*addr), "i" (-EFAULT), "0" (err)) > > #ifdef __powerpc64__ > #define __put_user_asm2(x, ptr, retval) \ > @@ -130,8 +130,8 @@ extern long __put_user_bad(void); > #else /* __powerpc64__ */ > #define __put_user_asm2(x, addr, err) \ > __asm__ __volatile__( \ > - "1: stw %1,0(%2)\n" \ > - "2: stw %1+1,4(%2)\n" \ > + "1: stw%X2 %1,%2\n" \ > + "2: stw%X2 %L1,%L2\n" \ > "3:\n" \ > ".section .fixup,\"ax\"\n" \ > "4: li %0,%3\n" \ > @@ -140,7 +140,7 @@ extern long __put_user_bad(void); > EX_TABLE(1b, 4b) \ > EX_TABLE(2b, 4b) \ > : "=r" (err) \ > - : "r" (x), "b" (addr), "i" (-EFAULT), "0" (err)) > + : "r" (x), "m" (*addr), "i" (-EFAULT), "0" (err)) > #endif /* __powerpc64__ */ > > #define __put_user_size_allowed(x, ptr, size, retval) \ > @@ -260,7 +260,7 @@ extern long __get_user_bad(void); > > #define __get_user_asm(x, addr, err, op) \ > __asm__ __volatile__( \ > - "1: "op" %1,0(%2) # get_user\n" \ > + "1: "op"%U2%X2 %1, %2 # get_user\n" \ > "2:\n" \ > ".section .fixup,\"ax\"\n" \ > "3: li %0,%3\n" \ > @@ -269,7 +269,7 @@ extern long __get_user_bad(void); > ".previous\n" \ > EX_TABLE(1b, 3b) \ > : "=r" (err), "=r" (x) \ > - : "b" (addr), "i" (-EFAULT), "0" (err)) > + : "m<>" (*addr), "i" (-EFAULT), "0" (err)) > > #ifdef __powerpc64__ > #define __get_user_asm2(x, addr, err) \ > @@ -277,8 +277,8 @@ extern long __get_user_bad(void); > #else /* __powerpc64__ */ > #define __get_user_asm2(x, addr, err) \ > __asm__ __volatile__( \ > - "1: lwz %1,0(%2)\n" \ > - "2: lwz %1+1,4(%2)\n" \ > + "1: lwz%X2 %1, %2\n" \ > + "2: lwz%X2 %L1, %L2\n" \ > "3:\n" \ > ".section .fixup,\"ax\"\n" \ > "4: li %0,%3\n" \ > @@ -289,7 +289,7 @@ extern long __get_user_bad(void); > EX_TABLE(1b, 4b) \ > EX_TABLE(2b, 4b) \ > : "=r" (err), "=&r" (x) \ > - : "b" (addr), "i" (-EFAULT), "0" (err)) > + : "m" (*addr), "i" (-EFAULT), "0" (err)) > #endif /* __powerpc64__ */ > > #define __get_user_size_allowed(x, ptr, size, retval) \ > @@ -299,10 +299,10 @@ do { \ > if (size > sizeof(x)) \ > (x) = __get_user_bad(); \ > switch (size) { \ > - case 1: __get_user_asm(x, ptr, retval, "lbz"); break; \ > - case 2: __get_user_asm(x, ptr, retval, "lhz"); break; \ > - case 4: __get_user_asm(x, ptr, retval, "lwz"); break; \ > - case 8: __get_user_asm2(x, ptr, retval); break; \ > + 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(); \ > } \ > } while (0) > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user() 2020-06-29 6:52 ` Christophe Leroy @ 2020-06-29 11:27 ` Michael Ellerman 2020-06-30 1:19 ` Michael Ellerman 0 siblings, 1 reply; 14+ messages in thread From: Michael Ellerman @ 2020-06-29 11:27 UTC (permalink / raw) To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras, npiggin, segher Cc: linuxppc-dev, linux-kernel Christophe Leroy <christophe.leroy@csgroup.eu> writes: > Hi Michael, > > I see this patch is marked as "defered" in patchwork, but I can't see > any related discussion. Is it normal ? Because it uses the "m<>" constraint which didn't work on GCC 4.6. https://github.com/linuxppc/issues/issues/297 So we should be able to pick it up for v5.9 hopefully. cheers > Le 16/04/2020 à 14:39, Christophe Leroy a écrit : >> At the time being, __put_user()/__get_user() and friends only use >> D-form addressing, with 0 offset. Ex: >> >> lwz reg1, 0(reg2) >> >> Give the compiler the opportunity to use other adressing modes >> whenever possible, to get more optimised code. >> >> Hereunder is a small exemple: >> >> struct test { >> u32 item1; >> u16 item2; >> u8 item3; >> u64 item4; >> }; >> >> int set_test_user(struct test __user *from, struct test __user *to) >> { >> int err; >> u32 item1; >> u16 item2; >> u8 item3; >> u64 item4; >> >> err = __get_user(item1, &from->item1); >> err |= __get_user(item2, &from->item2); >> err |= __get_user(item3, &from->item3); >> err |= __get_user(item4, &from->item4); >> >> err |= __put_user(item1, &to->item1); >> err |= __put_user(item2, &to->item2); >> err |= __put_user(item3, &to->item3); >> err |= __put_user(item4, &to->item4); >> >> return err; >> } >> >> Before the patch: >> >> 00000df0 <set_test_user>: >> df0: 94 21 ff f0 stwu r1,-16(r1) >> df4: 39 40 00 00 li r10,0 >> df8: 93 c1 00 08 stw r30,8(r1) >> dfc: 93 e1 00 0c stw r31,12(r1) >> e00: 7d 49 53 78 mr r9,r10 >> e04: 80 a3 00 00 lwz r5,0(r3) >> e08: 38 e3 00 04 addi r7,r3,4 >> e0c: 7d 46 53 78 mr r6,r10 >> e10: a0 e7 00 00 lhz r7,0(r7) >> e14: 7d 29 33 78 or r9,r9,r6 >> e18: 39 03 00 06 addi r8,r3,6 >> e1c: 7d 46 53 78 mr r6,r10 >> e20: 89 08 00 00 lbz r8,0(r8) >> e24: 7d 29 33 78 or r9,r9,r6 >> e28: 38 63 00 08 addi r3,r3,8 >> e2c: 7d 46 53 78 mr r6,r10 >> e30: 83 c3 00 00 lwz r30,0(r3) >> e34: 83 e3 00 04 lwz r31,4(r3) >> e38: 7d 29 33 78 or r9,r9,r6 >> e3c: 7d 43 53 78 mr r3,r10 >> e40: 90 a4 00 00 stw r5,0(r4) >> e44: 7d 29 1b 78 or r9,r9,r3 >> e48: 38 c4 00 04 addi r6,r4,4 >> e4c: 7d 43 53 78 mr r3,r10 >> e50: b0 e6 00 00 sth r7,0(r6) >> e54: 7d 29 1b 78 or r9,r9,r3 >> e58: 38 e4 00 06 addi r7,r4,6 >> e5c: 7d 43 53 78 mr r3,r10 >> e60: 99 07 00 00 stb r8,0(r7) >> e64: 7d 23 1b 78 or r3,r9,r3 >> e68: 38 84 00 08 addi r4,r4,8 >> e6c: 93 c4 00 00 stw r30,0(r4) >> e70: 93 e4 00 04 stw r31,4(r4) >> e74: 7c 63 53 78 or r3,r3,r10 >> e78: 83 c1 00 08 lwz r30,8(r1) >> e7c: 83 e1 00 0c lwz r31,12(r1) >> e80: 38 21 00 10 addi r1,r1,16 >> e84: 4e 80 00 20 blr >> >> After the patch: >> >> 00000dbc <set_test_user>: >> dbc: 39 40 00 00 li r10,0 >> dc0: 7d 49 53 78 mr r9,r10 >> dc4: 80 03 00 00 lwz r0,0(r3) >> dc8: 7d 48 53 78 mr r8,r10 >> dcc: a1 63 00 04 lhz r11,4(r3) >> dd0: 7d 29 43 78 or r9,r9,r8 >> dd4: 7d 48 53 78 mr r8,r10 >> dd8: 88 a3 00 06 lbz r5,6(r3) >> ddc: 7d 29 43 78 or r9,r9,r8 >> de0: 7d 48 53 78 mr r8,r10 >> de4: 80 c3 00 08 lwz r6,8(r3) >> de8: 80 e3 00 0c lwz r7,12(r3) >> dec: 7d 29 43 78 or r9,r9,r8 >> df0: 7d 43 53 78 mr r3,r10 >> df4: 90 04 00 00 stw r0,0(r4) >> df8: 7d 29 1b 78 or r9,r9,r3 >> dfc: 7d 43 53 78 mr r3,r10 >> e00: b1 64 00 04 sth r11,4(r4) >> e04: 7d 29 1b 78 or r9,r9,r3 >> e08: 7d 43 53 78 mr r3,r10 >> e0c: 98 a4 00 06 stb r5,6(r4) >> e10: 7d 23 1b 78 or r3,r9,r3 >> e14: 90 c4 00 08 stw r6,8(r4) >> e18: 90 e4 00 0c stw r7,12(r4) >> e1c: 7c 63 53 78 or r3,r3,r10 >> e20: 4e 80 00 20 blr >> >> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> >> Reviewed-by: Segher Boessenkool <segher@kernel.crashing.org> >> --- >> v2: >> - Added <> modifier in __put_user_asm() and __get_user_asm() >> - Removed %U2 in __put_user_asm2() and __get_user_asm2() >> - Reworded the commit log >> --- >> arch/powerpc/include/asm/uaccess.h | 28 ++++++++++++++-------------- >> 1 file changed, 14 insertions(+), 14 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h >> index 7c811442b607..9365b59495a2 100644 >> --- a/arch/powerpc/include/asm/uaccess.h >> +++ b/arch/powerpc/include/asm/uaccess.h >> @@ -114,7 +114,7 @@ extern long __put_user_bad(void); >> */ >> #define __put_user_asm(x, addr, err, op) \ >> __asm__ __volatile__( \ >> - "1: " op " %1,0(%2) # put_user\n" \ >> + "1: " op "%U2%X2 %1,%2 # put_user\n" \ >> "2:\n" \ >> ".section .fixup,\"ax\"\n" \ >> "3: li %0,%3\n" \ >> @@ -122,7 +122,7 @@ extern long __put_user_bad(void); >> ".previous\n" \ >> EX_TABLE(1b, 3b) \ >> : "=r" (err) \ >> - : "r" (x), "b" (addr), "i" (-EFAULT), "0" (err)) >> + : "r" (x), "m<>" (*addr), "i" (-EFAULT), "0" (err)) >> >> #ifdef __powerpc64__ >> #define __put_user_asm2(x, ptr, retval) \ >> @@ -130,8 +130,8 @@ extern long __put_user_bad(void); >> #else /* __powerpc64__ */ >> #define __put_user_asm2(x, addr, err) \ >> __asm__ __volatile__( \ >> - "1: stw %1,0(%2)\n" \ >> - "2: stw %1+1,4(%2)\n" \ >> + "1: stw%X2 %1,%2\n" \ >> + "2: stw%X2 %L1,%L2\n" \ >> "3:\n" \ >> ".section .fixup,\"ax\"\n" \ >> "4: li %0,%3\n" \ >> @@ -140,7 +140,7 @@ extern long __put_user_bad(void); >> EX_TABLE(1b, 4b) \ >> EX_TABLE(2b, 4b) \ >> : "=r" (err) \ >> - : "r" (x), "b" (addr), "i" (-EFAULT), "0" (err)) >> + : "r" (x), "m" (*addr), "i" (-EFAULT), "0" (err)) >> #endif /* __powerpc64__ */ >> >> #define __put_user_size_allowed(x, ptr, size, retval) \ >> @@ -260,7 +260,7 @@ extern long __get_user_bad(void); >> >> #define __get_user_asm(x, addr, err, op) \ >> __asm__ __volatile__( \ >> - "1: "op" %1,0(%2) # get_user\n" \ >> + "1: "op"%U2%X2 %1, %2 # get_user\n" \ >> "2:\n" \ >> ".section .fixup,\"ax\"\n" \ >> "3: li %0,%3\n" \ >> @@ -269,7 +269,7 @@ extern long __get_user_bad(void); >> ".previous\n" \ >> EX_TABLE(1b, 3b) \ >> : "=r" (err), "=r" (x) \ >> - : "b" (addr), "i" (-EFAULT), "0" (err)) >> + : "m<>" (*addr), "i" (-EFAULT), "0" (err)) >> >> #ifdef __powerpc64__ >> #define __get_user_asm2(x, addr, err) \ >> @@ -277,8 +277,8 @@ extern long __get_user_bad(void); >> #else /* __powerpc64__ */ >> #define __get_user_asm2(x, addr, err) \ >> __asm__ __volatile__( \ >> - "1: lwz %1,0(%2)\n" \ >> - "2: lwz %1+1,4(%2)\n" \ >> + "1: lwz%X2 %1, %2\n" \ >> + "2: lwz%X2 %L1, %L2\n" \ >> "3:\n" \ >> ".section .fixup,\"ax\"\n" \ >> "4: li %0,%3\n" \ >> @@ -289,7 +289,7 @@ extern long __get_user_bad(void); >> EX_TABLE(1b, 4b) \ >> EX_TABLE(2b, 4b) \ >> : "=r" (err), "=&r" (x) \ >> - : "b" (addr), "i" (-EFAULT), "0" (err)) >> + : "m" (*addr), "i" (-EFAULT), "0" (err)) >> #endif /* __powerpc64__ */ >> >> #define __get_user_size_allowed(x, ptr, size, retval) \ >> @@ -299,10 +299,10 @@ do { \ >> if (size > sizeof(x)) \ >> (x) = __get_user_bad(); \ >> switch (size) { \ >> - case 1: __get_user_asm(x, ptr, retval, "lbz"); break; \ >> - case 2: __get_user_asm(x, ptr, retval, "lhz"); break; \ >> - case 4: __get_user_asm(x, ptr, retval, "lwz"); break; \ >> - case 8: __get_user_asm2(x, ptr, retval); break; \ >> + 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(); \ >> } \ >> } while (0) >> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user() 2020-06-29 11:27 ` Michael Ellerman @ 2020-06-30 1:19 ` Michael Ellerman 2020-06-30 14:55 ` Christophe Leroy 2020-07-07 12:44 ` Christophe Leroy 0 siblings, 2 replies; 14+ messages in thread From: Michael Ellerman @ 2020-06-30 1:19 UTC (permalink / raw) To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras, npiggin, segher Cc: linuxppc-dev, linux-kernel Michael Ellerman <mpe@ellerman.id.au> writes: > Christophe Leroy <christophe.leroy@csgroup.eu> writes: >> Hi Michael, >> >> I see this patch is marked as "defered" in patchwork, but I can't see >> any related discussion. Is it normal ? > > Because it uses the "m<>" constraint which didn't work on GCC 4.6. > > https://github.com/linuxppc/issues/issues/297 > > So we should be able to pick it up for v5.9 hopefully. It seems to break the build with the kernel.org 4.9.4 compiler and corenet64_smp_defconfig: + make -s CC=powerpc64-linux-gnu-gcc -j 160 In file included from /linux/include/linux/uaccess.h:11:0, from /linux/include/linux/sched/task.h:11, from /linux/include/linux/sched/signal.h:9, from /linux/include/linux/rcuwait.h:6, from /linux/include/linux/percpu-rwsem.h:7, from /linux/include/linux/fs.h:33, from /linux/include/linux/huge_mm.h:8, from /linux/include/linux/mm.h:675, from /linux/arch/powerpc/kernel/signal_32.c:17: /linux/arch/powerpc/kernel/signal_32.c: In function 'save_user_regs.isra.14.constprop': /linux/arch/powerpc/include/asm/uaccess.h:161:2: error: 'asm' operand has impossible constraints __asm__ __volatile__( \ ^ /linux/arch/powerpc/include/asm/uaccess.h:197:12: note: in expansion of macro '__put_user_asm' case 4: __put_user_asm(x, ptr, retval, "stw"); break; \ ^ /linux/arch/powerpc/include/asm/uaccess.h:206:2: note: in expansion of macro '__put_user_size_allowed' __put_user_size_allowed(x, ptr, size, retval); \ ^ /linux/arch/powerpc/include/asm/uaccess.h:220:2: note: in expansion of macro '__put_user_size' __put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \ ^ /linux/arch/powerpc/include/asm/uaccess.h:96:2: note: in expansion of macro '__put_user_nocheck' __put_user_nocheck((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr))) ^ /linux/arch/powerpc/kernel/signal_32.c:120:7: note: in expansion of macro '__put_user' if (__put_user((unsigned int)gregs[i], &frame->mc_gregs[i])) ^ /linux/scripts/Makefile.build:280: recipe for target 'arch/powerpc/kernel/signal_32.o' failed make[3]: *** [arch/powerpc/kernel/signal_32.o] Error 1 make[3]: *** Waiting for unfinished jobs.... In file included from /linux/include/linux/uaccess.h:11:0, from /linux/include/linux/sched/task.h:11, from /linux/include/linux/sched/signal.h:9, from /linux/include/linux/rcuwait.h:6, from /linux/include/linux/percpu-rwsem.h:7, from /linux/include/linux/fs.h:33, from /linux/include/linux/huge_mm.h:8, from /linux/include/linux/mm.h:675, from /linux/arch/powerpc/kernel/signal_64.c:12: /linux/arch/powerpc/kernel/signal_64.c: In function '__se_sys_swapcontext': /linux/arch/powerpc/include/asm/uaccess.h:319:2: error: 'asm' operand has impossible constraints __asm__ __volatile__( \ ^ /linux/arch/powerpc/include/asm/uaccess.h:359:10: note: in expansion of macro '__get_user_asm' case 1: __get_user_asm(x, (u8 __user *)ptr, retval, "lbz"); break; \ ^ /linux/arch/powerpc/include/asm/uaccess.h:370:2: note: in expansion of macro '__get_user_size_allowed' __get_user_size_allowed(x, ptr, size, retval); \ ^ /linux/arch/powerpc/include/asm/uaccess.h:393:3: note: in expansion of macro '__get_user_size' __get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \ ^ /linux/arch/powerpc/include/asm/uaccess.h:94:2: note: in expansion of macro '__get_user_nocheck' __get_user_nocheck((x), (ptr), sizeof(*(ptr)), true) ^ /linux/arch/powerpc/kernel/signal_64.c:672:9: note: in expansion of macro '__get_user' || __get_user(tmp, (u8 __user *) new_ctx + ctx_size - 1)) ^ /linux/scripts/Makefile.build:280: recipe for target 'arch/powerpc/kernel/signal_64.o' failed make[3]: *** [arch/powerpc/kernel/signal_64.o] Error 1 /linux/scripts/Makefile.build:497: recipe for target 'arch/powerpc/kernel' failed make[2]: *** [arch/powerpc/kernel] Error 2 /linux/Makefile:1756: recipe for target 'arch/powerpc' failed make[1]: *** [arch/powerpc] Error 2 Makefile:185: recipe for target '__sub-make' failed make: *** [__sub-make] Error 2 cheers ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user() 2020-06-30 1:19 ` Michael Ellerman @ 2020-06-30 14:55 ` Christophe Leroy 2020-06-30 16:33 ` Segher Boessenkool 2020-07-07 12:44 ` Christophe Leroy 1 sibling, 1 reply; 14+ messages in thread From: Christophe Leroy @ 2020-06-30 14:55 UTC (permalink / raw) To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, npiggin, segher Cc: linuxppc-dev, linux-kernel Le 30/06/2020 à 03:19, Michael Ellerman a écrit : > Michael Ellerman <mpe@ellerman.id.au> writes: >> Christophe Leroy <christophe.leroy@csgroup.eu> writes: >>> Hi Michael, >>> >>> I see this patch is marked as "defered" in patchwork, but I can't see >>> any related discussion. Is it normal ? >> >> Because it uses the "m<>" constraint which didn't work on GCC 4.6. >> >> https://github.com/linuxppc/issues/issues/297 >> >> So we should be able to pick it up for v5.9 hopefully. > > It seems to break the build with the kernel.org 4.9.4 compiler and > corenet64_smp_defconfig: Looks like 4.9.4 doesn't accept "m<>" constraint either. Changing it to "m" make it build. Christophe > > + make -s CC=powerpc64-linux-gnu-gcc -j 160 > In file included from /linux/include/linux/uaccess.h:11:0, > from /linux/include/linux/sched/task.h:11, > from /linux/include/linux/sched/signal.h:9, > from /linux/include/linux/rcuwait.h:6, > from /linux/include/linux/percpu-rwsem.h:7, > from /linux/include/linux/fs.h:33, > from /linux/include/linux/huge_mm.h:8, > from /linux/include/linux/mm.h:675, > from /linux/arch/powerpc/kernel/signal_32.c:17: > /linux/arch/powerpc/kernel/signal_32.c: In function 'save_user_regs.isra.14.constprop': > /linux/arch/powerpc/include/asm/uaccess.h:161:2: error: 'asm' operand has impossible constraints > __asm__ __volatile__( \ > ^ > /linux/arch/powerpc/include/asm/uaccess.h:197:12: note: in expansion of macro '__put_user_asm' > case 4: __put_user_asm(x, ptr, retval, "stw"); break; \ > ^ > /linux/arch/powerpc/include/asm/uaccess.h:206:2: note: in expansion of macro '__put_user_size_allowed' > __put_user_size_allowed(x, ptr, size, retval); \ > ^ > /linux/arch/powerpc/include/asm/uaccess.h:220:2: note: in expansion of macro '__put_user_size' > __put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \ > ^ > /linux/arch/powerpc/include/asm/uaccess.h:96:2: note: in expansion of macro '__put_user_nocheck' > __put_user_nocheck((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr))) > ^ > /linux/arch/powerpc/kernel/signal_32.c:120:7: note: in expansion of macro '__put_user' > if (__put_user((unsigned int)gregs[i], &frame->mc_gregs[i])) > ^ > /linux/scripts/Makefile.build:280: recipe for target 'arch/powerpc/kernel/signal_32.o' failed > make[3]: *** [arch/powerpc/kernel/signal_32.o] Error 1 > make[3]: *** Waiting for unfinished jobs.... > In file included from /linux/include/linux/uaccess.h:11:0, > from /linux/include/linux/sched/task.h:11, > from /linux/include/linux/sched/signal.h:9, > from /linux/include/linux/rcuwait.h:6, > from /linux/include/linux/percpu-rwsem.h:7, > from /linux/include/linux/fs.h:33, > from /linux/include/linux/huge_mm.h:8, > from /linux/include/linux/mm.h:675, > from /linux/arch/powerpc/kernel/signal_64.c:12: > /linux/arch/powerpc/kernel/signal_64.c: In function '__se_sys_swapcontext': > /linux/arch/powerpc/include/asm/uaccess.h:319:2: error: 'asm' operand has impossible constraints > __asm__ __volatile__( \ > ^ > /linux/arch/powerpc/include/asm/uaccess.h:359:10: note: in expansion of macro '__get_user_asm' > case 1: __get_user_asm(x, (u8 __user *)ptr, retval, "lbz"); break; \ > ^ > /linux/arch/powerpc/include/asm/uaccess.h:370:2: note: in expansion of macro '__get_user_size_allowed' > __get_user_size_allowed(x, ptr, size, retval); \ > ^ > /linux/arch/powerpc/include/asm/uaccess.h:393:3: note: in expansion of macro '__get_user_size' > __get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \ > ^ > /linux/arch/powerpc/include/asm/uaccess.h:94:2: note: in expansion of macro '__get_user_nocheck' > __get_user_nocheck((x), (ptr), sizeof(*(ptr)), true) > ^ > /linux/arch/powerpc/kernel/signal_64.c:672:9: note: in expansion of macro '__get_user' > || __get_user(tmp, (u8 __user *) new_ctx + ctx_size - 1)) > ^ > /linux/scripts/Makefile.build:280: recipe for target 'arch/powerpc/kernel/signal_64.o' failed > make[3]: *** [arch/powerpc/kernel/signal_64.o] Error 1 > /linux/scripts/Makefile.build:497: recipe for target 'arch/powerpc/kernel' failed > make[2]: *** [arch/powerpc/kernel] Error 2 > /linux/Makefile:1756: recipe for target 'arch/powerpc' failed > make[1]: *** [arch/powerpc] Error 2 > Makefile:185: recipe for target '__sub-make' failed > make: *** [__sub-make] Error 2 > > > cheers > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user() 2020-06-30 14:55 ` Christophe Leroy @ 2020-06-30 16:33 ` Segher Boessenkool 2020-06-30 18:53 ` Christophe Leroy 0 siblings, 1 reply; 14+ messages in thread From: Segher Boessenkool @ 2020-06-30 16:33 UTC (permalink / raw) To: Christophe Leroy Cc: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, npiggin, linuxppc-dev, linux-kernel On Tue, Jun 30, 2020 at 04:55:05PM +0200, Christophe Leroy wrote: > Le 30/06/2020 à 03:19, Michael Ellerman a écrit : > >Michael Ellerman <mpe@ellerman.id.au> writes: > >>Because it uses the "m<>" constraint which didn't work on GCC 4.6. > >> > >>https://github.com/linuxppc/issues/issues/297 > >> > >>So we should be able to pick it up for v5.9 hopefully. > > > >It seems to break the build with the kernel.org 4.9.4 compiler and > >corenet64_smp_defconfig: > > Looks like 4.9.4 doesn't accept "m<>" constraint either. The evidence contradicts this assertion. > Changing it to "m" make it build. But that just means something else is wrong. > >+ make -s CC=powerpc64-linux-gnu-gcc -j 160 > >In file included from /linux/include/linux/uaccess.h:11:0, > > from /linux/include/linux/sched/task.h:11, > > from /linux/include/linux/sched/signal.h:9, > > from /linux/include/linux/rcuwait.h:6, > > from /linux/include/linux/percpu-rwsem.h:7, > > from /linux/include/linux/fs.h:33, > > from /linux/include/linux/huge_mm.h:8, > > from /linux/include/linux/mm.h:675, > > from /linux/arch/powerpc/kernel/signal_32.c:17: > >/linux/arch/powerpc/kernel/signal_32.c: In function > >'save_user_regs.isra.14.constprop': > >/linux/arch/powerpc/include/asm/uaccess.h:161:2: error: 'asm' operand has > >impossible constraints > > __asm__ __volatile__( \ > > ^ > >/linux/arch/powerpc/include/asm/uaccess.h:197:12: note: in expansion of > >macro '__put_user_asm' > > case 4: __put_user_asm(x, ptr, retval, "stw"); break; \ > > ^ > >/linux/arch/powerpc/include/asm/uaccess.h:206:2: note: in expansion of > >macro '__put_user_size_allowed' > > __put_user_size_allowed(x, ptr, size, retval); \ > > ^ > >/linux/arch/powerpc/include/asm/uaccess.h:220:2: note: in expansion of > >macro '__put_user_size' > > __put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \ > > ^ > >/linux/arch/powerpc/include/asm/uaccess.h:96:2: note: in expansion of > >macro '__put_user_nocheck' > > __put_user_nocheck((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr))) > > ^ > >/linux/arch/powerpc/kernel/signal_32.c:120:7: note: in expansion of macro > >'__put_user' > > if (__put_user((unsigned int)gregs[i], &frame->mc_gregs[i])) > > ^ Can we see what that was after the macro jungle? Like, the actual preprocessed code? Also, what GCC version *does* work on this? Segher ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user() 2020-06-30 16:33 ` Segher Boessenkool @ 2020-06-30 18:53 ` Christophe Leroy 2020-06-30 21:18 ` Segher Boessenkool 0 siblings, 1 reply; 14+ messages in thread From: Christophe Leroy @ 2020-06-30 18:53 UTC (permalink / raw) To: Segher Boessenkool Cc: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, npiggin, linuxppc-dev, linux-kernel On 06/30/2020 04:33 PM, Segher Boessenkool wrote: > On Tue, Jun 30, 2020 at 04:55:05PM +0200, Christophe Leroy wrote: >> Le 30/06/2020 à 03:19, Michael Ellerman a écrit : >>> Michael Ellerman <mpe@ellerman.id.au> writes: >>>> Because it uses the "m<>" constraint which didn't work on GCC 4.6. >>>> >>>> https://github.com/linuxppc/issues/issues/297 >>>> >>>> So we should be able to pick it up for v5.9 hopefully. >>> >>> It seems to break the build with the kernel.org 4.9.4 compiler and >>> corenet64_smp_defconfig: >> >> Looks like 4.9.4 doesn't accept "m<>" constraint either. > > The evidence contradicts this assertion. > >> Changing it to "m" make it build. > > But that just means something else is wrong. > >>> + make -s CC=powerpc64-linux-gnu-gcc -j 160 >>> In file included from /linux/include/linux/uaccess.h:11:0, >>> from /linux/include/linux/sched/task.h:11, >>> from /linux/include/linux/sched/signal.h:9, >>> from /linux/include/linux/rcuwait.h:6, >>> from /linux/include/linux/percpu-rwsem.h:7, >>> from /linux/include/linux/fs.h:33, >>> from /linux/include/linux/huge_mm.h:8, >>> from /linux/include/linux/mm.h:675, >>> from /linux/arch/powerpc/kernel/signal_32.c:17: >>> /linux/arch/powerpc/kernel/signal_32.c: In function >>> 'save_user_regs.isra.14.constprop': >>> /linux/arch/powerpc/include/asm/uaccess.h:161:2: error: 'asm' operand has >>> impossible constraints >>> __asm__ __volatile__( \ >>> ^ >>> /linux/arch/powerpc/include/asm/uaccess.h:197:12: note: in expansion of >>> macro '__put_user_asm' >>> case 4: __put_user_asm(x, ptr, retval, "stw"); break; \ >>> ^ >>> /linux/arch/powerpc/include/asm/uaccess.h:206:2: note: in expansion of >>> macro '__put_user_size_allowed' >>> __put_user_size_allowed(x, ptr, size, retval); \ >>> ^ >>> /linux/arch/powerpc/include/asm/uaccess.h:220:2: note: in expansion of >>> macro '__put_user_size' >>> __put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \ >>> ^ >>> /linux/arch/powerpc/include/asm/uaccess.h:96:2: note: in expansion of >>> macro '__put_user_nocheck' >>> __put_user_nocheck((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr))) >>> ^ >>> /linux/arch/powerpc/kernel/signal_32.c:120:7: note: in expansion of macro >>> '__put_user' >>> if (__put_user((unsigned int)gregs[i], &frame->mc_gregs[i])) >>> ^ > > Can we see what that was after the macro jungle? Like, the actual > preprocessed code? > Sorry for previous misunderstanding Here is the code: #define __put_user_asm(x, addr, err, op) \ __asm__ __volatile__( \ "1: " op "%U2%X2 %1,%2 # put_user\n" \ "2:\n" \ ".section .fixup,\"ax\"\n" \ "3: li %0,%3\n" \ " b 2b\n" \ ".previous\n" \ EX_TABLE(1b, 3b) \ : "=r" (err) \ : "r" (x), "m<>" (*addr), "i" (-EFAULT), "0" (err)) Christophe ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user() 2020-06-30 18:53 ` Christophe Leroy @ 2020-06-30 21:18 ` Segher Boessenkool 2020-07-01 7:05 ` Christophe Leroy 0 siblings, 1 reply; 14+ messages in thread From: Segher Boessenkool @ 2020-06-30 21:18 UTC (permalink / raw) To: Christophe Leroy Cc: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, npiggin, linuxppc-dev, linux-kernel Hi again, Thanks for your work so far! On Tue, Jun 30, 2020 at 06:53:39PM +0000, Christophe Leroy wrote: > On 06/30/2020 04:33 PM, Segher Boessenkool wrote: > >>>+ make -s CC=powerpc64-linux-gnu-gcc -j 160 > >>>In file included from /linux/include/linux/uaccess.h:11:0, > >>> from /linux/include/linux/sched/task.h:11, > >>> from /linux/include/linux/sched/signal.h:9, > >>> from /linux/include/linux/rcuwait.h:6, > >>> from /linux/include/linux/percpu-rwsem.h:7, > >>> from /linux/include/linux/fs.h:33, > >>> from /linux/include/linux/huge_mm.h:8, > >>> from /linux/include/linux/mm.h:675, > >>> from /linux/arch/powerpc/kernel/signal_32.c:17: > >>>/linux/arch/powerpc/kernel/signal_32.c: In function > >>>'save_user_regs.isra.14.constprop': > >>>/linux/arch/powerpc/include/asm/uaccess.h:161:2: error: 'asm' operand has > >>>impossible constraints > >>> __asm__ __volatile__( \ > >>> ^ > >>>/linux/arch/powerpc/include/asm/uaccess.h:197:12: note: in expansion of > >>>macro '__put_user_asm' > >>> case 4: __put_user_asm(x, ptr, retval, "stw"); break; \ > >>> ^ > >>>/linux/arch/powerpc/include/asm/uaccess.h:206:2: note: in expansion of > >>>macro '__put_user_size_allowed' > >>> __put_user_size_allowed(x, ptr, size, retval); \ > >>> ^ > >>>/linux/arch/powerpc/include/asm/uaccess.h:220:2: note: in expansion of > >>>macro '__put_user_size' > >>> __put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \ > >>> ^ > >>>/linux/arch/powerpc/include/asm/uaccess.h:96:2: note: in expansion of > >>>macro '__put_user_nocheck' > >>> __put_user_nocheck((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr))) > >>> ^ > >>>/linux/arch/powerpc/kernel/signal_32.c:120:7: note: in expansion of macro > >>>'__put_user' > >>> if (__put_user((unsigned int)gregs[i], &frame->mc_gregs[i])) > >>> ^ > > > >Can we see what that was after the macro jungle? Like, the actual > >preprocessed code? > > Sorry for previous misunderstanding > > Here is the code: > > #define __put_user_asm(x, addr, err, op) \ > __asm__ __volatile__( \ > "1: " op "%U2%X2 %1,%2 # put_user\n" \ > "2:\n" \ > ".section .fixup,\"ax\"\n" \ > "3: li %0,%3\n" \ > " b 2b\n" \ > ".previous\n" \ > EX_TABLE(1b, 3b) \ > : "=r" (err) \ > : "r" (x), "m<>" (*addr), "i" (-EFAULT), "0" (err)) Yeah I don't see it. I'll have to look at compiler debug dumps, but I don't have any working 4.9 around, and I cannot reproduce this with either older or newer compilers. It is complainig that constrain_operands just does not work *at all* on this "m<>" constraint apparently, which doesn't make much sense. I'll try later when I have more time, sorry :-/ Segher ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user() 2020-06-30 21:18 ` Segher Boessenkool @ 2020-07-01 7:05 ` Christophe Leroy 0 siblings, 0 replies; 14+ messages in thread From: Christophe Leroy @ 2020-07-01 7:05 UTC (permalink / raw) To: Segher Boessenkool Cc: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, npiggin, linuxppc-dev, linux-kernel Le 30/06/2020 à 23:18, Segher Boessenkool a écrit : > Hi again, > > Thanks for your work so far! > > On Tue, Jun 30, 2020 at 06:53:39PM +0000, Christophe Leroy wrote: >> On 06/30/2020 04:33 PM, Segher Boessenkool wrote: >>>>> + make -s CC=powerpc64-linux-gnu-gcc -j 160 >>>>> In file included from /linux/include/linux/uaccess.h:11:0, >>>>> from /linux/include/linux/sched/task.h:11, >>>>> from /linux/include/linux/sched/signal.h:9, >>>>> from /linux/include/linux/rcuwait.h:6, >>>>> from /linux/include/linux/percpu-rwsem.h:7, >>>>> from /linux/include/linux/fs.h:33, >>>>> from /linux/include/linux/huge_mm.h:8, >>>>> from /linux/include/linux/mm.h:675, >>>>> from /linux/arch/powerpc/kernel/signal_32.c:17: >>>>> /linux/arch/powerpc/kernel/signal_32.c: In function >>>>> 'save_user_regs.isra.14.constprop': >>>>> /linux/arch/powerpc/include/asm/uaccess.h:161:2: error: 'asm' operand has >>>>> impossible constraints >>>>> __asm__ __volatile__( \ >>>>> ^ >>>>> /linux/arch/powerpc/include/asm/uaccess.h:197:12: note: in expansion of >>>>> macro '__put_user_asm' >>>>> case 4: __put_user_asm(x, ptr, retval, "stw"); break; \ >>>>> ^ >>>>> /linux/arch/powerpc/include/asm/uaccess.h:206:2: note: in expansion of >>>>> macro '__put_user_size_allowed' >>>>> __put_user_size_allowed(x, ptr, size, retval); \ >>>>> ^ >>>>> /linux/arch/powerpc/include/asm/uaccess.h:220:2: note: in expansion of >>>>> macro '__put_user_size' >>>>> __put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \ >>>>> ^ >>>>> /linux/arch/powerpc/include/asm/uaccess.h:96:2: note: in expansion of >>>>> macro '__put_user_nocheck' >>>>> __put_user_nocheck((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr))) >>>>> ^ >>>>> /linux/arch/powerpc/kernel/signal_32.c:120:7: note: in expansion of macro >>>>> '__put_user' >>>>> if (__put_user((unsigned int)gregs[i], &frame->mc_gregs[i])) >>>>> ^ >>> >>> Can we see what that was after the macro jungle? Like, the actual >>> preprocessed code? >> >> Sorry for previous misunderstanding >> >> Here is the code: >> >> #define __put_user_asm(x, addr, err, op) \ >> __asm__ __volatile__( \ >> "1: " op "%U2%X2 %1,%2 # put_user\n" \ >> "2:\n" \ >> ".section .fixup,\"ax\"\n" \ >> "3: li %0,%3\n" \ >> " b 2b\n" \ >> ".previous\n" \ >> EX_TABLE(1b, 3b) \ >> : "=r" (err) \ >> : "r" (x), "m<>" (*addr), "i" (-EFAULT), "0" (err)) > > Yeah I don't see it. I'll have to look at compiler debug dumps, but I > don't have any working 4.9 around, and I cannot reproduce this with > either older or newer compilers. I reproduced it with 4.8.5 > > It is complainig that constrain_operands just does not work *at all* on > this "m<>" constraint apparently, which doesn't make much sense. > Here is a small reproducer: #include <linux/elf.h> #include <linux/ptrace.h> #include <linux/uaccess.h> struct mcontext { elf_gregset_t32 mc_gregs; elf_fpregset_t mc_fregs; unsigned int mc_pad[2]; elf_vrregset_t32 mc_vregs __attribute__((__aligned__(16))); elf_vsrreghalf_t32 mc_vsregs __attribute__((__aligned__(16))); }; int save_general_regs(struct pt_regs *regs, struct mcontext __user *frame) { elf_greg_t64 *gregs = (elf_greg_t64 *)regs; int i; for (i = 0; i <= PT_RESULT; i ++) { if (i == 14) i = 32; if (__put_user((unsigned int)gregs[i], &frame->mc_gregs[i])) return -EFAULT; } return 0; } If you remove the "if i == 14 ..." you get no failure. Preprocessor result: int save_general_regs(struct pt_regs *regs, struct mcontext *frame) { elf_greg_t64 *gregs = (elf_greg_t64 *)regs; int i; for (i = 0; i <= 43; i ++) { if (i == 14) i = 32; if (({ long __pu_err; __typeof__(*((&frame->mc_gregs[i]))) *__pu_addr = ((&frame->mc_gregs[i])); __typeof__(*((&frame->mc_gregs[i]))) __pu_val = ((__typeof__(*(&frame->mc_gregs[i])))((unsigned int)gregs[i])); __typeof__(sizeof(*(&frame->mc_gregs[i]))) __pu_size = (sizeof(*(&frame->mc_gregs[i]))); if (!(((unsigned long)__pu_addr) >= 0x8000000000000000ul)) might_fault(); (void)0; do { allow_write_to_user(__pu_addr, __pu_size); do { __pu_err = 0; switch (__pu_size) { case 1: __asm__ __volatile__( "1: " "stb" "%U2%X2 %1,%2 # put_user\n" "2:\n" ".section .fixup,\"ax\"\n" "3: li %0,%3\n" " b 2b\n" ".previous\n" ".section __ex_table,\"a\";" " " ".balign 4;" " " ".long (1b) - . ;" " " ".long (3b) - . ;" " " ".previous" " " : "=r" (__pu_err) : "r" (__pu_val), "m<>" (*__pu_addr), "i" (-14), "0" (__pu_err)); break; case 2: __asm__ __volatile__( "1: " "sth" "%U2%X2 %1,%2 # put_user\n" "2:\n" ".section .fixup,\"ax\"\n" "3: li %0,%3\n" " b 2b\n" ".previous\n" ".section __ex_table,\"a\";" " " ".balign 4;" " " ".long (1b) - . ;" " " ".long (3b) - . ;" " " ".previous" " " : "=r" (__pu_err) : "r" (__pu_val), "m<>" (*__pu_addr), "i" (-14), "0" (__pu_err)); break; case 4: __asm__ __volatile__( "1: " "stw" "%U2%X2 %1,%2 # put_user\n" "2:\n" ".section .fixup,\"ax\"\n" "3: li %0,%3\n" " b 2b\n" ".previous\n" ".section __ex_table,\"a\";" " " ".balign 4;" " " ".long (1b) - . ;" " " ".long (3b) - . ;" " " ".previous" " " : "=r" (__pu_err) : "r" (__pu_val), "m<>" (*__pu_addr), "i" (-14), "0" (__pu_err)); break; case 8: __asm__ __volatile__( "1: " "std" "%U2%X2 %1,%2 # put_user\n" "2:\n" ".section .fixup,\"ax\"\n" "3: li %0,%3\n" " b 2b\n" ".previous\n" ".section __ex_table,\"a\";" " " ".balign 4;" " " ".long (1b) - . ;" " " ".long (3b) - . ;" " " ".previous" " " : "=r" (__pu_err) : "r" (__pu_val), "m<>" (*__pu_addr), "i" (-14), "0" (__pu_err)); break; default: __put_user_bad(); } } while (0); prevent_write_to_user(__pu_addr, __pu_size); } while (0); __pu_err; })) return -14; } return 0; } Christophe ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user() 2020-06-30 1:19 ` Michael Ellerman 2020-06-30 14:55 ` Christophe Leroy @ 2020-07-07 12:44 ` Christophe Leroy 2020-07-07 19:02 ` Christophe Leroy 1 sibling, 1 reply; 14+ messages in thread From: Christophe Leroy @ 2020-07-07 12:44 UTC (permalink / raw) To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, npiggin, segher Cc: linuxppc-dev, linux-kernel Le 30/06/2020 à 03:19, Michael Ellerman a écrit : > Michael Ellerman <mpe@ellerman.id.au> writes: >> Christophe Leroy <christophe.leroy@csgroup.eu> writes: >>> Hi Michael, >>> >>> I see this patch is marked as "defered" in patchwork, but I can't see >>> any related discussion. Is it normal ? >> >> Because it uses the "m<>" constraint which didn't work on GCC 4.6. >> >> https://github.com/linuxppc/issues/issues/297 >> >> So we should be able to pick it up for v5.9 hopefully. > > It seems to break the build with the kernel.org 4.9.4 compiler and > corenet64_smp_defconfig: Most likely a GCC bug ? It seems the problem vanishes with patch https://patchwork.ozlabs.org/project/linuxppc-dev/patch/173de3b659fa3a5f126a0eb170522cccd909950f.1594125164.git.christophe.leroy@csgroup.eu/ Christophe ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user() 2020-07-07 12:44 ` Christophe Leroy @ 2020-07-07 19:02 ` Christophe Leroy 2020-07-08 4:49 ` Christophe Leroy 0 siblings, 1 reply; 14+ messages in thread From: Christophe Leroy @ 2020-07-07 19:02 UTC (permalink / raw) To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, npiggin, segher Cc: linuxppc-dev, linux-kernel Le 07/07/2020 à 14:44, Christophe Leroy a écrit : > > > Le 30/06/2020 à 03:19, Michael Ellerman a écrit : >> Michael Ellerman <mpe@ellerman.id.au> writes: >>> Christophe Leroy <christophe.leroy@csgroup.eu> writes: >>>> Hi Michael, >>>> >>>> I see this patch is marked as "defered" in patchwork, but I can't see >>>> any related discussion. Is it normal ? >>> >>> Because it uses the "m<>" constraint which didn't work on GCC 4.6. >>> >>> https://github.com/linuxppc/issues/issues/297 >>> >>> So we should be able to pick it up for v5.9 hopefully. >> >> It seems to break the build with the kernel.org 4.9.4 compiler and >> corenet64_smp_defconfig: > > Most likely a GCC bug ? > > It seems the problem vanishes with patch > https://patchwork.ozlabs.org/project/linuxppc-dev/patch/173de3b659fa3a5f126a0eb170522cccd909950f.1594125164.git.christophe.leroy@csgroup.eu/ > Same kind of issue in signal_64.c now. The following patch fixes it: https://patchwork.ozlabs.org/project/linuxppc-dev/patch/810bd8840ef990a200f58c9dea9abe767ca02a3a.1594146723.git.christophe.leroy@csgroup.eu/ Christophe ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user() 2020-07-07 19:02 ` Christophe Leroy @ 2020-07-08 4:49 ` Christophe Leroy 2020-08-12 12:32 ` Christophe Leroy 0 siblings, 1 reply; 14+ messages in thread From: Christophe Leroy @ 2020-07-08 4:49 UTC (permalink / raw) To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, npiggin, segher Cc: linuxppc-dev, linux-kernel Le 07/07/2020 à 21:02, Christophe Leroy a écrit : > > > Le 07/07/2020 à 14:44, Christophe Leroy a écrit : >> >> >> Le 30/06/2020 à 03:19, Michael Ellerman a écrit : >>> Michael Ellerman <mpe@ellerman.id.au> writes: >>>> Christophe Leroy <christophe.leroy@csgroup.eu> writes: >>>>> Hi Michael, >>>>> >>>>> I see this patch is marked as "defered" in patchwork, but I can't see >>>>> any related discussion. Is it normal ? >>>> >>>> Because it uses the "m<>" constraint which didn't work on GCC 4.6. >>>> >>>> https://github.com/linuxppc/issues/issues/297 >>>> >>>> So we should be able to pick it up for v5.9 hopefully. >>> >>> It seems to break the build with the kernel.org 4.9.4 compiler and >>> corenet64_smp_defconfig: >> >> Most likely a GCC bug ? >> >> It seems the problem vanishes with patch >> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/173de3b659fa3a5f126a0eb170522cccd909950f.1594125164.git.christophe.leroy@csgroup.eu/ >> > > Same kind of issue in signal_64.c now. > > The following patch fixes it: > https://patchwork.ozlabs.org/project/linuxppc-dev/patch/810bd8840ef990a200f58c9dea9abe767ca02a3a.1594146723.git.christophe.leroy@csgroup.eu/ > > This time I confirm, with the two above mentioned patches, it builds OK with 4.9, see http://kisskb.ellerman.id.au/kisskb/head/810bd8840ef990a200f58c9dea9abe767ca02a3a/ Christophe ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user() 2020-07-08 4:49 ` Christophe Leroy @ 2020-08-12 12:32 ` Christophe Leroy 2020-08-12 19:30 ` Segher Boessenkool 0 siblings, 1 reply; 14+ messages in thread From: Christophe Leroy @ 2020-08-12 12:32 UTC (permalink / raw) To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, npiggin, segher Cc: linuxppc-dev, linux-kernel Le 08/07/2020 à 06:49, Christophe Leroy a écrit : > > > Le 07/07/2020 à 21:02, Christophe Leroy a écrit : >> >> >> Le 07/07/2020 à 14:44, Christophe Leroy a écrit : >>> >>> >>> Le 30/06/2020 à 03:19, Michael Ellerman a écrit : >>>> Michael Ellerman <mpe@ellerman.id.au> writes: >>>>> Christophe Leroy <christophe.leroy@csgroup.eu> writes: >>>>>> Hi Michael, >>>>>> >>>>>> I see this patch is marked as "defered" in patchwork, but I can't see >>>>>> any related discussion. Is it normal ? >>>>> >>>>> Because it uses the "m<>" constraint which didn't work on GCC 4.6. >>>>> >>>>> https://github.com/linuxppc/issues/issues/297 >>>>> >>>>> So we should be able to pick it up for v5.9 hopefully. >>>> >>>> It seems to break the build with the kernel.org 4.9.4 compiler and >>>> corenet64_smp_defconfig: >>> >>> Most likely a GCC bug ? >>> >>> It seems the problem vanishes with patch >>> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/173de3b659fa3a5f126a0eb170522cccd909950f.1594125164.git.christophe.leroy@csgroup.eu/ >>> >> >> Same kind of issue in signal_64.c now. >> >> The following patch fixes it: >> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/810bd8840ef990a200f58c9dea9abe767ca02a3a.1594146723.git.christophe.leroy@csgroup.eu/ >> >> > > This time I confirm, with the two above mentioned patches, it builds OK > with 4.9, see > http://kisskb.ellerman.id.au/kisskb/head/810bd8840ef990a200f58c9dea9abe767ca02a3a/ > > I see you've merged those patches that make the issue disappear, yet not this patch yet. I guess you are still a bit chilly about it, so I split it in two parts for you to safely take patch 1 as soon as possible while handling the "m<>" constraint subject more carefully via https://github.com/linuxppc/issues/issues/297 in a later stage. Anyway, it seems that GCC doesn't make much use of the "m<>" and the pre-update form. Most of the benefit of flexible addressing seems to be achieved with patch 1 ie without the "m<>" constraint and update form. Christophe ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user() 2020-08-12 12:32 ` Christophe Leroy @ 2020-08-12 19:30 ` Segher Boessenkool 0 siblings, 0 replies; 14+ messages in thread From: Segher Boessenkool @ 2020-08-12 19:30 UTC (permalink / raw) To: Christophe Leroy Cc: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, npiggin, linuxppc-dev, linux-kernel On Wed, Aug 12, 2020 at 02:32:51PM +0200, Christophe Leroy wrote: > Anyway, it seems that GCC doesn't make much use of the "m<>" and the > pre-update form. GCC does not use update form outside of inner loops much. Did you expect anything else? > Most of the benefit of flexible addressing seems to be > achieved with patch 1 ie without the "m<>" constraint and update form. Yes. Segher ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-08-12 19:31 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-04-16 12:39 [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user() Christophe Leroy 2020-06-29 6:52 ` Christophe Leroy 2020-06-29 11:27 ` Michael Ellerman 2020-06-30 1:19 ` Michael Ellerman 2020-06-30 14:55 ` Christophe Leroy 2020-06-30 16:33 ` Segher Boessenkool 2020-06-30 18:53 ` Christophe Leroy 2020-06-30 21:18 ` Segher Boessenkool 2020-07-01 7:05 ` Christophe Leroy 2020-07-07 12:44 ` Christophe Leroy 2020-07-07 19:02 ` Christophe Leroy 2020-07-08 4:49 ` Christophe Leroy 2020-08-12 12:32 ` Christophe Leroy 2020-08-12 19:30 ` Segher Boessenkool
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox