Philippe Mathieu-Daudé <f4...@amsat.org> writes: > On 7/2/22 20:34, Peter Maydell wrote: >> On Thu, 27 Jan 2022 at 21:03, Mark Cave-Ayland >> <mark.cave-ayl...@ilande.co.uk> wrote: >>> >>> This displays detailed information about the device registers and timers to >>> aid >>> debugging problems with timers and interrupts. >>> >>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk> >>> --- >>> hmp-commands-info.hx | 12 ++++++ >>> hw/misc/mos6522.c | 92 ++++++++++++++++++++++++++++++++++++++++++++ >>> 2 files changed, 104 insertions(+) >> >> I'm not sure how keen we are on adding new device-specific >> HMP info commands, but it's not my area of expertise. Markus ? > > HMP is David :)
Yes. > IIRC it is OK as long as HMP is a QMP wrapper. That's "how to do it", and I'll get back to it in a jiffie, but Peter was wondering about the "whether to do it". Most HMP commands are always there. We have a few specific to compile-time configurable features: TCG, VNC, Spice, Slirp, Linux. Does not apply here. We have a few specific to targets, such as dump-skeys and info skeys for s390. Target-specific is not quite the same as device-specific. We have no device-specific commands so far. However, dump-skeys and info skeys appear to be about the skeys *device*, not the s390 target. Perhaps any s390 target has such a device? I don't know. My point is we already have device-specific commands, they're just masquerading as target-specific commands. The proposed device-specific command uses a mechanism originally made for modules instead (more on that below). I think we should make up our minds which way we want device-specific commands done, then do *all* of them that way. On to "how to do it", part 1. Most of the time, the command handler is declared with the command in hmp-commands{,-info}.hx, possibly with compile-time conditionals. But it can also be left null there, and set with monitor_register_hmp() or monitor_register_hmp_info_hrt(). This is intended for modules; see commit f0e48cbd791^..bca6eb34f03. Aside: can modules be unloaded? If yes, we better zap the handler then. The proposed "info via" uses monitor_register_hmp_info_hrt(). No objection from me, requires David's ACK. "How to do it", part 2, in reply to Philippe's remark. Ideally, HMP commands wrap around QMP commands, but we accept exceptions for certain use cases where the wrapping is more trouble than it's worth, with justification. I've explained this several times, and I'm happy to dig up a reference or explain it again if there's a need. Justifying an exception is bothersome, too. Daniel Berrangé recently created a way to reduce the wrapping trouble (merge commit e86e00a2493). The proposed "info via" makes use of it. >> (patch below for context) >> thanks >> -- PMM >> >>> >>> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx >>> index e90f20a107..4e714e79a2 100644 >>> --- a/hmp-commands-info.hx >>> +++ b/hmp-commands-info.hx >>> @@ -879,3 +879,15 @@ SRST >>> ``info sgx`` >>> Show intel SGX information. >>> ERST >>> + >>> + { >>> + .name = "via", >>> + .args_type = "", >>> + .params = "", >>> + .help = "show guest 6522 VIA devices", >>> + }, >>> + >>> +SRST >>> + ``info via`` >>> + Show guest 6522 VIA devices. >>> +ERST Should this be conditional on the targets where we actually link the device, like info skeys? [...]