Re: [Qemu-devel] [PATCH RFC 2/2] tests/tcg: target/s390: Add test for VECTOR LOAD GR FROM VR ELEMENT

[David Hildenbrand <david@redhat.com>]

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