Re: [RFC PATCH 13/13] cpus: Constify @cpu in SysemuCPUOps::has_work() handler

> All functions called by target has_work() handler take a const @cpu
> argument. We can now fulfill the comment added in commit c2804566f6
> ("target/arm: do not clear halting reason in has_work helper"),
> qualifying the handler const to denote its idempotency.
> 
> Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
> Message-ID: <[email protected]>
>
> diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h
> index 9a45596169a3..095bdcd997aa 100644
> --- a/include/hw/core/sysemu-cpu-ops.h
> +++ b/include/hw/core/sysemu-cpu-ops.h
> @@ -19,10 +19,10 @@ typedef struct SysemuCPUOps {
>      /**
>       * @has_work: Callback for checking if there is work to do.
>       *
> -     * This function should be idempotent (i.e. not change state) as
> +     * This function is idempotent (i.e. not change state) as
>       * it will likely be queried multiple times before a CPU resumes.
>       */
> -    bool (*has_work)(CPUState *cpu); /* MANDATORY NON-NULL */
> +    bool (*has_work)(const CPUState *cpu); /* MANDATORY NON-NULL */
>      /**
>       * @get_memory_mapping: Callback for obtaining the memory mappings.
>       */
> diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
> index 12e860216634..991dfd8c38a9 100644
> --- a/target/alpha/cpu.c
> +++ b/target/alpha/cpu.c
> @@ -77,7 +77,7 @@ static void alpha_restore_state_to_opc(CPUState *cs,
>  }
>  
>  #ifndef CONFIG_USER_ONLY
> -static bool alpha_cpu_has_work(CPUState *cs)
> +static bool alpha_cpu_has_work(const CPUState *cs)
>  {
>      /* Here we are checking to see if the CPU should wake up from HALT.
>         We will have gotten into this state only for WTINT from PALmode.  */
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 77aa78f00e2f..7ee701d5d138 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -141,9 +141,9 @@ int arm_cpu_mmu_index(CPUState *cs, bool ifetch)
>   * CPU_INTERRUPT_*NMI anyway. So we might as well accept NMI here
>   * unconditionally.
>   */
> -static bool arm_cpu_has_work(CPUState *cs)
> +static bool arm_cpu_has_work(const CPUState *cs)
>  {
> -    ARMCPU *cpu = ARM_CPU(cs);
> +    const ARMCPU *cpu = ARM_CPU(cs);
>  
>      /*
>       * Only another PSCI call can wake the CPU up in which case the
> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
> index f8409f32ab98..00f06d71eaae 100644
> --- a/target/avr/cpu.c
> +++ b/target/avr/cpu.c
> @@ -23,6 +23,7 @@
>  #include "qemu/qemu-print.h"
>  #include "exec/translation-block.h"
>  #include "system/address-spaces.h"
> +#include "hw/core/cpu.h"
>  #include "cpu.h"
>  #include "disas/dis-asm.h"
>  #include "tcg/debug-assert.h"
> @@ -43,10 +44,10 @@ static vaddr avr_cpu_get_pc(CPUState *cs)
>      return cpu->env.pc_w * 2;
>  }
>  
> -static bool avr_cpu_has_work(CPUState *cs)
> +static bool avr_cpu_has_work(const CPUState *cs)
>  {
>      return cpu_test_interrupt(cs, CPU_INTERRUPT_HARD | CPU_INTERRUPT_RESET)
> -            && cpu_interrupts_enabled(cpu_env(cs));
> +            && cpu_interrupts_enabled(cpu_env_const(cs));
>  }
>  
>  static int avr_cpu_mmu_index(CPUState *cs, bool ifetch)
> diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
> index da11d266c351..3139fcb695de 100644
> --- a/target/hexagon/cpu.c
> +++ b/target/hexagon/cpu.c
> @@ -364,9 +364,9 @@ bool hexagon_thread_is_enabled(const CPUHexagonState *env)
>      return E_bit;
>  }
>  
> -static bool hexagon_cpu_has_work(CPUState *cs)
> +static bool hexagon_cpu_has_work(const CPUState *cs)
>  {
> -    CPUHexagonState *env = cpu_env(cs);
> +    const CPUHexagonState *env = cpu_env_const(cs);
>  
>      return hexagon_thread_is_enabled(env) &&
>          (cs->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_SWI
> diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
> index 07b49e513263..28a1f1143166 100644
> --- a/target/hppa/cpu.c
> +++ b/target/hppa/cpu.c
> @@ -134,7 +134,7 @@ static void hppa_restore_state_to_opc(CPUState *cs,
>  }
>  
>  #ifndef CONFIG_USER_ONLY
> -static bool hppa_cpu_has_work(CPUState *cs)
> +static bool hppa_cpu_has_work(const CPUState *cs)
>  {
>      return cpu_test_interrupt(cs, CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI);
>  }
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 8777bcbae858..93b617f94dce 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -10639,7 +10639,7 @@ int x86_cpu_pending_interrupt(const CPUState *cs, int interrupt_request)
>      return 0;
>  }
>  
> -static bool x86_cpu_has_work(CPUState *cs)
> +static bool x86_cpu_has_work(const CPUState *cs)
>  {
>      return x86_cpu_pending_interrupt(cs, cs->interrupt_request) != 0;
>  }
> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
> index 82ec35e081fd..4c1aae4fb013 100644
> --- a/target/loongarch/cpu.c
> +++ b/target/loongarch/cpu.c
> @@ -96,12 +96,12 @@ bool cpu_loongarch_hw_interrupts_pending(const CPULoongArchState *env)
>  #endif
>  
>  #ifndef CONFIG_USER_ONLY
> -bool loongarch_cpu_has_work(CPUState *cs)
> +bool loongarch_cpu_has_work(const CPUState *cs)
>  {
>      bool has_work = false;
>  
>      if (cpu_test_interrupt(cs, CPU_INTERRUPT_HARD) &&
> -        cpu_loongarch_hw_interrupts_pending(cpu_env(cs))) {
> +        cpu_loongarch_hw_interrupts_pending(cpu_env_const(cs))) {
>          has_work = true;
>      }
>  
> diff --git a/target/loongarch/internals.h b/target/loongarch/internals.h
> index 6a733e6c3a58..d86029bcb31d 100644
> --- a/target/loongarch/internals.h
> +++ b/target/loongarch/internals.h
> @@ -38,7 +38,7 @@ uint64_t cpu_loongarch_get_constant_timer_counter(LoongArchCPU *cpu);
>  uint64_t cpu_loongarch_get_constant_timer_ticks(LoongArchCPU *cpu);
>  void cpu_loongarch_store_constant_timer_config(LoongArchCPU *cpu,
>                                                 uint64_t value);
> -bool loongarch_cpu_has_work(CPUState *cs);
> +bool loongarch_cpu_has_work(const CPUState *cs);
>  bool cpu_loongarch_hw_interrupts_pending(const CPULoongArchState *env);
>  #endif /* !CONFIG_USER_ONLY */
>  
> diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
> index 6012dc3186c7..49ef822cbd9a 100644
> --- a/target/m68k/cpu.c
> +++ b/target/m68k/cpu.c
> @@ -76,7 +76,7 @@ static void m68k_restore_state_to_opc(CPUState *cs,
>  }
>  
>  #ifndef CONFIG_USER_ONLY
> -static bool m68k_cpu_has_work(CPUState *cs)
> +static bool m68k_cpu_has_work(const CPUState *cs)
>  {
>      return cpu_test_interrupt(cs, CPU_INTERRUPT_HARD);
>  }
> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> index 389a5124b124..86692239a1e8 100644
> --- a/target/microblaze/cpu.c
> +++ b/target/microblaze/cpu.c
> @@ -127,7 +127,7 @@ static void mb_restore_state_to_opc(CPUState *cs,
>  }
>  
>  #ifndef CONFIG_USER_ONLY
> -static bool mb_cpu_has_work(CPUState *cs)
> +static bool mb_cpu_has_work(const CPUState *cs)
>  {
>      return cpu_test_interrupt(cs, CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI);
>  }
> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
> index 0fead20d6515..07129731e75c 100644
> --- a/target/mips/cpu.c
> +++ b/target/mips/cpu.c
> @@ -134,9 +134,9 @@ static vaddr mips_cpu_get_pc(CPUState *cs)
>  }
>  
>  #if !defined(CONFIG_USER_ONLY)
> -static bool mips_cpu_has_work(CPUState *cs)
> +static bool mips_cpu_has_work(const CPUState *cs)
>  {
> -    CPUMIPSState *env = cpu_env(cs);
> +    const CPUMIPSState *env = cpu_env_const(cs);
>      bool has_work = false;
>  
>      /*
> diff --git a/target/or1k/cpu.c b/target/or1k/cpu.c
> index 66c00c0930cd..d4d81d0c3512 100644
> --- a/target/or1k/cpu.c
> +++ b/target/or1k/cpu.c
> @@ -76,7 +76,7 @@ static void openrisc_restore_state_to_opc(CPUState *cs,
>  }
>  
>  #ifndef CONFIG_USER_ONLY
> -static bool openrisc_cpu_has_work(CPUState *cs)
> +static bool openrisc_cpu_has_work(const CPUState *cs)
>  {
>      return cpu_test_interrupt(cs, CPU_INTERRUPT_HARD | CPU_INTERRUPT_TIMER);
>  }
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index e3a1075aad3f..c5a29cfee856 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -7224,7 +7224,7 @@ static int ppc_cpu_mmu_index(CPUState *cs, bool ifetch)
>  #endif /* CONFIG_TCG */
>  
>  #ifndef CONFIG_USER_ONLY
> -static bool ppc_cpu_has_work(CPUState *cs)
> +static bool ppc_cpu_has_work(const CPUState *cs)
>  {
>      return cpu_test_interrupt(cs, CPU_INTERRUPT_HARD);
>  }
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index ab2868dc1e2f..b455b997561d 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -950,7 +950,7 @@ int riscv_cpu_vsirq_pending(const CPURISCVState *env)
>                                      (irqs | irqs_f_vs), env->hviprio);
>  }
>  
> -bool riscv_cpu_has_work(CPUState *cs)
> +bool riscv_cpu_has_work(const CPUState *cs)
>  {
>      RISCVCPU *cpu = RISCV_CPU(cs);
>      CPURISCVState *env = &cpu->env;

I guess you should make them const

> diff --git a/target/riscv/internals.h b/target/riscv/internals.h
> index 5d84e4de960a..782f12abfaf0 100644
> --- a/target/riscv/internals.h
> +++ b/target/riscv/internals.h
> @@ -203,7 +203,7 @@ static inline target_ulong get_xepc_mask(CPURISCVState *env)
>  
>  #ifndef CONFIG_USER_ONLY
>  /* Our implementation of SysemuCPUOps::has_work */
> -bool riscv_cpu_has_work(CPUState *cs);
> +bool riscv_cpu_has_work(const CPUState *cs);
>  #endif
>  
>  /* Zjpm addr masking routine */
> diff --git a/target/rx/cpu.c b/target/rx/cpu.c
> index 9b8473d71cf6..bc18dd250cf3 100644
> --- a/target/rx/cpu.c
> +++ b/target/rx/cpu.c
> @@ -73,7 +73,7 @@ static void rx_restore_state_to_opc(CPUState *cs,
>      cpu->env.pc = data[0];
>  }
>  
> -static bool rx_cpu_has_work(CPUState *cs)
> +static bool rx_cpu_has_work(const CPUState *cs)
>  {
>      return cpu_test_interrupt(cs, CPU_INTERRUPT_HARD | CPU_INTERRUPT_FIR);
>  }
> diff --git a/target/s390x/cpu-system.c b/target/s390x/cpu-system.c
> index cc9685221ae8..22ed2b097bbd 100644
> --- a/target/s390x/cpu-system.c
> +++ b/target/s390x/cpu-system.c
> @@ -39,7 +39,7 @@
>  #include "system/tcg.h"
>  #include "hw/core/sysemu-cpu-ops.h"
>  
> -bool s390_cpu_has_work(CPUState *cs)
> +bool s390_cpu_has_work(const CPUState *cs)
>  {
>      S390CPU *cpu = S390_CPU(cs);

same

>  
> diff --git a/target/s390x/s390x-internal.h b/target/s390x/s390x-internal.h
> index 54b62808d834..00917473a544 100644
> --- a/target/s390x/s390x-internal.h
> +++ b/target/s390x/s390x-internal.h
> @@ -248,7 +248,7 @@ bool s390_cpu_system_realize(DeviceState *dev, Error **errp);
>  void s390_cpu_finalize(Object *obj);
>  void s390_cpu_system_class_init(CPUClass *cc);
>  void s390_cpu_machine_reset_cb(void *opaque);
> -bool s390_cpu_has_work(CPUState *cs);
> +bool s390_cpu_has_work(const CPUState *cs);
>  #endif /* CONFIG_USER_ONLY */
>  
>  
> diff --git a/target/sh4/cpu.c b/target/sh4/cpu.c
> index 3bbdee301d5d..36e8b4725aae 100644
> --- a/target/sh4/cpu.c
> +++ b/target/sh4/cpu.c
> @@ -107,7 +107,7 @@ static bool superh_io_recompile_replay_branch(CPUState *cs,
>      return false;
>  }
>  
> -static bool superh_cpu_has_work(CPUState *cs)
> +static bool superh_cpu_has_work(const CPUState *cs)
>  {
>      return cpu_test_interrupt(cs, CPU_INTERRUPT_HARD);
>  }
> diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
> index 1bc14b586bb9..0ecb573129b1 100644
> --- a/target/sparc/cpu.c
> +++ b/target/sparc/cpu.c
> @@ -783,10 +783,10 @@ static void sparc_restore_state_to_opc(CPUState *cs,
>  }
>  
>  #ifndef CONFIG_USER_ONLY
> -static bool sparc_cpu_has_work(CPUState *cs)
> +static bool sparc_cpu_has_work(const CPUState *cs)
>  {
>      return cpu_test_interrupt(cs, CPU_INTERRUPT_HARD) &&
> -           cpu_interrupts_enabled(cpu_env(cs));
> +           cpu_interrupts_enabled(cpu_env_const(cs));
>  }
>  #endif /* !CONFIG_USER_ONLY */
>  
> diff --git a/target/tricore/cpu.c b/target/tricore/cpu.c
> index dcd5c5065bcd..837ecb9b343f 100644
> --- a/target/tricore/cpu.c
> +++ b/target/tricore/cpu.c
> @@ -111,9 +111,9 @@ static void tricore_cpu_reset_hold(Object *obj, ResetType type)
>      cpu_state_reset(cpu_env(cs));
>  }
>  
> -static bool tricore_cpu_has_work(CPUState *cs)
> +static bool tricore_cpu_has_work(const CPUState *cs)
>  {
> -    return true;
> +    return true; /* XXX */
>  }

Add more explicit FIXME?

-- 
Marc-André Lureau <[email protected]>