[PATCH-for-11.1 00/10] monitor: Remove need of per-target handlers

[PATCH-for-11.1 00/10] monitor: Remove need of per-target handlers

[Philippe Mathieu-Daudé]

Convert target_monitor_defs() and target_monitor_defs()
to CPUClass fields, adapting the 4 targets using these
legacy code.

Philippe Mathieu-Daudé (10):
  monitor: Extract completion declarations to 'monitor/hmp-completion.h'
  monitor: Forward-declare the MonitorDef type
  cpus: Introduce CPUClass::legacy_monitor_defs hook
  target/i386: Replace legacy target_monitor_defs -> legacy_monitor_defs
  target/m68k: Replace legacy target_monitor_defs -> legacy_monitor_defs
  target/sparc: Replace legacy target_monitor_defs ->
    legacy_monitor_defs
  monitor: Remove target_monitor_defs()
  cpus: Introduce CPUClass::legacy_monitor_get_register() hook
  target/riscv: Register target_get_monitor_def in CPUClass
  monitor: Remove target_get_monitor_def()

 MAINTAINERS                      |  1 +
 include/hw/core/cpu.h            |  8 +++++++
 include/monitor/hmp-completion.h | 40 ++++++++++++++++++++++++++++++++
 include/monitor/hmp.h            | 28 ++--------------------
 include/qemu/typedefs.h          |  1 +
 target/i386/cpu.h                |  2 ++
 target/m68k/cpu.h                |  2 ++
 target/riscv/internals.h         |  3 +++
 target/sparc/cpu.h               |  2 ++
 chardev/char-hmp-cmds.c          |  1 +
 migration/migration-hmp-cmds.c   |  1 +
 monitor/hmp-target.c             |  1 +
 monitor/hmp.c                    |  8 +++++--
 net/net-hmp-cmds.c               |  1 +
 qom/qom-hmp-cmds.c               |  1 +
 stubs/target-get-monitor-def.c   | 29 -----------------------
 stubs/target-monitor-defs.c      |  7 ------
 system/qdev-monitor.c            |  1 +
 system/runstate-hmp-cmds.c       |  1 +
 target/i386/cpu.c                |  1 +
 target/i386/monitor.c            |  7 ++----
 target/m68k/cpu.c                |  1 +
 target/m68k/monitor.c            |  5 +---
 target/riscv/cpu.c               |  1 +
 target/riscv/monitor.c           |  4 +++-
 target/sparc/cpu.c               |  3 +++
 target/sparc/monitor.c           |  9 +++----
 trace/trace-hmp-cmds.c           |  1 +
 ui/ui-hmp-cmds.c                 |  1 +
 stubs/meson.build                |  2 --
 30 files changed, 91 insertions(+), 82 deletions(-)
 create mode 100644 include/monitor/hmp-completion.h
 delete mode 100644 stubs/target-get-monitor-def.c
 delete mode 100644 stubs/target-monitor-defs.c

-- 
2.53.0

[PATCH-for-11.1 01/10] monitor: Extract completion declarations to 'monitor/hmp-completion.h'

[Philippe Mathieu-Daudé]

Many files include "monitor/hmp.h", but few of them really need
the completion declarations: move them to a distinct header.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 MAINTAINERS                      |  1 +
 include/monitor/hmp-completion.h | 40 ++++++++++++++++++++++++++++++++
 include/monitor/hmp.h            | 21 -----------------
 chardev/char-hmp-cmds.c          |  1 +
 migration/migration-hmp-cmds.c   |  1 +
 monitor/hmp-target.c             |  1 +
 net/net-hmp-cmds.c               |  1 +
 qom/qom-hmp-cmds.c               |  1 +
 system/qdev-monitor.c            |  1 +
 system/runstate-hmp-cmds.c       |  1 +
 trace/trace-hmp-cmds.c           |  1 +
 ui/ui-hmp-cmds.c                 |  1 +
 12 files changed, 50 insertions(+), 21 deletions(-)
 create mode 100644 include/monitor/hmp-completion.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 48976138be7..969c3601beb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3409,6 +3409,7 @@ F: monitor/hmp*
 F: hmp.h
 F: hmp-commands*.hx
 F: include/monitor/hmp.h
+F: include/monitor/hmp-completion.h
 F: tests/qtest/test-hmp.c
 F: include/qemu/qemu-print.h
 F: util/qemu-print.c
