| Message ID | 20210908103711.683940-5-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Stop adding HMP-only commands, allow QMP for all | expand |
On Wed, Sep 08, 2021 at 11:37:10AM +0100, Daniel P. Berrangé wrote: > This is a counterpart to the HMP "info registers" 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 ad hoc > structured data, nor will it have long term guaranteed stability. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > hw/core/machine-qmp-cmds.c | 28 ++++++++++++++++++++++++++++ > qapi/machine.json | 37 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 65 insertions(+) > > +++ b/qapi/machine.json > @@ -1312,3 +1312,40 @@ > '*cores': 'int', > '*threads': 'int', > '*maxcpus': 'int' } } > + > +## > +# @RegisterParams: > +# > +# Information about the CPU to query state of > +# > +# @cpu: the CPU number to query. If omitted, queries all CPUs > +# > +# Since: 6.2.0 Consensus seems to be "6.2", not "6.2.0".
Daniel P. Berrangé <berrange@redhat.com> writes: > This is a counterpart to the HMP "info registers" 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: Daniel P. Berrangé <berrange@redhat.com> > --- > hw/core/machine-qmp-cmds.c | 28 ++++++++++++++++++++++++++++ > qapi/machine.json | 37 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 65 insertions(+) > > diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c > index 216fdfaf3a..0d9943ff60 100644 > --- a/hw/core/machine-qmp-cmds.c > +++ b/hw/core/machine-qmp-cmds.c > @@ -204,3 +204,31 @@ MemdevList *qmp_query_memdev(Error **errp) > object_child_foreach(obj, query_memdev, &list); > return list; > } > + > +RegisterInfo *qmp_x_query_registers(bool has_cpu, int64_t cpu, Error **errp) > +{ > + RegisterInfo *info = g_new0(RegisterInfo, 1); > + g_autoptr(GString) buf = g_string_new(""); > + CPUState *cs = NULL, *tmp; > + > + if (has_cpu) { > + CPU_FOREACH(tmp) { > + if (cpu == tmp->cpu_index) { > + cs = tmp; > + } > + } > + if (!cs) { > + error_setg(errp, "CPU %"PRId64" not available", cpu); > + return NULL; > + } > + cpu_format_state(cs, buf, CPU_DUMP_FPU); > + } else { > + CPU_FOREACH(cs) { > + g_string_append_printf(buf, "\nCPU#%d\n", cs->cpu_index); > + cpu_format_state(cs, buf, CPU_DUMP_FPU); > + } > + } > + > + info->state = g_steal_pointer(&buf->str); > + return info; > +} > diff --git a/qapi/machine.json b/qapi/machine.json > index 157712f006..27b922f2ce 100644 > --- a/qapi/machine.json > +++ b/qapi/machine.json > @@ -1312,3 +1312,40 @@ > '*cores': 'int', > '*threads': 'int', > '*maxcpus': 'int' } } > + > +## > +# @RegisterParams: > +# > +# Information about the CPU to query state of > +# > +# @cpu: the CPU number to query. If omitted, queries all CPUs > +# > +# Since: 6.2.0 > +# > +## > +{ 'struct': 'RegisterParams', 'data': {'*cpu': 'int' } } > + > +## > +# @RegisterInfo: > +# > +# Information about the CPU state > +# > +# @state: the CPU state in an architecture specific format > +# > +# Since: 6.2.0 > +# > +## > +{ 'struct': 'RegisterInfo', 'data': {'state': 'str' } } > + > +## > +# @x-query-registers: > +# > +# Return information on the CPU registers > +# > +# Returns: the CPU state > +# > +# Since: 6.2.0 > +## > +{ 'command': 'x-query-registers', > + 'data': 'RegisterParams', Unless you have further uses of RegisterParams in mind, use an implicit type: 'data': { '*cpu': 'int' } } > + 'returns': 'RegisterInfo' } I'd like us to adopt a convention for commands returning formatted text for human consumption. Like so: 'returns': 'HumanReadableText' } with the obvious ## # @HumanReadableText: # # @human-readable-text: Formatted output intended for humans. # # Since: 6.2.0 ## { 'struct': 'HumanReadableText', 'data': { 'human-readable-text': 'str' } } When the output needs explaining, do that in the command's doc string. I figure ## # @x-query-registers: # # Returns information about the CPU state # # Returns: CPU state in an architecture-specific format # # Since: 6.2.0 ## would do in this case.
On Thu, Sep 09, 2021 at 11:05:20AM +0200, Markus Armbruster wrote: > Daniel P. Berrangé <berrange@redhat.com> writes: > > > This is a counterpart to the HMP "info registers" 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: Daniel P. Berrangé <berrange@redhat.com> > > --- > > hw/core/machine-qmp-cmds.c | 28 ++++++++++++++++++++++++++++ > > qapi/machine.json | 37 +++++++++++++++++++++++++++++++++++++ > > 2 files changed, 65 insertions(+) > > > > diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c > > index 216fdfaf3a..0d9943ff60 100644 > > --- a/hw/core/machine-qmp-cmds.c > > +++ b/hw/core/machine-qmp-cmds.c > > @@ -204,3 +204,31 @@ MemdevList *qmp_query_memdev(Error **errp) > > object_child_foreach(obj, query_memdev, &list); > > return list; > > } > > + > > +RegisterInfo *qmp_x_query_registers(bool has_cpu, int64_t cpu, Error **errp) > > +{ > > + RegisterInfo *info = g_new0(RegisterInfo, 1); > > + g_autoptr(GString) buf = g_string_new(""); > > + CPUState *cs = NULL, *tmp; > > + > > + if (has_cpu) { > > + CPU_FOREACH(tmp) { > > + if (cpu == tmp->cpu_index) { > > + cs = tmp; > > + } > > + } > > + if (!cs) { > > + error_setg(errp, "CPU %"PRId64" not available", cpu); > > + return NULL; > > + } > > + cpu_format_state(cs, buf, CPU_DUMP_FPU); > > + } else { > > + CPU_FOREACH(cs) { > > + g_string_append_printf(buf, "\nCPU#%d\n", cs->cpu_index); > > + cpu_format_state(cs, buf, CPU_DUMP_FPU); > > + } > > + } > > + > > + info->state = g_steal_pointer(&buf->str); > > + return info; > > +} > > diff --git a/qapi/machine.json b/qapi/machine.json > > index 157712f006..27b922f2ce 100644 > > --- a/qapi/machine.json > > +++ b/qapi/machine.json > > @@ -1312,3 +1312,40 @@ > > '*cores': 'int', > > '*threads': 'int', > > '*maxcpus': 'int' } } > > + > > +## > > +# @RegisterParams: > > +# > > +# Information about the CPU to query state of > > +# > > +# @cpu: the CPU number to query. If omitted, queries all CPUs > > +# > > +# Since: 6.2.0 > > +# > > +## > > +{ 'struct': 'RegisterParams', 'data': {'*cpu': 'int' } } > > + > > +## > > +# @RegisterInfo: > > +# > > +# Information about the CPU state > > +# > > +# @state: the CPU state in an architecture specific format > > +# > > +# Since: 6.2.0 > > +# > > +## > > +{ 'struct': 'RegisterInfo', 'data': {'state': 'str' } } > > + > > +## > > +# @x-query-registers: > > +# > > +# Return information on the CPU registers > > +# > > +# Returns: the CPU state > > +# > > +# Since: 6.2.0 > > +## > > +{ 'command': 'x-query-registers', > > + 'data': 'RegisterParams', > > Unless you have further uses of RegisterParams in mind, use an implicit > type: > > 'data': { '*cpu': 'int' } } No further usage, so will do this. > > > + 'returns': 'RegisterInfo' } > > I'd like us to adopt a convention for commands returning formatted text > for human consumption. Like so: > > 'returns': 'HumanReadableText' } > > with the obvious > > ## > # @HumanReadableText: > # > # @human-readable-text: Formatted output intended for humans. > # > # Since: 6.2.0 > ## > { 'struct': 'HumanReadableText', > 'data': { 'human-readable-text': 'str' } } Ah yes that's a nice idea that will apply easily for many/most of the "info xxxx" commands without current QMP equivs. > When the output needs explaining, do that in the command's doc string. > I figure > > ## > # @x-query-registers: > # > # Returns information about the CPU state > # > # Returns: CPU state in an architecture-specific format > # > # Since: 6.2.0 > ## > > would do in this case. Yep. Regards, Daniel
diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c index 216fdfaf3a..0d9943ff60 100644 --- a/hw/core/machine-qmp-cmds.c +++ b/hw/core/machine-qmp-cmds.c @@ -204,3 +204,31 @@ MemdevList *qmp_query_memdev(Error **errp) object_child_foreach(obj, query_memdev, &list); return list; } + +RegisterInfo *qmp_x_query_registers(bool has_cpu, int64_t cpu, Error **errp) +{ + RegisterInfo *info = g_new0(RegisterInfo, 1); + g_autoptr(GString) buf = g_string_new(""); + CPUState *cs = NULL, *tmp; + + if (has_cpu) { + CPU_FOREACH(tmp) { + if (cpu == tmp->cpu_index) { + cs = tmp; + } + } + if (!cs) { + error_setg(errp, "CPU %"PRId64" not available", cpu); + return NULL; + } + cpu_format_state(cs, buf, CPU_DUMP_FPU); + } else { + CPU_FOREACH(cs) { + g_string_append_printf(buf, "\nCPU#%d\n", cs->cpu_index); + cpu_format_state(cs, buf, CPU_DUMP_FPU); + } + } + + info->state = g_steal_pointer(&buf->str); + return info; +} diff --git a/qapi/machine.json b/qapi/machine.json index 157712f006..27b922f2ce 100644 --- a/qapi/machine.json +++ b/qapi/machine.json @@ -1312,3 +1312,40 @@ '*cores': 'int', '*threads': 'int', '*maxcpus': 'int' } } + +## +# @RegisterParams: +# +# Information about the CPU to query state of +# +# @cpu: the CPU number to query. If omitted, queries all CPUs +# +# Since: 6.2.0 +# +## +{ 'struct': 'RegisterParams', 'data': {'*cpu': 'int' } } + +## +# @RegisterInfo: +# +# Information about the CPU state +# +# @state: the CPU state in an architecture specific format +# +# Since: 6.2.0 +# +## +{ 'struct': 'RegisterInfo', 'data': {'state': 'str' } } + +## +# @x-query-registers: +# +# Return information on the CPU registers +# +# Returns: the CPU state +# +# Since: 6.2.0 +## +{ 'command': 'x-query-registers', + 'data': 'RegisterParams', + 'returns': 'RegisterInfo' }
This is a counterpart to the HMP "info registers" 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: Daniel P. Berrangé <berrange@redhat.com> --- hw/core/machine-qmp-cmds.c | 28 ++++++++++++++++++++++++++++ qapi/machine.json | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+)