[PATCH 00/14] target/s390x: Fix and improve PER

[PATCH 00/14] target/s390x: Fix and improve PER

[Richard Henderson]

Split out from 

v1: https://lore.kernel.org/qemu-devel/20220906101747.344559-1-richard.henderson@linaro.org/
v4: https://lore.kernel.org/qemu-devel/20230220184052.163465-1-richard.henderson@linaro.org/

after I noticed that the testcase,

https://lore.kernel.org/qemu-devel/20221130174610.434590-2-iii@linux.ibm.com/

does not actually work under KVM.

In fact, support for PER_CR9_EVENT_BRANCH in TCG is broken.
This cleans it up and improves things so that when only one PER bit
is set (what I assume to be the normal state of afairs), we have
less overhead.

I'm not sure that all of the interactions are correct, particularly
with multiple PER triggers or other exceptions.  For that, we'd need
a more complex test case.  But at least branches match hardware now.


r~


Richard Henderson (14):
  target/s390x: Do not use unwind for per_check_exception
  target/s390x: Move cpu_get_tb_cpu_state out of line
  target/s390x: Update CR9 bits
  target/s390x: Record separate PER bits in TB flags
  target/s390x: Disable conditional branch-to-next for PER
  target/s390x: Introduce help_goto_indirect
  target/s390x: Simplify help_branch
  target/s390x: Split per_breaking_event from per_branch_*
  target/s390x: Raise exception from helper_per_branch
  target/s390x: Raise exception from per_store_real
  target/s390x: Fix helper_per_ifetch flags
  target/s390x: Simplify per_ifetch, per_check_exception
  target/s390x: Adjust check of noreturn in translate_one
  tests/tcg/s390x: Add per.S

 target/s390x/cpu.h                      |  83 ++++----
 target/s390x/helper.h                   |   8 +-
 target/s390x/cpu.c                      |  36 ++++
 target/s390x/tcg/excp_helper.c          |   2 +-
 target/s390x/tcg/misc_helper.c          |  68 ++++---
 target/s390x/tcg/translate.c            | 248 +++++++++---------------
 tests/tcg/s390x/Makefile.softmmu-target |   1 +
 tests/tcg/s390x/per.S                   |  82 ++++++++
 8 files changed, 302 insertions(+), 226 deletions(-)
 create mode 100644 tests/tcg/s390x/per.S

-- 
2.34.1

[PATCH 01/14] target/s390x: Do not use unwind for per_check_exception

[Richard Henderson]

Using exception unwind via tcg_s390_program_interrupt,
we discard the current value of psw.addr, which discards
the result of a branch.

Pass in the address of the next instruction, which may
not be sequential.  Pass in ilen, which we would have
gotten from unwind and is passed to the exception handler.
Sync cc_op before the call, which we would have gotten
from unwind.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/s390x/helper.h          |  2 +-
 target/s390x/tcg/excp_helper.c |  2 +-
 target/s390x/tcg/misc_helper.c | 23 ++++++++++++++++++++---
 target/s390x/tcg/translate.c   | 13 +++++++------
 4 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index cc1c20e9e3..96ab71e877 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -359,7 +359,7 @@ DEF_HELPER_FLAGS_4(ipte, TCG_CALL_NO_RWG, void, env, i64, i64, i32)
 DEF_HELPER_FLAGS_1(ptlb, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_FLAGS_1(purge, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_3(lra, i64, env, i64, i64)
-DEF_HELPER_1(per_check_exception, void, env)
+DEF_HELPER_FLAGS_3(per_check_exception, TCG_CALL_NO_WG, void, env, i64, i32)
 DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_RWG, void, env, i64, i64)
 DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_RWG, void, env, i64)
 DEF_HELPER_FLAGS_1(per_store_real, TCG_CALL_NO_RWG, void, env)
