Re: [Qemu-devel] [PATCH v2 2/4] net/vmxnet3: return correct value for VMXNET3_CMD_GET_DID_* command

[Miao Yan]

2015-12-23 4:49 GMT+08:00 Shmulik Ladkani <address@hidden>:
> Hi,
>
> On Mon, 21 Dec 2015 22:18:22 -0800 Miao Yan <address@hidden> wrote:
>> VMXNET3_CMD_GET_DID_LO should return PCI ID of the device
>> and VMXNET3_CMD_GET_DID_HI should return vmxnet3 revision ID.
>>
>> This behavior can be observed by the following steps:
>>
>> 1) run a Linux distro on esxi server
>> 2) modify vmxnet3 Linux driver to read DID_HI and DID_LO:
>>
>>   VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD, VMXNET3_CMD_GET_DID_LO);
>>   lo =  VMXNET3_READ_BAR1_REG(adapter, VMXNET3_REG_CMD);
>>
>>   VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD, VMXNET3_CMD_GET_DID_HI);
>>   high =  VMXNET3_READ_BAR1_REG(adapter, VMXNET3_REG_CMD);
>>   pr_info("vmxnet3 DID lo: 0x%x, high: 0x%x\n", lo, high);
>>
>> The kernel log will have something like the following message:
>>
>>   [ 7005.111170] vmxnet3 DID lo: 0x7b0, high: 0x1
>
> [...]
>
>> +    case VMXNET3_CMD_GET_DID_HI:
>> +        ret = VMXNET3_DEVICE_REVISION;
>> +        break;
>> +
>
> Do we know whether VMXNET3_DEVICE_REVISION needs to be returned, or
> should it be VMXNET3_DEVICE_VERSION instead?

VMXNET3_DEVICE_VERSION is only used as return value of reading
UPT version register, maybe it's better to rename it to
VMXNET3_UPT_REVERSION.

>
> I see both are currently defined as 1, but I assume this could
> potentially be changed in the future.
>
> How can we tell what's the right semantics for VMXNET3_CMD_GET_DID_HI?

By inspecting what esxi does. It always returns 1 for VMXNET3_CMD_GET_DID_HI.
So maybe we should stick it to 1.

>
> Regards,
> Shmulik