[PATCH 07/11] monitor/hmp: Handle gdb-xml exposed registers via gdb_get_register()

Philippe Mathieu-Daudé posted 11 patches 1 month, 3 weeks ago
Maintainers: Laurent Vivier <laurent@vivier.eu>, Pierrick Bouvier <pierrick.bouvier@linaro.org>, "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, "Dr. David Alan Gilbert" <dave@treblig.org>, Paolo Bonzini <pbonzini@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>
[PATCH 07/11] monitor/hmp: Handle gdb-xml exposed registers via gdb_get_register()
Posted by Philippe Mathieu-Daudé 1 month, 3 weeks ago
Implement the gdb_get_register() helper and call it before the
regular get_monitor_def() one. Registers is exposed via the
GDB XML files will be directly handled, possibily allowing new
registers added to XML files to be automatically accessible in
QEMU monitor. All targets having GDB XML files can now be used
within the monitor.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 monitor/hmp.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/monitor/hmp.c b/monitor/hmp.c
index 0a5bbf82197..0e5913fabb1 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -27,14 +27,18 @@
 #include "hw/core/qdev.h"
 #include "monitor-internal.h"
 #include "monitor/hmp.h"
+#include "monitor/hmp-target.h"
 #include "qobject/qdict.h"
 #include "qobject/qnum.h"
+#include "qemu/bswap.h"
 #include "qemu/config-file.h"
 #include "qemu/ctype.h"
 #include "qemu/cutils.h"
 #include "qemu/log.h"
 #include "qemu/option.h"
+#include "qemu/target-info.h"
 #include "qemu/units.h"
+#include "exec/gdbstub.h"
 #include "system/block-backend.h"
 #include "trace.h"
 
@@ -306,6 +310,46 @@ void hmp_help_cmd(Monitor *mon, const char *name)
     free_cmdline_args(args, nb_args);
 }
 
+/*
+ * Set @pval to the value in the register identified by @name.
+ * return %true if the register is found, %false otherwise.
+ */
+static bool gdb_get_register(Monitor *mon, int64_t *pval, const char *name)
+{
+    g_autoptr(GArray) regs = NULL;
+    CPUState *cs = mon_get_cpu(mon);
+
+    if (cs == NULL) {
+        return false;
+    }
+
+    regs = gdb_get_register_list(cs);
+
+    for (int i = 0; i < regs->len; i++) {
+        GDBRegDesc *reg = &g_array_index(regs, GDBRegDesc, i);
+        g_autoptr(GByteArray) buf = NULL;
+        int reg_size;
+
+        if (!reg->name || g_strcmp0(name, reg->name)) {
+            continue;
+        }
+
+        buf = g_byte_array_new();
+        reg_size = gdb_read_register(cs, buf, reg->gdb_reg);
+        if (reg_size > sizeof(*pval)) {
+            return false;
+        }
+
+        if (target_big_endian()) {
+            *pval = ldn_be_p(buf->data, reg_size);
+        } else {
+            *pval = ldn_le_p(buf->data, reg_size);
+        }
+        return true;
+    }
+    return false;
+}
+
 /*******************************************************************/
 
 static const char *pch;
