From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 30A3F9A91 for ; Tue, 3 Feb 2015 06:11:19 +0100 (CET) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga103.fm.intel.com with ESMTP; 02 Feb 2015 21:04:44 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,510,1418112000"; d="scan'208";a="521685992" Received: from pgsmsx101.gar.corp.intel.com ([10.221.44.78]) by orsmga003.jf.intel.com with ESMTP; 02 Feb 2015 21:03:41 -0800 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by PGSMSX101.gar.corp.intel.com (10.221.44.78) with Microsoft SMTP Server (TLS) id 14.3.195.1; Tue, 3 Feb 2015 13:05:14 +0800 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.253]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.91]) with mapi id 14.03.0195.001; Tue, 3 Feb 2015 13:05:13 +0800 From: "Qiu, Michael" To: Tetsuya Mukawa , "dev@dpdk.org" Thread-Topic: [PATCH v6 10/13] eal/pci: Cleanup pci driver initialization code Thread-Index: AQHQPdPv2poZGqybH0yG3xymcxJNQA== Date: Tue, 3 Feb 2015 05:05:12 +0000 Message-ID: <533710CFB86FA344BFBF2D6802E60286CD3DE7@SHSMSX101.ccr.corp.intel.com> References: <1421664027-17971-9-git-send-email-mukawa@igel.co.jp> <1422763322-13742-1-git-send-email-mukawa@igel.co.jp> <1422763322-13742-11-git-send-email-mukawa@igel.co.jp> <533710CFB86FA344BFBF2D6802E60286CD3AE6@SHSMSX101.ccr.corp.intel.com> <54D0497F.9080206@igel.co.jp> 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 v6 10/13] eal/pci: Cleanup pci driver initialization code X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 03 Feb 2015 05:11:19 -0000 On 2/3/2015 12:07 PM, Tetsuya Mukawa wrote:=0A= > On 2015/02/03 11:35, Qiu, Michael wrote:=0A= >> On 2/1/2015 12:02 PM, Tetsuya Mukawa wrote:=0A= >>> - Add rte_eal_pci_close_one_dirver()=0A= >>> The function is used for closing the specified driver and device.=0A= >>> - Add pci_invoke_all_drivers()=0A= =0A= [...]=0A= >>> =0A= >>> +#ifdef ENABLE_HOTPLUG=0A= >>> +/*=0A= >>> + * If vendor/device ID match, call the devuninit() function of the=0A= >>> + * driver.=0A= >>> + */=0A= >>> +int=0A= >>> +rte_eal_pci_close_one_driver(struct rte_pci_driver *dr,=0A= >>> + struct rte_pci_device *dev)=0A= >>> +{=0A= >>> + struct rte_pci_id *id_table;=0A= >>> +=0A= >>> + if ((dr =3D=3D NULL) || (dev =3D=3D NULL))=0A= >>> + return -EINVAL;=0A= >>> +=0A= >>> + for (id_table =3D dr->id_table ; id_table->vendor_id !=3D 0; id_table= ++) {=0A= >>> +=0A= >>> + /* check if device's identifiers match the driver's ones */=0A= >>> + if (id_table->vendor_id !=3D dev->id.vendor_id &&=0A= >>> + id_table->vendor_id !=3D PCI_ANY_ID)=0A= >>> + continue;=0A= >>> + if (id_table->device_id !=3D dev->id.device_id &&=0A= >>> + id_table->device_id !=3D PCI_ANY_ID)=0A= >>> + continue;=0A= >>> + if (id_table->subsystem_vendor_id !=3D=0A= >>> + dev->id.subsystem_vendor_id &&=0A= >>> + id_table->subsystem_vendor_id !=3D PCI_ANY_ID)=0A= >>> + continue;=0A= >>> + if (id_table->subsystem_device_id !=3D=0A= >>> + dev->id.subsystem_device_id &&=0A= >>> + id_table->subsystem_device_id !=3D PCI_ANY_ID)=0A= >>> + continue;=0A= >>> +=0A= >>> + struct rte_pci_addr *loc =3D &dev->addr;=0A= >>> +=0A= >>> + RTE_LOG(DEBUG, EAL,=0A= >>> + "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",=0A= >>> + loc->domain, loc->bus, loc->devid,=0A= >>> + loc->function, dev->numa_node);=0A= >>> +=0A= >>> + RTE_LOG(DEBUG, EAL, " remove driver: %x:%x %s\n",=0A= >>> + dev->id.vendor_id, dev->id.device_id,=0A= >>> + dr->name);=0A= >>> +=0A= >>> + /* call the driver devuninit() function */=0A= >>> + if (dr->devuninit && (dr->devuninit(dr, dev) < 0))=0A= >>> + return -1; /* negative value is an error */=0A= >>> +=0A= >>> + /* clear driver structure */=0A= >>> + dev->driver =3D NULL;=0A= >>> +=0A= >>> + if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)=0A= >>> + /* unmap resources for devices that use igb_uio */=0A= >>> + pci_unmap_device(dev);=0A= >> Hi, Tetsuya=0A= >>=0A= >> I have one question, as the code shows, in pci_unmap_device(), will=0A= >> check pt_driver.=0A= >>=0A= >> But assume that, we are now try to detach a vfio device, after print out= =0A= >> a error message of unsupported, the does this port workable?=0A= >>=0A= >> I think this port will unworkable, am I right?=0A= >>=0A= >> But actually, we should keep it workable.=0A= >>=0A= >> My suggestion is to add a check in rte_eth_dev_check_detachable() for= =0A= >> pci_device port.=0A= > Hi Michael,=0A= >=0A= > I appreciate your comment.=0A= > In the function called "rte_eal_dev_detach_pdev()",=0A= > "rte_eth_dev_check_detachable()" has been already checked.=0A= =0A= What I mean is check the pt_driver for pci_dev in=0A= rte_eth_dev_check_detachable(), so that hotplug framework will not=0A= affect vfio devices, just as I reply in another mail.=0A= =0A= Current logic will affect vfio devices if try to detach( Not do the=0A= really test, just the logic shows), am I right?=0A= =0A= Thanks,=0A= Michael=0A= =0A= > But in the future, someone may want to reuse=0A= > "rte_eal_pci_close_one_driver()".=0A= > So I will add the checking like your suggestion.=0A= >=0A= > Thanks,=0A= > Tetsuya=0A= >=0A= >> Thanks=0A= >> Michael=0A= >>=0A= >>> +=0A= >>> + return 0;=0A= >>> + }=0A= >>> + /* return positive value if driver is not found */=0A= >>> + return 1;=0A= >>> +}=0A= >>> +#else /* ENABLE_HOTPLUG */=0A= >>> +int=0A= >>> +rte_eal_pci_close_one_driver(struct rte_pci_driver *dr __rte_unused,= =0A= >>> + struct rte_pci_device *dev __rte_unused)=0A= >>> +{=0A= >>> + RTE_LOG(ERR, EAL, "Hotplug support isn't enabled\n");=0A= >>> + return -1;=0A= >>> +}=0A= >>> +#endif /* ENABLE_HOTPLUG */=0A= >>> +=0A= >>> /* Init the PCI EAL subsystem */=0A= >>> int=0A= >>> rte_eal_pci_init(void)=0A= >=0A= >=0A= =0A=