diff --git a/include/monitor/hmp-completion.h b/include/monitor/hmp-completion.h
new file mode 100644
index 00000000000..dab21867dc9
--- /dev/null
+++ b/include/monitor/hmp-completion.h
@@ -0,0 +1,40 @@
+/*
+ * Human Monitor Completion handlers
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HMP_COMPLETION_H
+#define HMP_COMPLETION_H
+
+#include "qemu/readline.h"
+
+void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
+void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
+void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
+void device_del_completion(ReadLineState *rs, int nb_args, const char *str);
+void sendkey_completion(ReadLineState *rs, int nb_args, const char *str);
+void chardev_remove_completion(ReadLineState *rs, int nb_args, const char *str);
+void chardev_add_completion(ReadLineState *rs, int nb_args, const char *str);
+void set_link_completion(ReadLineState *rs, int nb_args, const char *str);
+void netdev_add_completion(ReadLineState *rs, int nb_args, const char *str);
+void netdev_del_completion(ReadLineState *rs, int nb_args, const char *str);
+void ringbuf_write_completion(ReadLineState *rs, int nb_args, const char *str);
+void info_trace_events_completion(ReadLineState *rs, int nb_args,
+                                  const char *str);
+void trace_event_completion(ReadLineState *rs, int nb_args, const char *str);
+void watchdog_action_completion(ReadLineState *rs, int nb_args,
+                                const char *str);
+void migrate_set_capability_completion(ReadLineState *rs, int nb_args,
+                                       const char *str);
+void migrate_set_parameter_completion(ReadLineState *rs, int nb_args,
+                                      const char *str);
+void delvm_completion(ReadLineState *rs, int nb_args, const char *str);
+void loadvm_completion(ReadLineState *rs, int nb_args, const char *str);
+
+#endif
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index 26571d35e84..f17a50a3fed 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -126,27 +126,6 @@ void hmp_vhost_queue_status(Monitor *mon, const QDict *qdict);
 void hmp_virtio_queue_element(Monitor *mon, const QDict *qdict);
 void hmp_xen_event_inject(Monitor *mon, const QDict *qdict);
 void hmp_xen_event_list(Monitor *mon, const QDict *qdict);
-void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
-void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
-void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
-void device_del_completion(ReadLineState *rs, int nb_args, const char *str);
-void sendkey_completion(ReadLineState *rs, int nb_args, const char *str);
-void chardev_remove_completion(ReadLineState *rs, int nb_args, const char *str);
-void chardev_add_completion(ReadLineState *rs, int nb_args, const char *str);
-void set_link_completion(ReadLineState *rs, int nb_args, const char *str);
-void netdev_add_completion(ReadLineState *rs, int nb_args, const char *str);
-void netdev_del_completion(ReadLineState *rs, int nb_args, const char *str);
-void ringbuf_write_completion(ReadLineState *rs, int nb_args, const char *str);
-void info_trace_events_completion(ReadLineState *rs, int nb_args, const char *str);
-void trace_event_completion(ReadLineState *rs, int nb_args, const char *str);
-void watchdog_action_completion(ReadLineState *rs, int nb_args,
-                                const char *str);
-void migrate_set_capability_completion(ReadLineState *rs, int nb_args,
-                                       const char *str);
-void migrate_set_parameter_completion(ReadLineState *rs, int nb_args,
-                                      const char *str);
-void delvm_completion(ReadLineState *rs, int nb_args, const char *str);
-void loadvm_completion(ReadLineState *rs, int nb_args, const char *str);
 void hmp_rocker(Monitor *mon, const QDict *qdict);
 void hmp_rocker_ports(Monitor *mon, const QDict *qdict);
 void hmp_rocker_of_dpa_flows(Monitor *mon, const QDict *qdict);
diff --git a/chardev/char-hmp-cmds.c b/chardev/char-hmp-cmds.c
index 8e9e1c1c021..f377e00b699 100644
--- a/chardev/char-hmp-cmds.c
+++ b/chardev/char-hmp-cmds.c
@@ -16,6 +16,7 @@
 #include "qemu/osdep.h"
 #include "chardev/char.h"
 #include "monitor/hmp.h"
+#include "monitor/hmp-completion.h"
 #include "monitor/monitor.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-char.h"
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 0a193b8f54a..d5712d11b8c 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -17,6 +17,7 @@
 #include "block/qapi.h"
 #include "migration/snapshot.h"
 #include "monitor/hmp.h"
+#include "monitor/hmp-completion.h"
 #include "monitor/monitor.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-migration.h"
diff --git a/monitor/hmp-target.c b/monitor/hmp-target.c
index 2574c5d8b4b..d07e545ef7e 100644
--- a/monitor/hmp-target.c
+++ b/monitor/hmp-target.c
@@ -28,6 +28,7 @@
 #include "net/slirp.h"
 #include "system/device_tree.h"
 #include "monitor/hmp.h"
+#include "monitor/hmp-completion.h"
 #include "block/block-hmp-cmds.h"
 #include "qapi/qapi-commands-control.h"
 #include "qapi/qapi-commands-misc.h"
diff --git a/net/net-hmp-cmds.c b/net/net-hmp-cmds.c
index e7c55d27876..2b24c9e6049 100644
--- a/net/net-hmp-cmds.c
+++ b/net/net-hmp-cmds.c
@@ -16,6 +16,7 @@
 #include "qemu/osdep.h"
 #include "migration/misc.h"
 #include "monitor/hmp.h"
+#include "monitor/hmp-completion.h"
 #include "monitor/monitor.h"
 #include "net/net.h"
 #include "net/hub.h"
diff --git a/qom/qom-hmp-cmds.c b/qom/qom-hmp-cmds.c
index 32e40630c96..47110ca58c0 100644
--- a/qom/qom-hmp-cmds.c
+++ b/qom/qom-hmp-cmds.c
@@ -8,6 +8,7 @@
 #include "qemu/osdep.h"
 #include "hw/core/qdev.h"
 #include "monitor/hmp.h"
+#include "monitor/hmp-completion.h"
 #include "monitor/monitor.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-qom.h"
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 1ac6d9a8575..2ed95e83602 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -20,6 +20,7 @@
 #include "qemu/osdep.h"
 #include "hw/core/sysbus.h"
 #include "monitor/hmp.h"
+#include "monitor/hmp-completion.h"
 #include "monitor/monitor.h"
 #include "monitor/qdev.h"
 #include "system/arch_init.h"
diff --git a/system/runstate-hmp-cmds.c b/system/runstate-hmp-cmds.c
index be1d6769926..02d1d42bf39 100644
--- a/system/runstate-hmp-cmds.c
+++ b/system/runstate-hmp-cmds.c
@@ -16,6 +16,7 @@
 #include "qemu/osdep.h"
 #include "exec/cpu-common.h"
 #include "monitor/hmp.h"
+#include "monitor/hmp-completion.h"
 #include "monitor/monitor.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-run-state.h"
diff --git a/trace/trace-hmp-cmds.c b/trace/trace-hmp-cmds.c
index 45f4335ff5d..390173095cf 100644
--- a/trace/trace-hmp-cmds.c
+++ b/trace/trace-hmp-cmds.c
@@ -24,6 +24,7 @@
 
 #include "qemu/osdep.h"
 #include "monitor/hmp.h"
+#include "monitor/hmp-completion.h"
 #include "monitor/monitor.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-trace.h"
diff --git a/ui/ui-hmp-cmds.c b/ui/ui-hmp-cmds.c
index 6c93d452c9c..1e9bc77bd80 100644
--- a/ui/ui-hmp-cmds.c
+++ b/ui/ui-hmp-cmds.c
@@ -18,6 +18,7 @@
 #include <spice/enums.h>
 #endif
 #include "monitor/hmp.h"
+#include "monitor/hmp-completion.h"
 #include "monitor/monitor-internal.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-ui.h"
-- 
2.53.0

[PATCH-for-11.1 02/10] monitor: Forward-declare the MonitorDef type

[Philippe Mathieu-Daudé]

Rather than having core header forced to include "monitor/hmp.h"
to get the MonitorDef type declaration, forward-declare it in
"qemu/typedefs.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/monitor/hmp.h   | 4 ++--
 include/qemu/typedefs.h | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index f17a50a3fed..a22cab4ae37 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -17,11 +17,11 @@
 #include "qemu/readline.h"
 #include "qapi/qapi-types-common.h"
 
-typedef struct MonitorDef {
+struct MonitorDef {
     const char *name;
     int offset;
     int64_t (*get_value)(Monitor *mon, const struct MonitorDef *md, int offset);
-} MonitorDef;
+};
 
 const MonitorDef *target_monitor_defs(void);
 int target_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval);
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 416a8c9acea..07f2ae7c9f1 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -72,6 +72,7 @@ typedef struct MemoryRegionSection MemoryRegionSection;
 typedef struct MigrationIncomingState MigrationIncomingState;
 typedef struct MigrationState MigrationState;
 typedef struct Monitor Monitor;
+typedef struct MonitorDef MonitorDef;
 typedef struct MSIMessage MSIMessage;
 typedef struct NetClientState NetClientState;
 typedef struct NetFilterState NetFilterState;
-- 
2.53.0

[PATCH-for-11.1 03/10] cpus: Introduce CPUClass::legacy_monitor_defs hook

[Philippe Mathieu-Daudé]

Allow targets to register their legacy target_monitor_defs()
in CPUClass; check it first in get_monitor_def() otherwise
fall back to previous per-target helper.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/core/cpu.h | 3 +++
 monitor/hmp.c         | 3 ++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 04e1f970caf..072f58bead5 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -148,6 +148,8 @@ struct SysemuCPUOps;
  * @max_as: Maximum valid index used to refer to the address spaces supported by
  *          the architecture, i.e., to refer to CPUAddressSpaces in
  *          CPUState::cpu_ases.
+ * @legacy_monitor_defs: Array of MonitorDef entries. This field is legacy,
+ *                       use @gdb_core_xml_file to dump registers instead.
  *
  * Represents a CPU family or model.
  */
