[PATCH 00/10] accel/tcg: API prototype cleanups

[PATCH 00/10] accel/tcg: API prototype cleanups

[Philippe Mathieu-Daudé]

Preparatory cleanup patches extracted to bigger series which
rename TCG frontend functions to avoid name clash when linking
multiple target into the same binary.

Worth to remove few QOM cast boilerplate, per diff-stat:

 64 files changed, 230 insertions(+), 355 deletions(-)
                   ---                ^^^

Philippe Mathieu-Daudé (10):
  target/mips: Drop left-over comment about Jazz machine
  target/loongarch: Declare loongarch_cpu_dump_state() locally
  target/sparc: Move sparc_restore_state_to_opc() to cpu.c
  accel/tcg: Ensure frontends define restore_state_to_opc handler
  accel/tcg: Move cpu_unwind_state_data() declaration
  accel/tcg: Remove cpu_unwind_state_data() unused CPUState argument
  accel/tcg: Reduce log_pc() declaration scope
  hw/core/cpu: Pass CPUArchState to set/get_pc() handlers
  hw/core/cpu: Pass CPUArchState to restore_state_to_opc() handler
  hw/core/cpu: Pass CPUArchState to cpu_dump_state() handler

 accel/tcg/internal-target.h    | 10 ---------
 include/exec/cpu-common.h      | 13 ------------
 include/exec/translate-all.h   | 11 ++++++++++
 include/hw/core/cpu.h          |  8 +++----
 include/hw/core/tcg-cpu-ops.h  |  2 +-
 target/alpha/cpu.h             |  2 +-
 target/arm/internals.h         |  2 +-
 target/hppa/cpu.h              |  2 +-
 target/i386/cpu.h              |  2 +-
 target/loongarch/internals.h   |  2 --
 target/m68k/cpu.h              |  2 +-
 target/microblaze/cpu.h        |  2 +-
 target/mips/tcg/tcg-internal.h |  2 +-
 target/openrisc/cpu.h          |  2 +-
 target/ppc/cpu.h               |  2 +-
 target/rx/cpu.h                |  2 +-
 target/s390x/s390x-internal.h  |  4 ++--
 target/sh4/cpu.h               |  2 +-
 target/sparc/cpu.h             | 11 +++++++---
 target/tricore/cpu.h           |  2 +-
 target/xtensa/cpu.h            |  2 +-
 accel/tcg/cpu-exec.c           | 25 +++++++++++++++++-----
 accel/tcg/translate-all.c      |  6 +++---
 hw/core/cpu-common.c           |  2 +-
 hw/core/generic-loader.c       |  2 +-
 target/alpha/cpu.c             | 10 +++------
 target/alpha/helper.c          |  3 +--
 target/arm/cpu.c               | 28 ++++++++----------------
 target/avr/cpu.c               | 19 ++++++-----------
 target/hexagon/cpu.c           | 21 +++++++-----------
 target/hppa/cpu.c              | 18 ++++++----------
 target/hppa/helper.c           |  3 +--
 target/hppa/int_helper.c       |  2 +-
 target/hppa/sys_helper.c       |  6 ++----
 target/i386/cpu-dump.c         |  5 ++---
 target/i386/cpu.c              | 12 ++++-------
 target/i386/helper.c           |  3 ++-
 target/i386/tcg/tcg-cpu.c      |  4 +---
 target/loongarch/cpu.c         | 15 ++++++-------
 target/m68k/cpu.c              | 19 ++++++-----------
 target/m68k/translate.c        |  3 +--
 target/microblaze/cpu.c        | 22 +++++++------------
 target/microblaze/translate.c  |  3 +--
 target/mips/cpu.c              | 17 +++++----------
 target/mips/tcg/translate.c    |  4 +---
 target/openrisc/cpu.c          | 24 ++++++++-------------
 target/openrisc/sys_helper.c   |  7 +++---
 target/openrisc/translate.c    |  3 +--
 target/ppc/cpu_init.c          | 23 +++++++-------------
 target/riscv/cpu.c             | 16 ++++----------
 target/riscv/tcg/tcg-cpu.c     |  4 +---
 target/rx/cpu.c                | 18 ++++++----------
 target/rx/translate.c          |  3 +--
 target/s390x/cpu-dump.c        |  3 +--
 target/s390x/cpu.c             | 12 ++++-------
 target/s390x/tcg/translate.c   |  3 +--
 target/sh4/cpu.c               | 20 ++++++-----------
 target/sh4/translate.c         |  3 +--
 target/sparc/cpu.c             | 39 ++++++++++++++++++++++++----------
 target/sparc/translate.c       | 32 ----------------------------
 target/tricore/cpu.c           | 12 +++++------
 target/tricore/translate.c     |  3 +--
 target/xtensa/cpu.c            | 18 ++++++----------
 target/xtensa/translate.c      |  3 +--
 64 files changed, 230 insertions(+), 355 deletions(-)

-- 
2.45.2

[PATCH 01/10] target/mips: Drop left-over comment about Jazz machine

[Philippe Mathieu-Daudé]

Commit 3803b6b427 ("target/mips: Fold jazz behaviour into
mips_cpu_do_transaction_failed") removed update on TCGCPUOps
and commit 119065574d ("hw/core: Constify TCGCPUOps") made
it const. Remove the now irrelevant comment.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/mips/cpu.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index d0a43b6d5c..7c6f438e5d 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -546,10 +546,6 @@ static Property mips_cpu_properties[] = {
 
 #ifdef CONFIG_TCG
 #include "hw/core/tcg-cpu-ops.h"
-/*
- * NB: cannot be const, as some elements are changed for specific
- * mips hardware (see hw/mips/jazz.c).
- */
 static const TCGCPUOps mips_tcg_ops = {
     .initialize = mips_tcg_init,
     .synchronize_from_tb = mips_cpu_synchronize_from_tb,
-- 
2.45.2

Re: [PATCH 01/10] target/mips: Drop left-over comment about Jazz machine

[Peter Maydell]

On Fri, 15 Nov 2024 at 15:21, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Commit 3803b6b427 ("target/mips: Fold jazz behaviour into
> mips_cpu_do_transaction_failed") removed update on TCGCPUOps
> and commit 119065574d ("hw/core: Constify TCGCPUOps") made
> it const. Remove the now irrelevant comment.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 01/10] target/mips: Drop left-over comment about Jazz machine

[Richard Henderson]

On 11/15/24 07:20, Philippe Mathieu-Daudé wrote:
> Commit 3803b6b427 ("target/mips: Fold jazz behaviour into
> mips_cpu_do_transaction_failed") removed update on TCGCPUOps
> and commit 119065574d ("hw/core: Constify TCGCPUOps") made
> it const. Remove the now irrelevant comment.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/mips/cpu.c | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
> index d0a43b6d5c..7c6f438e5d 100644
> --- a/target/mips/cpu.c
> +++ b/target/mips/cpu.c
> @@ -546,10 +546,6 @@ static Property mips_cpu_properties[] = {
>   
>   #ifdef CONFIG_TCG
>   #include "hw/core/tcg-cpu-ops.h"
> -/*
> - * NB: cannot be const, as some elements are changed for specific
> - * mips hardware (see hw/mips/jazz.c).
> - */
>   static const TCGCPUOps mips_tcg_ops = {
>       .initialize = mips_tcg_init,
>       .synchronize_from_tb = mips_cpu_synchronize_from_tb,

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 02/10] target/loongarch: Declare loongarch_cpu_dump_state() locally

[Philippe Mathieu-Daudé]

loongarch_cpu_dump_state() is not used outside of cpu.c,
no need to expose its prototype.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/loongarch/internals.h | 2 --
 target/loongarch/cpu.c       | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/target/loongarch/internals.h b/target/loongarch/internals.h
index 1a02427627..0655ac948b 100644
--- a/target/loongarch/internals.h
+++ b/target/loongarch/internals.h
@@ -18,8 +18,6 @@
 
 void loongarch_translate_init(void);
 
-void loongarch_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
-
 void G_NORETURN do_raise_exception(CPULoongArchState *env,
                                    uint32_t exception,
                                    uintptr_t pc);
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index 57cc4f314b..e599beb30a 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -742,7 +742,7 @@ static ObjectClass *loongarch_cpu_class_by_name(const char *cpu_model)
     return oc;
 }
 
-void loongarch_cpu_dump_state(CPUState *cs, FILE *f, int flags)
+static void loongarch_cpu_dump_state(CPUState *cs, FILE *f, int flags)
 {
     CPULoongArchState *env = cpu_env(cs);
     int i;
-- 
2.45.2

Re: [PATCH 02/10] target/loongarch: Declare loongarch_cpu_dump_state() locally

[Peter Maydell]

On Fri, 15 Nov 2024 at 15:21, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> loongarch_cpu_dump_state() is not used outside of cpu.c,
> no need to expose its prototype.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 02/10] target/loongarch: Declare loongarch_cpu_dump_state() locally

[Richard Henderson]

On 11/15/24 07:20, Philippe Mathieu-Daudé wrote:
> loongarch_cpu_dump_state() is not used outside of cpu.c,
> no need to expose its prototype.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   target/loongarch/internals.h | 2 --
>   target/loongarch/cpu.c       | 2 +-
>   2 files changed, 1 insertion(+), 3 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 03/10] target/sparc: Move sparc_restore_state_to_opc() to cpu.c

[Philippe Mathieu-Daudé]

Most targets define their restore_state_to_opc() handler in cpu.c.
In order to keep SPARC aligned, move sparc_restore_state_to_opc()
from translate.c to cpu.c.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/sparc/cpu.h       | 11 ++++++++---
 target/sparc/cpu.c       | 23 +++++++++++++++++++++++
 target/sparc/translate.c | 32 --------------------------------
 3 files changed, 31 insertions(+), 35 deletions(-)

diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
index f517e5a383..bcb3566a92 100644
--- a/target/sparc/cpu.h
+++ b/target/sparc/cpu.h
@@ -607,12 +607,17 @@ int sparc_cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
                               uint8_t *buf, int len, bool is_write);
 #endif
 
+/* Dynamic PC, must exit to main loop. */
+#define DYNAMIC_PC         1
+/* Dynamic PC, one of two values according to jump_pc[T2]. */
+#define JUMP_PC            2
+/* Dynamic PC, may lookup next TB. */
+#define DYNAMIC_PC_LOOKUP  3
+
+#define DISAS_EXIT  DISAS_TARGET_0
 
 /* translate.c */
 void sparc_tcg_init(void);
-void sparc_restore_state_to_opc(CPUState *cs,
-                                const TranslationBlock *tb,
-                                const uint64_t *data);
 
 /* fop_helper.c */
 target_ulong cpu_get_fsr(CPUSPARCState *);
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index dd7af86de7..59db8c8472 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -713,6 +713,29 @@ static void sparc_cpu_synchronize_from_tb(CPUState *cs,
     cpu->env.npc = tb->cs_base;
 }
 
+static void sparc_restore_state_to_opc(CPUState *cs,
+                                       const TranslationBlock *tb,
+                                       const uint64_t *data)
+{
+    CPUSPARCState *env = cpu_env(cs);
+    target_ulong pc = data[0];
+    target_ulong npc = data[1];
+
+    env->pc = pc;
+    if (npc == DYNAMIC_PC) {
+        /* dynamic NPC: already stored */
+    } else if (npc & JUMP_PC) {
+        /* jump PC: use 'cond' and the jump targets of the translation */
+        if (env->cond) {
+            env->npc = npc & ~3;
+        } else {
+            env->npc = pc + 4;
+        }
+    } else {
+        env->npc = npc;
+    }
+}
+
 static bool sparc_cpu_has_work(CPUState *cs)
 {
     return (cs->interrupt_request & CPU_INTERRUPT_HARD) &&
diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index cdd0a95c03..9942e78412 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -101,15 +101,6 @@
 # define MAXTL_MASK                             0
 #endif
 
-/* Dynamic PC, must exit to main loop. */
-#define DYNAMIC_PC         1
-/* Dynamic PC, one of two values according to jump_pc[T2]. */
-#define JUMP_PC            2
-/* Dynamic PC, may lookup next TB. */
-#define DYNAMIC_PC_LOOKUP  3
-
-#define DISAS_EXIT  DISAS_TARGET_0
-
 /* global register indexes */
 static TCGv_ptr cpu_regwptr;
 static TCGv cpu_pc, cpu_npc;
@@ -5881,26 +5872,3 @@ void sparc_tcg_init(void)
                                          gregnames[i]);
     }
 }
-
-void sparc_restore_state_to_opc(CPUState *cs,
-                                const TranslationBlock *tb,
-                                const uint64_t *data)
-{
-    CPUSPARCState *env = cpu_env(cs);
-    target_ulong pc = data[0];
-    target_ulong npc = data[1];
-
-    env->pc = pc;
-    if (npc == DYNAMIC_PC) {
-        /* dynamic NPC: already stored */
-    } else if (npc & JUMP_PC) {
-        /* jump PC: use 'cond' and the jump targets of the translation */
-        if (env->cond) {
-            env->npc = npc & ~3;
-        } else {
-            env->npc = pc + 4;
-        }
-    } else {
-        env->npc = npc;
-    }
-}
-- 
2.45.2

Re: [PATCH 03/10] target/sparc: Move sparc_restore_state_to_opc() to cpu.c

[Peter Maydell]

