Message ID | 58FF2D540200007800153D5E@prv-mh.provo.novell.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 25/04/17 10:04, Jan Beulich wrote: > Stub invocations need to have the space the stub occupies as an input, > to prevent the compiler from re-ordering (or omitting) writes to it. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
On 25/04/17 16:00, Andrew Cooper wrote: > On 25/04/17 10:04, Jan Beulich wrote: >> Stub invocations need to have the space the stub occupies as an input, >> to prevent the compiler from re-ordering (or omitting) writes to it. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Julien Grall <julien.grall@arm.com> >
On 04/25/2017 05:04 AM, Jan Beulich wrote: > Stub invocations need to have the space the stub occupies as an input, > to prevent the compiler from re-ordering (or omitting) writes to it. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- a/xen/arch/x86/x86_emulate/x86_emulate.c > +++ b/xen/arch/x86/x86_emulate/x86_emulate.c > @@ -837,7 +837,8 @@ do{ asm volatile ( > ".popsection\n\t" \ > _ASM_EXTABLE(.Lret%=, .Lfix%=) \ > : [exn] "+g" (res_), constraints, \ > - [stub] "rm" (stub.func) ); \ > + [stub] "rm" (stub.func), \ > + "m" (*(uint8_t(*)[MAX_INST_LEN + 1])stub.ptr) ); \ > if ( unlikely(~res_.raw) ) \ > { \ > gprintk(XENLOG_WARNING, \ > @@ -853,7 +854,8 @@ do{ asm volatile ( > #else > # define invoke_stub(pre, post, constraints...) \ > asm volatile ( pre "\n\tcall *%[stub]\n\t" post \ > - : constraints, [stub] "rm" (stub.func) ) > + : constraints, [stub] "rm" (stub.func), \ > + "m" (*(uint8_t(*)[MAX_INST_LEN + 1])stub.buf) ) > #endif > > #define emulate_stub(dst, src...) do { \ > This breaks on old compilers: FC-64 <build@build-mk2:~/xtt-x86_64/bootstrap/xen.git/tools/fuzz/x86_instruction_emulator> gcc --version gcc (GCC) 4.4.4 20100503 (Red Hat 4.4.4-2) Copyright (C) 2010 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. FC-64 <build@build-mk2:~/xtt-x86_64/bootstrap/xen.git/tools/fuzz/x86_instruction_emulator> pwd /home/build/xtt-x86_64/bootstrap/xen.git/tools/fuzz/x86_instruction_emulator FC-64 <build@build-mk2:~/xtt-x86_64/bootstrap/xen.git/tools/fuzz/x86_instruction_emulator> gcc -m64 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -g3 -O0 -fno-omit-frame-pointer -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MF .x86_emulate.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -I/home/build/xtt-x86_64/bootstrap/xen.git/tools/fuzz/x86_instruction_emulator/../../../tools/include -D__XEN_TOOLS__ -I. -c -o x86_emulate.o x86_emulate.c In file included from x86_emulate.c:157: x86_emulate/x86_emulate.c: In function ‘x86_emulate’: x86_emulate/x86_emulate.c:4085: error: memory input 3 is not directly addressable In file included from x86_emulate.c:157: x86_emulate/x86_emulate.c:4161: error: memory input 3 is not directly addressable In file included from x86_emulate.c:157: x86_emulate/x86_emulate.c:4226: error: memory input 5 is not directly addressable In file included from x86_emulate.c:157: x86_emulate/x86_emulate.c:4229: error: memory input 3 is not directly addressable In file included from x86_emulate.c:157: x86_emulate/x86_emulate.c:4279: error: memory input 5 is not directly addressable In file included from x86_emulate.c:157: x86_emulate/x86_emulate.c:4288: error: memory input 3 is not directly addressable In file included from x86_emulate.c:157: x86_emulate/x86_emulate.c:4353: error: memory input 3 is not directly addressable In file included from x86_emulate.c:157: x86_emulate/x86_emulate.c:4402: error: memory input 3 is not directly addressable In file included from x86_emulate.c:157: x86_emulate/x86_emulate.c:4465: error: memory input 3 is not directly addressable In file included from x86_emulate.c:157: x86_emulate/x86_emulate.c:4516: error: memory input 5 is not directly addressable In file included from x86_emulate.c:157: x86_emulate/x86_emulate.c:4522: error: memory input 3 is not directly addressable In file included from x86_emulate.c:157: x86_emulate/x86_emulate.c:5632: error: memory input 5 is not directly addressable In file included from x86_emulate.c:157: x86_emulate/x86_emulate.c:5679: error: memory input 8 is not directly addressable In file included from x86_emulate.c:157: x86_emulate/x86_emulate.c:5863: error: memory input 3 is not directly addressable x86_emulate/x86_emulate.c:6069: error: memory input 4 is not directly addressable x86_emulate/x86_emulate.c:6213: error: memory input 3 is not directly addressable In file included from x86_emulate.c:157: x86_emulate/x86_emulate.c:7029: error: memory input 3 is not directly addressable In file included from x86_emulate.c:157: x86_emulate/x86_emulate.c:7178: error: memory input 6 is not directly addressable In file included from x86_emulate.c:157: x86_emulate/x86_emulate.c:7328: error: memory input 7 is not directly addressable In file included from x86_emulate.c:157: x86_emulate/x86_emulate.c:7362: error: memory input 6 is not directly addressable In file included from x86_emulate.c:157: x86_emulate/x86_emulate.c:7482: error: memory input 3 is not directly addressable In file included from x86_emulate.c:157: x86_emulate/x86_emulate.c:7603: error: memory input 9 is not directly addressable In file included from x86_emulate.c:157: x86_emulate/x86_emulate.c:7607: error: memory input 9 is not directly addressable In file included from x86_emulate.c:157: x86_emulate/x86_emulate.c:7676: error: memory input 6 is not directly addressable In file included from x86_emulate.c:157: x86_emulate/x86_emulate.c:7715: error: memory input 6 is not directly addressable In file included from x86_emulate.c:157: x86_emulate/x86_emulate.c:7795: error: memory input 4 is not directly addressable x86_emulate/x86_emulate.c:7798: error: memory input 3 is not directly addressable FC-64 <build@build-mk2:~/xtt-x86_64/bootstrap/xen.git/tools/fuzz/x86_instruction_emulator> -boris
>>> On 26.04.17 at 16:01, <boris.ostrovsky@oracle.com> wrote: > On 04/25/2017 05:04 AM, Jan Beulich wrote: >> Stub invocations need to have the space the stub occupies as an input, >> to prevent the compiler from re-ordering (or omitting) writes to it. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> >> --- a/xen/arch/x86/x86_emulate/x86_emulate.c >> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c >> @@ -837,7 +837,8 @@ do{ asm volatile ( >> ".popsection\n\t" \ >> _ASM_EXTABLE(.Lret%=, .Lfix%=) \ >> : [exn] "+g" (res_), constraints, \ >> - [stub] "rm" (stub.func) ); \ >> + [stub] "rm" (stub.func), \ >> + "m" (*(uint8_t(*)[MAX_INST_LEN + 1])stub.ptr) ); \ >> if ( unlikely(~res_.raw) ) \ >> { \ >> gprintk(XENLOG_WARNING, \ >> @@ -853,7 +854,8 @@ do{ asm volatile ( >> #else >> # define invoke_stub(pre, post, constraints...) \ >> asm volatile ( pre "\n\tcall *%[stub]\n\t" post \ >> - : constraints, [stub] "rm" (stub.func) ) >> + : constraints, [stub] "rm" (stub.func), \ >> + "m" (*(uint8_t(*)[MAX_INST_LEN + 1])stub.buf) ) >> #endif >> >> #define emulate_stub(dst, src...) do { \ >> > > > This breaks on old compilers: > > FC-64 > <build@build-mk2:~/xtt-x86_64/bootstrap/xen.git/tools/fuzz/x86_instruction_em > ulator> > gcc --version > gcc (GCC) 4.4.4 20100503 (Red Hat 4.4.4-2) I did try with 4.3.x, fwiw (but I'm afraid I've lost that machine just now, and will hardly set it up again using an old distro). Also I can't immediately see what the compiler dislikes and hence how a fix may look like (short of adding memory clobbers instead). Jan > Copyright (C) 2010 Free Software Foundation, Inc. > This is free software; see the source for copying conditions. There is NO > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > > FC-64 > <build@build-mk2:~/xtt-x86_64/bootstrap/xen.git/tools/fuzz/x86_instruction_em > ulator> > pwd > /home/build/xtt-x86_64/bootstrap/xen.git/tools/fuzz/x86_instruction_emulator > FC-64 > <build@build-mk2:~/xtt-x86_64/bootstrap/xen.git/tools/fuzz/x86_instruction_em > ulator> > gcc -m64 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall > -Wstrict-prototypes -Wdeclaration-after-statement > -Wno-unused-but-set-variable -g3 -O0 -fno-omit-frame-pointer > -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MF > .x86_emulate.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE > -I/home/build/xtt-x86_64/bootstrap/xen.git/tools/fuzz/x86_instruction_emulat > or/../../../tools/include > -D__XEN_TOOLS__ -I. -c -o x86_emulate.o x86_emulate.c > In file included from x86_emulate.c:157: > x86_emulate/x86_emulate.c: In function ‘x86_emulate’: > x86_emulate/x86_emulate.c:4085: error: memory input 3 is not directly > addressable > In file included from x86_emulate.c:157: > x86_emulate/x86_emulate.c:4161: error: memory input 3 is not directly > addressable > In file included from x86_emulate.c:157: > x86_emulate/x86_emulate.c:4226: error: memory input 5 is not directly > addressable > In file included from x86_emulate.c:157: > x86_emulate/x86_emulate.c:4229: error: memory input 3 is not directly > addressable > In file included from x86_emulate.c:157: > x86_emulate/x86_emulate.c:4279: error: memory input 5 is not directly > addressable > In file included from x86_emulate.c:157: > x86_emulate/x86_emulate.c:4288: error: memory input 3 is not directly > addressable > In file included from x86_emulate.c:157: > x86_emulate/x86_emulate.c:4353: error: memory input 3 is not directly > addressable > In file included from x86_emulate.c:157: > x86_emulate/x86_emulate.c:4402: error: memory input 3 is not directly > addressable > In file included from x86_emulate.c:157: > x86_emulate/x86_emulate.c:4465: error: memory input 3 is not directly > addressable > In file included from x86_emulate.c:157: > x86_emulate/x86_emulate.c:4516: error: memory input 5 is not directly > addressable > In file included from x86_emulate.c:157: > x86_emulate/x86_emulate.c:4522: error: memory input 3 is not directly > addressable > In file included from x86_emulate.c:157: > x86_emulate/x86_emulate.c:5632: error: memory input 5 is not directly > addressable > In file included from x86_emulate.c:157: > x86_emulate/x86_emulate.c:5679: error: memory input 8 is not directly > addressable > In file included from x86_emulate.c:157: > x86_emulate/x86_emulate.c:5863: error: memory input 3 is not directly > addressable > x86_emulate/x86_emulate.c:6069: error: memory input 4 is not directly > addressable > x86_emulate/x86_emulate.c:6213: error: memory input 3 is not directly > addressable > In file included from x86_emulate.c:157: > x86_emulate/x86_emulate.c:7029: error: memory input 3 is not directly > addressable > In file included from x86_emulate.c:157: > x86_emulate/x86_emulate.c:7178: error: memory input 6 is not directly > addressable > In file included from x86_emulate.c:157: > x86_emulate/x86_emulate.c:7328: error: memory input 7 is not directly > addressable > In file included from x86_emulate.c:157: > x86_emulate/x86_emulate.c:7362: error: memory input 6 is not directly > addressable > In file included from x86_emulate.c:157: > x86_emulate/x86_emulate.c:7482: error: memory input 3 is not directly > addressable > In file included from x86_emulate.c:157: > x86_emulate/x86_emulate.c:7603: error: memory input 9 is not directly > addressable > In file included from x86_emulate.c:157: > x86_emulate/x86_emulate.c:7607: error: memory input 9 is not directly > addressable > In file included from x86_emulate.c:157: > x86_emulate/x86_emulate.c:7676: error: memory input 6 is not directly > addressable > In file included from x86_emulate.c:157: > x86_emulate/x86_emulate.c:7715: error: memory input 6 is not directly > addressable > In file included from x86_emulate.c:157: > x86_emulate/x86_emulate.c:7795: error: memory input 4 is not directly > addressable > x86_emulate/x86_emulate.c:7798: error: memory input 3 is not directly > addressable > FC-64 > <build@build-mk2:~/xtt-x86_64/bootstrap/xen.git/tools/fuzz/x86_instruction_em > ulator> > > > > -boris
>> This breaks on old compilers: >> >> FC-64 >> <build@build-mk2:~/xtt-x86_64/bootstrap/xen.git/tools/fuzz/x86_instruction_em >> ulator> >> gcc --version >> gcc (GCC) 4.4.4 20100503 (Red Hat 4.4.4-2) > I did try with 4.3.x, fwiw (but I'm afraid I've lost that machine just > now, and will hardly set it up again using an old distro). Also I can't > immediately see what the compiler dislikes and hence how a fix may > look like (short of adding memory clobbers instead). This is Fedora 13, if that helps. I can probably arrange your access to that environment (copying Konrad who owns it). -boris
>>> On 26.04.17 at 16:01, <boris.ostrovsky@oracle.com> wrote: > On 04/25/2017 05:04 AM, Jan Beulich wrote: >> Stub invocations need to have the space the stub occupies as an input, >> to prevent the compiler from re-ordering (or omitting) writes to it. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> >> --- a/xen/arch/x86/x86_emulate/x86_emulate.c >> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c >> @@ -837,7 +837,8 @@ do{ asm volatile ( >> ".popsection\n\t" \ >> _ASM_EXTABLE(.Lret%=, .Lfix%=) \ >> : [exn] "+g" (res_), constraints, \ >> - [stub] "rm" (stub.func) ); \ >> + [stub] "rm" (stub.func), \ >> + "m" (*(uint8_t(*)[MAX_INST_LEN + 1])stub.ptr) ); \ >> if ( unlikely(~res_.raw) ) \ >> { \ >> gprintk(XENLOG_WARNING, \ >> @@ -853,7 +854,8 @@ do{ asm volatile ( >> #else >> # define invoke_stub(pre, post, constraints...) \ >> asm volatile ( pre "\n\tcall *%[stub]\n\t" post \ >> - : constraints, [stub] "rm" (stub.func) ) >> + : constraints, [stub] "rm" (stub.func), \ >> + "m" (*(uint8_t(*)[MAX_INST_LEN + 1])stub.buf) ) >> #endif >> >> #define emulate_stub(dst, src...) do { \ >> > > > This breaks on old compilers: > > FC-64 > <build@build-mk2:~/xtt-x86_64/bootstrap/xen.git/tools/fuzz/x86_instruction_em > ulator> > gcc --version > gcc (GCC) 4.4.4 20100503 (Red Hat 4.4.4-2) Btw., I've just realized that I did use an old gcc only on the hypervisor build. Do you see the same issue there, or is this tools side specific? Jan
>>> On 27.04.17 at 10:07, <JBeulich@suse.com> wrote: >>>> On 26.04.17 at 16:01, <boris.ostrovsky@oracle.com> wrote: >> On 04/25/2017 05:04 AM, Jan Beulich wrote: >>> Stub invocations need to have the space the stub occupies as an input, >>> to prevent the compiler from re-ordering (or omitting) writes to it. >>> >>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >>> >>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c >>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c >>> @@ -837,7 +837,8 @@ do{ asm volatile ( >>> ".popsection\n\t" \ >>> _ASM_EXTABLE(.Lret%=, .Lfix%=) \ >>> : [exn] "+g" (res_), constraints, \ >>> - [stub] "rm" (stub.func) ); \ >>> + [stub] "rm" (stub.func), \ >>> + "m" (*(uint8_t(*)[MAX_INST_LEN + 1])stub.ptr) ); \ >>> if ( unlikely(~res_.raw) ) \ >>> { \ >>> gprintk(XENLOG_WARNING, \ >>> @@ -853,7 +854,8 @@ do{ asm volatile ( >>> #else >>> # define invoke_stub(pre, post, constraints...) \ >>> asm volatile ( pre "\n\tcall *%[stub]\n\t" post \ >>> - : constraints, [stub] "rm" (stub.func) ) >>> + : constraints, [stub] "rm" (stub.func), \ >>> + "m" (*(uint8_t(*)[MAX_INST_LEN + 1])stub.buf) ) >>> #endif >>> >>> #define emulate_stub(dst, src...) do { \ >>> >> >> >> This breaks on old compilers: >> >> FC-64 >> <build@build-mk2:~/xtt-x86_64/bootstrap/xen.git/tools/fuzz/x86_instruction_em >> ulator> >> gcc --version >> gcc (GCC) 4.4.4 20100503 (Red Hat 4.4.4-2) > > Btw., I've just realized that I did use an old gcc only on the > hypervisor build. Do you see the same issue there, or is this tools > side specific? And now that I've extracted it into a smaller example and thus was able to try, I can see the issue with 4.3.x. The problem is that with struct stub { unsigned long addr; void *ptr; unsigned char buf[16]; }; void test(const struct stub*ptr) { asm("" :: "m" (*(unsigned char(*)[16])ptr->addr)); asm("" :: "m" (*(unsigned char(*)[16])ptr->ptr)); asm("" :: "m" (*(unsigned char(*)[16])ptr->buf)); asm("" :: "m" (*(unsigned char(*)[16])&ptr->buf)); asm("" :: "m" (*(unsigned char(*)[16])&ptr->buf[0])); } none of the last three work, so we'll have to resort to using the first. I'll have to verify that this is good enough for the case where I did actually observe things to break without the extra constraint (with a not yet submitted patch). Jan
--- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -837,7 +837,8 @@ do{ asm volatile ( ".popsection\n\t" \ _ASM_EXTABLE(.Lret%=, .Lfix%=) \ : [exn] "+g" (res_), constraints, \ - [stub] "rm" (stub.func) ); \ + [stub] "rm" (stub.func), \ + "m" (*(uint8_t(*)[MAX_INST_LEN + 1])stub.ptr) ); \ if ( unlikely(~res_.raw) ) \ { \ gprintk(XENLOG_WARNING, \ @@ -853,7 +854,8 @@ do{ asm volatile ( #else # define invoke_stub(pre, post, constraints...) \ asm volatile ( pre "\n\tcall *%[stub]\n\t" post \ - : constraints, [stub] "rm" (stub.func) ) + : constraints, [stub] "rm" (stub.func), \ + "m" (*(uint8_t(*)[MAX_INST_LEN + 1])stub.buf) ) #endif #define emulate_stub(dst, src...) do { \