Re: [dpdk-dev] [PATCH v2] net/virtio: fix rxq intr config fails using vfio-pci

["Tan, Jianfeng" <jianfeng.tan@intel.com>]

On 11/9/2017 10:11 AM, Yang, Zhiyong wrote:
> Hi Jianfeng,
>
>> -----Original Message-----
>> From: Tan, Jianfeng
>> Sent: Wednesday, November 8, 2017 9:53 PM
>> To: Yang, Zhiyong <zhiyong.yang@intel.com>; dev@dpdk.org
>> Cc: yliu@fridaylinux.org; maxime.coquelin@redhat.com
>> Subject: Re: [PATCH v2] net/virtio: fix rxq intr config fails using vfio-pci
>>
>>
>> Hi Zhiyong,
>>
>>
>> On 11/8/2017 7:03 PM, Zhiyong Yang wrote:
>>> When running l3fwd-power to test virtio rxq interrupt using vfio pci
>>> noiommu mode, startup fails. In the function virtio_read_caps, the
>>> code if (flags & PCI_MSIX_ENABLE) intends to double check if vfio msix
>>> is enabled or not. However, it is not enable at that stage. So
>>> use_msix is assigned to "0", not "1", which causes the failure of
>>> configuring rxq intr in l3fwd-power.
>>> This patch adds the function vtpci_msix_detect to detect the status of
>>> msix when interrupt changes happen.
>>> In the meanwhile, virtio_intr_enable/disable are introduced to wrap
>>> rte_intr_enable/disable to enhance the ability to detect msix. Only
>>> support and enable msix can assign "1" to use_msix.
>> Should be "2". Better to use macro here.
>>
>>> Fixes: cb482cb3a305 ("net/virtio: fix MAC address read")
>>> Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
>>> ---
>>> @@ -1370,7 +1411,15 @@ virtio_init_device(struct rte_eth_dev *eth_dev,
>> uint64_t req_features)
>>>    	struct virtio_net_config local_config;
>>>    	struct rte_pci_device *pci_dev = NULL;
>>>    	int ret;
>>> +	int msix_detect;
>>>
>>> +	msix_detect = vtpci_msix_detect(RTE_ETH_DEV_TO_PCI(eth_dev));
>>> +	if (msix_detect < 0)
>>> +		return -1;
>>> +	else if (msix_detect == SUPPORT_MSIX_STATUS_ENABLED)
>>> +		hw->use_msix = 1;
>>> +	else
>>> +		hw->use_msix = 0;
>> Ditto, we directly assign return value to hw->use_msix.
>>
> The function call can be removed if use_misx  can use 0, 1, 2 directly
> We can implement the same logic  in  virtio_read_caps instead.

Actually, I prefer not hide such an assignment in this function name 
"*_detect".


> Thanks
> Zhiyong
>