| Message ID | 20230314110022.184717-2-iii@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/s390x: Implement Early Exception Recognition | expand |
On 3/14/23 04:00, Ilya Leoshkevich wrote: > Generate specification exception if a reserved bit is set in the PSW > mask or if the PSW address is out of bounds dictated by the addresing > mode. > > Reported-by: Nina Schoetterl-Glausch<nsg@linux.ibm.com> > Signed-off-by: Ilya Leoshkevich<iii@linux.ibm.com> > --- > target/s390x/cpu.c | 26 ++++++++++++++++++++++++++ > target/s390x/cpu.h | 1 + > target/s390x/tcg/excp_helper.c | 3 ++- > 3 files changed, 29 insertions(+), 1 deletion(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 14.03.23 12:00, Ilya Leoshkevich wrote: > Generate specification exception if a reserved bit is set in the PSW > mask or if the PSW address is out of bounds dictated by the addresing > mode. > > Reported-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com> Unofficially known to be broken (and ignored) for a long time :D > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > --- > target/s390x/cpu.c | 26 ++++++++++++++++++++++++++ > target/s390x/cpu.h | 1 + > target/s390x/tcg/excp_helper.c | 3 ++- > 3 files changed, 29 insertions(+), 1 deletion(-) > > diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c > index b10a8541ff8..8e6e46aa3d8 100644 > --- a/target/s390x/cpu.c > +++ b/target/s390x/cpu.c > @@ -41,6 +41,26 @@ > #define CR0_RESET 0xE0UL > #define CR14_RESET 0xC2000000UL; > > +#ifndef CONFIG_USER_ONLY > +static bool is_early_exception_recognized(uint64_t mask, uint64_t addr) Nit: I'd call this is_early_exception_psw() or sth like that. Thanks! Reviewed-by: David Hildenbrand <david@redhat.com>
On Tue, 2023-03-14 at 12:00 +0100, Ilya Leoshkevich wrote: > Generate specification exception if a reserved bit is set in the PSW Generate a ... > mask or if the PSW address is out of bounds dictated by the addresing addresSing > mode. Does this approach also work with SET SYSTEM MASK and STORE THEN OR SYSTEM MASK, i.e. do these call s390_cpu_set_psw via some code path I'm not seeing? In any case, you don't seem to implement their special requirements with regards to the ilen, so you should at least mention that. > > Reported-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > --- > target/s390x/cpu.c | 26 ++++++++++++++++++++++++++ > target/s390x/cpu.h | 1 + > target/s390x/tcg/excp_helper.c | 3 ++- > 3 files changed, 29 insertions(+), 1 deletion(-) > > diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c > index b10a8541ff8..8e6e46aa3d8 100644 > --- a/target/s390x/cpu.c > +++ b/target/s390x/cpu.c > @@ -41,6 +41,26 @@ > #define CR0_RESET 0xE0UL > #define CR14_RESET 0xC2000000UL; > > +#ifndef CONFIG_USER_ONLY > +static bool is_early_exception_recognized(uint64_t mask, uint64_t addr) > +{ > + if (mask & PSW_MASK_RESERVED) { > + return true; > + } > + > + switch (mask & (PSW_MASK_32 | PSW_MASK_64)) { > + case 0: > + return addr & ~0xffffffULL; > + case PSW_MASK_32: > + return addr & ~0x7fffffffULL; > + case PSW_MASK_32 | PSW_MASK_64: > + return false; > + default: /* PSW_MASK_64 */ > + return true; > + } > +} > +#endif > + > void s390_cpu_set_psw(CPUS390XState *env, uint64_t mask, uint64_t addr) > { > #ifndef CONFIG_USER_ONLY > @@ -57,6 +77,12 @@ void s390_cpu_set_psw(CPUS390XState *env, uint64_t mask, uint64_t addr) > env->cc_op = (mask >> 44) & 3; > > #ifndef CONFIG_USER_ONLY > + if (is_early_exception_recognized(mask, addr)) { > + env->int_pgm_ilen = 0; > + trigger_pgm_exception(env, PGM_SPECIFICATION); > + return; > + } > + > if ((old_mask ^ mask) & PSW_MASK_PER) { > s390_cpu_recompute_watchpoints(env_cpu(env)); > } > diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h > index 7d6d01325b2..b4de6945936 100644 > --- a/target/s390x/cpu.h > +++ b/target/s390x/cpu.h > @@ -292,6 +292,7 @@ extern const VMStateDescription vmstate_s390_cpu; > #define PSW_MASK_32 0x0000000080000000ULL > #define PSW_MASK_SHORT_ADDR 0x000000007fffffffULL > #define PSW_MASK_SHORT_CTRL 0xffffffff80000000ULL > +#define PSW_MASK_RESERVED 0xb800007e7fffffffULL What about bit 12? With bit 12 added: I haven't checked, but I don't think that would be sufficient for short PSWs loaded with lpsw. I'm not seeing op_lpsw flipping the 12th bit. op_lpsw looks pretty broken to me overall, putting the BA bit into the address. I think it should look something like this (didn't actually try it): static DisasJumpType op_lpsw(DisasContext *s, DisasOps *o) { TCGv_i64 t1, t2; per_breaking_event(s); t1 = tcg_temp_new_i64(); t2 = tcg_temp_new_i64(); tcg_gen_qemu_ld_i64(t1, o->in2, get_mem_index(s), MO_TEUQ | MO_ALIGN); //load 8byte instead of 4 tcg_gen_addi_i64(o->in2, o->in2, 4); tcg_gen_qemu_ld32u(t2, o->in2, get_mem_index(s)); tcg_gen_andi_i64(t2, t2, PSW_MASK_SHORT_ADDR); /* Convert the 32-bit PSW_MASK into the 64-bit PSW_MASK. */ tcg_gen_andi_i64(t1, t1, PSW_MASK_SHORT_CTRL); tcg_gen_xori_i64(t1, t1, PSW_MASK_SHORTPSW); gen_helper_load_psw(cpu_env, t1, t2); return DISAS_NORETURN; } > > #undef PSW_ASC_PRIMARY > #undef PSW_ASC_ACCREG > diff --git a/target/s390x/tcg/excp_helper.c b/target/s390x/tcg/excp_helper.c > index bc767f04438..a7829b1e494 100644 > --- a/target/s390x/tcg/excp_helper.c > +++ b/target/s390x/tcg/excp_helper.c > @@ -212,7 +212,8 @@ static void do_program_interrupt(CPUS390XState *env) > LowCore *lowcore; > int ilen = env->int_pgm_ilen; > > - assert(ilen == 2 || ilen == 4 || ilen == 6); > + assert((env->int_pgm_code == PGM_SPECIFICATION && ilen == 0) || > + ilen == 2 || ilen == 4 || ilen == 6); > > switch (env->int_pgm_code) { > case PGM_PER:
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c index b10a8541ff8..8e6e46aa3d8 100644 --- a/target/s390x/cpu.c +++ b/target/s390x/cpu.c @@ -41,6 +41,26 @@ #define CR0_RESET 0xE0UL #define CR14_RESET 0xC2000000UL; +#ifndef CONFIG_USER_ONLY +static bool is_early_exception_recognized(uint64_t mask, uint64_t addr) +{ + if (mask & PSW_MASK_RESERVED) { + return true; + } + + switch (mask & (PSW_MASK_32 | PSW_MASK_64)) { + case 0: + return addr & ~0xffffffULL; + case PSW_MASK_32: + return addr & ~0x7fffffffULL; + case PSW_MASK_32 | PSW_MASK_64: + return false; + default: /* PSW_MASK_64 */ + return true; + } +} +#endif + void s390_cpu_set_psw(CPUS390XState *env, uint64_t mask, uint64_t addr) { #ifndef CONFIG_USER_ONLY @@ -57,6 +77,12 @@ void s390_cpu_set_psw(CPUS390XState *env, uint64_t mask, uint64_t addr) env->cc_op = (mask >> 44) & 3; #ifndef CONFIG_USER_ONLY + if (is_early_exception_recognized(mask, addr)) { + env->int_pgm_ilen = 0; + trigger_pgm_exception(env, PGM_SPECIFICATION); + return; + } + if ((old_mask ^ mask) & PSW_MASK_PER) { s390_cpu_recompute_watchpoints(env_cpu(env)); } diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h index 7d6d01325b2..b4de6945936 100644 --- a/target/s390x/cpu.h +++ b/target/s390x/cpu.h @@ -292,6 +292,7 @@ extern const VMStateDescription vmstate_s390_cpu; #define PSW_MASK_32 0x0000000080000000ULL #define PSW_MASK_SHORT_ADDR 0x000000007fffffffULL #define PSW_MASK_SHORT_CTRL 0xffffffff80000000ULL +#define PSW_MASK_RESERVED 0xb800007e7fffffffULL #undef PSW_ASC_PRIMARY #undef PSW_ASC_ACCREG diff --git a/target/s390x/tcg/excp_helper.c b/target/s390x/tcg/excp_helper.c index bc767f04438..a7829b1e494 100644 --- a/target/s390x/tcg/excp_helper.c +++ b/target/s390x/tcg/excp_helper.c @@ -212,7 +212,8 @@ static void do_program_interrupt(CPUS390XState *env) LowCore *lowcore; int ilen = env->int_pgm_ilen; - assert(ilen == 2 || ilen == 4 || ilen == 6); + assert((env->int_pgm_code == PGM_SPECIFICATION && ilen == 0) || + ilen == 2 || ilen == 4 || ilen == 6); switch (env->int_pgm_code) { case PGM_PER:
Generate specification exception if a reserved bit is set in the PSW mask or if the PSW address is out of bounds dictated by the addresing mode. Reported-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- target/s390x/cpu.c | 26 ++++++++++++++++++++++++++ target/s390x/cpu.h | 1 + target/s390x/tcg/excp_helper.c | 3 ++- 3 files changed, 29 insertions(+), 1 deletion(-)