@@ -174,6 +176,7 @@ struct CPUClass {
     const char *gdb_core_xml_file;
     const char * (*gdb_arch_name)(CPUState *cpu);
     const char * (*gdb_get_core_xml_file)(CPUState *cpu);
+    const MonitorDef *legacy_monitor_defs;
 
     void (*disas_set_info)(const CPUState *cpu, disassemble_info *info);
 
diff --git a/monitor/hmp.c b/monitor/hmp.c
index c63da13e310..a2b6269d0ff 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -24,6 +24,7 @@
 
 #include "qemu/osdep.h"
 #include <dirent.h>
+#include "hw/core/cpu.h"
 #include "hw/core/qdev.h"
 #include "monitor-internal.h"
 #include "monitor/hmp.h"
@@ -1603,8 +1604,8 @@ void monitor_register_hmp_info_hrt(const char *name,
  */
 int get_monitor_def(Monitor *mon, int64_t *pval, const char *name)
 {
-    const MonitorDef *md = target_monitor_defs();
     CPUState *cs = mon_get_cpu(mon);
+    const MonitorDef *md = cs->cc->legacy_monitor_defs ?: target_monitor_defs();
     void *ptr;
     uint64_t tmp = 0;
     int ret;
-- 
2.53.0

[PATCH-for-11.1 04/10] target/i386: Replace legacy target_monitor_defs -> legacy_monitor_defs

[Philippe Mathieu-Daudé]

Expose x86_monitor_defs within target/i386/ and register
it as CPUClass::legacy_monitor_defs hook.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/i386/cpu.h     | 2 ++
 target/i386/cpu.c     | 1 +
 target/i386/monitor.c | 7 ++-----
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index b0c730b3390..7420ee3b8ea 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -34,6 +34,8 @@
 #include "qemu/timer.h"
 #include "standard-headers/asm-x86/kvm_para.h"
 
+extern const MonitorDef *x86_monitor_defs;
+
 #define XEN_NR_VIRQS 24
 
 #ifdef TARGET_X86_64
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index b5e483e8cd2..c5d9c51c1ad 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -10745,6 +10745,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, const void *data)
     cc->gdb_core_xml_file = "i386-32bit.xml";
 #endif
     cc->disas_set_info = x86_disas_set_info;
+    cc->legacy_monitor_defs = x86_monitor_defs;
 
     dc->user_creatable = true;
 
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index ddc666d3451..9a0cc73bbf4 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -599,7 +599,7 @@ static int64_t monitor_get_pc(Monitor *mon, const struct MonitorDef *md,
     return env->eip + env->segs[R_CS].base;
 }
 
-const MonitorDef monitor_defs[] = {
+static const MonitorDef monitor_defs[] = {
 #define SEG(name, seg) \
     { name ".limit", offsetof(CPUX86State, segs[seg].limit) },
     SEG("cs", R_CS)
@@ -612,7 +612,4 @@ const MonitorDef monitor_defs[] = {
     { NULL },
 };
 
-const MonitorDef *target_monitor_defs(void)
-{
-    return monitor_defs;
-}
+const MonitorDef *x86_monitor_defs = monitor_defs;
-- 
2.53.0

[PATCH-for-11.1 05/10] target/m68k: Replace legacy target_monitor_defs -> legacy_monitor_defs

[Philippe Mathieu-Daudé]

Expose m68k_monitor_defs within target/m68k/ and register
it as CPUClass::legacy_monitor_defs hook.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/m68k/cpu.h     | 2 ++
 target/m68k/cpu.c     | 1 +
 target/m68k/monitor.c | 5 +----
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
index d7f508a9e80..6f19b53158e 100644
--- a/target/m68k/cpu.h
+++ b/target/m68k/cpu.h
@@ -26,6 +26,8 @@
 #include "qemu/cpu-float.h"
 #include "cpu-qom.h"
 
+extern const MonitorDef *m68k_monitor_defs;
+
 #define OS_BYTE     0
 #define OS_WORD     1
 #define OS_LONG     2
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index d849a4a90fc..fcb14a44793 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -655,6 +655,7 @@ static void m68k_cpu_class_init(ObjectClass *c, const void *data)
     cc->sysemu_ops = &m68k_sysemu_ops;
 #endif
     cc->disas_set_info = m68k_cpu_disas_set_info;
+    cc->legacy_monitor_defs = m68k_monitor_defs;
 
     cc->tcg_ops = &m68k_tcg_ops;
 }
diff --git a/target/m68k/monitor.c b/target/m68k/monitor.c
index be7411edca6..97ec2bd38bc 100644
--- a/target/m68k/monitor.c
+++ b/target/m68k/monitor.c
@@ -38,7 +38,4 @@ static const MonitorDef monitor_defs[] = {
     { NULL },
 };
 
-const MonitorDef *target_monitor_defs(void)
-{
-    return monitor_defs;
-}
+const MonitorDef *m68k_monitor_defs = monitor_defs;
-- 
2.53.0

[PATCH-for-11.1 06/10] target/sparc: Replace legacy target_monitor_defs -> legacy_monitor_defs

[Philippe Mathieu-Daudé]

Expose sparc64_monitor_defs within target/sparc/ and register
it as CPUClass::legacy_monitor_defs hook (taking care to not
register it on 32-bit SPARC target).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/sparc/cpu.h     | 2 ++
 target/sparc/cpu.c     | 3 +++
 target/sparc/monitor.c | 9 +++------
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
index ac0eb9f0faa..83f6859019d 100644
--- a/target/sparc/cpu.h
+++ b/target/sparc/cpu.h
@@ -7,6 +7,8 @@
 #include "exec/target_long.h"
 #include "qemu/cpu-float.h"
 
+extern const MonitorDef *sparc64_monitor_defs;
+
 #if !defined(TARGET_SPARC64)
 #define TARGET_DPREGS 16
 #define TARGET_FCCREGS 1
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index 1493336e7a2..65468ceeb78 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -1096,6 +1096,9 @@ static void sparc_cpu_class_init(ObjectClass *oc, const void *data)
     cc->gdb_core_xml_file = "sparc64-cpu.xml";
 #else
     cc->gdb_core_xml_file = "sparc32-cpu.xml";
+#endif
+#if defined(TARGET_SPARC64)
+    cc->legacy_monitor_defs = sparc64_monitor_defs;
 #endif
     cc->tcg_ops = &sparc_tcg_ops;
 }
diff --git a/target/sparc/monitor.c b/target/sparc/monitor.c
index 99abddbf1e0..377c2b18cab 100644
--- a/target/sparc/monitor.c
+++ b/target/sparc/monitor.c
@@ -38,8 +38,8 @@ void hmp_info_tlb(Monitor *mon, const QDict *qdict)
     dump_mmu(env1);
 }
 
-const MonitorDef monitor_defs[] = {
 #ifdef TARGET_SPARC64
+static const MonitorDef monitor_defs[] = {
     { "asi", offsetof(CPUSPARCState, asi) },
     { "pstate", offsetof(CPUSPARCState, pstate) },
     { "cansave", offsetof(CPUSPARCState, cansave) },
@@ -47,11 +47,8 @@ const MonitorDef monitor_defs[] = {
     { "otherwin", offsetof(CPUSPARCState, otherwin) },
     { "wstate", offsetof(CPUSPARCState, wstate) },
     { "cleanwin", offsetof(CPUSPARCState, cleanwin) },
-#endif
     { NULL },
 };
 
-const MonitorDef *target_monitor_defs(void)
-{
-    return monitor_defs;
-}
+const MonitorDef *sparc64_monitor_defs = monitor_defs;
+#endif
-- 
2.53.0

[PATCH-for-11.1 07/10] monitor: Remove target_monitor_defs()

[Philippe Mathieu-Daudé]

target_monitor_defs() is now only a stub. Remove as pointless.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/monitor/hmp.h       | 1 -
 monitor/hmp.c               | 2 +-
 stubs/target-monitor-defs.c | 7 -------
 stubs/meson.build           | 1 -
 4 files changed, 1 insertion(+), 10 deletions(-)
 delete mode 100644 stubs/target-monitor-defs.c

diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index a22cab4ae37..ea7e53060a0 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -23,7 +23,6 @@ struct MonitorDef {
     int64_t (*get_value)(Monitor *mon, const struct MonitorDef *md, int offset);
 };
 
-const MonitorDef *target_monitor_defs(void);
 int target_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval);
 
 CPUArchState *mon_get_cpu_env(Monitor *mon);
diff --git a/monitor/hmp.c b/monitor/hmp.c
index a2b6269d0ff..6dfc59725c2 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -1605,7 +1605,7 @@ void monitor_register_hmp_info_hrt(const char *name,
 int get_monitor_def(Monitor *mon, int64_t *pval, const char *name)
 {
     CPUState *cs = mon_get_cpu(mon);
-    const MonitorDef *md = cs->cc->legacy_monitor_defs ?: target_monitor_defs();
+    const MonitorDef *md = cs->cc->legacy_monitor_defs;
     void *ptr;
     uint64_t tmp = 0;
     int ret;
diff --git a/stubs/target-monitor-defs.c b/stubs/target-monitor-defs.c
deleted file mode 100644
index 0dd4cdb34f6..00000000000
--- a/stubs/target-monitor-defs.c
+++ /dev/null
@@ -1,7 +0,0 @@
-#include "qemu/osdep.h"
-#include "monitor/hmp.h"
-
-const MonitorDef *target_monitor_defs(void)
-{
-    return NULL;
-}
diff --git a/stubs/meson.build b/stubs/meson.build
index 7189ff63ed5..10eb68ab55e 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -75,7 +75,6 @@ if have_system
   endif
   stub_ss.add(files('kvm.c'))
   stub_ss.add(files('target-get-monitor-def.c'))
-  stub_ss.add(files('target-monitor-defs.c'))
   stub_ss.add(files('win32-kbd-hook.c'))
   stub_ss.add(files('xen-hw-stub.c'))
   stub_ss.add(files('monitor-arm-gic.c'))
-- 
2.53.0

[PATCH-for-11.1 08/10] cpus: Introduce CPUClass::legacy_monitor_get_register() hook

[Philippe Mathieu-Daudé]

Allow targets to register their legacy target_get_monitor_def()
in CPUClass; check it first in get_monitor_def() otherwise fall
back to previous per-target helper.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/core/cpu.h | 5 +++++
 monitor/hmp.c         | 6 +++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 072f58bead5..8f9d62bfafb 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -150,6 +150,9 @@ struct SysemuCPUOps;
  *          CPUState::cpu_ases.
  * @legacy_monitor_defs: Array of MonitorDef entries. This field is legacy,
  *                       use @gdb_core_xml_file to dump registers instead.
+ * @legacy_monitor_get_register: Callback to fill @pval with register @name.
+ *                               This field is legacy, use @gdb_core_xml_file
+ *                               to dump registers instead.
  *
  * Represents a CPU family or model.
  */
@@ -177,6 +180,8 @@ struct CPUClass {
     const char * (*gdb_arch_name)(CPUState *cpu);
     const char * (*gdb_get_core_xml_file)(CPUState *cpu);
     const MonitorDef *legacy_monitor_defs;
+    int (*legacy_monitor_get_register)(CPUState *cs, const char *name,
+                                       uint64_t *pval);
 
     void (*disas_set_info)(const CPUState *cpu, disassemble_info *info);
 
diff --git a/monitor/hmp.c b/monitor/hmp.c
index 6dfc59725c2..ad253347892 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -1628,7 +1628,11 @@ int get_monitor_def(Monitor *mon, int64_t *pval, const char *name)
         }
     }
 
-    ret = target_get_monitor_def(cs, name, &tmp);
+    if (cs->cc->legacy_monitor_get_register) {
+        ret = cs->cc->legacy_monitor_get_register(cs, name, &tmp);
+    } else {
+        ret = target_get_monitor_def(cs, name, &tmp);
+    }
     if (!ret) {
         *pval = target_long_bits() == 32 ? (int32_t)tmp : tmp;
     }
-- 
2.53.0

[PATCH-for-11.1 09/10] target/riscv: Register target_get_monitor_def in CPUClass

[Philippe Mathieu-Daudé]

Rename target_get_monitor_def() as riscv_monitor_get_register_legacy()
and register it as CPUClass::legacy_monitor_get_register() handler.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/riscv/internals.h | 3 +++
 target/riscv/cpu.c       | 1 +
 target/riscv/monitor.c   | 4 +++-
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/target/riscv/internals.h b/target/riscv/internals.h
index 460346dd6de..90225d322ff 100644
--- a/target/riscv/internals.h
+++ b/target/riscv/internals.h
@@ -245,4 +245,7 @@ static inline int insn_len(uint16_t first_word)
     return (first_word & 3) == 3 ? 4 : 2;
 }
 
+int riscv_monitor_get_register_legacy(CPUState *cs, const char *name,
+                                      uint64_t *pval);
+
 #endif
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 8ac935ac06e..e73d15476a7 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -2750,6 +2750,7 @@ static void riscv_cpu_common_class_init(ObjectClass *c, const void *data)
     cc->get_arch_id = riscv_get_arch_id;
 #endif
     cc->gdb_arch_name = riscv_gdb_arch_name;
+    cc->legacy_monitor_get_register = riscv_monitor_get_register_legacy;
 #ifdef CONFIG_TCG
     cc->tcg_ops = &riscv_tcg_ops;
 #endif /* CONFIG_TCG */
diff --git a/target/riscv/monitor.c b/target/riscv/monitor.c
index 3f206b9fca5..1c90c779534 100644
--- a/target/riscv/monitor.c
+++ b/target/riscv/monitor.c
@@ -26,6 +26,7 @@
 #include "monitor/monitor.h"
 #include "monitor/hmp.h"
 #include "system/memory.h"
+#include "internals.h"
 
 #ifdef TARGET_RISCV64
 #define PTE_HEADER_FIELDS       "vaddr            paddr            "\
@@ -310,7 +311,8 @@ static bool reg_is_vreg(const char *name)
     return false;
 }
 
-int target_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval)
+int riscv_monitor_get_register_legacy(CPUState *cs, const char *name,
+                                      uint64_t *pval)
 {
     CPURISCVState *env = &RISCV_CPU(cs)->env;
     target_ulong val = 0;
-- 
2.53.0

[PATCH-for-11.1 10/10] monitor: Remove target_get_monitor_def()

[Philippe Mathieu-Daudé]

target_get_monitor_def() is now only a stub. Remove as pointless.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/monitor/hmp.h          |  2 --
 monitor/hmp.c                  |  7 +++----
 stubs/target-get-monitor-def.c | 29 -----------------------------
 stubs/meson.build              |  1 -
 4 files changed, 3 insertions(+), 36 deletions(-)
 delete mode 100644 stubs/target-get-monitor-def.c

diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index ea7e53060a0..a335e4ccd3e 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -23,8 +23,6 @@ struct MonitorDef {
     int64_t (*get_value)(Monitor *mon, const struct MonitorDef *md, int offset);
 };
 
-int target_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval);
-
 CPUArchState *mon_get_cpu_env(Monitor *mon);
 CPUState *mon_get_cpu(Monitor *mon);
 
