Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()

2020-07-07 Thread Christophe Leroy




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  writes:

Christophe Leroy  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.le...@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.le...@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


Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()

2020-07-07 Thread Christophe Leroy




Le 07/07/2020 à 14:44, Christophe Leroy a écrit :



Le 30/06/2020 à 03:19, Michael Ellerman a écrit :

Michael Ellerman  writes:

Christophe Leroy  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.le...@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.le...@csgroup.eu/


Christophe


Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()

2020-07-07 Thread Christophe Leroy




Le 30/06/2020 à 03:19, Michael Ellerman a écrit :

Michael Ellerman  writes:

Christophe Leroy  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.le...@csgroup.eu/ 



Christophe


Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()

2020-07-01 Thread Christophe Leroy




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 +, 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], >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 
#include 
#include 

struct mcontext {
elf_gregset_t32 mc_gregs;
elf_fpregset_t  mc_fregs;
unsigned intmc_pad[2];
elf_vrregset_t32mc_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], >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__(*((>mc_gregs[i]))) *__pu_addr 
= ((>mc_gregs[i])); __typeof__(*((>mc_gregs[i]))) __pu_val 
= ((__typeof__(*(>mc_gregs[i])))((unsigned int)gregs[i])); 
__typeof__(sizeof(*(>mc_gregs[i]))) __pu_size = 
(sizeof(*(>mc_gregs[i]))); if (!(((unsigned long)__pu_addr) >= 
0x8000ul)) 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" 

Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()

2020-06-30 Thread Segher Boessenkool
Hi again,

Thanks for your work so far!

On Tue, Jun 30, 2020 at 06:53:39PM +, 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], >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


Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()

2020-06-30 Thread Christophe Leroy




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  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], >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


Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()

2020-06-30 Thread Segher Boessenkool
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  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], >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


Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()

2020-06-30 Thread Christophe Leroy




Le 30/06/2020 à 03:19, Michael Ellerman a écrit :

Michael Ellerman  writes:

Christophe Leroy  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], >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



Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()

2020-06-29 Thread Michael Ellerman
Michael Ellerman  writes:
> Christophe Leroy  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], >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


Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()

2020-06-29 Thread Michael Ellerman
Christophe Leroy  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, >item1);
>>  err |= __get_user(item2, >item2);
>>  err |= __get_user(item3, >item3);
>>  err |= __get_user(item4, >item4);
>> 
>>  err |= __put_user(item1, >item1);
>>  err |= __put_user(item2, >item2);
>>  err |= __put_user(item3, >item3);
>>  err |= __put_user(item4, >item4);
>> 
>>  return err;
>> }
>> 
>> Before the patch:
>> 
>> 0df0 :
>>   df0:   94 21 ff f0 stwur1,-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 addir7,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 addir8,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 addir3,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 addir6,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 addir7,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 addir4,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 addir1,r1,16
>>   e84:   4e 80 00 20 blr
>> 
>> After the patch:
>> 
>> 0dbc :
>>   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 
>> Reviewed-by: Segher Boessenkool 
>> ---
>> 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 

Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()

2020-06-29 Thread Christophe Leroy

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, >item1);
err |= __get_user(item2, >item2);
err |= __get_user(item3, >item3);
err |= __get_user(item4, >item4);

err |= __put_user(item1, >item1);
err |= __put_user(item2, >item2);
err |= __put_user(item3, >item3);
err |= __put_user(item4, >item4);

return err;
}

Before the patch:

0df0 :
  df0:  94 21 ff f0 stwur1,-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 addir7,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 addir8,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 addir3,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 addir6,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 addir7,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 addir4,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 addir1,r1,16
  e84:  4e 80 00 20 blr

After the patch:

0dbc :
  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 
Reviewed-by: Segher Boessenkool 
---
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)