Re: [PATCH v2 2/3] target/s390x: Fix missing clock-comparator interrupts after reset

[Thomas Huth <thuth@redhat.com>]

  Hi Ilya!

On 15/10/2025 16.21, Ilya Leoshkevich wrote:
> After reset, CKC value is set to 0, so if clock-comparator interrupts
> are enabled, one should occur very shortly thereafter.
> 
> Currently the code does not set tod_timer, so this does not happen.
> 
> Fix by adding a tcg_s390_tod_updated() call. Initialize TOD clock
> before CPUs in order to ensure that the respective object exists
> during the CPU reset.

Can this really happen? Looking at CPUS390XState in target/s390x/cpu.h, the 
ckc is next to the cregs[] in the start_initial_reset_fields section, so if 
ckc gets cleared, the cregs get cleared, too. I.e. if ckc gets set to 0 
here, there is no way that the clock comparator interrupt could trigger 
immediately without the guest writing to the control registers first. So I 
think this patch is not really necessary. Or do I miss something?

  Thomas



> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   hw/s390x/s390-virtio-ccw.c | 6 +++---
>   target/s390x/cpu.c         | 8 ++++++++
>   2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index ad2c48188a8..9f37a96ae5a 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -286,6 +286,9 @@ static void ccw_init(MachineState *machine)
>       /* init memory + setup max page size. Required for the CPU model */
>       s390_memory_init(machine);
>   
> +    /* init the TOD clock */
> +    s390_init_tod();
> +
>       /* init CPUs (incl. CPU model) early so s390_has_feature() works */
>       s390_init_cpus(machine);
>   
> @@ -332,9 +335,6 @@ static void ccw_init(MachineState *machine)
>           s390_create_sclpconsole(ms->sclp, "sclplmconsole", serial_hd(1));
>       }
>   
> -    /* init the TOD clock */
> -    s390_init_tod();
> -
>       /* init SCLP event Control-Program Identification */
>       if (s390mc->use_cpi) {
>           s390_create_sclpcpi(ms->sclp);
> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index f05ce317da1..cc2de6ce08e 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -40,6 +40,7 @@
>   #include "system/reset.h"
>   #endif
>   #include "hw/s390x/cpu-topology.h"
> +#include "tcg/tcg_s390x.h"
>   
>   #define CR0_RESET       0xE0UL
>   #define CR14_RESET      0xC2000000UL;
> @@ -215,6 +216,13 @@ static void s390_cpu_reset_hold(Object *obj, ResetType type)
>               break;
>           }
>       }
> +
> +#ifndef CONFIG_USER_ONLY
> +    if (tcg_enabled()) {
> +        /* Rearm the CKC timer if necessary */
> +        tcg_s390_tod_updated(CPU(cpu), RUN_ON_CPU_NULL);
> +    }
> +#endif
>   }
>   
>   static void s390_cpu_disas_set_info(CPUState *cpu, disassemble_info *info)