public inbox for qemu-devel@nongnu.org
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dave@treblig.org>
To: "Philippe Mathieu-Daudé" <philmd@linaro.org>
Cc: qemu-devel@nongnu.org,
	Richard Henderson <richard.henderson@linaro.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Pierrick Bouvier <pierrick.bouvier@linaro.org>,
	Markus Armbruster <armbru@redhat.com>,
	Zhao Liu <zhao1.liu@intel.com>,
	Nicholas Piggin <npiggin@gmail.com>,
	Chinmay Rath <rathc@linux.ibm.com>,
	Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>,
	Artyom Tarasenko <atar4qemu@gmail.com>,
	qemu-ppc@nongnu.org
Subject: Re: [PATCH v2 4/8] monitor: Have MonitorDef::get_value() return an unsigned type
Date: Mon, 19 Jan 2026 00:59:53 +0000	[thread overview]
Message-ID: <aW2CCaZlpdPo31io@gallifrey> (raw)
In-Reply-To: <20260117162926.74225-5-philmd@linaro.org>

* 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   |_______/


  reply	other threads:[~2026-01-19  1:00 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-17 16:29 [PATCH v2 0/8] monitor/hmp: Reduce target-specific definitions Philippe Mathieu-Daudé
2026-01-17 16:29 ` [PATCH v2 1/8] target/i386: Include missing 'svm.h' header in 'sev.h' Philippe Mathieu-Daudé
2026-01-19  5:37   ` Zhao Liu
2026-01-17 16:29 ` [PATCH v2 2/8] monitor: Add hmp_cmds_for_target() helper Philippe Mathieu-Daudé
2026-01-19  2:04   ` Dr. David Alan Gilbert
2026-01-17 16:29 ` [PATCH v2 3/8] monitor: Reduce target-specific methods Philippe Mathieu-Daudé
2026-01-19  0:50   ` Dr. David Alan Gilbert
2026-01-17 16:29 ` [PATCH v2 4/8] monitor: Have MonitorDef::get_value() return an unsigned type Philippe Mathieu-Daudé
2026-01-19  0:59   ` Dr. David Alan Gilbert [this message]
2026-01-17 16:29 ` [PATCH v2 5/8] monitor: Have *get_monitor_def() fill an unsigned value Philippe Mathieu-Daudé
2026-01-19  1:12   ` Dr. David Alan Gilbert
2026-01-17 16:29 ` [PATCH v2 6/8] monitor: Truncate target register using ldn_he_p() API Philippe Mathieu-Daudé
2026-01-17 16:29 ` [PATCH v2 7/8] monitor: Reduce target-specific methods further Philippe Mathieu-Daudé
2026-01-19  2:01   ` Dr. David Alan Gilbert
2026-01-17 16:29 ` [PATCH v2 8/8] monitor: Remove 'monitor/hmp-target.h' header Philippe Mathieu-Daudé
2026-01-19  2:03   ` Dr. David Alan Gilbert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aW2CCaZlpdPo31io@gallifrey \
    --to=dave@treblig.org \
    --cc=armbru@redhat.com \
    --cc=atar4qemu@gmail.com \
    --cc=mark.cave-ayland@ilande.co.uk \
    --cc=npiggin@gmail.com \
    --cc=pbonzini@redhat.com \
    --cc=philmd@linaro.org \
    --cc=pierrick.bouvier@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    --cc=rathc@linux.ibm.com \
    --cc=richard.henderson@linaro.org \
    --cc=zhao1.liu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox