* [PATCH v2 0/2] linux-user/s390x: signal with SIGFPE on compare-and-trap
@ 2021-07-07 13:42 Jonathan Albrecht
2021-07-07 13:42 ` [PATCH v2 1/2] " Jonathan Albrecht
2021-07-07 13:42 ` [PATCH v2 2/2] tests/tcg: Test that compare-and-trap raises SIGFPE Jonathan Albrecht
0 siblings, 2 replies; 8+ messages in thread
From: Jonathan Albrecht @ 2021-07-07 13:42 UTC (permalink / raw)
To: qemu-devel
Cc: ruixin.bao, Jonathan Albrecht, iii, david, cohuck,
richard.henderson, laurent, borntraeger, qemu-s390x, krebbel
qemu-s390x signals with SIGILL on compare-and-trap instructions. This
breaks OpenJDK which expects SIGFPE in its implementation of implicit
exceptions.
This patch depends on [PATCH v6 0/2] target/s390x: Fix SIGILL and SIGFPE
psw.addr reporting
https://lore.kernel.org/qemu-devel/20210705210434.45824-1-iii@linux.ibm.com/
Based-on: 20210705210434.45824-1-iii@linux.ibm.com
v1 -> v2:
- Update to latest version of '... psw.addr reporting' patch
- Rebase to master and fix conflicts in tests/tcg/s390x/Makefile.target
Jonathan Albrecht (2):
linux-user/s390x: signal with SIGFPE on compare-and-trap
tests/tcg: Test that compare-and-trap raises SIGFPE
linux-user/s390x/cpu_loop.c | 19 +++---
tests/tcg/s390x/Makefile.target | 1 +
tests/tcg/s390x/trap.c | 102 ++++++++++++++++++++++++++++++++
3 files changed, 113 insertions(+), 9 deletions(-)
create mode 100644 tests/tcg/s390x/trap.c
--
2.31.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] linux-user/s390x: signal with SIGFPE on compare-and-trap
2021-07-07 13:42 [PATCH v2 0/2] linux-user/s390x: signal with SIGFPE on compare-and-trap Jonathan Albrecht
@ 2021-07-07 13:42 ` Jonathan Albrecht
2021-07-08 17:08 ` Richard Henderson
2021-07-07 13:42 ` [PATCH v2 2/2] tests/tcg: Test that compare-and-trap raises SIGFPE Jonathan Albrecht
1 sibling, 1 reply; 8+ messages in thread
From: Jonathan Albrecht @ 2021-07-07 13:42 UTC (permalink / raw)
To: qemu-devel
Cc: ruixin.bao, Jonathan Albrecht, iii, david, cohuck,
richard.henderson, laurent, borntraeger, qemu-s390x, krebbel
Currently when a compare-and-trap instruction is executed, qemu will
always raise a SIGILL signal. On real hardware, a SIGFPE is raised.
Change the PGM_DATA case in cpu_loop to follow the behavior in
linux kernel /arch/s390/kernel/traps.c.
* Only raise SIGILL if DXC == 0
* If DXC matches an IEEE exception, raise SIGFPE with correct si_code
* Raise SIGFPE with si_code == 0 for everything else
When applied on 20210705210434.45824-2-iii@linux.ibm.com, this fixes
crashes in the java jdk such as the linked bug.
Buglink: https://bugs.launchpad.net/qemu/+bug/1920913
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/319
Signed-off-by: Jonathan Albrecht <jonathan.albrecht@linux.vnet.ibm.com>
---
linux-user/s390x/cpu_loop.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/linux-user/s390x/cpu_loop.c b/linux-user/s390x/cpu_loop.c
index 22f2e89c62..6e7dfb290a 100644
--- a/linux-user/s390x/cpu_loop.c
+++ b/linux-user/s390x/cpu_loop.c
@@ -106,11 +106,13 @@ void cpu_loop(CPUS390XState *env)
case PGM_DATA:
n = (env->fpc >> 8) & 0xff;
- if (n == 0xff) {
- /* compare-and-trap */
+ if (n == 0) {
goto do_sigill_opn;
- } else {
- /* An IEEE exception, simulated or otherwise. */
+ }
+
+ sig = TARGET_SIGFPE;
+ if ((n & 0x03) == 0) {
+ /* An IEEE exception, simulated or otherwise. */
if (n & 0x80) {
n = TARGET_FPE_FLTINV;
} else if (n & 0x40) {
@@ -121,13 +123,12 @@ void cpu_loop(CPUS390XState *env)
n = TARGET_FPE_FLTUND;
} else if (n & 0x08) {
n = TARGET_FPE_FLTRES;
- } else {
- /* ??? Quantum exception; BFP, DFP error. */
- goto do_sigill_opn;
}
- sig = TARGET_SIGFPE;
- goto do_signal_pc;
+ } else {
+ /* compare-and-trap */
+ n = 0;
}
+ goto do_signal_pc;
default:
fprintf(stderr, "Unhandled program exception: %#x\n", n);
--
2.31.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] tests/tcg: Test that compare-and-trap raises SIGFPE
2021-07-07 13:42 [PATCH v2 0/2] linux-user/s390x: signal with SIGFPE on compare-and-trap Jonathan Albrecht
2021-07-07 13:42 ` [PATCH v2 1/2] " Jonathan Albrecht
@ 2021-07-07 13:42 ` Jonathan Albrecht
2021-07-08 17:18 ` Richard Henderson
1 sibling, 1 reply; 8+ messages in thread
From: Jonathan Albrecht @ 2021-07-07 13:42 UTC (permalink / raw)
To: qemu-devel
Cc: ruixin.bao, Jonathan Albrecht, iii, david, cohuck,
richard.henderson, laurent, borntraeger, qemu-s390x, krebbel
Signed-off-by: Jonathan Albrecht <jonathan.albrecht@linux.vnet.ibm.com>
---
tests/tcg/s390x/Makefile.target | 1 +
tests/tcg/s390x/trap.c | 102 ++++++++++++++++++++++++++++++++
2 files changed, 103 insertions(+)
create mode 100644 tests/tcg/s390x/trap.c
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 0a5b25c156..d440ecd6f7 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -9,6 +9,7 @@ TESTS+=exrl-trtr
TESTS+=pack
TESTS+=mvo
TESTS+=mvc
+TESTS+=trap
# This triggers failures on s390x hosts about 4% of the time
run-signals: signals
diff --git a/tests/tcg/s390x/trap.c b/tests/tcg/s390x/trap.c
new file mode 100644
index 0000000000..d4c61c7f52
--- /dev/null
+++ b/tests/tcg/s390x/trap.c
@@ -0,0 +1,102 @@
+/*
+ * Copyright 2021 IBM Corp.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+
+#include <stdarg.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <errno.h>
+#include <string.h>
+#include <signal.h>
+
+static void error1(const char *filename, int line, const char *fmt, ...)
+{
+ va_list ap;
+ va_start(ap, fmt);
+ fprintf(stderr, "%s:%d: ", filename, line);
+ vfprintf(stderr, fmt, ap);
+ fprintf(stderr, "\n");
+ va_end(ap);
+ exit(1);
+}
+
+static int __chk_error(const char *filename, int line, int ret)
+{
+ if (ret < 0) {
+ error1(filename, line, "%m (ret=%d, errno=%d/%s)",
+ ret, errno, strerror(errno));
+ }
+ return ret;
+}
+
+#define error(fmt, ...) error1(__FILE__, __LINE__, fmt, ## __VA_ARGS__)
+
+#define chk_error(ret) __chk_error(__FILE__, __LINE__, (ret))
+
+int sigfpe_count;
+int sigill_count;
+
+static void sig_handler(int sig, siginfo_t *si, void *puc)
+{
+ if (sig == SIGFPE) {
+ if (si->si_code != 0) {
+ error("unexpected si_code: 0x%x != 0", si->si_code);
+ }
+ ++sigfpe_count;
+ return;
+ }
+
+ if (sig == SIGILL) {
+ ++sigill_count;
+ return;
+ }
+
+ error("unexpected signal 0x%x\n", sig);
+}
+
+int main(int argc, char **argv)
+{
+ sigfpe_count = sigill_count = 0;
+
+ struct sigaction act;
+
+ /* Set up SIG handler */
+ act.sa_sigaction = sig_handler;
+ sigemptyset(&act.sa_mask);
+ act.sa_flags = SA_SIGINFO;
+ chk_error(sigaction(SIGFPE, &act, NULL));
+ chk_error(sigaction(SIGILL, &act, NULL));
+
+ uint64_t z = 0x0ull;
+ uint64_t lz = 0xffffffffffffffffull;
+ asm volatile (
+ "lg %%r13,%[lz]\n"
+ "cgitne %%r13,0\n" /* SIGFPE */
+ "lg %%r13,%[z]\n"
+ "cgitne %%r13,0\n" /* no trap */
+ "nopr\n"
+ "lg %%r13,%[lz]\n"
+ "citne %%r13,0\n" /* SIGFPE */
+ "lg %%r13,%[z]\n"
+ "citne %%r13,0\n" /* no trap */
+ "nopr\n"
+ :
+ : [z] "m" (z), [lz] "m" (lz)
+ : "memory", "r13");
+
+ if (sigfpe_count != 2) {
+ error("unexpected SIGFPE count: %d != 2", sigfpe_count);
+ }
+ if (sigill_count != 0) {
+ error("unexpected SIGILL count: %d != 0", sigill_count);
+ }
+
+ printf("PASS\n");
+ return 0;
+}
--
2.31.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] linux-user/s390x: signal with SIGFPE on compare-and-trap
2021-07-07 13:42 ` [PATCH v2 1/2] " Jonathan Albrecht
@ 2021-07-08 17:08 ` Richard Henderson
2021-07-09 14:23 ` jonathan.albrecht
0 siblings, 1 reply; 8+ messages in thread
From: Richard Henderson @ 2021-07-08 17:08 UTC (permalink / raw)
To: Jonathan Albrecht, qemu-devel
Cc: ruixin.bao, iii, david, cohuck, laurent, borntraeger, qemu-s390x,
krebbel
On 7/7/21 6:42 AM, Jonathan Albrecht wrote:
> + sig = TARGET_SIGFPE;
> + if ((n & 0x03) == 0) {
> + /* An IEEE exception, simulated or otherwise. */
> if (n & 0x80) {
> n = TARGET_FPE_FLTINV;
> } else if (n & 0x40) {
> @@ -121,13 +123,12 @@ void cpu_loop(CPUS390XState *env)
> n = TARGET_FPE_FLTUND;
> } else if (n & 0x08) {
> n = TARGET_FPE_FLTRES;
> - } else {
> - /* ??? Quantum exception; BFP, DFP error. */
> - goto do_sigill_opn;
> }
> - sig = TARGET_SIGFPE;
> - goto do_signal_pc;
> + } else {
> + /* compare-and-trap */
> + n = 0;
> }
Nearly, but not quite. Replace the ??? Quantum exception with n = 0, otherwise si_code
will be garbage for that case.
The structure of the kernel code is
if (n != 0) {
/* do_fp_trap */
si_code = 0;
if ((n & 3) == 0) {
/* select on bits 6 & 7 */
}
raise sigfpe w/ si_code
} else {
raise sigill
}
The comment for compare-and-trap is misleading, because there are lots of entries in
"Figure 6-2. Data-exception codes (DXC)" which arrive there and are not compare-and-trap.
As a general comment, I think a single switch over DXC would be cleaner for both kernel
and qemu. It seems like giving different si_code for e.g. "0x40 IEEE division by zero"
and "0x43 Simulated IEEE division by zero" is actively incorrect.
r~
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] tests/tcg: Test that compare-and-trap raises SIGFPE
2021-07-07 13:42 ` [PATCH v2 2/2] tests/tcg: Test that compare-and-trap raises SIGFPE Jonathan Albrecht
@ 2021-07-08 17:18 ` Richard Henderson
0 siblings, 0 replies; 8+ messages in thread
From: Richard Henderson @ 2021-07-08 17:18 UTC (permalink / raw)
To: Jonathan Albrecht, qemu-devel
Cc: ruixin.bao, iii, david, cohuck, laurent, borntraeger, qemu-s390x,
krebbel
On 7/7/21 6:42 AM, Jonathan Albrecht wrote:
> Signed-off-by: Jonathan Albrecht<jonathan.albrecht@linux.vnet.ibm.com>
> ---
> tests/tcg/s390x/Makefile.target | 1 +
> tests/tcg/s390x/trap.c | 102 ++++++++++++++++++++++++++++++++
> 2 files changed, 103 insertions(+)
> create mode 100644 tests/tcg/s390x/trap.c
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] linux-user/s390x: signal with SIGFPE on compare-and-trap
2021-07-08 17:08 ` Richard Henderson
@ 2021-07-09 14:23 ` jonathan.albrecht
2021-07-09 14:37 ` Richard Henderson
0 siblings, 1 reply; 8+ messages in thread
From: jonathan.albrecht @ 2021-07-09 14:23 UTC (permalink / raw)
To: Richard Henderson
Cc: ruixin.bao, iii, david, cohuck, laurent, qemu-devel, borntraeger,
qemu-s390x, krebbel
On 2021-07-08 1:08 pm, Richard Henderson wrote:
> On 7/7/21 6:42 AM, Jonathan Albrecht wrote:
>> + sig = TARGET_SIGFPE;
>> + if ((n & 0x03) == 0) {
>> + /* An IEEE exception, simulated or otherwise. */
>> if (n & 0x80) {
>> n = TARGET_FPE_FLTINV;
>> } else if (n & 0x40) {
>> @@ -121,13 +123,12 @@ void cpu_loop(CPUS390XState *env)
>> n = TARGET_FPE_FLTUND;
>> } else if (n & 0x08) {
>> n = TARGET_FPE_FLTRES;
>> - } else {
>> - /* ??? Quantum exception; BFP, DFP error. */
>> - goto do_sigill_opn;
>> }
>> - sig = TARGET_SIGFPE;
>> - goto do_signal_pc;
>> + } else {
>> + /* compare-and-trap */
>> + n = 0;
>> }
>
Thanks for the review. I should have a v3 ready shortly.
> Nearly, but not quite. Replace the ??? Quantum exception with n = 0,
> otherwise si_code will be garbage for that case.
>
Thx I'll fix that.
> The structure of the kernel code is
>
> if (n != 0) {
> /* do_fp_trap */
> si_code = 0;
> if ((n & 3) == 0) {
> /* select on bits 6 & 7 */
> }
> raise sigfpe w/ si_code
> } else {
> raise sigill
> }
>
> The comment for compare-and-trap is misleading, because there are lots
> of entries in "Figure 6-2. Data-exception codes (DXC)" which arrive
> there and are not compare-and-trap.
>
I'll make the comment less specific.
> As a general comment, I think a single switch over DXC would be
> cleaner for both kernel and qemu. It seems like giving different
> si_code for e.g. "0x40 IEEE division by zero" and "0x43 Simulated IEEE
> division by zero" is actively incorrect.
>
I went over the DXC section and I see what you mean about the si_codes
for simulated IEEE exceptions. I'll plan on handling those the same as
non-simulated IEEE if no objections. Otherwise all non-IEEE will have
si_code == 0 except DXC == 0x00 will still goto do_sigill_opn.
>
> r~
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] linux-user/s390x: signal with SIGFPE on compare-and-trap
2021-07-09 14:23 ` jonathan.albrecht
@ 2021-07-09 14:37 ` Richard Henderson
2021-07-09 14:48 ` jonathan.albrecht
0 siblings, 1 reply; 8+ messages in thread
From: Richard Henderson @ 2021-07-09 14:37 UTC (permalink / raw)
To: jonathan.albrecht
Cc: ruixin.bao, iii, david, cohuck, laurent, qemu-devel, borntraeger,
qemu-s390x, krebbel
On 7/9/21 7:23 AM, jonathan.albrecht wrote:
>> As a general comment, I think a single switch over DXC would be
>> cleaner for both kernel and qemu. It seems like giving different
>> si_code for e.g. "0x40 IEEE division by zero" and "0x43 Simulated IEEE
>> division by zero" is actively incorrect.
>>
> I went over the DXC section and I see what you mean about the si_codes
> for simulated IEEE exceptions. I'll plan on handling those the same as
> non-simulated IEEE if no objections.
Only if you plan on submitting a similar patch for the kernel.
Otherwise, qemu would not match the kernel abi.
r~
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] linux-user/s390x: signal with SIGFPE on compare-and-trap
2021-07-09 14:37 ` Richard Henderson
@ 2021-07-09 14:48 ` jonathan.albrecht
0 siblings, 0 replies; 8+ messages in thread
From: jonathan.albrecht @ 2021-07-09 14:48 UTC (permalink / raw)
To: Richard Henderson
Cc: ruixin.bao, iii, david, cohuck, laurent, qemu-devel, borntraeger,
qemu-s390x, krebbel
On 2021-07-09 10:37 am, Richard Henderson wrote:
> On 7/9/21 7:23 AM, jonathan.albrecht wrote:
>>> As a general comment, I think a single switch over DXC would be
>>> cleaner for both kernel and qemu. It seems like giving different
>>> si_code for e.g. "0x40 IEEE division by zero" and "0x43 Simulated
>>> IEEE
>>> division by zero" is actively incorrect.
>>>
>> I went over the DXC section and I see what you mean about the si_codes
>> for simulated IEEE exceptions. I'll plan on handling those the same as
>> non-simulated IEEE if no objections.
>
> Only if you plan on submitting a similar patch for the kernel.
> Otherwise, qemu would not match the kernel abi.
>
Thanks for clarifying. In that case, I'll handle simulated IEEE the same
as the current kernel.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-07-09 15:00 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-07 13:42 [PATCH v2 0/2] linux-user/s390x: signal with SIGFPE on compare-and-trap Jonathan Albrecht
2021-07-07 13:42 ` [PATCH v2 1/2] " Jonathan Albrecht
2021-07-08 17:08 ` Richard Henderson
2021-07-09 14:23 ` jonathan.albrecht
2021-07-09 14:37 ` Richard Henderson
2021-07-09 14:48 ` jonathan.albrecht
2021-07-07 13:42 ` [PATCH v2 2/2] tests/tcg: Test that compare-and-trap raises SIGFPE Jonathan Albrecht
2021-07-08 17:18 ` 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.