diff mbox series

[1/2] hw/misc/mos6522: Expose x-query-mos6522-devices QMP command

Message ID 20240610150758.2827-2-philmd@linaro.org
State New
Headers show
Series hw/misc/mos6522: Do not open-code hmp_info_human_readable_text() | expand

Commit Message

Philippe Mathieu-Daudé June 10, 2024, 3:07 p.m. UTC 
This is a counterpart to the HMP "info via" command. It is being
added with an "x-" prefix because this QMP command is intended as an
adhoc debugging tool and will thus not be modelled in QAPI as fully
structured data, nor will it have long term guaranteed stability.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 MAINTAINERS             |  2 +-
 qapi/machine.json       | 17 +++++++++++++++++
 hw/misc/mos6522-stubs.c | 18 ++++++++++++++++++
 hw/misc/mos6522.c       |  5 +++--
 hw/misc/meson.build     |  3 ++-
 5 files changed, 41 insertions(+), 4 deletions(-)
 create mode 100644 hw/misc/mos6522-stubs.c

Comments

Daniel P. Berrangé June 10, 2024, 5:08 p.m. UTC | #1 
On Mon, Jun 10, 2024 at 05:07:57PM +0200, Philippe Mathieu-Daudé wrote:
> This is a counterpart to the HMP "info via" command. It is being
> added with an "x-" prefix because this QMP command is intended as an
> adhoc debugging tool and will thus not be modelled in QAPI as fully
> structured data, nor will it have long term guaranteed stability.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  MAINTAINERS             |  2 +-
>  qapi/machine.json       | 17 +++++++++++++++++
>  hw/misc/mos6522-stubs.c | 18 ++++++++++++++++++
>  hw/misc/mos6522.c       |  5 +++--
>  hw/misc/meson.build     |  3 ++-
>  5 files changed, 41 insertions(+), 4 deletions(-)
>  create mode 100644 hw/misc/mos6522-stubs.c

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

With regards,
Daniel
Markus Armbruster June 11, 2024, 5:36 a.m. UTC | #2 
Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> This is a counterpart to the HMP "info via" command. It is being
> added with an "x-" prefix because this QMP command is intended as an
> adhoc debugging tool and will thus not be modelled in QAPI as fully
> structured data, nor will it have long term guaranteed stability.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  MAINTAINERS             |  2 +-
>  qapi/machine.json       | 17 +++++++++++++++++
>  hw/misc/mos6522-stubs.c | 18 ++++++++++++++++++
>  hw/misc/mos6522.c       |  5 +++--
>  hw/misc/meson.build     |  3 ++-
>  5 files changed, 41 insertions(+), 4 deletions(-)
>  create mode 100644 hw/misc/mos6522-stubs.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 951556224a..e86638c68c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1453,7 +1453,7 @@ F: hw/ppc/mac_newworld.c
>  F: hw/pci-host/uninorth.c
>  F: hw/pci-bridge/dec.[hc]
>  F: hw/misc/macio/
> -F: hw/misc/mos6522.c
> +F: hw/misc/mos6522*.c
>  F: hw/nvram/mac_nvram.c
>  F: hw/ppc/fw_cfg.c
>  F: hw/input/adb*
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 1283d14493..a82b8dd39d 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json

I figure you pick machine.json because it already serves as grabbag of
vaguely device-specific queries like x-query-usb.  misc-target.json is
another grabbag.

> @@ -1865,6 +1865,23 @@
>    'data': { 'filename': 'str' },
>    'if': 'CONFIG_FDT' }
>  
> +##
> +# @x-query-mos6522-devices:
> +#
> +# Query information on MOS6522 VIA devices
> +#
> +# Features:
> +#
> +# @unstable: This command is meant for debugging.
> +#
> +# Returns: MOS6522 VIA devices information
> +#
> +# Since: 9.1
> +##
> +{ 'command': 'x-query-mos6522-devices',
> +  'returns': 'HumanReadableText',
> +  'features': [ 'unstable' ]}
> +
>  ##
>  # @x-query-interrupt-controllers:
>  #

HMP "info via" is compile-time conditional on CONFIG_MOS6522.

Its new QMP counterpart x-query-mos6522-devices is unconditional.
Can you explain why?

Possibly related:

commit 409e9f7131e55e74eb09e65535779e311df5ebf5
Author: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Date:   Sat Mar 5 15:09:53 2022 +0000

    mos6522: add "info via" HMP command for debugging
    
    This displays detailed information about the device registers and timers to aid
    debugging problems with timers and interrupts.
    
