* [Qemu-devel] [PATCH RFC 0/2] tests/tcg: Vector instruction tests for target/s390x
@ 2019-02-27 11:14 David Hildenbrand
2019-02-27 11:14 ` [Qemu-devel] [PATCH RFC 1/2] tests/tcg: Allow targets to set the optimization level David Hildenbrand
2019-02-27 11:14 ` [Qemu-devel] [PATCH RFC 2/2] tests/tcg: target/s390: Add test for VECTOR LOAD GR FROM VR ELEMENT David Hildenbrand
0 siblings, 2 replies; 18+ messages in thread
From: David Hildenbrand @ 2019-02-27 11:14 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-s390x, Thomas Huth, Cornelia Huck, Richard Henderson,
Alex Bennée, Philippe Mathieu-Daudé,
David Hildenbrand
As I currently work on vector instruction support for s390x/tcg, for now
I wrote my tests for kvm-unit-tests, but tests/tcg seems to be a better fit.
The only tricky part is testing interrupt handling, but that also seems to
be possible using some signal hackery.
This is only one test to discuss if the approach make sense. These patches
only work when applied on top of:
https://github.com/davidhildenbrand/qemu/tree/vx
David Hildenbrand (2):
tests/tcg: Allow targets to set the optimization level
tests/tcg: target/s390: Add test for VECTOR LOAD GR FROM VR ELEMENT
tests/tcg/Makefile | 4 +--
tests/tcg/s390x/Makefile.target | 3 ++-
tests/tcg/s390x/helper.h | 28 +++++++++++++++++++++
tests/tcg/s390x/signal_helper.inc.c | 39 +++++++++++++++++++++++++++++
tests/tcg/s390x/vlgv.c | 37 +++++++++++++++++++++++++++
5 files changed, 108 insertions(+), 3 deletions(-)
create mode 100644 tests/tcg/s390x/helper.h
create mode 100644 tests/tcg/s390x/signal_helper.inc.c
create mode 100644 tests/tcg/s390x/vlgv.c
--
2.17.2
^ permalink raw reply [flat|nested] 18+ messages in thread* [Qemu-devel] [PATCH RFC 1/2] tests/tcg: Allow targets to set the optimization level 2019-02-27 11:14 [Qemu-devel] [PATCH RFC 0/2] tests/tcg: Vector instruction tests for target/s390x David Hildenbrand @ 2019-02-27 11:14 ` David Hildenbrand 2019-02-27 11:46 ` Alex Bennée 2019-02-28 0:26 ` Richard Henderson 2019-02-27 11:14 ` [Qemu-devel] [PATCH RFC 2/2] tests/tcg: target/s390: Add test for VECTOR LOAD GR FROM VR ELEMENT David Hildenbrand 1 sibling, 2 replies; 18+ messages in thread From: David Hildenbrand @ 2019-02-27 11:14 UTC (permalink / raw) To: qemu-devel Cc: qemu-s390x, Thomas Huth, Cornelia Huck, Richard Henderson, Alex Bennée, Philippe Mathieu-Daudé, David Hildenbrand We sometimes want basic optimizations, e.g. for constant propagation into inlined functions. Especially for inline asm with immediates; static inline void some_instr(uint8_t imm) { asm volatile("...", :: "i" (imm)); } static void test() { some_instr(1); } Which is impossible with -O0. So make -O0 the default but let targets override it. Signed-off-by: David Hildenbrand <david@redhat.com> --- tests/tcg/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/tcg/Makefile b/tests/tcg/Makefile index bf06415390..bd0bace0d5 100644 --- a/tests/tcg/Makefile +++ b/tests/tcg/Makefile @@ -47,7 +47,7 @@ skip-test = @printf " SKIPPED %s on $(TARGET_NAME) because %s\n" $1 $2 TESTS= # Start with a blank slate, the build targets get to add stuff first -CFLAGS= +CFLAGS=-O0 QEMU_CFLAGS= LDFLAGS= @@ -70,7 +70,7 @@ ifneq ($(TARGET_BASE_ARCH),$(TARGET_NAME)) endif # Add the common build options -CFLAGS+=-Wall -O0 -g -fno-strict-aliasing +CFLAGS+=-Wall -g -fno-strict-aliasing ifeq ($(BUILD_STATIC),y) LDFLAGS+=-static endif -- 2.17.2 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 1/2] tests/tcg: Allow targets to set the optimization level 2019-02-27 11:14 ` [Qemu-devel] [PATCH RFC 1/2] tests/tcg: Allow targets to set the optimization level David Hildenbrand @ 2019-02-27 11:46 ` Alex Bennée 2019-02-27 11:58 ` David Hildenbrand 2019-02-27 12:42 ` David Hildenbrand 2019-02-28 0:26 ` Richard Henderson 1 sibling, 2 replies; 18+ messages in thread From: Alex Bennée @ 2019-02-27 11:46 UTC (permalink / raw) To: David Hildenbrand Cc: qemu-devel, qemu-s390x, Thomas Huth, Cornelia Huck, Richard Henderson, Philippe Mathieu-Daudé David Hildenbrand <david@redhat.com> writes: > We sometimes want basic optimizations, e.g. for constant propagation > into inlined functions. > > Especially for inline asm with immediates; I agree you need this, but... <snip> > # Start with a blank slate, the build targets get to add stuff first > -CFLAGS= > +CFLAGS=-O0 > QEMU_CFLAGS= > LDFLAGS= > > @@ -70,7 +70,7 @@ ifneq ($(TARGET_BASE_ARCH),$(TARGET_NAME)) > endif > > # Add the common build options > -CFLAGS+=-Wall -O0 -g -fno-strict-aliasing > +CFLAGS+=-Wall -g -fno-strict-aliasing You don't need to do this - we already have EXTRA_CFLAGS which will always be at the end of the compile invocation: # enable vectors and optimisation to vectorise for this test vglv: CFLAGS+=-march=z13 -m64 vglv: EXTRA_CFLAGS+=-O2 > ifeq ($(BUILD_STATIC),y) > LDFLAGS+=-static > endif -- Alex Bennée ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 1/2] tests/tcg: Allow targets to set the optimization level 2019-02-27 11:46 ` Alex Bennée @ 2019-02-27 11:58 ` David Hildenbrand 2019-02-27 12:42 ` David Hildenbrand 1 sibling, 0 replies; 18+ messages in thread From: David Hildenbrand @ 2019-02-27 11:58 UTC (permalink / raw) To: Alex Bennée Cc: qemu-devel, qemu-s390x, Thomas Huth, Cornelia Huck, Richard Henderson, Philippe Mathieu-Daudé On 27.02.19 12:46, Alex Bennée wrote: > > David Hildenbrand <david@redhat.com> writes: > >> We sometimes want basic optimizations, e.g. for constant propagation >> into inlined functions. >> >> Especially for inline asm with immediates; > > I agree you need this, but... > > <snip> >> # Start with a blank slate, the build targets get to add stuff first >> -CFLAGS= >> +CFLAGS=-O0 >> QEMU_CFLAGS= >> LDFLAGS= >> >> @@ -70,7 +70,7 @@ ifneq ($(TARGET_BASE_ARCH),$(TARGET_NAME)) >> endif >> >> # Add the common build options >> -CFLAGS+=-Wall -O0 -g -fno-strict-aliasing >> +CFLAGS+=-Wall -g -fno-strict-aliasing > > > You don't need to do this - we already have EXTRA_CFLAGS which will > always be at the end of the compile invocation: > > # enable vectors and optimisation to vectorise for this test > vglv: CFLAGS+=-march=z13 -m64 > vglv: EXTRA_CFLAGS+=-O2 > Cool, missed that - thanks! -- Thanks, David / dhildenb ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 1/2] tests/tcg: Allow targets to set the optimization level 2019-02-27 11:46 ` Alex Bennée 2019-02-27 11:58 ` David Hildenbrand @ 2019-02-27 12:42 ` David Hildenbrand 2019-02-27 13:44 ` David Hildenbrand 1 sibling, 1 reply; 18+ messages in thread From: David Hildenbrand @ 2019-02-27 12:42 UTC (permalink / raw) To: Alex Bennée Cc: qemu-devel, qemu-s390x, Thomas Huth, Cornelia Huck, Richard Henderson, Philippe Mathieu-Daudé On 27.02.19 12:46, Alex Bennée wrote: > > David Hildenbrand <david@redhat.com> writes: > >> We sometimes want basic optimizations, e.g. for constant propagation >> into inlined functions. >> >> Especially for inline asm with immediates; > > I agree you need this, but... > > <snip> >> # Start with a blank slate, the build targets get to add stuff first >> -CFLAGS= >> +CFLAGS=-O0 >> QEMU_CFLAGS= >> LDFLAGS= >> >> @@ -70,7 +70,7 @@ ifneq ($(TARGET_BASE_ARCH),$(TARGET_NAME)) >> endif >> >> # Add the common build options >> -CFLAGS+=-Wall -O0 -g -fno-strict-aliasing >> +CFLAGS+=-Wall -g -fno-strict-aliasing > > > You don't need to do this - we already have EXTRA_CFLAGS which will > always be at the end of the compile invocation: > > # enable vectors and optimisation to vectorise for this test > vglv: CFLAGS+=-march=z13 -m64 > vglv: EXTRA_CFLAGS+=-O2 For whatever reason I can't get this to work. EXTRA_CFLAGS+=-O2 is always silently dropped: make SHELL='sh -x' run-tcg-tests-s390x-linux-user + /home/dhildenb/git/qemu/tests/docker/docker.py cc --user 100813 --cc s390x-linux-gnu-gcc -i qemu:debian-s390x-cross -s /home/dhildenb/git/qemu -- -march=zEC12 -m64 -Wall -O0 -g -fno-strict-aliasing -march=z13 /home/dhildenb/git/qemu/tests/tcg/s390x/vlgv.c -o vlgv -static /home/dhildenb/git/qemu/tests/tcg/s390x/vlgv.c: In function 'vlgv': /home/dhildenb/git/qemu/tests/tcg/s390x/vlgv.c:8:5: warning: asm operand 3 probably doesn't match constraints asm volatile("vlgv %[r1], %[v3], 0(%[a2]), %[m4]\n" ^~~ /home/dhildenb/git/qemu/tests/tcg/s390x/vlgv.c:8:5: error: impossible constraint in 'asm' -- Thanks, David / dhildenb ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 1/2] tests/tcg: Allow targets to set the optimization level 2019-02-27 12:42 ` David Hildenbrand @ 2019-02-27 13:44 ` David Hildenbrand 0 siblings, 0 replies; 18+ messages in thread From: David Hildenbrand @ 2019-02-27 13:44 UTC (permalink / raw) To: Alex Bennée Cc: qemu-devel, qemu-s390x, Thomas Huth, Cornelia Huck, Richard Henderson, Philippe Mathieu-Daudé On 27.02.19 13:42, David Hildenbrand wrote: > On 27.02.19 12:46, Alex Bennée wrote: >> >> David Hildenbrand <david@redhat.com> writes: >> >>> We sometimes want basic optimizations, e.g. for constant propagation >>> into inlined functions. >>> >>> Especially for inline asm with immediates; >> >> I agree you need this, but... >> >> <snip> >>> # Start with a blank slate, the build targets get to add stuff first >>> -CFLAGS= >>> +CFLAGS=-O0 >>> QEMU_CFLAGS= >>> LDFLAGS= >>> >>> @@ -70,7 +70,7 @@ ifneq ($(TARGET_BASE_ARCH),$(TARGET_NAME)) >>> endif >>> >>> # Add the common build options >>> -CFLAGS+=-Wall -O0 -g -fno-strict-aliasing >>> +CFLAGS+=-Wall -g -fno-strict-aliasing >> >> >> You don't need to do this - we already have EXTRA_CFLAGS which will >> always be at the end of the compile invocation: >> >> # enable vectors and optimisation to vectorise for this test >> vglv: CFLAGS+=-march=z13 -m64 >> vglv: EXTRA_CFLAGS+=-O2 > > For whatever reason I can't get this to work. EXTRA_CFLAGS+=-O2 is > always silently dropped: > > make SHELL='sh -x' run-tcg-tests-s390x-linux-user > > + /home/dhildenb/git/qemu/tests/docker/docker.py cc --user 100813 --cc > s390x-linux-gnu-gcc -i qemu:debian-s390x-cross -s > /home/dhildenb/git/qemu -- -march=zEC12 -m64 -Wall -O0 -g > -fno-strict-aliasing -march=z13 > /home/dhildenb/git/qemu/tests/tcg/s390x/vlgv.c -o vlgv -static > /home/dhildenb/git/qemu/tests/tcg/s390x/vlgv.c: In function 'vlgv': > /home/dhildenb/git/qemu/tests/tcg/s390x/vlgv.c:8:5: warning: asm operand > 3 probably doesn't match constraints > asm volatile("vlgv %[r1], %[v3], 0(%[a2]), %[m4]\n" > ^~~ > /home/dhildenb/git/qemu/tests/tcg/s390x/vlgv.c:8:5: error: impossible > constraint in 'asm' > vlgv: CFLAGS+=-march=z13 -O2 However works. I am very confused now. I guess this is about evaluation order or something like that. -- Thanks, David / dhildenb ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 1/2] tests/tcg: Allow targets to set the optimization level 2019-02-27 11:14 ` [Qemu-devel] [PATCH RFC 1/2] tests/tcg: Allow targets to set the optimization level David Hildenbrand 2019-02-27 11:46 ` Alex Bennée @ 2019-02-28 0:26 ` Richard Henderson 1 sibling, 0 replies; 18+ messages in thread From: Richard Henderson @ 2019-02-28 0:26 UTC (permalink / raw) To: David Hildenbrand, qemu-devel Cc: Thomas Huth, Cornelia Huck, Philippe Mathieu-Daudé, qemu-s390x, Alex Bennée, Richard Henderson On 2/27/19 3:14 AM, David Hildenbrand wrote: > Especially for inline asm with immediates; > > static inline void some_instr(uint8_t imm) > { > asm volatile("...", :: "i" (imm)); > } > > static void test() > { > some_instr(1); > } FWIW, __attribute__((always_inline)) should work even with -O0. r~ ^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH RFC 2/2] tests/tcg: target/s390: Add test for VECTOR LOAD GR FROM VR ELEMENT 2019-02-27 11:14 [Qemu-devel] [PATCH RFC 0/2] tests/tcg: Vector instruction tests for target/s390x David Hildenbrand 2019-02-27 11:14 ` [Qemu-devel] [PATCH RFC 1/2] tests/tcg: Allow targets to set the optimization level David Hildenbrand @ 2019-02-27 11:14 ` David Hildenbrand 2019-02-27 11:19 ` David Hildenbrand 2019-02-27 19:37 ` David Hildenbrand 1 sibling, 2 replies; 18+ messages in thread From: David Hildenbrand @ 2019-02-27 11:14 UTC (permalink / raw) To: qemu-devel Cc: qemu-s390x, Thomas Huth, Cornelia Huck, Richard Henderson, Alex Bennée, Philippe Mathieu-Daudé, David Hildenbrand We want to make use of vectors, so use -march=z13. To make it compile, use a reasonable optimization level (-O2), which seems to work just fine with all tests. Add some infrastructure for checking if SIGILL will be properly triggered. Signed-off-by: David Hildenbrand <david@redhat.com> --- tests/tcg/s390x/Makefile.target | 3 ++- tests/tcg/s390x/helper.h | 28 +++++++++++++++++++++ tests/tcg/s390x/signal_helper.inc.c | 39 +++++++++++++++++++++++++++++ tests/tcg/s390x/vlgv.c | 37 +++++++++++++++++++++++++++ 4 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 tests/tcg/s390x/helper.h create mode 100644 tests/tcg/s390x/signal_helper.inc.c create mode 100644 tests/tcg/s390x/vlgv.c diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target index 151dc075aa..d1ae755ab9 100644 --- a/tests/tcg/s390x/Makefile.target +++ b/tests/tcg/s390x/Makefile.target @@ -1,8 +1,9 @@ VPATH+=$(SRC_PATH)/tests/tcg/s390x -CFLAGS+=-march=zEC12 -m64 +CFLAGS+=-march=z13 -m64 -O2 TESTS+=hello-s390x TESTS+=csst TESTS+=ipm TESTS+=exrl-trt TESTS+=exrl-trtr TESTS+=pack +TESTS+=vlgv diff --git a/tests/tcg/s390x/helper.h b/tests/tcg/s390x/helper.h new file mode 100644 index 0000000000..845b8bb504 --- /dev/null +++ b/tests/tcg/s390x/helper.h @@ -0,0 +1,28 @@ +#ifndef TEST_TCG_S390x_VECTOR_H +#define TEST_TCG_S390x_VECTOR_H + +#include <stdint.h> + +#define ES_8 0 +#define ES_16 1 +#define ES_32 2 +#define ES_64 3 +#define ES_128 4 + +typedef union S390Vector { + __uint128_t v; + uint64_t q[2]; + uint32_t d[4]; + uint16_t w[8]; + uint8_t h[16]; +} S390Vector; + +static inline void check(const char *s, bool cond) +{ + if (!cond) { + fprintf(stderr, "Check failed: %s\n", s); + exit(-1); + } +} + +#endif /* TEST_TCG_S390x_VECTOR_H */ diff --git a/tests/tcg/s390x/signal_helper.inc.c b/tests/tcg/s390x/signal_helper.inc.c new file mode 100644 index 0000000000..5bd69ca76a --- /dev/null +++ b/tests/tcg/s390x/signal_helper.inc.c @@ -0,0 +1,39 @@ +#include <stdlib.h> +#include <stdio.h> +#include <stdbool.h> +#include <signal.h> +#include <setjmp.h> +#include "helper.h" + +jmp_buf jmp_env; + +static void sig_sigill(int sig) +{ + if (sig != SIGILL) { + check("Wrong signal received", false); + } + longjmp(jmp_env, 1); +} + +#define CHECK_SIGILL(STATEMENT) \ +do { \ + struct sigaction act; \ + \ + act.sa_handler = sig_sigill; \ + act.sa_flags = 0; \ + if (sigaction(SIGILL, &act, NULL)) { \ + check("SIGILL handler not registered", false); \ + } \ + \ + if (setjmp(jmp_env) == 0) { \ + STATEMENT; \ + check("SIGILL not triggered", false); \ + } \ + \ + act.sa_handler = SIG_DFL; \ + sigemptyset(&act.sa_mask); \ + act.sa_flags = 0; \ + if (sigaction(SIGILL, &act, NULL)) { \ + check("SIGILL handler not unregistered", false); \ + } \ +} while (0) diff --git a/tests/tcg/s390x/vlgv.c b/tests/tcg/s390x/vlgv.c new file mode 100644 index 0000000000..3c37ee2035 --- /dev/null +++ b/tests/tcg/s390x/vlgv.c @@ -0,0 +1,37 @@ +#include <stdint.h> +#include <unistd.h> +#include "signal_helper.inc.c" + +static inline void vlgv(uint64_t *r1, S390Vector *v3, const void *a2, + uint8_t m4) +{ + asm volatile("vlgv %[r1], %[v3], 0(%[a2]), %[m4]\n" + : [r1] "+d" (*r1), + [v3] "+v" (v3->v) + : [a2] "d" (a2), + [m4] "i" (m4)); +} + +int main(void) +{ + S390Vector v3 = { + .q[0] = 0x0011223344556677ull, + .q[1] = 0x8899aabbccddeeffull, + }; + uint64_t r1 = 0; + + /* Directly set all ignored bits to */ + vlgv(&r1, &v3, (void *)(7 | ~0xf), ES_8); + check("8 bit", r1 == 0x77); + vlgv(&r1, &v3, (void *)(4 | ~0x7), ES_16); + check("16 bit", r1 == 0x8899); + vlgv(&r1, &v3, (void *)(3 | ~0x3), ES_32); + check("32 bit", r1 == 0xccddeeff); + vlgv(&r1, &v3, (void *)(1 | ~0x1), ES_64); + check("64 bit", r1 == 0x8899aabbccddeeffull); + check("v3 not modified", v3.q[0] == 0x0011223344556677ull && + v3.q[1] == 0x8899aabbccddeeffull); + + CHECK_SIGILL(vlgv(&r1, &v3, NULL, ES_128)); + return 0; +} -- 2.17.2 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 2/2] tests/tcg: target/s390: Add test for VECTOR LOAD GR FROM VR ELEMENT 2019-02-27 11:14 ` [Qemu-devel] [PATCH RFC 2/2] tests/tcg: target/s390: Add test for VECTOR LOAD GR FROM VR ELEMENT David Hildenbrand @ 2019-02-27 11:19 ` David Hildenbrand 2019-02-27 19:37 ` David Hildenbrand 1 sibling, 0 replies; 18+ messages in thread From: David Hildenbrand @ 2019-02-27 11:19 UTC (permalink / raw) To: qemu-devel Cc: qemu-s390x, Thomas Huth, Cornelia Huck, Richard Henderson, Alex Bennée, Philippe Mathieu-Daudé On 27.02.19 12:14, David Hildenbrand wrote: > We want to make use of vectors, so use -march=z13. To make it compile, > use a reasonable optimization level (-O2), which seems to work just fine > with all tests. Just double-checked, at least exrl-trt will need a fixup to work with -O2. > > Add some infrastructure for checking if SIGILL will be properly > triggered. > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > tests/tcg/s390x/Makefile.target | 3 ++- > tests/tcg/s390x/helper.h | 28 +++++++++++++++++++++ > tests/tcg/s390x/signal_helper.inc.c | 39 +++++++++++++++++++++++++++++ > tests/tcg/s390x/vlgv.c | 37 +++++++++++++++++++++++++++ > 4 files changed, 106 insertions(+), 1 deletion(-) > create mode 100644 tests/tcg/s390x/helper.h > create mode 100644 tests/tcg/s390x/signal_helper.inc.c > create mode 100644 tests/tcg/s390x/vlgv.c > > diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target > index 151dc075aa..d1ae755ab9 100644 > --- a/tests/tcg/s390x/Makefile.target > +++ b/tests/tcg/s390x/Makefile.target > @@ -1,8 +1,9 @@ > VPATH+=$(SRC_PATH)/tests/tcg/s390x > -CFLAGS+=-march=zEC12 -m64 > +CFLAGS+=-march=z13 -m64 -O2 > TESTS+=hello-s390x > TESTS+=csst > TESTS+=ipm > TESTS+=exrl-trt > TESTS+=exrl-trtr > TESTS+=pack > +TESTS+=vlgv > diff --git a/tests/tcg/s390x/helper.h b/tests/tcg/s390x/helper.h > new file mode 100644 > index 0000000000..845b8bb504 > --- /dev/null > +++ b/tests/tcg/s390x/helper.h > @@ -0,0 +1,28 @@ > +#ifndef TEST_TCG_S390x_VECTOR_H > +#define TEST_TCG_S390x_VECTOR_H > + > +#include <stdint.h> > + > +#define ES_8 0 > +#define ES_16 1 > +#define ES_32 2 > +#define ES_64 3 > +#define ES_128 4 > + > +typedef union S390Vector { > + __uint128_t v; > + uint64_t q[2]; > + uint32_t d[4]; > + uint16_t w[8]; > + uint8_t h[16]; > +} S390Vector; > + > +static inline void check(const char *s, bool cond) > +{ > + if (!cond) { > + fprintf(stderr, "Check failed: %s\n", s); > + exit(-1); > + } > +} > + > +#endif /* TEST_TCG_S390x_VECTOR_H */ > diff --git a/tests/tcg/s390x/signal_helper.inc.c b/tests/tcg/s390x/signal_helper.inc.c > new file mode 100644 > index 0000000000..5bd69ca76a > --- /dev/null > +++ b/tests/tcg/s390x/signal_helper.inc.c > @@ -0,0 +1,39 @@ > +#include <stdlib.h> > +#include <stdio.h> > +#include <stdbool.h> > +#include <signal.h> > +#include <setjmp.h> > +#include "helper.h" > + > +jmp_buf jmp_env; > + > +static void sig_sigill(int sig) > +{ > + if (sig != SIGILL) { > + check("Wrong signal received", false); > + } > + longjmp(jmp_env, 1); > +} > + > +#define CHECK_SIGILL(STATEMENT) \ > +do { \ > + struct sigaction act; \ > + \ > + act.sa_handler = sig_sigill; \ > + act.sa_flags = 0; \ > + if (sigaction(SIGILL, &act, NULL)) { \ > + check("SIGILL handler not registered", false); \ > + } \ > + \ > + if (setjmp(jmp_env) == 0) { \ > + STATEMENT; \ > + check("SIGILL not triggered", false); \ > + } \ > + \ > + act.sa_handler = SIG_DFL; \ > + sigemptyset(&act.sa_mask); \ > + act.sa_flags = 0; \ > + if (sigaction(SIGILL, &act, NULL)) { \ > + check("SIGILL handler not unregistered", false); \ > + } \ > +} while (0) > diff --git a/tests/tcg/s390x/vlgv.c b/tests/tcg/s390x/vlgv.c > new file mode 100644 > index 0000000000..3c37ee2035 > --- /dev/null > +++ b/tests/tcg/s390x/vlgv.c > @@ -0,0 +1,37 @@ > +#include <stdint.h> > +#include <unistd.h> > +#include "signal_helper.inc.c" > + > +static inline void vlgv(uint64_t *r1, S390Vector *v3, const void *a2, > + uint8_t m4) > +{ > + asm volatile("vlgv %[r1], %[v3], 0(%[a2]), %[m4]\n" > + : [r1] "+d" (*r1), > + [v3] "+v" (v3->v) > + : [a2] "d" (a2), > + [m4] "i" (m4)); > +} > + > +int main(void) > +{ > + S390Vector v3 = { > + .q[0] = 0x0011223344556677ull, > + .q[1] = 0x8899aabbccddeeffull, > + }; > + uint64_t r1 = 0; > + > + /* Directly set all ignored bits to */ > + vlgv(&r1, &v3, (void *)(7 | ~0xf), ES_8); > + check("8 bit", r1 == 0x77); > + vlgv(&r1, &v3, (void *)(4 | ~0x7), ES_16); > + check("16 bit", r1 == 0x8899); > + vlgv(&r1, &v3, (void *)(3 | ~0x3), ES_32); > + check("32 bit", r1 == 0xccddeeff); > + vlgv(&r1, &v3, (void *)(1 | ~0x1), ES_64); > + check("64 bit", r1 == 0x8899aabbccddeeffull); > + check("v3 not modified", v3.q[0] == 0x0011223344556677ull && > + v3.q[1] == 0x8899aabbccddeeffull); > + > + CHECK_SIGILL(vlgv(&r1, &v3, NULL, ES_128)); > + return 0; > +} > -- Thanks, David / dhildenb ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 2/2] tests/tcg: target/s390: Add test for VECTOR LOAD GR FROM VR ELEMENT 2019-02-27 11:14 ` [Qemu-devel] [PATCH RFC 2/2] tests/tcg: target/s390: Add test for VECTOR LOAD GR FROM VR ELEMENT David Hildenbrand 2019-02-27 11:19 ` David Hildenbrand @ 2019-02-27 19:37 ` David Hildenbrand 2019-02-27 20:19 ` Alex Bennée 2019-02-28 0:17 ` Richard Henderson 1 sibling, 2 replies; 18+ messages in thread From: David Hildenbrand @ 2019-02-27 19:37 UTC (permalink / raw) To: qemu-devel Cc: qemu-s390x, Thomas Huth, Cornelia Huck, Richard Henderson, Alex Bennée, Philippe Mathieu-Daudé On 27.02.19 12:14, David Hildenbrand wrote: > We want to make use of vectors, so use -march=z13. To make it compile, > use a reasonable optimization level (-O2), which seems to work just fine > with all tests. > > Add some infrastructure for checking if SIGILL will be properly > triggered. > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > tests/tcg/s390x/Makefile.target | 3 ++- > tests/tcg/s390x/helper.h | 28 +++++++++++++++++++++ > tests/tcg/s390x/signal_helper.inc.c | 39 +++++++++++++++++++++++++++++ > tests/tcg/s390x/vlgv.c | 37 +++++++++++++++++++++++++++ > 4 files changed, 106 insertions(+), 1 deletion(-) > create mode 100644 tests/tcg/s390x/helper.h > create mode 100644 tests/tcg/s390x/signal_helper.inc.c > create mode 100644 tests/tcg/s390x/vlgv.c > > diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target > index 151dc075aa..d1ae755ab9 100644 > --- a/tests/tcg/s390x/Makefile.target > +++ b/tests/tcg/s390x/Makefile.target > @@ -1,8 +1,9 @@ > VPATH+=$(SRC_PATH)/tests/tcg/s390x > -CFLAGS+=-march=zEC12 -m64 > +CFLAGS+=-march=z13 -m64 -O2 > TESTS+=hello-s390x > TESTS+=csst > TESTS+=ipm > TESTS+=exrl-trt > TESTS+=exrl-trtr > TESTS+=pack > +TESTS+=vlgv > diff --git a/tests/tcg/s390x/helper.h b/tests/tcg/s390x/helper.h > new file mode 100644 > index 0000000000..845b8bb504 > --- /dev/null > +++ b/tests/tcg/s390x/helper.h > @@ -0,0 +1,28 @@ > +#ifndef TEST_TCG_S390x_VECTOR_H > +#define TEST_TCG_S390x_VECTOR_H > + > +#include <stdint.h> > + > +#define ES_8 0 > +#define ES_16 1 > +#define ES_32 2 > +#define ES_64 3 > +#define ES_128 4 > + > +typedef union S390Vector { > + __uint128_t v; > + uint64_t q[2]; > + uint32_t d[4]; > + uint16_t w[8]; > + uint8_t h[16]; > +} S390Vector; > + > +static inline void check(const char *s, bool cond) > +{ > + if (!cond) { > + fprintf(stderr, "Check failed: %s\n", s); > + exit(-1); > + } > +} > + > +#endif /* TEST_TCG_S390x_VECTOR_H */ > diff --git a/tests/tcg/s390x/signal_helper.inc.c b/tests/tcg/s390x/signal_helper.inc.c > new file mode 100644 > index 0000000000..5bd69ca76a > --- /dev/null > +++ b/tests/tcg/s390x/signal_helper.inc.c > @@ -0,0 +1,39 @@ > +#include <stdlib.h> > +#include <stdio.h> > +#include <stdbool.h> > +#include <signal.h> > +#include <setjmp.h> > +#include "helper.h" > + > +jmp_buf jmp_env; > + > +static void sig_sigill(int sig) > +{ > + if (sig != SIGILL) { > + check("Wrong signal received", false); > + } > + longjmp(jmp_env, 1); > +} > + > +#define CHECK_SIGILL(STATEMENT) \ > +do { \ > + struct sigaction act; \ > + \ > + act.sa_handler = sig_sigill; \ > + act.sa_flags = 0; \ > + if (sigaction(SIGILL, &act, NULL)) { \ > + check("SIGILL handler not registered", false); \ > + } \ > + \ > + if (setjmp(jmp_env) == 0) { \ > + STATEMENT; \ > + check("SIGILL not triggered", false); \ > + } \ > + \ > + act.sa_handler = SIG_DFL; \ > + sigemptyset(&act.sa_mask); \ > + act.sa_flags = 0; \ > + if (sigaction(SIGILL, &act, NULL)) { \ > + check("SIGILL handler not unregistered", false); \ > + } \ > +} while (0) Now this is some interesting hackery. I changed it to jmp_buf jmp_env; static void sig_sigill(int sig) { if (sig != SIGILL) { check("Wrong signal received", false); } longjmp(jmp_env, 1); } #define CHECK_SIGILL(STATEMENT) \ do { \ if (signal(SIGILL, sig_sigill) == SIG_ERR) { \ check("SIGILL not registered", false); \ } \ if (setjmp(jmp_env) == 0) { \ STATEMENT; \ check("SIGILL not triggered", false); \ } \ if (signal(SIGILL, SIG_DFL) == SIG_ERR) { \ check("SIGILL not registered", false); \ } \ } while (0) However it only works once. During the second signal, QEMU decides to set the default handler. 1. In a signal handler that signal is blocked. We leave that handler via a longjump. So after the first signal, the signal is blocked. 2. In QEMU, we decide that synchronous signals cannot be blocked and simply override the signal handler to default handler. So on the second signal, we execute the default handler and not our defined handler. Multiple ways to fix: 1. We have to manually unblock the signal in that hackery above. 2. In QEMU, don't block synchronous signals. 3. In QEMU, don't set the signal handler to the default handler in case a synchronous signal is blocked. Trying to run the test on a real s390x linux indicates that 1. should be the right approach. [linux1@redhat-kvm ~]$ ./vge Illegal instruction (core dumped) -- Thanks, David / dhildenb ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 2/2] tests/tcg: target/s390: Add test for VECTOR LOAD GR FROM VR ELEMENT 2019-02-27 19:37 ` David Hildenbrand @ 2019-02-27 20:19 ` Alex Bennée 2019-02-27 20:20 ` David Hildenbrand 2019-02-28 0:17 ` Richard Henderson 1 sibling, 1 reply; 18+ messages in thread From: Alex Bennée @ 2019-02-27 20:19 UTC (permalink / raw) To: David Hildenbrand Cc: qemu-devel, qemu-s390x, Thomas Huth, Cornelia Huck, Richard Henderson, Philippe Mathieu-Daudé David Hildenbrand <david@redhat.com> writes: > On 27.02.19 12:14, David Hildenbrand wrote: >> We want to make use of vectors, so use -march=z13. To make it compile, >> use a reasonable optimization level (-O2), which seems to work just fine >> with all tests. >> >> Add some infrastructure for checking if SIGILL will be properly >> triggered. >> >> Signed-off-by: David Hildenbrand <david@redhat.com> >> --- >> tests/tcg/s390x/Makefile.target | 3 ++- >> tests/tcg/s390x/helper.h | 28 +++++++++++++++++++++ >> tests/tcg/s390x/signal_helper.inc.c | 39 +++++++++++++++++++++++++++++ >> tests/tcg/s390x/vlgv.c | 37 +++++++++++++++++++++++++++ >> 4 files changed, 106 insertions(+), 1 deletion(-) >> create mode 100644 tests/tcg/s390x/helper.h >> create mode 100644 tests/tcg/s390x/signal_helper.inc.c >> create mode 100644 tests/tcg/s390x/vlgv.c >> >> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target >> index 151dc075aa..d1ae755ab9 100644 >> --- a/tests/tcg/s390x/Makefile.target >> +++ b/tests/tcg/s390x/Makefile.target >> @@ -1,8 +1,9 @@ >> VPATH+=$(SRC_PATH)/tests/tcg/s390x >> -CFLAGS+=-march=zEC12 -m64 >> +CFLAGS+=-march=z13 -m64 -O2 >> TESTS+=hello-s390x >> TESTS+=csst >> TESTS+=ipm >> TESTS+=exrl-trt >> TESTS+=exrl-trtr >> TESTS+=pack >> +TESTS+=vlgv >> diff --git a/tests/tcg/s390x/helper.h b/tests/tcg/s390x/helper.h >> new file mode 100644 >> index 0000000000..845b8bb504 >> --- /dev/null >> +++ b/tests/tcg/s390x/helper.h >> @@ -0,0 +1,28 @@ >> +#ifndef TEST_TCG_S390x_VECTOR_H >> +#define TEST_TCG_S390x_VECTOR_H >> + >> +#include <stdint.h> >> + >> +#define ES_8 0 >> +#define ES_16 1 >> +#define ES_32 2 >> +#define ES_64 3 >> +#define ES_128 4 >> + >> +typedef union S390Vector { >> + __uint128_t v; >> + uint64_t q[2]; >> + uint32_t d[4]; >> + uint16_t w[8]; >> + uint8_t h[16]; >> +} S390Vector; >> + >> +static inline void check(const char *s, bool cond) >> +{ >> + if (!cond) { >> + fprintf(stderr, "Check failed: %s\n", s); >> + exit(-1); >> + } >> +} >> + >> +#endif /* TEST_TCG_S390x_VECTOR_H */ >> diff --git a/tests/tcg/s390x/signal_helper.inc.c b/tests/tcg/s390x/signal_helper.inc.c >> new file mode 100644 >> index 0000000000..5bd69ca76a >> --- /dev/null >> +++ b/tests/tcg/s390x/signal_helper.inc.c >> @@ -0,0 +1,39 @@ >> +#include <stdlib.h> >> +#include <stdio.h> >> +#include <stdbool.h> >> +#include <signal.h> >> +#include <setjmp.h> >> +#include "helper.h" >> + >> +jmp_buf jmp_env; >> + >> +static void sig_sigill(int sig) >> +{ >> + if (sig != SIGILL) { >> + check("Wrong signal received", false); >> + } >> + longjmp(jmp_env, 1); >> +} >> + >> +#define CHECK_SIGILL(STATEMENT) \ >> +do { \ >> + struct sigaction act; \ >> + \ >> + act.sa_handler = sig_sigill; \ >> + act.sa_flags = 0; \ >> + if (sigaction(SIGILL, &act, NULL)) { \ >> + check("SIGILL handler not registered", false); \ >> + } \ >> + \ >> + if (setjmp(jmp_env) == 0) { \ >> + STATEMENT; \ >> + check("SIGILL not triggered", false); \ >> + } \ >> + \ >> + act.sa_handler = SIG_DFL; \ >> + sigemptyset(&act.sa_mask); \ >> + act.sa_flags = 0; \ >> + if (sigaction(SIGILL, &act, NULL)) { \ >> + check("SIGILL handler not unregistered", false); \ >> + } \ >> +} while (0) > > Now this is some interesting hackery. > > I changed it to > > jmp_buf jmp_env; > > static void sig_sigill(int sig) > { > if (sig != SIGILL) { > check("Wrong signal received", false); > } > longjmp(jmp_env, 1); > } > > #define CHECK_SIGILL(STATEMENT) \ > do { \ > if (signal(SIGILL, sig_sigill) == SIG_ERR) { \ > check("SIGILL not registered", false); \ > } \ > if (setjmp(jmp_env) == 0) { \ > STATEMENT; \ > check("SIGILL not triggered", false); \ > } \ > if (signal(SIGILL, SIG_DFL) == SIG_ERR) { \ > check("SIGILL not registered", false); \ > } \ > } while (0) > > > However it only works once. During the second signal, QEMU decides to > set the default handler. > > 1. In a signal handler that signal is blocked. We leave that handler via > a longjump. So after the first signal, the signal is blocked. > > 2. In QEMU, we decide that synchronous signals cannot be blocked and > simply override the signal handler to default handler. So on the second > signal, we execute the default handler and not our defined handler. > > Multiple ways to fix: > > 1. We have to manually unblock the signal in that hackery above. > 2. In QEMU, don't block synchronous signals. > 3. In QEMU, don't set the signal handler to the default handler in case > a synchronous signal is blocked. This all seems a little over-engineered. This is a single-threaded test so what's wrong with a couple of flags: bool should_get_sigill; and the handler doing: if (!should_get_sigill) { unexpected_sigills++; } Tests don't have to be pretty, just reliable. > > > Trying to run the test on a real s390x linux indicates that 1. should be > the right approach. > > [linux1@redhat-kvm ~]$ ./vge > Illegal instruction (core dumped) -- Alex Bennée ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 2/2] tests/tcg: target/s390: Add test for VECTOR LOAD GR FROM VR ELEMENT 2019-02-27 20:19 ` Alex Bennée @ 2019-02-27 20:20 ` David Hildenbrand 2019-02-27 21:40 ` Alex Bennée 0 siblings, 1 reply; 18+ messages in thread From: David Hildenbrand @ 2019-02-27 20:20 UTC (permalink / raw) To: Alex Bennée Cc: qemu-devel, qemu-s390x, Thomas Huth, Cornelia Huck, Richard Henderson, Philippe Mathieu-Daudé On 27.02.19 21:19, Alex Bennée wrote: > > David Hildenbrand <david@redhat.com> writes: > >> On 27.02.19 12:14, David Hildenbrand wrote: >>> We want to make use of vectors, so use -march=z13. To make it compile, >>> use a reasonable optimization level (-O2), which seems to work just fine >>> with all tests. >>> >>> Add some infrastructure for checking if SIGILL will be properly >>> triggered. >>> >>> Signed-off-by: David Hildenbrand <david@redhat.com> >>> --- >>> tests/tcg/s390x/Makefile.target | 3 ++- >>> tests/tcg/s390x/helper.h | 28 +++++++++++++++++++++ >>> tests/tcg/s390x/signal_helper.inc.c | 39 +++++++++++++++++++++++++++++ >>> tests/tcg/s390x/vlgv.c | 37 +++++++++++++++++++++++++++ >>> 4 files changed, 106 insertions(+), 1 deletion(-) >>> create mode 100644 tests/tcg/s390x/helper.h >>> create mode 100644 tests/tcg/s390x/signal_helper.inc.c >>> create mode 100644 tests/tcg/s390x/vlgv.c >>> >>> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target >>> index 151dc075aa..d1ae755ab9 100644 >>> --- a/tests/tcg/s390x/Makefile.target >>> +++ b/tests/tcg/s390x/Makefile.target >>> @@ -1,8 +1,9 @@ >>> VPATH+=$(SRC_PATH)/tests/tcg/s390x >>> -CFLAGS+=-march=zEC12 -m64 >>> +CFLAGS+=-march=z13 -m64 -O2 >>> TESTS+=hello-s390x >>> TESTS+=csst >>> TESTS+=ipm >>> TESTS+=exrl-trt >>> TESTS+=exrl-trtr >>> TESTS+=pack >>> +TESTS+=vlgv >>> diff --git a/tests/tcg/s390x/helper.h b/tests/tcg/s390x/helper.h >>> new file mode 100644 >>> index 0000000000..845b8bb504 >>> --- /dev/null >>> +++ b/tests/tcg/s390x/helper.h >>> @@ -0,0 +1,28 @@ >>> +#ifndef TEST_TCG_S390x_VECTOR_H >>> +#define TEST_TCG_S390x_VECTOR_H >>> + >>> +#include <stdint.h> >>> + >>> +#define ES_8 0 >>> +#define ES_16 1 >>> +#define ES_32 2 >>> +#define ES_64 3 >>> +#define ES_128 4 >>> + >>> +typedef union S390Vector { >>> + __uint128_t v; >>> + uint64_t q[2]; >>> + uint32_t d[4]; >>> + uint16_t w[8]; >>> + uint8_t h[16]; >>> +} S390Vector; >>> + >>> +static inline void check(const char *s, bool cond) >>> +{ >>> + if (!cond) { >>> + fprintf(stderr, "Check failed: %s\n", s); >>> + exit(-1); >>> + } >>> +} >>> + >>> +#endif /* TEST_TCG_S390x_VECTOR_H */ >>> diff --git a/tests/tcg/s390x/signal_helper.inc.c b/tests/tcg/s390x/signal_helper.inc.c >>> new file mode 100644 >>> index 0000000000..5bd69ca76a >>> --- /dev/null >>> +++ b/tests/tcg/s390x/signal_helper.inc.c >>> @@ -0,0 +1,39 @@ >>> +#include <stdlib.h> >>> +#include <stdio.h> >>> +#include <stdbool.h> >>> +#include <signal.h> >>> +#include <setjmp.h> >>> +#include "helper.h" >>> + >>> +jmp_buf jmp_env; >>> + >>> +static void sig_sigill(int sig) >>> +{ >>> + if (sig != SIGILL) { >>> + check("Wrong signal received", false); >>> + } >>> + longjmp(jmp_env, 1); >>> +} >>> + >>> +#define CHECK_SIGILL(STATEMENT) \ >>> +do { \ >>> + struct sigaction act; \ >>> + \ >>> + act.sa_handler = sig_sigill; \ >>> + act.sa_flags = 0; \ >>> + if (sigaction(SIGILL, &act, NULL)) { \ >>> + check("SIGILL handler not registered", false); \ >>> + } \ >>> + \ >>> + if (setjmp(jmp_env) == 0) { \ >>> + STATEMENT; \ >>> + check("SIGILL not triggered", false); \ >>> + } \ >>> + \ >>> + act.sa_handler = SIG_DFL; \ >>> + sigemptyset(&act.sa_mask); \ >>> + act.sa_flags = 0; \ >>> + if (sigaction(SIGILL, &act, NULL)) { \ >>> + check("SIGILL handler not unregistered", false); \ >>> + } \ >>> +} while (0) >> >> Now this is some interesting hackery. >> >> I changed it to >> >> jmp_buf jmp_env; >> >> static void sig_sigill(int sig) >> { >> if (sig != SIGILL) { >> check("Wrong signal received", false); >> } >> longjmp(jmp_env, 1); >> } >> >> #define CHECK_SIGILL(STATEMENT) \ >> do { \ >> if (signal(SIGILL, sig_sigill) == SIG_ERR) { \ >> check("SIGILL not registered", false); \ >> } \ >> if (setjmp(jmp_env) == 0) { \ >> STATEMENT; \ >> check("SIGILL not triggered", false); \ >> } \ >> if (signal(SIGILL, SIG_DFL) == SIG_ERR) { \ >> check("SIGILL not registered", false); \ >> } \ >> } while (0) >> >> >> However it only works once. During the second signal, QEMU decides to >> set the default handler. >> >> 1. In a signal handler that signal is blocked. We leave that handler via >> a longjump. So after the first signal, the signal is blocked. >> >> 2. In QEMU, we decide that synchronous signals cannot be blocked and >> simply override the signal handler to default handler. So on the second >> signal, we execute the default handler and not our defined handler. >> >> Multiple ways to fix: >> >> 1. We have to manually unblock the signal in that hackery above. >> 2. In QEMU, don't block synchronous signals. >> 3. In QEMU, don't set the signal handler to the default handler in case >> a synchronous signal is blocked. > > > This all seems a little over-engineered. This is a single-threaded test > so what's wrong with a couple of flags: I have to jump over the actual broken instruction and want to avoid patching it. Otherwise I'll loop forever ... -- Thanks, David / dhildenb ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 2/2] tests/tcg: target/s390: Add test for VECTOR LOAD GR FROM VR ELEMENT 2019-02-27 20:20 ` David Hildenbrand @ 2019-02-27 21:40 ` Alex Bennée 2019-02-28 0:24 ` Richard Henderson 0 siblings, 1 reply; 18+ messages in thread From: Alex Bennée @ 2019-02-27 21:40 UTC (permalink / raw) To: David Hildenbrand Cc: qemu-devel, qemu-s390x, Thomas Huth, Cornelia Huck, Richard Henderson, Philippe Mathieu-Daudé David Hildenbrand <david@redhat.com> writes: > On 27.02.19 21:19, Alex Bennée wrote: >> >> David Hildenbrand <david@redhat.com> writes: >> >>> On 27.02.19 12:14, David Hildenbrand wrote: >>>> We want to make use of vectors, so use -march=z13. To make it compile, >>>> use a reasonable optimization level (-O2), which seems to work just fine >>>> with all tests. >>>> >>>> Add some infrastructure for checking if SIGILL will be properly >>>> triggered. >>>> >>>> Signed-off-by: David Hildenbrand <david@redhat.com> >>>> --- >>>> tests/tcg/s390x/Makefile.target | 3 ++- >>>> tests/tcg/s390x/helper.h | 28 +++++++++++++++++++++ >>>> tests/tcg/s390x/signal_helper.inc.c | 39 +++++++++++++++++++++++++++++ >>>> tests/tcg/s390x/vlgv.c | 37 +++++++++++++++++++++++++++ >>>> 4 files changed, 106 insertions(+), 1 deletion(-) >>>> create mode 100644 tests/tcg/s390x/helper.h >>>> create mode 100644 tests/tcg/s390x/signal_helper.inc.c >>>> create mode 100644 tests/tcg/s390x/vlgv.c >>>> >>>> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target >>>> index 151dc075aa..d1ae755ab9 100644 >>>> --- a/tests/tcg/s390x/Makefile.target >>>> +++ b/tests/tcg/s390x/Makefile.target >>>> @@ -1,8 +1,9 @@ >>>> VPATH+=$(SRC_PATH)/tests/tcg/s390x >>>> -CFLAGS+=-march=zEC12 -m64 >>>> +CFLAGS+=-march=z13 -m64 -O2 >>>> TESTS+=hello-s390x >>>> TESTS+=csst >>>> TESTS+=ipm >>>> TESTS+=exrl-trt >>>> TESTS+=exrl-trtr >>>> TESTS+=pack >>>> +TESTS+=vlgv >>>> diff --git a/tests/tcg/s390x/helper.h b/tests/tcg/s390x/helper.h >>>> new file mode 100644 >>>> index 0000000000..845b8bb504 >>>> --- /dev/null >>>> +++ b/tests/tcg/s390x/helper.h >>>> @@ -0,0 +1,28 @@ >>>> +#ifndef TEST_TCG_S390x_VECTOR_H >>>> +#define TEST_TCG_S390x_VECTOR_H >>>> + >>>> +#include <stdint.h> >>>> + >>>> +#define ES_8 0 >>>> +#define ES_16 1 >>>> +#define ES_32 2 >>>> +#define ES_64 3 >>>> +#define ES_128 4 >>>> + >>>> +typedef union S390Vector { >>>> + __uint128_t v; >>>> + uint64_t q[2]; >>>> + uint32_t d[4]; >>>> + uint16_t w[8]; >>>> + uint8_t h[16]; >>>> +} S390Vector; >>>> + >>>> +static inline void check(const char *s, bool cond) >>>> +{ >>>> + if (!cond) { >>>> + fprintf(stderr, "Check failed: %s\n", s); >>>> + exit(-1); >>>> + } >>>> +} >>>> + >>>> +#endif /* TEST_TCG_S390x_VECTOR_H */ >>>> diff --git a/tests/tcg/s390x/signal_helper.inc.c b/tests/tcg/s390x/signal_helper.inc.c >>>> new file mode 100644 >>>> index 0000000000..5bd69ca76a >>>> --- /dev/null >>>> +++ b/tests/tcg/s390x/signal_helper.inc.c >>>> @@ -0,0 +1,39 @@ >>>> +#include <stdlib.h> >>>> +#include <stdio.h> >>>> +#include <stdbool.h> >>>> +#include <signal.h> >>>> +#include <setjmp.h> >>>> +#include "helper.h" >>>> + >>>> +jmp_buf jmp_env; >>>> + >>>> +static void sig_sigill(int sig) >>>> +{ >>>> + if (sig != SIGILL) { >>>> + check("Wrong signal received", false); >>>> + } >>>> + longjmp(jmp_env, 1); >>>> +} >>>> + >>>> +#define CHECK_SIGILL(STATEMENT) \ >>>> +do { \ >>>> + struct sigaction act; \ >>>> + \ >>>> + act.sa_handler = sig_sigill; \ >>>> + act.sa_flags = 0; \ >>>> + if (sigaction(SIGILL, &act, NULL)) { \ >>>> + check("SIGILL handler not registered", false); \ >>>> + } \ >>>> + \ >>>> + if (setjmp(jmp_env) == 0) { \ >>>> + STATEMENT; \ >>>> + check("SIGILL not triggered", false); \ >>>> + } \ >>>> + \ >>>> + act.sa_handler = SIG_DFL; \ >>>> + sigemptyset(&act.sa_mask); \ >>>> + act.sa_flags = 0; \ >>>> + if (sigaction(SIGILL, &act, NULL)) { \ >>>> + check("SIGILL handler not unregistered", false); \ >>>> + } \ >>>> +} while (0) >>> >>> Now this is some interesting hackery. >>> >>> I changed it to >>> >>> jmp_buf jmp_env; >>> >>> static void sig_sigill(int sig) >>> { >>> if (sig != SIGILL) { >>> check("Wrong signal received", false); >>> } >>> longjmp(jmp_env, 1); >>> } >>> >>> #define CHECK_SIGILL(STATEMENT) \ >>> do { \ >>> if (signal(SIGILL, sig_sigill) == SIG_ERR) { \ >>> check("SIGILL not registered", false); \ >>> } \ >>> if (setjmp(jmp_env) == 0) { \ >>> STATEMENT; \ >>> check("SIGILL not triggered", false); \ >>> } \ >>> if (signal(SIGILL, SIG_DFL) == SIG_ERR) { \ >>> check("SIGILL not registered", false); \ >>> } \ >>> } while (0) >>> >>> >>> However it only works once. During the second signal, QEMU decides to >>> set the default handler. >>> >>> 1. In a signal handler that signal is blocked. We leave that handler via >>> a longjump. So after the first signal, the signal is blocked. >>> >>> 2. In QEMU, we decide that synchronous signals cannot be blocked and >>> simply override the signal handler to default handler. So on the second >>> signal, we execute the default handler and not our defined handler. >>> >>> Multiple ways to fix: >>> >>> 1. We have to manually unblock the signal in that hackery above. >>> 2. In QEMU, don't block synchronous signals. >>> 3. In QEMU, don't set the signal handler to the default handler in case >>> a synchronous signal is blocked. >> >> >> This all seems a little over-engineered. This is a single-threaded test >> so what's wrong with a couple of flags: > > I have to jump over the actual broken instruction and want to avoid > patching it. Otherwise I'll loop forever ... So from: Subject: [PATCH v2 6/6] tests/tcg/aarch64: userspace system register test Date: Tue, 5 Feb 2019 19:02:24 +0000 Message-Id: <20190205190224.2198-7-alex.bennee@linaro.org> Where I had to do something similar: bool should_fail; int should_fail_count; int should_not_fail_count; uintptr_t failed_pc[10]; void sigill_handler(int signo, siginfo_t *si, void *data) { ucontext_t *uc = (ucontext_t *)data; if (should_fail) { should_fail_count++; } else { uintptr_t pc = (uintptr_t) uc->uc_mcontext.pc; failed_pc[should_not_fail_count++] = pc; } uc->uc_mcontext.pc += 4; } But that does depend on arch making pc hackable: /* Hook in a SIGILL handler */ memset(&sa, 0, sizeof(struct sigaction)); sa.sa_flags = SA_SIGINFO; sa.sa_sigaction = &sigill_handler; sigemptyset(&sa.sa_mask); And crucially have nice regular sized instructions. Is that not an option? -- Alex Bennée ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 2/2] tests/tcg: target/s390: Add test for VECTOR LOAD GR FROM VR ELEMENT 2019-02-27 21:40 ` Alex Bennée @ 2019-02-28 0:24 ` Richard Henderson 2019-02-28 7:11 ` David Hildenbrand 0 siblings, 1 reply; 18+ messages in thread From: Richard Henderson @ 2019-02-28 0:24 UTC (permalink / raw) To: Alex Bennée, David Hildenbrand Cc: qemu-devel, qemu-s390x, Thomas Huth, Cornelia Huck, Philippe Mathieu-Daudé On 2/27/19 1:40 PM, Alex Bennée wrote: > And crucially have nice regular sized instructions. Is that not an option? s390 insns are {2,4,6} bytes. I don't think that there's an easy way to pick out the hw status codes that would give the ilen of the faulting insn. r~ ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 2/2] tests/tcg: target/s390: Add test for VECTOR LOAD GR FROM VR ELEMENT 2019-02-28 0:24 ` Richard Henderson @ 2019-02-28 7:11 ` David Hildenbrand 0 siblings, 0 replies; 18+ messages in thread From: David Hildenbrand @ 2019-02-28 7:11 UTC (permalink / raw) To: Richard Henderson, Alex Bennée Cc: qemu-devel, qemu-s390x, Thomas Huth, Cornelia Huck, Philippe Mathieu-Daudé On 28.02.19 01:24, Richard Henderson wrote: > On 2/27/19 1:40 PM, Alex Bennée wrote: >> And crucially have nice regular sized instructions. Is that not an option? > > s390 insns are {2,4,6} bytes. I don't think that there's an easy way to pick > out the hw status codes that would give the ilen of the faulting insn. > We would have to read the faulting instruction to determine based on the two leftmost bits the ilen. But I don't consider that approach much cleaner compared to signal + setjmp (signal + sigsetjmp after I fixed it ;) ) Thanks for the pointer though, Alex > > r~ > -- Thanks, David / dhildenb ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 2/2] tests/tcg: target/s390: Add test for VECTOR LOAD GR FROM VR ELEMENT 2019-02-27 19:37 ` David Hildenbrand 2019-02-27 20:19 ` Alex Bennée @ 2019-02-28 0:17 ` Richard Henderson 2019-02-28 7:14 ` David Hildenbrand 1 sibling, 1 reply; 18+ messages in thread From: Richard Henderson @ 2019-02-28 0:17 UTC (permalink / raw) To: David Hildenbrand, qemu-devel Cc: qemu-s390x, Thomas Huth, Cornelia Huck, Alex Bennée, Philippe Mathieu-Daudé On 2/27/19 11:37 AM, David Hildenbrand wrote: > #define CHECK_SIGILL(STATEMENT) \ > do { \ > if (signal(SIGILL, sig_sigill) == SIG_ERR) { \ > check("SIGILL not registered", false); \ > } \ > if (setjmp(jmp_env) == 0) { \ > STATEMENT; \ > check("SIGILL not triggered", false); \ > } \ > if (signal(SIGILL, SIG_DFL) == SIG_ERR) { \ > check("SIGILL not registered", false); \ > } \ > } while (0) > > > However it only works once. During the second signal, QEMU decides to > set the default handler. > > 1. In a signal handler that signal is blocked. We leave that handler via > a longjump. So after the first signal, the signal is blocked. And this is why we use sigaction not signal. You can set action.sa_flags |= SA_RESETHAND to avoid needing the explicit reset (or leave it clear to avoid the reset action that you are seeing). You can set action.sa_flags |= SA_NODEFER to avoid having the signal added to the signal mask of the thread, to avoid neeing to use sigsetjmp/siglongjmp. Or you can just use sigsetjmp/siglongjmp. ;-) r~ ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 2/2] tests/tcg: target/s390: Add test for VECTOR LOAD GR FROM VR ELEMENT 2019-02-28 0:17 ` Richard Henderson @ 2019-02-28 7:14 ` David Hildenbrand 2019-02-28 17:39 ` Richard Henderson 0 siblings, 1 reply; 18+ messages in thread From: David Hildenbrand @ 2019-02-28 7:14 UTC (permalink / raw) To: Richard Henderson, qemu-devel Cc: qemu-s390x, Thomas Huth, Cornelia Huck, Alex Bennée, Philippe Mathieu-Daudé On 28.02.19 01:17, Richard Henderson wrote: > On 2/27/19 11:37 AM, David Hildenbrand wrote: >> #define CHECK_SIGILL(STATEMENT) \ >> do { \ >> if (signal(SIGILL, sig_sigill) == SIG_ERR) { \ >> check("SIGILL not registered", false); \ >> } \ >> if (setjmp(jmp_env) == 0) { \ >> STATEMENT; \ >> check("SIGILL not triggered", false); \ >> } \ >> if (signal(SIGILL, SIG_DFL) == SIG_ERR) { \ >> check("SIGILL not registered", false); \ >> } \ >> } while (0) >> >> >> However it only works once. During the second signal, QEMU decides to >> set the default handler. >> >> 1. In a signal handler that signal is blocked. We leave that handler via >> a longjump. So after the first signal, the signal is blocked. > > And this is why we use sigaction not signal. > > You can set action.sa_flags |= SA_RESETHAND to avoid needing the explicit reset > (or leave it clear to avoid the reset action that you are seeing). > > You can set action.sa_flags |= SA_NODEFER to avoid having the signal added to > the signal mask of the thread, to avoid neeing to use sigsetjmp/siglongjmp. Or > you can just use sigsetjmp/siglongjmp. ;-) You might tell at this point that it is my first time hacking on signals and I already learned a lot, thanks ;) I guess signal + sigsetjmp/siglongjmp is the one with the smalles LOC? Will give it a try. BTW: I got the idea of using setjml/longjmp from tests/tcg/multiarch/linux-test.c - we're also not caring about blocked signals there, but I guess it doesn't matter as we trigger SISEGV only once. -- Thanks, David / dhildenb ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH RFC 2/2] tests/tcg: target/s390: Add test for VECTOR LOAD GR FROM VR ELEMENT 2019-02-28 7:14 ` David Hildenbrand @ 2019-02-28 17:39 ` Richard Henderson 0 siblings, 0 replies; 18+ messages in thread From: Richard Henderson @ 2019-02-28 17:39 UTC (permalink / raw) To: David Hildenbrand, qemu-devel Cc: qemu-s390x, Thomas Huth, Cornelia Huck, Alex Bennée, Philippe Mathieu-Daudé On 2/27/19 11:14 PM, David Hildenbrand wrote: > I guess signal + sigsetjmp/siglongjmp is the one with the smalles LOC? > Will give it a try. I suppose it depends on how you write it. struct sigaction sa = { .sa_mask = SA_RESETHAND | SA_NODEFER, .sa_handler = sig_sigill, }; check("SIGILL not registered", sigaction(SIGILL, &sa, NULL) == 0); is 5 lines to the 6 you currently use for registering and unregistering the signal handler. ;-) r~ ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2019-02-28 17:39 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-02-27 11:14 [Qemu-devel] [PATCH RFC 0/2] tests/tcg: Vector instruction tests for target/s390x David Hildenbrand 2019-02-27 11:14 ` [Qemu-devel] [PATCH RFC 1/2] tests/tcg: Allow targets to set the optimization level David Hildenbrand 2019-02-27 11:46 ` Alex Bennée 2019-02-27 11:58 ` David Hildenbrand 2019-02-27 12:42 ` David Hildenbrand 2019-02-27 13:44 ` David Hildenbrand 2019-02-28 0:26 ` Richard Henderson 2019-02-27 11:14 ` [Qemu-devel] [PATCH RFC 2/2] tests/tcg: target/s390: Add test for VECTOR LOAD GR FROM VR ELEMENT David Hildenbrand 2019-02-27 11:19 ` David Hildenbrand 2019-02-27 19:37 ` David Hildenbrand 2019-02-27 20:19 ` Alex Bennée 2019-02-27 20:20 ` David Hildenbrand 2019-02-27 21:40 ` Alex Bennée 2019-02-28 0:24 ` Richard Henderson 2019-02-28 7:11 ` David Hildenbrand 2019-02-28 0:17 ` Richard Henderson 2019-02-28 7:14 ` David Hildenbrand 2019-02-28 17:39 ` Richard Henderson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.