diff --git a/monitor/hmp.c b/monitor/hmp.c
index ad253347892..33197b35e41 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -1628,11 +1628,10 @@ int get_monitor_def(Monitor *mon, int64_t *pval, const char *name)
         }
     }
 
-    if (cs->cc->legacy_monitor_get_register) {
-        ret = cs->cc->legacy_monitor_get_register(cs, name, &tmp);
-    } else {
-        ret = target_get_monitor_def(cs, name, &tmp);
+    if (!cs->cc->legacy_monitor_get_register) {
+        return -1;
     }
+    ret = cs->cc->legacy_monitor_get_register(cs, name, &tmp);
     if (!ret) {
         *pval = target_long_bits() == 32 ? (int32_t)tmp : tmp;
     }
diff --git a/stubs/target-get-monitor-def.c b/stubs/target-get-monitor-def.c
deleted file mode 100644
index 394e0f9a7d2..00000000000
--- a/stubs/target-get-monitor-def.c
+++ /dev/null
@@ -1,29 +0,0 @@
-/*
- *  Stub for target_get_monitor_def.
- *
- *  Copyright IBM Corp., 2015
- *
- *  Author: Alexey Kardashevskiy <aik@ozlabs.ru>
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 2 of the License,
- *  or (at your option) any later version.
- *
- *  This program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with this program; if not, see <http://www.gnu.org/licenses/>.
- */
-
-#include "qemu/osdep.h"
-
-int target_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval);
-
-int target_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval)
-{
-    return -1;
-}
diff --git a/stubs/meson.build b/stubs/meson.build
index 10eb68ab55e..85f9e3a4f23 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -74,7 +74,6 @@ if have_system
     stub_ss.add(files('igvm.c'))
   endif
   stub_ss.add(files('kvm.c'))
