[PATCH v4 0/3] target/s390x: Fix missing clock-comparator interrupts

[PATCH v4 0/3] target/s390x: Fix missing clock-comparator interrupts

[Ilya Leoshkevich]

v3: https://lore.kernel.org/qemu-devel/20251016120928.22467-1-iii@linux.ibm.com/
v3 -> v4: Add a patch for the missing address generation affecting
          interrupts. Slightly adjust the test, keep Thomas' R-b.
          Add Thomas' R-b to the timer rearm fix.

v2: https://lore.kernel.org/qemu-devel/20251015142141.3238-1-iii@linux.ibm.com/
v2 -> v3: Rearm the timer on control register load (Thomas).
          Add Thomas' R-b to the test.

v1: https://lore.kernel.org/qemu-devel/20251014160743.398093-1-iii@linux.ibm.com/
v1 -> v2: Add Thomas' R-b.
          Cc: stable (Michael).
          Improve formatting, commit messages, and test (Ilya).

Hi,

While trying to reproduce [1], I found two bugs in the clock comparator
handling. This series fixes all three issues and adds a test.

[1] https://lore.kernel.org/lkml/ab3131a2-c42a-47ff-bf03-e9f68ac053c0@t-8ch.de/

Best regards,
Ilya

Ilya Leoshkevich (4):
  target/s390x: Fix missing interrupts for small CKC values
  target/s390x: Fix missing clock-comparator interrupts after reset
  target/s390x: Use address generation for register branch targets
  tests/tcg/s390x: Test SET CLOCK COMPARATOR

 target/s390x/tcg/mem_helper.c           | 11 ++++-
 target/s390x/tcg/misc_helper.c          | 12 +++--
 target/s390x/tcg/translate.c            | 11 +++--
 tests/tcg/s390x/Makefile.softmmu-target |  1 +
 tests/tcg/s390x/sckc.S                  | 63 +++++++++++++++++++++++++
 5 files changed, 89 insertions(+), 9 deletions(-)
 create mode 100644 tests/tcg/s390x/sckc.S

-- 
2.51.0

[PATCH v4 1/4] target/s390x: Fix missing interrupts for small CKC values

[Ilya Leoshkevich]

Suppose TOD clock value is 0x1111111111111111 and clock-comparator
value is 0, in which case clock-comparator interruption should occur
immediately.

With the current code, tod2time(env->ckc - td->base.low) ends up being
a very large number, so this interruption never happens.

Fix by firing the timer immediately if env->ckc < td->base.low.

Cc: qemu-stable@nongnu.org
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 target/s390x/tcg/misc_helper.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/target/s390x/tcg/misc_helper.c b/target/s390x/tcg/misc_helper.c
index 6d9d601d29a..215b5b9d933 100644
--- a/target/s390x/tcg/misc_helper.c
+++ b/target/s390x/tcg/misc_helper.c
@@ -199,11 +199,15 @@ static void update_ckc_timer(CPUS390XState *env)
         return;
     }
 
-    /* difference between origins */
-    time = env->ckc - td->base.low;
+    if (env->ckc < td->base.low) {
+        time = 0;
+    } else {
+        /* difference between origins */
+        time = env->ckc - td->base.low;
 
-    /* nanoseconds */
-    time = tod2time(time);
+        /* nanoseconds */
+        time = tod2time(time);
+    }
 
     timer_mod(env->tod_timer, time);
 }
-- 
2.51.0

[PATCH v4 2/4] target/s390x: Fix missing clock-comparator interrupts after reset

[Ilya Leoshkevich]

After reset, CKC value is set to 0, so if clock-comparator interrupts
are enabled, one should occur very shortly thereafter.

Currently the code that loads the respective control register does not
set tod_timer, so this does not happen.

Fix by adding a tcg_s390_tod_updated() call to LCTL and LCTLG.

Cc: qemu-stable@nongnu.org
Suggested-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 target/s390x/tcg/mem_helper.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
index f1acb1618f7..24675fc818d 100644
--- a/target/s390x/tcg/mem_helper.c
+++ b/target/s390x/tcg/mem_helper.c
@@ -1959,6 +1959,10 @@ void HELPER(lctlg)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
         if (env->cregs[i] != val && i >= 9 && i <= 11) {
             PERchanged = true;
         }