On Fri, 15 Nov 2024 at 15:22, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Most targets define their restore_state_to_opc() handler in cpu.c.
> In order to keep SPARC aligned, move sparc_restore_state_to_opc()
> from translate.c to cpu.c.
>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/sparc/cpu.h       | 11 ++++++++---
>  target/sparc/cpu.c       | 23 +++++++++++++++++++++++
>  target/sparc/translate.c | 32 --------------------------------
>  3 files changed, 31 insertions(+), 35 deletions(-)
>
> diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
> index f517e5a383..bcb3566a92 100644
> --- a/target/sparc/cpu.h
> +++ b/target/sparc/cpu.h
> @@ -607,12 +607,17 @@ int sparc_cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
>                                uint8_t *buf, int len, bool is_write);
>  #endif
>
> +/* Dynamic PC, must exit to main loop. */
> +#define DYNAMIC_PC         1
> +/* Dynamic PC, one of two values according to jump_pc[T2]. */
> +#define JUMP_PC            2
> +/* Dynamic PC, may lookup next TB. */
> +#define DYNAMIC_PC_LOOKUP  3
> +
> +#define DISAS_EXIT  DISAS_TARGET_0

Why move the definition of DISAS_EXIT ?
sparc_restore_state_to_opc() doesn't use it, and
the DISAS_ constants are a translate-time-only concept.

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 03/10] target/sparc: Move sparc_restore_state_to_opc() to cpu.c

[Richard Henderson]

On 11/15/24 07:20, Philippe Mathieu-Daudé wrote:
> Most targets define their restore_state_to_opc() handler in cpu.c.
> In order to keep SPARC aligned, move sparc_restore_state_to_opc()
> from translate.c to cpu.c.
> 
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/sparc/cpu.h       | 11 ++++++++---
>   target/sparc/cpu.c       | 23 +++++++++++++++++++++++
>   target/sparc/translate.c | 32 --------------------------------
>   3 files changed, 31 insertions(+), 35 deletions(-)
> 
> diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
> index f517e5a383..bcb3566a92 100644
> --- a/target/sparc/cpu.h
> +++ b/target/sparc/cpu.h
> @@ -607,12 +607,17 @@ int sparc_cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
>                                 uint8_t *buf, int len, bool is_write);
>   #endif
>   
> +/* Dynamic PC, must exit to main loop. */
> +#define DYNAMIC_PC         1
> +/* Dynamic PC, one of two values according to jump_pc[T2]. */
> +#define JUMP_PC            2
> +/* Dynamic PC, may lookup next TB. */
> +#define DYNAMIC_PC_LOOKUP  3

Keep these out of cpu.h.
But frankly, moving the sparc_restore_state_to_opc to an "internal.h" kind of header is 
just as effective and keeps all of the private-to-translate.c things contained.

> +
> +#define DISAS_EXIT  DISAS_TARGET_0

Definitely shouldn't be moved.


r~

[PATCH 04/10] accel/tcg: Ensure frontends define restore_state_to_opc handler

[Philippe Mathieu-Daudé]

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/tcg/cpu-exec.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 8163295f34..033f5fab10 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -1074,11 +1074,15 @@ bool tcg_exec_realizefn(CPUState *cpu, Error **errp)
 
     if (!tcg_target_initialized) {
         /* Check mandatory TCGCPUOps handlers */
+        const TCGCPUOps *tcg_ops = cpu->cc->tcg_ops;
+
+        assert(tcg_ops->restore_state_to_opc);
 #ifndef CONFIG_USER_ONLY
-        assert(cpu->cc->tcg_ops->cpu_exec_halt);
-        assert(cpu->cc->tcg_ops->cpu_exec_interrupt);
+        assert(tcg_ops->cpu_exec_halt);
+        assert(tcg_ops->cpu_exec_interrupt);
 #endif /* !CONFIG_USER_ONLY */
-        cpu->cc->tcg_ops->initialize();
+
+        tcg_ops->initialize();
         tcg_target_initialized = true;
     }
 
-- 
2.45.2

[PATCH 05/10] accel/tcg: Move cpu_unwind_state_data() declaration

[Philippe Mathieu-Daudé]

cpu_unwind_state_data() is specific to TCG accelerator,
move it to "exec/translate-all.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/exec/cpu-common.h    | 13 -------------
 include/exec/translate-all.h | 12 ++++++++++++
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 638dc806a5..b36fbf2a39 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -193,19 +193,6 @@ void tcg_cflags_set(CPUState *cpu, uint32_t flags);
 /* current cflags for hashing/comparison */
 uint32_t curr_cflags(CPUState *cpu);
 
-/**
- * cpu_unwind_state_data:
- * @cpu: the cpu context
- * @host_pc: the host pc within the translation
- * @data: output data
- *
- * Attempt to load the the unwind state for a host pc occurring in
- * translated code.  If @host_pc is not in translated code, the
- * function returns false; otherwise @data is loaded.
- * This is the same unwind info as given to restore_state_to_opc.
- */
-bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data);
-
 /**
  * cpu_restore_state:
  * @cpu: the cpu context
diff --git a/include/exec/translate-all.h b/include/exec/translate-all.h
index 85c9460c7c..f06cfedd52 100644
--- a/include/exec/translate-all.h
+++ b/include/exec/translate-all.h
@@ -21,6 +21,18 @@
 
 #include "exec/exec-all.h"
 
+/**
+ * cpu_unwind_state_data:
+ * @cpu: the cpu context
+ * @host_pc: the host pc within the translation
+ * @data: output data
+ *
+ * Attempt to load the the unwind state for a host pc occurring in
+ * translated code.  If @host_pc is not in translated code, the
+ * function returns false; otherwise @data is loaded.
+ * This is the same unwind info as given to restore_state_to_opc.
+ */
+bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data);
 
 /* translate-all.c */
 void tb_check_watchpoint(CPUState *cpu, uintptr_t retaddr);
-- 
2.45.2

Re: [PATCH 05/10] accel/tcg: Move cpu_unwind_state_data() declaration

[Peter Maydell]

On Fri, 15 Nov 2024 at 15:21, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> cpu_unwind_state_data() is specific to TCG accelerator,
> move it to "exec/translate-all.h".
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/exec/cpu-common.h    | 13 -------------
>  include/exec/translate-all.h | 12 ++++++++++++
>  2 files changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
> index 638dc806a5..b36fbf2a39 100644
> --- a/include/exec/cpu-common.h
> +++ b/include/exec/cpu-common.h
> @@ -193,19 +193,6 @@ void tcg_cflags_set(CPUState *cpu, uint32_t flags);
>  /* current cflags for hashing/comparison */
>  uint32_t curr_cflags(CPUState *cpu);
>
> -/**
> - * cpu_unwind_state_data:
> - * @cpu: the cpu context
> - * @host_pc: the host pc within the translation
> - * @data: output data
> - *
> - * Attempt to load the the unwind state for a host pc occurring in
> - * translated code.  If @host_pc is not in translated code, the
> - * function returns false; otherwise @data is loaded.
> - * This is the same unwind info as given to restore_state_to_opc.
> - */
> -bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data);
> -
>  /**
>   * cpu_restore_state:
>   * @cpu: the cpu context
> diff --git a/include/exec/translate-all.h b/include/exec/translate-all.h
> index 85c9460c7c..f06cfedd52 100644
> --- a/include/exec/translate-all.h
> +++ b/include/exec/translate-all.h
> @@ -21,6 +21,18 @@
>
>  #include "exec/exec-all.h"
>
> +/**
> + * cpu_unwind_state_data:
> + * @cpu: the cpu context
> + * @host_pc: the host pc within the translation
> + * @data: output data
> + *
> + * Attempt to load the the unwind state for a host pc occurring in
> + * translated code.  If @host_pc is not in translated code, the
> + * function returns false; otherwise @data is loaded.
> + * This is the same unwind info as given to restore_state_to_opc.
> + */
> +bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data);
>
>  /* translate-all.c */
>  void tb_check_watchpoint(CPUState *cpu, uintptr_t retaddr);

This function is used by some code in target/i386 and
target/openrisc, but a quick grep suggests that they don't
include translate-all.h or any other header that would pull
it in indirectly.

It also seems a bit odd to move the cpu_unwind_state_data()
prototype but not the similar cpu_restore_state() (which is
also TCG-only).

thanks
-- PMM

Re: [PATCH 05/10] accel/tcg: Move cpu_unwind_state_data() declaration

[Philippe Mathieu-Daudé]

On 15/11/24 15:48, Peter Maydell wrote:
> On Fri, 15 Nov 2024 at 15:21, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> cpu_unwind_state_data() is specific to TCG accelerator,
>> move it to "exec/translate-all.h".
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   include/exec/cpu-common.h    | 13 -------------
>>   include/exec/translate-all.h | 12 ++++++++++++
>>   2 files changed, 12 insertions(+), 13 deletions(-)
>>
>> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
>> index 638dc806a5..b36fbf2a39 100644
>> --- a/include/exec/cpu-common.h
>> +++ b/include/exec/cpu-common.h
>> @@ -193,19 +193,6 @@ void tcg_cflags_set(CPUState *cpu, uint32_t flags);
>>   /* current cflags for hashing/comparison */
>>   uint32_t curr_cflags(CPUState *cpu);
>>
>> -/**
>> - * cpu_unwind_state_data:
>> - * @cpu: the cpu context
>> - * @host_pc: the host pc within the translation
>> - * @data: output data
>> - *
>> - * Attempt to load the the unwind state for a host pc occurring in
>> - * translated code.  If @host_pc is not in translated code, the
>> - * function returns false; otherwise @data is loaded.
>> - * This is the same unwind info as given to restore_state_to_opc.
>> - */
>> -bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data);
>> -
>>   /**
>>    * cpu_restore_state:
>>    * @cpu: the cpu context
>> diff --git a/include/exec/translate-all.h b/include/exec/translate-all.h
>> index 85c9460c7c..f06cfedd52 100644
>> --- a/include/exec/translate-all.h
>> +++ b/include/exec/translate-all.h
>> @@ -21,6 +21,18 @@
>>
>>   #include "exec/exec-all.h"
>>
>> +/**
>> + * cpu_unwind_state_data:
>> + * @cpu: the cpu context
>> + * @host_pc: the host pc within the translation
>> + * @data: output data
>> + *
>> + * Attempt to load the the unwind state for a host pc occurring in
>> + * translated code.  If @host_pc is not in translated code, the
>> + * function returns false; otherwise @data is loaded.
>> + * This is the same unwind info as given to restore_state_to_opc.
>> + */
>> +bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data);
>>
>>   /* translate-all.c */
>>   void tb_check_watchpoint(CPUState *cpu, uintptr_t retaddr);
> 
> This function is used by some code in target/i386 and
> target/openrisc, but a quick grep suggests that they don't
> include translate-all.h or any other header that would pull
> it in indirectly.

Oops, this slipped to the next patch.

> It also seems a bit odd to move the cpu_unwind_state_data()
> prototype but not the similar cpu_restore_state() (which is
> also TCG-only).

OK I'll move them together (clearing "exec/cpu-common.h" is
not easy...)

[PATCH 06/10] accel/tcg: Remove cpu_unwind_state_data() unused CPUState argument

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/exec/translate-all.h | 3 +--
 accel/tcg/translate-all.c    | 2 +-
 target/i386/helper.c         | 3 ++-
 target/openrisc/sys_helper.c | 7 +++----
 4 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/include/exec/translate-all.h b/include/exec/translate-all.h
index f06cfedd52..9303318953 100644
--- a/include/exec/translate-all.h
+++ b/include/exec/translate-all.h
@@ -23,7 +23,6 @@
 
 /**
  * cpu_unwind_state_data:
- * @cpu: the cpu context
  * @host_pc: the host pc within the translation
  * @data: output data
  *
@@ -32,7 +31,7 @@
  * function returns false; otherwise @data is loaded.
  * This is the same unwind info as given to restore_state_to_opc.
  */
-bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data);
+bool cpu_unwind_state_data(uintptr_t host_pc, uint64_t *data);
 
 /* translate-all.c */
 void tb_check_watchpoint(CPUState *cpu, uintptr_t retaddr);
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index fdf6d8ac19..8d5530e341 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -243,7 +243,7 @@ bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc)
     return false;
 }
 
-bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data)
+bool cpu_unwind_state_data(uintptr_t host_pc, uint64_t *data)
 {
     if (in_code_gen_buffer((const void *)(host_pc - tcg_splitwx_diff))) {
         TranslationBlock *tb = tcg_tb_lookup(host_pc);
diff --git a/target/i386/helper.c b/target/i386/helper.c
index 01a268a30b..b848936441 100644
--- a/target/i386/helper.c
+++ b/target/i386/helper.c
@@ -29,6 +29,7 @@
 #endif
 #include "qemu/log.h"
 #ifdef CONFIG_TCG
+#include "exec/translate-all.h"
 #include "tcg/insn-start-words.h"
 #endif
 
@@ -526,7 +527,7 @@ static inline target_ulong get_memio_eip(CPUX86State *env)
     uint64_t data[TARGET_INSN_START_WORDS];
     CPUState *cs = env_cpu(env);
 
-    if (!cpu_unwind_state_data(cs, cs->mem_io_pc, data)) {
+    if (!cpu_unwind_state_data(cs->mem_io_pc, data)) {
         return env->eip;
     }
 
diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c
index 77567afba4..67e1f53fca 100644
--- a/target/openrisc/sys_helper.c
+++ b/target/openrisc/sys_helper.c
@@ -20,7 +20,7 @@
 
 #include "qemu/osdep.h"
 #include "cpu.h"
-#include "exec/exec-all.h"
+#include "exec/translate-all.h"
 #include "exec/helper-proto.h"
 #include "exception.h"
 #ifndef CONFIG_USER_ONLY
@@ -219,7 +219,6 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env, target_ulong rd,
 #ifndef CONFIG_USER_ONLY
     uint64_t data[TARGET_INSN_START_WORDS];
     MachineState *ms = MACHINE(qdev_get_machine());
-    CPUState *cs = env_cpu(env);
     int idx;
 #endif
 
@@ -260,7 +259,7 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env, target_ulong rd,
         return env->evbar;
 
     case TO_SPR(0, 16): /* NPC (equals PC) */
-        if (cpu_unwind_state_data(cs, GETPC(), data)) {
+        if (cpu_unwind_state_data(GETPC(), data)) {
             return data[0];
         }
         return env->pc;
@@ -269,7 +268,7 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env, target_ulong rd,
         return cpu_get_sr(env);
 
     case TO_SPR(0, 18): /* PPC */
-        if (cpu_unwind_state_data(cs, GETPC(), data)) {
+        if (cpu_unwind_state_data(GETPC(), data)) {
             if (data[1] & 2) {
                 return data[0] - 4;
             }
-- 
2.45.2

Re: [PATCH 06/10] accel/tcg: Remove cpu_unwind_state_data() unused CPUState argument

[Peter Maydell]

On Fri, 15 Nov 2024 at 15:23, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/exec/translate-all.h | 3 +--
>  accel/tcg/translate-all.c    | 2 +-
>  target/i386/helper.c         | 3 ++-
>  target/openrisc/sys_helper.c | 7 +++----
>  4 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/include/exec/translate-all.h b/include/exec/translate-all.h
> index f06cfedd52..9303318953 100644
> --- a/include/exec/translate-all.h
> +++ b/include/exec/translate-all.h
> @@ -23,7 +23,6 @@
>
>  /**
>   * cpu_unwind_state_data:
> - * @cpu: the cpu context
>   * @host_pc: the host pc within the translation
>   * @data: output data
>   *
> @@ -32,7 +31,7 @@
>   * function returns false; otherwise @data is loaded.
>   * This is the same unwind info as given to restore_state_to_opc.
>   */
> -bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data);
> +bool cpu_unwind_state_data(uintptr_t host_pc, uint64_t *data);
>
>  /* translate-all.c */
>  void tb_check_watchpoint(CPUState *cpu, uintptr_t retaddr);
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index fdf6d8ac19..8d5530e341 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -243,7 +243,7 @@ bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc)
>      return false;
>  }
>
> -bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data)
> +bool cpu_unwind_state_data(uintptr_t host_pc, uint64_t *data)
>  {
>      if (in_code_gen_buffer((const void *)(host_pc - tcg_splitwx_diff))) {
>          TranslationBlock *tb = tcg_tb_lookup(host_pc);
> diff --git a/target/i386/helper.c b/target/i386/helper.c
> index 01a268a30b..b848936441 100644
> --- a/target/i386/helper.c
> +++ b/target/i386/helper.c
> @@ -29,6 +29,7 @@
>  #endif
>  #include "qemu/log.h"
>  #ifdef CONFIG_TCG
> +#include "exec/translate-all.h"

Ah, here are those includes. These should be in the
previous patch I think.


-- PMM

Re: [PATCH 06/10] accel/tcg: Remove cpu_unwind_state_data() unused CPUState argument

[Richard Henderson]

On 11/15/24 07:20, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/exec/translate-all.h | 3 +--
>   accel/tcg/translate-all.c    | 2 +-
>   target/i386/helper.c         | 3 ++-
>   target/openrisc/sys_helper.c | 7 +++----
>   4 files changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/include/exec/translate-all.h b/include/exec/translate-all.h
> index f06cfedd52..9303318953 100644
> --- a/include/exec/translate-all.h
> +++ b/include/exec/translate-all.h
> @@ -23,7 +23,6 @@
>   
>   /**
>    * cpu_unwind_state_data:
> - * @cpu: the cpu context
>    * @host_pc: the host pc within the translation
>    * @data: output data
>    *
> @@ -32,7 +31,7 @@
>    * function returns false; otherwise @data is loaded.
>    * This is the same unwind info as given to restore_state_to_opc.
>    */
> -bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data);
> +bool cpu_unwind_state_data(uintptr_t host_pc, uint64_t *data);

Hmm.  I wonder if it should be called "cpu_*" at all then?
Worth renaming to "tcg_*" or something?

Anyway,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 06/10] accel/tcg: Remove cpu_unwind_state_data() unused CPUState argument

[Peter Maydell]

On Fri, 15 Nov 2024 at 17:24, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 11/15/24 07:20, Philippe Mathieu-Daudé wrote:
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   include/exec/translate-all.h | 3 +--
> >   accel/tcg/translate-all.c    | 2 +-
> >   target/i386/helper.c         | 3 ++-
> >   target/openrisc/sys_helper.c | 7 +++----
> >   4 files changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/exec/translate-all.h b/include/exec/translate-all.h
> > index f06cfedd52..9303318953 100644
> > --- a/include/exec/translate-all.h
> > +++ b/include/exec/translate-all.h
> > @@ -23,7 +23,6 @@
> >
> >   /**
> >    * cpu_unwind_state_data:
> > - * @cpu: the cpu context
> >    * @host_pc: the host pc within the translation
> >    * @data: output data
> >    *
> > @@ -32,7 +31,7 @@
> >    * function returns false; otherwise @data is loaded.
> >    * This is the same unwind info as given to restore_state_to_opc.
> >    */
> > -bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data);
> > +bool cpu_unwind_state_data(uintptr_t host_pc, uint64_t *data);
>
> Hmm.  I wonder if it should be called "cpu_*" at all then?
> Worth renaming to "tcg_*" or something?

Yes, it's odd, isn't it?

What's the plan for this function in a multi-target
emulation world? At the moment it (or functions it
calls) uses TARGET_INSN_START_WORDS which is a
target-CPU-type-specific value. If in the future we're
going to want it instead to look that up as e.g. some
property of the CPU class then maybe we should keep
passing it the CPU pointer? Or would we instead say
that we'll define TARGET_INSN_START_WORDS as the worst
case for any target, since it's always between 1 and 3,
so it doesn't waste that much space if we have a couple
of extra sleb128 zero values for targets that don't need
all 3 words?

thanks
-- PMM

Re: [PATCH 06/10] accel/tcg: Remove cpu_unwind_state_data() unused CPUState argument

[Philippe Mathieu-Daudé]

+Pierrick & Anton

On 15/11/24 18:33, Peter Maydell wrote:
> On Fri, 15 Nov 2024 at 17:24, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> On 11/15/24 07:20, Philippe Mathieu-Daudé wrote:
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>    include/exec/translate-all.h | 3 +--
>>>    accel/tcg/translate-all.c    | 2 +-
>>>    target/i386/helper.c         | 3 ++-
>>>    target/openrisc/sys_helper.c | 7 +++----
>>>    4 files changed, 7 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/include/exec/translate-all.h b/include/exec/translate-all.h
>>> index f06cfedd52..9303318953 100644
>>> --- a/include/exec/translate-all.h
>>> +++ b/include/exec/translate-all.h
>>> @@ -23,7 +23,6 @@
>>>
>>>    /**
>>>     * cpu_unwind_state_data:
>>> - * @cpu: the cpu context
>>>     * @host_pc: the host pc within the translation
>>>     * @data: output data
>>>     *
>>> @@ -32,7 +31,7 @@
>>>     * function returns false; otherwise @data is loaded.
>>>     * This is the same unwind info as given to restore_state_to_opc.
>>>     */
>>> -bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data);
>>> +bool cpu_unwind_state_data(uintptr_t host_pc, uint64_t *data);
>>
>> Hmm.  I wonder if it should be called "cpu_*" at all then?
>> Worth renaming to "tcg_*" or something?
> 
> Yes, it's odd, isn't it?
> 
> What's the plan for this function in a multi-target
> emulation world? At the moment it (or functions it
> calls) uses TARGET_INSN_START_WORDS which is a
> target-CPU-type-specific value. If in the future we're
> going to want it instead to look that up as e.g. some
> property of the CPU class then maybe we should keep
> passing it the CPU pointer? Or would we instead say
> that we'll define TARGET_INSN_START_WORDS as the worst
> case for any target, since it's always between 1 and 3,
> so it doesn't waste that much space if we have a couple
> of extra sleb128 zero values for targets that don't need
> all 3 words?
> 
> thanks
> -- PMM

[PATCH 07/10] accel/tcg: Reduce log_pc() declaration scope

[Philippe Mathieu-Daudé]

log_pc() is only used in cpu-exec.c, move it there.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/tcg/internal-target.h | 10 ----------
 accel/tcg/cpu-exec.c        | 10 ++++++++++
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/accel/tcg/internal-target.h b/accel/tcg/internal-target.h
index fe109724c6..750b3706b1 100644
--- a/accel/tcg/internal-target.h
+++ b/accel/tcg/internal-target.h
@@ -71,16 +71,6 @@ G_NORETURN void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr);
 
 bool tb_invalidate_phys_page_unwind(tb_page_addr_t addr, uintptr_t pc);
 
-/* Return the current PC from CPU, which may be cached in TB. */
-static inline vaddr log_pc(CPUState *cpu, const TranslationBlock *tb)
-{
-    if (tb_cflags(tb) & CF_PCREL) {
-        return cpu->cc->get_pc(cpu);
-    } else {
-        return tb->pc;
-    }
-}
-
 /**
  * tcg_req_mo:
  * @type: TCGBar
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 033f5fab10..73bc9f00f7 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -433,6 +433,16 @@ const void *HELPER(lookup_tb_ptr)(CPUArchState *env)
     return tb->tc.ptr;
 }
 
+/* Return the current PC from CPU, which may be cached in TB. */
+static inline vaddr log_pc(CPUState *cpu, const TranslationBlock *tb)
+{
+    if (tb_cflags(tb) & CF_PCREL) {
+        return cpu->cc->get_pc(cpu);
+    } else {
+        return tb->pc;
+    }
+}
+
 /* Execute a TB, and fix up the CPU state afterwards if necessary */
 /*
  * Disable CFI checks.
-- 
2.45.2

Re: [PATCH 07/10] accel/tcg: Reduce log_pc() declaration scope

[Peter Maydell]

On Fri, 15 Nov 2024 at 15:22, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> log_pc() is only used in cpu-exec.c, move it there.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 07/10] accel/tcg: Reduce log_pc() declaration scope

[Richard Henderson]

On 11/15/24 07:20, Philippe Mathieu-Daudé wrote:
> log_pc() is only used in cpu-exec.c, move it there.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   accel/tcg/internal-target.h | 10 ----------
>   accel/tcg/cpu-exec.c        | 10 ++++++++++
>   2 files changed, 10 insertions(+), 10 deletions(-)

Last use outside of cpu-exec.c removed in dafa0ecc9785.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

[PATCH 08/10] hw/core/cpu: Pass CPUArchState to set/get_pc() handlers

[Philippe Mathieu-Daudé]

CPUClass set_pc() and get_pc() handlers are target specific.
Rather than passing a generic CPUState and forcing QOM casts,
we can directly pass the target CPUArchState, simplifying.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/core/cpu.h     |  6 +++---
 accel/tcg/cpu-exec.c      |  7 ++++---
 accel/tcg/translate-all.c |  2 +-
 hw/core/generic-loader.c  |  2 +-
 target/alpha/cpu.c        |  6 ++----
 target/arm/cpu.c          | 10 ++--------
 target/avr/cpu.c          | 12 ++++--------
 target/hexagon/cpu.c      |  8 ++++----
 target/hppa/cpu.c         | 14 +++++---------
 target/i386/cpu.c         | 12 ++++--------
 target/loongarch/cpu.c    |  8 ++++----
 target/m68k/cpu.c         | 12 ++++--------
 target/microblaze/cpu.c   | 14 +++++---------
 target/mips/cpu.c         | 10 ++++------
 target/openrisc/cpu.c     | 14 +++++---------
 target/ppc/cpu_init.c     | 12 ++++--------
 target/riscv/cpu.c        | 10 ++--------
 target/rx/cpu.c           | 12 ++++--------
 target/s390x/cpu.c        | 12 ++++--------
 target/sh4/cpu.c          | 12 ++++--------
 target/sparc/cpu.c        | 14 +++++---------
 target/tricore/cpu.c      |  8 ++++----
 target/xtensa/cpu.c       | 12 ++++--------
 23 files changed, 83 insertions(+), 146 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index db8a6fbc6e..70f5f8c3bf 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -160,8 +160,8 @@ struct CPUClass {
     int64_t (*get_arch_id)(CPUState *cpu);
     bool (*cpu_persistent_status)(CPUState *cpu);
     bool (*cpu_enabled_status)(CPUState *cpu);
-    void (*set_pc)(CPUState *cpu, vaddr value);
-    vaddr (*get_pc)(CPUState *cpu);
+    void (*set_pc)(CPUArchState *env, vaddr value);
+    vaddr (*get_pc)(CPUArchState *env);
     int (*gdb_read_register)(CPUState *cpu, GByteArray *buf, int reg);
     int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg);
     vaddr (*gdb_adjust_breakpoint)(CPUState *cpu, vaddr addr);
@@ -972,7 +972,7 @@ static inline void cpu_set_pc(CPUState *cpu, vaddr addr)
 {
     CPUClass *cc = CPU_GET_CLASS(cpu);
 
-    cc->set_pc(cpu, addr);
+    cc->set_pc(cpu_env(cpu), addr);
 }
 
 /**
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 73bc9f00f7..b73607fea0 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -437,7 +437,7 @@ const void *HELPER(lookup_tb_ptr)(CPUArchState *env)
 static inline vaddr log_pc(CPUState *cpu, const TranslationBlock *tb)
 {
     if (tb_cflags(tb) & CF_PCREL) {
-        return cpu->cc->get_pc(cpu);
+        return cpu->cc->get_pc(cpu_env(cpu));
     } else {
         return tb->pc;
     }
@@ -459,13 +459,14 @@ cpu_tb_exec(CPUState *cpu, TranslationBlock *itb, int *tb_exit)
     uintptr_t ret;
     TranslationBlock *last_tb;
     const void *tb_ptr = itb->tc.ptr;
+    CPUArchState *env = cpu_env(cpu);
 
     if (qemu_loglevel_mask(CPU_LOG_TB_CPU | CPU_LOG_EXEC)) {
         log_cpu_exec(log_pc(cpu, itb), cpu, itb);
     }
 
     qemu_thread_jit_execute();
-    ret = tcg_qemu_tb_exec(cpu_env(cpu), tb_ptr);
+    ret = tcg_qemu_tb_exec(env, tb_ptr);
     cpu->neg.can_do_io = true;
     qemu_plugin_disable_mem_helpers(cpu);
     /*
@@ -494,7 +495,7 @@ cpu_tb_exec(CPUState *cpu, TranslationBlock *itb, int *tb_exit)
         } else {
             tcg_debug_assert(!(tb_cflags(last_tb) & CF_PCREL));
             assert(cc->set_pc);
-            cc->set_pc(cpu, last_tb->pc);
+            cc->set_pc(env, last_tb->pc);
         }
         if (qemu_loglevel_mask(CPU_LOG_EXEC)) {
             vaddr pc = log_pc(cpu, last_tb);
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 8d5530e341..375100b483 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -634,7 +634,7 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr)
     cpu->cflags_next_tb = curr_cflags(cpu) | CF_MEMI_ONLY | n;
 
     if (qemu_loglevel_mask(CPU_LOG_EXEC)) {
-        vaddr pc = cpu->cc->get_pc(cpu);
+        vaddr pc = cpu->cc->get_pc(cpu_env(cpu));
         if (qemu_log_in_addr_range(pc)) {
             qemu_log("cpu_io_recompile: rewound execution of TB to %016"
                      VADDR_PRIx "\n", pc);
diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c
index ea8628b892..32cb5d9639 100644
--- a/hw/core/generic-loader.c
+++ b/hw/core/generic-loader.c
@@ -51,7 +51,7 @@ static void generic_loader_reset(void *opaque)
         CPUClass *cc = CPU_GET_CLASS(s->cpu);
         cpu_reset(s->cpu);
         if (cc) {
-            cc->set_pc(s->cpu, s->addr);
+            cc->set_pc(cpu_env(s->cpu), s->addr);
         }
     }
 
diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index 5d75c941f7..dce7a3ea5d 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -27,15 +27,13 @@
 #include "fpu/softfloat.h"
 
 
-static void alpha_cpu_set_pc(CPUState *cs, vaddr value)
+static void alpha_cpu_set_pc(CPUAlphaState *env, vaddr value)
 {
-    CPUAlphaState *env = cpu_env(cs);
     env->pc = value;
 }
 
-static vaddr alpha_cpu_get_pc(CPUState *cs)
+static vaddr alpha_cpu_get_pc(CPUAlphaState *env)
 {
-    CPUAlphaState *env = cpu_env(cs);
     return env->pc;
 }
 
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 6938161b95..b7cf084019 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -51,11 +51,8 @@
 #include "target/arm/cpu-qom.h"
 #include "target/arm/gtimer.h"
 
-static void arm_cpu_set_pc(CPUState *cs, vaddr value)
+static void arm_cpu_set_pc(CPUARMState *env, vaddr value)
 {
-    ARMCPU *cpu = ARM_CPU(cs);
-    CPUARMState *env = &cpu->env;
-
     if (is_a64(env)) {
         env->pc = value;
         env->thumb = false;
@@ -65,11 +62,8 @@ static void arm_cpu_set_pc(CPUState *cs, vaddr value)
     }
 }
 
-static vaddr arm_cpu_get_pc(CPUState *cs)
+static vaddr arm_cpu_get_pc(CPUARMState *env)
 {
-    ARMCPU *cpu = ARM_CPU(cs);
-    CPUARMState *env = &cpu->env;
-
     if (is_a64(env)) {
         return env->pc;
     } else {
diff --git a/target/avr/cpu.c b/target/avr/cpu.c
index 3132842d56..e85e54feb8 100644
--- a/target/avr/cpu.c
+++ b/target/avr/cpu.c
@@ -27,18 +27,14 @@
 #include "tcg/debug-assert.h"
 #include "hw/qdev-properties.h"
 
-static void avr_cpu_set_pc(CPUState *cs, vaddr value)
+static void avr_cpu_set_pc(CPUAVRState *env, vaddr value)
 {
-    AVRCPU *cpu = AVR_CPU(cs);
-
-    cpu->env.pc_w = value / 2; /* internally PC points to words */
+    env->pc_w = value / 2; /* internally PC points to words */
 }
 
