Re: [PATCH v2 04/17] target/s390x: Use S390CpuState for CPU state APIs

Wed, 23 Sep 2026 10:07:44 -0700 



On 9/22/26 12:49 PM, Philippe Mathieu-Daudé wrote:

Use the QAPI S390CpuState enum for CPU state accessors instead
of uint8_t. Handle state checks with switch statements and assert
unexpected states in SIGP restart and stop handling.

Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
---
FIXME: g_assert_not_reached -> break?


Yes, please.


Those orders sent against any other cpu state should be NOPs, rather 
than a fatal assertion.



---
  target/s390x/cpu.h           |  6 +++---
  target/s390x/kvm/kvm_s390x.h |  2 +-
  hw/intc/s390_flic.c          |  7 +++++--
  target/s390x/cpu-system.c    | 18 ++++++++++++------
  target/s390x/kvm/kvm.c       |  2 +-
  target/s390x/kvm/stubs.c     |  2 +-
  target/s390x/sigp.c          |  6 +++++-
  7 files changed, 28 insertions(+), 15 deletions(-)


The rest of this looks fine to me.



diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 2d1dcc7045c..bf9f6ed1de2 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -847,13 +847,13 @@ void s390_do_cpu_set_diag318(CPUState *cs, 
run_on_cpu_data arg);
  int s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch_id,
                                  int vq, bool assign);
  #ifndef CONFIG_USER_ONLY
-void s390_cpu_set_state(uint8_t cpu_state, S390CPU *cpu);
+void s390_cpu_set_state(S390CpuState cpu_state, S390CPU *cpu);
  #else
-static inline void s390_cpu_set_state(uint8_t cpu_state, S390CPU *cpu)
+static inline void s390_cpu_set_state(S390CpuState cpu_state, S390CPU *cpu)
  {
  }
  #endif /* CONFIG_USER_ONLY */
-static inline uint8_t s390_cpu_get_state(const S390CPU *cpu)
+static inline S390CpuState s390_cpu_get_state(const S390CPU *cpu)
  {
      return cpu->env.cpu_state;
  }
