From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Tan, Jianfeng" Subject: Re: [PATCH 2/5] net/virtio-user: add rxq interrupt mode support Date: Tue, 28 Mar 2017 01:33:31 +0000 Message-ID: References: <1488563803-87754-1-git-send-email-jianfeng.tan@intel.com> <1488563803-87754-3-git-send-email-jianfeng.tan@intel.com> <20170317064757.GG18844@yliu-dev.sh.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" , "david.marchand@6wind.com" To: Yuanhan Liu Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id F2D363B5 for ; Tue, 28 Mar 2017 03:33:34 +0200 (CEST) In-Reply-To: <20170317064757.GG18844@yliu-dev.sh.intel.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > -----Original Message----- > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com] > Sent: Friday, March 17, 2017 2:48 PM > To: Tan, Jianfeng > Cc: dev@dpdk.org; david.marchand@6wind.com > Subject: Re: [PATCH 2/5] net/virtio-user: add rxq interrupt mode support >=20 > On Fri, Mar 03, 2017 at 05:56:40PM +0000, Jianfeng Tan wrote: > > Signed-off-by: Jianfeng Tan >=20 > I don't see a single word to explain how this patch works :/ Sorry, will add that in next version. >=20 > > --- > > drivers/net/virtio/virtio_ethdev.c | 17 ++++++++++++++-- > > drivers/net/virtio/virtio_user/virtio_user_dev.c | 25 > +++++++++++++++++++++++- > > drivers/net/virtio/virtio_user/virtio_user_dev.h | 2 +- > > drivers/net/virtio/virtio_user_ethdev.c | 12 +++++++++++- > > 4 files changed, 51 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/net/virtio/virtio_ethdev.c > b/drivers/net/virtio/virtio_ethdev.c > > index 4dc03b9..5d80d1a 100644 > > --- a/drivers/net/virtio/virtio_ethdev.c > > +++ b/drivers/net/virtio/virtio_ethdev.c > > @@ -1264,6 +1264,10 @@ virtio_configure_intr(struct rte_eth_dev *dev) > > { > > struct virtio_hw *hw =3D dev->data->dev_private; > > > > + > > +#ifdef RTE_VIRTIO_USER > > + if (!hw->virtio_user_dev) { > > +#endif >=20 > No need to put the #ifdef block here. virtio_user_dev is defined even > when RTE_VIRTIO_USER is not configured. Correct. I'll remove that. >=20 > ... >=20 > > + vtpci_reinit_complete(hw); > > + > > if (eth_dev->data->dev_conf.intr_conf.rxq) { > > if (virtio_configure_intr(eth_dev) < 0) { > > PMD_INIT_LOG(ERR, "failed to configure interrupt"); > > @@ -1416,8 +1431,6 @@ virtio_init_device(struct rte_eth_dev *eth_dev, > uint64_t req_features) > > } > > } > > > > - vtpci_reinit_complete(hw); >=20 >=20 > Hmm, why ...? Such stealthy change definitely need an explanation. > And it's more likely it needs a single patch. I should have described it more. The reason of above change is to make sure= intr_handle is filled in virtio_user_start_device(). But reconsidering thi= s, this could have effect on virtio pci device as it requires interrupts ar= e set up before setting DRIVER_OK. So how about: 1. configure intr for PCI devices; 2. vtpci_reinit_complete(); 3. configure intr for vdev; Thanks, Jianfeng