Hi Markus, On 7/14/21 2:42 AM, Markus Armbruster wrote: > 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.
Thank you very much for the explanation! > > 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. > Yes, this is for debugging purpose. The primary objective is to confirm a specific vector is not erroneously masked permanently, when a specific device queue is stuck. I will replace commit message and send v4. Thank you very much! Dongli Zhang