Re: [PATCH 1/2] hw/nvme: add 'info nvme' HMP command

[Markus Armbruster <armbru@redhat.com>]

"Dr. David Alan Gilbert" <dave@treblig.org> writes:

> * Mateusz Nowicki (mateusz.nowicki@posteo.net) wrote:
>> Add an 'info nvme' HMP command for inspecting emulated NVMe
>> controllers from the QEMU monitor.
>> 
>> For each NVMe controller in the QOM tree the command prints:
>> 
>>   - PCI BDF, vendor/device ID and BAR0 base address;
>>   - Identify Controller fields visible to the host driver: SN, MN, FR
>>     and CNTLID;
>>   - the CC, CSTS and AQA registers (raw 32-bit values);
>>   - the number of admin and active I/O submission/completion queues.
>> 
>> Useful when debugging the Linux NVMe driver against the QEMU emulation
>> without attaching gdb to the QEMU process.
>> 
>> Example:
>> 
>>   (qemu) info nvme
>>   /machine/peripheral-anon/device[0]
>>     PCI:    BDF 00:04.0  VID=8086 DID=5845  BAR0=0x00000000feb50000
>>     ID:     SN=NVME0001  MN=QEMU NVMe Ctrl  FR=...  CNTLID=0x0000
>>     CC:     0x00460001
>>     CSTS:   0x00000001
>>     AQA:    0x001f001f
>>     Queues: 1 admin + 4 IO SQ / 4 IO CQ
>
> I won't claim to know what those NVMe magic numbers mean, but I'll
> trust you they're useful.
>
>> Signed-off-by: Mateusz Nowicki <mateusz.nowicki@posteo.net>
>> ---
>>  hmp-commands-info.hx | 13 ++++++++
>>  hw/nvme/meson.build  |  2 +-
>>  hw/nvme/monitor.c    | 76 ++++++++++++++++++++++++++++++++++++++++++++
>>  qapi/machine.json    | 17 ++++++++++
>
> So that lot looks OK from an HMP point of view, I wonder if
> qapi/machine.json is the right place for it.  Markus?

qapi/machine.json has become a bit of a dumping ground, I'm afraid.

Precedence for putting queries for the machine's device frontends of
certain kinds there: x-query-irq, x-query-interrupt-controllers, and
x-query-usb.

All the other NVMe QAPI stuff is in qapi/block-core.json, but that's the
block backend, not frontend.  Not a good home for x-query-nvme.

Let's take the patch as is.

One nitpick below.

> Acked-by: Dr. David Alan Gilbert <dave@treblig.org>
>
>>  4 files changed, 107 insertions(+), 1 deletion(-)
>>  create mode 100644 hw/nvme/monitor.c
>> 
>> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
>> index 82134eb6c2..b984691c3c 100644
>> --- a/hmp-commands-info.hx
>> +++ b/hmp-commands-info.hx
>> @@ -340,6 +340,19 @@ SRST
>>      Show guest USB devices.
>>  ERST
>>  
>> +    {
>> +        .name       = "nvme",
>> +        .args_type  = "",
>> +        .params     = "",
>> +        .help       = "show emulated NVMe controllers",
>> +        .cmd_info_hrt = qmp_x_query_nvme,
>> +    },
>> +
>> +SRST
>> +  ``info nvme``
>> +    Show emulated NVMe controllers.
>> +ERST
>> +
>>      {
>>          .name       = "usbhost",
>>          .args_type  = "",
>> diff --git a/hw/nvme/meson.build b/hw/nvme/meson.build
>> index 7d5caa53c2..3017f058f9 100644
>> --- a/hw/nvme/meson.build
>> +++ b/hw/nvme/meson.build
>> @@ -1 +1 @@
>> -system_ss.add(when: 'CONFIG_NVME_PCI', if_true: files('ctrl.c', 'dif.c', 'ns.c', 'subsys.c', 'nguid.c'))
>> \ No newline at end of file
>> +system_ss.add(when: 'CONFIG_NVME_PCI', if_true: files('ctrl.c', 'dif.c', 'ns.c', 'subsys.c', 'nguid.c', 'monitor.c'))
>> \ No newline at end of file

Please use the opportunity to add the newline.

[...]

QAPI schema
Acked-by: Markus Armbruster <armbru@redhat.com>