* [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals
@ 2021-06-15 3:07 Richard Henderson
2021-06-15 3:07 ` [PATCH 1/5] target/s390x: Expose load_psw and get_psw_mask to cpu.h Richard Henderson
` (8 more replies)
0 siblings, 9 replies; 23+ messages in thread
From: Richard Henderson @ 2021-06-15 3:07 UTC (permalink / raw)
To: qemu-devel; +Cc: ruixin.bao, jonathan.albrecht, cohuck, qemu-s390x, david
The PSW_MASK_CC component of psw.mask was not handled properly
in the creation or restoration of signal frames.
r~
Richard Henderson (5):
target/s390x: Expose load_psw and get_psw_mask to cpu.h
target/s390x: Do not modify cpu state in s390_cpu_get_psw_mask
target/s390x: Improve s390_cpu_dump_state vs cc_op
target/s390x: Use s390_cpu_{set_psw,get_psw_mask} in gdbstub
linux-user/s390x: Save and restore psw.mask properly
target/s390x/cpu.h | 3 ++
target/s390x/internal.h | 5 --
linux-user/s390x/signal.c | 37 ++++++++++++--
target/s390x/cc_helper.c | 2 +-
target/s390x/excp_helper.c | 28 +++++-----
target/s390x/gdbstub.c | 15 +-----
target/s390x/helper.c | 101 ++++++++++++++++++++-----------------
target/s390x/sigp.c | 3 +-
8 files changed, 110 insertions(+), 84 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 1/5] target/s390x: Expose load_psw and get_psw_mask to cpu.h
2021-06-15 3:07 [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals Richard Henderson
@ 2021-06-15 3:07 ` Richard Henderson
2021-06-15 7:47 ` David Hildenbrand
2021-06-15 8:57 ` Alex Bennée
2021-06-15 3:07 ` [PATCH 2/5] target/s390x: Do not modify cpu state in s390_cpu_get_psw_mask Richard Henderson
` (7 subsequent siblings)
8 siblings, 2 replies; 23+ messages in thread
From: Richard Henderson @ 2021-06-15 3:07 UTC (permalink / raw)
To: qemu-devel; +Cc: ruixin.bao, jonathan.albrecht, cohuck, qemu-s390x, david
Rename to s390_cpu_set_psw and s390_cpu_get_psw_mask at the
same time. Adjust so that they compile for user-only.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/s390x/cpu.h | 3 ++
target/s390x/internal.h | 5 ---
target/s390x/cc_helper.c | 2 +-
target/s390x/excp_helper.c | 28 ++++++------
target/s390x/helper.c | 89 ++++++++++++++++++++------------------
target/s390x/sigp.c | 3 +-
6 files changed, 69 insertions(+), 61 deletions(-)
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 2464d4076c..b26ae8fff2 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -845,6 +845,9 @@ int s390_cpu_pv_mem_rw(S390CPU *cpu, unsigned int offset, void *hostbuf,
int s390_cpu_restart(S390CPU *cpu);
void s390_init_sigp(void);
+/* helper.c */
+void s390_cpu_set_psw(CPUS390XState *env, uint64_t mask, uint64_t addr);
+uint64_t s390_cpu_get_psw_mask(CPUS390XState *env);
/* outside of target/s390x/ */
S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index 11515bb617..4a18b74d10 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -235,10 +235,6 @@ int s390_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
const char *cc_name(enum cc_op cc_op);
uint32_t calc_cc(CPUS390XState *env, uint32_t cc_op, uint64_t src, uint64_t dst,
uint64_t vr);
-#ifndef CONFIG_USER_ONLY
-void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr);
-#endif /* CONFIG_USER_ONLY */
-
/* cpu.c */
#ifndef CONFIG_USER_ONLY
@@ -303,7 +299,6 @@ void s390_cpu_gdb_init(CPUState *cs);
void s390_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
void do_restart_interrupt(CPUS390XState *env);
#ifndef CONFIG_USER_ONLY
-uint64_t get_psw_mask(CPUS390XState *env);
void s390_cpu_recompute_watchpoints(CPUState *cs);
void s390x_tod_timer(void *opaque);
void s390x_cpu_timer(void *opaque);
diff --git a/target/s390x/cc_helper.c b/target/s390x/cc_helper.c
index e7039d0d18..e7a74d66dd 100644
--- a/target/s390x/cc_helper.c
+++ b/target/s390x/cc_helper.c
@@ -509,7 +509,7 @@ uint32_t HELPER(calc_cc)(CPUS390XState *env, uint32_t cc_op, uint64_t src,
#ifndef CONFIG_USER_ONLY
void HELPER(load_psw)(CPUS390XState *env, uint64_t mask, uint64_t addr)
{
- load_psw(env, mask, addr);
+ s390_cpu_set_psw(env, mask, addr);
cpu_loop_exit(env_cpu(env));
}
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index 20625c2c8f..9c361428c8 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -252,7 +252,7 @@ static void do_program_interrupt(CPUS390XState *env)
lowcore->pgm_ilen = cpu_to_be16(ilen);
lowcore->pgm_code = cpu_to_be16(env->int_pgm_code);
- lowcore->program_old_psw.mask = cpu_to_be64(get_psw_mask(env));
+ lowcore->program_old_psw.mask = cpu_to_be64(s390_cpu_get_psw_mask(env));
lowcore->program_old_psw.addr = cpu_to_be64(env->psw.addr);
mask = be64_to_cpu(lowcore->program_new_psw.mask);
addr = be64_to_cpu(lowcore->program_new_psw.addr);
@@ -260,7 +260,7 @@ static void do_program_interrupt(CPUS390XState *env)
cpu_unmap_lowcore(lowcore);
- load_psw(env, mask, addr);
+ s390_cpu_set_psw(env, mask, addr);
}
static void do_svc_interrupt(CPUS390XState *env)
@@ -272,14 +272,14 @@ static void do_svc_interrupt(CPUS390XState *env)
lowcore->svc_code = cpu_to_be16(env->int_svc_code);
lowcore->svc_ilen = cpu_to_be16(env->int_svc_ilen);
- lowcore->svc_old_psw.mask = cpu_to_be64(get_psw_mask(env));
+ lowcore->svc_old_psw.mask = cpu_to_be64(s390_cpu_get_psw_mask(env));
lowcore->svc_old_psw.addr = cpu_to_be64(env->psw.addr + env->int_svc_ilen);
mask = be64_to_cpu(lowcore->svc_new_psw.mask);
addr = be64_to_cpu(lowcore->svc_new_psw.addr);
cpu_unmap_lowcore(lowcore);
- load_psw(env, mask, addr);
+ s390_cpu_set_psw(env, mask, addr);
/* When a PER event is pending, the PER exception has to happen
immediately after the SERVICE CALL one. */
@@ -348,12 +348,12 @@ static void do_ext_interrupt(CPUS390XState *env)
mask = be64_to_cpu(lowcore->external_new_psw.mask);
addr = be64_to_cpu(lowcore->external_new_psw.addr);
- lowcore->external_old_psw.mask = cpu_to_be64(get_psw_mask(env));
+ lowcore->external_old_psw.mask = cpu_to_be64(s390_cpu_get_psw_mask(env));
lowcore->external_old_psw.addr = cpu_to_be64(env->psw.addr);
cpu_unmap_lowcore(lowcore);
- load_psw(env, mask, addr);
+ s390_cpu_set_psw(env, mask, addr);
}
static void do_io_interrupt(CPUS390XState *env)
@@ -373,7 +373,7 @@ static void do_io_interrupt(CPUS390XState *env)
lowcore->subchannel_nr = cpu_to_be16(io->nr);
lowcore->io_int_parm = cpu_to_be32(io->parm);
lowcore->io_int_word = cpu_to_be32(io->word);
- lowcore->io_old_psw.mask = cpu_to_be64(get_psw_mask(env));
+ lowcore->io_old_psw.mask = cpu_to_be64(s390_cpu_get_psw_mask(env));
lowcore->io_old_psw.addr = cpu_to_be64(env->psw.addr);
mask = be64_to_cpu(lowcore->io_new_psw.mask);
addr = be64_to_cpu(lowcore->io_new_psw.addr);
@@ -381,7 +381,7 @@ static void do_io_interrupt(CPUS390XState *env)
cpu_unmap_lowcore(lowcore);
g_free(io);
- load_psw(env, mask, addr);
+ s390_cpu_set_psw(env, mask, addr);
}
typedef struct MchkExtSaveArea {
@@ -457,14 +457,14 @@ static void do_mchk_interrupt(CPUS390XState *env)
lowcore->clock_comp_save_area = cpu_to_be64(env->ckc >> 8);
lowcore->mcic = cpu_to_be64(mcic);
- lowcore->mcck_old_psw.mask = cpu_to_be64(get_psw_mask(env));
+ lowcore->mcck_old_psw.mask = cpu_to_be64(s390_cpu_get_psw_mask(env));
lowcore->mcck_old_psw.addr = cpu_to_be64(env->psw.addr);
mask = be64_to_cpu(lowcore->mcck_new_psw.mask);
addr = be64_to_cpu(lowcore->mcck_new_psw.addr);
cpu_unmap_lowcore(lowcore);
- load_psw(env, mask, addr);
+ s390_cpu_set_psw(env, mask, addr);
}
void s390_cpu_do_interrupt(CPUState *cs)
@@ -592,9 +592,11 @@ void s390x_cpu_debug_excp_handler(CPUState *cs)
and MVCS instrutions are not used. */
env->per_perc_atmid |= env->psw.mask & (PSW_MASK_ASC) >> 46;
- /* Remove all watchpoints to re-execute the code. A PER exception
- will be triggered, it will call load_psw which will recompute
- the watchpoints. */
+ /*
+ * Remove all watchpoints to re-execute the code. A PER exception
+ * will be triggered, it will call s390_cpu_set_psw which will
+ * recompute the watchpoints.
+ */
cpu_watchpoint_remove_all(cs, BP_CPU);
cpu_loop_exit_noexc(cs);
}
diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index 7678994feb..d311903b94 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -104,44 +104,6 @@ void s390_handle_wait(S390CPU *cpu)
}
}
-void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr)
-{
- uint64_t old_mask = env->psw.mask;
-
- env->psw.addr = addr;
- env->psw.mask = mask;
-
- /* KVM will handle all WAITs and trigger a WAIT exit on disabled_wait */
- if (!tcg_enabled()) {
- return;
- }
- env->cc_op = (mask >> 44) & 3;
-
- if ((old_mask ^ mask) & PSW_MASK_PER) {
- s390_cpu_recompute_watchpoints(env_cpu(env));
- }
-
- if (mask & PSW_MASK_WAIT) {
- s390_handle_wait(env_archcpu(env));
- }
-}
-
-uint64_t get_psw_mask(CPUS390XState *env)
-{
- uint64_t r = env->psw.mask;
-
- if (tcg_enabled()) {
- env->cc_op = calc_cc(env, env->cc_op, env->cc_src, env->cc_dst,
- env->cc_vr);
-
- r &= ~PSW_MASK_CC;
- assert(!(env->cc_op & ~3));
- r |= (uint64_t)env->cc_op << 44;
- }
-
- return r;
-}
-
LowCore *cpu_map_lowcore(CPUS390XState *env)
{
LowCore *lowcore;
@@ -168,7 +130,7 @@ void do_restart_interrupt(CPUS390XState *env)
lowcore = cpu_map_lowcore(env);
- lowcore->restart_old_psw.mask = cpu_to_be64(get_psw_mask(env));
+ lowcore->restart_old_psw.mask = cpu_to_be64(s390_cpu_get_psw_mask(env));
lowcore->restart_old_psw.addr = cpu_to_be64(env->psw.addr);
mask = be64_to_cpu(lowcore->restart_new_psw.mask);
addr = be64_to_cpu(lowcore->restart_new_psw.addr);
@@ -176,7 +138,7 @@ void do_restart_interrupt(CPUS390XState *env)
cpu_unmap_lowcore(lowcore);
env->pending_int &= ~INTERRUPT_RESTART;
- load_psw(env, mask, addr);
+ s390_cpu_set_psw(env, mask, addr);
}
void s390_cpu_recompute_watchpoints(CPUState *cs)
@@ -266,7 +228,7 @@ int s390_store_status(S390CPU *cpu, hwaddr addr, bool store_arch)
sa->grs[i] = cpu_to_be64(cpu->env.regs[i]);
}
sa->psw.addr = cpu_to_be64(cpu->env.psw.addr);
- sa->psw.mask = cpu_to_be64(get_psw_mask(&cpu->env));
+ sa->psw.mask = cpu_to_be64(s390_cpu_get_psw_mask(&cpu->env));
sa->prefix = cpu_to_be32(cpu->env.psa);
sa->fpc = cpu_to_be32(cpu->env.fpc);
sa->todpr = cpu_to_be32(cpu->env.todpr);
@@ -323,8 +285,53 @@ int s390_store_adtl_status(S390CPU *cpu, hwaddr addr, hwaddr len)
cpu_physical_memory_unmap(sa, len, 1, len);
return 0;
}
+#else
+/* For user-only, tcg is always enabled. */
+#define tcg_enabled() true
#endif /* CONFIG_USER_ONLY */
+void s390_cpu_set_psw(CPUS390XState *env, uint64_t mask, uint64_t addr)
+{
+#ifndef CONFIG_USER_ONLY
+ uint64_t old_mask = env->psw.mask;
+#endif
+
+ env->psw.addr = addr;
+ env->psw.mask = mask;
+
+ /* KVM will handle all WAITs and trigger a WAIT exit on disabled_wait */
+ if (!tcg_enabled()) {
+ return;
+ }
+ env->cc_op = (mask >> 44) & 3;
+
+#ifndef CONFIG_USER_ONLY
+ if ((old_mask ^ mask) & PSW_MASK_PER) {
+ s390_cpu_recompute_watchpoints(env_cpu(env));
+ }
+
+ if (mask & PSW_MASK_WAIT) {
+ s390_handle_wait(env_archcpu(env));
+ }
+#endif
+}
+
+uint64_t s390_cpu_get_psw_mask(CPUS390XState *env)
+{
+ uint64_t r = env->psw.mask;
+
+ if (tcg_enabled()) {
+ env->cc_op = calc_cc(env, env->cc_op, env->cc_src, env->cc_dst,
+ env->cc_vr);
+
+ r &= ~PSW_MASK_CC;
+ assert(!(env->cc_op & ~3));
+ r |= (uint64_t)env->cc_op << 44;
+ }
+
+ return r;
+}
+
void s390_cpu_dump_state(CPUState *cs, FILE *f, int flags)
{
S390CPU *cpu = S390_CPU(cs);
diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
index c604f17710..c2d5cdf061 100644
--- a/target/s390x/sigp.c
+++ b/target/s390x/sigp.c
@@ -235,7 +235,8 @@ static void sigp_restart(CPUState *cs, run_on_cpu_data arg)
cpu_synchronize_state(cs);
/*
* Set OPERATING (and unhalting) before loading the restart PSW.
- * load_psw() will then properly halt the CPU again if necessary (TCG).
+ * s390_cpu_set_psw() will then properly halt the CPU again if
+ * necessary (TCG).
*/
s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu);
do_restart_interrupt(&cpu->env);
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 2/5] target/s390x: Do not modify cpu state in s390_cpu_get_psw_mask
2021-06-15 3:07 [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals Richard Henderson
2021-06-15 3:07 ` [PATCH 1/5] target/s390x: Expose load_psw and get_psw_mask to cpu.h Richard Henderson
@ 2021-06-15 3:07 ` Richard Henderson
2021-06-15 7:49 ` David Hildenbrand
2021-06-15 3:07 ` [PATCH 3/5] target/s390x: Improve s390_cpu_dump_state vs cc_op Richard Henderson
` (6 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Richard Henderson @ 2021-06-15 3:07 UTC (permalink / raw)
To: qemu-devel; +Cc: ruixin.bao, jonathan.albrecht, cohuck, qemu-s390x, david
We want to use this function for debugging, and debug should
not modify cpu state (even non-architectural cpu state) lest
we introduce heisenbugs.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/s390x/helper.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index d311903b94..559fc3573f 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -321,12 +321,12 @@ uint64_t s390_cpu_get_psw_mask(CPUS390XState *env)
uint64_t r = env->psw.mask;
if (tcg_enabled()) {
- env->cc_op = calc_cc(env, env->cc_op, env->cc_src, env->cc_dst,
- env->cc_vr);
+ uint64_t cc = calc_cc(env, env->cc_op, env->cc_src,
+ env->cc_dst, env->cc_vr);
+ assert(cc <= 3);
r &= ~PSW_MASK_CC;
- assert(!(env->cc_op & ~3));
- r |= (uint64_t)env->cc_op << 44;
+ r |= cc << 44;
}
return r;
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 3/5] target/s390x: Improve s390_cpu_dump_state vs cc_op
2021-06-15 3:07 [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals Richard Henderson
2021-06-15 3:07 ` [PATCH 1/5] target/s390x: Expose load_psw and get_psw_mask to cpu.h Richard Henderson
2021-06-15 3:07 ` [PATCH 2/5] target/s390x: Do not modify cpu state in s390_cpu_get_psw_mask Richard Henderson
@ 2021-06-15 3:07 ` Richard Henderson
2021-06-15 7:51 ` David Hildenbrand
2021-06-15 3:07 ` [PATCH 4/5] target/s390x: Use s390_cpu_{set_psw, get_psw_mask} in gdbstub Richard Henderson
` (5 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Richard Henderson @ 2021-06-15 3:07 UTC (permalink / raw)
To: qemu-devel; +Cc: ruixin.bao, jonathan.albrecht, cohuck, qemu-s390x, david
Use s390_cpu_get_psw_mask so that we print the correct
architectural value of psw.mask. Do not print cc_op
unless tcg_enabled.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/s390x/helper.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index 559fc3573f..1445b74451 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -338,12 +338,14 @@ void s390_cpu_dump_state(CPUState *cs, FILE *f, int flags)
CPUS390XState *env = &cpu->env;
int i;
- if (env->cc_op > 3) {
- qemu_fprintf(f, "PSW=mask %016" PRIx64 " addr %016" PRIx64 " cc %15s\n",
- env->psw.mask, env->psw.addr, cc_name(env->cc_op));
+ qemu_fprintf(f, "PSW=mask %016" PRIx64 " addr %016" PRIx64,
+ s390_cpu_get_psw_mask(env), env->psw.addr);
+ if (!tcg_enabled()) {
+ qemu_fprintf(f, "\n");
+ } else if (env->cc_op > 3) {
+ qemu_fprintf(f, " cc %15s\n", cc_name(env->cc_op));
} else {
- qemu_fprintf(f, "PSW=mask %016" PRIx64 " addr %016" PRIx64 " cc %02x\n",
- env->psw.mask, env->psw.addr, env->cc_op);
+ qemu_fprintf(f, " cc %02x\n", env->cc_op);
}
for (i = 0; i < 16; i++) {
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 4/5] target/s390x: Use s390_cpu_{set_psw, get_psw_mask} in gdbstub
2021-06-15 3:07 [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals Richard Henderson
` (2 preceding siblings ...)
2021-06-15 3:07 ` [PATCH 3/5] target/s390x: Improve s390_cpu_dump_state vs cc_op Richard Henderson
@ 2021-06-15 3:07 ` Richard Henderson
2021-06-15 7:52 ` David Hildenbrand
2021-06-15 3:07 ` [PATCH 5/5] linux-user/s390x: Save and restore psw.mask properly Richard Henderson
` (4 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Richard Henderson @ 2021-06-15 3:07 UTC (permalink / raw)
To: qemu-devel; +Cc: ruixin.bao, jonathan.albrecht, cohuck, qemu-s390x, david
No change in behaviour, as gdbstub was correctly written to
install and extract the cc value.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/s390x/gdbstub.c | 15 ++-------------
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/target/s390x/gdbstub.c b/target/s390x/gdbstub.c
index d6fce5ff1e..5b4e38a13b 100644
--- a/target/s390x/gdbstub.c
+++ b/target/s390x/gdbstub.c
@@ -31,18 +31,10 @@ int s390_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
{
S390CPU *cpu = S390_CPU(cs);
CPUS390XState *env = &cpu->env;
- uint64_t val;
- int cc_op;
switch (n) {
case S390_PSWM_REGNUM:
- if (tcg_enabled()) {
- cc_op = calc_cc(env, env->cc_op, env->cc_src, env->cc_dst,
- env->cc_vr);
- val = deposit64(env->psw.mask, 44, 2, cc_op);
- return gdb_get_regl(mem_buf, val);
- }
- return gdb_get_regl(mem_buf, env->psw.mask);
+ return gdb_get_regl(mem_buf, s390_cpu_get_psw_mask(env));
case S390_PSWA_REGNUM:
return gdb_get_regl(mem_buf, env->psw.addr);
case S390_R0_REGNUM ... S390_R15_REGNUM:
@@ -59,10 +51,7 @@ int s390_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
switch (n) {
case S390_PSWM_REGNUM:
- env->psw.mask = tmpl;
- if (tcg_enabled()) {
- env->cc_op = extract64(tmpl, 44, 2);
- }
+ s390_cpu_set_psw(env, tmpl, env->psw.addr);
break;
case S390_PSWA_REGNUM:
env->psw.addr = tmpl;
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 5/5] linux-user/s390x: Save and restore psw.mask properly
2021-06-15 3:07 [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals Richard Henderson
` (3 preceding siblings ...)
2021-06-15 3:07 ` [PATCH 4/5] target/s390x: Use s390_cpu_{set_psw, get_psw_mask} in gdbstub Richard Henderson
@ 2021-06-15 3:07 ` Richard Henderson
2021-06-15 7:59 ` David Hildenbrand
2021-06-15 9:02 ` [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals Christian Borntraeger
` (3 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Richard Henderson @ 2021-06-15 3:07 UTC (permalink / raw)
To: qemu-devel; +Cc: ruixin.bao, jonathan.albrecht, cohuck, qemu-s390x, david
At present, we're referencing env->psw.mask directly, which
fails to ensure that env->cc_op is incorporated or updated.
Use s390_cpu_{set_psw,get_psw_mask} to fix this.
Mirror the kernel's cleaning of the psw.mask in save_sigregs
and restore_sigregs. Ignore PSW_MASK_RI for now, as qemu does
not support that.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/s390x/signal.c | 37 ++++++++++++++++++++++++++++++++-----
1 file changed, 32 insertions(+), 5 deletions(-)
diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index ef136dae33..bf8a8fbfe9 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -112,15 +112,23 @@ get_sigframe(struct target_sigaction *ka, CPUS390XState *env, size_t frame_size)
return (sp - frame_size) & -8ul;
}
+#define PSW_USER_BITS (PSW_MASK_DAT | PSW_MASK_IO | PSW_MASK_EXT | \
+ PSW_MASK_MCHECK | PSW_MASK_PSTATE | PSW_ASC_PRIMARY)
+#define PSW_MASK_USER (PSW_MASK_ASC | PSW_MASK_CC | PSW_MASK_PM | \
+ PSW_MASK_64 | PSW_MASK_32)
+
static void save_sigregs(CPUS390XState *env, target_sigregs *sregs)
{
+ uint64_t psw_mask = s390_cpu_get_psw_mask(env);
int i;
/*
* Copy a 'clean' PSW mask to the user to avoid leaking
* information about whether PER is currently on.
+ * TODO: qemu does not support PSW_MASK_RI; it will never be set.
*/
- __put_user(env->psw.mask, &sregs->regs.psw.mask);
+ psw_mask = PSW_USER_BITS | (psw_mask & PSW_MASK_USER);
+ __put_user(psw_mask, &sregs->regs.psw.mask);
__put_user(env->psw.addr, &sregs->regs.psw.addr);
for (i = 0; i < 16; i++) {
@@ -289,7 +297,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
static void restore_sigregs(CPUS390XState *env, target_sigregs *sc)
{
- target_ulong prev_addr;
+ uint64_t prev_addr, prev_mask, mask, addr;
int i;
for (i = 0; i < 16; i++) {
@@ -297,9 +305,28 @@ static void restore_sigregs(CPUS390XState *env, target_sigregs *sc)
}
prev_addr = env->psw.addr;
- __get_user(env->psw.mask, &sc->regs.psw.mask);
- __get_user(env->psw.addr, &sc->regs.psw.addr);
- trace_user_s390x_restore_sigregs(env, env->psw.addr, prev_addr);
+ __get_user(mask, &sc->regs.psw.mask);
+ __get_user(addr, &sc->regs.psw.addr);
+ trace_user_s390x_restore_sigregs(env, addr, prev_addr);
+
+ /*
+ * Use current psw.mask to preserve PER bit.
+ * TODO:
+ * if (!is_ri_task(current) && (user_sregs.regs.psw.mask & PSW_MASK_RI))
+ * return -EINVAL;
+ * Simply do not allow it to be set in mask.
+ */
+ prev_mask = s390_cpu_get_psw_mask(env);
+ mask = (prev_mask & ~PSW_MASK_USER) | (mask & PSW_MASK_USER);
+ /* Check for invalid user address space control. */
+ if ((mask & PSW_MASK_ASC) == PSW_ASC_HOME) {
+ mask = (mask & ~PSW_MASK_ASC) | PSW_ASC_PRIMARY;
+ }
+ /* Check for invalid amode. */
+ if (mask & PSW_MASK_64) {
+ mask |= PSW_MASK_32;
+ }
+ s390_cpu_set_psw(env, mask, addr);
for (i = 0; i < 16; i++) {
__get_user(env->aregs[i], &sc->regs.acrs[i]);
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 1/5] target/s390x: Expose load_psw and get_psw_mask to cpu.h
2021-06-15 3:07 ` [PATCH 1/5] target/s390x: Expose load_psw and get_psw_mask to cpu.h Richard Henderson
@ 2021-06-15 7:47 ` David Hildenbrand
2021-06-15 8:57 ` Alex Bennée
1 sibling, 0 replies; 23+ messages in thread
From: David Hildenbrand @ 2021-06-15 7:47 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: ruixin.bao, jonathan.albrecht, cohuck, qemu-s390x
On 15.06.21 05:07, Richard Henderson wrote:
> Rename to s390_cpu_set_psw and s390_cpu_get_psw_mask at the
> same time. Adjust so that they compile for user-only.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/5] target/s390x: Do not modify cpu state in s390_cpu_get_psw_mask
2021-06-15 3:07 ` [PATCH 2/5] target/s390x: Do not modify cpu state in s390_cpu_get_psw_mask Richard Henderson
@ 2021-06-15 7:49 ` David Hildenbrand
0 siblings, 0 replies; 23+ messages in thread
From: David Hildenbrand @ 2021-06-15 7:49 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: ruixin.bao, jonathan.albrecht, cohuck, qemu-s390x
On 15.06.21 05:07, Richard Henderson wrote:
> We want to use this function for debugging, and debug should
> not modify cpu state (even non-architectural cpu state) lest
> we introduce heisenbugs.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/s390x/helper.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/target/s390x/helper.c b/target/s390x/helper.c
> index d311903b94..559fc3573f 100644
> --- a/target/s390x/helper.c
> +++ b/target/s390x/helper.c
> @@ -321,12 +321,12 @@ uint64_t s390_cpu_get_psw_mask(CPUS390XState *env)
> uint64_t r = env->psw.mask;
>
> if (tcg_enabled()) {
> - env->cc_op = calc_cc(env, env->cc_op, env->cc_src, env->cc_dst,
> - env->cc_vr);
> + uint64_t cc = calc_cc(env, env->cc_op, env->cc_src,
> + env->cc_dst, env->cc_vr);
>
> + assert(cc <= 3);
> r &= ~PSW_MASK_CC;
> - assert(!(env->cc_op & ~3));
> - r |= (uint64_t)env->cc_op << 44;
> + r |= cc << 44;
> }
>
> return r;
>
I wonder if this can actually suboptimal performance-wise, because we
might be recomputing the same thing. However, I guess it's rather a
corner case
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 3/5] target/s390x: Improve s390_cpu_dump_state vs cc_op
2021-06-15 3:07 ` [PATCH 3/5] target/s390x: Improve s390_cpu_dump_state vs cc_op Richard Henderson
@ 2021-06-15 7:51 ` David Hildenbrand
0 siblings, 0 replies; 23+ messages in thread
From: David Hildenbrand @ 2021-06-15 7:51 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: ruixin.bao, jonathan.albrecht, cohuck, qemu-s390x
On 15.06.21 05:07, Richard Henderson wrote:
> Use s390_cpu_get_psw_mask so that we print the correct
> architectural value of psw.mask. Do not print cc_op
> unless tcg_enabled.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/s390x/helper.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/target/s390x/helper.c b/target/s390x/helper.c
> index 559fc3573f..1445b74451 100644
> --- a/target/s390x/helper.c
> +++ b/target/s390x/helper.c
> @@ -338,12 +338,14 @@ void s390_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> CPUS390XState *env = &cpu->env;
> int i;
>
> - if (env->cc_op > 3) {
> - qemu_fprintf(f, "PSW=mask %016" PRIx64 " addr %016" PRIx64 " cc %15s\n",
> - env->psw.mask, env->psw.addr, cc_name(env->cc_op));
> + qemu_fprintf(f, "PSW=mask %016" PRIx64 " addr %016" PRIx64,
> + s390_cpu_get_psw_mask(env), env->psw.addr);
> + if (!tcg_enabled()) {
> + qemu_fprintf(f, "\n");
> + } else if (env->cc_op > 3) {
> + qemu_fprintf(f, " cc %15s\n", cc_name(env->cc_op));
> } else {
> - qemu_fprintf(f, "PSW=mask %016" PRIx64 " addr %016" PRIx64 " cc %02x\n",
> - env->psw.mask, env->psw.addr, env->cc_op);
> + qemu_fprintf(f, " cc %02x\n", env->cc_op);
> }
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/5] target/s390x: Use s390_cpu_{set_psw, get_psw_mask} in gdbstub
2021-06-15 3:07 ` [PATCH 4/5] target/s390x: Use s390_cpu_{set_psw, get_psw_mask} in gdbstub Richard Henderson
@ 2021-06-15 7:52 ` David Hildenbrand
0 siblings, 0 replies; 23+ messages in thread
From: David Hildenbrand @ 2021-06-15 7:52 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: ruixin.bao, jonathan.albrecht, cohuck, qemu-s390x
On 15.06.21 05:07, Richard Henderson wrote:
> No change in behaviour, as gdbstub was correctly written to
> install and extract the cc value.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/s390x/gdbstub.c | 15 ++-------------
> 1 file changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/target/s390x/gdbstub.c b/target/s390x/gdbstub.c
> index d6fce5ff1e..5b4e38a13b 100644
> --- a/target/s390x/gdbstub.c
> +++ b/target/s390x/gdbstub.c
> @@ -31,18 +31,10 @@ int s390_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
> {
> S390CPU *cpu = S390_CPU(cs);
> CPUS390XState *env = &cpu->env;
> - uint64_t val;
> - int cc_op;
>
> switch (n) {
> case S390_PSWM_REGNUM:
> - if (tcg_enabled()) {
> - cc_op = calc_cc(env, env->cc_op, env->cc_src, env->cc_dst,
> - env->cc_vr);
> - val = deposit64(env->psw.mask, 44, 2, cc_op);
> - return gdb_get_regl(mem_buf, val);
> - }
> - return gdb_get_regl(mem_buf, env->psw.mask);
> + return gdb_get_regl(mem_buf, s390_cpu_get_psw_mask(env));
> case S390_PSWA_REGNUM:
> return gdb_get_regl(mem_buf, env->psw.addr);
> case S390_R0_REGNUM ... S390_R15_REGNUM:
> @@ -59,10 +51,7 @@ int s390_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
>
> switch (n) {
> case S390_PSWM_REGNUM:
> - env->psw.mask = tmpl;
> - if (tcg_enabled()) {
> - env->cc_op = extract64(tmpl, 44, 2);
> - }
> + s390_cpu_set_psw(env, tmpl, env->psw.addr);
> break;
> case S390_PSWA_REGNUM:
> env->psw.addr = tmpl;
>
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 5/5] linux-user/s390x: Save and restore psw.mask properly
2021-06-15 3:07 ` [PATCH 5/5] linux-user/s390x: Save and restore psw.mask properly Richard Henderson
@ 2021-06-15 7:59 ` David Hildenbrand
0 siblings, 0 replies; 23+ messages in thread
From: David Hildenbrand @ 2021-06-15 7:59 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: ruixin.bao, jonathan.albrecht, cohuck, qemu-s390x
On 15.06.21 05:07, Richard Henderson wrote:
> At present, we're referencing env->psw.mask directly, which
> fails to ensure that env->cc_op is incorporated or updated.
> Use s390_cpu_{set_psw,get_psw_mask} to fix this.
>
> Mirror the kernel's cleaning of the psw.mask in save_sigregs
> and restore_sigregs. Ignore PSW_MASK_RI for now, as qemu does
> not support that.
s/qemu/qemu\/tcg/ ? because qemu/kvm supports it .
Maybe reference the bug reports this fixes?
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> linux-user/s390x/signal.c | 37 ++++++++++++++++++++++++++++++++-----
> 1 file changed, 32 insertions(+), 5 deletions(-)
>
> diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
> index ef136dae33..bf8a8fbfe9 100644
> --- a/linux-user/s390x/signal.c
> +++ b/linux-user/s390x/signal.c
> @@ -112,15 +112,23 @@ get_sigframe(struct target_sigaction *ka, CPUS390XState *env, size_t frame_size)
> return (sp - frame_size) & -8ul;
> }
>
> +#define PSW_USER_BITS (PSW_MASK_DAT | PSW_MASK_IO | PSW_MASK_EXT | \
> + PSW_MASK_MCHECK | PSW_MASK_PSTATE | PSW_ASC_PRIMARY)
> +#define PSW_MASK_USER (PSW_MASK_ASC | PSW_MASK_CC | PSW_MASK_PM | \
> + PSW_MASK_64 | PSW_MASK_32)
> +
> static void save_sigregs(CPUS390XState *env, target_sigregs *sregs)
> {
> + uint64_t psw_mask = s390_cpu_get_psw_mask(env);
> int i;
>
> /*
> * Copy a 'clean' PSW mask to the user to avoid leaking
> * information about whether PER is currently on.
> + * TODO: qemu does not support PSW_MASK_RI; it will never be set.
> */
> - __put_user(env->psw.mask, &sregs->regs.psw.mask);
> + psw_mask = PSW_USER_BITS | (psw_mask & PSW_MASK_USER);
> + __put_user(psw_mask, &sregs->regs.psw.mask);
> __put_user(env->psw.addr, &sregs->regs.psw.addr);
>
> for (i = 0; i < 16; i++) {
> @@ -289,7 +297,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
>
> static void restore_sigregs(CPUS390XState *env, target_sigregs *sc)
> {
> - target_ulong prev_addr;
> + uint64_t prev_addr, prev_mask, mask, addr;
> int i;
>
> for (i = 0; i < 16; i++) {
> @@ -297,9 +305,28 @@ static void restore_sigregs(CPUS390XState *env, target_sigregs *sc)
> }
>
> prev_addr = env->psw.addr;
> - __get_user(env->psw.mask, &sc->regs.psw.mask);
> - __get_user(env->psw.addr, &sc->regs.psw.addr);
> - trace_user_s390x_restore_sigregs(env, env->psw.addr, prev_addr);
> + __get_user(mask, &sc->regs.psw.mask);
> + __get_user(addr, &sc->regs.psw.addr);
> + trace_user_s390x_restore_sigregs(env, addr, prev_addr);
> +
> + /*
> + * Use current psw.mask to preserve PER bit.
> + * TODO:
> + * if (!is_ri_task(current) && (user_sregs.regs.psw.mask & PSW_MASK_RI))
> + * return -EINVAL;
> + * Simply do not allow it to be set in mask.
> + */
> + prev_mask = s390_cpu_get_psw_mask(env);
> + mask = (prev_mask & ~PSW_MASK_USER) | (mask & PSW_MASK_USER);
> + /* Check for invalid user address space control. */
> + if ((mask & PSW_MASK_ASC) == PSW_ASC_HOME) {
> + mask = (mask & ~PSW_MASK_ASC) | PSW_ASC_PRIMARY;
> + }
> + /* Check for invalid amode. */
> + if (mask & PSW_MASK_64) {
> + mask |= PSW_MASK_32;
> + }
> + s390_cpu_set_psw(env, mask, addr);
>
> for (i = 0; i < 16; i++) {
> __get_user(env->aregs[i], &sc->regs.acrs[i]);
>
LGTM
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/5] target/s390x: Expose load_psw and get_psw_mask to cpu.h
2021-06-15 3:07 ` [PATCH 1/5] target/s390x: Expose load_psw and get_psw_mask to cpu.h Richard Henderson
2021-06-15 7:47 ` David Hildenbrand
@ 2021-06-15 8:57 ` Alex Bennée
1 sibling, 0 replies; 23+ messages in thread
From: Alex Bennée @ 2021-06-15 8:57 UTC (permalink / raw)
To: Richard Henderson
Cc: ruixin.bao, jonathan.albrecht, david, cohuck, qemu-devel, qemu-s390x
Richard Henderson <richard.henderson@linaro.org> writes:
> Rename to s390_cpu_set_psw and s390_cpu_get_psw_mask at the
> same time. Adjust so that they compile for user-only.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals
2021-06-15 3:07 [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals Richard Henderson
` (4 preceding siblings ...)
2021-06-15 3:07 ` [PATCH 5/5] linux-user/s390x: Save and restore psw.mask properly Richard Henderson
@ 2021-06-15 9:02 ` Christian Borntraeger
2021-06-16 15:00 ` Cornelia Huck
2021-06-15 9:27 ` Alex Bennée
` (2 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Christian Borntraeger @ 2021-06-15 9:02 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: ruixin.bao, jonathan.albrecht, cohuck, qemu-s390x, david
On 15.06.21 05:07, Richard Henderson wrote:
> The PSW_MASK_CC component of psw.mask was not handled properly
> in the creation or restoration of signal frames.
>
Maybe add a Reported-by: jonathan.albrecht@linux.vnet.ibm.com
in the right patches?
> Richard Henderson (5):
> target/s390x: Expose load_psw and get_psw_mask to cpu.h
> target/s390x: Do not modify cpu state in s390_cpu_get_psw_mask
> target/s390x: Improve s390_cpu_dump_state vs cc_op
> target/s390x: Use s390_cpu_{set_psw,get_psw_mask} in gdbstub
> linux-user/s390x: Save and restore psw.mask properly
>
> target/s390x/cpu.h | 3 ++
> target/s390x/internal.h | 5 --
> linux-user/s390x/signal.c | 37 ++++++++++++--
> target/s390x/cc_helper.c | 2 +-
> target/s390x/excp_helper.c | 28 +++++-----
> target/s390x/gdbstub.c | 15 +-----
> target/s390x/helper.c | 101 ++++++++++++++++++++-----------------
> target/s390x/sigp.c | 3 +-
> 8 files changed, 110 insertions(+), 84 deletions(-)
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals
2021-06-15 3:07 [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals Richard Henderson
` (5 preceding siblings ...)
2021-06-15 9:02 ` [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals Christian Borntraeger
@ 2021-06-15 9:27 ` Alex Bennée
2021-06-15 9:51 ` Alex Bennée
2021-06-16 10:38 ` Cornelia Huck
2021-06-15 18:03 ` jonathan.albrecht
2021-06-17 8:33 ` Cornelia Huck
8 siblings, 2 replies; 23+ messages in thread
From: Alex Bennée @ 2021-06-15 9:27 UTC (permalink / raw)
To: Richard Henderson
Cc: ruixin.bao, jonathan.albrecht, david, cohuck, qemu-devel, qemu-s390x
Richard Henderson <richard.henderson@linaro.org> writes:
> The PSW_MASK_CC component of psw.mask was not handled properly
> in the creation or restoration of signal frames.
Still seeing issues running on s390x machine:
05:00:29 [ajb@qemu01:~/l/q/b/debug] s390x/signal-fixes|… 38 + retry.py -n 100 -c -- ./qemu-s390x ./tests/tcg/s390x-linux-user/signals
...
...
Results summary:
0: 62 times (62.00%), avg time 2.253 (0.00 varience/0.00 deviation)
-11: 38 times (38.00%), avg time 0.251 (0.00 varience/0.00 deviation)
Ran command 100 times, 62 passes
I don't get much from the backtrace, maybe the atomic triggered the seg?
#0 0x0000000001016244 in vfprintf ()
[Current thread is 1 (Thread 0x4001001910 (LWP 27308))]
(gdb) bt
#0 0x0000000001016244 in vfprintf ()
#1 0x000000000101d484 in printf ()
#2 0x0000000001000b2e in background_thread_func (arg=0x10a3620) at /home/ajb/lsrc/qemu.git/tests/tcg/multiarch/signals.c:65
#3 0x0000000001003150 in start_thread (arg=0x4001001910) at pthread_create.c:463
#4 0x0000000001035b40 in thread_start ()
(gdb) frame 2
#2 0x0000000001000b2e in background_thread_func (arg=0x10a3620) at /home/ajb/lsrc/qemu.git/tests/tcg/multiarch/signals.c:65
65 printf("thread%d: started\n", job->number);
(gdb) info locals
job = 0x10a3620
(gdb) p job
$1 = (ThreadJob *) 0x10a3620
(gdb) p job->number
$2 = 0
(gdb) frame 0
#0 0x0000000001016244 in vfprintf ()
(gdb) x/5i $pc
=> 0x1016244 <vfprintf+1316>: asi 224(%r11),1
0x101624a <vfprintf+1322>: cgijne %r1,0,0x1017570 <vfprintf+6224>
0x1016250 <vfprintf+1328>: lg %r1,336(%r11)
0x1016256 <vfprintf+1334>: lghi %r3,37
0x101625a <vfprintf+1338>: aghik %r6,%r1,1
(gdb) p/x $r11
$3 = 0x4001000708
(gdb) p/x $r11 + 224
$4 = 0x40010007e8
(gdb) x/1g $4
0x40010007e8: 0x0000000000000000
(gdb)
However running on x86 backend everything seems to be fine.
Results summary:
0: 200 times (100.00%), avg time 2.255 (0.00 varience/0.00 deviation)
Ran command 200 times, 200 passes
>
>
> r~
>
>
> Richard Henderson (5):
> target/s390x: Expose load_psw and get_psw_mask to cpu.h
> target/s390x: Do not modify cpu state in s390_cpu_get_psw_mask
> target/s390x: Improve s390_cpu_dump_state vs cc_op
> target/s390x: Use s390_cpu_{set_psw,get_psw_mask} in gdbstub
> linux-user/s390x: Save and restore psw.mask properly
>
> target/s390x/cpu.h | 3 ++
> target/s390x/internal.h | 5 --
> linux-user/s390x/signal.c | 37 ++++++++++++--
> target/s390x/cc_helper.c | 2 +-
> target/s390x/excp_helper.c | 28 +++++-----
> target/s390x/gdbstub.c | 15 +-----
> target/s390x/helper.c | 101 ++++++++++++++++++++-----------------
> target/s390x/sigp.c | 3 +-
> 8 files changed, 110 insertions(+), 84 deletions(-)
--
Alex Bennée
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals
2021-06-15 9:27 ` Alex Bennée
@ 2021-06-15 9:51 ` Alex Bennée
2021-06-16 10:38 ` Cornelia Huck
1 sibling, 0 replies; 23+ messages in thread
From: Alex Bennée @ 2021-06-15 9:51 UTC (permalink / raw)
To: Richard Henderson
Cc: ruixin.bao, jonathan.albrecht, david, cohuck, qemu-devel, qemu-s390x
Alex Bennée <alex.bennee@linaro.org> writes:
> Richard Henderson <richard.henderson@linaro.org> writes:
>
>> The PSW_MASK_CC component of psw.mask was not handled properly
>> in the creation or restoration of signal frames.
>
> Still seeing issues running on s390x machine:
>
> 05:00:29 [ajb@qemu01:~/l/q/b/debug] s390x/signal-fixes|… 38 + retry.py -n 100 -c -- ./qemu-s390x ./tests/tcg/s390x-linux-user/signals
> ...
> ...
> Results summary:
> 0: 62 times (62.00%), avg time 2.253 (0.00 varience/0.00 deviation)
> -11: 38 times (38.00%), avg time 0.251 (0.00 varience/0.00 deviation)
> Ran command 100 times, 62 passes
>
> I don't get much from the backtrace, maybe the atomic triggered the seg?
>
> #0 0x0000000001016244 in vfprintf ()
> [Current thread is 1 (Thread 0x4001001910 (LWP 27308))]
> (gdb) bt
> #0 0x0000000001016244 in vfprintf ()
> #1 0x000000000101d484 in printf ()
> #2 0x0000000001000b2e in background_thread_func (arg=0x10a3620) at /home/ajb/lsrc/qemu.git/tests/tcg/multiarch/signals.c:65
> #3 0x0000000001003150 in start_thread (arg=0x4001001910) at pthread_create.c:463
> #4 0x0000000001035b40 in thread_start ()
> (gdb) frame 2
> #2 0x0000000001000b2e in background_thread_func (arg=0x10a3620) at /home/ajb/lsrc/qemu.git/tests/tcg/multiarch/signals.c:65
> 65 printf("thread%d: started\n", job->number);
> (gdb) info locals
> job = 0x10a3620
> (gdb) p job
> $1 = (ThreadJob *) 0x10a3620
> (gdb) p job->number
> $2 = 0
> (gdb) frame 0
> #0 0x0000000001016244 in vfprintf ()
> (gdb) x/5i $pc
> => 0x1016244 <vfprintf+1316>: asi 224(%r11),1
> 0x101624a <vfprintf+1322>: cgijne %r1,0,0x1017570 <vfprintf+6224>
> 0x1016250 <vfprintf+1328>: lg %r1,336(%r11)
> 0x1016256 <vfprintf+1334>: lghi %r3,37
> 0x101625a <vfprintf+1338>: aghik %r6,%r1,1
> (gdb) p/x $r11
> $3 = 0x4001000708
> (gdb) p/x $r11 + 224
> $4 = 0x40010007e8
> (gdb) x/1g $4
> 0x40010007e8: 0x0000000000000000
> (gdb)
>
> However running on x86 backend everything seems to be fine.
>
> Results summary:
> 0: 200 times (100.00%), avg time 2.255 (0.00 varience/0.00 deviation)
> Ran command 200 times, 200 passes
It's hard to desegregate the SEGVs we get during normal runs but a pass
followed by a fail shows:
qemu: SIGSEGV pc=0x3fff473bb1e address=40007ff000 w=1 oldset=0x00000000
qemu: SIGSEGV pc=0x3fff473bb1e address=40007ff000 w=1 oldset=0x00000000
qemu: SIGSEGV pc=0x3fff473bb1e address=40007ff000 w=1 oldset=0x00000000
qemu: SIGSEGV pc=0x3fff473bb1e address=40007ff000 w=1 oldset=0x00000000
shutting down after: 2000 signals
qemu: SIGSEGV pc=0x3fff484bd1e address=40007ff000 w=1 oldset=0x00000000
qemu: SIGSEGV pc=0x3fff4848f1a address=40007ff000 w=1 oldset=0x00000000
qemu: SIGSEGV pc=0x3fff47477f2 address=40007ff000 w=1 oldset=0x00000000
[Thread 0x3fffdff1780 (LWP 32928) exited]
[Inferior 1 (process 32928) exited normally]
(gdb) r
Starting program: /home/ajb/lsrc/qemu.git/builds/debug/qemu-s390x ./tests/tcg/s390x-linux-user/signals
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/s390x-linux-gnu/libthread_db.so.1".
[New Thread 0x3fffceff910 (LWP 32964)]
qemu: SIGSEGV pc=0x3fff4752f2a address=40007ff000 w=1 oldset=0x00000000
qemu: SIGSEGV pc=0x3fff471fa1e address=40007ff000 w=1 oldset=0x00000000
qemu: SIGSEGV pc=0x3fff475c49c address=40007ff000 w=1 oldset=0x00000000
[New Thread 0x3fffdff0910 (LWP 32965)]
qemu: SIGSEGV pc=0x3fff4703b18 address=40007ff000 w=1 oldset=0x00000000
qemu: SIGSEGV pc=0x3fffd812efe address=4001000000 w=1 oldset=0x00000000
qemu: SIGSEGV pc=0x3fff471081e address=40007ff000 w=1 oldset=0x00000000
qemu: SIGSEGV pc=0x3fff4715ee6 address=40007ff000 w=1 oldset=0x00000000
qemu: SIGSEGV pc=0x3fff471a02a address=40007ff000 w=1 oldset=0x00000000
qemu: SIGSEGV pc=0x3fff472491e address=40007ff000 w=1 oldset=0x00000000
qemu: SIGSEGV pc=0x3fff4725d1e address=4001000000 w=1 oldset=0x00000000
qemu: SIGSEGV pc=0x3fff4730536 address=4001000000 w=0 oldset=0x00000000
qemu: SIGSEGV pc=0x3fff473171e address=40007ff000 w=1 oldset=0x00000000
[Thread 0x3fffdff0910 (LWP 32965) exited]
[Thread 0x3fffceff910 (LWP 32964) exited]
Program terminated with signal SIGSEGV, Segmentation fault.
The program no longer exists.
(gdb)
So something is going astray to 4001000000 which otherwise doesn't.
>
>>
>>
>> r~
>>
>>
>> Richard Henderson (5):
>> target/s390x: Expose load_psw and get_psw_mask to cpu.h
>> target/s390x: Do not modify cpu state in s390_cpu_get_psw_mask
>> target/s390x: Improve s390_cpu_dump_state vs cc_op
>> target/s390x: Use s390_cpu_{set_psw,get_psw_mask} in gdbstub
>> linux-user/s390x: Save and restore psw.mask properly
>>
>> target/s390x/cpu.h | 3 ++
>> target/s390x/internal.h | 5 --
>> linux-user/s390x/signal.c | 37 ++++++++++++--
>> target/s390x/cc_helper.c | 2 +-
>> target/s390x/excp_helper.c | 28 +++++-----
>> target/s390x/gdbstub.c | 15 +-----
>> target/s390x/helper.c | 101 ++++++++++++++++++++-----------------
>> target/s390x/sigp.c | 3 +-
>> 8 files changed, 110 insertions(+), 84 deletions(-)
--
Alex Bennée
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals
2021-06-15 3:07 [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals Richard Henderson
` (6 preceding siblings ...)
2021-06-15 9:27 ` Alex Bennée
@ 2021-06-15 18:03 ` jonathan.albrecht
2021-06-16 15:01 ` Cornelia Huck
2021-06-17 8:33 ` Cornelia Huck
8 siblings, 1 reply; 23+ messages in thread
From: jonathan.albrecht @ 2021-06-15 18:03 UTC (permalink / raw)
To: Richard Henderson
Cc: ruixin.bao, david, cohuck, qemu-s390x, qemu-devel, qemu-s390x
On 2021-06-14 11:07 pm, Richard Henderson wrote:
> The PSW_MASK_CC component of psw.mask was not handled properly
> in the creation or restoration of signal frames.
>
Thanks Richard! Peter and I tested this series against:
* https://bugs.launchpad.net/qemu/+bug/1886793
* https://bugs.launchpad.net/qemu/+bug/1893040
and they look fixed now.
Appreciate your time on this,
Jon
>
> r~
>
>
> Richard Henderson (5):
> target/s390x: Expose load_psw and get_psw_mask to cpu.h
> target/s390x: Do not modify cpu state in s390_cpu_get_psw_mask
> target/s390x: Improve s390_cpu_dump_state vs cc_op
> target/s390x: Use s390_cpu_{set_psw,get_psw_mask} in gdbstub
> linux-user/s390x: Save and restore psw.mask properly
>
> target/s390x/cpu.h | 3 ++
> target/s390x/internal.h | 5 --
> linux-user/s390x/signal.c | 37 ++++++++++++--
> target/s390x/cc_helper.c | 2 +-
> target/s390x/excp_helper.c | 28 +++++-----
> target/s390x/gdbstub.c | 15 +-----
> target/s390x/helper.c | 101 ++++++++++++++++++++-----------------
> target/s390x/sigp.c | 3 +-
> 8 files changed, 110 insertions(+), 84 deletions(-)
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals
2021-06-15 9:27 ` Alex Bennée
2021-06-15 9:51 ` Alex Bennée
@ 2021-06-16 10:38 ` Cornelia Huck
2021-06-16 14:48 ` Richard Henderson
1 sibling, 1 reply; 23+ messages in thread
From: Cornelia Huck @ 2021-06-16 10:38 UTC (permalink / raw)
To: Alex Bennée, Richard Henderson
Cc: ruixin.bao, qemu-s390x, jonathan.albrecht, qemu-devel, david
On Tue, Jun 15 2021, Alex Bennée <alex.bennee@linaro.org> wrote:
> Richard Henderson <richard.henderson@linaro.org> writes:
>
>> The PSW_MASK_CC component of psw.mask was not handled properly
>> in the creation or restoration of signal frames.
>
> Still seeing issues running on s390x machine:
(...)
> However running on x86 backend everything seems to be fine.
(...)
So, in summary, this improves things, although there are some remaining
problems?
Asking because I'm looking to queue this.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals
2021-06-16 10:38 ` Cornelia Huck
@ 2021-06-16 14:48 ` Richard Henderson
2021-06-16 14:59 ` Cornelia Huck
0 siblings, 1 reply; 23+ messages in thread
From: Richard Henderson @ 2021-06-16 14:48 UTC (permalink / raw)
To: Cornelia Huck, Alex Bennée
Cc: ruixin.bao, qemu-s390x, jonathan.albrecht, qemu-devel, david
On 6/16/21 3:38 AM, Cornelia Huck wrote:
> On Tue, Jun 15 2021, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>> Richard Henderson <richard.henderson@linaro.org> writes:
>>
>>> The PSW_MASK_CC component of psw.mask was not handled properly
>>> in the creation or restoration of signal frames.
>>
>> Still seeing issues running on s390x machine:
>
> (...)
>
>> However running on x86 backend everything seems to be fine.
>
> (...)
>
> So, in summary, this improves things, although there are some remaining
> problems?
>
> Asking because I'm looking to queue this.
Alex is seeing something that I believe is unrelated.
And also, I have a patch set out for that as well. :-)
r~
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals
2021-06-16 14:48 ` Richard Henderson
@ 2021-06-16 14:59 ` Cornelia Huck
0 siblings, 0 replies; 23+ messages in thread
From: Cornelia Huck @ 2021-06-16 14:59 UTC (permalink / raw)
To: Richard Henderson, Alex Bennée
Cc: ruixin.bao, qemu-s390x, david, jonathan.albrecht, qemu-devel
On Wed, Jun 16 2021, Richard Henderson <richard.henderson@linaro.org> wrote:
> On 6/16/21 3:38 AM, Cornelia Huck wrote:
>> On Tue, Jun 15 2021, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>>> Richard Henderson <richard.henderson@linaro.org> writes:
>>>
>>>> The PSW_MASK_CC component of psw.mask was not handled properly
>>>> in the creation or restoration of signal frames.
>>>
>>> Still seeing issues running on s390x machine:
>>
>> (...)
>>
>>> However running on x86 backend everything seems to be fine.
>>
>> (...)
>>
>> So, in summary, this improves things, although there are some remaining
>> problems?
>>
>> Asking because I'm looking to queue this.
>
> Alex is seeing something that I believe is unrelated.
> And also, I have a patch set out for that as well. :-)
Good to hear :)
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals
2021-06-15 9:02 ` [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals Christian Borntraeger
@ 2021-06-16 15:00 ` Cornelia Huck
0 siblings, 0 replies; 23+ messages in thread
From: Cornelia Huck @ 2021-06-16 15:00 UTC (permalink / raw)
To: Christian Borntraeger, Richard Henderson, qemu-devel
Cc: ruixin.bao, jonathan.albrecht, qemu-s390x, david
On Tue, Jun 15 2021, Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> On 15.06.21 05:07, Richard Henderson wrote:
>> The PSW_MASK_CC component of psw.mask was not handled properly
>> in the creation or restoration of signal frames.
>>
>
>
> Maybe add a Reported-by: jonathan.albrecht@linux.vnet.ibm.com
> in the right patches?
Let me know to which ones, and I will add them.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals
2021-06-15 18:03 ` jonathan.albrecht
@ 2021-06-16 15:01 ` Cornelia Huck
2021-06-16 15:22 ` jonathan.albrecht
0 siblings, 1 reply; 23+ messages in thread
From: Cornelia Huck @ 2021-06-16 15:01 UTC (permalink / raw)
To: jonathan.albrecht, Richard Henderson
Cc: ruixin.bao, qemu-s390x, qemu-devel, qemu-s390x, david
On Tue, Jun 15 2021, "jonathan.albrecht" <jonathan.albrecht@linux.vnet.ibm.com> wrote:
> On 2021-06-14 11:07 pm, Richard Henderson wrote:
>> The PSW_MASK_CC component of psw.mask was not handled properly
>> in the creation or restoration of signal frames.
>>
>
> Thanks Richard! Peter and I tested this series against:
> * https://bugs.launchpad.net/qemu/+bug/1886793
> * https://bugs.launchpad.net/qemu/+bug/1893040
> and they look fixed now.
May I add some Tested-by: tags for this? (I don't know who Peter is.)
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals
2021-06-16 15:01 ` Cornelia Huck
@ 2021-06-16 15:22 ` jonathan.albrecht
0 siblings, 0 replies; 23+ messages in thread
From: jonathan.albrecht @ 2021-06-16 15:22 UTC (permalink / raw)
To: Cornelia Huck
Cc: ruixin.bao, david, Richard Henderson, qemu-devel, qemu-s390x, qemu-s390x
On 2021-06-16 11:01 am, Cornelia Huck wrote:
> On Tue, Jun 15 2021, "jonathan.albrecht"
> <jonathan.albrecht@linux.vnet.ibm.com> wrote:
>
>> On 2021-06-14 11:07 pm, Richard Henderson wrote:
>>> The PSW_MASK_CC component of psw.mask was not handled properly
>>> in the creation or restoration of signal frames.
>>>
>>
>> Thanks Richard! Peter and I tested this series against:
>> * https://bugs.launchpad.net/qemu/+bug/1886793
>> * https://bugs.launchpad.net/qemu/+bug/1893040
>> and they look fixed now.
>
> May I add some Tested-by: tags for this? (I don't know who Peter is.)
Yes, thanks. Peter is ruixin.bao@ibm.com
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals
2021-06-15 3:07 [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals Richard Henderson
` (7 preceding siblings ...)
2021-06-15 18:03 ` jonathan.albrecht
@ 2021-06-17 8:33 ` Cornelia Huck
8 siblings, 0 replies; 23+ messages in thread
From: Cornelia Huck @ 2021-06-17 8:33 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: ruixin.bao, jonathan.albrecht, qemu-s390x, david
On Mon, Jun 14 2021, Richard Henderson <richard.henderson@linaro.org> wrote:
> The PSW_MASK_CC component of psw.mask was not handled properly
> in the creation or restoration of signal frames.
>
>
> r~
>
>
> Richard Henderson (5):
> target/s390x: Expose load_psw and get_psw_mask to cpu.h
> target/s390x: Do not modify cpu state in s390_cpu_get_psw_mask
> target/s390x: Improve s390_cpu_dump_state vs cc_op
> target/s390x: Use s390_cpu_{set_psw,get_psw_mask} in gdbstub
> linux-user/s390x: Save and restore psw.mask properly
>
> target/s390x/cpu.h | 3 ++
> target/s390x/internal.h | 5 --
> linux-user/s390x/signal.c | 37 ++++++++++++--
> target/s390x/cc_helper.c | 2 +-
> target/s390x/excp_helper.c | 28 +++++-----
> target/s390x/gdbstub.c | 15 +-----
> target/s390x/helper.c | 101 ++++++++++++++++++++-----------------
> target/s390x/sigp.c | 3 +-
> 8 files changed, 110 insertions(+), 84 deletions(-)
Thanks, applied.
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2021-06-17 8:35 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-15 3:07 [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals Richard Henderson
2021-06-15 3:07 ` [PATCH 1/5] target/s390x: Expose load_psw and get_psw_mask to cpu.h Richard Henderson
2021-06-15 7:47 ` David Hildenbrand
2021-06-15 8:57 ` Alex Bennée
2021-06-15 3:07 ` [PATCH 2/5] target/s390x: Do not modify cpu state in s390_cpu_get_psw_mask Richard Henderson
2021-06-15 7:49 ` David Hildenbrand
2021-06-15 3:07 ` [PATCH 3/5] target/s390x: Improve s390_cpu_dump_state vs cc_op Richard Henderson
2021-06-15 7:51 ` David Hildenbrand
2021-06-15 3:07 ` [PATCH 4/5] target/s390x: Use s390_cpu_{set_psw, get_psw_mask} in gdbstub Richard Henderson
2021-06-15 7:52 ` David Hildenbrand
2021-06-15 3:07 ` [PATCH 5/5] linux-user/s390x: Save and restore psw.mask properly Richard Henderson
2021-06-15 7:59 ` David Hildenbrand
2021-06-15 9:02 ` [PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals Christian Borntraeger
2021-06-16 15:00 ` Cornelia Huck
2021-06-15 9:27 ` Alex Bennée
2021-06-15 9:51 ` Alex Bennée
2021-06-16 10:38 ` Cornelia Huck
2021-06-16 14:48 ` Richard Henderson
2021-06-16 14:59 ` Cornelia Huck
2021-06-15 18:03 ` jonathan.albrecht
2021-06-16 15:01 ` Cornelia Huck
2021-06-16 15:22 ` jonathan.albrecht
2021-06-17 8:33 ` Cornelia Huck
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.