Dongli Zhang <dongli.zh...@oracle.com> writes:
> Hi Markus,
>
> On 7/13/21 10:46 PM, Markus Armbruster wrote:
>> Dongli Zhang <dongli.zh...@oracle.com> writes:
>>
>>> This patch is to add the HMP interface to dump MSI-X table and PBA, in
>>> order to help diagnose the loss of IRQ issue in VM (e.g., if an MSI-X
>>> vector is erroneously masked permanently). Here is the example with
>>> vhost-scsi:
>>>
>>> (qemu) info msix /machine/peripheral/vscsi0
>>> Msg L.Addr Msg U.Addr Msg Data Vect Ctrl
>>> 0xfee00000 0x00000000 0x00004041 0x00000000
>>> 0xfee00000 0x00000000 0x00004051 0x00000000
>>> 0xfee00000 0x00000000 0x00004061 0x00000000
>>> 0xfee00000 0x00000000 0x00004071 0x00000000
>>> 0xfee01000 0x00000000 0x000040b1 0x00000000
>>> 0xfee02000 0x00000000 0x000040c1 0x00000000
>>> 0xfee03000 0x00000000 0x000040d1 0x00000000
>>>
>>> MSI-X PBA
>>> 0 0 0 0 0 0 0
>>>
>>> Since the number of MSI-X entries is not determined and might be very
>>> large, it is sometimes inappropriate to dump via QMP.
>>
>> Why? What makes HMP different?
>
> Here are two reasons.
>
> 1. The size of MSI-X table is nondeterministic and might be very large, e.g.,
> the PCI_MSIX_FLAGS_QSIZE is 0x07FF. The "info tlb" (which is a table and
> similar
> to MSI-X) and "info lapic" also only support hmp.
>
> 2. The [PATCH 3/3] of this patchset support device specific data, the
> definitional of which varies depending on each device type (so far only
> virtio-pci supports the interface).
>
> Thank you very much!
>
> Dongli Zhang
>
>>
>>> Therefore, this patch dumps MSI-X information only via HMP, which is
>>> similar to the implementation of hmp_info_mem().
I think you're mixing up valid and invalid arguments :) Let me try to
pick them apart.
1a. "Command output may be too large for QMP, therefore provide only the
HMP command" makes no sense.
Both QMP and HMP are not meant for bulk data transfer. They are control
plane, not data plane. To illustrate what that means, consider a backup
feature. The commands to control the backup task are QMP (and HMP, if
desired), but the actual data transfer uses some other channel, so it
doesn't clog the QMP pipes.
If the data is too bulky for QMP, then it's too bulky for HMP, too.
1b. "info tlb" and "info lapic" are HMP only because they are debugging
commands for humans. Debugging is not necessarily done by humans only.
Sometimes, humans use programs to help them debug, and these programs
would be better off with QMP commands. For the HMP-only debugging
commands, we decided providing for (largely hypothetical) debugging
scripts wasn't worthwhile. A similar argument could probably be made
for "info msix".
2. "Output is in part specific to Devices, therefore provide only the
HMP command" is also iffy. Yes, hacking up a bunch of monitor_printf()s
is probably easier than specifying a QAPI schema. "It's work" is no
excuse. "It's more work than it's worth" can be one. But then we're
back at argument 1b.
Your commit message's first sentence suggests this is indeed just for
debugging. If this is the case, consider replacing
Since the number of MSI-X entries is not determined and might be very
large, it is sometimes inappropriate to dump via QMP.
Therefore, this patch dumps MSI-X information only via HMP, which is
similar to the implementation of hmp_info_mem().
by
Since this is just for debugging by humans, provide the command only
in HMP, not in QMP.