+        if (i == 0 && !(env->cregs[i] & CR0_CKC_SC) && (val & CR0_CKC_SC)) {
+            BQL_LOCK_GUARD();
+            tcg_s390_tod_updated(env_cpu(env), RUN_ON_CPU_NULL);
+        }
         env->cregs[i] = val;
         HELPER_LOG("load ctl %d from 0x%" PRIx64 " == 0x%" PRIx64 "\n",
                    i, src, val);
@@ -1989,10 +1993,15 @@ void HELPER(lctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
 
     for (i = r1;; i = (i + 1) % 16) {
         uint32_t val = cpu_ldl_data_ra(env, src, ra);
+        uint64_t val64 = deposit64(env->cregs[i], 0, 32, val);
         if ((uint32_t)env->cregs[i] != val && i >= 9 && i <= 11) {
             PERchanged = true;
         }
-        env->cregs[i] = deposit64(env->cregs[i], 0, 32, val);
+        if (i == 0 && !(env->cregs[i] & CR0_CKC_SC) && (val64 & CR0_CKC_SC)) {
+            BQL_LOCK_GUARD();
+            tcg_s390_tod_updated(env_cpu(env), RUN_ON_CPU_NULL);
+        }
+        env->cregs[i] = val64;
         HELPER_LOG("load ctl %d from 0x%" PRIx64 " == 0x%x\n", i, src, val);
         src += sizeof(uint32_t);
 
-- 
2.51.0

[PATCH v4 3/4] target/s390x: Use address generation for register branch targets

[Ilya Leoshkevich]

Indirect branches to addresses taken from registers go through address
generation, e.g., for BRANCH ON CONDITION Principles of Operation says:

    In the RR format, the contents of general register R2 are used to
    generate the branch address

QEMU uses r2_nz handler for the respective register operands. Currently
it does not zero out extra bits in 24- and 31-bit addressing modes as
required by address generation. The very frequently used
s390x_tr_init_disas_context() function has a workaround for this,
but the code for saving an old PSW during an interrupt does not.

Add the missing masking to r2_nz. Enforce PSW validity by replacing the
workaround with an assersion.

Reported-by: Thomas Weißschuh <linux@weissschuh.net>
Reported-by: Heiko Carstens <hca@linux.ibm.com>
Link: https://lore.kernel.org/lkml/ab3131a2-c42a-47ff-bf03-e9f68ac053c0@t-8ch.de/
Cc: qemu-stable@nongnu.org
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 target/s390x/tcg/translate.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index c7e8574438c..f9487f4044d 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -5613,6 +5613,7 @@ static void in2_r2_nz(DisasContext *s, DisasOps *o)
     int r2 = get_field(s, r2);
     if (r2 != 0) {
         o->in2 = load_reg(r2);
+        gen_addi_and_wrap_i64(s, o->in2, o->in2, 0);
     }
 }
 #define SPEC_in2_r2_nz 0
@@ -6379,10 +6380,12 @@ static void s390x_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
 {
     DisasContext *dc = container_of(dcbase, DisasContext, base);
 
-    /* 31-bit mode */
-    if (!(dc->base.tb->flags & FLAG_MASK_64)) {
-        dc->base.pc_first &= 0x7fffffff;
-        dc->base.pc_next = dc->base.pc_first;
+    if (dc->base.tb->flags & FLAG_MASK_32) {
+        if (!(dc->base.tb->flags & FLAG_MASK_64)) {
+            assert(!(dc->base.pc_first & ~((1ULL << 31) - 1)));
+        }
+    } else {
+        assert(!(dc->base.pc_first & ~((1ULL << 24) - 1)));
     }
 
     dc->cc_op = CC_OP_DYNAMIC;
-- 
2.51.0

[PATCH v4 4/4] tests/tcg/s390x: Test SET CLOCK COMPARATOR

[Ilya Leoshkevich]

Add a small test to prevent regressions.

Cc: qemu-stable@nongnu.org
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tests/tcg/s390x/Makefile.softmmu-target |  1 +
 tests/tcg/s390x/sckc.S                  | 63 +++++++++++++++++++++++++
 2 files changed, 64 insertions(+)
 create mode 100644 tests/tcg/s390x/sckc.S

diff --git a/tests/tcg/s390x/Makefile.softmmu-target b/tests/tcg/s390x/Makefile.softmmu-target
index 8cd4667c63b..a4425d3184a 100644
--- a/tests/tcg/s390x/Makefile.softmmu-target
+++ b/tests/tcg/s390x/Makefile.softmmu-target
@@ -28,6 +28,7 @@ ASM_TESTS =                                                                    \
     mc                                                                         \
     per                                                                        \
     precise-smc-softmmu                                                        \
+    sckc                                                                       \
     ssm-early                                                                  \
     stosm-early                                                                \
     stpq                                                                       \
diff --git a/tests/tcg/s390x/sckc.S b/tests/tcg/s390x/sckc.S
new file mode 100644
index 00000000000..ecd64a3059e
--- /dev/null
+++ b/tests/tcg/s390x/sckc.S
@@ -0,0 +1,63 @@
+/*
+ * Test clock comparator.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+    .org 0x130
+ext_old_psw:
+    .org 0x1b0
+ext_new_psw:
+    .quad 0x180000000, _ext            /* 64-bit mode */
+    .org 0x1d0
+pgm_new_psw:
+    .quad 0x2000000000000,0            /* disabled wait */
+    .org 0x200                         /* lowcore padding */
+
+    .globl _start
+_start:
+    lpswe start31_psw
+_start31:
+    stctg %c0,%c0,c0
+    oi c0+6,8                          /* set clock-comparator subclass mask */
+    lctlg %c0,%c0,c0
+
+0:
+    brasl %r14,_f                      /* %r14's most significant bit is 1 */
+    jg 0b
+_f:
+    br %r14                            /* it must not end up in ext_old_psw */
+
+_ext:
+    stg %r0,ext_saved_r0
+
+    lg %r0,ext_counter
+    aghi %r0,1
+    stg %r0,ext_counter
+
+    cgfi %r0,0x1000
+    jnz 0f
+    lpswe success_psw
+0:
+
+    stck clock
+    lg %r0,clock
+    agfi %r0,0x40000                   /* 64us * 0x1000 =~ 0.25s */
+    stg %r0,clock
+    sckc clock
+
+    lg %r0,ext_saved_r0
+    lpswe ext_old_psw
+
+    .align 8
+start31_psw:
+    .quad 0x100000080000000,_start31   /* EX, 31-bit mode */
+success_psw:
+    .quad 0x2000000000000,0xfff        /* see is_special_wait_psw() */
+c0:
+    .skip 8
+clock:
+    .quad 0
+ext_counter:
+    .quad 0
+ext_saved_r0:
+    .skip 8
-- 
2.51.0

Re: [PATCH v4 3/4] target/s390x: Use address generation for register branch targets

[Thomas Weißschuh]

On 2025-10-16 19:58:32+0200, Ilya Leoshkevich wrote:
> Indirect branches to addresses taken from registers go through address
> generation, e.g., for BRANCH ON CONDITION Principles of Operation says:
> 
>     In the RR format, the contents of general register R2 are used to
>     generate the branch address
> 
> QEMU uses r2_nz handler for the respective register operands. Currently
> it does not zero out extra bits in 24- and 31-bit addressing modes as
> required by address generation. The very frequently used
> s390x_tr_init_disas_context() function has a workaround for this,
> but the code for saving an old PSW during an interrupt does not.
> 
> Add the missing masking to r2_nz. Enforce PSW validity by replacing the
> workaround with an assersion.
> 
> Reported-by: Thomas Weißschuh <linux@weissschuh.net>
> Reported-by: Heiko Carstens <hca@linux.ibm.com>
> Link: https://lore.kernel.org/lkml/ab3131a2-c42a-47ff-bf03-e9f68ac053c0@t-8ch.de/
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>

Thanks!

Tested-by: Thomas Weißschuh <linux@weissschuh.net>

(...)

Re: [PATCH PING v4 0/3] target/s390x: Fix missing clock-comparator interrupts

[Ilya Leoshkevich]

On Thu, 2025-10-16 at 19:58 +0200, Ilya Leoshkevich wrote:
> v3:
> https://lore.kernel.org/qemu-devel/20251016120928.22467-1-iii@linux.ibm.com/
> v3 -> v4: Add a patch for the missing address generation affecting
>           interrupts. Slightly adjust the test, keep Thomas' R-b.
>           Add Thomas' R-b to the timer rearm fix.
> 
> v2:
> https://lore.kernel.org/qemu-devel/20251015142141.3238-1-iii@linux.ibm.com/
> v2 -> v3: Rearm the timer on control register load (Thomas).
>           Add Thomas' R-b to the test.
> 
> v1:
> https://lore.kernel.org/qemu-devel/20251014160743.398093-1-iii@linux.ibm.com/
> v1 -> v2: Add Thomas' R-b.
>           Cc: stable (Michael).
>           Improve formatting, commit messages, and test (Ilya).
> 
> Hi,
> 
> While trying to reproduce [1], I found two bugs in the clock
> comparator
> handling. This series fixes all three issues and adds a test.
> 
> [1]
> https://lore.kernel.org/lkml/ab3131a2-c42a-47ff-bf03-e9f68ac053c0@t-8ch.de/
> 
> Best regards,
> Ilya
> 
> Ilya Leoshkevich (4):
>   target/s390x: Fix missing interrupts for small CKC values
>   target/s390x: Fix missing clock-comparator interrupts after reset
>   target/s390x: Use address generation for register branch targets
>   tests/tcg/s390x: Test SET CLOCK COMPARATOR
> 
>  target/s390x/tcg/mem_helper.c           | 11 ++++-
>  target/s390x/tcg/misc_helper.c          | 12 +++--
>  target/s390x/tcg/translate.c            | 11 +++--
>  tests/tcg/s390x/Makefile.softmmu-target |  1 +
>  tests/tcg/s390x/sckc.S                  | 63
> +++++++++++++++++++++++++
>  5 files changed, 89 insertions(+), 9 deletions(-)
>  create mode 100644 tests/tcg/s390x/sckc.S

Hello,

I would like to ping this series.

Patches 1, 2, and 4 are already reviewed (thanks, Thomas!).
Patch 3 still needs a review.

Best regards,
Ilya

Re: [PATCH PING v4 0/3] target/s390x: Fix missing clock-comparator interrupts

[Thomas Huth]

On 29/10/2025 10.55, Ilya Leoshkevich wrote:
> On Thu, 2025-10-16 at 19:58 +0200, Ilya Leoshkevich wrote:
>> v3:
>> https://lore.kernel.org/qemu-devel/20251016120928.22467-1-iii@linux.ibm.com/
>> v3 -> v4: Add a patch for the missing address generation affecting
>>            interrupts. Slightly adjust the test, keep Thomas' R-b.
>>            Add Thomas' R-b to the timer rearm fix.
>>
>> v2:
>> https://lore.kernel.org/qemu-devel/20251015142141.3238-1-iii@linux.ibm.com/
>> v2 -> v3: Rearm the timer on control register load (Thomas).
>>            Add Thomas' R-b to the test.
>>
>> v1:
>> https://lore.kernel.org/qemu-devel/20251014160743.398093-1-iii@linux.ibm.com/
>> v1 -> v2: Add Thomas' R-b.
>>            Cc: stable (Michael).
>>            Improve formatting, commit messages, and test (Ilya).
>>
>> Hi,
>>
>> While trying to reproduce [1], I found two bugs in the clock
>> comparator
>> handling. This series fixes all three issues and adds a test.
>>
>> [1]
>> https://lore.kernel.org/lkml/ab3131a2-c42a-47ff-bf03-e9f68ac053c0@t-8ch.de/
>>
>> Best regards,
>> Ilya
>>
>> Ilya Leoshkevich (4):
>>    target/s390x: Fix missing interrupts for small CKC values
>>    target/s390x: Fix missing clock-comparator interrupts after reset
>>    target/s390x: Use address generation for register branch targets
>>    tests/tcg/s390x: Test SET CLOCK COMPARATOR
>>
>>   target/s390x/tcg/mem_helper.c           | 11 ++++-
>>   target/s390x/tcg/misc_helper.c          | 12 +++--
>>   target/s390x/tcg/translate.c            | 11 +++--
>>   tests/tcg/s390x/Makefile.softmmu-target |  1 +
>>   tests/tcg/s390x/sckc.S                  | 63
>> +++++++++++++++++++++++++
>>   5 files changed, 89 insertions(+), 9 deletions(-)
>>   create mode 100644 tests/tcg/s390x/sckc.S
> 
> Hello,
> 
> I would like to ping this series.
> 
> Patches 1, 2, and 4 are already reviewed (thanks, Thomas!).
> Patch 3 still needs a review.

  Hi Ilya,

I still got it on my radar, and indeed, it would be great to get an Ack from 
Richard or David for patch 3.

However, the main problem is that we need the fix for the C23 problem in the 
fourth patch first before this could get merged 
(https://gitlab.com/qemu-project/SLOF/-/merge_requests/2). It should 
hopefully get fixed with Harsh's next ppc pull request, see:

  https://lore.kernel.org/qemu-devel/21a94013-c3ef-49e4-abfa-a277277a03b0@linux.ibm.com

  Thomas