qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes
@ 2024-05-29 10:54 Thomas Huth
  2024-05-29 10:54 ` [PULL 01/22] target/s390x: Do not use unwind for per_check_exception Thomas Huth
                   ` (22 more replies)
  0 siblings, 23 replies; 24+ messages in thread
From: Thomas Huth @ 2024-05-29 10:54 UTC (permalink / raw)
  To: qemu-devel, Richard Henderson; +Cc: qemu-s390x

 Hi Richard!

The following changes since commit 79d7475f39f1b0f05fcb159f5cdcbf162340dc7e:

  Merge tag 'pull-block-jobs-2024-04-29-v2' of https://gitlab.com/vsementsov/qemu into staging (2024-05-28 11:28:34 -0700)

are available in the Git repository at:

  https://gitlab.com/thuth/qemu.git tags/pull-request-2024-05-29

for you to fetch changes up to b04091393e6a71065aee6c91b2566f2dec95a4c9:

  qapi: Do not cast function pointers (2024-05-29 12:41:56 +0200)

----------------------------------------------------------------
* Fix and improve PER emulation on s390x
* Fix problems of the build-oss-fuzz CI job
* Fix broken update-linux-headers.sh script
* Fixes for compiling with -fsanitize=undefined on latest Clang versions

----------------------------------------------------------------
Akihiko Odaki (3):
      qemu-keymap: Make references to allocations static
      lockable: Do not cast function pointers
      qapi: Do not cast function pointers

Alexander Bulekov (2):
      fuzz: specify audiodev for usb-audio
      fuzz: disable leak-detection for oss-fuzz builds

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

Thomas Huth (3):
      hw/s390x: Remove unused macro VMSTATE_ADAPTER_ROUTES
      scripts/update-linux-headers.sh: Remove temporary directory inbetween
      scripts/update-linux-headers.sh: Fix the path of setup_data.h

 include/hw/s390x/s390_flic.h            |   3 -
 include/qapi/clone-visitor.h            |  37 +++--
 include/qemu/lockable.h                 |  23 ++-
 target/s390x/cpu.h                      |  85 +++++------
 target/s390x/helper.h                   |   8 +-
 tests/qtest/fuzz/generic_fuzz_configs.h |   3 +-
 qapi/qapi-clone-visitor.c               |  30 +---
 qemu-keymap.c                           |   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            | 242 ++++++++++++--------------------
 scripts/oss-fuzz/build.sh               |   1 +
 scripts/update-linux-headers.sh         |   3 +-
 tests/tcg/s390x/Makefile.softmmu-target |   1 +
 tests/tcg/s390x/per.S                   |  82 +++++++++++
 16 files changed, 355 insertions(+), 277 deletions(-)
 create mode 100644 tests/tcg/s390x/per.S



