Re: [PATCH RFC 0/1] To add HMP interface to dump PCI MSI-X table/PBA

[Jason Wang <jasowang@redhat.com>]

在 2021/4/28 下午1:10, Dongli Zhang 写道:
> Hi Jason,
>
> On 4/27/21 7:31 PM, Jason Wang wrote:
>> 在 2021/4/27 下午4:53, Dr. David Alan Gilbert 写道:
>>> * Dongli Zhang (dongli.zhang@oracle.com) wrote:
>>>> On 4/22/21 11:01 PM, Jason Wang wrote:
>>>>> 在 2021/4/23 下午12:47, Dongli Zhang 写道:
>>>>>> This is inspired by the discussion with Jason on below patchset.
>>>>>>
>>>>>> https://urldefense.com/v3/__https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg09020.html__;!!GqivPVa7Brio!KbGQZW5lq3JZ60k12NuWZ6Th1lT6AwmBTF0pBgoWUKKQ4-2UhdW57PtvXUN5XQnZ2NU$
>>>>>>
>>>>>>
>>>>>> The new HMP command is introduced to dump the MSI-X table and PBA.
>>>>>>
>>>>>> Initially, I was going to add new option to "info pci". However, as the
>>>>>> number of entries is not determined and the output of MSI-X table is much
>>>>>> more similar to the output of hmp_info_tlb()/hmp_info_mem(), this patch
>>>>>> adds interface for only HMP.
>>>>>>
>>>>>> The patch is tagged with RFC because I am looking for suggestions on:
>>>>>>
>>>>>> 1. Is it fine to add new "info msix <dev>" command?
>>>>> I wonder the reason for not simply reusing "info pci"?
>>>> The "info pci" will show PCI data for all devices and it does not accept any
>>>> argument to print for a specific device.
>>>>
>>>> In addition, the "info pci" relies on qmp_query_pci(), where this patch will not
>>>> implement the interface for QMP considering the number of MSI-X entries is not
>>>> determined.
>>>>
>>>> Suppose we have 10 NVMe (emulated by QEMU with default number of queues), we
>>>> will have about 600+ lines of output.
>>>   From an HMP perspective I'm happy, so:
>>>
>>> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>>
>>> but since I don't know much about MSI I'd like to see Jason's reply.
>>
>> I think we'd better have more information, e.g the device can optionally report
>> how the MSI-X vector is used.
>>
>> Virtio-pci could be the first user for this.
> As discussed in another thread, you were talking about to print MSIMessage.
>
> However, I prefer to print the raw data as I think the user of this interface
> should be able to understand it as MSI-X messages.
>
> For instance, below is the data printed by "info msix".


Just to clarify, I meant e.g for virtio-pci device, we can let it to 
print the mapping between vq and msix vectors:

vq[0].msix_vector = 0
vq[1].msix_vector = 1
config.msix_vector = 2
...

But this could be added on top if you wish.

Does this make sense?

Thanks


>
> 0xfee01004 0x00000000 0x00000022 0x00000000
> 0xfee02004 0x00000000 0x00000023 0x00000000
> 0xfee01004 0x00000000 0x00000023 0x00000000
> 0xfee01004 0x00000000 0x00000021 0x00000000
> 0xfee02004 0x00000000 0x00000022 0x00000000
> 0x00000000 0x00000000 0x00000000 0x00000001
> 0x00000000 0x00000000 0x00000000 0x00000001
>
> The 1st column is Message Lower Address.
>
> The 2nd column is Message Upper Address.
>
> The 3rd column is Message Data.
>
> The 4th column is Vector Control.
>
> In my opinion, this is equivalent to MSIMessage.
>
> 26 struct MSIMessage {
> 27     uint64_t address; --> column 1 and 2
> 28     uint32_t data;    --> column 3
> 29 };
>
>
> We use the similar way to read from Linux OS, e,g., given the address of MSI-X
> cap, here is how we read from OS side.
>
> # busybox devmem 0xc1001000 32
> 0xFEE00000
> # busybox devmem 0xc1001004 32
> 0x00000000
> # busybox devmem 0xc1001008 32
> 0x00004049
> # busybox devmem 0xc100100c 32
> 0x00000000
>
> Thank you very much!
>
> Dongli Zhang
>
>>
>>> Adding an optional option to 'info pci' to limit to one device would be easy
>>> though; that bit is probably easier than adding a new command.
>>
>> One interesting point is that MSI could be extended for other bus, (e.g MMIO).
>> So "info msi" should be better I guess.
>>
>>
>>> Figuring out the QMP representation of your entries might be harder -
>>> and if this is strictly for debug, probably not worth it?
>>
>> I think so.
>>
>> Thanks
>>
>>
>>> Dave
>>>
>>>
>>>> Dongli Zhang
>>>>
>>>>>> 2. Is there any issue with output format?
>>>>> If it's not for QMP, I guess it's not a part of ABI so it should be fine.
>>>>>
>>>>>
>>>>>> 3. Is it fine to add only for HMP, but not QMP?
>>>>> I think so.
>>>>>
>>>>> Thanks
>>>>>
>>>>>
>>>>>> Thank you very much!
>>>>>>
>>>>>> Dongli Zhang
>>>>>>
>>>>>>
>>>>>>