Re: [Qemu-devel] [PATCH 0/3] correct some register return values for vxmnet3

[Miao Yan]

2015-12-23 10:15 GMT+08:00 Jason Wang <address@hidden>:
>
>
> On 12/22/2015 03:05 PM, Dmitry Fleytman wrote:
>>
>>> On 22 Dec 2015, at 04:44 AM, Miao Yan <address@hidden
>>> <mailto:address@hidden>> wrote:
>>>
>>> Hi Dmitry,
>>>
>>> 2015-12-22 1:19 GMT+08:00 Dmitry Fleytman <address@hidden
>>> <mailto:address@hidden>>:
>>>> Hello Miao,
>>>>
>>>> While patches look good technically, I cannot find any code snippets
>>>> that prove correctness of these changes.
>>>
>>> Linux driver does not read those registers currently,
>>> not sure about Windows version.
>>>
>>>
>>>> How do you know this is the correct behaviour? Could you please
>>>> extend commit messages with corresponding references?
>>>
>>>
>>> This behavior can be observed by modifying linux driver to read those
>>> registers
>>> at probe time (in vmxnet3_device_probe) and kernel log will have the
>>> following
>>> message (running on esxi server, of source):
>>>
>>> [  198.427389] VMware vmxnet3 virtual NIC driver - version 1.2.0.0-k-NAPI
>>> [  198.428863] vmxnet3 0000:03:00.0: # of Tx queues : 1, # of Rx
>>> queues : 1
>>> [  198.559625] vmxnet3 DID lo: 0x7b0, high: 0x1, dev_info: 0x0
>>> [  198.561151] vmxnet3 0000:03:00.0: irq 72 for MSI/MSI-X
>>>
>>> Here DID_LO is the pci device id, DID_HIGH is 0x1 and
>>> GET_DEV_EXTRA_INFO returns 0.
>>>
>>> putting above into commit message should be enough ?
>>
>> Yes, this should be good enough. Please also provide ESX version
>> you’re running on.
>>
>> Reviewed-by: Dmitry Fleytman <address@hidden
>> <mailto:address@hidden>>
>>
>> Jason, these changes are guest visible but init time only. Do you
>> think compatibility code is needed in this case?
>
> I think it was probably no need to do compatibility things here,
> consider no device state but only register read changes and I believe no
> driver should depend on those values.

Yes, if they depend on those values, then things are already
broken as we give them -1 which is hardly any driver would
expect.

I will prepare v3 to address review comments.


>
>>
>>>
>>>
>>>>
>>>> Thanks,
>>>> Dmitry
>>>>
>>>>> On 21 Dec 2015, at 13:06 PM, Miao Yan <address@hidden
>>>>> <mailto:address@hidden>> wrote:
>>>>>
>>>>> Qemu vmxnet3 emulation doesn't recognize VMXNET3_CMD_GET_DID_LO,
>>>>> VMXNET3_CMD_GET_DID_HI and VMXNET3_CMD_GET_DEV_EXTRA_INFO command and
>>>>> returns -1 on all of them.
>>>>>
>>>>> This patchset makes them return correct values.
>>>>>
>>>>> Miao Yan (3):
>>>>> net/vmxnet3: return 1 on device activation failure
>>>>> net/vmxnet3: return correct value for VMXNET3_CMD_GET_DID command
>>>>> net/vmxnet3: return correct value for VMXNET3_CMD_GET_DEV_EXTRA_INFO
>>>>>
>>>>> hw/net/vmxnet3.c | 11 ++++++++++-
>>>>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>>>>
>>>>> --
>>>>> 1.9.1
>>>>>
>>>>
>>
>