diff mbox series

[1/2] target/s390x: Implement Early Exception Recognition

Message ID 20230314110022.184717-2-iii@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series target/s390x: Implement Early Exception Recognition | expand

Commit Message

Ilya Leoshkevich March 14, 2023, 11 a.m. UTC
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(-)

Comments

Richard Henderson March 14, 2023, 4:43 p.m. UTC | #1
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~
David Hildenbrand March 14, 2023, 4:54 p.m. UTC | #2
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>
Nina Schoetterl-Glausch March 14, 2023, 5:58 p.m. UTC | #3
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 mbox series

Patch

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: