linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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;
as well as URLs for NNTP newsgroup(s).