-static vaddr avr_cpu_get_pc(CPUState *cs)
+static vaddr avr_cpu_get_pc(CPUAVRState *env)
 {
-    AVRCPU *cpu = AVR_CPU(cs);
-
-    return cpu->env.pc_w * 2;
+    return env->pc_w * 2;
 }
 
 static bool avr_cpu_has_work(CPUState *cs)
diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
index 020038fc49..828b7d1df3 100644
--- a/target/hexagon/cpu.c
+++ b/target/hexagon/cpu.c
@@ -245,14 +245,14 @@ void hexagon_debug(CPUHexagonState *env)
     hexagon_dump(env, stdout, CPU_DUMP_FPU);
 }
 
-static void hexagon_cpu_set_pc(CPUState *cs, vaddr value)
+static void hexagon_cpu_set_pc(CPUHexagonState *env, vaddr value)
 {
-    cpu_env(cs)->gpr[HEX_REG_PC] = value;
+    env->gpr[HEX_REG_PC] = value;
 }
 
-static vaddr hexagon_cpu_get_pc(CPUState *cs)
+static vaddr hexagon_cpu_get_pc(CPUHexagonState *env)
 {
-    return cpu_env(cs)->gpr[HEX_REG_PC];
+    return env->gpr[HEX_REG_PC];
 }
 
 static void hexagon_cpu_synchronize_from_tb(CPUState *cs,
diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
index c38439c180..d73a88b279 100644
--- a/target/hppa/cpu.c
+++ b/target/hppa/cpu.c
@@ -28,21 +28,17 @@
 #include "fpu/softfloat.h"
 #include "tcg/tcg.h"
 
-static void hppa_cpu_set_pc(CPUState *cs, vaddr value)
+static void hppa_cpu_set_pc(CPUHPPAState *env, vaddr value)
 {
-    HPPACPU *cpu = HPPA_CPU(cs);
-
 #ifdef CONFIG_USER_ONLY
     value |= PRIV_USER;
 #endif
-    cpu->env.iaoq_f = value;
-    cpu->env.iaoq_b = value + 4;
+    env->iaoq_f = value;
+    env->iaoq_b = value + 4;
 }
 
-static vaddr hppa_cpu_get_pc(CPUState *cs)
+static vaddr hppa_cpu_get_pc(CPUHPPAState *env)
 {
-    CPUHPPAState *env = cpu_env(cs);
-
     return hppa_form_gva_psw(env->psw, (env->psw & PSW_C ? env->iasq_f : 0),
                              env->iaoq_f & -4);
 }
@@ -59,7 +55,7 @@ void cpu_get_tb_cpu_state(CPUHPPAState *env, vaddr *pc,
      * incomplete virtual address.  This also means that we must separate
      * out current cpu privilege from the low bits of IAOQ_F.
      */
-    *pc = hppa_cpu_get_pc(env_cpu(env));
+    *pc = hppa_cpu_get_pc(env);
     flags |= (env->iaoq_f & 3) << TB_FLAG_PRIV_SHIFT;
 
     /*
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 3725dbbc4b..5f063b18c4 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -8251,19 +8251,15 @@ static bool x86_cpu_get_paging_enabled(const CPUState *cs)
 }
 #endif /* !CONFIG_USER_ONLY */
 
-static void x86_cpu_set_pc(CPUState *cs, vaddr value)
+static void x86_cpu_set_pc(CPUX86State *env, vaddr value)
 {
-    X86CPU *cpu = X86_CPU(cs);
-
-    cpu->env.eip = value;
+    env->eip = value;
 }
 
-static vaddr x86_cpu_get_pc(CPUState *cs)
+static vaddr x86_cpu_get_pc(CPUX86State *env)
 {
-    X86CPU *cpu = X86_CPU(cs);
-
     /* Match cpu_get_tb_cpu_state. */
-    return cpu->env.eip + cpu->env.segs[R_CS].base;
+    return env->eip + env->segs[R_CS].base;
 }
 
 int x86_cpu_pending_interrupt(CPUState *cs, int interrupt_request)
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index e599beb30a..add7323f05 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -101,14 +101,14 @@ void G_NORETURN do_raise_exception(CPULoongArchState *env,
     cpu_loop_exit_restore(cs, pc);
 }
 
-static void loongarch_cpu_set_pc(CPUState *cs, vaddr value)
+static void loongarch_cpu_set_pc(CPULoongArchState *env, vaddr value)
 {
-    set_pc(cpu_env(cs), value);
+    set_pc(env, value);
 }
 
-static vaddr loongarch_cpu_get_pc(CPUState *cs)
+static vaddr loongarch_cpu_get_pc(CPULoongArchState *env)
 {
-    return cpu_env(cs)->pc;
+    return env->pc;
 }
 
 #ifndef CONFIG_USER_ONLY
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index 5fe335558a..39bf6f3d90 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -24,18 +24,14 @@
 #include "migration/vmstate.h"
 #include "fpu/softfloat.h"
 
-static void m68k_cpu_set_pc(CPUState *cs, vaddr value)
+static void m68k_cpu_set_pc(CPUM68KState *env, vaddr value)
 {
-    M68kCPU *cpu = M68K_CPU(cs);
-
-    cpu->env.pc = value;
+    env->pc = value;
 }
 
-static vaddr m68k_cpu_get_pc(CPUState *cs)
+static vaddr m68k_cpu_get_pc(CPUM68KState *env)
 {
-    M68kCPU *cpu = M68K_CPU(cs);
-
-    return cpu->env.pc;
+    return env->pc;
 }
 
 static void m68k_restore_state_to_opc(CPUState *cs,
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index 710eb1146c..3e68c73898 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -78,20 +78,16 @@ static const struct {
 /* If no specific version gets selected, default to the following.  */
 #define DEFAULT_CPU_VERSION "10.0"
 
-static void mb_cpu_set_pc(CPUState *cs, vaddr value)
+static void mb_cpu_set_pc(CPUMBState *env, vaddr value)
 {
-    MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
-
-    cpu->env.pc = value;
+    env->pc = value;
     /* Ensure D_FLAG and IMM_FLAG are clear for the new PC */
-    cpu->env.iflags = 0;
+    env->iflags = 0;
 }
 
-static vaddr mb_cpu_get_pc(CPUState *cs)
+static vaddr mb_cpu_get_pc(CPUMBState *env)
 {
-    MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
-
-    return cpu->env.pc;
+    return env->pc;
 }
 
 static void mb_cpu_synchronize_from_tb(CPUState *cs,
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index 7c6f438e5d..506494f7e6 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -120,16 +120,14 @@ void cpu_set_exception_base(int vp_index, target_ulong address)
     vp->env.exception_base = address;
 }
 
-static void mips_cpu_set_pc(CPUState *cs, vaddr value)
+static void mips_cpu_set_pc(CPUMIPSState *env, vaddr value)
 {
-    mips_env_set_pc(cpu_env(cs), value);
+    mips_env_set_pc(env, value);
 }
 
-static vaddr mips_cpu_get_pc(CPUState *cs)
+static vaddr mips_cpu_get_pc(CPUMIPSState *env)
 {
-    MIPSCPU *cpu = MIPS_CPU(cs);
-
-    return cpu->env.active_tc.PC;
+    return env->active_tc.PC;
 }
 
 static bool mips_cpu_has_work(CPUState *cs)
diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
index b96561d1f2..51ab0df82b 100644
--- a/target/openrisc/cpu.c
+++ b/target/openrisc/cpu.c
@@ -25,19 +25,15 @@
 #include "fpu/softfloat-helpers.h"
 #include "tcg/tcg.h"
 
-static void openrisc_cpu_set_pc(CPUState *cs, vaddr value)
+static void openrisc_cpu_set_pc(CPUOpenRISCState *env, vaddr value)
 {
-    OpenRISCCPU *cpu = OPENRISC_CPU(cs);
-
-    cpu->env.pc = value;
-    cpu->env.dflag = 0;
+    env->pc = value;
+    env->dflag = 0;
 }
 
-static vaddr openrisc_cpu_get_pc(CPUState *cs)
+static vaddr openrisc_cpu_get_pc(CPUOpenRISCState *env)
 {
-    OpenRISCCPU *cpu = OPENRISC_CPU(cs);
-
-    return cpu->env.pc;
+    return env->pc;
 }
 
 static void openrisc_cpu_synchronize_from_tb(CPUState *cs,
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index efcb80d1c2..c8b4445aea 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -7152,18 +7152,14 @@ void ppc_cpu_list(void)
 #endif
 }
 
-static void ppc_cpu_set_pc(CPUState *cs, vaddr value)
+static void ppc_cpu_set_pc(CPUPPCState *env, vaddr value)
 {
-    PowerPCCPU *cpu = POWERPC_CPU(cs);
-
-    cpu->env.nip = value;
+    env->nip = value;
 }
 
-static vaddr ppc_cpu_get_pc(CPUState *cs)
+static vaddr ppc_cpu_get_pc(CPUPPCState *env)
 {
-    PowerPCCPU *cpu = POWERPC_CPU(cs);
-
-    return cpu->env.nip;
+    return env->nip;
 }
 
 #ifdef CONFIG_TCG
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index f219f0c3b5..dfaa9a9c1c 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -878,11 +878,8 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int flags)
     }
 }
 
-static void riscv_cpu_set_pc(CPUState *cs, vaddr value)
+static void riscv_cpu_set_pc(CPURISCVState *env, vaddr value)
 {
-    RISCVCPU *cpu = RISCV_CPU(cs);
-    CPURISCVState *env = &cpu->env;
-
     if (env->xl == MXL_RV32) {
         env->pc = (int32_t)value;
     } else {
@@ -890,11 +887,8 @@ static void riscv_cpu_set_pc(CPUState *cs, vaddr value)
     }
 }
 
-static vaddr riscv_cpu_get_pc(CPUState *cs)
+static vaddr riscv_cpu_get_pc(CPURISCVState *env)
 {
-    RISCVCPU *cpu = RISCV_CPU(cs);
-    CPURISCVState *env = &cpu->env;
-
     /* Match cpu_get_tb_cpu_state. */
     if (env->xl == MXL_RV32) {
         return env->pc & UINT32_MAX;
diff --git a/target/rx/cpu.c b/target/rx/cpu.c
index 65a74ce720..0c4b63b114 100644
--- a/target/rx/cpu.c
+++ b/target/rx/cpu.c
@@ -27,18 +27,14 @@
 #include "fpu/softfloat.h"
 #include "tcg/debug-assert.h"
 
-static void rx_cpu_set_pc(CPUState *cs, vaddr value)
+static void rx_cpu_set_pc(CPURXState *env, vaddr value)
 {
-    RXCPU *cpu = RX_CPU(cs);
-
-    cpu->env.pc = value;
+    env->pc = value;
 }
 
-static vaddr rx_cpu_get_pc(CPUState *cs)
+static vaddr rx_cpu_get_pc(CPURXState *env)
 {
-    RXCPU *cpu = RX_CPU(cs);
-
-    return cpu->env.pc;
+    return env->pc;
 }
 
 static void rx_cpu_synchronize_from_tb(CPUState *cs,
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 514c70f301..12975385e0 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -112,18 +112,14 @@ uint64_t s390_cpu_get_psw_mask(CPUS390XState *env)
     return r;
 }
 
-static void s390_cpu_set_pc(CPUState *cs, vaddr value)
+static void s390_cpu_set_pc(CPUS390XState *env, vaddr value)
 {
-    S390CPU *cpu = S390_CPU(cs);
-
-    cpu->env.psw.addr = value;
+    env->psw.addr = value;
 }
 
-static vaddr s390_cpu_get_pc(CPUState *cs)
+static vaddr s390_cpu_get_pc(CPUS390XState *env)
 {
-    S390CPU *cpu = S390_CPU(cs);
-
-    return cpu->env.psw.addr;
+    return env->psw.addr;
 }
 
 static bool s390_cpu_has_work(CPUState *cs)
diff --git a/target/sh4/cpu.c b/target/sh4/cpu.c
index 8f07261dcf..5c6840841b 100644
--- a/target/sh4/cpu.c
+++ b/target/sh4/cpu.c
@@ -28,18 +28,14 @@
 #include "fpu/softfloat-helpers.h"
 #include "tcg/tcg.h"
 
-static void superh_cpu_set_pc(CPUState *cs, vaddr value)
+static void superh_cpu_set_pc(CPUSH4State *env, vaddr value)
 {
-    SuperHCPU *cpu = SUPERH_CPU(cs);
-
-    cpu->env.pc = value;
+    env->pc = value;
 }
 
-static vaddr superh_cpu_get_pc(CPUState *cs)
+static vaddr superh_cpu_get_pc(CPUSH4State *env)
 {
-    SuperHCPU *cpu = SUPERH_CPU(cs);
-
-    return cpu->env.pc;
+    return env->pc;
 }
 
 static void superh_cpu_synchronize_from_tb(CPUState *cs,
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index 59db8c8472..e1f0dfcbbd 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -688,19 +688,15 @@ static void sparc_cpu_dump_state(CPUState *cs, FILE *f, int flags)
     qemu_fprintf(f, "\n");
 }
 
-static void sparc_cpu_set_pc(CPUState *cs, vaddr value)
+static void sparc_cpu_set_pc(CPUSPARCState *env, vaddr value)
 {
-    SPARCCPU *cpu = SPARC_CPU(cs);
-
-    cpu->env.pc = value;
-    cpu->env.npc = value + 4;
+    env->pc = value;
+    env->npc = value + 4;
 }
 
-static vaddr sparc_cpu_get_pc(CPUState *cs)
+static vaddr sparc_cpu_get_pc(CPUSPARCState *env)
 {
-    SPARCCPU *cpu = SPARC_CPU(cs);
-
-    return cpu->env.pc;
+    return env->pc;
 }
 
 static void sparc_cpu_synchronize_from_tb(CPUState *cs,
diff --git a/target/tricore/cpu.c b/target/tricore/cpu.c
index 1a26171590..20de29d114 100644
--- a/target/tricore/cpu.c
+++ b/target/tricore/cpu.c
@@ -34,14 +34,14 @@ static const gchar *tricore_gdb_arch_name(CPUState *cs)
     return "tricore";
 }
 
-static void tricore_cpu_set_pc(CPUState *cs, vaddr value)
+static void tricore_cpu_set_pc(CPUTriCoreState *env, vaddr value)
 {
-    cpu_env(cs)->PC = value & ~(target_ulong)1;
+    env->PC = value & ~(target_ulong)1;
 }
 
-static vaddr tricore_cpu_get_pc(CPUState *cs)
+static vaddr tricore_cpu_get_pc(CPUTriCoreState *env)
 {
-    return cpu_env(cs)->PC;
+    return env->PC;
 }
 
 static void tricore_cpu_synchronize_from_tb(CPUState *cs,
diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
index 6f9039abae..3ab7b794f0 100644
--- a/target/xtensa/cpu.c
+++ b/target/xtensa/cpu.c
@@ -40,18 +40,14 @@
 #endif
 
 
-static void xtensa_cpu_set_pc(CPUState *cs, vaddr value)
+static void xtensa_cpu_set_pc(CPUXtensaState *env, vaddr value)
 {
-    XtensaCPU *cpu = XTENSA_CPU(cs);
-
-    cpu->env.pc = value;
+    env->pc = value;
 }
 
-static vaddr xtensa_cpu_get_pc(CPUState *cs)
+static vaddr xtensa_cpu_get_pc(CPUXtensaState *env)
 {
-    XtensaCPU *cpu = XTENSA_CPU(cs);
-
-    return cpu->env.pc;
+    return env->pc;
 }
 
 static void xtensa_restore_state_to_opc(CPUState *cs,
-- 
2.45.2

Re: [PATCH 08/10] hw/core/cpu: Pass CPUArchState to set/get_pc() handlers

[Peter Maydell]

On Fri, 15 Nov 2024 at 15:22, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> CPUClass set_pc() and get_pc() handlers are target specific.
> Rather than passing a generic CPUState and forcing QOM casts,
> we can directly pass the target CPUArchState, simplifying.

> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index db8a6fbc6e..70f5f8c3bf 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -160,8 +160,8 @@ struct CPUClass {
>      int64_t (*get_arch_id)(CPUState *cpu);
>      bool (*cpu_persistent_status)(CPUState *cpu);
>      bool (*cpu_enabled_status)(CPUState *cpu);
> -    void (*set_pc)(CPUState *cpu, vaddr value);
> -    vaddr (*get_pc)(CPUState *cpu);
> +    void (*set_pc)(CPUArchState *env, vaddr value);
> +    vaddr (*get_pc)(CPUArchState *env);
>      int (*gdb_read_register)(CPUState *cpu, GByteArray *buf, int reg);
>      int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg);
>      vaddr (*gdb_adjust_breakpoint)(CPUState *cpu, vaddr addr);

This is effectively the table of methods for the CPUClass
class. I think that methods on class A should take a pointer to the
object of that type, not to something else.

thanks
-- PMM

Re: [PATCH 08/10] hw/core/cpu: Pass CPUArchState to set/get_pc() handlers

[Paolo Bonzini]

On 11/15/24 16:54, Peter Maydell wrote:
> On Fri, 15 Nov 2024 at 15:22, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> CPUClass set_pc() and get_pc() handlers are target specific.
>> Rather than passing a generic CPUState and forcing QOM casts,
>> we can directly pass the target CPUArchState, simplifying.
> 
>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
>> index db8a6fbc6e..70f5f8c3bf 100644
>> --- a/include/hw/core/cpu.h
>> +++ b/include/hw/core/cpu.h
>> @@ -160,8 +160,8 @@ struct CPUClass {
>>       int64_t (*get_arch_id)(CPUState *cpu);
>>       bool (*cpu_persistent_status)(CPUState *cpu);
>>       bool (*cpu_enabled_status)(CPUState *cpu);
>> -    void (*set_pc)(CPUState *cpu, vaddr value);
>> -    vaddr (*get_pc)(CPUState *cpu);
>> +    void (*set_pc)(CPUArchState *env, vaddr value);
>> +    vaddr (*get_pc)(CPUArchState *env);
>>       int (*gdb_read_register)(CPUState *cpu, GByteArray *buf, int reg);
>>       int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg);
>>       vaddr (*gdb_adjust_breakpoint)(CPUState *cpu, vaddr addr);
> 
> This is effectively the table of methods for the CPUClass
> class. I think that methods on class A should take a pointer to the
> object of that type, not to something else.

Yeah, the diffstat is enticing but I agree.  It's particularly confusing 
because there isn't a single type called CPUArchState*, it's different 
depending on which target/ subdirectory you're compiling. In a 
multi-target binary it'd have to be void*, and then you get back the casts.


Paolo

Re: [PATCH 08/10] hw/core/cpu: Pass CPUArchState to set/get_pc() handlers

[Richard Henderson]

On 11/15/24 08:21, Paolo Bonzini wrote:
> On 11/15/24 16:54, Peter Maydell wrote:
>> On Fri, 15 Nov 2024 at 15:22, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>>
>>> CPUClass set_pc() and get_pc() handlers are target specific.
>>> Rather than passing a generic CPUState and forcing QOM casts,
>>> we can directly pass the target CPUArchState, simplifying.
>>
>>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
>>> index db8a6fbc6e..70f5f8c3bf 100644
>>> --- a/include/hw/core/cpu.h
>>> +++ b/include/hw/core/cpu.h
>>> @@ -160,8 +160,8 @@ struct CPUClass {
>>>       int64_t (*get_arch_id)(CPUState *cpu);
>>>       bool (*cpu_persistent_status)(CPUState *cpu);
>>>       bool (*cpu_enabled_status)(CPUState *cpu);
>>> -    void (*set_pc)(CPUState *cpu, vaddr value);
>>> -    vaddr (*get_pc)(CPUState *cpu);
>>> +    void (*set_pc)(CPUArchState *env, vaddr value);
>>> +    vaddr (*get_pc)(CPUArchState *env);
>>>       int (*gdb_read_register)(CPUState *cpu, GByteArray *buf, int reg);
>>>       int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg);
>>>       vaddr (*gdb_adjust_breakpoint)(CPUState *cpu, vaddr addr);
>>
>> This is effectively the table of methods for the CPUClass
>> class. I think that methods on class A should take a pointer to the
>> object of that type, not to something else.
> 
> Yeah, the diffstat is enticing but I agree.  It's particularly confusing because there 
> isn't a single type called CPUArchState*, it's different depending on which target/ 
> subdirectory you're compiling. In a multi-target binary it'd have to be void*, and then 
> you get back the casts.

Likewise.  Any cleanup should be to swap the QOM casts with cpu_env() within the 
individual methods.


r~

[PATCH 09/10] hw/core/cpu: Pass CPUArchState to restore_state_to_opc() handler

[Philippe Mathieu-Daudé]

CPUClass::restore_state_to_opc() handler is target specific.
Rather than passing a generic CPUState and forcing QOM casts,
we can directly pass the target CPUArchState, simplifying.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/core/tcg-cpu-ops.h  |  2 +-
 target/arm/internals.h         |  2 +-
 target/mips/tcg/tcg-internal.h |  2 +-
 target/s390x/s390x-internal.h  |  2 +-
 accel/tcg/translate-all.c      |  2 +-
 target/alpha/cpu.c             |  4 +---
 target/arm/cpu.c               |  4 +---
 target/avr/cpu.c               |  4 ++--
 target/hexagon/cpu.c           |  4 ++--
 target/hppa/cpu.c              |  4 +---
 target/i386/tcg/tcg-cpu.c      |  4 +---
 target/loongarch/cpu.c         |  4 ++--
 target/m68k/cpu.c              |  7 +++----
 target/microblaze/cpu.c        |  8 +++-----
 target/mips/tcg/translate.c    |  4 +---
 target/openrisc/cpu.c          | 10 ++++------
 target/ppc/cpu_init.c          |  6 ++----
 target/riscv/tcg/tcg-cpu.c     |  4 +---
 target/rx/cpu.c                |  6 ++----
 target/s390x/tcg/translate.c   |  3 +--
 target/sh4/cpu.c               |  8 +++-----
 target/sparc/cpu.c             |  3 +--
 target/tricore/cpu.c           |  4 ++--
 target/xtensa/cpu.c            |  6 ++----
 24 files changed, 40 insertions(+), 67 deletions(-)

diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
index 663efb9133..a15ff36dd7 100644
--- a/include/hw/core/tcg-cpu-ops.h
+++ b/include/hw/core/tcg-cpu-ops.h
@@ -44,7 +44,7 @@ struct TCGCPUOps {
      * state which are tracked insn-by-insn in the target-specific
      * arguments to start_insn, passed as @data.
      */
-    void (*restore_state_to_opc)(CPUState *cpu, const TranslationBlock *tb,
+    void (*restore_state_to_opc)(CPUArchState *env, const TranslationBlock *tb,
                                  const uint64_t *data);
 
     /** @cpu_exec_enter: Callback for cpu_exec preparation */
diff --git a/target/arm/internals.h b/target/arm/internals.h
index e37f459af3..b7b15800e8 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -362,7 +362,7 @@ void arm_cpu_register_gdb_commands(ARMCPU *cpu);
 void aarch64_cpu_register_gdb_commands(ARMCPU *cpu, GString *,
                                        GPtrArray *, GPtrArray *);
 
-void arm_restore_state_to_opc(CPUState *cs,
+void arm_restore_state_to_opc(CPUARMState *env,
                               const TranslationBlock *tb,
                               const uint64_t *data);
 
diff --git a/target/mips/tcg/tcg-internal.h b/target/mips/tcg/tcg-internal.h
index aef032c48d..79d39801a6 100644
--- a/target/mips/tcg/tcg-internal.h
+++ b/target/mips/tcg/tcg-internal.h
@@ -21,7 +21,7 @@ void mips_cpu_synchronize_from_tb(CPUState *cs, const TranslationBlock *tb);
 G_NORETURN void mips_cpu_do_unaligned_access(CPUState *cpu, vaddr addr,
                                              MMUAccessType access_type, int mmu_idx,
                                              uintptr_t retaddr);
-void mips_restore_state_to_opc(CPUState *cs,
+void mips_restore_state_to_opc(CPUMIPSState *env,
                                const TranslationBlock *tb,
                                const uint64_t *data);
 
diff --git a/target/s390x/s390x-internal.h b/target/s390x/s390x-internal.h
index 825252d728..891e5f576c 100644
--- a/target/s390x/s390x-internal.h
+++ b/target/s390x/s390x-internal.h
@@ -399,7 +399,7 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3,
 
 /* translate.c */
 void s390x_translate_init(void);
-void s390x_restore_state_to_opc(CPUState *cs,
+void s390x_restore_state_to_opc(CPUS390XState *env,
                                 const TranslationBlock *tb,
                                 const uint64_t *data);
 
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 375100b483..264bc968e7 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -218,7 +218,7 @@ void cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb,
         cpu->neg.icount_decr.u16.low += insns_left;
     }
 
-    cpu->cc->tcg_ops->restore_state_to_opc(cpu, tb, data);
+    cpu->cc->tcg_ops->restore_state_to_opc(cpu_env(cpu), tb, data);
 }
 
 bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc)
diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index dce7a3ea5d..2b55bb0bd9 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -47,12 +47,10 @@ static void alpha_cpu_synchronize_from_tb(CPUState *cs,
     }
 }
 
-static void alpha_restore_state_to_opc(CPUState *cs,
+static void alpha_restore_state_to_opc(CPUAlphaState *env,
                                        const TranslationBlock *tb,
                                        const uint64_t *data)
 {
-    CPUAlphaState *env = cpu_env(cs);
-
     if (tb_cflags(tb) & CF_PCREL) {
         env->pc = (env->pc & TARGET_PAGE_MASK) | data[0];
     } else {
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index b7cf084019..c8e032d433 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -90,12 +90,10 @@ void arm_cpu_synchronize_from_tb(CPUState *cs,
     }
 }
 
-void arm_restore_state_to_opc(CPUState *cs,
+void arm_restore_state_to_opc(CPUARMState *env,
                               const TranslationBlock *tb,
                               const uint64_t *data)
 {
-    CPUARMState *env = cpu_env(cs);
-
     if (is_a64(env)) {
         if (tb_cflags(tb) & CF_PCREL) {
             env->pc = (env->pc & TARGET_PAGE_MASK) | data[0];
diff --git a/target/avr/cpu.c b/target/avr/cpu.c
index e85e54feb8..19b6298a31 100644
--- a/target/avr/cpu.c
+++ b/target/avr/cpu.c
@@ -55,11 +55,11 @@ static void avr_cpu_synchronize_from_tb(CPUState *cs,
     cpu_env(cs)->pc_w = tb->pc / 2; /* internally PC points to words */
 }
 
-static void avr_restore_state_to_opc(CPUState *cs,
+static void avr_restore_state_to_opc(CPUAVRState *env,
                                      const TranslationBlock *tb,
                                      const uint64_t *data)
 {
-    cpu_env(cs)->pc_w = data[0];
+    env->pc_w = data[0];
 }
 
 static void avr_cpu_reset_hold(Object *obj, ResetType type)
diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
index 828b7d1df3..8038df1c82 100644
--- a/target/hexagon/cpu.c
+++ b/target/hexagon/cpu.c
@@ -267,11 +267,11 @@ static bool hexagon_cpu_has_work(CPUState *cs)
     return true;
 }
 
-static void hexagon_restore_state_to_opc(CPUState *cs,
+static void hexagon_restore_state_to_opc(CPUHexagonState *env,
                                          const TranslationBlock *tb,
                                          const uint64_t *data)
 {
-    cpu_env(cs)->gpr[HEX_REG_PC] = data[0];
+    env->gpr[HEX_REG_PC] = data[0];
 }
 
 static void hexagon_cpu_reset_hold(Object *obj, ResetType type)
diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
index d73a88b279..ff937c8171 100644
--- a/target/hppa/cpu.c
+++ b/target/hppa/cpu.c
@@ -101,12 +101,10 @@ static void hppa_cpu_synchronize_from_tb(CPUState *cs,
     cpu->env.psw_xb = tb->flags & (PSW_X | PSW_B);
 }
 
-static void hppa_restore_state_to_opc(CPUState *cs,
+static void hppa_restore_state_to_opc(CPUHPPAState *env,
                                       const TranslationBlock *tb,
                                       const uint64_t *data)
 {
-    CPUHPPAState *env = cpu_env(cs);
-
     env->iaoq_f = (env->iaoq_f & TARGET_PAGE_MASK) | data[0];
     if (data[1] != INT32_MIN) {
         env->iaoq_b = env->iaoq_f + data[1];
diff --git a/target/i386/tcg/tcg-cpu.c b/target/i386/tcg/tcg-cpu.c
index cca19cd40e..6e624710f5 100644
--- a/target/i386/tcg/tcg-cpu.c
+++ b/target/i386/tcg/tcg-cpu.c
@@ -61,12 +61,10 @@ static void x86_cpu_synchronize_from_tb(CPUState *cs,
     }
 }
 
-static void x86_restore_state_to_opc(CPUState *cs,
+static void x86_restore_state_to_opc(CPUX86State *env,
                                      const TranslationBlock *tb,
                                      const uint64_t *data)
 {
-    X86CPU *cpu = X86_CPU(cs);
-    CPUX86State *env = &cpu->env;
     int cc_op = data[1];
     uint64_t new_pc;
 
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index add7323f05..6962f4b6de 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -340,11 +340,11 @@ static void loongarch_cpu_synchronize_from_tb(CPUState *cs,
     set_pc(cpu_env(cs), tb->pc);
 }
 
-static void loongarch_restore_state_to_opc(CPUState *cs,
+static void loongarch_restore_state_to_opc(CPULoongArchState *env,
                                            const TranslationBlock *tb,
                                            const uint64_t *data)
 {
-    set_pc(cpu_env(cs), data[0]);
+    set_pc(env, data[0]);
 }
 #endif /* CONFIG_TCG */
 
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index 39bf6f3d90..fc923dcf83 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -34,16 +34,15 @@ static vaddr m68k_cpu_get_pc(CPUM68KState *env)
     return env->pc;
 }
 
-static void m68k_restore_state_to_opc(CPUState *cs,
+static void m68k_restore_state_to_opc(CPUM68KState *env,
                                       const TranslationBlock *tb,
                                       const uint64_t *data)
 {
-    M68kCPU *cpu = M68K_CPU(cs);
     int cc_op = data[1];
 
-    cpu->env.pc = data[0];
+    env->pc = data[0];
     if (cc_op != CC_OP_DYNAMIC) {
-        cpu->env.cc_op = cc_op;
+        env->cc_op = cc_op;
     }
 }
 
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index 3e68c73898..c2cfecd78f 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -100,14 +100,12 @@ static void mb_cpu_synchronize_from_tb(CPUState *cs,
     cpu->env.iflags = tb->flags & IFLAGS_TB_MASK;
 }
 
-static void mb_restore_state_to_opc(CPUState *cs,
+static void mb_restore_state_to_opc(CPUMBState *env,
                                     const TranslationBlock *tb,
                                     const uint64_t *data)
 {
-    MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
-
-    cpu->env.pc = data[0];
-    cpu->env.iflags = data[1];
+    env->pc = data[0];
+    env->iflags = data[1];
 }
 
 static bool mb_cpu_has_work(CPUState *cs)
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index de7045874d..7a6fedb758 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -15302,12 +15302,10 @@ void mips_tcg_init(void)
     }
 }
 
-void mips_restore_state_to_opc(CPUState *cs,
+void mips_restore_state_to_opc(CPUMIPSState *env,
                                const TranslationBlock *tb,
                                const uint64_t *data)
 {
-    CPUMIPSState *env = cpu_env(cs);
-
     env->active_tc.PC = data[0];
     env->hflags &= ~MIPS_HFLAG_BMASK;
     env->hflags |= data[1];
diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
index 51ab0df82b..5601465789 100644
--- a/target/openrisc/cpu.c
+++ b/target/openrisc/cpu.c
@@ -45,16 +45,14 @@ static void openrisc_cpu_synchronize_from_tb(CPUState *cs,
     cpu->env.pc = tb->pc;
 }
 
-static void openrisc_restore_state_to_opc(CPUState *cs,
+static void openrisc_restore_state_to_opc(CPUOpenRISCState *env,
                                           const TranslationBlock *tb,
                                           const uint64_t *data)
 {
-    OpenRISCCPU *cpu = OPENRISC_CPU(cs);
-
-    cpu->env.pc = data[0];
-    cpu->env.dflag = data[1] & 1;
+    env->pc = data[0];
+    env->dflag = data[1] & 1;
     if (data[1] & 2) {
-        cpu->env.ppc = cpu->env.pc - 4;
+        env->ppc = env->pc - 4;
     }
 }
 
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index c8b4445aea..95bf78a3b7 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -7163,13 +7163,11 @@ static vaddr ppc_cpu_get_pc(CPUPPCState *env)
 }
 
 #ifdef CONFIG_TCG
-static void ppc_restore_state_to_opc(CPUState *cs,
+static void ppc_restore_state_to_opc(CPUPPCState *env,
                                      const TranslationBlock *tb,
                                      const uint64_t *data)
 {
-    PowerPCCPU *cpu = POWERPC_CPU(cs);
-
-    cpu->env.nip = data[0];
+    env->nip = data[0];
 }
 #endif /* CONFIG_TCG */
 
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index c62c221696..82689f06c4 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -108,12 +108,10 @@ static void riscv_cpu_synchronize_from_tb(CPUState *cs,
     }
 }
 
-static void riscv_restore_state_to_opc(CPUState *cs,
+static void riscv_restore_state_to_opc(CPURISCVState *env,
                                        const TranslationBlock *tb,
                                        const uint64_t *data)
 {
-    RISCVCPU *cpu = RISCV_CPU(cs);
-    CPURISCVState *env = &cpu->env;
     RISCVMXL xl = FIELD_EX32(tb->flags, TB_FLAGS, XL);
     target_ulong pc;
 
diff --git a/target/rx/cpu.c b/target/rx/cpu.c
index 0c4b63b114..0f24893f86 100644
--- a/target/rx/cpu.c
+++ b/target/rx/cpu.c
@@ -46,13 +46,11 @@ static void rx_cpu_synchronize_from_tb(CPUState *cs,
     cpu->env.pc = tb->pc;
 }
 
-static void rx_restore_state_to_opc(CPUState *cs,
+static void rx_restore_state_to_opc(CPURXState *env,
                                     const TranslationBlock *tb,
                                     const uint64_t *data)
 {
-    RXCPU *cpu = RX_CPU(cs);
-
-    cpu->env.pc = data[0];
+    env->pc = data[0];
 }
 
 static bool rx_cpu_has_work(CPUState *cs)
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index bcfff40b25..182be8b3ca 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -6488,11 +6488,10 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int *max_insns,
     translator_loop(cs, tb, max_insns, pc, host_pc, &s390x_tr_ops, &dc.base);
 }
 
-void s390x_restore_state_to_opc(CPUState *cs,
+void s390x_restore_state_to_opc(CPUS390XState *env,
                                 const TranslationBlock *tb,
                                 const uint64_t *data)
 {
-    CPUS390XState *env = cpu_env(cs);
     int cc_op = data[1];
 
     env->psw.addr = data[0];
diff --git a/target/sh4/cpu.c b/target/sh4/cpu.c
index 5c6840841b..c378d0ec83 100644
--- a/target/sh4/cpu.c
+++ b/target/sh4/cpu.c
@@ -48,14 +48,12 @@ static void superh_cpu_synchronize_from_tb(CPUState *cs,
     cpu->env.flags = tb->flags & TB_FLAG_ENVFLAGS_MASK;
 }
 
-static void superh_restore_state_to_opc(CPUState *cs,
+static void superh_restore_state_to_opc(CPUSH4State *env,
                                         const TranslationBlock *tb,
                                         const uint64_t *data)
 {
-    SuperHCPU *cpu = SUPERH_CPU(cs);
-
-    cpu->env.pc = data[0];
-    cpu->env.flags = data[1];
+    env->pc = data[0];
+    env->flags = data[1];
     /*
      * Theoretically delayed_pc should also be restored. In practice the
      * branch instruction is re-executed after exception, so the delayed
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index e1f0dfcbbd..83c86c03bd 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -709,11 +709,10 @@ static void sparc_cpu_synchronize_from_tb(CPUState *cs,
     cpu->env.npc = tb->cs_base;
 }
 
-static void sparc_restore_state_to_opc(CPUState *cs,
+static void sparc_restore_state_to_opc(CPUSPARCState *env,
                                        const TranslationBlock *tb,
                                        const uint64_t *data)
 {
-    CPUSPARCState *env = cpu_env(cs);
     target_ulong pc = data[0];
     target_ulong npc = data[1];
 
diff --git a/target/tricore/cpu.c b/target/tricore/cpu.c
index 20de29d114..f8a5bb8979 100644
--- a/target/tricore/cpu.c
+++ b/target/tricore/cpu.c
@@ -51,11 +51,11 @@ static void tricore_cpu_synchronize_from_tb(CPUState *cs,
     cpu_env(cs)->PC = tb->pc;
 }
 
-static void tricore_restore_state_to_opc(CPUState *cs,
+static void tricore_restore_state_to_opc(CPUTriCoreState *env,
                                          const TranslationBlock *tb,
                                          const uint64_t *data)
 {
-    cpu_env(cs)->PC = data[0];
+    env->PC = data[0];
 }
 
 static void tricore_cpu_reset_hold(Object *obj, ResetType type)
diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
index 3ab7b794f0..8ba8280ae9 100644
--- a/target/xtensa/cpu.c
+++ b/target/xtensa/cpu.c
@@ -50,13 +50,11 @@ static vaddr xtensa_cpu_get_pc(CPUXtensaState *env)
     return env->pc;
 }
 
-static void xtensa_restore_state_to_opc(CPUState *cs,
+static void xtensa_restore_state_to_opc(CPUXtensaState *env,
                                         const TranslationBlock *tb,
                                         const uint64_t *data)
 {
-    XtensaCPU *cpu = XTENSA_CPU(cs);
-
-    cpu->env.pc = data[0];
+    env->pc = data[0];
 }
 
 static bool xtensa_cpu_has_work(CPUState *cs)
-- 
2.45.2

[PATCH 10/10] hw/core/cpu: Pass CPUArchState to cpu_dump_state() handler

[Philippe Mathieu-Daudé]

CPUClass::cpu_dump_state() handler is target specific.
Rather than passing a generic CPUState and forcing QOM casts,
we can directly pass the target CPUArchState, simplifying.

Only x86_cpu_dump_state() has to do an extra env_cpu() to
access the original CPUState.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/core/cpu.h         |  2 +-
 target/alpha/cpu.h            |  2 +-
 target/hppa/cpu.h             |  2 +-
 target/i386/cpu.h             |  2 +-
 target/m68k/cpu.h             |  2 +-
 target/microblaze/cpu.h       |  2 +-
 target/openrisc/cpu.h         |  2 +-
 target/ppc/cpu.h              |  2 +-
 target/rx/cpu.h               |  2 +-
 target/s390x/s390x-internal.h |  2 +-
 target/sh4/cpu.h              |  2 +-
 target/tricore/cpu.h          |  2 +-
 target/xtensa/cpu.h           |  2 +-
 hw/core/cpu-common.c          |  2 +-
 target/alpha/helper.c         |  3 +--
 target/arm/cpu.c              | 14 ++++++--------
 target/avr/cpu.c              |  3 +--
 target/hexagon/cpu.c          |  9 ++-------
 target/hppa/helper.c          |  3 +--
 target/hppa/int_helper.c      |  2 +-
 target/hppa/sys_helper.c      |  6 ++----
 target/i386/cpu-dump.c        |  5 ++---
 target/loongarch/cpu.c        |  3 +--
 target/m68k/translate.c       |  3 +--
 target/microblaze/translate.c |  3 +--
 target/mips/cpu.c             |  3 +--
 target/openrisc/translate.c   |  3 +--
 target/ppc/cpu_init.c         |  5 ++---
 target/riscv/cpu.c            |  6 ++----
 target/rx/translate.c         |  3 +--
 target/s390x/cpu-dump.c       |  3 +--
 target/sh4/translate.c        |  3 +--
 target/sparc/cpu.c            |  3 +--
 target/tricore/translate.c    |  3 +--
 target/xtensa/translate.c     |  3 +--
 35 files changed, 45 insertions(+), 72 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 70f5f8c3bf..f647717add 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -155,7 +155,7 @@ struct CPUClass {
     int (*mmu_index)(CPUState *cpu, bool ifetch);
     int (*memory_rw_debug)(CPUState *cpu, vaddr addr,
                            uint8_t *buf, int len, bool is_write);
-    void (*dump_state)(CPUState *cpu, FILE *, int flags);
+    void (*dump_state)(CPUArchState *env, FILE *, int flags);
     void (*query_cpu_fast)(CPUState *cpu, CpuInfoFast *value);
     int64_t (*get_arch_id)(CPUState *cpu);
     bool (*cpu_persistent_status)(CPUState *cpu);
diff --git a/target/alpha/cpu.h b/target/alpha/cpu.h
index 3556d3227f..e05bc70428 100644
--- a/target/alpha/cpu.h
+++ b/target/alpha/cpu.h
@@ -283,7 +283,7 @@ void alpha_cpu_do_interrupt(CPUState *cpu);
 bool alpha_cpu_exec_interrupt(CPUState *cpu, int int_req);
 hwaddr alpha_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 #endif /* !CONFIG_USER_ONLY */
-void alpha_cpu_dump_state(CPUState *cs, FILE *f, int flags);
+void alpha_cpu_dump_state(CPUAlphaState *env, FILE *f, int flags);
 int alpha_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
 int alpha_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
 
diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index e45ba50a59..43bdcccb2f 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -358,7 +358,7 @@ void cpu_hppa_change_prot_id(CPUHPPAState *env);
 
 int hppa_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
 int hppa_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
-void hppa_cpu_dump_state(CPUState *cs, FILE *f, int);
+void hppa_cpu_dump_state(CPUHPPAState *env, FILE *f, int);
 #ifndef CONFIG_USER_ONLY
 void hppa_ptlbe(CPUHPPAState *env);
 hwaddr hppa_cpu_get_phys_page_debug(CPUState *cs, vaddr addr);
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 4c239a6970..94c3d09fe3 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2284,7 +2284,7 @@ int x86_cpu_write_elf32_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
 bool x86_cpu_get_memory_mapping(CPUState *cpu, MemoryMappingList *list,
                                 Error **errp);
 
-void x86_cpu_dump_state(CPUState *cs, FILE *f, int flags);
+void x86_cpu_dump_state(CPUX86State *env, FILE *f, int flags);
 
 int x86_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
 int x86_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
index b5bbeedb7a..7df68b8dbd 100644
--- a/target/m68k/cpu.h
+++ b/target/m68k/cpu.h
@@ -188,7 +188,7 @@ void m68k_cpu_do_interrupt(CPUState *cpu);
 bool m68k_cpu_exec_interrupt(CPUState *cpu, int int_req);
 hwaddr m68k_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 #endif /* !CONFIG_USER_ONLY */
-void m68k_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
+void m68k_cpu_dump_state(CPUM68KState *env, FILE *f, int flags);
 int m68k_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
 int m68k_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
 
diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
index 3e5a3e5c60..1571038979 100644
--- a/target/microblaze/cpu.h
+++ b/target/microblaze/cpu.h
@@ -375,7 +375,7 @@ hwaddr mb_cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr addr,
 G_NORETURN void mb_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,
                                            MMUAccessType access_type,
                                            int mmu_idx, uintptr_t retaddr);
-void mb_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
+void mb_cpu_dump_state(CPUMBState *env, FILE *f, int flags);
 int mb_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
 int mb_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
 int mb_cpu_gdb_read_stack_protect(CPUState *cs, GByteArray *buf, int reg);
diff --git a/target/openrisc/cpu.h b/target/openrisc/cpu.h
index c9fe9ae12d..7bd7578eee 100644
--- a/target/openrisc/cpu.h
+++ b/target/openrisc/cpu.h
@@ -297,7 +297,7 @@ struct ArchCPU {
     CPUOpenRISCState env;
 };
 
-void openrisc_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
+void openrisc_cpu_dump_state(CPUOpenRISCState *env, FILE *f, int flags);
 int openrisc_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
 int openrisc_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
 void openrisc_translate_init(void);
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 945af07a64..14da103db7 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1554,7 +1554,7 @@ static inline bool vhyp_cpu_in_nested(PowerPCCPU *cpu)
 }
 #endif /* CONFIG_USER_ONLY */
 
-void ppc_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
+void ppc_cpu_dump_state(CPUPPCState *env, FILE *f, int flags);
 int ppc_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
 int ppc_cpu_gdb_read_register_apple(CPUState *cpu, GByteArray *buf, int reg);
 int ppc_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
diff --git a/target/rx/cpu.h b/target/rx/cpu.h
index c53593d7aa..b582304a48 100644
--- a/target/rx/cpu.h
+++ b/target/rx/cpu.h
@@ -134,7 +134,7 @@ void rx_cpu_do_interrupt(CPUState *cpu);
 bool rx_cpu_exec_interrupt(CPUState *cpu, int int_req);
 hwaddr rx_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 #endif /* !CONFIG_USER_ONLY */
-void rx_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
+void rx_cpu_dump_state(CPURXState *env, FILE *f, int flags);
 int rx_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
 int rx_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
 
diff --git a/target/s390x/s390x-internal.h b/target/s390x/s390x-internal.h
index 891e5f576c..9ae89b3d24 100644
--- a/target/s390x/s390x-internal.h
+++ b/target/s390x/s390x-internal.h
@@ -322,7 +322,7 @@ void s390_cpu_gdb_init(CPUState *cs);
 
 
 /* helper.c */
-void s390_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
+void s390_cpu_dump_state(CPUS390XState *env, FILE *f, int flags);
 void do_restart_interrupt(CPUS390XState *env);
 #ifndef CONFIG_USER_ONLY
 void s390_cpu_recompute_watchpoints(CPUState *cs);
diff --git a/target/sh4/cpu.h b/target/sh4/cpu.h
index d928bcf006..f4f38a9ab2 100644
--- a/target/sh4/cpu.h
+++ b/target/sh4/cpu.h
@@ -240,7 +240,7 @@ struct SuperHCPUClass {
     uint32_t cvr;
 };
 
-void superh_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
+void superh_cpu_dump_state(CPUSH4State *env, FILE *f, int flags);
 int superh_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
 int superh_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
 G_NORETURN void superh_cpu_do_unaligned_access(CPUState *cpu, vaddr addr,
diff --git a/target/tricore/cpu.h b/target/tricore/cpu.h
index 220af69fc2..423589f609 100644
--- a/target/tricore/cpu.h
+++ b/target/tricore/cpu.h
@@ -76,7 +76,7 @@ struct TriCoreCPUClass {
 };
 
 hwaddr tricore_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
-void tricore_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
+void tricore_cpu_dump_state(CPUTriCoreState *env, FILE *f, int flags);
 
 FIELD(PCXI, PCPN_13, 24, 8)
 FIELD(PCXI, PCPN_161, 22, 8)
diff --git a/target/xtensa/cpu.h b/target/xtensa/cpu.h
index 77e48eef19..28ebba10e3 100644
--- a/target/xtensa/cpu.h
+++ b/target/xtensa/cpu.h
@@ -592,7 +592,7 @@ void xtensa_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
 hwaddr xtensa_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 bool xtensa_debug_check_breakpoint(CPUState *cs);
 #endif
-void xtensa_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
+void xtensa_cpu_dump_state(CPUXtensaState *env, FILE *f, int flags);
 void xtensa_count_regs(const XtensaConfig *config,
                        unsigned *n_regs, unsigned *n_core_regs);
 int xtensa_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index 09c7903594..bc608b38f5 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -105,7 +105,7 @@ void cpu_dump_state(CPUState *cpu, FILE *f, int flags)
 
     if (cc->dump_state) {
         cpu_synchronize_state(cpu);
-        cc->dump_state(cpu, f, flags);
+        cc->dump_state(cpu_env(cpu), f, flags);
     }
 }
 
diff --git a/target/alpha/helper.c b/target/alpha/helper.c
index 2f1000c99f..990b2edde4 100644
--- a/target/alpha/helper.c
+++ b/target/alpha/helper.c
@@ -476,7 +476,7 @@ bool alpha_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
 
 #endif /* !CONFIG_USER_ONLY */
 
-void alpha_cpu_dump_state(CPUState *cs, FILE *f, int flags)
+void alpha_cpu_dump_state(CPUAlphaState *env, FILE *f, int flags)
 {
     static const char linux_reg_names[31][4] = {
         "v0",  "t0",  "t1", "t2",  "t3", "t4", "t5", "t6",
@@ -484,7 +484,6 @@ void alpha_cpu_dump_state(CPUState *cs, FILE *f, int flags)
         "a0",  "a1",  "a2", "a3",  "a4", "a5", "t8", "t9",
         "t10", "t11", "ra", "t12", "at", "gp", "sp"
     };
-    CPUAlphaState *env = cpu_env(cs);
     int i;
 
     qemu_fprintf(f, "PC      " TARGET_FMT_lx " PS      %02x\n",
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index c8e032d433..a7bb025c11 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1213,10 +1213,9 @@ static void arm_disas_set_info(CPUState *cpu, disassemble_info *info)
 
 #ifdef TARGET_AARCH64
 
-static void aarch64_cpu_dump_state(CPUState *cs, FILE *f, int flags)
+static void aarch64_cpu_dump_state(CPUARMState *env, FILE *f, int flags)
 {
-    ARMCPU *cpu = ARM_CPU(cs);
-    CPUARMState *env = &cpu->env;
+    ARMCPU *cpu = env_archcpu(env);
     uint32_t psr = pstate_read(env);
     int i, j;
     int el = arm_current_el(env);
@@ -1372,21 +1371,20 @@ static void aarch64_cpu_dump_state(CPUState *cs, FILE *f, int flags)
 
 #else
 
-static inline void aarch64_cpu_dump_state(CPUState *cs, FILE *f, int flags)
+static inline void aarch64_cpu_dump_state(CPUARMState *env, FILE *f, int flags)
 {
     g_assert_not_reached();
 }
 
 #endif
 
-static void arm_cpu_dump_state(CPUState *cs, FILE *f, int flags)
+static void arm_cpu_dump_state(CPUARMState *env, FILE *f, int flags)
 {
-    ARMCPU *cpu = ARM_CPU(cs);
-    CPUARMState *env = &cpu->env;
+    ARMCPU *cpu = env_archcpu(env);
     int i;
 
     if (is_a64(env)) {
-        aarch64_cpu_dump_state(cs, f, flags);
+        aarch64_cpu_dump_state(env, f, flags);
         return;
     }
 
diff --git a/target/avr/cpu.c b/target/avr/cpu.c
index 19b6298a31..46a1ba3b3a 100644
--- a/target/avr/cpu.c
+++ b/target/avr/cpu.c
@@ -155,9 +155,8 @@ static ObjectClass *avr_cpu_class_by_name(const char *cpu_model)
     return object_class_by_name(cpu_model);
 }
 
-static void avr_cpu_dump_state(CPUState *cs, FILE *f, int flags)
+static void avr_cpu_dump_state(CPUAVRState *env, FILE *f, int flags)
 {
-    CPUAVRState *env = cpu_env(cs);
     int i;
 
     qemu_fprintf(f, "\n");
diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
index 8038df1c82..58c627946b 100644
--- a/target/hexagon/cpu.c
+++ b/target/hexagon/cpu.c
@@ -176,7 +176,7 @@ void hexagon_debug_qreg(CPUHexagonState *env, int regnum)
     print_qreg(stdout, env, regnum, false);
 }
 
-static void hexagon_dump(CPUHexagonState *env, FILE *f, int flags)
+static void hexagon_dump_state(CPUHexagonState *env, FILE *f, int flags)
 {
     HexagonCPU *cpu = env_archcpu(env);
 
@@ -235,14 +235,9 @@ static void hexagon_dump(CPUHexagonState *env, FILE *f, int flags)
     }
 }
 
-static void hexagon_dump_state(CPUState *cs, FILE *f, int flags)
-{
-    hexagon_dump(cpu_env(cs), f, flags);
-}
-
 void hexagon_debug(CPUHexagonState *env)
 {
-    hexagon_dump(env, stdout, CPU_DUMP_FPU);
+    hexagon_dump_state(env, stdout, CPU_DUMP_FPU);
 }
 
 static void hexagon_cpu_set_pc(CPUHexagonState *env, vaddr value)
diff --git a/target/hppa/helper.c b/target/hppa/helper.c
index d4b1a3cd5a..2456d36abe 100644
--- a/target/hppa/helper.c
+++ b/target/hppa/helper.c
@@ -100,7 +100,7 @@ void cpu_hppa_put_psw(CPUHPPAState *env, target_ulong psw)
     env->psw_cb = cb;
 }
 
-void hppa_cpu_dump_state(CPUState *cs, FILE *f, int flags)
+void hppa_cpu_dump_state(CPUHPPAState *env, FILE *f, int flags)
 {
 #ifndef CONFIG_USER_ONLY
     static const char cr_name[32][5] = {
@@ -115,7 +115,6 @@ void hppa_cpu_dump_state(CPUState *cs, FILE *f, int flags)
     };
 #endif
 
-    CPUHPPAState *env = cpu_env(cs);
     target_ulong psw = cpu_hppa_get_psw(env);
     target_ulong psw_cb;
     char psw_c[20];
diff --git a/target/hppa/int_helper.c b/target/hppa/int_helper.c
index 58695def82..8cb9defa50 100644
--- a/target/hppa/int_helper.c
+++ b/target/hppa/int_helper.c
@@ -254,7 +254,7 @@ void hppa_cpu_do_interrupt(CPUState *cs)
             } else {
                 fprintf(logfile, "INT: cpu %d unknown %d\n", cs->cpu_index, i);
             }
-            hppa_cpu_dump_state(cs, logfile, 0);
+            hppa_cpu_dump_state(env, logfile, 0);
             qemu_log_unlock(logfile);
         }
     }
diff --git a/target/hppa/sys_helper.c b/target/hppa/sys_helper.c
index 9b43b556fd..320d8ad995 100644
--- a/target/hppa/sys_helper.c
+++ b/target/hppa/sys_helper.c
@@ -98,10 +98,8 @@ void HELPER(rfi)(CPUHPPAState *env)
     if (qemu_loglevel_mask(CPU_LOG_INT)) {
         FILE *logfile = qemu_log_trylock();
         if (logfile) {
-            CPUState *cs = env_cpu(env);
-
-            fprintf(logfile, "RFI: cpu %d\n", cs->cpu_index);
-            hppa_cpu_dump_state(cs, logfile, 0);
+            fprintf(logfile, "RFI: cpu %d\n", env_cpu(env)->cpu_index);
+            hppa_cpu_dump_state(env, logfile, 0);
             qemu_log_unlock(logfile);
         }
     }
diff --git a/target/i386/cpu-dump.c b/target/i386/cpu-dump.c
index a72ed93bd2..9e4b8d6742 100644
--- a/target/i386/cpu-dump.c
+++ b/target/i386/cpu-dump.c
@@ -341,10 +341,9 @@ void x86_cpu_dump_local_apic_state(CPUState *cs, int flags)
 #define DUMP_CODE_BYTES_TOTAL    50
 #define DUMP_CODE_BYTES_BACKWARD 20
 
-void x86_cpu_dump_state(CPUState *cs, FILE *f, int flags)
+void x86_cpu_dump_state(CPUX86State *env, FILE *f, int flags)
 {
-    X86CPU *cpu = X86_CPU(cs);
-    CPUX86State *env = &cpu->env;
+    CPUState *cs = env_cpu(env);
     int eflags, i, nb;
     static const char *seg_name[6] = { "ES", "CS", "SS", "DS", "FS", "GS" };
 
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index 6962f4b6de..ae00d5e222 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -742,9 +742,8 @@ static ObjectClass *loongarch_cpu_class_by_name(const char *cpu_model)
     return oc;
 }
 
-static void loongarch_cpu_dump_state(CPUState *cs, FILE *f, int flags)
+static void loongarch_cpu_dump_state(CPULoongArchState *env, FILE *f, int flags)
 {
-    CPULoongArchState *env = cpu_env(cs);
     int i;
 
     qemu_fprintf(f, " PC=%016" PRIx64 " ", env->pc);
diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index ad3ce34501..aafc32aa48 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -6136,9 +6136,8 @@ static double floatx80_to_double(CPUM68KState *env, uint16_t high, uint64_t low)
     return u.d;
 }
 
-void m68k_cpu_dump_state(CPUState *cs, FILE *f, int flags)
+void m68k_cpu_dump_state(CPUM68KState *env, FILE *f, int flags)
 {
-    CPUM68KState *env = cpu_env(cs);
     int i;
     uint16_t sr;
     for (i = 0; i < 8; i++) {
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 4beaf69e76..98404a3036 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -1785,9 +1785,8 @@ void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int *max_insns,
     translator_loop(cpu, tb, max_insns, pc, host_pc, &mb_tr_ops, &dc.base);
 }
 
-void mb_cpu_dump_state(CPUState *cs, FILE *f, int flags)
+void mb_cpu_dump_state(CPUMBState *env, FILE *f, int flags)
 {
-    CPUMBState *env = cpu_env(cs);
     uint32_t iflags;
     int i;
 
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index 506494f7e6..d631249216 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -78,9 +78,8 @@ static void fpu_dump_state(CPUMIPSState *env, FILE *f, int flags)
     }
 }
 
-static void mips_cpu_dump_state(CPUState *cs, FILE *f, int flags)
+static void mips_cpu_dump_state(CPUMIPSState *env, FILE *f, int flags)
 {
-    CPUMIPSState *env = cpu_env(cs);
     int i;
 
     qemu_fprintf(f, "pc=0x" TARGET_FMT_lx " HI=0x" TARGET_FMT_lx
diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
index ca566847cb..3ca94f00b1 100644
--- a/target/openrisc/translate.c
+++ b/target/openrisc/translate.c
@@ -1654,9 +1654,8 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int *max_insns,
                     &openrisc_tr_ops, &ctx.base);
 }
 
-void openrisc_cpu_dump_state(CPUState *cs, FILE *f, int flags)
+void openrisc_cpu_dump_state(CPUOpenRISCState *env, FILE *f, int flags)
 {
-    CPUOpenRISCState *env = cpu_env(cs);
     int i;
 
     qemu_fprintf(f, "PC=%08x\n", env->pc);
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 95bf78a3b7..b8d859846f 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -7522,18 +7522,17 @@ static void ppc_cpu_register_types(void)
 #endif
 }
 
-void ppc_cpu_dump_state(CPUState *cs, FILE *f, int flags)
+void ppc_cpu_dump_state(CPUPPCState *env, FILE *f, int flags)
 {
 #define RGPL  4
 #define RFPL  4
 
-    CPUPPCState *env = cpu_env(cs);
     int i;
 
     qemu_fprintf(f, "NIP " TARGET_FMT_lx "   LR " TARGET_FMT_lx " CTR "
                  TARGET_FMT_lx " XER " TARGET_FMT_lx " CPU#%d\n",
                  env->nip, env->lr, env->ctr, cpu_read_xer(env),
-                 cs->cpu_index);
+                 env_cpu(env)->cpu_index);
     qemu_fprintf(f, "MSR " TARGET_FMT_lx " HID0 " TARGET_FMT_lx "  HF "
                  "%08x iidx %d didx %d\n",
                  env->msr, env->spr[SPR_HID0], env->hflags,
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index dfaa9a9c1c..152f5c4e76 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -746,10 +746,8 @@ char *riscv_cpu_get_name(RISCVCPU *cpu)
     return cpu_model_from_type(typename);
 }
 
-static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int flags)
+static void riscv_cpu_dump_state(CPURISCVState *env, FILE *f, int flags)
 {
-    RISCVCPU *cpu = RISCV_CPU(cs);
-    CPURISCVState *env = &cpu->env;
     int i, j;
     uint8_t *p;
 
@@ -865,7 +863,7 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int flags)
                              csr_ops[csrno].name, val);
             }
         }
-        uint16_t vlenb = cpu->cfg.vlenb;
+        uint16_t vlenb = env_archcpu(env)->cfg.vlenb;
 
         for (i = 0; i < 32; i++) {
             qemu_fprintf(f, " %-8s ", riscv_rvv_regnames[i]);
diff --git a/target/rx/translate.c b/target/rx/translate.c
index 9aade2b6e5..5747425345 100644
--- a/target/rx/translate.c
+++ b/target/rx/translate.c
@@ -131,9 +131,8 @@ static int bdsp_s(DisasContext *ctx, int d)
 /* Include the auto-generated decoder. */
 #include "decode-insns.c.inc"
 
-void rx_cpu_dump_state(CPUState *cs, FILE *f, int flags)
+void rx_cpu_dump_state(CPURXState *env, FILE *f, int flags)
 {
-    CPURXState *env = cpu_env(cs);
     int i;
     uint32_t psw;
 
diff --git a/target/s390x/cpu-dump.c b/target/s390x/cpu-dump.c
index 69cc9f7746..c33d6b2855 100644
--- a/target/s390x/cpu-dump.c
+++ b/target/s390x/cpu-dump.c
@@ -25,9 +25,8 @@
 #include "qemu/qemu-print.h"
 #include "sysemu/tcg.h"
 
-void s390_cpu_dump_state(CPUState *cs, FILE *f, int flags)
+void s390_cpu_dump_state(CPUS390XState *env, FILE *f, int flags)
 {
-    CPUS390XState *env = cpu_env(cs);
     int i;
 
     qemu_fprintf(f, "PSW=mask %016" PRIx64 " addr %016" PRIx64,
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 53b092175d..b4eaafe2c4 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -156,9 +156,8 @@ void sh4_translate_init(void)
                                               fregnames[i]);
 }
 
-void superh_cpu_dump_state(CPUState *cs, FILE *f, int flags)
+void superh_cpu_dump_state(CPUSH4State *env, FILE *f, int flags)
 {
-    CPUSH4State *env = cpu_env(cs);
     int i;
 
     qemu_fprintf(f, "pc=0x%08x sr=0x%08x pr=0x%08x fpscr=0x%08x\n",
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index 83c86c03bd..bf37734efd 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -616,9 +616,8 @@ static void cpu_print_cc(FILE *f, uint32_t cc)
 #define REGS_PER_LINE 8
 #endif
 
-static void sparc_cpu_dump_state(CPUState *cs, FILE *f, int flags)
+static void sparc_cpu_dump_state(CPUSPARCState *env, FILE *f, int flags)
 {
-    CPUSPARCState *env = cpu_env(cs);
     int i, x;
 
     qemu_fprintf(f, "pc: " TARGET_FMT_lx "  npc: " TARGET_FMT_lx "\n", env->pc,
diff --git a/target/tricore/translate.c b/target/tricore/translate.c
index 4a12d2ca19..c9f96ef3ae 100644
--- a/target/tricore/translate.c
+++ b/target/tricore/translate.c
@@ -92,9 +92,8 @@ enum {
     MODE_UU = 3,
 };
 
-void tricore_cpu_dump_state(CPUState *cs, FILE *f, int flags)
+void tricore_cpu_dump_state(CPUTriCoreState *env, FILE *f, int flags)
 {
-    CPUTriCoreState *env = cpu_env(cs);
     uint32_t psw;
     int i;
 
diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index f4da4a40f9..6827f39492 100644
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -1235,9 +1235,8 @@ void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int *max_insns,
                     &xtensa_translator_ops, &dc.base);
 }
 
-void xtensa_cpu_dump_state(CPUState *cs, FILE *f, int flags)
+void xtensa_cpu_dump_state(CPUXtensaState *env, FILE *f, int flags)
 {
-    CPUXtensaState *env = cpu_env(cs);
     xtensa_isa isa = env->config->isa;
     int i, j;
 
-- 
2.45.2