| Message ID | 20190916135806.1269-30-david@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | s390x/tcg: mem_helper: Fault-safe handling | expand |
On 9/16/19 9:58 AM, David Hildenbrand wrote: > Let's add the simple test based on the example from the PoP. > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > tests/tcg/s390x/Makefile.target | 1 + > tests/tcg/s390x/mvo.c | 25 +++++++++++++++++++++++++ > 2 files changed, 26 insertions(+) > create mode 100644 tests/tcg/s390x/mvo.c Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
David Hildenbrand <david@redhat.com> writes: > Let's add the simple test based on the example from the PoP. > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > tests/tcg/s390x/Makefile.target | 1 + > tests/tcg/s390x/mvo.c | 25 +++++++++++++++++++++++++ > 2 files changed, 26 insertions(+) > create mode 100644 tests/tcg/s390x/mvo.c > > diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target > index 151dc075aa..6a3bfa8b29 100644 > --- a/tests/tcg/s390x/Makefile.target > +++ b/tests/tcg/s390x/Makefile.target > @@ -6,3 +6,4 @@ TESTS+=ipm > TESTS+=exrl-trt > TESTS+=exrl-trtr > TESTS+=pack > +TESTS+=mvo > diff --git a/tests/tcg/s390x/mvo.c b/tests/tcg/s390x/mvo.c > new file mode 100644 > index 0000000000..5546fe2a97 > --- /dev/null > +++ b/tests/tcg/s390x/mvo.c > @@ -0,0 +1,25 @@ > +#include <stdint.h> > +#include <stdio.h> > + > +int main(void) > +{ > + uint8_t dest[6] = {0xff, 0x77, 0x88, 0x99, 0x0c, 0xff}; > + uint8_t src[5] = {0xee, 0x12, 0x34, 0x56, 0xee}; > + uint8_t expected[6] = {0xff, 0x01, 0x23, 0x45, 0x6c, 0xff}; > + int i; > + > + asm volatile ( > + " mvo 0(4,%[dest]),0(3,%[src])\n" > + : > + : [dest] "d" (dest + 1), > + [src] "d" (src + 1) > + : "memory"); > + > + for (i = 0; i < sizeof(expected); i++) { > + if (dest[i] != expected[i]) { > + fprintf(stderr, "bad data\n"); > + return 1; > + } > + } > + return 0; > +} Reviewed-by: Alex Bennée <alex.bennee@linaro.org> but... can this test be expanded to check the page cross cases that caused you so much trouble to track down? -- Alex Bennée
On 18.09.19 11:47, Alex Bennée wrote: > > David Hildenbrand <david@redhat.com> writes: > >> Let's add the simple test based on the example from the PoP. >> >> Signed-off-by: David Hildenbrand <david@redhat.com> >> --- >> tests/tcg/s390x/Makefile.target | 1 + >> tests/tcg/s390x/mvo.c | 25 +++++++++++++++++++++++++ >> 2 files changed, 26 insertions(+) >> create mode 100644 tests/tcg/s390x/mvo.c >> >> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target >> index 151dc075aa..6a3bfa8b29 100644 >> --- a/tests/tcg/s390x/Makefile.target >> +++ b/tests/tcg/s390x/Makefile.target >> @@ -6,3 +6,4 @@ TESTS+=ipm >> TESTS+=exrl-trt >> TESTS+=exrl-trtr >> TESTS+=pack >> +TESTS+=mvo >> diff --git a/tests/tcg/s390x/mvo.c b/tests/tcg/s390x/mvo.c >> new file mode 100644 >> index 0000000000..5546fe2a97 >> --- /dev/null >> +++ b/tests/tcg/s390x/mvo.c >> @@ -0,0 +1,25 @@ >> +#include <stdint.h> >> +#include <stdio.h> >> + >> +int main(void) >> +{ >> + uint8_t dest[6] = {0xff, 0x77, 0x88, 0x99, 0x0c, 0xff}; >> + uint8_t src[5] = {0xee, 0x12, 0x34, 0x56, 0xee}; >> + uint8_t expected[6] = {0xff, 0x01, 0x23, 0x45, 0x6c, 0xff}; >> + int i; >> + >> + asm volatile ( >> + " mvo 0(4,%[dest]),0(3,%[src])\n" >> + : >> + : [dest] "d" (dest + 1), >> + [src] "d" (src + 1) >> + : "memory"); >> + >> + for (i = 0; i < sizeof(expected); i++) { >> + if (dest[i] != expected[i]) { >> + fprintf(stderr, "bad data\n"); >> + return 1; >> + } >> + } >> + return 0; >> +} > > Reviewed-by: Alex Bennée <alex.bennee@linaro.org> > > but... > > can this test be expanded to check the page cross cases that caused you > so much trouble to track down? I might add a MVC test that tries to reproduce this. But with speculative page faults and things like that it might not be very easy to reproduce. However, I can give it a try. Thanks! > > -- > Alex Bennée >
David Hildenbrand <david@redhat.com> writes: > On 18.09.19 11:47, Alex Bennée wrote: >> >> David Hildenbrand <david@redhat.com> writes: >> >>> Let's add the simple test based on the example from the PoP. >>> >>> Signed-off-by: David Hildenbrand <david@redhat.com> >>> --- >>> tests/tcg/s390x/Makefile.target | 1 + >>> tests/tcg/s390x/mvo.c | 25 +++++++++++++++++++++++++ >>> 2 files changed, 26 insertions(+) >>> create mode 100644 tests/tcg/s390x/mvo.c >>> >>> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target >>> index 151dc075aa..6a3bfa8b29 100644 >>> --- a/tests/tcg/s390x/Makefile.target >>> +++ b/tests/tcg/s390x/Makefile.target >>> @@ -6,3 +6,4 @@ TESTS+=ipm >>> TESTS+=exrl-trt >>> TESTS+=exrl-trtr >>> TESTS+=pack >>> +TESTS+=mvo >>> diff --git a/tests/tcg/s390x/mvo.c b/tests/tcg/s390x/mvo.c >>> new file mode 100644 >>> index 0000000000..5546fe2a97 >>> --- /dev/null >>> +++ b/tests/tcg/s390x/mvo.c >>> @@ -0,0 +1,25 @@ >>> +#include <stdint.h> >>> +#include <stdio.h> >>> + >>> +int main(void) >>> +{ >>> + uint8_t dest[6] = {0xff, 0x77, 0x88, 0x99, 0x0c, 0xff}; >>> + uint8_t src[5] = {0xee, 0x12, 0x34, 0x56, 0xee}; >>> + uint8_t expected[6] = {0xff, 0x01, 0x23, 0x45, 0x6c, 0xff}; >>> + int i; >>> + >>> + asm volatile ( >>> + " mvo 0(4,%[dest]),0(3,%[src])\n" >>> + : >>> + : [dest] "d" (dest + 1), >>> + [src] "d" (src + 1) >>> + : "memory"); >>> + >>> + for (i = 0; i < sizeof(expected); i++) { >>> + if (dest[i] != expected[i]) { >>> + fprintf(stderr, "bad data\n"); >>> + return 1; >>> + } >>> + } >>> + return 0; >>> +} >> >> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> >> >> but... >> >> can this test be expanded to check the page cross cases that caused you >> so much trouble to track down? > > I might add a MVC test that tries to reproduce this. But with > speculative page faults and things like that it might not be very easy > to reproduce. However, I can give it a try. I may not be fully understanding the correct behaviour but wouldn't the test be: * map two page's worth of address space * mprot(!write) the top page * attempt mvc based copy to mmap region (say p0+(PAGE_SIZE>>1) to p1+(PAGE_SIZE>>1)) * catch the fault * check the bottom page wasn't written to or does the test need to be lower level and run in kernel mode in softmmu and catch the appropriate low level exceptions? > > Thanks! > >> >> -- >> Alex Bennée >> -- Alex Bennée
On 18.09.19 13:24, Alex Bennée wrote: > > David Hildenbrand <david@redhat.com> writes: > >> On 18.09.19 11:47, Alex Bennée wrote: >>> >>> David Hildenbrand <david@redhat.com> writes: >>> >>>> Let's add the simple test based on the example from the PoP. >>>> >>>> Signed-off-by: David Hildenbrand <david@redhat.com> >>>> --- >>>> tests/tcg/s390x/Makefile.target | 1 + >>>> tests/tcg/s390x/mvo.c | 25 +++++++++++++++++++++++++ >>>> 2 files changed, 26 insertions(+) >>>> create mode 100644 tests/tcg/s390x/mvo.c >>>> >>>> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target >>>> index 151dc075aa..6a3bfa8b29 100644 >>>> --- a/tests/tcg/s390x/Makefile.target >>>> +++ b/tests/tcg/s390x/Makefile.target >>>> @@ -6,3 +6,4 @@ TESTS+=ipm >>>> TESTS+=exrl-trt >>>> TESTS+=exrl-trtr >>>> TESTS+=pack >>>> +TESTS+=mvo >>>> diff --git a/tests/tcg/s390x/mvo.c b/tests/tcg/s390x/mvo.c >>>> new file mode 100644 >>>> index 0000000000..5546fe2a97 >>>> --- /dev/null >>>> +++ b/tests/tcg/s390x/mvo.c >>>> @@ -0,0 +1,25 @@ >>>> +#include <stdint.h> >>>> +#include <stdio.h> >>>> + >>>> +int main(void) >>>> +{ >>>> + uint8_t dest[6] = {0xff, 0x77, 0x88, 0x99, 0x0c, 0xff}; >>>> + uint8_t src[5] = {0xee, 0x12, 0x34, 0x56, 0xee}; >>>> + uint8_t expected[6] = {0xff, 0x01, 0x23, 0x45, 0x6c, 0xff}; >>>> + int i; >>>> + >>>> + asm volatile ( >>>> + " mvo 0(4,%[dest]),0(3,%[src])\n" >>>> + : >>>> + : [dest] "d" (dest + 1), >>>> + [src] "d" (src + 1) >>>> + : "memory"); >>>> + >>>> + for (i = 0; i < sizeof(expected); i++) { >>>> + if (dest[i] != expected[i]) { >>>> + fprintf(stderr, "bad data\n"); >>>> + return 1; >>>> + } >>>> + } >>>> + return 0; >>>> +} >>> >>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> >>> >>> but... >>> >>> can this test be expanded to check the page cross cases that caused you >>> so much trouble to track down? >> >> I might add a MVC test that tries to reproduce this. But with >> speculative page faults and things like that it might not be very easy >> to reproduce. However, I can give it a try. > > I may not be fully understanding the correct behaviour but wouldn't the > test be: > > * map two page's worth of address space > * mprot(!write) the top page > * attempt mvc based copy to mmap region (say p0+(PAGE_SIZE>>1) to p1+(PAGE_SIZE>>1)) > * catch the fault > * check the bottom page wasn't written to No, you are absolutely right, with memmap/mprot hackery + signal handler this can be tested just fine. I'll craft something for the MVC instruction! Thanks!
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target index 151dc075aa..6a3bfa8b29 100644 --- a/tests/tcg/s390x/Makefile.target +++ b/tests/tcg/s390x/Makefile.target @@ -6,3 +6,4 @@ TESTS+=ipm TESTS+=exrl-trt TESTS+=exrl-trtr TESTS+=pack +TESTS+=mvo diff --git a/tests/tcg/s390x/mvo.c b/tests/tcg/s390x/mvo.c new file mode 100644 index 0000000000..5546fe2a97 --- /dev/null +++ b/tests/tcg/s390x/mvo.c @@ -0,0 +1,25 @@ +#include <stdint.h> +#include <stdio.h> + +int main(void) +{ + uint8_t dest[6] = {0xff, 0x77, 0x88, 0x99, 0x0c, 0xff}; + uint8_t src[5] = {0xee, 0x12, 0x34, 0x56, 0xee}; + uint8_t expected[6] = {0xff, 0x01, 0x23, 0x45, 0x6c, 0xff}; + int i; + + asm volatile ( + " mvo 0(4,%[dest]),0(3,%[src])\n" + : + : [dest] "d" (dest + 1), + [src] "d" (src + 1) + : "memory"); + + for (i = 0; i < sizeof(expected); i++) { + if (dest[i] != expected[i]) { + fprintf(stderr, "bad data\n"); + return 1; + } + } + return 0; +}
Let's add the simple test based on the example from the PoP. Signed-off-by: David Hildenbrand <david@redhat.com> --- tests/tcg/s390x/Makefile.target | 1 + tests/tcg/s390x/mvo.c | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 tests/tcg/s390x/mvo.c