From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerin Jacob Subject: Re: [PATCH] net/thunderx: manage PCI device mapping for SQS VFs Date: Tue, 6 Jun 2017 19:35:07 +0530 Message-ID: <20170606140506.GA31583@jerin> References: <20170601130530.11443-1-jerin.jacob@caviumnetworks.com> <130021ab-eac2-88ae-4b32-0ffe88f0bb55@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: dev@dpdk.org, Angela Czubak , Thomas Monjalon To: Ferruh Yigit Return-path: Received: from NAM03-DM3-obe.outbound.protection.outlook.com (mail-dm3nam03on0059.outbound.protection.outlook.com [104.47.41.59]) by dpdk.org (Postfix) with ESMTP id 333FE5599 for ; Tue, 6 Jun 2017 16:05:28 +0200 (CEST) Content-Disposition: inline In-Reply-To: <130021ab-eac2-88ae-4b32-0ffe88f0bb55@intel.com> 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----- > Date: Tue, 6 Jun 2017 14:36:09 +0100 > From: Ferruh Yigit > To: Jerin Jacob , dev@dpdk.org > CC: Angela Czubak , Thomas Monjalon > > Subject: Re: [dpdk-dev] [PATCH] net/thunderx: manage PCI device mapping for > SQS VFs > User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 > Thunderbird/52.1.1 > > On 6/1/2017 2:05 PM, Jerin Jacob wrote: > > Since the commit e84ad157b7bc ("pci: unmap resources if probe fails"), > > EAL unmaps the PCI device if ethdev probe returns positive or > > negative value. > > > > nicvf thunderx PMD needs special treatment for Secondary queue set(SQS) > > PCIe VF devices, where, it expects to not unmap or free the memory > > without registering the ethdev subsystem. > > > > To keep the same behavior, moved the PCI map function inside > > the driver without using the EAL services. > > What do you think adding a flag something like > RTE_PCI_DRV_FIXED_MAPPING? Does mapping but not unmap on error. > This would be more generic solution. > > I am concerned about calling eal level API from PMD. Understood. Another option is to unmap only on ERROR(ie, when probe return <0 value) ret = dr->probe(dr, dev); if (ret) { // change to if (ret < 0) dev->driver = NULL; if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) rte_pci_unmap_device(dev); } I am fine with either way. Let me know, what you prefer. I will change accordingly. > > > > > Signed-off-by: Jerin Jacob > > Signed-off-by: Angela Czubak > > --- > > drivers/net/thunderx/nicvf_ethdev.c | 9 ++++++++- > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/thunderx/nicvf_ethdev.c b/drivers/net/thunderx/nicvf_ethdev.c > > index 796701b0f..6ec2f9266 100644 > > --- a/drivers/net/thunderx/nicvf_ethdev.c > > +++ b/drivers/net/thunderx/nicvf_ethdev.c > > @@ -2025,6 +2025,13 @@ nicvf_eth_dev_init(struct rte_eth_dev *eth_dev) > > } > > > > pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev); > > + > > + ret = rte_pci_map_device(pci_dev); > > + if (ret) { > > + PMD_INIT_LOG(ERR, "Failed to map pci device"); > > + goto fail; > > + } > > + > > rte_eth_copy_pci_info(eth_dev, pci_dev); > > > > nic->device_id = pci_dev->id.device_id; > > @@ -2171,7 +2178,7 @@ static int nicvf_eth_pci_remove(struct rte_pci_device *pci_dev) > > > > static struct rte_pci_driver rte_nicvf_pmd = { > > .id_table = pci_id_nicvf_map, > > - .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC, > > + .drv_flags = RTE_PCI_DRV_INTR_LSC, > > .probe = nicvf_eth_pci_probe, > > .remove = nicvf_eth_pci_remove, > > }; > > >