Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()
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
Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()
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 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/ 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
Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()
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()
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()
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()
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], &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 #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], &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) >= 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" ".sect
Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()
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], &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
Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()
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], &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
Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()
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], &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
Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()
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], &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
Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()
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], &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
Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()
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, &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: >> >> 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/power
Re: [PATCH v2] powerpc/uaccess: Use flexible addressing with __put_user()/__get_user()
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: 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_TA