-  stub_ss.add(files('target-get-monitor-def.c'))
   stub_ss.add(files('win32-kbd-hook.c'))
   stub_ss.add(files('xen-hw-stub.c'))
   stub_ss.add(files('monitor-arm-gic.c'))
-- 
2.53.0

Re: [PATCH-for-11.1 01/10] monitor: Extract completion declarations to 'monitor/hmp-completion.h'

[Pierrick Bouvier]

On 3/20/26 9:08 AM, Philippe Mathieu-Daudé wrote:
> Many files include "monitor/hmp.h", but few of them really need
> the completion declarations: move them to a distinct header.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   MAINTAINERS                      |  1 +
>   include/monitor/hmp-completion.h | 40 ++++++++++++++++++++++++++++++++
>   include/monitor/hmp.h            | 21 -----------------
>   chardev/char-hmp-cmds.c          |  1 +
>   migration/migration-hmp-cmds.c   |  1 +
>   monitor/hmp-target.c             |  1 +
>   net/net-hmp-cmds.c               |  1 +
>   qom/qom-hmp-cmds.c               |  1 +
>   system/qdev-monitor.c            |  1 +
>   system/runstate-hmp-cmds.c       |  1 +
>   trace/trace-hmp-cmds.c           |  1 +
>   ui/ui-hmp-cmds.c                 |  1 +
>   12 files changed, 50 insertions(+), 21 deletions(-)
>   create mode 100644 include/monitor/hmp-completion.h
> 

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH-for-11.1 02/10] monitor: Forward-declare the MonitorDef type

[Pierrick Bouvier]

On 3/20/26 9:08 AM, Philippe Mathieu-Daudé wrote:
> Rather than having core header forced to include "monitor/hmp.h"
> to get the MonitorDef type declaration, forward-declare it in
> "qemu/typedefs.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/monitor/hmp.h   | 4 ++--
>   include/qemu/typedefs.h | 1 +
>   2 files changed, 3 insertions(+), 2 deletions(-)
> 

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH-for-11.1 03/10] cpus: Introduce CPUClass::legacy_monitor_defs hook

[Pierrick Bouvier]

