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
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
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
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
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
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
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~
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
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~
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
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~
© 2016 - 2025 Red Hat, Inc.