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