--> Currently the QAPI generators for HumanReadableText don't work correctly if
--> used in qapi/target-misc.json when a non-specified target is built, so for
--> now manually add a hmp_info_via() wrapper until direct support for per-device
--> HMP/QMP commands is implemented.
    
    Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
    Reviewed-by: Laurent Vivier <laurent@vivier.eu>
    Message-Id: <20220305150957.5053-9-mark.cave-ayland@ilande.co.uk>
    Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

[...]
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 951556224a..e86638c68c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1453,7 +1453,7 @@  F: hw/ppc/mac_newworld.c
 F: hw/pci-host/uninorth.c
 F: hw/pci-bridge/dec.[hc]
 F: hw/misc/macio/
-F: hw/misc/mos6522.c
+F: hw/misc/mos6522*.c
 F: hw/nvram/mac_nvram.c
 F: hw/ppc/fw_cfg.c
 F: hw/input/adb*
diff --git a/qapi/machine.json b/qapi/machine.json
index 1283d14493..a82b8dd39d 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1865,6 +1865,23 @@ 
   'data': { 'filename': 'str' },
   'if': 'CONFIG_FDT' }
 
+##
+# @x-query-mos6522-devices:
+#
+# Query information on MOS6522 VIA devices
+#
+# Features:
+#
+# @unstable: This command is meant for debugging.
+#
+# Returns: MOS6522 VIA devices information
+#
+# Since: 9.1
+##
+{ 'command': 'x-query-mos6522-devices',
+  'returns': 'HumanReadableText',
+  'features': [ 'unstable' ]}
+
 ##
 # @x-query-interrupt-controllers:
 #
diff --git a/hw/misc/mos6522-stubs.c b/hw/misc/mos6522-stubs.c
new file mode 100644
index 0000000000..c953f01a16
--- /dev/null
+++ b/hw/misc/mos6522-stubs.c
@@ -0,0 +1,18 @@ 
+/*
+ * QEMU MOS6522 VIA stubs
+ *
+ * SPDX-FileContributor: Philippe Mathieu-Daudé <philmd@linaro.org>
+ * SPDX-FileCopyrightText: 2024 Linaro Ltd.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-machine.h"
+
+HumanReadableText *qmp_x_query_mos6522_devices(Error **errp)
+{
+    error_setg(errp, "Support for MOS6522 VIA devices not built-in");
+
+    return NULL;
+}
diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
index 515f62e687..b1bb7f54f0 100644
--- a/hw/misc/mos6522.c
+++ b/hw/misc/mos6522.c
@@ -31,6 +31,7 @@ 
 #include "migration/vmstate.h"
 #include "monitor/monitor.h"
 #include "monitor/hmp.h"
+#include "qapi/qapi-commands-machine.h"
 #include "qapi/type-helpers.h"
 #include "qemu/timer.h"
 #include "qemu/cutils.h"
@@ -576,7 +577,7 @@  static int qmp_x_query_via_foreach(Object *obj, void *opaque)
     return 0;
 }
 
-static HumanReadableText *qmp_x_query_via(Error **errp)
+HumanReadableText *qmp_x_query_mos6522_devices(Error **errp)
 {
     g_autoptr(GString) buf = g_string_new("");
 
@@ -589,7 +590,7 @@  static HumanReadableText *qmp_x_query_via(Error **errp)
 void hmp_info_via(Monitor *mon, const QDict *qdict)
 {
     Error *err = NULL;
-    g_autoptr(HumanReadableText) info = qmp_x_query_via(&err);
+    g_autoptr(HumanReadableText) info = qmp_x_query_mos6522_devices(&err);
 
     if (hmp_handle_error(mon, err)) {
         return;
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index 86596a3888..9fa0e98794 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -18,7 +18,8 @@  system_ss.add(when: 'CONFIG_ARM11SCU', if_true: files('arm11scu.c'))
 system_ss.add(when: 'CONFIG_ARM_V7M', if_true: files('armv7m_ras.c'))
 
 # Mac devices
-system_ss.add(when: 'CONFIG_MOS6522', if_true: files('mos6522.c'))
+system_ss.add(when: 'CONFIG_MOS6522', if_true: files('mos6522.c'),
+                                      if_false: files('mos6522-stubs.c'))
 system_ss.add(when: 'CONFIG_DJMEMC', if_true: files('djmemc.c'))
 system_ss.add(when: 'CONFIG_IOSB', if_true: files('iosb.c'))