Re: [PATCH 1/2] target/s390x: Fix SIGILL psw.addr reporting
On 21.05.21 05:01, Ilya Leoshkevich wrote:
When a s390x CPU attempts to execute an illegal instruction, an
operation exception is recognized. This is a suppressing exception,
which means that the PSW is advanced by the length of the illegal
instruction.
On the real hardware or in qemu-system-s390x the kernel then raises
SIGILL with si_addr pointing to the suppressed instruction and
psw.addr containing the updated PSW.
Unfortunately qemu-s390x sets both to the address of the suppressed
instruction at the moment. Fix by sharing the PSW advancement logic
with qemu-system-s390x and setting si_addr to the address of the
instruction that raised the exception.
Buglink: https://bugs.launchpad.net/qemu/+bug/1920913
Signed-off-by: Ilya Leoshkevich
---
linux-user/s390x/cpu_loop.c | 6 +++-
target/s390x/excp_helper.c | 69 -
target/s390x/internal.h | 1 +
3 files changed, 43 insertions(+), 33 deletions(-)
diff --git a/linux-user/s390x/cpu_loop.c b/linux-user/s390x/cpu_loop.c
index f2d1215fb1..6f5462d4f8 100644
--- a/linux-user/s390x/cpu_loop.c
+++ b/linux-user/s390x/cpu_loop.c
@@ -21,6 +21,7 @@
#include "qemu-common.h"
#include "qemu.h"
#include "cpu_loop-common.h"
+#include "internal.h"
/* s390x masks the fault address it reports in si_addr for SIGSEGV and SIGBUS */
#define S390X_FAIL_ADDR_MASK -4096LL
@@ -29,6 +30,7 @@ void cpu_loop(CPUS390XState *env)
{
CPUState *cs = env_cpu(env);
int trapnr, n, sig;
+target_ulong excp_psw_addr;
target_siginfo_t info;
target_ulong addr;
abi_long ret;
@@ -38,6 +40,7 @@ void cpu_loop(CPUS390XState *env)
trapnr = cpu_exec(cs);
cpu_exec_end(cs);
process_queued_cpu_work(cs);
+excp_psw_addr = env->psw.addr;
switch (trapnr) {
case EXCP_INTERRUPT:
@@ -66,6 +69,7 @@ void cpu_loop(CPUS390XState *env)
n = TARGET_TRAP_BRKPT;
goto do_signal_pc;
case EXCP_PGM:
+s390_cpu_program_interrupt_advance_psw(env);
n = env->int_pgm_code;
switch (n) {
case PGM_OPERATION:
@@ -131,7 +135,7 @@ void cpu_loop(CPUS390XState *env)
break;
do_signal_pc:
-addr = env->psw.addr;
+addr = excp_psw_addr;
do_signal:
info.si_signo = sig;
info.si_errno = 0;
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index 20625c2c8f..0a323967ae 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -82,6 +82,42 @@ void HELPER(data_exception)(CPUS390XState *env, uint32_t dxc)
tcg_s390_data_exception(env, dxc, GETPC());
}
+void s390_cpu_program_interrupt_advance_psw(CPUS390XState *env)
+{
+switch (env->int_pgm_code) {
+case PGM_PER:
+if (env->per_perc_atmid & PER_CODE_EVENT_NULLIFICATION) {
+break;
+}
+/* FALL THROUGH */
+case PGM_OPERATION:
+case PGM_PRIVILEGED:
+case PGM_EXECUTE:
+case PGM_PROTECTION:
+case PGM_ADDRESSING:
+case PGM_SPECIFICATION:
+case PGM_DATA:
+case PGM_FIXPT_OVERFLOW:
+case PGM_FIXPT_DIVIDE:
+case PGM_DEC_OVERFLOW:
+case PGM_DEC_DIVIDE:
+case PGM_HFP_EXP_OVERFLOW:
+case PGM_HFP_EXP_UNDERFLOW:
+case PGM_HFP_SIGNIFICANCE:
+case PGM_HFP_DIVIDE:
+case PGM_TRANS_SPEC:
+case PGM_SPECIAL_OP:
+case PGM_OPERAND:
+case PGM_HFP_SQRT:
+case PGM_PC_TRANS_SPEC:
+case PGM_ALET_SPEC:
+case PGM_MONITOR:
+/* advance the PSW if our exception is not nullifying */
+env->psw.addr += env->int_pgm_ilen;
+break;
+}
+}
+
#if defined(CONFIG_USER_ONLY)
void s390_cpu_do_interrupt(CPUState *cs)
@@ -202,38 +238,7 @@ static void do_program_interrupt(CPUS390XState *env)
assert(ilen == 2 || ilen == 4 || ilen == 6);
-switch (env->int_pgm_code) {
-case PGM_PER:
-if (env->per_perc_atmid & PER_CODE_EVENT_NULLIFICATION) {
-break;
-}
-/* FALL THROUGH */
-case PGM_OPERATION:
-case PGM_PRIVILEGED:
-case PGM_EXECUTE:
-case PGM_PROTECTION:
-case PGM_ADDRESSING:
-case PGM_SPECIFICATION:
-case PGM_DATA:
-case PGM_FIXPT_OVERFLOW:
-case PGM_FIXPT_DIVIDE:
-case PGM_DEC_OVERFLOW:
-case PGM_DEC_DIVIDE:
-case PGM_HFP_EXP_OVERFLOW:
-case PGM_HFP_EXP_UNDERFLOW:
-case PGM_HFP_SIGNIFICANCE:
-case PGM_HFP_DIVIDE:
-case PGM_TRANS_SPEC:
-case PGM_SPECIAL_OP:
-case PGM_OPERAND:
-case PGM_HFP_SQRT:
-case PGM_PC_TRANS_SPEC:
-case PGM_ALET_SPEC:
-case PGM_MONITOR:
-/* advance the PSW if our exception is not nullifying */
-env->psw.addr += ilen;
-break;
-}
+s390_cpu_program_interrupt_advance_psw(env);
qemu_log_mask(CPU_LOG_INT,
"%s: code=0x%x ilen=%d psw: %" PRIx64 " %" PRIx64 "\n",
diff --git
[PATCH 1/2] target/s390x: Fix SIGILL psw.addr reporting
When a s390x CPU attempts to execute an illegal instruction, an
operation exception is recognized. This is a suppressing exception,
which means that the PSW is advanced by the length of the illegal
instruction.
On the real hardware or in qemu-system-s390x the kernel then raises
SIGILL with si_addr pointing to the suppressed instruction and
psw.addr containing the updated PSW.
Unfortunately qemu-s390x sets both to the address of the suppressed
instruction at the moment. Fix by sharing the PSW advancement logic
with qemu-system-s390x and setting si_addr to the address of the
instruction that raised the exception.
Buglink: https://bugs.launchpad.net/qemu/+bug/1920913
Signed-off-by: Ilya Leoshkevich
---
linux-user/s390x/cpu_loop.c | 6 +++-
target/s390x/excp_helper.c | 69 -
target/s390x/internal.h | 1 +
3 files changed, 43 insertions(+), 33 deletions(-)
diff --git a/linux-user/s390x/cpu_loop.c b/linux-user/s390x/cpu_loop.c
index f2d1215fb1..6f5462d4f8 100644
--- a/linux-user/s390x/cpu_loop.c
+++ b/linux-user/s390x/cpu_loop.c
@@ -21,6 +21,7 @@
#include "qemu-common.h"
#include "qemu.h"
#include "cpu_loop-common.h"
+#include "internal.h"
/* s390x masks the fault address it reports in si_addr for SIGSEGV and SIGBUS
*/
#define S390X_FAIL_ADDR_MASK -4096LL
@@ -29,6 +30,7 @@ void cpu_loop(CPUS390XState *env)
{
CPUState *cs = env_cpu(env);
int trapnr, n, sig;
+target_ulong excp_psw_addr;
target_siginfo_t info;
target_ulong addr;
abi_long ret;
@@ -38,6 +40,7 @@ void cpu_loop(CPUS390XState *env)
trapnr = cpu_exec(cs);
cpu_exec_end(cs);
process_queued_cpu_work(cs);
+excp_psw_addr = env->psw.addr;
switch (trapnr) {
case EXCP_INTERRUPT:
@@ -66,6 +69,7 @@ void cpu_loop(CPUS390XState *env)
n = TARGET_TRAP_BRKPT;
goto do_signal_pc;
case EXCP_PGM:
+s390_cpu_program_interrupt_advance_psw(env);
n = env->int_pgm_code;
switch (n) {
case PGM_OPERATION:
@@ -131,7 +135,7 @@ void cpu_loop(CPUS390XState *env)
break;
do_signal_pc:
-addr = env->psw.addr;
+addr = excp_psw_addr;
do_signal:
info.si_signo = sig;
info.si_errno = 0;
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index 20625c2c8f..0a323967ae 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -82,6 +82,42 @@ void HELPER(data_exception)(CPUS390XState *env, uint32_t dxc)
tcg_s390_data_exception(env, dxc, GETPC());
}
+void s390_cpu_program_interrupt_advance_psw(CPUS390XState *env)
+{
+switch (env->int_pgm_code) {
+case PGM_PER:
+if (env->per_perc_atmid & PER_CODE_EVENT_NULLIFICATION) {
+break;
+}
+/* FALL THROUGH */
+case PGM_OPERATION:
+case PGM_PRIVILEGED:
+case PGM_EXECUTE:
+case PGM_PROTECTION:
+case PGM_ADDRESSING:
+case PGM_SPECIFICATION:
+case PGM_DATA:
+case PGM_FIXPT_OVERFLOW:
+case PGM_FIXPT_DIVIDE:
+case PGM_DEC_OVERFLOW:
+case PGM_DEC_DIVIDE:
+case PGM_HFP_EXP_OVERFLOW:
+case PGM_HFP_EXP_UNDERFLOW:
+case PGM_HFP_SIGNIFICANCE:
+case PGM_HFP_DIVIDE:
+case PGM_TRANS_SPEC:
+case PGM_SPECIAL_OP:
+case PGM_OPERAND:
+case PGM_HFP_SQRT:
+case PGM_PC_TRANS_SPEC:
+case PGM_ALET_SPEC:
+case PGM_MONITOR:
+/* advance the PSW if our exception is not nullifying */
+env->psw.addr += env->int_pgm_ilen;
+break;
+}
+}
+
#if defined(CONFIG_USER_ONLY)
void s390_cpu_do_interrupt(CPUState *cs)
@@ -202,38 +238,7 @@ static void do_program_interrupt(CPUS390XState *env)
assert(ilen == 2 || ilen == 4 || ilen == 6);
-switch (env->int_pgm_code) {
-case PGM_PER:
-if (env->per_perc_atmid & PER_CODE_EVENT_NULLIFICATION) {
-break;
-}
-/* FALL THROUGH */
-case PGM_OPERATION:
-case PGM_PRIVILEGED:
-case PGM_EXECUTE:
-case PGM_PROTECTION:
-case PGM_ADDRESSING:
-case PGM_SPECIFICATION:
-case PGM_DATA:
-case PGM_FIXPT_OVERFLOW:
-case PGM_FIXPT_DIVIDE:
-case PGM_DEC_OVERFLOW:
-case PGM_DEC_DIVIDE:
-case PGM_HFP_EXP_OVERFLOW:
-case PGM_HFP_EXP_UNDERFLOW:
-case PGM_HFP_SIGNIFICANCE:
-case PGM_HFP_DIVIDE:
-case PGM_TRANS_SPEC:
-case PGM_SPECIAL_OP:
-case PGM_OPERAND:
-case PGM_HFP_SQRT:
-case PGM_PC_TRANS_SPEC:
-case PGM_ALET_SPEC:
-case PGM_MONITOR:
-/* advance the PSW if our exception is not nullifying */
-env->psw.addr += ilen;
-break;
-}
+s390_cpu_program_interrupt_advance_psw(env);
qemu_log_mask(CPU_LOG_INT,
"%s: code=0x%x ilen=%d psw: %" PRIx64 " %" PRIx64 "\n",
diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index 11515bb617..9f1665ccbf 100644
---
