[PATCH for-9.0 v2 0/7] target/hppa: more small fixes

[PATCH for-9.0 v2 0/7] target/hppa: more small fixes

[Richard Henderson]

Two new fixes for -icount assertion failures wrt
not setting can_do_io properly.  With these I can
boot Linux with -icount shift=auto enabled.


r~


Richard Henderson (5):
  target/hppa: Fix BE,L set of sr0
  target/hppa: Fix B,GATE for wide mode
  target/hppa: Mark interval timer write as io
  target/hppa: Tidy read of interval timer
  target/hppa: Fix EIRR, EIEM versus icount

Sven Schnelle (2):
  target/hppa: Handle unit conditions for wide mode
  target/hppa: Fix ADD/SUB trap on overflow for narrow mode

 target/hppa/helper.h     |  1 -
 target/hppa/int_helper.c | 14 ++++-------
 target/hppa/translate.c  | 52 ++++++++++++++++++++++++++++++++--------
 3 files changed, 46 insertions(+), 21 deletions(-)

-- 
2.34.1

[PATCH v2 1/7] target/hppa: Fix BE,L set of sr0

[Richard Henderson]

The return address comes from IA*Q_Next, and IASQ_Next
is always equal to IASQ_Back, not IASQ_Front.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/hppa/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 19594f917e..1766a63001 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -3817,7 +3817,7 @@ static bool trans_be(DisasContext *ctx, arg_be *a)
     load_spr(ctx, new_spc, a->sp);
     if (a->l) {
         copy_iaoq_entry(ctx, cpu_gr[31], ctx->iaoq_n, ctx->iaoq_n_var);
-        tcg_gen_mov_i64(cpu_sr[0], cpu_iasq_f);
+        tcg_gen_mov_i64(cpu_sr[0], cpu_iasq_b);
     }
     if (a->n && use_nullify_skip(ctx)) {
         copy_iaoq_entry(ctx, cpu_iaoq_f, -1, tmp);
-- 
2.34.1

Re: [PATCH v2 1/7] target/hppa: Fix BE,L set of sr0

[Helge Deller]

On 3/23/24 18:29, Richard Henderson wrote:
> The return address comes from IA*Q_Next, and IASQ_Next
> is always equal to IASQ_Back, not IASQ_Front.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Tested-by: Helge Deller <deller@gmx.de>


> ---
>   target/hppa/translate.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index 19594f917e..1766a63001 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -3817,7 +3817,7 @@ static bool trans_be(DisasContext *ctx, arg_be *a)
>       load_spr(ctx, new_spc, a->sp);
>       if (a->l) {
>           copy_iaoq_entry(ctx, cpu_gr[31], ctx->iaoq_n, ctx->iaoq_n_var);
> -        tcg_gen_mov_i64(cpu_sr[0], cpu_iasq_f);
> +        tcg_gen_mov_i64(cpu_sr[0], cpu_iasq_b);
>       }
>       if (a->n && use_nullify_skip(ctx)) {
>           copy_iaoq_entry(ctx, cpu_iaoq_f, -1, tmp);

[PATCH v2 2/7] target/hppa: Fix B,GATE for wide mode

[Richard Henderson]

Do not clobber the high bits of the address by using a 32-bit deposit.

Reviewed-by: Helge Deller <deller@gmx.de>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/hppa/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 1766a63001..f875d76a23 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -3880,7 +3880,7 @@ static bool trans_b_gate(DisasContext *ctx, arg_b_gate *a)
         }
         /* No change for non-gateway pages or for priv decrease.  */
         if (type >= 4 && type - 4 < ctx->privilege) {
-            dest = deposit32(dest, 0, 2, type - 4);
+            dest = deposit64(dest, 0, 2, type - 4);
         }
     } else {
         dest &= -4;  /* priv = 0 */
-- 
2.34.1

[PATCH v2 3/7] target/hppa: Handle unit conditions for wide mode

[Richard Henderson]

From: Sven Schnelle <svens@stackframe.org>

Wide mode provides two more conditions, add them.

Fixes: 59963d8fdf42 ("target/hppa: Pass d to do_unit_cond")
Signed-off-by: Sven Schnelle <svens@stackframe.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20240321184228.611897-1-svens@stackframe.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/hppa/translate.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index f875d76a23..2cb91956da 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -967,11 +967,22 @@ static DisasCond do_unit_cond(unsigned cf, bool d, TCGv_i64 res,
 
     switch (cf >> 1) {
     case 0: /* never / TR */
-    case 1: /* undefined */
-    case 5: /* undefined */
         cond = cond_make_f();
         break;
 
+    case 1: /* SBW / NBW */
+        if (d) {
+            tmp = tcg_temp_new_i64();
+            tcg_gen_subi_i64(tmp, res, d_repl * 0x00000001u);
+            tcg_gen_andc_i64(tmp, tmp, res);
+            tcg_gen_andi_i64(tmp, tmp, d_repl * 0x80000000u);
+            cond = cond_make_0(TCG_COND_NE, tmp);
+        } else {
+            /* undefined */
+            cond = cond_make_f();
+        }
+        break;
+
     case 2: /* SBZ / NBZ */
         /* See hasless(v,1) from
          * https://graphics.stanford.edu/~seander/bithacks.html#ZeroInWord
@@ -996,6 +1007,16 @@ static DisasCond do_unit_cond(unsigned cf, bool d, TCGv_i64 res,
         cond = cond_make_0(TCG_COND_NE, cb);
         break;
 
+    case 5: /* SWC / NWC */
+        if (d) {
+            tcg_gen_andi_i64(cb, cb, d_repl * 0x80000000u);
+            cond = cond_make_0(TCG_COND_NE, cb);
+        } else {
+            /* undefined */
+            cond = cond_make_f();
+        }
+        break;
+
     case 6: /* SBC / NBC */
         tcg_gen_andi_i64(cb, cb, d_repl * 0x80808080u);
         cond = cond_make_0(TCG_COND_NE, cb);
-- 
2.34.1

[PATCH v2 4/7] target/hppa: Fix ADD/SUB trap on overflow for narrow mode

[Richard Henderson]

From: Sven Schnelle <svens@stackframe.org>

Fixes: c53e401ed9ff ("target/hppa: Remove TARGET_REGISTER_BITS")
Signed-off-by: Sven Schnelle <svens@stackframe.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20240321184228.611897-2-svens@stackframe.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/hppa/translate.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 2cb91956da..ceb739c54a 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -1126,6 +1126,9 @@ static void do_add(DisasContext *ctx, unsigned rt, TCGv_i64 in1,
     if (is_tsv || cond_need_sv(c)) {
         sv = do_add_sv(ctx, dest, in1, in2);
         if (is_tsv) {
+            if (!d) {
+                tcg_gen_ext32s_i64(sv, sv);
+            }
             /* ??? Need to include overflow from shift.  */
             gen_helper_tsv(tcg_env, sv);
         }
@@ -1217,6 +1220,9 @@ static void do_sub(DisasContext *ctx, unsigned rt, TCGv_i64 in1,
     if (is_tsv || cond_need_sv(c)) {
         sv = do_sub_sv(ctx, dest, in1, in2);
         if (is_tsv) {
+            if (!d) {
+                tcg_gen_ext32s_i64(sv, sv);
+            }
             gen_helper_tsv(tcg_env, sv);
         }
     }
-- 
2.34.1

[PATCH v2 5/7] target/hppa: Mark interval timer write as io

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/hppa/translate.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index ceb739c54a..8c1a564c5d 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -2162,6 +2162,9 @@ static bool trans_mtctl(DisasContext *ctx, arg_mtctl *a)
 
     switch (ctl) {
     case CR_IT:
+        if (translator_io_start(&ctx->base)) {
+            ctx->base.is_jmp = DISAS_IAQ_N_STALE;
+        }
         gen_helper_write_interval_timer(tcg_env, reg);
         break;
     case CR_EIRR:
-- 
2.34.1

Re: [PATCH v2 5/7] target/hppa: Mark interval timer write as io

[Helge Deller]

On 3/23/24 18:29, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Helge Deller <deller@gmx.de>
Tested-by: Helge Deller <deller@gmx.de>

Thanks!
Helge

> ---
>   target/hppa/translate.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index ceb739c54a..8c1a564c5d 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -2162,6 +2162,9 @@ static bool trans_mtctl(DisasContext *ctx, arg_mtctl *a)
>
>       switch (ctl) {
>       case CR_IT:
> +        if (translator_io_start(&ctx->base)) {
> +            ctx->base.is_jmp = DISAS_IAQ_N_STALE;
> +        }
>           gen_helper_write_interval_timer(tcg_env, reg);
>           break;
>       case CR_EIRR:

[PATCH v2 6/7] target/hppa: Tidy read of interval timer

[Richard Henderson]

The call to gen_helper_read_interval_timer is
identical on both sides of the IF.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/hppa/translate.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 8c1a564c5d..5b8c1b06c3 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -2082,11 +2082,9 @@ static bool trans_mfctl(DisasContext *ctx, arg_mfctl *a)
         nullify_over(ctx);
         tmp = dest_gpr(ctx, rt);
         if (translator_io_start(&ctx->base)) {
-            gen_helper_read_interval_timer(tmp);
             ctx->base.is_jmp = DISAS_IAQ_N_STALE;
-        } else {
-            gen_helper_read_interval_timer(tmp);
         }
+        gen_helper_read_interval_timer(tmp);
         save_gpr(ctx, rt, tmp);
         return nullify_end(ctx);
     case 26:
-- 
2.34.1

Re: [PATCH v2 6/7] target/hppa: Tidy read of interval timer

[Helge Deller]

On 3/23/24 18:29, Richard Henderson wrote:
> The call to gen_helper_read_interval_timer is
> identical on both sides of the IF.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>


Reviewed-by: Helge Deller <deller@gmx.de>
Tested-by: Helge Deller <deller@gmx.de>

Thanks!
Helge

> ---
>   target/hppa/translate.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index 8c1a564c5d..5b8c1b06c3 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -2082,11 +2082,9 @@ static bool trans_mfctl(DisasContext *ctx, arg_mfctl *a)
>           nullify_over(ctx);
>           tmp = dest_gpr(ctx, rt);
>           if (translator_io_start(&ctx->base)) {
> -            gen_helper_read_interval_timer(tmp);
>               ctx->base.is_jmp = DISAS_IAQ_N_STALE;
> -        } else {
> -            gen_helper_read_interval_timer(tmp);
>           }
> +        gen_helper_read_interval_timer(tmp);
>           save_gpr(ctx, rt, tmp);
>           return nullify_end(ctx);
>       case 26:

[PATCH v2 7/7] target/hppa: Fix EIRR, EIEM versus icount

[Richard Henderson]

Call translator_io_start before write to EIRR.
Move evaluation of EIRR vs EIEM to hppa_cpu_exec_interrupt.
Exit TB after write to EIEM, but otherwise use a straight store.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/hppa/helper.h     |  1 -
 target/hppa/int_helper.c | 14 ++++----------
 target/hppa/translate.c  | 10 +++++++---
 3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/target/hppa/helper.h b/target/hppa/helper.h
index 1bdbcd8f98..8fd7ba65d8 100644
--- a/target/hppa/helper.h
+++ b/target/hppa/helper.h
@@ -91,7 +91,6 @@ DEF_HELPER_1(rfi, void, env)
 DEF_HELPER_1(rfi_r, void, env)
 DEF_HELPER_FLAGS_2(write_interval_timer, TCG_CALL_NO_RWG, void, env, tl)
 DEF_HELPER_FLAGS_2(write_eirr, TCG_CALL_NO_RWG, void, env, tl)
-DEF_HELPER_FLAGS_2(write_eiem, TCG_CALL_NO_RWG, void, env, tl)
 DEF_HELPER_FLAGS_2(swap_system_mask, TCG_CALL_NO_RWG, tl, env, tl)
 DEF_HELPER_FLAGS_3(itlba_pa11, TCG_CALL_NO_RWG, void, env, tl, tl)
 DEF_HELPER_FLAGS_3(itlbp_pa11, TCG_CALL_NO_RWG, void, env, tl, tl)
diff --git a/target/hppa/int_helper.c b/target/hppa/int_helper.c
index efe638b36e..90437a92cd 100644
--- a/target/hppa/int_helper.c
+++ b/target/hppa/int_helper.c
@@ -28,7 +28,7 @@
 static void eval_interrupt(HPPACPU *cpu)
 {
     CPUState *cs = CPU(cpu);
-    if (cpu->env.cr[CR_EIRR] & cpu->env.cr[CR_EIEM]) {
+    if (cpu->env.cr[CR_EIRR]) {
         cpu_interrupt(cs, CPU_INTERRUPT_HARD);
     } else {
         cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
@@ -89,14 +89,6 @@ void HELPER(write_eirr)(CPUHPPAState *env, target_ulong val)
     bql_unlock();
 }
 
-void HELPER(write_eiem)(CPUHPPAState *env, target_ulong val)
-{
-    env->cr[CR_EIEM] = val;
-    bql_lock();
-    eval_interrupt(env_archcpu(env));
-    bql_unlock();
-}
-
 void hppa_cpu_do_interrupt(CPUState *cs)
 {
     HPPACPU *cpu = HPPA_CPU(cs);
@@ -280,7 +272,9 @@ bool hppa_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
     }
 
     /* If interrupts are requested and enabled, raise them.  */
-    if ((env->psw & PSW_I) && (interrupt_request & CPU_INTERRUPT_HARD)) {
+    if ((interrupt_request & CPU_INTERRUPT_HARD)
+        && (env->psw & PSW_I)
+        && (env->cr[CR_EIRR] & env->cr[CR_EIEM])) {
         cs->exception_index = EXCP_EXT_INTERRUPT;
         hppa_cpu_do_interrupt(cs);
         return true;
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 5b8c1b06c3..46b2d6508d 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -2166,10 +2166,10 @@ static bool trans_mtctl(DisasContext *ctx, arg_mtctl *a)
         gen_helper_write_interval_timer(tcg_env, reg);
         break;
     case CR_EIRR:
+        /* Helper modifies interrupt lines and is therefore IO. */
+        translator_io_start(&ctx->base);
         gen_helper_write_eirr(tcg_env, reg);
-        break;
-    case CR_EIEM:
-        gen_helper_write_eiem(tcg_env, reg);
+        /* Exit to re-evaluate interrupts in the main loop. */
         ctx->base.is_jmp = DISAS_IAQ_N_STALE_EXIT;
         break;
 
@@ -2195,6 +2195,10 @@ static bool trans_mtctl(DisasContext *ctx, arg_mtctl *a)
 #endif
         break;
 
+    case CR_EIEM:
+        /* Exit to re-evaluate interrupts in the main loop. */
+        ctx->base.is_jmp = DISAS_IAQ_N_STALE_EXIT;
+        /* FALLTHRU */
     default:
         tcg_gen_st_i64(reg, tcg_env, offsetof(CPUHPPAState, cr[ctl]));
         break;
-- 
2.34.1

Re: [PATCH v2 7/7] target/hppa: Fix EIRR, EIEM versus icount

[Helge Deller]

On 3/23/24 18:29, Richard Henderson wrote:
> Call translator_io_start before write to EIRR.
> Move evaluation of EIRR vs EIEM to hppa_cpu_exec_interrupt.
> Exit TB after write to EIEM, but otherwise use a straight store.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Helge Deller <deller@gmx.de>
Tested-by: Helge Deller <deller@gmx.de>

Thanks!
Helge


> ---
>   target/hppa/helper.h     |  1 -
>   target/hppa/int_helper.c | 14 ++++----------
>   target/hppa/translate.c  | 10 +++++++---
>   3 files changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/target/hppa/helper.h b/target/hppa/helper.h
> index 1bdbcd8f98..8fd7ba65d8 100644
> --- a/target/hppa/helper.h
> +++ b/target/hppa/helper.h
> @@ -91,7 +91,6 @@ DEF_HELPER_1(rfi, void, env)
>   DEF_HELPER_1(rfi_r, void, env)
>   DEF_HELPER_FLAGS_2(write_interval_timer, TCG_CALL_NO_RWG, void, env, tl)
>   DEF_HELPER_FLAGS_2(write_eirr, TCG_CALL_NO_RWG, void, env, tl)
> -DEF_HELPER_FLAGS_2(write_eiem, TCG_CALL_NO_RWG, void, env, tl)
>   DEF_HELPER_FLAGS_2(swap_system_mask, TCG_CALL_NO_RWG, tl, env, tl)
>   DEF_HELPER_FLAGS_3(itlba_pa11, TCG_CALL_NO_RWG, void, env, tl, tl)
>   DEF_HELPER_FLAGS_3(itlbp_pa11, TCG_CALL_NO_RWG, void, env, tl, tl)
> diff --git a/target/hppa/int_helper.c b/target/hppa/int_helper.c
> index efe638b36e..90437a92cd 100644
> --- a/target/hppa/int_helper.c
> +++ b/target/hppa/int_helper.c
> @@ -28,7 +28,7 @@
>   static void eval_interrupt(HPPACPU *cpu)
>   {
>       CPUState *cs = CPU(cpu);
> -    if (cpu->env.cr[CR_EIRR] & cpu->env.cr[CR_EIEM]) {
> +    if (cpu->env.cr[CR_EIRR]) {
>           cpu_interrupt(cs, CPU_INTERRUPT_HARD);
>       } else {
>           cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
> @@ -89,14 +89,6 @@ void HELPER(write_eirr)(CPUHPPAState *env, target_ulong val)
>       bql_unlock();
>   }
>
> -void HELPER(write_eiem)(CPUHPPAState *env, target_ulong val)
> -{
> -    env->cr[CR_EIEM] = val;
> -    bql_lock();
> -    eval_interrupt(env_archcpu(env));
> -    bql_unlock();
> -}
> -
>   void hppa_cpu_do_interrupt(CPUState *cs)
>   {
>       HPPACPU *cpu = HPPA_CPU(cs);
> @@ -280,7 +272,9 @@ bool hppa_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>       }
>
>       /* If interrupts are requested and enabled, raise them.  */
> -    if ((env->psw & PSW_I) && (interrupt_request & CPU_INTERRUPT_HARD)) {
> +    if ((interrupt_request & CPU_INTERRUPT_HARD)
> +        && (env->psw & PSW_I)
> +        && (env->cr[CR_EIRR] & env->cr[CR_EIEM])) {
>           cs->exception_index = EXCP_EXT_INTERRUPT;
>           hppa_cpu_do_interrupt(cs);
>           return true;
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index 5b8c1b06c3..46b2d6508d 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -2166,10 +2166,10 @@ static bool trans_mtctl(DisasContext *ctx, arg_mtctl *a)
>           gen_helper_write_interval_timer(tcg_env, reg);
>           break;
>       case CR_EIRR:
> +        /* Helper modifies interrupt lines and is therefore IO. */
> +        translator_io_start(&ctx->base);
>           gen_helper_write_eirr(tcg_env, reg);
> -        break;
> -    case CR_EIEM:
> -        gen_helper_write_eiem(tcg_env, reg);
> +        /* Exit to re-evaluate interrupts in the main loop. */
>           ctx->base.is_jmp = DISAS_IAQ_N_STALE_EXIT;
>           break;
>
> @@ -2195,6 +2195,10 @@ static bool trans_mtctl(DisasContext *ctx, arg_mtctl *a)
>   #endif
>           break;
>
> +    case CR_EIEM:
> +        /* Exit to re-evaluate interrupts in the main loop. */
> +        ctx->base.is_jmp = DISAS_IAQ_N_STALE_EXIT;
> +        /* FALLTHRU */
>       default:
>           tcg_gen_st_i64(reg, tcg_env, offsetof(CPUHPPAState, cr[ctl]));
>           break;