^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PULL 01/22] target/s390x: Do not use unwind for per_check_exception
  2024-05-29 10:54 [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Thomas Huth
@ 2024-05-29 10:54 ` Thomas Huth
  2024-05-29 10:54 ` [PULL 02/22] target/s390x: Move cpu_get_tb_cpu_state out of line Thomas Huth
                   ` (21 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2024-05-29 10:54 UTC (permalink / raw)
  To: qemu-devel, Richard Henderson; +Cc: qemu-s390x, Ilya Leoshkevich

From: Richard Henderson <richard.henderson@linaro.org>

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>
Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-ID: <20240502054417.234340-2-richard.henderson@linaro.org>
[thuth: Silence checkpatch.pl errors]
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 target/s390x/helper.h          |  2 +-
 target/s390x/tcg/excp_helper.c |  2 +-
 target/s390x/tcg/misc_helper.c | 23 ++++++++++++++++++++---
 target/s390x/tcg/translate.c   | 11 ++++++-----
 4 files changed, 28 insertions(+), 10 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..7c94468392 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)
+static G_NORETURN 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));
+}
+
+static G_NORETURN void per_raise_exception_log(CPUS390XState *env)
+{
+    qemu_log_mask(CPU_LOG_INT, "PER interrupt after 0x%" 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 ebd96abe6c..4c3ff1931b 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -6424,13 +6424,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.  */
+        TCGv_i64 next_pc = psw_addr;
+
         if (ret == DISAS_NEXT || ret == DISAS_TOO_MANY) {
-            tcg_gen_movi_i64(psw_addr, s->pc_tmp);
+            next_pc = tcg_constant_i64(s->pc_tmp);
         }
-
-        /* Call the helper to check for a possible PER exception.  */
-        gen_helper_per_check_exception(tcg_env);
+        update_cc_op(s);
+        gen_helper_per_check_exception(tcg_env, next_pc,
+                                       tcg_constant_i32(s->ilen));
     }
 #endif
 
-- 
2.45.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PULL 02/22] target/s390x: Move cpu_get_tb_cpu_state out of line
  2024-05-29 10:54 [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Thomas Huth
  2024-05-29 10:54 ` [PULL 01/22] target/s390x: Do not use unwind for per_check_exception Thomas Huth
@ 2024-05-29 10:54 ` Thomas Huth
  2024-05-29 10:54 ` [PULL 03/22] target/s390x: Update CR9 bits Thomas Huth
                   ` (20 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2024-05-29 10:54 UTC (permalink / raw)
  To: qemu-devel, Richard Henderson; +Cc: qemu-s390x, Ilya Leoshkevich

From: Richard Henderson <richard.henderson@linaro.org>

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-ID: <20240502054417.234340-3-richard.henderson@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 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.45.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PULL 03/22] target/s390x: Update CR9 bits
  2024-05-29 10:54 [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Thomas Huth
  2024-05-29 10:54 ` [PULL 01/22] target/s390x: Do not use unwind for per_check_exception Thomas Huth
  2024-05-29 10:54 ` [PULL 02/22] target/s390x: Move cpu_get_tb_cpu_state out of line Thomas Huth
@ 2024-05-29 10:54 ` Thomas Huth
  2024-05-29 10:54 ` [PULL 04/22] target/s390x: Record separate PER bits in TB flags Thomas Huth
                   ` (19 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2024-05-29 10:54 UTC (permalink / raw)
  To: qemu-devel, Richard Henderson; +Cc: qemu-s390x, Ilya Leoshkevich

From: Richard Henderson <richard.henderson@linaro.org>

Update from the PoO 14th edition.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-ID: <20240502054417.234340-4-richard.henderson@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 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 7c94468392..0482442458 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.45.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PULL 04/22] target/s390x: Record separate PER bits in TB flags
  2024-05-29 10:54 [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Thomas Huth
                   ` (2 preceding siblings ...)
  2024-05-29 10:54 ` [PULL 03/22] target/s390x: Update CR9 bits Thomas Huth
@ 2024-05-29 10:54 ` Thomas Huth
  2024-05-29 10:54 ` [PULL 05/22] target/s390x: Disable conditional branch-to-next for PER Thomas Huth
                   ` (18 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2024-05-29 10:54 UTC (permalink / raw)
  To: qemu-devel, Richard Henderson; +Cc: qemu-s390x, Ilya Leoshkevich

From: Richard Henderson <richard.henderson@linaro.org>

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>
Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-ID: <20240502054417.234340-5-richard.henderson@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 target/s390x/cpu.h             | 44 ++++++++++++++++++++++++----------
 target/s390x/cpu.c             | 22 +++++++++++++----
 target/s390x/tcg/misc_helper.c | 21 +++++++---------
 target/s390x/tcg/translate.c   | 10 ++++----
 4 files changed, 62 insertions(+), 35 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)
-
-/* 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)
+#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
+
+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 0482442458..3f94f71437 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 4c3ff1931b..de98115c4d 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -346,7 +346,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);
     }
@@ -357,7 +357,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);
 
@@ -656,7 +656,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);
@@ -4409,7 +4409,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);
     }
@@ -6323,7 +6323,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.45.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PULL 05/22] target/s390x: Disable conditional branch-to-next for PER
  2024-05-29 10:54 [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Thomas Huth
                   ` (3 preceding siblings ...)
  2024-05-29 10:54 ` [PULL 04/22] target/s390x: Record separate PER bits in TB flags Thomas Huth
@ 2024-05-29 10:54 ` Thomas Huth
  2024-05-29 10:54 ` [PULL 06/22] target/s390x: Introduce help_goto_indirect Thomas Huth
                   ` (17 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2024-05-29 10:54 UTC (permalink / raw)
  To: qemu-devel, Richard Henderson; +Cc: qemu-s390x, Ilya Leoshkevich

From: Richard Henderson <richard.henderson@linaro.org>

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>
Message-ID: <20240502054417.234340-6-richard.henderson@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 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 de98115c4d..07620948ae 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -1131,13 +1131,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.45.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PULL 06/22] target/s390x: Introduce help_goto_indirect
  2024-05-29 10:54 [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Thomas Huth
                   ` (4 preceding siblings ...)
  2024-05-29 10:54 ` [PULL 05/22] target/s390x: Disable conditional branch-to-next for PER Thomas Huth
@ 2024-05-29 10:54 ` Thomas Huth
  2024-05-29 10:54 ` [PULL 07/22] target/s390x: Simplify help_branch Thomas Huth
                   ` (16 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2024-05-29 10:54 UTC (permalink / raw)
  To: qemu-devel, Richard Henderson
  Cc: qemu-s390x, Philippe Mathieu-Daudé, Ilya Leoshkevich

From: Richard Henderson <richard.henderson@linaro.org>

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>
Message-ID: <20240502054417.234340-7-richard.henderson@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 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 07620948ae..7ae928c3b0 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -1118,6 +1118,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)
 {
@@ -1148,9 +1155,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;
         }
     }
@@ -1463,9 +1468,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;
     }
@@ -1495,9 +1498,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.45.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PULL 07/22] target/s390x: Simplify help_branch
  2024-05-29 10:54 [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Thomas Huth
                   ` (5 preceding siblings ...)
  2024-05-29 10:54 ` [PULL 06/22] target/s390x: Introduce help_goto_indirect Thomas Huth
@ 2024-05-29 10:54 ` Thomas Huth
  2024-05-29 10:54 ` [PULL 08/22] target/s390x: Split per_breaking_event from per_branch_* Thomas Huth
                   ` (15 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2024-05-29 10:54 UTC (permalink / raw)
  To: qemu-devel, Richard Henderson; +Cc: qemu-s390x, Ilya Leoshkevich

From: Richard Henderson <richard.henderson@linaro.org>

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>
Message-ID: <20240502054417.234340-8-richard.henderson@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 target/s390x/tcg/translate.c | 152 ++++++++++++-----------------------
 1 file changed, 50 insertions(+), 102 deletions(-)

diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 7ae928c3b0..78066aaf84 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -353,25 +353,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);
@@ -1128,14 +1109,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) {
         /*
@@ -1145,104 +1124,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);
-
-            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);
+    update_cc_op(s);
 
-            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);
-            }
+    /*
+     * 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 {
+        over = NULL;
+    }
 
-            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);
-            }
+    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 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);
+    /* Branch taken.  */
+    if (is_imm) {
+        tcg_gen_movi_i64(psw_addr, dest);
+    } else {
+        tcg_gen_mov_i64(psw_addr, cdest);
+    }
+    per_branch(s, false);
 
-            gen_set_label(lab);
-            if (is_imm) {
-                tcg_gen_movi_i64(psw_addr, dest);
-            }
-            per_breaking_event(s);
-            ret = DISAS_PC_UPDATED;
-        }
+    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 {
-        /* 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.  */
+        tcg_gen_lookup_and_goto_ptr();
+    }
 
-        TCGv_i64 next = tcg_constant_i64(s->pc_tmp);
-        if (is_imm) {
-            cdest = tcg_constant_i64(dest);
-        }
+    gen_set_label(lab);
 
-        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);
-        }
+    /* 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);
+    }
 
-        ret = DISAS_PC_UPDATED;
+    if (over) {
+        gen_set_label(over);
+        return DISAS_PC_UPDATED;
     }
 
- egress:
-    return ret;
+    tcg_gen_lookup_and_goto_ptr();
+    return DISAS_NORETURN;
 }
 
 /* ====================================================================== */