On 3/20/26 9:08 AM, Philippe Mathieu-Daudé wrote:
> Allow targets to register their legacy target_monitor_defs()
> in CPUClass; check it first in get_monitor_def() otherwise
> fall back to previous per-target helper.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/core/cpu.h | 3 +++
>   monitor/hmp.c         | 3 ++-
>   2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 04e1f970caf..072f58bead5 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -148,6 +148,8 @@ struct SysemuCPUOps;
>    * @max_as: Maximum valid index used to refer to the address spaces supported by
>    *          the architecture, i.e., to refer to CPUAddressSpaces in
>    *          CPUState::cpu_ases.
> + * @legacy_monitor_defs: Array of MonitorDef entries. This field is legacy,
> + *                       use @gdb_core_xml_file to dump registers instead.
>    *
>    * Represents a CPU family or model.
>    */
> @@ -174,6 +176,7 @@ struct CPUClass {
>       const char *gdb_core_xml_file;
>       const char * (*gdb_arch_name)(CPUState *cpu);
>       const char * (*gdb_get_core_xml_file)(CPUState *cpu);
> +    const MonitorDef *legacy_monitor_defs;
>   
>       void (*disas_set_info)(const CPUState *cpu, disassemble_info *info);
>   
> diff --git a/monitor/hmp.c b/monitor/hmp.c
> index c63da13e310..a2b6269d0ff 100644
> --- a/monitor/hmp.c
> +++ b/monitor/hmp.c
> @@ -24,6 +24,7 @@
>   
>   #include "qemu/osdep.h"
>   #include <dirent.h>
> +#include "hw/core/cpu.h"
>   #include "hw/core/qdev.h"
>   #include "monitor-internal.h"
>   #include "monitor/hmp.h"
> @@ -1603,8 +1604,8 @@ void monitor_register_hmp_info_hrt(const char *name,
>    */
>   int get_monitor_def(Monitor *mon, int64_t *pval, const char *name)
>   {
> -    const MonitorDef *md = target_monitor_defs();
>       CPUState *cs = mon_get_cpu(mon);
> +    const MonitorDef *md = cs->cc->legacy_monitor_defs ?: target_monitor_defs();
>       void *ptr;
>       uint64_t tmp = 0;
>       int ret;

I'm not very fond of the "keep legacy callback for the N targets that 
might require it". Either set it for every target, using a default 
implementation if needed, or...

As an alternative, we could use a dispatch function like:

const MonitorDef *target_monitor_defs()
{
   switch (target_arch()) {
   case
     SYS_EMU_TARGET_X:
       return X_target_monitor_defs();
     SYS_EMU_TARGET_Y:
       return Y_target_monitor_defs();
   default:
     return default_target_monitor_defs();
   }
}

Personally, I prefer the dispatch approach, as it's very clear to see 
what happens per target. This solves the symbol duplication by manually 
name them differently.

It's just a suggestion though, and I'm ok to have a per target field, as 
long as we enforce it to be set for all targets, and not using a 
'legacy' callback for a few of them only.

Pick the one you prefer :)

Regards,
Pierrick

Re: [PATCH-for-11.1 03/10] cpus: Introduce CPUClass::legacy_monitor_defs hook

[Philippe Mathieu-Daudé]

On 20/3/26 23:01, Pierrick Bouvier wrote:
> On 3/20/26 9:08 AM, Philippe Mathieu-Daudé wrote:
>> Allow targets to register their legacy target_monitor_defs()
>> in CPUClass; check it first in get_monitor_def() otherwise
>> fall back to previous per-target helper.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   include/hw/core/cpu.h | 3 +++
>>   monitor/hmp.c         | 3 ++-
>>   2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
>> index 04e1f970caf..072f58bead5 100644
>> --- a/include/hw/core/cpu.h
>> +++ b/include/hw/core/cpu.h
>> @@ -148,6 +148,8 @@ struct SysemuCPUOps;
>>    * @max_as: Maximum valid index used to refer to the address spaces 
>> supported by
>>    *          the architecture, i.e., to refer to CPUAddressSpaces in
>>    *          CPUState::cpu_ases.
>> + * @legacy_monitor_defs: Array of MonitorDef entries. This field is 
>> legacy,
>> + *                       use @gdb_core_xml_file to dump registers 
>> instead.
>>    *
>>    * Represents a CPU family or model.
>>    */
>> @@ -174,6 +176,7 @@ struct CPUClass {
>>       const char *gdb_core_xml_file;
>>       const char * (*gdb_arch_name)(CPUState *cpu);
>>       const char * (*gdb_get_core_xml_file)(CPUState *cpu);
>> +    const MonitorDef *legacy_monitor_defs;
>>       void (*disas_set_info)(const CPUState *cpu, disassemble_info 
>> *info);
>> diff --git a/monitor/hmp.c b/monitor/hmp.c
>> index c63da13e310..a2b6269d0ff 100644
>> --- a/monitor/hmp.c
>> +++ b/monitor/hmp.c
>> @@ -24,6 +24,7 @@
>>   #include "qemu/osdep.h"
>>   #include <dirent.h>
>> +#include "hw/core/cpu.h"
>>   #include "hw/core/qdev.h"
>>   #include "monitor-internal.h"
>>   #include "monitor/hmp.h"
>> @@ -1603,8 +1604,8 @@ void monitor_register_hmp_info_hrt(const char 
>> *name,
>>    */
>>   int get_monitor_def(Monitor *mon, int64_t *pval, const char *name)
>>   {
>> -    const MonitorDef *md = target_monitor_defs();
>>       CPUState *cs = mon_get_cpu(mon);
>> +    const MonitorDef *md = cs->cc->legacy_monitor_defs ?: 
>> target_monitor_defs();
>>       void *ptr;
>>       uint64_t tmp = 0;
>>       int ret;
> 
> I'm not very fond of the "keep legacy callback for the N targets that 
> might require it". Either set it for every target, using a default 
> implementation if needed, or...
> 
> As an alternative, we could use a dispatch function like:
> 
> const MonitorDef *target_monitor_defs()
> {
>    switch (target_arch()) {
>    case
>      SYS_EMU_TARGET_X:
>        return X_target_monitor_defs();
>      SYS_EMU_TARGET_Y:
>        return Y_target_monitor_defs();
>    default:
>      return default_target_monitor_defs();
>    }
> }
> 
> Personally, I prefer the dispatch approach, as it's very clear to see 
> what happens per target. This solves the symbol duplication by manually 
> name them differently.
> 
> It's just a suggestion though, and I'm ok to have a per target field, as 
> long as we enforce it to be set for all targets, and not using a 
> 'legacy' callback for a few of them only.
> 
> Pick the one you prefer :)

My preference would be to remove this legacy code altogether, as
I'm pretty sure it isn't used anymore (see for the example the
issues RISC-V folks had with it before giving up).

Re: [PATCH-for-11.1 03/10] cpus: Introduce CPUClass::legacy_monitor_defs hook

[Pierrick Bouvier]

On 3/21/26 8:19 AM, Philippe Mathieu-Daudé wrote:
> On 20/3/26 23:01, Pierrick Bouvier wrote:
>> On 3/20/26 9:08 AM, Philippe Mathieu-Daudé wrote:
>>> Allow targets to register their legacy target_monitor_defs()
>>> in CPUClass; check it first in get_monitor_def() otherwise
>>> fall back to previous per-target helper.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>    include/hw/core/cpu.h | 3 +++
>>>    monitor/hmp.c         | 3 ++-
>>>    2 files changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
>>> index 04e1f970caf..072f58bead5 100644
>>> --- a/include/hw/core/cpu.h
>>> +++ b/include/hw/core/cpu.h
>>> @@ -148,6 +148,8 @@ struct SysemuCPUOps;
>>>     * @max_as: Maximum valid index used to refer to the address spaces
>>> supported by
>>>     *          the architecture, i.e., to refer to CPUAddressSpaces in
>>>     *          CPUState::cpu_ases.
>>> + * @legacy_monitor_defs: Array of MonitorDef entries. This field is
>>> legacy,
>>> + *                       use @gdb_core_xml_file to dump registers
>>> instead.
>>>     *
>>>     * Represents a CPU family or model.
>>>     */
>>> @@ -174,6 +176,7 @@ struct CPUClass {
>>>        const char *gdb_core_xml_file;
>>>        const char * (*gdb_arch_name)(CPUState *cpu);
>>>        const char * (*gdb_get_core_xml_file)(CPUState *cpu);
>>> +    const MonitorDef *legacy_monitor_defs;
>>>        void (*disas_set_info)(const CPUState *cpu, disassemble_info
>>> *info);
>>> diff --git a/monitor/hmp.c b/monitor/hmp.c
>>> index c63da13e310..a2b6269d0ff 100644
>>> --- a/monitor/hmp.c
>>> +++ b/monitor/hmp.c
>>> @@ -24,6 +24,7 @@
>>>    #include "qemu/osdep.h"
>>>    #include <dirent.h>
>>> +#include "hw/core/cpu.h"
>>>    #include "hw/core/qdev.h"
>>>    #include "monitor-internal.h"
>>>    #include "monitor/hmp.h"
>>> @@ -1603,8 +1604,8 @@ void monitor_register_hmp_info_hrt(const char
>>> *name,
>>>     */
>>>    int get_monitor_def(Monitor *mon, int64_t *pval, const char *name)
>>>    {
>>> -    const MonitorDef *md = target_monitor_defs();
>>>        CPUState *cs = mon_get_cpu(mon);
>>> +    const MonitorDef *md = cs->cc->legacy_monitor_defs ?:
>>> target_monitor_defs();
>>>        void *ptr;
>>>        uint64_t tmp = 0;
>>>        int ret;
>>
>> I'm not very fond of the "keep legacy callback for the N targets that
>> might require it". Either set it for every target, using a default
>> implementation if needed, or...
>>
>> As an alternative, we could use a dispatch function like:
>>
>> const MonitorDef *target_monitor_defs()
>> {
>>     switch (target_arch()) {
>>     case
>>       SYS_EMU_TARGET_X:
>>         return X_target_monitor_defs();
>>       SYS_EMU_TARGET_Y:
>>         return Y_target_monitor_defs();
>>     default:
>>       return default_target_monitor_defs();
>>     }
>> }
>>
>> Personally, I prefer the dispatch approach, as it's very clear to see
>> what happens per target. This solves the symbol duplication by manually
>> name them differently.
>>
>> It's just a suggestion though, and I'm ok to have a per target field, as
>> long as we enforce it to be set for all targets, and not using a
>> 'legacy' callback for a few of them only.
>>
>> Pick the one you prefer :)
> 
> My preference would be to remove this legacy code altogether, as
> I'm pretty sure it isn't used anymore (see for the example the
> issues RISC-V folks had with it before giving up).

I'm not sure what you mean.

If it's not used, then we simply remove it.
If it's used, then let's keep it, but just make it consistent with all 
targets without having a special case named "legacy". If it's here to 
stay, it's not legacy.

Do you have something else in mind?

Re: [PATCH-for-11.1 03/10] cpus: Introduce CPUClass::legacy_monitor_defs hook

[Philippe Mathieu-Daudé]

On 21/3/26 20:51, Pierrick Bouvier wrote:
> On 3/21/26 8:19 AM, Philippe Mathieu-Daudé wrote:
>> On 20/3/26 23:01, Pierrick Bouvier wrote:
>>> On 3/20/26 9:08 AM, Philippe Mathieu-Daudé wrote:
>>>> Allow targets to register their legacy target_monitor_defs()
>>>> in CPUClass; check it first in get_monitor_def() otherwise
>>>> fall back to previous per-target helper.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> ---
>>>>    include/hw/core/cpu.h | 3 +++
>>>>    monitor/hmp.c         | 3 ++-
>>>>    2 files changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
>>>> index 04e1f970caf..072f58bead5 100644
>>>> --- a/include/hw/core/cpu.h
>>>> +++ b/include/hw/core/cpu.h
>>>> @@ -148,6 +148,8 @@ struct SysemuCPUOps;
>>>>     * @max_as: Maximum valid index used to refer to the address spaces
>>>> supported by
>>>>     *          the architecture, i.e., to refer to CPUAddressSpaces in
>>>>     *          CPUState::cpu_ases.
>>>> + * @legacy_monitor_defs: Array of MonitorDef entries. This field is
>>>> legacy,
>>>> + *                       use @gdb_core_xml_file to dump registers
>>>> instead.
>>>>     *
>>>>     * Represents a CPU family or model.
>>>>     */
>>>> @@ -174,6 +176,7 @@ struct CPUClass {
>>>>        const char *gdb_core_xml_file;
>>>>        const char * (*gdb_arch_name)(CPUState *cpu);
>>>>        const char * (*gdb_get_core_xml_file)(CPUState *cpu);
>>>> +    const MonitorDef *legacy_monitor_defs;
>>>>        void (*disas_set_info)(const CPUState *cpu, disassemble_info
>>>> *info);
>>>> diff --git a/monitor/hmp.c b/monitor/hmp.c
>>>> index c63da13e310..a2b6269d0ff 100644
>>>> --- a/monitor/hmp.c
>>>> +++ b/monitor/hmp.c
>>>> @@ -24,6 +24,7 @@
>>>>    #include "qemu/osdep.h"
>>>>    #include <dirent.h>
>>>> +#include "hw/core/cpu.h"
>>>>    #include "hw/core/qdev.h"
>>>>    #include "monitor-internal.h"
>>>>    #include "monitor/hmp.h"
>>>> @@ -1603,8 +1604,8 @@ void monitor_register_hmp_info_hrt(const char
>>>> *name,
>>>>     */
>>>>    int get_monitor_def(Monitor *mon, int64_t *pval, const char *name)
>>>>    {
>>>> -    const MonitorDef *md = target_monitor_defs();
>>>>        CPUState *cs = mon_get_cpu(mon);
>>>> +    const MonitorDef *md = cs->cc->legacy_monitor_defs ?:
>>>> target_monitor_defs();
>>>>        void *ptr;
>>>>        uint64_t tmp = 0;
>>>>        int ret;
>>>
>>> I'm not very fond of the "keep legacy callback for the N targets that
>>> might require it". Either set it for every target, using a default
>>> implementation if needed, or...
>>>
>>> As an alternative, we could use a dispatch function like:
>>>
>>> const MonitorDef *target_monitor_defs()
>>> {
>>>     switch (target_arch()) {
>>>     case
>>>       SYS_EMU_TARGET_X:
>>>         return X_target_monitor_defs();
>>>       SYS_EMU_TARGET_Y:
>>>         return Y_target_monitor_defs();
>>>     default:
>>>       return default_target_monitor_defs();
>>>     }
>>> }
>>>
>>> Personally, I prefer the dispatch approach, as it's very clear to see
>>> what happens per target. This solves the symbol duplication by manually
>>> name them differently.
>>>
>>> It's just a suggestion though, and I'm ok to have a per target field, as
>>> long as we enforce it to be set for all targets, and not using a
>>> 'legacy' callback for a few of them only.
>>>
>>> Pick the one you prefer :)
>>
>> My preference would be to remove this legacy code altogether, as
>> I'm pretty sure it isn't used anymore (see for the example the
>> issues RISC-V folks had with it before giving up).
> 
> I'm not sure what you mean.
> 
> If it's not used, then we simply remove it.
> If it's used, then let's keep it, but just make it consistent with all 
> targets without having a special case named "legacy". If it's here to 
> stay, it's not legacy.
> 
> Do you have something else in mind?

As a software engineer I prefer to deal with code. Unfortunately in my
experience with QEMU development, removing legacy code means endless
bikesheeding debates. I'm not judging they are bad, just they drain my
time and energy. I agree legacy code should be removed, but again this
is the QEMU project here, where we have bad habbits of adding new APIs
with the promise of removing the previous one but never finish API
conversions, not even marking legacy one. With that in mind, I think
mentioning "legacy" is still a good first step.

Anyway, I suppose what you are asking is me starting a poll for each
target array and ask "Who is still using this SPARC 'canrestore' monitor
command?" etc., wait 2 months for replies, then if possible deprecate
them, wait 2 more releases, and eventually remove. Then we'll be able
to link this file in single binary. As of today I'm not motivated enough
for that so I'm looking for alternatives. Looking at mailing list
archives I apparently started to tackle this 9 weeks ago, long enough to
keep working on the single binary with HMP code disable until we get
a working proof of concept; then revisit.

Re: [PATCH-for-11.1 00/10] monitor: Remove need of per-target handlers

[Daniel Henrique Barboza]

On 3/20/2026 1:08 PM, Philippe Mathieu-Daudé wrote:
> Convert target_monitor_defs() and target_monitor_defs()
> to CPUClass fields, adapting the 4 targets using these
> legacy code.

So I saw in patch 3 this explanation:


+ * @legacy_monitor_defs: Array of MonitorDef entries. This field is legacy,
+ *                       use @gdb_core_xml_file to dump registers instead.


I can move target/riscv to gdb_core_xml_file and get out of your way in
deprecating this API so you have one less target to worry about.  I'll
see what other targets are doing w.r.t gdb_core_xml_file.

In fact, depending on the amount of pain involved, we should just convert
all targets and remove the legacy API completely.  Or maybe I'm not fully
appreciating the scope of what you're doing here: will we impact users if
we just convert all users to gdb_core_xml_file?  Do we have HMP/QMP commands
that will be impacted?  Because if not we should consider just convert everything
and be done with it - there's not 'turn legacy for a couple of releases
before removing it' for internals APIs.


Thanks,
Daniel

> 
> Philippe Mathieu-Daudé (10):
>    monitor: Extract completion declarations to 'monitor/hmp-completion.h'
>    monitor: Forward-declare the MonitorDef type
>    cpus: Introduce CPUClass::legacy_monitor_defs hook
>    target/i386: Replace legacy target_monitor_defs -> legacy_monitor_defs
>    target/m68k: Replace legacy target_monitor_defs -> legacy_monitor_defs
>    target/sparc: Replace legacy target_monitor_defs ->
>      legacy_monitor_defs
>    monitor: Remove target_monitor_defs()
>    cpus: Introduce CPUClass::legacy_monitor_get_register() hook
>    target/riscv: Register target_get_monitor_def in CPUClass
>    monitor: Remove target_get_monitor_def()
> 
>   MAINTAINERS                      |  1 +
>   include/hw/core/cpu.h            |  8 +++++++
>   include/monitor/hmp-completion.h | 40 ++++++++++++++++++++++++++++++++
>   include/monitor/hmp.h            | 28 ++--------------------
>   include/qemu/typedefs.h          |  1 +
>   target/i386/cpu.h                |  2 ++
>   target/m68k/cpu.h                |  2 ++
>   target/riscv/internals.h         |  3 +++
>   target/sparc/cpu.h               |  2 ++
>   chardev/char-hmp-cmds.c          |  1 +
>   migration/migration-hmp-cmds.c   |  1 +
>   monitor/hmp-target.c             |  1 +
>   monitor/hmp.c                    |  8 +++++--
>   net/net-hmp-cmds.c               |  1 +
>   qom/qom-hmp-cmds.c               |  1 +
>   stubs/target-get-monitor-def.c   | 29 -----------------------
>   stubs/target-monitor-defs.c      |  7 ------
>   system/qdev-monitor.c            |  1 +
>   system/runstate-hmp-cmds.c       |  1 +
>   target/i386/cpu.c                |  1 +
>   target/i386/monitor.c            |  7 ++----
>   target/m68k/cpu.c                |  1 +
>   target/m68k/monitor.c            |  5 +---
>   target/riscv/cpu.c               |  1 +
>   target/riscv/monitor.c           |  4 +++-
>   target/sparc/cpu.c               |  3 +++
>   target/sparc/monitor.c           |  9 +++----
>   trace/trace-hmp-cmds.c           |  1 +
>   ui/ui-hmp-cmds.c                 |  1 +
>   stubs/meson.build                |  2 --
>   30 files changed, 91 insertions(+), 82 deletions(-)
>   create mode 100644 include/monitor/hmp-completion.h
>   delete mode 100644 stubs/target-get-monitor-def.c
>   delete mode 100644 stubs/target-monitor-defs.c
> 

Re: [PATCH-for-11.1 03/10] cpus: Introduce CPUClass::legacy_monitor_defs hook

[Pierrick Bouvier]

On 3/23/26 2:19 AM, Philippe Mathieu-Daudé wrote:
> On 21/3/26 20:51, Pierrick Bouvier wrote:
>> On 3/21/26 8:19 AM, Philippe Mathieu-Daudé wrote:
>>> On 20/3/26 23:01, Pierrick Bouvier wrote:
>>>> On 3/20/26 9:08 AM, Philippe Mathieu-Daudé wrote:
>>>>> Allow targets to register their legacy target_monitor_defs()
>>>>> in CPUClass; check it first in get_monitor_def() otherwise
>>>>> fall back to previous per-target helper.
>>>>>
>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>> ---
>>>>>     include/hw/core/cpu.h | 3 +++
>>>>>     monitor/hmp.c         | 3 ++-
>>>>>     2 files changed, 5 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
>>>>> index 04e1f970caf..072f58bead5 100644
>>>>> --- a/include/hw/core/cpu.h
>>>>> +++ b/include/hw/core/cpu.h
>>>>> @@ -148,6 +148,8 @@ struct SysemuCPUOps;
>>>>>      * @max_as: Maximum valid index used to refer to the address spaces
>>>>> supported by
>>>>>      *          the architecture, i.e., to refer to CPUAddressSpaces in
>>>>>      *          CPUState::cpu_ases.
>>>>> + * @legacy_monitor_defs: Array of MonitorDef entries. This field is
>>>>> legacy,
>>>>> + *                       use @gdb_core_xml_file to dump registers
>>>>> instead.
>>>>>      *
>>>>>      * Represents a CPU family or model.
>>>>>      */
>>>>> @@ -174,6 +176,7 @@ struct CPUClass {
>>>>>         const char *gdb_core_xml_file;
>>>>>         const char * (*gdb_arch_name)(CPUState *cpu);
>>>>>         const char * (*gdb_get_core_xml_file)(CPUState *cpu);
>>>>> +    const MonitorDef *legacy_monitor_defs;
>>>>>         void (*disas_set_info)(const CPUState *cpu, disassemble_info
>>>>> *info);
>>>>> diff --git a/monitor/hmp.c b/monitor/hmp.c
>>>>> index c63da13e310..a2b6269d0ff 100644
>>>>> --- a/monitor/hmp.c
>>>>> +++ b/monitor/hmp.c
>>>>> @@ -24,6 +24,7 @@
>>>>>     #include "qemu/osdep.h"
>>>>>     #include <dirent.h>
>>>>> +#include "hw/core/cpu.h"
>>>>>     #include "hw/core/qdev.h"
>>>>>     #include "monitor-internal.h"
>>>>>     #include "monitor/hmp.h"
>>>>> @@ -1603,8 +1604,8 @@ void monitor_register_hmp_info_hrt(const char
>>>>> *name,
>>>>>      */
>>>>>     int get_monitor_def(Monitor *mon, int64_t *pval, const char *name)
>>>>>     {
>>>>> -    const MonitorDef *md = target_monitor_defs();
>>>>>         CPUState *cs = mon_get_cpu(mon);
>>>>> +    const MonitorDef *md = cs->cc->legacy_monitor_defs ?:
>>>>> target_monitor_defs();
>>>>>         void *ptr;
>>>>>         uint64_t tmp = 0;
>>>>>         int ret;
>>>>
>>>> I'm not very fond of the "keep legacy callback for the N targets that
>>>> might require it". Either set it for every target, using a default
>>>> implementation if needed, or...
>>>>
>>>> As an alternative, we could use a dispatch function like:
>>>>
>>>> const MonitorDef *target_monitor_defs()
>>>> {
>>>>      switch (target_arch()) {
>>>>      case
>>>>        SYS_EMU_TARGET_X:
>>>>          return X_target_monitor_defs();
>>>>        SYS_EMU_TARGET_Y:
>>>>          return Y_target_monitor_defs();
>>>>      default:
>>>>        return default_target_monitor_defs();
>>>>      }
>>>> }
>>>>
>>>> Personally, I prefer the dispatch approach, as it's very clear to see
>>>> what happens per target. This solves the symbol duplication by manually
>>>> name them differently.
>>>>
>>>> It's just a suggestion though, and I'm ok to have a per target field, as
>>>> long as we enforce it to be set for all targets, and not using a
>>>> 'legacy' callback for a few of them only.
>>>>
>>>> Pick the one you prefer :)
>>>
>>> My preference would be to remove this legacy code altogether, as
>>> I'm pretty sure it isn't used anymore (see for the example the
>>> issues RISC-V folks had with it before giving up).
>>
>> I'm not sure what you mean.
>>
>> If it's not used, then we simply remove it.
>> If it's used, then let's keep it, but just make it consistent with all
>> targets without having a special case named "legacy". If it's here to
>> stay, it's not legacy.
>>
>> Do you have something else in mind?
> 
> As a software engineer I prefer to deal with code. Unfortunately in my
> experience with QEMU development, removing legacy code means endless
> bikesheeding debates. I'm not judging they are bad, just they drain my
> time and energy. I agree legacy code should be removed, but again this
> is the QEMU project here, where we have bad habbits of adding new APIs
> with the promise of removing the previous one but never finish API
> conversions, not even marking legacy one. With that in mind, I think
> mentioning "legacy" is still a good first step.
>

I agree, and it's *not* your responsibility to refactor any piece of 
code you have to touch for another purpose. In this case, I was just 
suggesting to make the callback consistent between all targets (i.e. 
force all them to implement it) instead of having a conditional legacy 
callback.

All the rest of the series (symbol renaming, etc) stays intact.

> Anyway, I suppose what you are asking is me starting a poll for each
> target array and ask "Who is still using this SPARC 'canrestore' monitor
> command?" etc., wait 2 months for replies, then if possible deprecate
> them, wait 2 more releases, and eventually remove. Then we'll be able
> to link this file in single binary. As of today I'm not motivated enough
> for that so I'm looking for alternatives. Looking at mailing list
> archives I apparently started to tackle this 9 weeks ago, long enough to
> keep working on the single binary with HMP code disable until we get
> a working proof of concept; then revisit.

I was just curious about your mention "My preference would be to remove 
this legacy code altogether", but no one expects you to do it.
I think having consistent callbacks through all targets is a very 
acceptable solution. The only difference with your series is to rename 
"legacy*" and set a default callback for all other targets.

If it's creating too much friction, I can also hear it and accept your 
current series as it is.

Regards,
Pierrick