@@ -338,7 +382,6 @@ static int64_t expr_unary(Monitor *mon)
 {
     int64_t n;
     char *p;
-    int ret;
 
     switch (*pch) {
     case '+':
@@ -393,8 +436,8 @@ static int64_t expr_unary(Monitor *mon)
                 pch++;
             }
             *q = 0;
-            ret = get_monitor_def(mon, &reg, buf);
-            if (ret < 0) {
+            if (!gdb_get_register(mon, &reg, buf)
+                && get_monitor_def(mon, &reg, buf) < 0) {
                 expr_error(mon, "unknown register");
             }
             n = reg;
-- 
2.52.0


Re: [PATCH 07/11] monitor/hmp: Handle gdb-xml exposed registers via gdb_get_register()
Posted by Dr. David Alan Gilbert 1 month, 3 weeks ago
* Philippe Mathieu-Daudé (philmd@linaro.org) wrote:
> Implement the gdb_get_register() helper and call it before the
> regular get_monitor_def() one. Registers is exposed via the
> GDB XML files will be directly handled, possibily allowing new
> registers added to XML files to be automatically accessible in
> QEMU monitor. All targets having GDB XML files can now be used
> within the monitor.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Nice;  I might be tempted to add more checks on the return size of
gdb_read_register(..) just in case any arch is a bit screwy
(e.g. if they're 0 for example?) - but it should fine.

So for HMP;

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


> ---
>  monitor/hmp.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 46 insertions(+), 3 deletions(-)
> 
> diff --git a/monitor/hmp.c b/monitor/hmp.c
> index 0a5bbf82197..0e5913fabb1 100644
> --- a/monitor/hmp.c
> +++ b/monitor/hmp.c
> @@ -27,14 +27,18 @@
>  #include "hw/core/qdev.h"
>  #include "monitor-internal.h"
>  #include "monitor/hmp.h"
> +#include "monitor/hmp-target.h"
>  #include "qobject/qdict.h"
>  #include "qobject/qnum.h"
> +#include "qemu/bswap.h"
>  #include "qemu/config-file.h"
>  #include "qemu/ctype.h"
>  #include "qemu/cutils.h"
>  #include "qemu/log.h"
>  #include "qemu/option.h"
> +#include "qemu/target-info.h"
>  #include "qemu/units.h"
> +#include "exec/gdbstub.h"
>  #include "system/block-backend.h"
>  #include "trace.h"
>  
> @@ -306,6 +310,46 @@ void hmp_help_cmd(Monitor *mon, const char *name)
>      free_cmdline_args(args, nb_args);
>  }
>  
> +/*
> + * Set @pval to the value in the register identified by @name.
> + * return %true if the register is found, %false otherwise.
> + */
> +static bool gdb_get_register(Monitor *mon, int64_t *pval, const char *name)
> +{
> +    g_autoptr(GArray) regs = NULL;
> +    CPUState *cs = mon_get_cpu(mon);
> +
> +    if (cs == NULL) {
> +        return false;
> +    }
> +
> +    regs = gdb_get_register_list(cs);
> +
> +    for (int i = 0; i < regs->len; i++) {
> +        GDBRegDesc *reg = &g_array_index(regs, GDBRegDesc, i);
> +        g_autoptr(GByteArray) buf = NULL;
> +        int reg_size;
> +
> +        if (!reg->name || g_strcmp0(name, reg->name)) {
> +            continue;
> +        }
> +
> +        buf = g_byte_array_new();
> +        reg_size = gdb_read_register(cs, buf, reg->gdb_reg);
> +        if (reg_size > sizeof(*pval)) {
> +            return false;
> +        }
> +
> +        if (target_big_endian()) {
> +            *pval = ldn_be_p(buf->data, reg_size);
> +        } else {
> +            *pval = ldn_le_p(buf->data, reg_size);
> +        }
> +        return true;
> +    }
> +    return false;
> +}
> +
>  /*******************************************************************/
>  
>  static const char *pch;
> @@ -338,7 +382,6 @@ static int64_t expr_unary(Monitor *mon)
>  {
>      int64_t n;
>      char *p;
> -    int ret;
>  
>      switch (*pch) {
>      case '+':
> @@ -393,8 +436,8 @@ static int64_t expr_unary(Monitor *mon)
>                  pch++;
>              }
>              *q = 0;
> -            ret = get_monitor_def(mon, &reg, buf);
> -            if (ret < 0) {
> +            if (!gdb_get_register(mon, &reg, buf)
> +                && get_monitor_def(mon, &reg, buf) < 0) {
>                  expr_error(mon, "unknown register");
>              }
>              n = reg;
> -- 
> 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   |_______/
Re: [PATCH 07/11] monitor/hmp: Handle gdb-xml exposed registers via gdb_get_register()
Posted by Philippe Mathieu-Daudé 1 month, 3 weeks ago
On 16/2/26 23:52, Philippe Mathieu-Daudé wrote:
> Implement the gdb_get_register() helper and call it before the
> regular get_monitor_def() one. Registers is exposed via the
> GDB XML files will be directly handled, possibily allowing new
> registers added to XML files to be automatically accessible in
> QEMU monitor. All targets having GDB XML files can now be used
> within the monitor.

For example with Loongarch, before:

   $ qemu-system-loongarch64 -M virt -S -monitor stdio
   QEMU 10.2.0 monitor - type 'help' for more information

   (qemu) info registers

   CPU#0
    PC=000000001c000000  FCSR0 0x00000000
    ...

   (qemu) p/x $pc
   unknown register
   Try "help p" for more information
   (qemu)

and after:

   $ ./qemu-system-loongarch64 -M virt -S -monitor stdio
   QEMU 10.2.50 monitor - type 'help' for more information
   (qemu) p/x $pc
   0x1c000000
   (qemu)

Similarly RISC-V:

   QEMU 10.2.0 monitor - type 'help' for more information
   (qemu) p/x $pc
   unknown register
   Try "help p" for more information

VS

   QEMU 10.2.50 monitor - type 'help' for more information
   (qemu) p/x $pc
   0x1000
   (qemu)

> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   monitor/hmp.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 46 insertions(+), 3 deletions(-)