-- 
2.45.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PULL 08/22] target/s390x: Split per_breaking_event from per_branch_*
  2024-05-29 10:54 [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Thomas Huth
                   ` (6 preceding siblings ...)
  2024-05-29 10:54 ` [PULL 07/22] target/s390x: Simplify help_branch Thomas Huth
@ 2024-05-29 10:54 ` Thomas Huth
  2024-05-29 10:54 ` [PULL 09/22] target/s390x: Raise exception from helper_per_branch Thomas Huth
                   ` (14 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2024-05-29 10:54 UTC (permalink / raw)
  To: qemu-devel, Richard Henderson; +Cc: qemu-s390x, Ilya Leoshkevich

From: Richard Henderson <richard.henderson@linaro.org>

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>
Message-ID: <20240502054417.234340-9-richard.henderson@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 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 78066aaf84..10d5e87bb4 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -344,8 +344,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);
@@ -1081,13 +1079,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);
@@ -1101,6 +1099,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;
@@ -1159,6 +1158,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.45.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PULL 09/22] target/s390x: Raise exception from helper_per_branch
  2024-05-29 10:54 [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Thomas Huth
                   ` (7 preceding siblings ...)
  2024-05-29 10:54 ` [PULL 08/22] target/s390x: Split per_breaking_event from per_branch_* Thomas Huth
@ 2024-05-29 10:54 ` Thomas Huth
  2024-05-29 10:54 ` [PULL 10/22] target/s390x: Raise exception from per_store_real Thomas Huth
                   ` (13 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2024-05-29 10:54 UTC (permalink / raw)
  To: qemu-devel, Richard Henderson; +Cc: qemu-s390x

From: Richard Henderson <richard.henderson@linaro.org>

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>
Message-ID: <20240502054417.234340-10-richard.henderson@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 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 3f94f71437..974d14703c 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 10d5e87bb4..de029185e0 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -341,12 +341,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
 }
@@ -635,9 +634,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);
 }
 
@@ -1079,37 +1075,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) {
@@ -1143,12 +1140,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);
@@ -1164,13 +1155,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();
     }
@@ -1182,15 +1171,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;
 }
 
 /* ====================================================================== */
@@ -6372,7 +6355,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) {
@@ -6402,7 +6386,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.45.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PULL 10/22] target/s390x: Raise exception from per_store_real
  2024-05-29 10:54 [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Thomas Huth
                   ` (8 preceding siblings ...)
  2024-05-29 10:54 ` [PULL 09/22] target/s390x: Raise exception from helper_per_branch Thomas Huth
@ 2024-05-29 10:54 ` Thomas Huth
  2024-05-29 10:54 ` [PULL 11/22] target/s390x: Fix helper_per_ifetch flags Thomas Huth
                   ` (12 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2024-05-29 10:54 UTC (permalink / raw)
  To: qemu-devel, Richard Henderson; +Cc: qemu-s390x

From: Richard Henderson <richard.henderson@linaro.org>

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>
Message-ID: <20240502054417.234340-11-richard.henderson@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 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 974d14703c..43cacc448f 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 de029185e0..5bb15a46e0 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -4342,8 +4342,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;
 }
@@ -6355,8 +6357,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.45.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PULL 11/22] target/s390x: Fix helper_per_ifetch flags
  2024-05-29 10:54 [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Thomas Huth
                   ` (9 preceding siblings ...)
  2024-05-29 10:54 ` [PULL 10/22] target/s390x: Raise exception from per_store_real Thomas Huth
@ 2024-05-29 10:54 ` Thomas Huth
  2024-05-29 10:54 ` [PULL 12/22] target/s390x: Simplify per_ifetch, per_check_exception Thomas Huth
                   ` (11 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2024-05-29 10:54 UTC (permalink / raw)
  To: qemu-devel, Richard Henderson; +Cc: qemu-s390x, David Hildenbrand

From: Richard Henderson <richard.henderson@linaro.org>

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>
Reviewed-by: David Hildenbrand <david@redhat.com>
Message-ID: <20240502054417.234340-12-richard.henderson@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 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.45.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PULL 12/22] target/s390x: Simplify per_ifetch, per_check_exception
  2024-05-29 10:54 [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Thomas Huth
                   ` (10 preceding siblings ...)
  2024-05-29 10:54 ` [PULL 11/22] target/s390x: Fix helper_per_ifetch flags Thomas Huth
@ 2024-05-29 10:54 ` Thomas Huth
  2024-05-29 10:54 ` [PULL 13/22] target/s390x: Adjust check of noreturn in translate_one Thomas Huth
                   ` (10 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2024-05-29 10:54 UTC (permalink / raw)
  To: qemu-devel, Richard Henderson; +Cc: qemu-s390x

From: Richard Henderson <richard.henderson@linaro.org>

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>
Message-ID: <20240502054417.234340-13-richard.henderson@linaro.org>
[thuth: Silence checkpatch.pl errors]
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 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 43cacc448f..303f86d363 100644
--- a/target/s390x/tcg/misc_helper.c
+++ b/target/s390x/tcg/misc_helper.c
@@ -604,12 +604,10 @@ static G_NORETURN 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 0x%" PRIx64 "\n",
+                          env->per_address);
+            per_raise_exception(env);
         }
     }
 }
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 5bb15a46e0..c9a5a1687e 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -6258,8 +6258,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
 
@@ -6358,14 +6358,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.45.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PULL 13/22] target/s390x: Adjust check of noreturn in translate_one
  2024-05-29 10:54 [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Thomas Huth
                   ` (11 preceding siblings ...)
  2024-05-29 10:54 ` [PULL 12/22] target/s390x: Simplify per_ifetch, per_check_exception Thomas Huth
@ 2024-05-29 10:54 ` Thomas Huth
  2024-05-29 10:54 ` [PULL 14/22] tests/tcg/s390x: Add per.S Thomas Huth
                   ` (9 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2024-05-29 10:54 UTC (permalink / raw)
  To: qemu-devel, Richard Henderson; +Cc: qemu-s390x

From: Richard Henderson <richard.henderson@linaro.org>

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>
Message-ID: <20240502054417.234340-14-richard.henderson@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 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 c9a5a1687e..c81e035dea 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -6341,15 +6341,16 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
     }
     if (insn->help_op) {
         ret = insn->help_op(s, &o);
-    }
-    if (ret != DISAS_NORETURN) {
-        if (insn->help_wout) {
-            insn->help_wout(s, &o);
-        }
-        if (insn->help_cout) {
-            insn->help_cout(s, &o);
+        if (ret == DISAS_NORETURN) {
+            goto out;
         }
     }
+    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 */
     if (unlikely(icount && ret == DISAS_NEXT)) {
-- 
2.45.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PULL 14/22] tests/tcg/s390x: Add per.S
  2024-05-29 10:54 [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Thomas Huth
                   ` (12 preceding siblings ...)
  2024-05-29 10:54 ` [PULL 13/22] target/s390x: Adjust check of noreturn in translate_one Thomas Huth
@ 2024-05-29 10:54 ` Thomas Huth
  2024-05-29 10:54 ` [PULL 15/22] fuzz: specify audiodev for usb-audio Thomas Huth
                   ` (8 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2024-05-29 10:54 UTC (permalink / raw)
  To: qemu-devel, Richard Henderson; +Cc: qemu-s390x, Ilya Leoshkevich

From: Richard Henderson <richard.henderson@linaro.org>

Add a small test to avoid regressions.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
Tested-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-ID: <20240502054417.234340-15-richard.henderson@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 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.45.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PULL 15/22] fuzz: specify audiodev for usb-audio
  2024-05-29 10:54 [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Thomas Huth
                   ` (13 preceding siblings ...)
  2024-05-29 10:54 ` [PULL 14/22] tests/tcg/s390x: Add per.S Thomas Huth
@ 2024-05-29 10:54 ` Thomas Huth
  2024-05-29 10:54 ` [PULL 16/22] fuzz: disable leak-detection for oss-fuzz builds Thomas Huth
                   ` (7 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2024-05-29 10:54 UTC (permalink / raw)
  To: qemu-devel, Richard Henderson; +Cc: qemu-s390x, Alexander Bulekov

From: Alexander Bulekov <alxndr@bu.edu>

Fixes test-failure on Fedora 40 CI.

Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20240527040711.311865-1-alxndr@bu.edu>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/fuzz/generic_fuzz_configs.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h b/tests/qtest/fuzz/generic_fuzz_configs.h
index 4d7c8ca4ec..ef0ad95712 100644
--- a/tests/qtest/fuzz/generic_fuzz_configs.h
+++ b/tests/qtest/fuzz/generic_fuzz_configs.h
@@ -150,7 +150,8 @@ const generic_fuzz_config predefined_configs[] = {
         "-chardev null,id=cd0 -chardev null,id=cd1 "
         "-device usb-braille,chardev=cd0 -device usb-ccid -device usb-ccid "
         "-device usb-kbd -device usb-mouse -device usb-serial,chardev=cd1 "
-        "-device usb-tablet -device usb-wacom-tablet -device usb-audio",
+        "-device usb-tablet -device usb-wacom-tablet "
+        "-device usb-audio,audiodev=snd0 -audiodev none,id=snd0",
         .objects = "*usb* *uhci* *xhci*",
     },{
         .name = "pc-i440fx",
-- 
2.45.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PULL 16/22] fuzz: disable leak-detection for oss-fuzz builds
  2024-05-29 10:54 [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Thomas Huth
                   ` (14 preceding siblings ...)
  2024-05-29 10:54 ` [PULL 15/22] fuzz: specify audiodev for usb-audio Thomas Huth
@ 2024-05-29 10:54 ` Thomas Huth
  2024-05-29 10:54 ` [PULL 17/22] hw/s390x: Remove unused macro VMSTATE_ADAPTER_ROUTES Thomas Huth
                   ` (6 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2024-05-29 10:54 UTC (permalink / raw)
  To: qemu-devel, Richard Henderson
  Cc: qemu-s390x, Alexander Bulekov, Philippe Mathieu-Daudé

From: Alexander Bulekov <alxndr@bu.edu>

When we are building for OSS-Fuzz, we want to ensure that the fuzzer
targets are actually created, regardless of leaks. Leaks will be
detected by the subsequent tests of the individual fuzz-targets.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20240527150001.325565-1-alxndr@bu.edu>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 scripts/oss-fuzz/build.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
index 5238f83343..7398298173 100755
--- a/scripts/oss-fuzz/build.sh
+++ b/scripts/oss-fuzz/build.sh
@@ -92,6 +92,7 @@ make install DESTDIR=$DEST_DIR/qemu-bundle
 rm -rf $DEST_DIR/qemu-bundle/opt/qemu-oss-fuzz/bin
 rm -rf $DEST_DIR/qemu-bundle/opt/qemu-oss-fuzz/libexec
 
+export ASAN_OPTIONS=detect_leaks=0
 targets=$(./qemu-fuzz-i386 | grep generic-fuzz | awk '$1 ~ /\*/  {print $2}')
 base_copy="$DEST_DIR/qemu-fuzz-i386-target-$(echo "$targets" | head -n 1)"
 
-- 
2.45.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PULL 17/22] hw/s390x: Remove unused macro VMSTATE_ADAPTER_ROUTES
  2024-05-29 10:54 [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Thomas Huth
                   ` (15 preceding siblings ...)
  2024-05-29 10:54 ` [PULL 16/22] fuzz: disable leak-detection for oss-fuzz builds Thomas Huth
@ 2024-05-29 10:54 ` Thomas Huth
  2024-05-29 10:54 ` [PULL 18/22] scripts/update-linux-headers.sh: Remove temporary directory inbetween Thomas Huth
                   ` (5 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2024-05-29 10:54 UTC (permalink / raw)
  To: qemu-devel, Richard Henderson
  Cc: qemu-s390x, Cédric Le Goater, Eric Farman

It's not used anywhere, so let's simply remove it.

Message-ID: <20240527121351.211266-1-thuth@redhat.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Eric Farman <farman@linux.ibm.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 include/hw/s390x/s390_flic.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h
index bcb081def5..382d9833f1 100644
--- a/include/hw/s390x/s390_flic.h
+++ b/include/hw/s390x/s390_flic.h
@@ -35,9 +35,6 @@ typedef struct AdapterRoutes {
 
 extern const VMStateDescription vmstate_adapter_routes;
 
-#define VMSTATE_ADAPTER_ROUTES(_f, _s) \
-    VMSTATE_STRUCT(_f, _s, 1, vmstate_adapter_routes, AdapterRoutes)
-
 #define TYPE_S390_FLIC_COMMON "s390-flic"
 OBJECT_DECLARE_TYPE(S390FLICState, S390FLICStateClass,
                     S390_FLIC_COMMON)
-- 
2.45.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PULL 18/22] scripts/update-linux-headers.sh: Remove temporary directory inbetween
  2024-05-29 10:54 [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Thomas Huth
                   ` (16 preceding siblings ...)
  2024-05-29 10:54 ` [PULL 17/22] hw/s390x: Remove unused macro VMSTATE_ADAPTER_ROUTES Thomas Huth
@ 2024-05-29 10:54 ` Thomas Huth
  2024-05-29 10:54 ` [PULL 19/22] scripts/update-linux-headers.sh: Fix the path of setup_data.h Thomas Huth
                   ` (4 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2024-05-29 10:54 UTC (permalink / raw)
  To: qemu-devel, Richard Henderson
  Cc: qemu-s390x, Michael S . Tsirkin, Cornelia Huck

We are reusing the same temporary directory for installing the headers
of all targets, so there could be stale files here when switching from
one target to another. Make sure to delete the folder before installing
a new set of target headers into it.

Message-ID: <20240527060243.12647-1-thuth@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 scripts/update-linux-headers.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
index 8963c39189..fbf7e119bc 100755
--- a/scripts/update-linux-headers.sh
+++ b/scripts/update-linux-headers.sh
@@ -112,6 +112,7 @@ for arch in $ARCHLIST; do
         arch_var=ARCH
     fi
 
+    rm -rf "$hdrdir"
     make -C "$linux" O="$blddir" INSTALL_HDR_PATH="$hdrdir" $arch_var=$arch headers_install
 
     rm -rf "$output/linux-headers/asm-$arch"
-- 
2.45.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PULL 19/22] scripts/update-linux-headers.sh: Fix the path of setup_data.h
  2024-05-29 10:54 [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Thomas Huth
                   ` (17 preceding siblings ...)
  2024-05-29 10:54 ` [PULL 18/22] scripts/update-linux-headers.sh: Remove temporary directory inbetween Thomas Huth
@ 2024-05-29 10:54 ` Thomas Huth
  2024-05-29 10:54 ` [PULL 20/22] qemu-keymap: Make references to allocations static Thomas Huth
                   ` (3 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2024-05-29 10:54 UTC (permalink / raw)
  To: qemu-devel, Richard Henderson; +Cc: qemu-s390x, Cornelia Huck

When running the update-linx-headers.sh script, it currently fails with:

scripts/update-linux-headers.sh: line 73: .../qemu/standard-headers/asm-x86/setup_data.h: No such file or directory

The "include" folder is obviously missing here - no clue how this could
have worked before?

Fixes: 66210a1a30 ("scripts/update-linux-headers: Add setup_data.h to import list")
Message-ID: <20240527060126.12578-1-thuth@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 scripts/update-linux-headers.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
index fbf7e119bc..23afe8c08a 100755
--- a/scripts/update-linux-headers.sh
+++ b/scripts/update-linux-headers.sh
@@ -159,7 +159,7 @@ for arch in $ARCHLIST; do
         cp_portable "$hdrdir/bootparam.h" \
                     "$output/include/standard-headers/asm-$arch"
         cp_portable "$hdrdir/include/asm/setup_data.h" \
-                    "$output/standard-headers/asm-x86"
+                    "$output/include/standard-headers/asm-x86"
     fi
     if [ $arch = riscv ]; then
         cp "$hdrdir/include/asm/ptrace.h" "$output/linux-headers/asm-riscv/"
-- 
2.45.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PULL 20/22] qemu-keymap: Make references to allocations static
  2024-05-29 10:54 [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Thomas Huth
                   ` (18 preceding siblings ...)
  2024-05-29 10:54 ` [PULL 19/22] scripts/update-linux-headers.sh: Fix the path of setup_data.h Thomas Huth
@ 2024-05-29 10:54 ` Thomas Huth
  2024-05-29 10:54 ` [PULL 21/22] lockable: Do not cast function pointers Thomas Huth
                   ` (2 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2024-05-29 10:54 UTC (permalink / raw)
  To: qemu-devel, Richard Henderson
  Cc: qemu-s390x, Akihiko Odaki, Philippe Mathieu-Daudé

From: Akihiko Odaki <akihiko.odaki@daynix.com>

LeakSanitizer complains about allocations whose references are held
only by automatic variables. It is possible to free them to suppress
the complaints, but it is a chore to make sure they are freed in all
exit paths so make them static instead.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20240524-xkb-v4-1-2de564e5c859@daynix.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 qemu-keymap.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/qemu-keymap.c b/qemu-keymap.c
index 8c80f7a4ed..701e4332af 100644
--- a/qemu-keymap.c
+++ b/qemu-keymap.c
@@ -154,9 +154,9 @@ static xkb_mod_mask_t get_mod(struct xkb_keymap *map, const char *name)
 
 int main(int argc, char *argv[])
 {
-    struct xkb_context *ctx;
-    struct xkb_keymap *map;
-    struct xkb_state *state;
+    static struct xkb_context *ctx;
+    static struct xkb_keymap *map;
+    static struct xkb_state *state;
     xkb_mod_index_t mod, mods;
     int rc;
 
@@ -234,8 +234,6 @@ int main(int argc, char *argv[])
 
     state = xkb_state_new(map);
     xkb_keymap_key_for_each(map, walk_map, state);
-    xkb_state_unref(state);
-    state = NULL;
 
     /* add quirks */
     fprintf(outfile,
-- 
2.45.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PULL 21/22] lockable: Do not cast function pointers
  2024-05-29 10:54 [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Thomas Huth
                   ` (19 preceding siblings ...)
  2024-05-29 10:54 ` [PULL 20/22] qemu-keymap: Make references to allocations static Thomas Huth
@ 2024-05-29 10:54 ` Thomas Huth
  2024-05-29 10:54 ` [PULL 22/22] qapi: " Thomas Huth
  2024-05-29 18:54 ` [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Richard Henderson
  22 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2024-05-29 10:54 UTC (permalink / raw)
  To: qemu-devel, Richard Henderson
  Cc: qemu-s390x, Akihiko Odaki, Philippe Mathieu-Daudé

From: Akihiko Odaki <akihiko.odaki@daynix.com>

-fsanitize=undefined complains if function pointers are casted. It
also prevents enabling the strict mode of CFI which is currently
disabled with -fsanitize-cfi-icall-generalize-pointers.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2345
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20240524-xkb-v4-2-2de564e5c859@daynix.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 include/qemu/lockable.h | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/include/qemu/lockable.h b/include/qemu/lockable.h
index 62110d2eb7..66713bd429 100644
--- a/include/qemu/lockable.h
+++ b/include/qemu/lockable.h
@@ -43,15 +43,30 @@ qemu_null_lockable(void *x)
     return NULL;
 }
 
+#define QML_FUNC_(name)                                           \
+    static inline void qemu_lockable_ ## name ## _lock(void *x)   \
+    {                                                             \
+        qemu_ ## name ## _lock(x);                                \
+    }                                                             \
+    static inline void qemu_lockable_ ## name ## _unlock(void *x) \
+    {                                                             \
+        qemu_ ## name ## _unlock(x);                              \
+    }
+
+QML_FUNC_(mutex)
+QML_FUNC_(rec_mutex)
+QML_FUNC_(co_mutex)
+QML_FUNC_(spin)
+
 /*
  * In C, compound literals have the lifetime of an automatic variable.
  * In C++ it would be different, but then C++ wouldn't need QemuLockable
  * either...
  */
-#define QML_OBJ_(x, name) (&(QemuLockable) {                            \
-        .object = (x),                                                  \
-        .lock = (QemuLockUnlockFunc *) qemu_ ## name ## _lock,          \
-        .unlock = (QemuLockUnlockFunc *) qemu_ ## name ## _unlock       \
+#define QML_OBJ_(x, name) (&(QemuLockable) {        \
+        .object = (x),                              \
+        .lock = qemu_lockable_ ## name ## _lock,    \
+        .unlock = qemu_lockable_ ## name ## _unlock \
     })
 
 /**
-- 
2.45.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PULL 22/22] qapi: Do not cast function pointers
  2024-05-29 10:54 [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Thomas Huth
                   ` (20 preceding siblings ...)
  2024-05-29 10:54 ` [PULL 21/22] lockable: Do not cast function pointers Thomas Huth
@ 2024-05-29 10:54 ` Thomas Huth
  2024-05-29 18:54 ` [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Richard Henderson
  22 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2024-05-29 10:54 UTC (permalink / raw)
  To: qemu-devel, Richard Henderson
  Cc: qemu-s390x, Akihiko Odaki, Markus Armbruster

From: Akihiko Odaki <akihiko.odaki@daynix.com>

Using -fsanitize=undefined with Clang v18 causes an error if function
pointers are casted:

 qapi/qapi-clone-visitor.c:188:5: runtime error: call to function visit_type_SocketAddress through pointer to incorrect function type 'bool (*)(struct Visitor *, const char *, void **, struct Error **)'
 /tmp/qemu-ubsan/qapi/qapi-visit-sockets.c:487: note: visit_type_SocketAddress defined here
     #0 0x5642aa2f7f3b in qapi_clone qapi/qapi-clone-visitor.c:188:5
     #1 0x5642aa2c8ce5 in qio_channel_socket_listen_async io/channel-socket.c:285:18
     #2 0x5642aa2b8903 in test_io_channel_setup_async tests/unit/test-io-channel-socket.c:116:5
     #3 0x5642aa2b8204 in test_io_channel tests/unit/test-io-channel-socket.c:179:9
     #4 0x5642aa2b8129 in test_io_channel_ipv4 tests/unit/test-io-channel-socket.c:323:5
     ...

It also prevents enabling the strict mode of CFI which is currently
disabled with -fsanitize-cfi-icall-generalize-pointers.

The problematic casts are necessary to pass visit_type_T() and
visit_type_T_members() as callbacks to qapi_clone() and qapi_clone_members(),
respectively. Open-code these two functions to avoid the callbacks, and
thus the type casts.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2346
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-ID: <20240524-xkb-v4-3-2de564e5c859@daynix.com>
[thuth: Improve commit message according to Markus' suggestions]
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 include/qapi/clone-visitor.h | 37 +++++++++++++++++++++++-------------
 qapi/qapi-clone-visitor.c    | 30 ++++-------------------------
 2 files changed, 28 insertions(+), 39 deletions(-)

diff --git a/include/qapi/clone-visitor.h b/include/qapi/clone-visitor.h
index adf9a788e2..ebc182b034 100644
--- a/include/qapi/clone-visitor.h
+++ b/include/qapi/clone-visitor.h
@@ -11,6 +11,7 @@
 #ifndef QAPI_CLONE_VISITOR_H
 #define QAPI_CLONE_VISITOR_H
 
+#include "qapi/error.h"
 #include "qapi/visitor.h"
 
 /*
@@ -20,11 +21,8 @@
  */
 typedef struct QapiCloneVisitor QapiCloneVisitor;
 
-void *qapi_clone(const void *src, bool (*visit_type)(Visitor *, const char *,
-                                                     void **, Error **));
-void qapi_clone_members(void *dst, const void *src, size_t sz,
-                        bool (*visit_type_members)(Visitor *, void *,
-                                                   Error **));
+Visitor *qapi_clone_visitor_new(void);
+Visitor *qapi_clone_members_visitor_new(void);
 
 /*
  * Deep-clone QAPI object @src of the given @type, and return the result.
@@ -32,10 +30,18 @@ void qapi_clone_members(void *dst, const void *src, size_t sz,
  * Not usable on QAPI scalars (integers, strings, enums), nor on a
  * QAPI object that references the 'any' type.  Safe when @src is NULL.
  */
-#define QAPI_CLONE(type, src)                                           \
-    ((type *)qapi_clone(src,                                            \
-                        (bool (*)(Visitor *, const char *, void **,     \
-                                  Error **))visit_type_ ## type))
+#define QAPI_CLONE(type, src)                                   \
+    ({                                                          \
+        Visitor *v_;                                            \
+        type *dst_ = (type *) (src); /* Cast away const */      \
+                                                                \
+        if (dst_) {                                             \
+            v_ = qapi_clone_visitor_new();                      \
+            visit_type_ ## type(v_, NULL, &dst_, &error_abort); \
+            visit_free(v_);                                     \
+        }                                                       \
+        dst_;                                                   \
+    })
 
 /*
  * Copy deep clones of @type members from @src to @dst.
@@ -43,9 +49,14 @@ void qapi_clone_members(void *dst, const void *src, size_t sz,
  * Not usable on QAPI scalars (integers, strings, enums), nor on a
  * QAPI object that references the 'any' type.
  */
-#define QAPI_CLONE_MEMBERS(type, dst, src)                              \
-    qapi_clone_members(dst, src, sizeof(type),                          \
-                       (bool (*)(Visitor *, void *,                     \
-                                 Error **))visit_type_ ## type ## _members)
+#define QAPI_CLONE_MEMBERS(type, dst, src)                                \
+    ({                                                                    \
+        Visitor *v_;                                                      \
+                                                                          \
+        v_ = qapi_clone_members_visitor_new();                            \
+        *(type *)(dst) = *(src);                                          \
+        visit_type_ ## type ## _members(v_, (type *)(dst), &error_abort); \
+        visit_free(v_);                                                   \
+    })
 
 #endif
diff --git a/qapi/qapi-clone-visitor.c b/qapi/qapi-clone-visitor.c
index c45c5caa3b..bbf953698f 100644
--- a/qapi/qapi-clone-visitor.c
+++ b/qapi/qapi-clone-visitor.c
@@ -149,7 +149,7 @@ static void qapi_clone_free(Visitor *v)
     g_free(v);
 }
 
-static Visitor *qapi_clone_visitor_new(void)
+Visitor *qapi_clone_visitor_new(void)
 {
     QapiCloneVisitor *v;
 
@@ -174,31 +174,9 @@ static Visitor *qapi_clone_visitor_new(void)
     return &v->visitor;
 }
 
-void *qapi_clone(const void *src, bool (*visit_type)(Visitor *, const char *,
-                                                     void **, Error **))
+Visitor *qapi_clone_members_visitor_new(void)
 {
-    Visitor *v;
-    void *dst = (void *) src; /* Cast away const */
-
-    if (!src) {
-        return NULL;
-    }
-
-    v = qapi_clone_visitor_new();
-    visit_type(v, NULL, &dst, &error_abort);
-    visit_free(v);
-    return dst;
-}
-
-void qapi_clone_members(void *dst, const void *src, size_t sz,
-                        bool (*visit_type_members)(Visitor *, void *,
-                                                   Error **))
-{
-    Visitor *v;
-
-    v = qapi_clone_visitor_new();
-    memcpy(dst, src, sz);
+    Visitor *v = qapi_clone_visitor_new();
     to_qcv(v)->depth++;
-    visit_type_members(v, dst, &error_abort);
-    visit_free(v);
+    return v;
 }
-- 
2.45.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes
  2024-05-29 10:54 [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Thomas Huth
                   ` (21 preceding siblings ...)
  2024-05-29 10:54 ` [PULL 22/22] qapi: " Thomas Huth
@ 2024-05-29 18:54 ` Richard Henderson
  22 siblings, 0 replies; 24+ messages in thread
From: Richard Henderson @ 2024-05-29 18:54 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel; +Cc: qemu-s390x

On 5/29/24 03:54, Thomas Huth wrote:
>   Hi Richard!
> 
> The following changes since commit 79d7475f39f1b0f05fcb159f5cdcbf162340dc7e:
> 
>    Merge tag 'pull-block-jobs-2024-04-29-v2' ofhttps://gitlab.com/vsementsov/qemu  into staging (2024-05-28 11:28:34 -0700)
> 
> are available in the Git repository at:
> 
>    https://gitlab.com/thuth/qemu.git  tags/pull-request-2024-05-29
> 
> for you to fetch changes up to b04091393e6a71065aee6c91b2566f2dec95a4c9:
> 
>    qapi: Do not cast function pointers (2024-05-29 12:41:56 +0200)
> 
> ----------------------------------------------------------------
> * Fix and improve PER emulation on s390x
> * Fix problems of the build-oss-fuzz CI job
> * Fix broken update-linux-headers.sh script
> * Fixes for compiling with -fsanitize=undefined on latest Clang versions

Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/9.1 as appropriate.


r~



^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2024-05-29 18:54 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-29 10:54 [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Thomas Huth
2024-05-29 10:54 ` [PULL 01/22] target/s390x: Do not use unwind for per_check_exception Thomas Huth
2024-05-29 10:54 ` [PULL 02/22] target/s390x: Move cpu_get_tb_cpu_state out of line Thomas Huth
2024-05-29 10:54 ` [PULL 03/22] target/s390x: Update CR9 bits Thomas Huth
2024-05-29 10:54 ` [PULL 04/22] target/s390x: Record separate PER bits in TB flags Thomas Huth
2024-05-29 10:54 ` [PULL 05/22] target/s390x: Disable conditional branch-to-next for PER Thomas Huth
2024-05-29 10:54 ` [PULL 06/22] target/s390x: Introduce help_goto_indirect Thomas Huth
2024-05-29 10:54 ` [PULL 07/22] target/s390x: Simplify help_branch Thomas Huth
2024-05-29 10:54 ` [PULL 08/22] target/s390x: Split per_breaking_event from per_branch_* Thomas Huth
2024-05-29 10:54 ` [PULL 09/22] target/s390x: Raise exception from helper_per_branch Thomas Huth
2024-05-29 10:54 ` [PULL 10/22] target/s390x: Raise exception from per_store_real Thomas Huth
2024-05-29 10:54 ` [PULL 11/22] target/s390x: Fix helper_per_ifetch flags Thomas Huth
2024-05-29 10:54 ` [PULL 12/22] target/s390x: Simplify per_ifetch, per_check_exception Thomas Huth
2024-05-29 10:54 ` [PULL 13/22] target/s390x: Adjust check of noreturn in translate_one Thomas Huth
2024-05-29 10:54 ` [PULL 14/22] tests/tcg/s390x: Add per.S Thomas Huth
2024-05-29 10:54 ` [PULL 15/22] fuzz: specify audiodev for usb-audio Thomas Huth
2024-05-29 10:54 ` [PULL 16/22] fuzz: disable leak-detection for oss-fuzz builds Thomas Huth
2024-05-29 10:54 ` [PULL 17/22] hw/s390x: Remove unused macro VMSTATE_ADAPTER_ROUTES Thomas Huth
2024-05-29 10:54 ` [PULL 18/22] scripts/update-linux-headers.sh: Remove temporary directory inbetween Thomas Huth
2024-05-29 10:54 ` [PULL 19/22] scripts/update-linux-headers.sh: Fix the path of setup_data.h Thomas Huth
2024-05-29 10:54 ` [PULL 20/22] qemu-keymap: Make references to allocations static Thomas Huth
2024-05-29 10:54 ` [PULL 21/22] lockable: Do not cast function pointers Thomas Huth
2024-05-29 10:54 ` [PULL 22/22] qapi: " Thomas Huth
2024-05-29 18:54 ` [PULL 00/22] s390x, build-oss-fuzz and Clang -fsanitize=undefined fixes Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).