diff mbox series

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

Message ID 20241115152053.66442-9-philmd@linaro.org (mailing list archive)
State New
Headers show
Series accel/tcg: API prototype cleanups | expand

Commit Message

Philippe Mathieu-Daudé Nov. 15, 2024, 3:20 p.m. UTC 
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(-)

Comments

Peter Maydell Nov. 15, 2024, 3:54 p.m. UTC | #1 
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
Paolo Bonzini Nov. 15, 2024, 4:21 p.m. UTC | #2 
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
Richard Henderson Nov. 15, 2024, 5:51 p.m. UTC | #3 
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~
diff mbox series

Patch

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,