[PATCH v5 5/9] monitor: Have MonitorDef::get_value() return an unsigned type

Philippe Mathieu-Daudé posted 9 patches 1 week, 1 day ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, "Dr. David Alan Gilbert" <dave@treblig.org>, Markus Armbruster <armbru@redhat.com>, Zhao Liu <zhao1.liu@intel.com>, Marcelo Tosatti <mtosatti@redhat.com>, Laurent Vivier <laurent@vivier.eu>, Nicholas Piggin <npiggin@gmail.com>, Chinmay Rath <rathc@linux.ibm.com>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Yoshinori Sato <yoshinori.sato@nifty.com>, Artyom Tarasenko <atar4qemu@gmail.com>, Max Filippov <jcmvbkbc@gmail.com>
[PATCH v5 5/9] monitor: Have MonitorDef::get_value() return an unsigned type
Posted by Philippe Mathieu-Daudé 1 week, 1 day ago
All implementations of the get_value() handler return an
unsigned type:

- target/i386/monitor.c

  monitor_get_pc() -> target_ulong eip;

- target/ppc/ppc-qmp-cmds.c

  monitor_get_ccr() -> uint64_t ppc_get_cr(const CPUPPCState *env);

  monitor_get_xer() -> target_ulong cpu_read_xer(const CPUPPCState *env);

  monitor_get_decr() -> target_ulong cpu_ppc_load_decr(CPUPPCState *env);

  monitor_get_tbu() -> uint32_t cpu_ppc_load_tbu(CPUPPCState *env);

  monitor_get_tbl() -> uint64_t cpu_ppc_load_tbl(CPUPPCState *env);

- target/sparc/monitor.c

  monitor_get_psr() -> target_ulong cpu_get_psr(CPUSPARCState *env1);

  monitor_get_reg() -> target_ulong *regwptr;

Convert the MonitorDef::get_value() handler to return unsigned.

Rename the MD_I32/MD_TLONG definitions mechanically doing:

 $ sed -i -e s/MD_I32/MD_U32/g $(git grep -lw MD_I32)
 $ sed -i -e s/MD_TLONG/MD_TULONG/g $(git grep -lw MD_TLONG)

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
---
 include/monitor/hmp-target.h |  7 ++---
 monitor/hmp-target.c         | 12 ++++----
 target/i386/monitor.c        |  8 ++---
 target/m68k/monitor.c        | 60 ++++++++++++++++++------------------
 target/ppc/ppc-qmp-cmds.c    | 25 +++++++--------
 target/sparc/monitor.c       | 10 +++---
 6 files changed, 59 insertions(+), 63 deletions(-)

diff --git a/include/monitor/hmp-target.h b/include/monitor/hmp-target.h
index 5167d17d41d..6d6653aee6e 100644
--- a/include/monitor/hmp-target.h
+++ b/include/monitor/hmp-target.h
@@ -32,14 +32,13 @@ typedef struct MonitorDef MonitorDef;
 struct MonitorDef {
     const char *name;
     int offset;
-    target_long (*get_value)(Monitor *mon, const struct MonitorDef *md,
-                             int val);
+    uint64_t (*get_value)(Monitor *mon, const struct MonitorDef *md, int val);
     int type;
 };
 #endif
 
-#define MD_TLONG 0
-#define MD_I32   1
+#define MD_TULONG 0
+#define MD_U32    1
 
 const MonitorDef *target_monitor_defs(void);
 int target_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval);
diff --git a/monitor/hmp-target.c b/monitor/hmp-target.c
index 420969bd6eb..1600666ee92 100644
--- a/monitor/hmp-target.c
+++ b/monitor/hmp-target.c
@@ -67,7 +67,6 @@ int get_monitor_def(Monitor *mon, int64_t *pval, const char *name)
 {
     const MonitorDef *md = target_monitor_defs();
     CPUState *cs = mon_get_cpu(mon);
-    void *ptr;
     uint64_t tmp = 0;
     int ret;
 
@@ -81,13 +80,14 @@ int get_monitor_def(Monitor *mon, int64_t *pval, const char *name)
                 *pval = md->get_value(mon, md, md->offset);
             } else {
                 CPUArchState *env = mon_get_cpu_env(mon);
-                ptr = (uint8_t *)env + md->offset;
+                void *ptr = (uint8_t *)env + md->offset;
+
                 switch(md->type) {
-                case MD_I32:
-                    *pval = *(int32_t *)ptr;
+                case MD_U32:
+                    *pval = *(uint32_t *)ptr;
                     break;
-                case MD_TLONG:
-                    *pval = *(target_long *)ptr;
+                case MD_TULONG:
+                    *pval = *(target_ulong *)ptr;
                     break;
                 default:
                     *pval = 0;
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 99b32cb7b0f..427f1990399 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -593,8 +593,8 @@ void hmp_mce(Monitor *mon, const QDict *qdict)
     }
 }
 
