[v2] cpus: Prefer cached CpuClass over CPU_GET_CLASS() macro

[PATCH v2 02/10] gdbstub: Clarify no more than @gdb_num_core_regs can be accessed

Posted by Philippe Mathieu-Daudé 18 hours ago

Both CPUClass::gdb_read_register() and CPUClass::gdb_write_register()
handlers are called from common gdbstub code, and won't be called with
register index over CPUClass::gdb_num_core_regs:

  int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg)
  {
      CPUClass *cc = CPU_GET_CLASS(cpu);

      if (reg < cc->gdb_num_core_regs) {
          return cc->gdb_read_register(cpu, buf, reg);
      }
      ...
  }

  static int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg)
  {
      CPUClass *cc = CPU_GET_CLASS(cpu);

      if (reg < cc->gdb_num_core_regs) {
          return cc->gdb_write_register(cpu, mem_buf, reg);
      }
      ...
  }

Clarify that in CPUClass docstring, and remove unreachable code on
the microblaze and tricore implementations.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/core/cpu.h       | 2 ++
 target/microblaze/gdbstub.c | 5 -----
 target/openrisc/gdbstub.c   | 5 -----
 3 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index fb397cdfc53..7b6b22c431b 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -124,7 +124,9 @@ struct SysemuCPUOps;
  * @get_pc: Callback for getting the Program Counter register.
  *       As above, with the semantics of the target architecture.
  * @gdb_read_register: Callback for letting GDB read a register.
+ *                     No more than @gdb_num_core_regs registers can be read.
  * @gdb_write_register: Callback for letting GDB write a register.
+ *                     No more than @gdb_num_core_regs registers can be written.
  * @gdb_adjust_breakpoint: Callback for adjusting the address of a
  *       breakpoint.  Used by AVR to handle a gdb mis-feature with
  *       its Harvard architecture split code and data.
diff --git a/target/microblaze/gdbstub.c b/target/microblaze/gdbstub.c
index 09d74e164d0..d493681d38d 100644
--- a/target/microblaze/gdbstub.c
+++ b/target/microblaze/gdbstub.c
@@ -110,14 +110,9 @@ int mb_cpu_gdb_read_stack_protect(CPUState *cs, GByteArray *mem_buf, int n)
 
 int mb_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
 {
-    CPUClass *cc = CPU_GET_CLASS(cs);
     CPUMBState *env = cpu_env(cs);
     uint32_t tmp;
 
-    if (n > cc->gdb_num_core_regs) {
-        return 0;
-    }
-
     tmp = ldl_p(mem_buf);
 
     switch (n) {
diff --git a/target/openrisc/gdbstub.c b/target/openrisc/gdbstub.c
index c2a77d5d4d5..45bba80d878 100644
--- a/target/openrisc/gdbstub.c
+++ b/target/openrisc/gdbstub.c
@@ -47,14 +47,9 @@ int openrisc_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
 
 int openrisc_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
 {
-    CPUClass *cc = CPU_GET_CLASS(cs);
     CPUOpenRISCState *env = cpu_env(cs);
     uint32_t tmp;
 
-    if (n > cc->gdb_num_core_regs) {
-        return 0;
-    }
-
     tmp = ldl_p(mem_buf);
 
     if (n < 32) {
-- 
2.47.1

Re: [PATCH v2 02/10] gdbstub: Clarify no more than @gdb_num_core_regs can be accessed

Posted by Alex Bennée 6 hours ago

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Both CPUClass::gdb_read_register() and CPUClass::gdb_write_register()
> handlers are called from common gdbstub code, and won't be called with
> register index over CPUClass::gdb_num_core_regs:
>
>   int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg)
>   {
>       CPUClass *cc = CPU_GET_CLASS(cpu);
>
>       if (reg < cc->gdb_num_core_regs) {
>           return cc->gdb_read_register(cpu, buf, reg);
>       }
>       ...
>   }
>
>   static int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg)
>   {
>       CPUClass *cc = CPU_GET_CLASS(cpu);
>
>       if (reg < cc->gdb_num_core_regs) {
>           return cc->gdb_write_register(cpu, mem_buf, reg);
>       }
>       ...
>   }
>
> Clarify that in CPUClass docstring, and remove unreachable code on
> the microblaze and tricore implementations.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v2 02/10] gdbstub: Clarify no more than @gdb_num_core_regs can be accessed

Posted by Philippe Mathieu-Daudé 17 hours ago

On 22/1/25 10:30, Philippe Mathieu-Daudé wrote:
> Both CPUClass::gdb_read_register() and CPUClass::gdb_write_register()
> handlers are called from common gdbstub code, and won't be called with
> register index over CPUClass::gdb_num_core_regs:
> 
>    int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg)
>    {
>        CPUClass *cc = CPU_GET_CLASS(cpu);
> 
>        if (reg < cc->gdb_num_core_regs) {
>            return cc->gdb_read_register(cpu, buf, reg);
>        }
>        ...
>    }
> 
>    static int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg)
>    {
>        CPUClass *cc = CPU_GET_CLASS(cpu);
> 
>        if (reg < cc->gdb_num_core_regs) {
>            return cc->gdb_write_register(cpu, mem_buf, reg);
>        }
>        ...
>    }
> 
> Clarify that in CPUClass docstring, and remove unreachable code on
> the microblaze and tricore implementations.

s/tricore/openrisc/ 🤦

> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/core/cpu.h       | 2 ++
>   target/microblaze/gdbstub.c | 5 -----
>   target/openrisc/gdbstub.c   | 5 -----
>   3 files changed, 2 insertions(+), 10 deletions(-)