From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id EA8C91B319; Thu, 9 Nov 2017 07:51:08 +0100 (CET) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga104.jf.intel.com with ESMTP; 08 Nov 2017 22:51:07 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,368,1505804400"; d="scan'208";a="147792048" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga004.jf.intel.com with ESMTP; 08 Nov 2017 22:51:07 -0800 Received: from fmsmsx158.amr.corp.intel.com (10.18.116.75) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 8 Nov 2017 22:51:07 -0800 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by fmsmsx158.amr.corp.intel.com (10.18.116.75) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 8 Nov 2017 22:51:07 -0800 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.213]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.159]) with mapi id 14.03.0319.002; Thu, 9 Nov 2017 14:51:04 +0800 From: "Tan, Jianfeng" To: "Yang, Zhiyong" , "dev@dpdk.org" CC: "stable@dpdk.org" , "yliu@fridaylinux.org" , "maxime.coquelin@redhat.com" Thread-Topic: [PATCH v4] net/virtio: fix rxq intr config fails using vfio-pci Thread-Index: AQHTWRdIKyOZfvWRF06pi3cnmgQJaaMLmrJg Date: Thu, 9 Nov 2017 06:51:04 +0000 Message-ID: References: <20171109031818.31040-1-zhiyong.yang@intel.com> <20171109044607.3327-1-zhiyong.yang@intel.com> In-Reply-To: <20171109044607.3327-1-zhiyong.yang@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v4] net/virtio: fix rxq intr config fails using vfio-pci X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Nov 2017 06:51:09 -0000 > -----Original Message----- > From: Yang, Zhiyong > Sent: Thursday, November 9, 2017 12:46 PM > To: dev@dpdk.org > Cc: stable@dpdk.org; Tan, Jianfeng; yliu@fridaylinux.org; > maxime.coquelin@redhat.com; Yang, Zhiyong > Subject: [PATCH v4] net/virtio: fix rxq intr config fails using vfio-pci >=20 > 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 > time. 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. > use_msix can indicate three different msix status by: > VIRTIO_MSIX_NONE (0) > VIRTIO_MSIX_DISABLED (1) > VIRTIO_MSIX_ENABLED (2) >=20 > CC: stable@dpdk.org > CC: jianfeng.tan@intel.com > CC: yliu@fridaylinux.org > CC: maxime.coquelin@redhat.com >=20 > Fixes: cb482cb3a305 ("net/virtio: fix MAC address read") > Signed-off-by: Zhiyong Yang Except a small nit, looks good to me: Acked-by: Jianfeng Tan > --- >=20 > Changes in v4: > Enhance the commit log description. >=20 > Changes in v3: > Simply the code according to jianfeng's comments. >=20 > Changes in v2: > Add the capability to detect msix if virtio intr changes. >=20 > drivers/net/virtio/virtio_ethdev.c | 46 > ++++++++++++++++++++++++++++++------ > drivers/net/virtio/virtio_pci.c | 48 > ++++++++++++++++++++++++++++++++++++-- > drivers/net/virtio/virtio_pci.h | 6 +++++ > 3 files changed, 91 insertions(+), 9 deletions(-) >=20 > diff --git a/drivers/net/virtio/virtio_ethdev.c > b/drivers/net/virtio/virtio_ethdev.c > index d2576d5e0..87ac2bee6 100644 > --- a/drivers/net/virtio/virtio_ethdev.c > +++ b/drivers/net/virtio/virtio_ethdev.c > @@ -97,6 +97,9 @@ static void virtio_mac_addr_remove(struct rte_eth_dev > *dev, uint32_t index); > static void virtio_mac_addr_set(struct rte_eth_dev *dev, > struct ether_addr *mac_addr); >=20 > +static int virtio_intr_enable(struct rte_eth_dev *dev); > +static int virtio_intr_disable(struct rte_eth_dev *dev); > + > static int virtio_dev_queue_stats_mapping_set( > struct rte_eth_dev *eth_dev, > uint16_t queue_id, > @@ -618,7 +621,7 @@ virtio_dev_close(struct rte_eth_dev *dev) > virtio_queues_unbind_intr(dev); >=20 > if (intr_conf->lsc || intr_conf->rxq) { > - rte_intr_disable(dev->intr_handle); > + virtio_intr_disable(dev); > rte_intr_efd_disable(dev->intr_handle); > rte_free(dev->intr_handle->intr_vec); > dev->intr_handle->intr_vec =3D NULL; > @@ -1160,6 +1163,34 @@ virtio_vlan_filter_set(struct rte_eth_dev *dev, > uint16_t vlan_id, int on) > } >=20 > static int > +virtio_intr_enable(struct rte_eth_dev *dev) > +{ > + struct virtio_hw *hw =3D dev->data->dev_private; > + > + if (rte_intr_enable(dev->intr_handle) < 0) > + return -1; > + > + if (!hw->virtio_user_dev) > + hw->use_msix =3D > vtpci_msix_detect(RTE_ETH_DEV_TO_PCI(dev)); > + > + return 0; > +} > + > +static int > +virtio_intr_disable(struct rte_eth_dev *dev) > +{ > + struct virtio_hw *hw =3D dev->data->dev_private; > + > + if (rte_intr_disable(dev->intr_handle) < 0) > + return -1; > + > + if (!hw->virtio_user_dev) > + hw->use_msix =3D > vtpci_msix_detect(RTE_ETH_DEV_TO_PCI(dev)); > + > + return 0; > +} > + > +static int > virtio_negotiate_features(struct virtio_hw *hw, uint64_t req_features) > { > uint64_t host_features; > @@ -1228,7 +1259,7 @@ virtio_interrupt_handler(void *param) > isr =3D vtpci_isr(hw); > PMD_DRV_LOG(INFO, "interrupt status =3D %#x", isr); >=20 > - if (rte_intr_enable(dev->intr_handle) < 0) > + if (virtio_intr_enable(dev) < 0) > PMD_DRV_LOG(ERR, "interrupt enable failed"); >=20 > if (isr & VIRTIO_PCI_ISR_CONFIG) { > @@ -1348,7 +1379,7 @@ virtio_configure_intr(struct rte_eth_dev *dev) > * to change the config size from 20 to 24, or > VIRTIO_MSI_QUEUE_VECTOR > * (22) will be ignored. > */ > - if (rte_intr_enable(dev->intr_handle) < 0) { > + if (virtio_intr_enable(dev) < 0) { > PMD_DRV_LOG(ERR, "interrupt enable failed"); > return -1; > } > @@ -1388,7 +1419,8 @@ virtio_init_device(struct rte_eth_dev *eth_dev, > uint64_t req_features) > } >=20 > /* If host does not support both status and MSI-X then disable LSC */ > - if (vtpci_with_feature(hw, VIRTIO_NET_F_STATUS) && hw- > >use_msix) > + if (vtpci_with_feature(hw, VIRTIO_NET_F_STATUS) && > + hw->use_msix !=3D VIRTIO_MSIX_NONE) > eth_dev->data->dev_flags |=3D RTE_ETH_DEV_INTR_LSC; > else > eth_dev->data->dev_flags &=3D ~RTE_ETH_DEV_INTR_LSC; > @@ -1801,9 +1833,9 @@ virtio_dev_start(struct rte_eth_dev *dev) > */ > if (dev->data->dev_conf.intr_conf.lsc || > dev->data->dev_conf.intr_conf.rxq) { > - rte_intr_disable(dev->intr_handle); > + virtio_intr_disable(dev); >=20 > - if (rte_intr_enable(dev->intr_handle) < 0) { > + if (virtio_intr_enable(dev) < 0) { > PMD_DRV_LOG(ERR, "interrupt enable failed"); > return -EIO; > } > @@ -1912,7 +1944,7 @@ virtio_dev_stop(struct rte_eth_dev *dev) > PMD_INIT_LOG(DEBUG, "stop"); >=20 > if (intr_conf->lsc || intr_conf->rxq) > - rte_intr_disable(dev->intr_handle); > + virtio_intr_disable(dev); >=20 > hw->started =3D 0; > memset(&link, 0, sizeof(link)); > diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_= pci.c > index 55b717c03..10c45228c 100644 > --- a/drivers/net/virtio/virtio_pci.c > +++ b/drivers/net/virtio/virtio_pci.c > @@ -57,7 +57,8 @@ > * The remaining space is defined by each driver as the per-driver > * configuration space. > */ > -#define VIRTIO_PCI_CONFIG(hw) (((hw)->use_msix) ? 24 : 20) > +#define VIRTIO_PCI_CONFIG(hw) \ > + (((hw)->use_msix =3D=3D VIRTIO_MSIX_ENABLED) ? 24 : 20) >=20 > static inline int > check_vq_phys_addr_ok(struct virtqueue *vq) > @@ -617,7 +618,9 @@ virtio_read_caps(struct rte_pci_device *dev, struct > virtio_hw *hw) > uint16_t flags =3D ((uint16_t *)&cap)[1]; >=20 > if (flags & PCI_MSIX_ENABLE) > - hw->use_msix =3D 1; > + hw->use_msix =3D VIRTIO_MSIX_ENABLED; > + else > + hw->use_msix =3D VIRTIO_MSIX_DISABLED; > } >=20 > if (cap.cap_vndr !=3D PCI_CAP_ID_VNDR) { > @@ -710,3 +713,44 @@ vtpci_init(struct rte_pci_device *dev, struct > virtio_hw *hw) >=20 > return 0; > } > + > +/* Return 0 (VIRTIO_MSIX_NONE) > + * Return 1 (VIRTIO_MSIX_DISABLED) > + * Return 2 (VIRTIO_MSIX_ENABLED) > + */ Above comments seems not necessary as it just repeat it's just the macro's = definition. Thanks, Jianfeng