From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Tan, Jianfeng" Subject: Re: [PATCH v2] net/virtio: fix rxq intr config fails using vfio-pci Date: Thu, 9 Nov 2017 11:31:56 +0800 Message-ID: References: <20171031094456.24912-1-zhiyong.yang@intel.com> <20171108110348.38548-1-zhiyong.yang@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Cc: "yliu@fridaylinux.org" , "maxime.coquelin@redhat.com" To: "Yang, Zhiyong" , "dev@dpdk.org" Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 7225E1B2A4 for ; Thu, 9 Nov 2017 04:31:59 +0100 (CET) In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 ; 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 >>> --- >>> @@ -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 >