-static target_long monitor_get_pc(Monitor *mon, const struct MonitorDef *md,
-                                  int val)
+static uint64_t monitor_get_pc(Monitor *mon, const struct MonitorDef *md,
+                               int val)
 {
     CPUArchState *env = mon_get_cpu_env(mon);
     return env->eip + env->segs[R_CS].base;
@@ -602,9 +602,9 @@ static target_long monitor_get_pc(Monitor *mon, const struct MonitorDef *md,
 
 const MonitorDef monitor_defs[] = {
 #define SEG(name, seg) \
-    { name, offsetof(CPUX86State, segs[seg].selector), NULL, MD_I32 },\
+    { name, offsetof(CPUX86State, segs[seg].selector), NULL, MD_U32 },\
     { name ".base", offsetof(CPUX86State, segs[seg].base) },\
-    { name ".limit", offsetof(CPUX86State, segs[seg].limit), NULL, MD_I32 },
+    { name ".limit", offsetof(CPUX86State, segs[seg].limit), NULL, MD_U32 },
 
     { "eax", offsetof(CPUX86State, regs[0]) },
     { "ecx", offsetof(CPUX86State, regs[1]) },
diff --git a/target/m68k/monitor.c b/target/m68k/monitor.c
index 6d101c75df0..1bb5012da91 100644
--- a/target/m68k/monitor.c
+++ b/target/m68k/monitor.c
@@ -24,36 +24,36 @@ void hmp_info_tlb(Monitor *mon, const QDict *qdict)
 }
 
 static const MonitorDef monitor_defs[] = {
-    { "d0", offsetof(CPUM68KState, dregs[0]), NULL, MD_I32 },
-    { "d1", offsetof(CPUM68KState, dregs[1]), NULL, MD_I32 },
-    { "d2", offsetof(CPUM68KState, dregs[2]), NULL, MD_I32 },
-    { "d3", offsetof(CPUM68KState, dregs[3]), NULL, MD_I32 },
-    { "d4", offsetof(CPUM68KState, dregs[4]), NULL, MD_I32 },
-    { "d5", offsetof(CPUM68KState, dregs[5]), NULL, MD_I32 },
-    { "d6", offsetof(CPUM68KState, dregs[6]), NULL, MD_I32 },
-    { "d7", offsetof(CPUM68KState, dregs[7]), NULL, MD_I32 },
-    { "a0", offsetof(CPUM68KState, aregs[0]), NULL, MD_I32 },
-    { "a1", offsetof(CPUM68KState, aregs[1]), NULL, MD_I32 },
-    { "a2", offsetof(CPUM68KState, aregs[2]), NULL, MD_I32 },
-    { "a3", offsetof(CPUM68KState, aregs[3]), NULL, MD_I32 },
-    { "a4", offsetof(CPUM68KState, aregs[4]), NULL, MD_I32 },
-    { "a5", offsetof(CPUM68KState, aregs[5]), NULL, MD_I32 },
-    { "a6", offsetof(CPUM68KState, aregs[6]), NULL, MD_I32 },
-    { "a7", offsetof(CPUM68KState, aregs[7]), NULL, MD_I32 },
-    { "pc", offsetof(CPUM68KState, pc), NULL, MD_I32 },
-    { "sr", offsetof(CPUM68KState, sr), NULL, MD_I32 },
-    { "ssp", offsetof(CPUM68KState, sp[0]), NULL, MD_I32 },
-    { "usp", offsetof(CPUM68KState, sp[1]), NULL, MD_I32 },
-    { "isp", offsetof(CPUM68KState, sp[2]), NULL, MD_I32 },
-    { "sfc", offsetof(CPUM68KState, sfc), NULL, MD_I32 },
-    { "dfc", offsetof(CPUM68KState, dfc), NULL, MD_I32 },
-    { "urp", offsetof(CPUM68KState, mmu.urp), NULL, MD_I32 },
-    { "srp", offsetof(CPUM68KState, mmu.srp), NULL, MD_I32 },
-    { "dttr0", offsetof(CPUM68KState, mmu.ttr[M68K_DTTR0]), NULL, MD_I32 },
-    { "dttr1", offsetof(CPUM68KState, mmu.ttr[M68K_DTTR1]), NULL, MD_I32 },
-    { "ittr0", offsetof(CPUM68KState, mmu.ttr[M68K_ITTR0]), NULL, MD_I32 },
-    { "ittr1", offsetof(CPUM68KState, mmu.ttr[M68K_ITTR1]), NULL, MD_I32 },
-    { "mmusr", offsetof(CPUM68KState, mmu.mmusr), NULL, MD_I32 },
+    { "d0", offsetof(CPUM68KState, dregs[0]), NULL, MD_U32 },
+    { "d1", offsetof(CPUM68KState, dregs[1]), NULL, MD_U32 },
+    { "d2", offsetof(CPUM68KState, dregs[2]), NULL, MD_U32 },
+    { "d3", offsetof(CPUM68KState, dregs[3]), NULL, MD_U32 },
+    { "d4", offsetof(CPUM68KState, dregs[4]), NULL, MD_U32 },
+    { "d5", offsetof(CPUM68KState, dregs[5]), NULL, MD_U32 },
+    { "d6", offsetof(CPUM68KState, dregs[6]), NULL, MD_U32 },
+    { "d7", offsetof(CPUM68KState, dregs[7]), NULL, MD_U32 },
+    { "a0", offsetof(CPUM68KState, aregs[0]), NULL, MD_U32 },
+    { "a1", offsetof(CPUM68KState, aregs[1]), NULL, MD_U32 },
+    { "a2", offsetof(CPUM68KState, aregs[2]), NULL, MD_U32 },
+    { "a3", offsetof(CPUM68KState, aregs[3]), NULL, MD_U32 },
+    { "a4", offsetof(CPUM68KState, aregs[4]), NULL, MD_U32 },
+    { "a5", offsetof(CPUM68KState, aregs[5]), NULL, MD_U32 },
+    { "a6", offsetof(CPUM68KState, aregs[6]), NULL, MD_U32 },
+    { "a7", offsetof(CPUM68KState, aregs[7]), NULL, MD_U32 },
+    { "pc", offsetof(CPUM68KState, pc), NULL, MD_U32 },
+    { "sr", offsetof(CPUM68KState, sr), NULL, MD_U32 },
+    { "ssp", offsetof(CPUM68KState, sp[0]), NULL, MD_U32 },
+    { "usp", offsetof(CPUM68KState, sp[1]), NULL, MD_U32 },
+    { "isp", offsetof(CPUM68KState, sp[2]), NULL, MD_U32 },
+    { "sfc", offsetof(CPUM68KState, sfc), NULL, MD_U32 },
+    { "dfc", offsetof(CPUM68KState, dfc), NULL, MD_U32 },
+    { "urp", offsetof(CPUM68KState, mmu.urp), NULL, MD_U32 },
+    { "srp", offsetof(CPUM68KState, mmu.srp), NULL, MD_U32 },
+    { "dttr0", offsetof(CPUM68KState, mmu.ttr[M68K_DTTR0]), NULL, MD_U32 },
+    { "dttr1", offsetof(CPUM68KState, mmu.ttr[M68K_DTTR1]), NULL, MD_U32 },
+    { "ittr0", offsetof(CPUM68KState, mmu.ttr[M68K_ITTR0]), NULL, MD_U32 },
+    { "ittr1", offsetof(CPUM68KState, mmu.ttr[M68K_ITTR1]), NULL, MD_U32 },
+    { "mmusr", offsetof(CPUM68KState, mmu.mmusr), NULL, MD_U32 },
     { NULL },
 };
 
diff --git a/target/ppc/ppc-qmp-cmds.c b/target/ppc/ppc-qmp-cmds.c
index 7022564604f..07938abb15f 100644
--- a/target/ppc/ppc-qmp-cmds.c
+++ b/target/ppc/ppc-qmp-cmds.c
@@ -33,26 +33,23 @@
 #include "cpu-models.h"
 #include "cpu-qom.h"
 
-static target_long monitor_get_ccr(Monitor *mon, const struct MonitorDef *md,
-                                   int val)
+static uint64_t monitor_get_ccr(Monitor *mon, const struct MonitorDef *md,
+                               int val)
 {
     CPUArchState *env = mon_get_cpu_env(mon);
-    unsigned int u;
 
-    u = ppc_get_cr(env);
-
-    return u;
+    return ppc_get_cr(env);
 }
 
-static target_long monitor_get_xer(Monitor *mon, const struct MonitorDef *md,
-                                   int val)
+static uint64_t monitor_get_xer(Monitor *mon, const struct MonitorDef *md,
+                                int val)
 {
     CPUArchState *env = mon_get_cpu_env(mon);
     return cpu_read_xer(env);
 }
 
-static target_long monitor_get_decr(Monitor *mon, const struct MonitorDef *md,
-                                    int val)
+static uint64_t monitor_get_decr(Monitor *mon, const struct MonitorDef *md,
+                                 int val)
 {
     CPUArchState *env = mon_get_cpu_env(mon);
     if (!env->tb_env) {
@@ -61,8 +58,8 @@ static target_long monitor_get_decr(Monitor *mon, const struct MonitorDef *md,
     return cpu_ppc_load_decr(env);
 }
 
-static target_long monitor_get_tbu(Monitor *mon, const struct MonitorDef *md,
-                                   int val)
+static uint64_t monitor_get_tbu(Monitor *mon, const struct MonitorDef *md,
+                                int val)
 {
     CPUArchState *env = mon_get_cpu_env(mon);
     if (!env->tb_env) {
@@ -71,8 +68,8 @@ static target_long monitor_get_tbu(Monitor *mon, const struct MonitorDef *md,
     return cpu_ppc_load_tbu(env);
 }
 
-static target_long monitor_get_tbl(Monitor *mon, const struct MonitorDef *md,
-                                   int val)
+static uint64_t monitor_get_tbl(Monitor *mon, const struct MonitorDef *md,
+                                int val)
 {
     CPUArchState *env = mon_get_cpu_env(mon);
     if (!env->tb_env) {
diff --git a/target/sparc/monitor.c b/target/sparc/monitor.c
index 73f15aa272d..378967f8164 100644
--- a/target/sparc/monitor.c
+++ b/target/sparc/monitor.c
@@ -40,8 +40,8 @@ void hmp_info_tlb(Monitor *mon, const QDict *qdict)
 }
 
 #ifndef TARGET_SPARC64
-static target_long monitor_get_psr(Monitor *mon, const struct MonitorDef *md,
-                                   int val)
+static uint64_t monitor_get_psr(Monitor *mon, const struct MonitorDef *md,
+                                int val)
 {
     CPUArchState *env = mon_get_cpu_env(mon);
 
@@ -49,8 +49,8 @@ static target_long monitor_get_psr(Monitor *mon, const struct MonitorDef *md,
 }
 #endif
 
-static target_long monitor_get_reg(Monitor *mon, const struct MonitorDef *md,
-                                   int val)
+static uint64_t monitor_get_reg(Monitor *mon, const struct MonitorDef *md,
+                                int val)
 {
     CPUArchState *env = mon_get_cpu_env(mon);
     return env->regwptr[val];
@@ -154,7 +154,7 @@ const MonitorDef monitor_defs[] = {
     { "otherwin", offsetof(CPUSPARCState, otherwin) },
     { "wstate", offsetof(CPUSPARCState, wstate) },
     { "cleanwin", offsetof(CPUSPARCState, cleanwin) },
-    { "fprs", offsetof(CPUSPARCState, fprs), NULL, MD_I32 },
+    { "fprs", offsetof(CPUSPARCState, fprs), NULL, MD_U32 },
 #endif
     { NULL },
 };
-- 
2.52.0


Re: [PATCH v5 5/9] monitor: Have MonitorDef::get_value() return an unsigned type
Posted by Mark Cave-Ayland 1 week ago
On 29/01/2026 16:40, Philippe Mathieu-Daudé wrote:

> All implementations of the get_value() handler return an
> unsigned type:
> 
> - target/i386/monitor.c
> 
>    monitor_get_pc() -> target_ulong eip;
> 
> - target/ppc/ppc-qmp-cmds.c
> 
>    monitor_get_ccr() -> uint64_t ppc_get_cr(const CPUPPCState *env);
> 
>    monitor_get_xer() -> target_ulong cpu_read_xer(const CPUPPCState *env);
> 
>    monitor_get_decr() -> target_ulong cpu_ppc_load_decr(CPUPPCState *env);
> 
>    monitor_get_tbu() -> uint32_t cpu_ppc_load_tbu(CPUPPCState *env);
> 
>    monitor_get_tbl() -> uint64_t cpu_ppc_load_tbl(CPUPPCState *env);
> 
> - target/sparc/monitor.c
> 
>    monitor_get_psr() -> target_ulong cpu_get_psr(CPUSPARCState *env1);
> 
>    monitor_get_reg() -> target_ulong *regwptr;
> 
> Convert the MonitorDef::get_value() handler to return unsigned.
> 
> Rename the MD_I32/MD_TLONG definitions mechanically doing:
> 
>   $ sed -i -e s/MD_I32/MD_U32/g $(git grep -lw MD_I32)
>   $ sed -i -e s/MD_TLONG/MD_TULONG/g $(git grep -lw MD_TLONG)
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
> ---
>   include/monitor/hmp-target.h |  7 ++---
>   monitor/hmp-target.c         | 12 ++++----
>   target/i386/monitor.c        |  8 ++---
>   target/m68k/monitor.c        | 60 ++++++++++++++++++------------------
>   target/ppc/ppc-qmp-cmds.c    | 25 +++++++--------
>   target/sparc/monitor.c       | 10 +++---
>   6 files changed, 59 insertions(+), 63 deletions(-)
> 
> diff --git a/include/monitor/hmp-target.h b/include/monitor/hmp-target.h
> index 5167d17d41d..6d6653aee6e 100644
> --- a/include/monitor/hmp-target.h
> +++ b/include/monitor/hmp-target.h
> @@ -32,14 +32,13 @@ typedef struct MonitorDef MonitorDef;
>   struct MonitorDef {
>       const char *name;
>       int offset;
> -    target_long (*get_value)(Monitor *mon, const struct MonitorDef *md,
> -                             int val);
> +    uint64_t (*get_value)(Monitor *mon, const struct MonitorDef *md, int val);
>       int type;
>   };
>   #endif
>   
> -#define MD_TLONG 0
> -#define MD_I32   1
> +#define MD_TULONG 0
> +#define MD_U32    1
>   
>   const MonitorDef *target_monitor_defs(void);
>   int target_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval);
> diff --git a/monitor/hmp-target.c b/monitor/hmp-target.c
> index 420969bd6eb..1600666ee92 100644
> --- a/monitor/hmp-target.c
> +++ b/monitor/hmp-target.c
> @@ -67,7 +67,6 @@ int get_monitor_def(Monitor *mon, int64_t *pval, const char *name)
>   {
>       const MonitorDef *md = target_monitor_defs();
>       CPUState *cs = mon_get_cpu(mon);
> -    void *ptr;
>       uint64_t tmp = 0;
>       int ret;
>   
> @@ -81,13 +80,14 @@ int get_monitor_def(Monitor *mon, int64_t *pval, const char *name)
>                   *pval = md->get_value(mon, md, md->offset);
>               } else {
>                   CPUArchState *env = mon_get_cpu_env(mon);
> -                ptr = (uint8_t *)env + md->offset;
> +                void *ptr = (uint8_t *)env + md->offset;
> +
>                   switch(md->type) {
> -                case MD_I32:
> -                    *pval = *(int32_t *)ptr;
> +                case MD_U32:
> +                    *pval = *(uint32_t *)ptr;
>                       break;
> -                case MD_TLONG:
> -                    *pval = *(target_long *)ptr;
> +                case MD_TULONG:
> +                    *pval = *(target_ulong *)ptr;
>                       break;
>                   default:
>                       *pval = 0;
> diff --git a/target/i386/monitor.c b/target/i386/monitor.c
> index 99b32cb7b0f..427f1990399 100644
> --- a/target/i386/monitor.c
> +++ b/target/i386/monitor.c
> @@ -593,8 +593,8 @@ void hmp_mce(Monitor *mon, const QDict *qdict)
>       }
>   }
>   
> -static target_long monitor_get_pc(Monitor *mon, const struct MonitorDef *md,
> -                                  int val)
> +static uint64_t monitor_get_pc(Monitor *mon, const struct MonitorDef *md,
> +                               int val)
>   {
>       CPUArchState *env = mon_get_cpu_env(mon);
>       return env->eip + env->segs[R_CS].base;
> @@ -602,9 +602,9 @@ static target_long monitor_get_pc(Monitor *mon, const struct MonitorDef *md,
>   
>   const MonitorDef monitor_defs[] = {
>   #define SEG(name, seg) \
> -    { name, offsetof(CPUX86State, segs[seg].selector), NULL, MD_I32 },\
> +    { name, offsetof(CPUX86State, segs[seg].selector), NULL, MD_U32 },\
>       { name ".base", offsetof(CPUX86State, segs[seg].base) },\
> -    { name ".limit", offsetof(CPUX86State, segs[seg].limit), NULL, MD_I32 },
> +    { name ".limit", offsetof(CPUX86State, segs[seg].limit), NULL, MD_U32 },
>   
>       { "eax", offsetof(CPUX86State, regs[0]) },
>       { "ecx", offsetof(CPUX86State, regs[1]) },
> diff --git a/target/m68k/monitor.c b/target/m68k/monitor.c
> index 6d101c75df0..1bb5012da91 100644
> --- a/target/m68k/monitor.c
> +++ b/target/m68k/monitor.c
> @@ -24,36 +24,36 @@ void hmp_info_tlb(Monitor *mon, const QDict *qdict)
>   }
>   
>   static const MonitorDef monitor_defs[] = {
> -    { "d0", offsetof(CPUM68KState, dregs[0]), NULL, MD_I32 },
> -    { "d1", offsetof(CPUM68KState, dregs[1]), NULL, MD_I32 },
> -    { "d2", offsetof(CPUM68KState, dregs[2]), NULL, MD_I32 },
> -    { "d3", offsetof(CPUM68KState, dregs[3]), NULL, MD_I32 },
> -    { "d4", offsetof(CPUM68KState, dregs[4]), NULL, MD_I32 },
> -    { "d5", offsetof(CPUM68KState, dregs[5]), NULL, MD_I32 },
> -    { "d6", offsetof(CPUM68KState, dregs[6]), NULL, MD_I32 },
> -    { "d7", offsetof(CPUM68KState, dregs[7]), NULL, MD_I32 },
> -    { "a0", offsetof(CPUM68KState, aregs[0]), NULL, MD_I32 },
> -    { "a1", offsetof(CPUM68KState, aregs[1]), NULL, MD_I32 },
> -    { "a2", offsetof(CPUM68KState, aregs[2]), NULL, MD_I32 },
> -    { "a3", offsetof(CPUM68KState, aregs[3]), NULL, MD_I32 },
> -    { "a4", offsetof(CPUM68KState, aregs[4]), NULL, MD_I32 },
> -    { "a5", offsetof(CPUM68KState, aregs[5]), NULL, MD_I32 },
> -    { "a6", offsetof(CPUM68KState, aregs[6]), NULL, MD_I32 },
> -    { "a7", offsetof(CPUM68KState, aregs[7]), NULL, MD_I32 },
> -    { "pc", offsetof(CPUM68KState, pc), NULL, MD_I32 },
> -    { "sr", offsetof(CPUM68KState, sr), NULL, MD_I32 },
> -    { "ssp", offsetof(CPUM68KState, sp[0]), NULL, MD_I32 },
> -    { "usp", offsetof(CPUM68KState, sp[1]), NULL, MD_I32 },
> -    { "isp", offsetof(CPUM68KState, sp[2]), NULL, MD_I32 },
> -    { "sfc", offsetof(CPUM68KState, sfc), NULL, MD_I32 },
> -    { "dfc", offsetof(CPUM68KState, dfc), NULL, MD_I32 },
> -    { "urp", offsetof(CPUM68KState, mmu.urp), NULL, MD_I32 },
> -    { "srp", offsetof(CPUM68KState, mmu.srp), NULL, MD_I32 },
> -    { "dttr0", offsetof(CPUM68KState, mmu.ttr[M68K_DTTR0]), NULL, MD_I32 },
> -    { "dttr1", offsetof(CPUM68KState, mmu.ttr[M68K_DTTR1]), NULL, MD_I32 },
> -    { "ittr0", offsetof(CPUM68KState, mmu.ttr[M68K_ITTR0]), NULL, MD_I32 },
> -    { "ittr1", offsetof(CPUM68KState, mmu.ttr[M68K_ITTR1]), NULL, MD_I32 },
> -    { "mmusr", offsetof(CPUM68KState, mmu.mmusr), NULL, MD_I32 },
> +    { "d0", offsetof(CPUM68KState, dregs[0]), NULL, MD_U32 },
> +    { "d1", offsetof(CPUM68KState, dregs[1]), NULL, MD_U32 },
> +    { "d2", offsetof(CPUM68KState, dregs[2]), NULL, MD_U32 },
> +    { "d3", offsetof(CPUM68KState, dregs[3]), NULL, MD_U32 },
> +    { "d4", offsetof(CPUM68KState, dregs[4]), NULL, MD_U32 },
> +    { "d5", offsetof(CPUM68KState, dregs[5]), NULL, MD_U32 },
> +    { "d6", offsetof(CPUM68KState, dregs[6]), NULL, MD_U32 },
> +    { "d7", offsetof(CPUM68KState, dregs[7]), NULL, MD_U32 },
> +    { "a0", offsetof(CPUM68KState, aregs[0]), NULL, MD_U32 },
> +    { "a1", offsetof(CPUM68KState, aregs[1]), NULL, MD_U32 },
> +    { "a2", offsetof(CPUM68KState, aregs[2]), NULL, MD_U32 },
> +    { "a3", offsetof(CPUM68KState, aregs[3]), NULL, MD_U32 },
> +    { "a4", offsetof(CPUM68KState, aregs[4]), NULL, MD_U32 },
> +    { "a5", offsetof(CPUM68KState, aregs[5]), NULL, MD_U32 },
> +    { "a6", offsetof(CPUM68KState, aregs[6]), NULL, MD_U32 },
> +    { "a7", offsetof(CPUM68KState, aregs[7]), NULL, MD_U32 },
> +    { "pc", offsetof(CPUM68KState, pc), NULL, MD_U32 },
> +    { "sr", offsetof(CPUM68KState, sr), NULL, MD_U32 },
> +    { "ssp", offsetof(CPUM68KState, sp[0]), NULL, MD_U32 },
> +    { "usp", offsetof(CPUM68KState, sp[1]), NULL, MD_U32 },
> +    { "isp", offsetof(CPUM68KState, sp[2]), NULL, MD_U32 },
> +    { "sfc", offsetof(CPUM68KState, sfc), NULL, MD_U32 },
> +    { "dfc", offsetof(CPUM68KState, dfc), NULL, MD_U32 },
> +    { "urp", offsetof(CPUM68KState, mmu.urp), NULL, MD_U32 },
> +    { "srp", offsetof(CPUM68KState, mmu.srp), NULL, MD_U32 },
> +    { "dttr0", offsetof(CPUM68KState, mmu.ttr[M68K_DTTR0]), NULL, MD_U32 },
> +    { "dttr1", offsetof(CPUM68KState, mmu.ttr[M68K_DTTR1]), NULL, MD_U32 },
> +    { "ittr0", offsetof(CPUM68KState, mmu.ttr[M68K_ITTR0]), NULL, MD_U32 },
> +    { "ittr1", offsetof(CPUM68KState, mmu.ttr[M68K_ITTR1]), NULL, MD_U32 },
> +    { "mmusr", offsetof(CPUM68KState, mmu.mmusr), NULL, MD_U32 },
>       { NULL },
>   };
>   
> diff --git a/target/ppc/ppc-qmp-cmds.c b/target/ppc/ppc-qmp-cmds.c
> index 7022564604f..07938abb15f 100644
> --- a/target/ppc/ppc-qmp-cmds.c
> +++ b/target/ppc/ppc-qmp-cmds.c
> @@ -33,26 +33,23 @@
>   #include "cpu-models.h"
>   #include "cpu-qom.h"
>   
> -static target_long monitor_get_ccr(Monitor *mon, const struct MonitorDef *md,
> -                                   int val)
> +static uint64_t monitor_get_ccr(Monitor *mon, const struct MonitorDef *md,
> +                               int val)
>   {
>       CPUArchState *env = mon_get_cpu_env(mon);
> -    unsigned int u;
>   
> -    u = ppc_get_cr(env);
> -
> -    return u;
> +    return ppc_get_cr(env);
>   }
>   
> -static target_long monitor_get_xer(Monitor *mon, const struct MonitorDef *md,
> -                                   int val)
> +static uint64_t monitor_get_xer(Monitor *mon, const struct MonitorDef *md,
> +                                int val)
>   {
>       CPUArchState *env = mon_get_cpu_env(mon);
>       return cpu_read_xer(env);
>   }
>   
> -static target_long monitor_get_decr(Monitor *mon, const struct MonitorDef *md,
> -                                    int val)
> +static uint64_t monitor_get_decr(Monitor *mon, const struct MonitorDef *md,
> +                                 int val)
>   {
>       CPUArchState *env = mon_get_cpu_env(mon);
>       if (!env->tb_env) {
> @@ -61,8 +58,8 @@ static target_long monitor_get_decr(Monitor *mon, const struct MonitorDef *md,
>       return cpu_ppc_load_decr(env);
>   }
>   
> -static target_long monitor_get_tbu(Monitor *mon, const struct MonitorDef *md,
> -                                   int val)
> +static uint64_t monitor_get_tbu(Monitor *mon, const struct MonitorDef *md,
> +                                int val)
>   {
>       CPUArchState *env = mon_get_cpu_env(mon);
>       if (!env->tb_env) {
> @@ -71,8 +68,8 @@ static target_long monitor_get_tbu(Monitor *mon, const struct MonitorDef *md,
>       return cpu_ppc_load_tbu(env);
>   }
>   
> -static target_long monitor_get_tbl(Monitor *mon, const struct MonitorDef *md,
> -                                   int val)
> +static uint64_t monitor_get_tbl(Monitor *mon, const struct MonitorDef *md,
> +                                int val)
>   {
>       CPUArchState *env = mon_get_cpu_env(mon);
>       if (!env->tb_env) {
> diff --git a/target/sparc/monitor.c b/target/sparc/monitor.c
> index 73f15aa272d..378967f8164 100644
> --- a/target/sparc/monitor.c
> +++ b/target/sparc/monitor.c
> @@ -40,8 +40,8 @@ void hmp_info_tlb(Monitor *mon, const QDict *qdict)
>   }
>   
>   #ifndef TARGET_SPARC64
> -static target_long monitor_get_psr(Monitor *mon, const struct MonitorDef *md,
> -                                   int val)
> +static uint64_t monitor_get_psr(Monitor *mon, const struct MonitorDef *md,
> +                                int val)
>   {
>       CPUArchState *env = mon_get_cpu_env(mon);
>   
> @@ -49,8 +49,8 @@ static target_long monitor_get_psr(Monitor *mon, const struct MonitorDef *md,
>   }
>   #endif
>   
> -static target_long monitor_get_reg(Monitor *mon, const struct MonitorDef *md,
> -                                   int val)
> +static uint64_t monitor_get_reg(Monitor *mon, const struct MonitorDef *md,
> +                                int val)
>   {
>       CPUArchState *env = mon_get_cpu_env(mon);
>       return env->regwptr[val];
> @@ -154,7 +154,7 @@ const MonitorDef monitor_defs[] = {
>       { "otherwin", offsetof(CPUSPARCState, otherwin) },
>       { "wstate", offsetof(CPUSPARCState, wstate) },
>       { "cleanwin", offsetof(CPUSPARCState, cleanwin) },
> -    { "fprs", offsetof(CPUSPARCState, fprs), NULL, MD_I32 },
> +    { "fprs", offsetof(CPUSPARCState, fprs), NULL, MD_U32 },
>   #endif
>       { NULL },
>   };

For SPARC:

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.


Re: [PATCH v5 5/9] monitor: Have MonitorDef::get_value() return an unsigned type
Posted by Pierrick Bouvier 1 week, 1 day ago
On 1/29/26 8:40 AM, Philippe Mathieu-Daudé wrote:
> diff --git a/include/monitor/hmp-target.h b/include/monitor/hmp-target.h
> index 5167d17d41d..6d6653aee6e 100644
> --- a/include/monitor/hmp-target.h
> +++ b/include/monitor/hmp-target.h
> @@ -32,14 +32,13 @@ typedef struct MonitorDef MonitorDef;
>   struct MonitorDef {
>       const char *name;
>       int offset;
> -    target_long (*get_value)(Monitor *mon, const struct MonitorDef *md,
> -                             int val);
> +    uint64_t (*get_value)(Monitor *mon, const struct MonitorDef *md, int val);
>       int type;
>   };
>   #endif
>   
> -#define MD_TLONG 0
> -#define MD_I32   1
> +#define MD_TULONG 0
> +#define MD_U32    1
>   
>   const MonitorDef *target_monitor_defs(void);
>   int target_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval);
> diff --git a/monitor/hmp-target.c b/monitor/hmp-target.c
> index 420969bd6eb..1600666ee92 100644
> --- a/monitor/hmp-target.c
> +++ b/monitor/hmp-target.c
> @@ -67,7 +67,6 @@ int get_monitor_def(Monitor *mon, int64_t *pval, const char *name)
>   {
>       const MonitorDef *md = target_monitor_defs();
>       CPUState *cs = mon_get_cpu(mon);
> -    void *ptr;
>       uint64_t tmp = 0;
>       int ret;
>   
> @@ -81,13 +80,14 @@ int get_monitor_def(Monitor *mon, int64_t *pval, const char *name)
>                   *pval = md->get_value(mon, md, md->offset);
>               } else {
>                   CPUArchState *env = mon_get_cpu_env(mon);
> -                ptr = (uint8_t *)env + md->offset;
> +                void *ptr = (uint8_t *)env + md->offset;
> +
>                   switch(md->type) {
> -                case MD_I32:
> -                    *pval = *(int32_t *)ptr;
> +                case MD_U32:
> +                    *pval = *(uint32_t *)ptr;
>                       break;
> -                case MD_TLONG:
> -                    *pval = *(target_long *)ptr;
> +                case MD_TULONG:
> +                    *pval = *(target_ulong *)ptr;
>                       break;
>                   default:
>                       *pval = 0;

This one is not obvious to me.
All the caller chain is using int64_t (maybe wrongly), so would be safer 
to keep a signed value, and adapt the read size.
So best would to merge this diff with next patch in series.