Re: [PATCH v17 06/14] gdbstub: Change gdb_get_reg_cb and gdb_set_reg_cb

2023-12-17 Thread Alistair Francis 
On Wed, Dec 13, 2023 at 5:47 PM Akihiko Odaki  wrote:
>
> Align the parameters of gdb_get_reg_cb and gdb_set_reg_cb with the
> gdb_read_register and gdb_write_register members of CPUClass to allow
> to unify the logic to access registers of the core and coprocessors
> in the future.
>
> Signed-off-by: Akihiko Odaki 
> Reviewed-by: Alex Bennée 

Acked-by: Alistair Francis 

Alistair

> ---
>  include/exec/gdbstub.h  |  4 +--
>  target/arm/internals.h  | 12 +++
>  target/hexagon/internal.h   |  4 +--
>  target/microblaze/cpu.h |  4 +--
>  gdbstub/gdbstub.c   |  6 ++--
>  target/arm/gdbstub.c| 51 +-
>  target/arm/gdbstub64.c  | 27 +++-
>  target/hexagon/gdbstub.c| 10 --
>  target/loongarch/gdbstub.c  | 11 ---
>  target/m68k/helper.c| 20 +---
>  target/microblaze/gdbstub.c |  9 --
>  target/ppc/gdbstub.c| 46 +--
>  target/riscv/gdbstub.c  | 46 ---
>  target/s390x/gdbstub.c  | 77 
> ++---
>  14 files changed, 236 insertions(+), 91 deletions(-)
>
> diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
> index ac6fce99a64e..bcaab1bc750e 100644
> --- a/include/exec/gdbstub.h
> +++ b/include/exec/gdbstub.h
> @@ -24,8 +24,8 @@ typedef struct GDBFeatureBuilder {
>
>
>  /* Get or set a register.  Returns the size of the register.  */
> -typedef int (*gdb_get_reg_cb)(CPUArchState *env, GByteArray *buf, int reg);
> -typedef int (*gdb_set_reg_cb)(CPUArchState *env, uint8_t *buf, int reg);
> +typedef int (*gdb_get_reg_cb)(CPUState *cpu, GByteArray *buf, int reg);
> +typedef int (*gdb_set_reg_cb)(CPUState *cpu, uint8_t *buf, int reg);
>
>  /**
>   * gdb_register_coprocessor() - register a supplemental set of registers
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index 1136710741f0..a08f461f444b 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -1447,12 +1447,12 @@ static inline uint64_t pmu_counter_mask(CPUARMState 
> *env)
>
>  #ifdef TARGET_AARCH64
>  GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cpu, int base_reg);
> -int aarch64_gdb_get_sve_reg(CPUARMState *env, GByteArray *buf, int reg);
> -int aarch64_gdb_set_sve_reg(CPUARMState *env, uint8_t *buf, int reg);
> -int aarch64_gdb_get_fpu_reg(CPUARMState *env, GByteArray *buf, int reg);
> -int aarch64_gdb_set_fpu_reg(CPUARMState *env, uint8_t *buf, int reg);
> -int aarch64_gdb_get_pauth_reg(CPUARMState *env, GByteArray *buf, int reg);
> -int aarch64_gdb_set_pauth_reg(CPUARMState *env, uint8_t *buf, int reg);
> +int aarch64_gdb_get_sve_reg(CPUState *cs, GByteArray *buf, int reg);
> +int aarch64_gdb_set_sve_reg(CPUState *cs, uint8_t *buf, int reg);
> +int aarch64_gdb_get_fpu_reg(CPUState *cs, GByteArray *buf, int reg);
> +int aarch64_gdb_set_fpu_reg(CPUState *cs, uint8_t *buf, int reg);
> +int aarch64_gdb_get_pauth_reg(CPUState *cs, GByteArray *buf, int reg);
> +int aarch64_gdb_set_pauth_reg(CPUState *cs, uint8_t *buf, int reg);
>  void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp);
>  void arm_cpu_sme_finalize(ARMCPU *cpu, Error **errp);
>  void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp);
> diff --git a/target/hexagon/internal.h b/target/hexagon/internal.h
> index d732b6bb3c73..beb08cb7e382 100644
> --- a/target/hexagon/internal.h
> +++ b/target/hexagon/internal.h
> @@ -33,8 +33,8 @@
>
>  int hexagon_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
>  int hexagon_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
> -int hexagon_hvx_gdb_read_register(CPUHexagonState *env, GByteArray *mem_buf, 
> int n);
> -int hexagon_hvx_gdb_write_register(CPUHexagonState *env, uint8_t *mem_buf, 
> int n);
> +int hexagon_hvx_gdb_read_register(CPUState *env, GByteArray *mem_buf, int n);
> +int hexagon_hvx_gdb_write_register(CPUState *env, uint8_t *mem_buf, int n);
>
>  void hexagon_debug_vreg(CPUHexagonState *env, int regnum);
>  void hexagon_debug_qreg(CPUHexagonState *env, int regnum);
> diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
> index b5374365f5f5..1906d8f266af 100644
> --- a/target/microblaze/cpu.h
> +++ b/target/microblaze/cpu.h
> @@ -381,8 +381,8 @@ G_NORETURN void mb_cpu_do_unaligned_access(CPUState *cs, 
> vaddr vaddr,
>  void mb_cpu_dump_state(CPUState *cpu, 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(CPUArchState *cpu, GByteArray *buf, int 
> reg);
> -int mb_cpu_gdb_write_stack_protect(CPUArchState *cpu, uint8_t *buf, int reg);
> +int mb_cpu_gdb_read_stack_protect(CPUState *cs, GByteArray *buf, int reg);
> +int mb_cpu_gdb_write_stack_protect(CPUState *cs, uint8_t *buf, int reg);
>
>  static inline uint32_t mb_cpu_read_msr(const CPUMBState *env)
>  {
> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
> index

[PATCH v17 06/14] gdbstub: Change gdb_get_reg_cb and gdb_set_reg_cb

2023-12-12 Thread Akihiko Odaki 
Align the parameters of gdb_get_reg_cb and gdb_set_reg_cb with the
gdb_read_register and gdb_write_register members of CPUClass to allow
to unify the logic to access registers of the core and coprocessors
in the future.

Signed-off-by: Akihiko Odaki 
Reviewed-by: Alex Bennée 
---
 include/exec/gdbstub.h  |  4 +--
 target/arm/internals.h  | 12 +++
 target/hexagon/internal.h   |  4 +--
 target/microblaze/cpu.h |  4 +--
 gdbstub/gdbstub.c   |  6 ++--
 target/arm/gdbstub.c| 51 +-
 target/arm/gdbstub64.c  | 27 +++-
 target/hexagon/gdbstub.c| 10 --
 target/loongarch/gdbstub.c  | 11 ---
 target/m68k/helper.c| 20 +---
 target/microblaze/gdbstub.c |  9 --
 target/ppc/gdbstub.c| 46 +--
 target/riscv/gdbstub.c  | 46 ---
 target/s390x/gdbstub.c  | 77 ++---
 14 files changed, 236 insertions(+), 91 deletions(-)

diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index ac6fce99a64e..bcaab1bc750e 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -24,8 +24,8 @@ typedef struct GDBFeatureBuilder {
 
 
 /* Get or set a register.  Returns the size of the register.  */
-typedef int (*gdb_get_reg_cb)(CPUArchState *env, GByteArray *buf, int reg);
-typedef int (*gdb_set_reg_cb)(CPUArchState *env, uint8_t *buf, int reg);
+typedef int (*gdb_get_reg_cb)(CPUState *cpu, GByteArray *buf, int reg);
+typedef int (*gdb_set_reg_cb)(CPUState *cpu, uint8_t *buf, int reg);
 
 /**
  * gdb_register_coprocessor() - register a supplemental set of registers
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 1136710741f0..a08f461f444b 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1447,12 +1447,12 @@ static inline uint64_t pmu_counter_mask(CPUARMState 
*env)
 
 #ifdef TARGET_AARCH64
 GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cpu, int base_reg);
-int aarch64_gdb_get_sve_reg(CPUARMState *env, GByteArray *buf, int reg);
-int aarch64_gdb_set_sve_reg(CPUARMState *env, uint8_t *buf, int reg);
-int aarch64_gdb_get_fpu_reg(CPUARMState *env, GByteArray *buf, int reg);
-int aarch64_gdb_set_fpu_reg(CPUARMState *env, uint8_t *buf, int reg);
-int aarch64_gdb_get_pauth_reg(CPUARMState *env, GByteArray *buf, int reg);
-int aarch64_gdb_set_pauth_reg(CPUARMState *env, uint8_t *buf, int reg);
+int aarch64_gdb_get_sve_reg(CPUState *cs, GByteArray *buf, int reg);
+int aarch64_gdb_set_sve_reg(CPUState *cs, uint8_t *buf, int reg);
+int aarch64_gdb_get_fpu_reg(CPUState *cs, GByteArray *buf, int reg);
+int aarch64_gdb_set_fpu_reg(CPUState *cs, uint8_t *buf, int reg);
+int aarch64_gdb_get_pauth_reg(CPUState *cs, GByteArray *buf, int reg);
+int aarch64_gdb_set_pauth_reg(CPUState *cs, uint8_t *buf, int reg);
 void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp);
 void arm_cpu_sme_finalize(ARMCPU *cpu, Error **errp);
 void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp);
diff --git a/target/hexagon/internal.h b/target/hexagon/internal.h
index d732b6bb3c73..beb08cb7e382 100644
--- a/target/hexagon/internal.h
+++ b/target/hexagon/internal.h
@@ -33,8 +33,8 @@
 
 int hexagon_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
 int hexagon_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
-int hexagon_hvx_gdb_read_register(CPUHexagonState *env, GByteArray *mem_buf, 
int n);
-int hexagon_hvx_gdb_write_register(CPUHexagonState *env, uint8_t *mem_buf, int 
n);
+int hexagon_hvx_gdb_read_register(CPUState *env, GByteArray *mem_buf, int n);
+int hexagon_hvx_gdb_write_register(CPUState *env, uint8_t *mem_buf, int n);
 
 void hexagon_debug_vreg(CPUHexagonState *env, int regnum);
 void hexagon_debug_qreg(CPUHexagonState *env, int regnum);
diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
index b5374365f5f5..1906d8f266af 100644
--- a/target/microblaze/cpu.h
+++ b/target/microblaze/cpu.h
@@ -381,8 +381,8 @@ G_NORETURN void mb_cpu_do_unaligned_access(CPUState *cs, 
vaddr vaddr,
 void mb_cpu_dump_state(CPUState *cpu, 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(CPUArchState *cpu, GByteArray *buf, int reg);
-int mb_cpu_gdb_write_stack_protect(CPUArchState *cpu, uint8_t *buf, int reg);
+int mb_cpu_gdb_read_stack_protect(CPUState *cs, GByteArray *buf, int reg);
+int mb_cpu_gdb_write_stack_protect(CPUState *cs, uint8_t *buf, int reg);
 
 static inline uint32_t mb_cpu_read_msr(const CPUMBState *env)
 {
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index a80729436b66..21fea7fffae8 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -502,7 +502,6 @@ const GDBFeature *gdb_find_static_feature(const char 
*xmlname)
 static int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg)
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
-