diff --git a/target/s390x/tcg/excp_helper.c b/target/s390x/tcg/excp_helper.c
index f1c33f7967..4c0b692c9e 100644
--- a/target/s390x/tcg/excp_helper.c
+++ b/target/s390x/tcg/excp_helper.c
@@ -209,7 +209,7 @@ static void do_program_interrupt(CPUS390XState *env)
 
     switch (env->int_pgm_code) {
     case PGM_PER:
-        advance = !(env->per_perc_atmid & PER_CODE_EVENT_NULLIFICATION);
+        /* advance already handled */
         break;
     case PGM_ASCE_TYPE:
     case PGM_REG_FIRST_TRANS:
diff --git a/target/s390x/tcg/misc_helper.c b/target/s390x/tcg/misc_helper.c
index 8764846ce8..8c27998ab9 100644
--- a/target/s390x/tcg/misc_helper.c
+++ b/target/s390x/tcg/misc_helper.c
@@ -20,6 +20,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/cutils.h"
+#include "qemu/log.h"
 #include "cpu.h"
 #include "s390x-internal.h"
 #include "qemu/host-utils.h"
@@ -590,10 +591,26 @@ void HELPER(chsc)(CPUS390XState *env, uint64_t inst)
 #endif
 
 #ifndef CONFIG_USER_ONLY
-void HELPER(per_check_exception)(CPUS390XState *env)
+G_NORETURN static void per_raise_exception(CPUS390XState *env)
 {
-    if (env->per_perc_atmid) {
-        tcg_s390_program_interrupt(env, PGM_PER, GETPC());
+    trigger_pgm_exception(env, PGM_PER);
+    cpu_loop_exit(env_cpu(env));
+}
+
+G_NORETURN static void per_raise_exception_log(CPUS390XState *env)
+{
+    qemu_log_mask(CPU_LOG_INT, "PER interrupt after %#" PRIx64 "\n",
+                  env->per_address);
+    per_raise_exception(env);
+}
+
+void HELPER(per_check_exception)(CPUS390XState *env, uint64_t next_pc,
+                                 uint32_t ilen)
+{
+    if (unlikely(env->per_perc_atmid)) {
+        env->psw.addr = next_pc;
+        env->int_pgm_ilen = ilen;
+        per_raise_exception_log(env);
     }
 }
 
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 90a74ee795..2319dcf259 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -6426,13 +6426,14 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
 
 #ifndef CONFIG_USER_ONLY
     if (s->base.tb->flags & FLAG_MASK_PER) {
-        /* An exception might be triggered, save PSW if not already done.  */
-        if (ret == DISAS_NEXT || ret == DISAS_TOO_MANY) {
-            tcg_gen_movi_i64(psw_addr, s->pc_tmp);
-        }
+        TCGv_i64 next_pc = psw_addr;
 
-        /* Call the helper to check for a possible PER exception.  */
-        gen_helper_per_check_exception(tcg_env);
+        if (ret == DISAS_NEXT || ret == DISAS_TOO_MANY) {
+            next_pc = tcg_constant_i64(s->pc_tmp);
+        }
+        update_cc_op(s);
+        gen_helper_per_check_exception(tcg_env, next_pc,
+                                       tcg_constant_i32(s->ilen));
     }
 #endif
 
-- 
2.34.1

[PATCH 02/14] target/s390x: Move cpu_get_tb_cpu_state out of line

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/s390x/cpu.h | 23 ++---------------------
 target/s390x/cpu.c | 22 ++++++++++++++++++++++
 2 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 414680eed1..950f84f316 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -413,27 +413,8 @@ static inline int s390x_env_mmu_index(CPUS390XState *env, bool ifetch)
 
 #include "tcg/tcg_s390x.h"
 
-static inline void cpu_get_tb_cpu_state(CPUS390XState *env, vaddr *pc,
-                                        uint64_t *cs_base, uint32_t *flags)
-{
-    if (env->psw.addr & 1) {
-        /*
-         * Instructions must be at even addresses.
-         * This needs to be checked before address translation.
-         */
-        env->int_pgm_ilen = 2; /* see s390_cpu_tlb_fill() */
-        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, 0);
-    }
-    *pc = env->psw.addr;
-    *cs_base = env->ex_value;
-    *flags = (env->psw.mask >> FLAG_MASK_PSW_SHIFT) & FLAG_MASK_PSW;
-    if (env->cregs[0] & CR0_AFP) {
-        *flags |= FLAG_MASK_AFP;
-    }
-    if (env->cregs[0] & CR0_VECTOR) {
-        *flags |= FLAG_MASK_VECTOR;
-    }
-}
+void cpu_get_tb_cpu_state(CPUS390XState *env, vaddr *pc,
+                          uint64_t *cs_base, uint32_t *flags);
 
 #endif /* CONFIG_TCG */
 
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index f7194534ae..a8428b5a1e 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -324,6 +324,28 @@ static void s390_cpu_reset_full(DeviceState *dev)
 #ifdef CONFIG_TCG
 #include "hw/core/tcg-cpu-ops.h"
 
+void cpu_get_tb_cpu_state(CPUS390XState *env, vaddr *pc,
+                          uint64_t *cs_base, uint32_t *flags)
+{
+    if (env->psw.addr & 1) {
+        /*
+         * Instructions must be at even addresses.
+         * This needs to be checked before address translation.
+         */
+        env->int_pgm_ilen = 2; /* see s390_cpu_tlb_fill() */
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, 0);
+    }
+    *pc = env->psw.addr;
+    *cs_base = env->ex_value;
+    *flags = (env->psw.mask >> FLAG_MASK_PSW_SHIFT) & FLAG_MASK_PSW;
+    if (env->cregs[0] & CR0_AFP) {
+        *flags |= FLAG_MASK_AFP;
+    }
+    if (env->cregs[0] & CR0_VECTOR) {
+        *flags |= FLAG_MASK_VECTOR;
+    }
+}
+
 static const TCGCPUOps s390_tcg_ops = {
     .initialize = s390x_translate_init,
     .restore_state_to_opc = s390x_restore_state_to_opc,
-- 
2.34.1

[PATCH 03/14] target/s390x: Update CR9 bits

[Richard Henderson]

Update from the PoO 14th edition.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/s390x/cpu.h             | 18 +++++++++++-------
 target/s390x/tcg/misc_helper.c |  2 +-
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 950f84f316..1bb723a9d3 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -419,13 +419,17 @@ void cpu_get_tb_cpu_state(CPUS390XState *env, vaddr *pc,
 #endif /* CONFIG_TCG */
 
 /* PER bits from control register 9 */
-#define PER_CR9_EVENT_BRANCH           0x80000000
-#define PER_CR9_EVENT_IFETCH           0x40000000
-#define PER_CR9_EVENT_STORE            0x20000000
-#define PER_CR9_EVENT_STORE_REAL       0x08000000
-#define PER_CR9_EVENT_NULLIFICATION    0x01000000
-#define PER_CR9_CONTROL_BRANCH_ADDRESS 0x00800000
-#define PER_CR9_CONTROL_ALTERATION     0x00200000
+#define PER_CR9_EVENT_BRANCH                    0x80000000
+#define PER_CR9_EVENT_IFETCH                    0x40000000
+#define PER_CR9_EVENT_STORE                     0x20000000
+#define PER_CR9_EVENT_STORAGE_KEY_ALTERATION    0x10000000
+#define PER_CR9_EVENT_STORE_REAL                0x08000000
+#define PER_CR9_EVENT_ZERO_ADDRESS_DETECTION    0x04000000
+#define PER_CR9_EVENT_TRANSACTION_END           0x02000000
+#define PER_CR9_EVENT_IFETCH_NULLIFICATION      0x01000000
+#define PER_CR9_CONTROL_BRANCH_ADDRESS          0x00800000
+#define PER_CR9_CONTROL_TRANSACTION_SUPRESS     0x00400000
+#define PER_CR9_CONTROL_STORAGE_ALTERATION      0x00200000
 
 /* PER bits from the PER CODE/ATMID/AI in lowcore */
 #define PER_CODE_EVENT_BRANCH          0x8000
diff --git a/target/s390x/tcg/misc_helper.c b/target/s390x/tcg/misc_helper.c
index 8c27998ab9..02215a7f18 100644
--- a/target/s390x/tcg/misc_helper.c
+++ b/target/s390x/tcg/misc_helper.c
@@ -644,7 +644,7 @@ void HELPER(per_ifetch)(CPUS390XState *env, uint64_t addr)
 
         /* If the instruction has to be nullified, trigger the
            exception immediately. */
-        if (env->cregs[9] & PER_CR9_EVENT_NULLIFICATION) {
+        if (env->cregs[9] & PER_CR9_EVENT_IFETCH_NULLIFICATION) {
             CPUState *cs = env_cpu(env);
 
             env->per_perc_atmid |= PER_CODE_EVENT_NULLIFICATION;
-- 
2.34.1

[PATCH 04/14] target/s390x: Record separate PER bits in TB flags

[Richard Henderson]

Record successful-branching, instruction-fetching, and
store-using-real-address.  The other PER bits are not used
during translation.  Having checked these at translation time,
we can remove runtime tests from the helpers.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/s390x/cpu.h             | 42 ++++++++++++++++++++++++----------
 target/s390x/cpu.c             | 22 ++++++++++++++----
 target/s390x/tcg/misc_helper.c | 21 +++++++----------
 target/s390x/tcg/translate.c   | 10 ++++----
 4 files changed, 61 insertions(+), 34 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 1bb723a9d3..d6b75ad0e0 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -342,19 +342,32 @@ extern const VMStateDescription vmstate_s390_cpu;
 
 /* tb flags */
 
-#define FLAG_MASK_PSW_SHIFT     31
-#define FLAG_MASK_PER           (PSW_MASK_PER    >> FLAG_MASK_PSW_SHIFT)
-#define FLAG_MASK_DAT           (PSW_MASK_DAT    >> FLAG_MASK_PSW_SHIFT)
-#define FLAG_MASK_PSTATE        (PSW_MASK_PSTATE >> FLAG_MASK_PSW_SHIFT)
-#define FLAG_MASK_ASC           (PSW_MASK_ASC    >> FLAG_MASK_PSW_SHIFT)
-#define FLAG_MASK_64            (PSW_MASK_64     >> FLAG_MASK_PSW_SHIFT)
-#define FLAG_MASK_32            (PSW_MASK_32     >> FLAG_MASK_PSW_SHIFT)
-#define FLAG_MASK_PSW           (FLAG_MASK_PER | FLAG_MASK_DAT | FLAG_MASK_PSTATE \
-                                | FLAG_MASK_ASC | FLAG_MASK_64 | FLAG_MASK_32)
+#define FLAG_MASK_PSW_SHIFT             31
+#define FLAG_MASK_32                    0x00000001u
+#define FLAG_MASK_64                    0x00000002u
+#define FLAG_MASK_AFP                   0x00000004u
+#define FLAG_MASK_VECTOR                0x00000008u
+#define FLAG_MASK_ASC                   0x00018000u
+#define FLAG_MASK_PSTATE                0x00020000u
+#define FLAG_MASK_PER_IFETCH_NULLIFY    0x01000000u
+#define FLAG_MASK_DAT                   0x08000000u
+#define FLAG_MASK_PER_STORE_REAL        0x20000000u
+#define FLAG_MASK_PER_IFETCH            0x40000000u
+#define FLAG_MASK_PER_BRANCH            0x80000000u
 
-/* we'll use some unused PSW positions to store CR flags in tb flags */
-#define FLAG_MASK_AFP           (PSW_MASK_UNUSED_2 >> FLAG_MASK_PSW_SHIFT)
-#define FLAG_MASK_VECTOR        (PSW_MASK_UNUSED_3 >> FLAG_MASK_PSW_SHIFT)
+QEMU_BUILD_BUG_ON(FLAG_MASK_32 != PSW_MASK_32 >> FLAG_MASK_PSW_SHIFT);
+QEMU_BUILD_BUG_ON(FLAG_MASK_64 != PSW_MASK_64 >> FLAG_MASK_PSW_SHIFT);
+QEMU_BUILD_BUG_ON(FLAG_MASK_ASC != PSW_MASK_ASC >> FLAG_MASK_PSW_SHIFT);
+QEMU_BUILD_BUG_ON(FLAG_MASK_PSTATE != PSW_MASK_PSTATE >> FLAG_MASK_PSW_SHIFT);
+QEMU_BUILD_BUG_ON(FLAG_MASK_DAT != PSW_MASK_DAT >> FLAG_MASK_PSW_SHIFT);
+
+#define FLAG_MASK_PSW           (FLAG_MASK_DAT | FLAG_MASK_PSTATE | \
+                                 FLAG_MASK_ASC | FLAG_MASK_64 | FLAG_MASK_32)
+#define FLAG_MASK_CR9           (FLAG_MASK_PER_BRANCH | FLAG_MASK_PER_IFETCH)
+#define FLAG_MASK_PER           (FLAG_MASK_PER_BRANCH | \
+                                 FLAG_MASK_PER_IFETCH | \
+                                 FLAG_MASK_PER_IFETCH_NULLIFY | \
+                                 FLAG_MASK_PER_STORE_REAL)
 
 /* Control register 0 bits */
 #define CR0_LOWPROT             0x0000000010000000ULL
@@ -431,6 +444,11 @@ void cpu_get_tb_cpu_state(CPUS390XState *env, vaddr *pc,
 #define PER_CR9_CONTROL_TRANSACTION_SUPRESS     0x00400000
 #define PER_CR9_CONTROL_STORAGE_ALTERATION      0x00200000
 
+QEMU_BUILD_BUG_ON(FLAG_MASK_PER_BRANCH != PER_CR9_EVENT_BRANCH);
+QEMU_BUILD_BUG_ON(FLAG_MASK_PER_IFETCH != PER_CR9_EVENT_IFETCH);
+QEMU_BUILD_BUG_ON(FLAG_MASK_PER_IFETCH_NULLIFY !=
+                  PER_CR9_EVENT_IFETCH_NULLIFICATION);
+
 /* PER bits from the PER CODE/ATMID/AI in lowcore */
 #define PER_CODE_EVENT_BRANCH          0x8000
 #define PER_CODE_EVENT_IFETCH          0x4000
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index a8428b5a1e..2bbeaca36e 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -325,8 +325,10 @@ static void s390_cpu_reset_full(DeviceState *dev)
 #include "hw/core/tcg-cpu-ops.h"
 
 void cpu_get_tb_cpu_state(CPUS390XState *env, vaddr *pc,
-                          uint64_t *cs_base, uint32_t *flags)
+                          uint64_t *cs_base, uint32_t *pflags)
 {
+    uint32_t flags;
+
     if (env->psw.addr & 1) {
         /*
          * Instructions must be at even addresses.
@@ -335,15 +337,27 @@ void cpu_get_tb_cpu_state(CPUS390XState *env, vaddr *pc,
         env->int_pgm_ilen = 2; /* see s390_cpu_tlb_fill() */
         tcg_s390_program_interrupt(env, PGM_SPECIFICATION, 0);
     }
+
     *pc = env->psw.addr;
     *cs_base = env->ex_value;
-    *flags = (env->psw.mask >> FLAG_MASK_PSW_SHIFT) & FLAG_MASK_PSW;
+
+    flags = (env->psw.mask >> FLAG_MASK_PSW_SHIFT) & FLAG_MASK_PSW;
+    if (env->psw.mask & PSW_MASK_PER) {
+        flags |= env->cregs[9] & (FLAG_MASK_PER_BRANCH |
+                                  FLAG_MASK_PER_IFETCH |
+                                  FLAG_MASK_PER_IFETCH_NULLIFY);
+        if ((env->cregs[9] & PER_CR9_EVENT_STORE) &&
+            (env->cregs[9] & PER_CR9_EVENT_STORE_REAL)) {
+            flags |= FLAG_MASK_PER_STORE_REAL;
+        }
+    }
     if (env->cregs[0] & CR0_AFP) {
-        *flags |= FLAG_MASK_AFP;
+        flags |= FLAG_MASK_AFP;
     }
     if (env->cregs[0] & CR0_VECTOR) {
-        *flags |= FLAG_MASK_VECTOR;
+        flags |= FLAG_MASK_VECTOR;
     }
+    *pflags = flags;
 }
 
 static const TCGCPUOps s390_tcg_ops = {
diff --git a/target/s390x/tcg/misc_helper.c b/target/s390x/tcg/misc_helper.c
index 02215a7f18..b7100c06c0 100644
--- a/target/s390x/tcg/misc_helper.c
+++ b/target/s390x/tcg/misc_helper.c
@@ -627,18 +627,16 @@ static inline bool get_per_in_range(CPUS390XState *env, uint64_t addr)
 
 void HELPER(per_branch)(CPUS390XState *env, uint64_t from, uint64_t to)
 {
-    if ((env->cregs[9] & PER_CR9_EVENT_BRANCH)) {
-        if (!(env->cregs[9] & PER_CR9_CONTROL_BRANCH_ADDRESS)
-            || get_per_in_range(env, to)) {
-            env->per_address = from;
-            env->per_perc_atmid = PER_CODE_EVENT_BRANCH | get_per_atmid(env);
-        }
+    if (!(env->cregs[9] & PER_CR9_CONTROL_BRANCH_ADDRESS)
+        || get_per_in_range(env, to)) {
+        env->per_address = from;
+        env->per_perc_atmid = PER_CODE_EVENT_BRANCH | get_per_atmid(env);
     }
 }
 
 void HELPER(per_ifetch)(CPUS390XState *env, uint64_t addr)
 {
-    if ((env->cregs[9] & PER_CR9_EVENT_IFETCH) && get_per_in_range(env, addr)) {
+    if (get_per_in_range(env, addr)) {
         env->per_address = addr;
         env->per_perc_atmid = PER_CODE_EVENT_IFETCH | get_per_atmid(env);
 
@@ -659,12 +657,9 @@ void HELPER(per_ifetch)(CPUS390XState *env, uint64_t addr)
 
 void HELPER(per_store_real)(CPUS390XState *env)
 {
-    if ((env->cregs[9] & PER_CR9_EVENT_STORE) &&
-        (env->cregs[9] & PER_CR9_EVENT_STORE_REAL)) {
-        /* PSW is saved just before calling the helper.  */
-        env->per_address = env->psw.addr;
-        env->per_perc_atmid = PER_CODE_EVENT_STORE_REAL | get_per_atmid(env);
-    }
+    /* PSW is saved just before calling the helper.  */
+    env->per_address = env->psw.addr;
+    env->per_perc_atmid = PER_CODE_EVENT_STORE_REAL | get_per_atmid(env);
 }
 #endif
 
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 2319dcf259..3622c5c883 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -348,7 +348,7 @@ static void per_branch(DisasContext *s, bool to_next)
 #ifndef CONFIG_USER_ONLY
     tcg_gen_movi_i64(gbea, s->base.pc_next);
 
-    if (s->base.tb->flags & FLAG_MASK_PER) {
+    if (s->base.tb->flags & FLAG_MASK_PER_BRANCH) {
         TCGv_i64 next_pc = to_next ? tcg_constant_i64(s->pc_tmp) : psw_addr;
         gen_helper_per_branch(tcg_env, gbea, next_pc);
     }
@@ -359,7 +359,7 @@ static void per_branch_cond(DisasContext *s, TCGCond cond,
                             TCGv_i64 arg1, TCGv_i64 arg2)
 {
 #ifndef CONFIG_USER_ONLY
-    if (s->base.tb->flags & FLAG_MASK_PER) {
+    if (s->base.tb->flags & FLAG_MASK_PER_BRANCH) {
         TCGLabel *lab = gen_new_label();
         tcg_gen_brcond_i64(tcg_invert_cond(cond), arg1, arg2, lab);
 
@@ -658,7 +658,7 @@ static void gen_op_calc_cc(DisasContext *s)
 
 static bool use_goto_tb(DisasContext *s, uint64_t dest)
 {
-    if (unlikely(s->base.tb->flags & FLAG_MASK_PER)) {
+    if (unlikely(s->base.tb->flags & FLAG_MASK_PER_BRANCH)) {
         return false;
     }
     return translator_use_goto_tb(&s->base, dest);
@@ -4411,7 +4411,7 @@ static DisasJumpType op_stura(DisasContext *s, DisasOps *o)
 {
     tcg_gen_qemu_st_tl(o->in1, o->in2, MMU_REAL_IDX, s->insn->data);
 
-    if (s->base.tb->flags & FLAG_MASK_PER) {
+    if (s->base.tb->flags & FLAG_MASK_PER_STORE_REAL) {
         update_psw_addr(s);
         gen_helper_per_store_real(tcg_env);
     }
@@ -6325,7 +6325,7 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
     }
 
 #ifndef CONFIG_USER_ONLY
-    if (s->base.tb->flags & FLAG_MASK_PER) {
+    if (s->base.tb->flags & FLAG_MASK_PER_IFETCH) {
         TCGv_i64 addr = tcg_constant_i64(s->base.pc_next);
         gen_helper_per_ifetch(tcg_env, addr);
     }
-- 
2.34.1

[PATCH 05/14] target/s390x: Disable conditional branch-to-next for PER

[Richard Henderson]

For PER, we require a conditional call to helper_per_branch
for the conditional branch.  Fold the remaining optimization
into a call to helper_goto_direct, which will take care of
the remaining gbea adjustment.

Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/s390x/tcg/translate.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 3622c5c883..c3c821430d 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -1133,13 +1133,13 @@ static DisasJumpType help_branch(DisasContext *s, DisasCompare *c,
         goto egress;
     }
     if (is_imm) {
-        if (dest == s->pc_tmp) {
-            /* Branch to next.  */
-            per_branch(s, true);
-            ret = DISAS_NEXT;
-            goto egress;
-        }
-        if (c->cond == TCG_COND_ALWAYS) {
+        /*
+         * Do not optimize a conditional branch if PER enabled, because we
+         * still need a conditional call to helper_per_branch.
+         */
+        if (c->cond == TCG_COND_ALWAYS
+            || (dest == s->pc_tmp &&
+                !(s->base.tb->flags & FLAG_MASK_PER_BRANCH))) {
             ret = help_goto_direct(s, dest);
             goto egress;
         }
-- 
2.34.1

[PATCH 06/14] target/s390x: Introduce help_goto_indirect

[Richard Henderson]

Add a small helper to handle unconditional indirect jumps.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/s390x/tcg/translate.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index c3c821430d..b1a93070cb 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -1120,6 +1120,13 @@ static DisasJumpType help_goto_direct(DisasContext *s, uint64_t dest)
     }
 }
 
+static DisasJumpType help_goto_indirect(DisasContext *s, TCGv_i64 dest)
+{
+    tcg_gen_mov_i64(psw_addr, dest);
+    per_branch(s, false);
+    return DISAS_PC_UPDATED;
+}
+
 static DisasJumpType help_branch(DisasContext *s, DisasCompare *c,
                                  bool is_imm, int imm, TCGv_i64 cdest)
 {
@@ -1150,9 +1157,7 @@ static DisasJumpType help_branch(DisasContext *s, DisasCompare *c,
             goto egress;
         }
         if (c->cond == TCG_COND_ALWAYS) {
-            tcg_gen_mov_i64(psw_addr, cdest);
-            per_branch(s, false);
-            ret = DISAS_PC_UPDATED;
+            ret = help_goto_indirect(s, cdest);
             goto egress;
         }
     }
@@ -1465,9 +1470,7 @@ static DisasJumpType op_bas(DisasContext *s, DisasOps *o)
 {
     pc_to_link_info(o->out, s, s->pc_tmp);
     if (o->in2) {
-        tcg_gen_mov_i64(psw_addr, o->in2);
-        per_branch(s, false);
-        return DISAS_PC_UPDATED;
+        return help_goto_indirect(s, o->in2);
     } else {
         return DISAS_NEXT;
     }
@@ -1497,9 +1500,7 @@ static DisasJumpType op_bal(DisasContext *s, DisasOps *o)
 {
     save_link_info(s, o);
     if (o->in2) {
-        tcg_gen_mov_i64(psw_addr, o->in2);
-        per_branch(s, false);
-        return DISAS_PC_UPDATED;
+        return help_goto_indirect(s, o->in2);
     } else {
         return DISAS_NEXT;
     }
-- 
2.34.1

[PATCH 07/14] target/s390x: Simplify help_branch

[Richard Henderson]

Always use a tcg branch, instead of movcond.  The movcond
was not a bad idea before PER was added, but since then
we have either 2 or 3 actions to perform on each leg of
the branch, and multiple movcond is inefficient.

Reorder the taken branch to be fallthrough of the tcg branch.

Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/s390x/tcg/translate.c | 164 ++++++++++++-----------------------
 1 file changed, 56 insertions(+), 108 deletions(-)

diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index b1a93070cb..e77660ee50 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -355,25 +355,6 @@ static void per_branch(DisasContext *s, bool to_next)
 #endif
 }
 
-static void per_branch_cond(DisasContext *s, TCGCond cond,
-                            TCGv_i64 arg1, TCGv_i64 arg2)
-{
-#ifndef CONFIG_USER_ONLY
-    if (s->base.tb->flags & FLAG_MASK_PER_BRANCH) {
-        TCGLabel *lab = gen_new_label();
-        tcg_gen_brcond_i64(tcg_invert_cond(cond), arg1, arg2, lab);
-
-        tcg_gen_movi_i64(gbea, s->base.pc_next);
-        gen_helper_per_branch(tcg_env, gbea, psw_addr);
-
-        gen_set_label(lab);
-    } else {
-        TCGv_i64 pc = tcg_constant_i64(s->base.pc_next);
-        tcg_gen_movcond_i64(cond, gbea, arg1, arg2, gbea, pc);
-    }
-#endif
-}
-
 static void per_breaking_event(DisasContext *s)
 {
     tcg_gen_movi_i64(gbea, s->base.pc_next);
@@ -1130,14 +1111,12 @@ static DisasJumpType help_goto_indirect(DisasContext *s, TCGv_i64 dest)
 static DisasJumpType help_branch(DisasContext *s, DisasCompare *c,
                                  bool is_imm, int imm, TCGv_i64 cdest)
 {
-    DisasJumpType ret;
     uint64_t dest = s->base.pc_next + (int64_t)imm * 2;
-    TCGLabel *lab;
+    TCGLabel *lab, *over;
 
     /* Take care of the special cases first.  */
     if (c->cond == TCG_COND_NEVER) {
-        ret = DISAS_NEXT;
-        goto egress;
+        return DISAS_NEXT;
     }
     if (is_imm) {
         /*
@@ -1147,104 +1126,73 @@ static DisasJumpType help_branch(DisasContext *s, DisasCompare *c,
         if (c->cond == TCG_COND_ALWAYS
             || (dest == s->pc_tmp &&
                 !(s->base.tb->flags & FLAG_MASK_PER_BRANCH))) {
-            ret = help_goto_direct(s, dest);
-            goto egress;
+            return help_goto_direct(s, dest);
         }
     } else {
         if (!cdest) {
             /* E.g. bcr %r0 -> no branch.  */
-            ret = DISAS_NEXT;
-            goto egress;
+            return DISAS_NEXT;
         }
         if (c->cond == TCG_COND_ALWAYS) {
-            ret = help_goto_indirect(s, cdest);
-            goto egress;
+            return help_goto_indirect(s, cdest);
         }
     }
 
-    if (use_goto_tb(s, s->pc_tmp)) {
-        if (is_imm && use_goto_tb(s, dest)) {
-            /* Both exits can use goto_tb.  */
-            update_cc_op(s);
+    update_cc_op(s);
 
-            lab = gen_new_label();
-            if (c->is_64) {
-                tcg_gen_brcond_i64(c->cond, c->u.s64.a, c->u.s64.b, lab);
-            } else {
-                tcg_gen_brcond_i32(c->cond, c->u.s32.a, c->u.s32.b, lab);
-            }
-
-            /* Branch not taken.  */
-            tcg_gen_goto_tb(0);
-            tcg_gen_movi_i64(psw_addr, s->pc_tmp);
-            tcg_gen_exit_tb(s->base.tb, 0);
-
-            /* Branch taken.  */
-            gen_set_label(lab);
-            per_breaking_event(s);
-            tcg_gen_goto_tb(1);
-            tcg_gen_movi_i64(psw_addr, dest);
-            tcg_gen_exit_tb(s->base.tb, 1);
-
-            ret = DISAS_NORETURN;
-        } else {
-            /* Fallthru can use goto_tb, but taken branch cannot.  */
-            /* Store taken branch destination before the brcond.  This
-               avoids having to allocate a new local temp to hold it.
-               We'll overwrite this in the not taken case anyway.  */
-            if (!is_imm) {
-                tcg_gen_mov_i64(psw_addr, cdest);
-            }
-
-            lab = gen_new_label();
-            if (c->is_64) {
-                tcg_gen_brcond_i64(c->cond, c->u.s64.a, c->u.s64.b, lab);
-            } else {
-                tcg_gen_brcond_i32(c->cond, c->u.s32.a, c->u.s32.b, lab);
-            }
-
-            /* Branch not taken.  */
-            update_cc_op(s);
-            tcg_gen_goto_tb(0);
-            tcg_gen_movi_i64(psw_addr, s->pc_tmp);
-            tcg_gen_exit_tb(s->base.tb, 0);
-
-            gen_set_label(lab);
-            if (is_imm) {
-                tcg_gen_movi_i64(psw_addr, dest);
-            }
-            per_breaking_event(s);
-            ret = DISAS_PC_UPDATED;
-        }
+    /*
+     * Ensure the taken branch is fall-through of the tcg branch.
+     * This keeps @cdest usage within the extended basic block,
+     * which avoids an otherwise unnecessary spill to the stack.
+     */
+    lab = gen_new_label();
+    if (s->base.tb->flags & FLAG_MASK_PER_BRANCH) {
+        over = gen_new_label();
     } else {
-        /* Fallthru cannot use goto_tb.  This by itself is vanishingly rare.
-           Most commonly we're single-stepping or some other condition that
-           disables all use of goto_tb.  Just update the PC and exit.  */
-
-        TCGv_i64 next = tcg_constant_i64(s->pc_tmp);
-        if (is_imm) {
-            cdest = tcg_constant_i64(dest);
-        }
-
-        if (c->is_64) {
-            tcg_gen_movcond_i64(c->cond, psw_addr, c->u.s64.a, c->u.s64.b,
-                                cdest, next);
-            per_branch_cond(s, c->cond, c->u.s64.a, c->u.s64.b);
-        } else {
-            TCGv_i32 t0 = tcg_temp_new_i32();
-            TCGv_i64 t1 = tcg_temp_new_i64();
-            TCGv_i64 z = tcg_constant_i64(0);
-            tcg_gen_setcond_i32(c->cond, t0, c->u.s32.a, c->u.s32.b);
-            tcg_gen_extu_i32_i64(t1, t0);
-            tcg_gen_movcond_i64(TCG_COND_NE, psw_addr, t1, z, cdest, next);
-            per_branch_cond(s, TCG_COND_NE, t1, z);
-        }
-
-        ret = DISAS_PC_UPDATED;
+        over = NULL;
     }
 
- egress:
-    return ret;
+    if (c->is_64) {
+        tcg_gen_brcond_i64(tcg_invert_cond(c->cond),
+                           c->u.s64.a, c->u.s64.b, lab);
+    } else {
+        tcg_gen_brcond_i32(tcg_invert_cond(c->cond),
+                           c->u.s32.a, c->u.s32.b, lab);
+    }
+
+    /* Branch taken.  */
+    if (is_imm) {
+        tcg_gen_movi_i64(psw_addr, dest);
+    } else {
+        tcg_gen_mov_i64(psw_addr, cdest);
+    }
+    per_branch(s, false);
+
+    if (is_imm && use_goto_tb(s, dest)) {
+        tcg_gen_goto_tb(0);
+        tcg_gen_exit_tb(s->base.tb, 0);
+    } else if (over) {
+        tcg_gen_br(over);
+    } else {
+        tcg_gen_lookup_and_goto_ptr();
+    }
+
+    gen_set_label(lab);
+
+    /* Branch not taken.  */
+    tcg_gen_movi_i64(psw_addr, s->pc_tmp);
+    if (use_goto_tb(s, s->pc_tmp)) {
+        tcg_gen_goto_tb(1);
+        tcg_gen_exit_tb(s->base.tb, 1);
+    }
+
+    if (over) {
+        gen_set_label(over);
+        return DISAS_PC_UPDATED;
+    }
+
+    tcg_gen_lookup_and_goto_ptr();
+    return DISAS_NORETURN;
 }
 
 /* ====================================================================== */
-- 
2.34.1

[PATCH 08/14] target/s390x: Split per_breaking_event from per_branch_*

[Richard Henderson]

The breaking-event-address register is updated regardless
of PER being enabled.

Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/s390x/tcg/translate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index e77660ee50..a384192d15 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -346,8 +346,6 @@ static void update_psw_addr(DisasContext *s)
 static void per_branch(DisasContext *s, bool to_next)
 {
 #ifndef CONFIG_USER_ONLY
-    tcg_gen_movi_i64(gbea, s->base.pc_next);
-
     if (s->base.tb->flags & FLAG_MASK_PER_BRANCH) {
         TCGv_i64 next_pc = to_next ? tcg_constant_i64(s->pc_tmp) : psw_addr;
         gen_helper_per_branch(tcg_env, gbea, next_pc);
@@ -1083,13 +1081,13 @@ struct DisasInsn {
 
 static DisasJumpType help_goto_direct(DisasContext *s, uint64_t dest)
 {
+    per_breaking_event(s);
     if (dest == s->pc_tmp) {
         per_branch(s, true);
         return DISAS_NEXT;
     }
     if (use_goto_tb(s, dest)) {
         update_cc_op(s);
-        per_breaking_event(s);
         tcg_gen_goto_tb(0);
         tcg_gen_movi_i64(psw_addr, dest);
         tcg_gen_exit_tb(s->base.tb, 0);
@@ -1103,6 +1101,7 @@ static DisasJumpType help_goto_direct(DisasContext *s, uint64_t dest)
 
 static DisasJumpType help_goto_indirect(DisasContext *s, TCGv_i64 dest)
 {
+    per_breaking_event(s);
     tcg_gen_mov_i64(psw_addr, dest);
     per_branch(s, false);
     return DISAS_PC_UPDATED;
@@ -1161,6 +1160,7 @@ static DisasJumpType help_branch(DisasContext *s, DisasCompare *c,
     }
 
     /* Branch taken.  */
+    per_breaking_event(s);
     if (is_imm) {
         tcg_gen_movi_i64(psw_addr, dest);
     } else {
-- 
2.34.1

[PATCH 09/14] target/s390x: Raise exception from helper_per_branch

[Richard Henderson]

Drop from argument, since gbea has always been updated with
this address.  Add ilen argument for setting int_pgm_ilen.
Use update_cc_op before calling per_branch.

By raising the exception here, we need not call
per_check_exception later, which means we can clean up the
normal non-exception branch path.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/s390x/helper.h          |  2 +-
 target/s390x/tcg/misc_helper.c | 15 +++++++----
 target/s390x/tcg/translate.c   | 48 ++++++++++++----------------------
 3 files changed, 27 insertions(+), 38 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 96ab71e877..061b379065 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -360,7 +360,7 @@ DEF_HELPER_FLAGS_1(ptlb, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_FLAGS_1(purge, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_3(lra, i64, env, i64, i64)
 DEF_HELPER_FLAGS_3(per_check_exception, TCG_CALL_NO_WG, void, env, i64, i32)
-DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_RWG, void, env, i64, i64)
+DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_WG, void, env, i64, i32)
 DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_RWG, void, env, i64)
 DEF_HELPER_FLAGS_1(per_store_real, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env)
diff --git a/target/s390x/tcg/misc_helper.c b/target/s390x/tcg/misc_helper.c
index b7100c06c0..5b1c512367 100644
--- a/target/s390x/tcg/misc_helper.c
+++ b/target/s390x/tcg/misc_helper.c
@@ -625,13 +625,18 @@ static inline bool get_per_in_range(CPUS390XState *env, uint64_t addr)
     }
 }
 
-void HELPER(per_branch)(CPUS390XState *env, uint64_t from, uint64_t to)
+void HELPER(per_branch)(CPUS390XState *env, uint64_t dest, uint32_t ilen)
 {
-    if (!(env->cregs[9] & PER_CR9_CONTROL_BRANCH_ADDRESS)
-        || get_per_in_range(env, to)) {
-        env->per_address = from;
-        env->per_perc_atmid = PER_CODE_EVENT_BRANCH | get_per_atmid(env);
+    if ((env->cregs[9] & PER_CR9_CONTROL_BRANCH_ADDRESS)
+        && !get_per_in_range(env, dest)) {
+        return;
     }
+
+    env->psw.addr = dest;
+    env->int_pgm_ilen = ilen;
+    env->per_address = env->gbea;
+    env->per_perc_atmid = PER_CODE_EVENT_BRANCH | get_per_atmid(env);
+    per_raise_exception_log(env);
 }
 
 void HELPER(per_ifetch)(CPUS390XState *env, uint64_t addr)
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index a384192d15..4a6ef144b7 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -343,12 +343,11 @@ static void update_psw_addr(DisasContext *s)
     tcg_gen_movi_i64(psw_addr, s->base.pc_next);
 }
 
-static void per_branch(DisasContext *s, bool to_next)
+static void per_branch(DisasContext *s, TCGv_i64 dest)
 {
 #ifndef CONFIG_USER_ONLY
     if (s->base.tb->flags & FLAG_MASK_PER_BRANCH) {
-        TCGv_i64 next_pc = to_next ? tcg_constant_i64(s->pc_tmp) : psw_addr;
-        gen_helper_per_branch(tcg_env, gbea, next_pc);
+        gen_helper_per_branch(tcg_env, dest, tcg_constant_i32(s->ilen));
     }
 #endif
 }
@@ -637,9 +636,6 @@ static void gen_op_calc_cc(DisasContext *s)
 
 static bool use_goto_tb(DisasContext *s, uint64_t dest)
 {
-    if (unlikely(s->base.tb->flags & FLAG_MASK_PER_BRANCH)) {
-        return false;
-    }
     return translator_use_goto_tb(&s->base, dest);
 }
 
@@ -1081,37 +1077,38 @@ struct DisasInsn {
 
 static DisasJumpType help_goto_direct(DisasContext *s, uint64_t dest)
 {
+    update_cc_op(s);
     per_breaking_event(s);
+    per_branch(s, tcg_constant_i64(dest));
+
     if (dest == s->pc_tmp) {
-        per_branch(s, true);
         return DISAS_NEXT;
     }
     if (use_goto_tb(s, dest)) {
-        update_cc_op(s);
         tcg_gen_goto_tb(0);
         tcg_gen_movi_i64(psw_addr, dest);
         tcg_gen_exit_tb(s->base.tb, 0);
         return DISAS_NORETURN;
     } else {
         tcg_gen_movi_i64(psw_addr, dest);
-        per_branch(s, false);
-        return DISAS_PC_UPDATED;
+        return DISAS_PC_CC_UPDATED;
     }
 }
 
 static DisasJumpType help_goto_indirect(DisasContext *s, TCGv_i64 dest)
 {
+    update_cc_op(s);
     per_breaking_event(s);
     tcg_gen_mov_i64(psw_addr, dest);
-    per_branch(s, false);
-    return DISAS_PC_UPDATED;
+    per_branch(s, psw_addr);
+    return DISAS_PC_CC_UPDATED;
 }
 
 static DisasJumpType help_branch(DisasContext *s, DisasCompare *c,
                                  bool is_imm, int imm, TCGv_i64 cdest)
 {
     uint64_t dest = s->base.pc_next + (int64_t)imm * 2;
-    TCGLabel *lab, *over;
+    TCGLabel *lab;
 
     /* Take care of the special cases first.  */
     if (c->cond == TCG_COND_NEVER) {
@@ -1145,12 +1142,6 @@ static DisasJumpType help_branch(DisasContext *s, DisasCompare *c,
      * which avoids an otherwise unnecessary spill to the stack.
      */
     lab = gen_new_label();
-    if (s->base.tb->flags & FLAG_MASK_PER_BRANCH) {
-        over = gen_new_label();
-    } else {
-        over = NULL;
-    }
-
     if (c->is_64) {
         tcg_gen_brcond_i64(tcg_invert_cond(c->cond),
                            c->u.s64.a, c->u.s64.b, lab);
@@ -1166,13 +1157,11 @@ static DisasJumpType help_branch(DisasContext *s, DisasCompare *c,
     } else {
         tcg_gen_mov_i64(psw_addr, cdest);
     }
-    per_branch(s, false);
+    per_branch(s, psw_addr);
 
     if (is_imm && use_goto_tb(s, dest)) {
         tcg_gen_goto_tb(0);
         tcg_gen_exit_tb(s->base.tb, 0);
-    } else if (over) {
-        tcg_gen_br(over);
     } else {
         tcg_gen_lookup_and_goto_ptr();
     }
@@ -1184,15 +1173,9 @@ static DisasJumpType help_branch(DisasContext *s, DisasCompare *c,
     if (use_goto_tb(s, s->pc_tmp)) {
         tcg_gen_goto_tb(1);
         tcg_gen_exit_tb(s->base.tb, 1);
+        return DISAS_NORETURN;
     }
-
-    if (over) {
-        gen_set_label(over);
-        return DISAS_PC_UPDATED;
-    }
-
-    tcg_gen_lookup_and_goto_ptr();
-    return DISAS_NORETURN;
+    return DISAS_PC_CC_UPDATED;
 }
 
 /* ====================================================================== */
@@ -6374,7 +6357,8 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
     }
 
 #ifndef CONFIG_USER_ONLY
-    if (s->base.tb->flags & FLAG_MASK_PER) {
+    if (s->base.tb->flags & (FLAG_MASK_PER_STORE_REAL |
+                             FLAG_MASK_PER_IFETCH)) {
         TCGv_i64 next_pc = psw_addr;
 
         if (ret == DISAS_NEXT || ret == DISAS_TOO_MANY) {
@@ -6404,7 +6388,7 @@ static void s390x_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
 
     dc->cc_op = CC_OP_DYNAMIC;
     dc->ex_value = dc->base.tb->cs_base;
-    dc->exit_to_mainloop = (dc->base.tb->flags & FLAG_MASK_PER) || dc->ex_value;
+    dc->exit_to_mainloop = dc->ex_value;
 }
 
 static void s390x_tr_tb_start(DisasContextBase *db, CPUState *cs)
-- 
2.34.1

[PATCH 10/14] target/s390x: Raise exception from per_store_real

[Richard Henderson]

At this point the instruction is complete and there's nothing
left to do but raise the exception.  With this change we need
not make two helper calls for this event.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/s390x/helper.h          | 2 +-
 target/s390x/tcg/misc_helper.c | 4 +++-
 target/s390x/tcg/translate.c   | 7 ++++---
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 061b379065..5611155ba1 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -362,7 +362,7 @@ DEF_HELPER_3(lra, i64, env, i64, i64)
 DEF_HELPER_FLAGS_3(per_check_exception, TCG_CALL_NO_WG, void, env, i64, i32)
 DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_WG, void, env, i64, i32)
 DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_RWG, void, env, i64)
-DEF_HELPER_FLAGS_1(per_store_real, TCG_CALL_NO_RWG, void, env)
+DEF_HELPER_FLAGS_2(per_store_real, TCG_CALL_NO_WG, noreturn, env, i32)
 DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env)
 
 DEF_HELPER_2(xsch, void, env, i64)
diff --git a/target/s390x/tcg/misc_helper.c b/target/s390x/tcg/misc_helper.c
index 5b1c512367..5f1efc6a32 100644
--- a/target/s390x/tcg/misc_helper.c
+++ b/target/s390x/tcg/misc_helper.c
@@ -660,11 +660,13 @@ void HELPER(per_ifetch)(CPUS390XState *env, uint64_t addr)
     }
 }
 
-void HELPER(per_store_real)(CPUS390XState *env)
+void HELPER(per_store_real)(CPUS390XState *env, uint32_t ilen)
 {
     /* PSW is saved just before calling the helper.  */
     env->per_address = env->psw.addr;
+    env->int_pgm_ilen = ilen;
     env->per_perc_atmid = PER_CODE_EVENT_STORE_REAL | get_per_atmid(env);
+    per_raise_exception_log(env);
 }
 #endif
 
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 4a6ef144b7..041857e720 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -4344,8 +4344,10 @@ static DisasJumpType op_stura(DisasContext *s, DisasOps *o)
     tcg_gen_qemu_st_tl(o->in1, o->in2, MMU_REAL_IDX, s->insn->data);
 
     if (s->base.tb->flags & FLAG_MASK_PER_STORE_REAL) {
+        update_cc_op(s);
         update_psw_addr(s);
-        gen_helper_per_store_real(tcg_env);
+        gen_helper_per_store_real(tcg_env, tcg_constant_i32(s->ilen));
+        return DISAS_NORETURN;
     }
     return DISAS_NEXT;
 }
@@ -6357,8 +6359,7 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
     }
 
 #ifndef CONFIG_USER_ONLY
-    if (s->base.tb->flags & (FLAG_MASK_PER_STORE_REAL |
-                             FLAG_MASK_PER_IFETCH)) {
+    if (s->base.tb->flags & FLAG_MASK_PER_IFETCH) {
         TCGv_i64 next_pc = psw_addr;
 
         if (ret == DISAS_NEXT || ret == DISAS_TOO_MANY) {
-- 
2.34.1

[PATCH 11/14] target/s390x: Fix helper_per_ifetch flags

[Richard Henderson]

CPU state is read on the exception path.

Fixes: 83bb161299c ("target-s390x: PER instruction-fetch nullification event support")
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/s390x/helper.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 5611155ba1..31bd193322 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -361,7 +361,7 @@ DEF_HELPER_FLAGS_1(purge, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_3(lra, i64, env, i64, i64)
 DEF_HELPER_FLAGS_3(per_check_exception, TCG_CALL_NO_WG, void, env, i64, i32)
 DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_WG, void, env, i64, i32)
-DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_RWG, void, env, i64)
+DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_WG, void, env, i64)
 DEF_HELPER_FLAGS_2(per_store_real, TCG_CALL_NO_WG, noreturn, env, i32)
 DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env)
 
-- 
2.34.1

[PATCH 12/14] target/s390x: Simplify per_ifetch, per_check_exception

[Richard Henderson]

Set per_address and ilen in per_ifetch; this is valid for
all PER exceptions and will last until the end of the
instruction.  Therefore we don't need to give the same
data to per_check_exception.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

# Conflicts:
#	target/s390x/tcg/misc_helper.c
---
 target/s390x/helper.h          |  4 ++--
 target/s390x/tcg/misc_helper.c | 23 +++++++++--------------
 target/s390x/tcg/translate.c   | 20 ++++++++++++--------
 3 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 31bd193322..1a8a76abb9 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -359,9 +359,9 @@ DEF_HELPER_FLAGS_4(ipte, TCG_CALL_NO_RWG, void, env, i64, i64, i32)
 DEF_HELPER_FLAGS_1(ptlb, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_FLAGS_1(purge, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_3(lra, i64, env, i64, i64)
-DEF_HELPER_FLAGS_3(per_check_exception, TCG_CALL_NO_WG, void, env, i64, i32)
+DEF_HELPER_FLAGS_1(per_check_exception, TCG_CALL_NO_WG, void, env)
 DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_WG, void, env, i64, i32)
-DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_WG, void, env, i64)
+DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_WG, void, env, i32)
 DEF_HELPER_FLAGS_2(per_store_real, TCG_CALL_NO_WG, noreturn, env, i32)
 DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env)
 
diff --git a/target/s390x/tcg/misc_helper.c b/target/s390x/tcg/misc_helper.c
index 5f1efc6a32..f5e674a26e 100644
--- a/target/s390x/tcg/misc_helper.c
+++ b/target/s390x/tcg/misc_helper.c
@@ -604,12 +604,10 @@ G_NORETURN static void per_raise_exception_log(CPUS390XState *env)
     per_raise_exception(env);
 }
 
-void HELPER(per_check_exception)(CPUS390XState *env, uint64_t next_pc,
-                                 uint32_t ilen)
+void HELPER(per_check_exception)(CPUS390XState *env)
 {
+    /* psw_addr, per_address and int_pgm_ilen are already set. */
     if (unlikely(env->per_perc_atmid)) {
-        env->psw.addr = next_pc;
-        env->int_pgm_ilen = ilen;
         per_raise_exception_log(env);
     }
 }
@@ -639,23 +637,20 @@ void HELPER(per_branch)(CPUS390XState *env, uint64_t dest, uint32_t ilen)
     per_raise_exception_log(env);
 }
 
-void HELPER(per_ifetch)(CPUS390XState *env, uint64_t addr)
+void HELPER(per_ifetch)(CPUS390XState *env, uint32_t ilen)
 {
-    if (get_per_in_range(env, addr)) {
-        env->per_address = addr;
+    if (get_per_in_range(env, env->psw.addr)) {
+        env->per_address = env->psw.addr;
+        env->int_pgm_ilen = ilen;
         env->per_perc_atmid = PER_CODE_EVENT_IFETCH | get_per_atmid(env);
 
         /* If the instruction has to be nullified, trigger the
            exception immediately. */
         if (env->cregs[9] & PER_CR9_EVENT_IFETCH_NULLIFICATION) {
-            CPUState *cs = env_cpu(env);
-
             env->per_perc_atmid |= PER_CODE_EVENT_NULLIFICATION;
-            env->int_pgm_code = PGM_PER;
-            env->int_pgm_ilen = get_ilen(cpu_ldub_code(env, addr));
-
-            cs->exception_index = EXCP_PGM;
-            cpu_loop_exit(cs);
+            qemu_log_mask(CPU_LOG_INT, "PER interrupt before %#" PRIx64 "\n",
+                          env->per_address);
+            per_raise_exception(env);
         }
     }
 }
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 041857e720..7cf2920aec 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -6260,8 +6260,8 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
 
 #ifndef CONFIG_USER_ONLY
     if (s->base.tb->flags & FLAG_MASK_PER_IFETCH) {
-        TCGv_i64 addr = tcg_constant_i64(s->base.pc_next);
-        gen_helper_per_ifetch(tcg_env, addr);
+        /* With ifetch set, psw_addr and cc_op are always up-to-date. */
+        gen_helper_per_ifetch(tcg_env, tcg_constant_i32(s->ilen));
     }
 #endif
 
@@ -6360,14 +6360,18 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
 
 #ifndef CONFIG_USER_ONLY
     if (s->base.tb->flags & FLAG_MASK_PER_IFETCH) {
-        TCGv_i64 next_pc = psw_addr;
-
-        if (ret == DISAS_NEXT || ret == DISAS_TOO_MANY) {
-            next_pc = tcg_constant_i64(s->pc_tmp);
+        switch (ret) {
+        case DISAS_TOO_MANY:
+            s->base.is_jmp = DISAS_PC_CC_UPDATED;
+            /* fall through */
+        case DISAS_NEXT:
+            tcg_gen_movi_i64(psw_addr, s->pc_tmp);
+            break;
+        default:
+            break;
         }
         update_cc_op(s);
-        gen_helper_per_check_exception(tcg_env, next_pc,
-                                       tcg_constant_i32(s->ilen));
+        gen_helper_per_check_exception(tcg_env);
     }
 #endif
 
-- 
2.34.1

[PATCH 13/14] target/s390x: Adjust check of noreturn in translate_one

[Richard Henderson]

If help_op is not set, ret == DISAS_NEXT.
Shift the test up from surrounding help_wout, help_cout
to skipping to out, as we do elsewhere in the function.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/s390x/tcg/translate.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 7cf2920aec..8e9c0847e3 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -6343,14 +6343,15 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
     }
     if (insn->help_op) {
         ret = insn->help_op(s, &o);
+        if (ret == DISAS_NORETURN) {
+            goto out;
+        }
     }
-    if (ret != DISAS_NORETURN) {
-        if (insn->help_wout) {
-            insn->help_wout(s, &o);
-        }
-        if (insn->help_cout) {
-            insn->help_cout(s, &o);
-        }
+    if (insn->help_wout) {
+        insn->help_wout(s, &o);
+    }
+    if (insn->help_cout) {
+        insn->help_cout(s, &o);
     }
 
     /* io should be the last instruction in tb when icount is enabled */
-- 
2.34.1

[PATCH 14/14] tests/tcg/s390x: Add per.S

[Richard Henderson]

Add a small test to avoid regressions.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tests/tcg/s390x/Makefile.softmmu-target |  1 +
 tests/tcg/s390x/per.S                   | 82 +++++++++++++++++++++++++
 2 files changed, 83 insertions(+)
 create mode 100644 tests/tcg/s390x/per.S

diff --git a/tests/tcg/s390x/Makefile.softmmu-target b/tests/tcg/s390x/Makefile.softmmu-target
index 1a1f088b28..80159cccf5 100644
--- a/tests/tcg/s390x/Makefile.softmmu-target
+++ b/tests/tcg/s390x/Makefile.softmmu-target
@@ -25,6 +25,7 @@ ASM_TESTS =                                                                    \
     lpswe-early                                                                \
     lra                                                                        \
     mc                                                                         \
+    per                                                                        \
     precise-smc-softmmu                                                        \
     ssm-early                                                                  \
     stosm-early                                                                \
diff --git a/tests/tcg/s390x/per.S b/tests/tcg/s390x/per.S
new file mode 100644
index 0000000000..79e704a6ff
--- /dev/null
+++ b/tests/tcg/s390x/per.S
@@ -0,0 +1,82 @@
+	.org	0x8d
+ilc:
+	.org	0x8e
+program_interruption_code:
+	.org	0x96
+per_code:
+	.org	0x98
+per_address:
+	.org	0x150
+program_old_psw:
+	.org	0x1d0
+program_new_psw:
+	.quad	0, pgm_handler
+
+	.org	0x200			/* exit lowcore */
+
+per_on_psw:
+	.quad	0x4000000000000000, start_per
+per_on_regs:
+	.quad	0x80000000, 0, -1	/* successful-branching everywhere */
+per_off_regs:
+	.quad	0, 0 ,0
+success_psw:
+	.quad	0x2000000000000, 0xfff	/* see is_special_wait_psw() */
+failure_psw:
+	.quad	0x2000000000000, 0	/* disabled wait */
+
+	.org	0x2000			/* exit lowcore pages */
+
+	.globl _start
+_start:
+	lpswe	per_on_psw
+start_per:
+	lctlg	%c9, %c11, per_on_regs
+
+/* Test unconditional relative branch. */
+	larl	%r0, j1
+	larl	%r1, d1
+	lhi	%r2, 0
+j1:	j	d1
+	lpswe	failure_psw
+d1:
+
+/* Test unconditional indirect branch. */
+	larl	%r0, j2
+	larl	%r1, d2
+j2:	br	%r1
+	lpswe	failure_psw
+d2:
+
+/* Test conditional relative branch. */
+	larl	%r0, j3
+	larl	%r1, d3
+	clr	%r1, %r2	/* d3 != 0 */
+j3:	jne	d3
+	lpswe	failure_psw
+d3:
+
+/* Test conditional register branch. */
+	larl	%r0, j4
+	larl	%r1, d4
+	clr	%r1, %r2	/* d4 != 0 */
+j4:	bner	%r1
+	lpswe	failure_psw
+d4:
+
+/* Success! */
+	nop
+	lpswe	success_psw
+
+pgm_handler:
+	chhsi	program_interruption_code, 0x80	/* PER event? */
+	jne	fail
+	cli	per_code, 0x80		/* successful-branching event? */
+	jne	fail
+	clg	%r0, per_address	/* per_address == jump insn? */
+	jne	fail
+	clg	%r1, program_old_psw+8	/* psw.addr updated to dest? */
+	jne	fail
+	lpswe	program_old_psw
+fail:
+	lpswe	failure_psw
-- 
2.34.1

Re: [PATCH 02/14] target/s390x: Move cpu_get_tb_cpu_state out of line

[Thomas Huth]

On 02/05/2024 07.44, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/s390x/cpu.h | 23 ++---------------------
>   target/s390x/cpu.c | 22 ++++++++++++++++++++++
>   2 files changed, 24 insertions(+), 21 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 03/14] target/s390x: Update CR9 bits

[Thomas Huth]

On 02/05/2024 07.44, Richard Henderson wrote:
> Update from the PoO 14th edition.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/s390x/cpu.h             | 18 +++++++++++-------
>   target/s390x/tcg/misc_helper.c |  2 +-
>   2 files changed, 12 insertions(+), 8 deletions(-)


Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 11/14] target/s390x: Fix helper_per_ifetch flags

[David Hildenbrand]

On 02.05.24 07:44, Richard Henderson wrote:
> CPU state is read on the exception path.
> 
> Fixes: 83bb161299c ("target-s390x: PER instruction-fetch nullification event support")
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/s390x/helper.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/s390x/helper.h b/target/s390x/helper.h
> index 5611155ba1..31bd193322 100644
> --- a/target/s390x/helper.h
> +++ b/target/s390x/helper.h
> @@ -361,7 +361,7 @@ DEF_HELPER_FLAGS_1(purge, TCG_CALL_NO_RWG, void, env)
>   DEF_HELPER_3(lra, i64, env, i64, i64)
>   DEF_HELPER_FLAGS_3(per_check_exception, TCG_CALL_NO_WG, void, env, i64, i32)
>   DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_WG, void, env, i64, i32)
> -DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_RWG, void, env, i64)
> +DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_WG, void, env, i64)
>   DEF_HELPER_FLAGS_2(per_store_real, TCG_CALL_NO_WG, noreturn, env, i32)
>   DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env)
>   

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

Re: [PATCH 14/14] tests/tcg/s390x: Add per.S

[Ilya Leoshkevich]

On Wed, 2024-05-01 at 22:44 -0700, Richard Henderson wrote:
> Add a small test to avoid regressions.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tests/tcg/s390x/Makefile.softmmu-target |  1 +
>  tests/tcg/s390x/per.S                   | 82
> +++++++++++++++++++++++++
>  2 files changed, 83 insertions(+)
>  create mode 100644 tests/tcg/s390x/per.S

Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
Tested-by: Ilya Leoshkevich <iii@linux.ibm.com>

Re: [PATCH 01/14] target/s390x: Do not use unwind for per_check_exception

[Ilya Leoshkevich]

On Wed, 2024-05-01 at 22:44 -0700, Richard Henderson wrote:
> Using exception unwind via tcg_s390_program_interrupt,
> we discard the current value of psw.addr, which discards
> the result of a branch.
> 
> Pass in the address of the next instruction, which may
> not be sequential.  Pass in ilen, which we would have
> gotten from unwind and is passed to the exception handler.
> Sync cc_op before the call, which we would have gotten
> from unwind.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/s390x/helper.h          |  2 +-
>  target/s390x/tcg/excp_helper.c |  2 +-
>  target/s390x/tcg/misc_helper.c | 23 ++++++++++++++++++++---
>  target/s390x/tcg/translate.c   | 13 +++++++------
>  4 files changed, 29 insertions(+), 11 deletions(-)

IIUC this fixes the contents of old PSW so that it matches the
following wording from PoP:

  When a PER interruption is caused by branch-
  ing, the PER address identifies the branch
  instruction (or execute-type instruction, as appro-
  priate), whereas the old PSW points to the next
  instruction to be executed.

whereas currently old PSW = unwind(GETPC()) = branch instruction.

Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>

Re: [PATCH 02/14] target/s390x: Move cpu_get_tb_cpu_state out of line

[Ilya Leoshkevich]

On Wed, 2024-05-01 at 22:44 -0700, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/s390x/cpu.h | 23 ++---------------------
>  target/s390x/cpu.c | 22 ++++++++++++++++++++++
>  2 files changed, 24 insertions(+), 21 deletions(-)

Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>

Re: [PATCH 03/14] target/s390x: Update CR9 bits

[Ilya Leoshkevich]

On Wed, 2024-05-01 at 22:44 -0700, Richard Henderson wrote:
> Update from the PoO 14th edition.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/s390x/cpu.h             | 18 +++++++++++-------
>  target/s390x/tcg/misc_helper.c |  2 +-
>  2 files changed, 12 insertions(+), 8 deletions(-)

Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>

Re: [PATCH 04/14] target/s390x: Record separate PER bits in TB flags

[Ilya Leoshkevich]

On Wed, 2024-05-01 at 22:44 -0700, Richard Henderson wrote:
> Record successful-branching, instruction-fetching, and
> store-using-real-address.  The other PER bits are not used
> during translation.  Having checked these at translation time,
> we can remove runtime tests from the helpers.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/s390x/cpu.h             | 42 ++++++++++++++++++++++++--------
> --
>  target/s390x/cpu.c             | 22 ++++++++++++++----
>  target/s390x/tcg/misc_helper.c | 21 +++++++----------
>  target/s390x/tcg/translate.c   | 10 ++++----
>  4 files changed, 61 insertions(+), 34 deletions(-)

Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>

Re: [PATCH 09/14] target/s390x: Raise exception from helper_per_branch

[Ilya Leoshkevich]

On Wed, 2024-05-01 at 22:44 -0700, Richard Henderson wrote:
> Drop from argument, since gbea has always been updated with
> this address.  Add ilen argument for setting int_pgm_ilen.
> Use update_cc_op before calling per_branch.
> 
> By raising the exception here, we need not call
> per_check_exception later, which means we can clean up the
> normal non-exception branch path.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/s390x/helper.h          |  2 +-
>  target/s390x/tcg/misc_helper.c | 15 +++++++----
>  target/s390x/tcg/translate.c   | 48 ++++++++++++--------------------
> --
>  3 files changed, 27 insertions(+), 38 deletions(-)

[...]

>  static bool use_goto_tb(DisasContext *s, uint64_t dest)
>  {
> -    if (unlikely(s->base.tb->flags & FLAG_MASK_PER_BRANCH)) {
> -        return false;
> -    }

Why was this required in the first place and why can we remove it now?

Re: [PATCH 09/14] target/s390x: Raise exception from helper_per_branch

[Richard Henderson]

On 5/22/24 15:45, Ilya Leoshkevich wrote:
> On Wed, 2024-05-01 at 22:44 -0700, Richard Henderson wrote:
>> Drop from argument, since gbea has always been updated with
>> this address.  Add ilen argument for setting int_pgm_ilen.
>> Use update_cc_op before calling per_branch.
>>
>> By raising the exception here, we need not call
>> per_check_exception later, which means we can clean up the
>> normal non-exception branch path.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   target/s390x/helper.h          |  2 +-
>>   target/s390x/tcg/misc_helper.c | 15 +++++++----
>>   target/s390x/tcg/translate.c   | 48 ++++++++++++--------------------
>> --
>>   3 files changed, 27 insertions(+), 38 deletions(-)
> 
> [...]
> 
>>   static bool use_goto_tb(DisasContext *s, uint64_t dest)
>>   {
>> -    if (unlikely(s->base.tb->flags & FLAG_MASK_PER_BRANCH)) {
>> -        return false;
>> -    }
> 
> Why was this required in the first place and why can we remove it now?
> 

This was required because of the structure of the code: the PER exception was only raised 
from translate_one, and therefore we could not allow the TB to end beforehand with goto_tb.

We can eliminate this now because we're raising the PER branch exception before attempting 
goto_tb.


r~

Re: [PATCH 12/14] target/s390x: Simplify per_ifetch, per_check_exception

[Thomas Huth]

On 02/05/2024 07.44, Richard Henderson wrote:
> Set per_address and ilen in per_ifetch; this is valid for
> all PER exceptions and will last until the end of the
> instruction.  Therefore we don't need to give the same
> data to per_check_exception.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> 
> # Conflicts:
> #	target/s390x/tcg/misc_helper.c
> ---
>   target/s390x/helper.h          |  4 ++--
>   target/s390x/tcg/misc_helper.c | 23 +++++++++--------------
>   target/s390x/tcg/translate.c   | 20 ++++++++++++--------
>   3 files changed, 23 insertions(+), 24 deletions(-)
> 
> diff --git a/target/s390x/helper.h b/target/s390x/helper.h
> index 31bd193322..1a8a76abb9 100644
> --- a/target/s390x/helper.h
> +++ b/target/s390x/helper.h
> @@ -359,9 +359,9 @@ DEF_HELPER_FLAGS_4(ipte, TCG_CALL_NO_RWG, void, env, i64, i64, i32)
>   DEF_HELPER_FLAGS_1(ptlb, TCG_CALL_NO_RWG, void, env)
>   DEF_HELPER_FLAGS_1(purge, TCG_CALL_NO_RWG, void, env)
>   DEF_HELPER_3(lra, i64, env, i64, i64)
> -DEF_HELPER_FLAGS_3(per_check_exception, TCG_CALL_NO_WG, void, env, i64, i32)
> +DEF_HELPER_FLAGS_1(per_check_exception, TCG_CALL_NO_WG, void, env)
>   DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_WG, void, env, i64, i32)
> -DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_WG, void, env, i64)
> +DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_WG, void, env, i32)
>   DEF_HELPER_FLAGS_2(per_store_real, TCG_CALL_NO_WG, noreturn, env, i32)
>   DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env)
>   
> diff --git a/target/s390x/tcg/misc_helper.c b/target/s390x/tcg/misc_helper.c
> index 5f1efc6a32..f5e674a26e 100644
> --- a/target/s390x/tcg/misc_helper.c
> +++ b/target/s390x/tcg/misc_helper.c
> @@ -604,12 +604,10 @@ G_NORETURN static void per_raise_exception_log(CPUS390XState *env)
>       per_raise_exception(env);
>   }
>   
> -void HELPER(per_check_exception)(CPUS390XState *env, uint64_t next_pc,
> -                                 uint32_t ilen)
> +void HELPER(per_check_exception)(CPUS390XState *env)
>   {
> +    /* psw_addr, per_address and int_pgm_ilen are already set. */
>       if (unlikely(env->per_perc_atmid)) {
> -        env->psw.addr = next_pc;
> -        env->int_pgm_ilen = ilen;
>           per_raise_exception_log(env);
>       }
>   }
> @@ -639,23 +637,20 @@ void HELPER(per_branch)(CPUS390XState *env, uint64_t dest, uint32_t ilen)
>       per_raise_exception_log(env);
>   }
>   
> -void HELPER(per_ifetch)(CPUS390XState *env, uint64_t addr)
> +void HELPER(per_ifetch)(CPUS390XState *env, uint32_t ilen)
>   {
> -    if (get_per_in_range(env, addr)) {
> -        env->per_address = addr;
> +    if (get_per_in_range(env, env->psw.addr)) {
> +        env->per_address = env->psw.addr;
> +        env->int_pgm_ilen = ilen;
>           env->per_perc_atmid = PER_CODE_EVENT_IFETCH | get_per_atmid(env);
>   
>           /* If the instruction has to be nullified, trigger the
>              exception immediately. */
>           if (env->cregs[9] & PER_CR9_EVENT_IFETCH_NULLIFICATION) {
> -            CPUState *cs = env_cpu(env);
> -
>               env->per_perc_atmid |= PER_CODE_EVENT_NULLIFICATION;
> -            env->int_pgm_code = PGM_PER;
> -            env->int_pgm_ilen = get_ilen(cpu_ldub_code(env, addr));
> -
> -            cs->exception_index = EXCP_PGM;
> -            cpu_loop_exit(cs);
> +            qemu_log_mask(CPU_LOG_INT, "PER interrupt before %#" PRIx64 "\n",
> +                          env->per_address);

FYI, checkpatch.pl complains:

ERROR: Don't use '#' flag of printf format ('%#') in format strings, use 
'0x' prefix instead
#84: FILE: target/s390x/tcg/misc_helper.c:651:
+            qemu_log_mask(CPU_LOG_INT, "PER interrupt before %#" PRIx64

I'll fix it up while picking up your patch.

  Thomas