diff --git a/target/s390x/kvm/kvm_s390x.h b/target/s390x/kvm/kvm_s390x.h
index 3c4fa0489cf..b1ff66f04e3 100644
--- a/target/s390x/kvm/kvm_s390x.h
+++ b/target/s390x/kvm/kvm_s390x.h
@@ -22,7 +22,7 @@ int kvm_s390_mem_op(S390CPU *cpu, vaddr addr, uint8_t ar, 
void *hostbuf,
  int kvm_s390_mem_op_pv(S390CPU *cpu, vaddr addr, void *hostbuf, int len,
                         bool is_write);
  void kvm_s390_program_interrupt(S390CPU *cpu, uint16_t code);
-int kvm_s390_set_cpu_state(S390CPU *cpu, uint8_t cpu_state);
+int kvm_s390_set_cpu_state(S390CPU *cpu, S390CpuState cpu_state);
  void kvm_s390_vcpu_interrupt_pre_save(S390CPU *cpu);
  int kvm_s390_vcpu_interrupt_post_load(S390CPU *cpu);
  int kvm_s390_get_hpage(void);
diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
index 57fd4b2b81c..b5256b3ba88 100644
--- a/hw/intc/s390_flic.c
+++ b/hw/intc/s390_flic.c
@@ -193,8 +193,11 @@ static void qemu_s390_flic_notify(uint32_t type)
          cpu_set_interrupt(cs, CPU_INTERRUPT_HARD);

  
          /* ignore CPUs that are not sleeping */

-        if (s390_cpu_get_state(cpu) != S390_CPU_STATE_OPERATING &&
-            s390_cpu_get_state(cpu) != S390_CPU_STATE_LOAD) {
+        switch (s390_cpu_get_state(cpu)) {
+        case S390_CPU_STATE_LOAD:
+        case S390_CPU_STATE_OPERATING:
+            break;
+        default:
              continue;
          }

  
diff --git a/target/s390x/cpu-system.c b/target/s390x/cpu-system.c

index 52561fffa3a..fffac09f241 100644
--- a/target/s390x/cpu-system.c
+++ b/target/s390x/cpu-system.c
@@ -44,8 +44,11 @@ bool s390_cpu_has_work(CPUState *cs)
      S390CPU *cpu = S390_CPU(cs);

  
      /* STOPPED cpus can never wake up */

-    if (s390_cpu_get_state(cpu) != S390_CPU_STATE_LOAD &&
-        s390_cpu_get_state(cpu) != S390_CPU_STATE_OPERATING) {
+    switch (s390_cpu_get_state(cpu)) {
+    case S390_CPU_STATE_LOAD:
+    case S390_CPU_STATE_OPERATING:
+        break;
+    default:
          return false;
      }

  
@@ -202,12 +205,15 @@ unsigned s390_count_running_cpus(void)

      int nr_running = 0;

  
      CPU_FOREACH(cpu) {

-        uint8_t state = S390_CPU(cpu)->env.cpu_state;
-        if (state == S390_CPU_STATE_OPERATING ||
-            state == S390_CPU_STATE_LOAD) {
+        switch (s390_cpu_get_state(S390_CPU(cpu))) {
+        case S390_CPU_STATE_LOAD:
+        case S390_CPU_STATE_OPERATING:
              if (!disabled_wait(cpu)) {
                  nr_running++;
              }
+            break;
+        default:
+            break;
          }
      }

  
@@ -236,7 +242,7 @@ void s390_cpu_unhalt(S390CPU *cpu)

      }
  }

  
-void s390_cpu_set_state(uint8_t cpu_state, S390CPU *cpu)

+void s390_cpu_set_state(S390CpuState cpu_state, S390CPU *cpu)
   {
      trace_cpu_set_state(CPU(cpu)->cpu_index, cpu_state);

  
diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c

index 6622886032e..dc67e848864 100644
--- a/target/s390x/kvm/kvm.c
+++ b/target/s390x/kvm/kvm.c
@@ -2007,7 +2007,7 @@ int kvm_s390_get_ri(void)
      return cap_ri;
  }

  
-int kvm_s390_set_cpu_state(S390CPU *cpu, uint8_t cpu_state)

+int kvm_s390_set_cpu_state(S390CPU *cpu, S390CpuState cpu_state)
  {
      struct kvm_mp_state mp_state = {};
      int ret;
diff --git a/target/s390x/kvm/stubs.c b/target/s390x/kvm/stubs.c
index ebf3c83994d..c5ec7d3f1d3 100644
--- a/target/s390x/kvm/stubs.c
+++ b/target/s390x/kvm/stubs.c
@@ -128,7 +128,7 @@ int kvm_s390_mem_op_pv(S390CPU *cpu, vaddr addr, void 
*hostbuf, int len,
      g_assert_not_reached();
  }

  
-int kvm_s390_set_cpu_state(S390CPU *cpu, uint8_t cpu_state)

+int kvm_s390_set_cpu_state(S390CPU *cpu, S390CpuState cpu_state)
  {
      g_assert_not_reached();
  }
diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
index 1801b8caa6e..6e3367e2725 100644
--- a/target/s390x/sigp.c
+++ b/target/s390x/sigp.c
@@ -39,7 +39,7 @@ static void set_sigp_status(SigpInfo *si, uint64_t status)

  
  static void sigp_sense(S390CPU *dst_cpu, SigpInfo *si)

  {
-    uint8_t state = s390_cpu_get_state(dst_cpu);
+    S390CpuState state = s390_cpu_get_state(dst_cpu);
      bool ext_call = dst_cpu->env.pending_int & INTERRUPT_EXTERNAL_CALL;
      uint64_t status = 0;

  
@@ -221,6 +221,8 @@ static void sigp_stop_and_store_status(CPUState *cs, run_on_cpu_data arg)

          cpu_synchronize_state(cs);
          s390_store_status(cpu, S390_STORE_STATUS_DEF_ADDR, true);
          break;
+    default:
+        g_assert_not_reached();
      }
      si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
  }
@@ -362,6 +364,8 @@ static void sigp_restart(CPUState *cs, run_on_cpu_data arg)
      case S390_CPU_STATE_OPERATING:
          cpu_inject_restart(cpu);
          break;
+    default:
+        g_assert_not_reached();
      }
      si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
  }

























					Reply via email to