Re: [PATCH 08/11] mos6522: add "info via" HMP command for debugging

[Dr. David Alan Gilbert]

* Mark Cave-Ayland (mark.cave-ayland@ilande.co.uk) wrote:
> On 08/02/2022 10:29, Dr. David Alan Gilbert wrote:
> 
> > * Markus Armbruster (armbru@redhat.com) wrote:
> > > Philippe Mathieu-Daudé <f4bug@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-ayland@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-ayland@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.
> > 
> > So let me start with an:
> > 
> > Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > (If it's useful info for the author of the device, then I'm happy for
> > HMP to have that), but then - (moving the reply around a bit):
> > 
> > 
> > > Should this be conditional on the targets where we actually link the
> > > device, like info skeys?
> > > 
> > 
> > Yes, I think so; it's a reasonably old/obscure device, there's no reason
> > everyone having it built in.
> 
> Unfortunately that doesn't seem to work: whilst the target CONFIG_* defines
> are declared when processing hmp-commands-info.hx it seems the the device
> CONFIG_* defines are missing?

I'd be happy to take it down to the target level.

Dave

> > > >                  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.
> > 
> > Yeh we've got info lapic/ioapic as well.
> 
> ... which I suspect is a workaround for only the target CONFIG_* defines 
> being declared.
> 
> > > 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.
> > 
> > I think device specific commands make sense, but I think it would
> > probably be better if we had an 'info dev $name' and that a method on
> > the device rather than registering each one separately.
> > I'd assume that this would be a QMP level thing that got unwrapped at
> > HMP.
> > 
> > But that's not a problem for this contribution; someone else can figure
> > that out later.
> 
> 
> ATB,
> 
> Mark.
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK