1. Patchew
  2. QEMU
  3. [PATCH v2 0/8] monitor/hmp: Reduce target-specific definitions
  4. View patch

[PATCH v2 4/8] monitor: Have MonitorDef::get_value() return an unsigned type

Philippe Mathieu-Daudé posted 8 patches 2 weeks, 6 days ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, "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>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Artyom Tarasenko <atar4qemu@gmail.com>, Max Filippov <jcmvbkbc@gmail.com>
There is a newer version of this series
[PATCH v2 4/8] monitor: Have MonitorDef::get_value() return an unsigned type
Posted by Philippe Mathieu-Daudé 2 weeks, 6 days 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.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/monitor/hmp-target.h |  3 +--
 monitor/hmp-target.c         |  8 ++++----
 target/i386/monitor.c        |  4 ++--
 target/ppc/ppc-qmp-cmds.c    | 25 +++++++++++--------------
 target/sparc/monitor.c       |  8 ++++----
 5 files changed, 22 insertions(+), 26 deletions(-)

diff --git a/include/monitor/hmp-target.h b/include/monitor/hmp-target.h
index b679aaebbff..bd9baeaa3ad 100644
--- a/include/monitor/hmp-target.h
+++ b/include/monitor/hmp-target.h
@@ -32,8 +32,7 @@ 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
diff --git a/monitor/hmp-target.c b/monitor/hmp-target.c
index 420969bd6eb..3fb4fb12508 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;
+                    *pval = *(uint32_t *)ptr;
                     break;
                 case MD_TLONG:
-                    *pval = *(target_long *)ptr;
+                    *pval = *(target_ulong *)ptr;
                     break;
                 default:
                     *pval = 0;
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 99b32cb7b0f..cce23f987ef 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;
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..3e1f4dd5c9c 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];
-- 
2.52.0
Re: [PATCH v2 4/8] monitor: Have MonitorDef::get_value() return an unsigned type
Posted by Dr. David Alan Gilbert 2 weeks, 5 days ago 
* Philippe Mathieu-Daudé (philmd@linaro.org) 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.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/monitor/hmp-target.h |  3 +--
>  monitor/hmp-target.c         |  8 ++++----
>  target/i386/monitor.c        |  4 ++--
>  target/ppc/ppc-qmp-cmds.c    | 25 +++++++++++--------------
>  target/sparc/monitor.c       |  8 ++++----
>  5 files changed, 22 insertions(+), 26 deletions(-)
> 
> diff --git a/include/monitor/hmp-target.h b/include/monitor/hmp-target.h
> index b679aaebbff..bd9baeaa3ad 100644
> --- a/include/monitor/hmp-target.h
> +++ b/include/monitor/hmp-target.h
> @@ -32,8 +32,7 @@ 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
> diff --git a/monitor/hmp-target.c b/monitor/hmp-target.c
> index 420969bd6eb..3fb4fb12508 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;
> +                    *pval = *(uint32_t *)ptr;
>                      break;
>                  case MD_TLONG:
> -                    *pval = *(target_long *)ptr;
> +                    *pval = *(target_ulong *)ptr;
>                      break;

So I think this patch does make sense, but it does feel a bit hideous
to have 'MD_I32' and 'MD_TLONG' as names for unsigneds.

Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>

>                  default:
>                      *pval = 0;
> diff --git a/target/i386/monitor.c b/target/i386/monitor.c
> index 99b32cb7b0f..cce23f987ef 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;
> 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..3e1f4dd5c9c 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];
> -- 
> 2.52.0
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.