From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 8FC5837B4 for ; Tue, 3 Feb 2015 03:35:33 +0100 (CET) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga102.fm.intel.com with ESMTP; 02 Feb 2015 18:35:31 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,510,1418112000"; d="scan'208";a="671813902" Received: from pgsmsx103.gar.corp.intel.com ([10.221.44.82]) by fmsmga002.fm.intel.com with ESMTP; 02 Feb 2015 18:35:29 -0800 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by PGSMSX103.gar.corp.intel.com (10.221.44.82) with Microsoft SMTP Server (TLS) id 14.3.195.1; Tue, 3 Feb 2015 10:35:19 +0800 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.253]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.168]) with mapi id 14.03.0195.001; Tue, 3 Feb 2015 10:35:19 +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 02:35:17 +0000 Message-ID: <533710CFB86FA344BFBF2D6802E60286CD3AE6@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> 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 02:35:34 -0000 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= > The function is based on pci_probe_all_drivers. But it can not only=0A= > probe but also close drivers.=0A= > - Add pci_close_all_drivers()=0A= > The function tries to find a driver for the specified device, and=0A= > then close the driver.=0A= > - Add rte_eal_pci_probe_one() and rte_eal_pci_close_one()=0A= > The functions are used for probe and close a device.=0A= > First the function tries to find a device that has the specfied=0A= > PCI address. Then, probe or close the device.=0A= >=0A= > v5:=0A= > - Remove RTE_EAL_INVOKE_TYPE_UNKNOWN, because it's unused.=0A= > v4:=0A= > - Fix paramerter checking.=0A= > - Fix indent of 'if' statement.=0A= >=0A= > Signed-off-by: Tetsuya Mukawa =0A= > ---=0A= > lib/librte_eal/common/eal_common_pci.c | 90 +++++++++++++++++++++++++++= ++----=0A= > lib/librte_eal/common/eal_private.h | 24 +++++++++=0A= > lib/librte_eal/common/include/rte_pci.h | 33 ++++++++++++=0A= > lib/librte_eal/linuxapp/eal/eal_pci.c | 69 +++++++++++++++++++++++++= =0A= > 4 files changed, 206 insertions(+), 10 deletions(-)=0A= >=0A= > diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/comm= on/eal_common_pci.c=0A= > index a89f5c3..7c9b8c5 100644=0A= > --- a/lib/librte_eal/common/eal_common_pci.c=0A= > +++ b/lib/librte_eal/common/eal_common_pci.c=0A= > @@ -99,19 +99,27 @@ static struct rte_devargs *pci_devargs_lookup(struct = rte_pci_device *dev)=0A= > return NULL;=0A= > }=0A= > =0A= > -/*=0A= > - * If vendor/device ID match, call the devinit() function of all=0A= > - * registered driver for the given device. Return -1 if initialization= =0A= > - * failed, return 1 if no driver is found for this device.=0A= > - */=0A= > static int=0A= > -pci_probe_all_drivers(struct rte_pci_device *dev)=0A= > +pci_invoke_all_drivers(struct rte_pci_device *dev,=0A= > + enum rte_eal_invoke_type type)=0A= > {=0A= > struct rte_pci_driver *dr =3D NULL;=0A= > - int rc;=0A= > + int rc =3D 0;=0A= > +=0A= > + if ((dev =3D=3D NULL) || (type >=3D RTE_EAL_INVOKE_TYPE_MAX))=0A= > + return -1;=0A= > =0A= > TAILQ_FOREACH(dr, &pci_driver_list, next) {=0A= > - rc =3D rte_eal_pci_probe_one_driver(dr, dev);=0A= > + switch (type) {=0A= > + case RTE_EAL_INVOKE_TYPE_PROBE:=0A= > + rc =3D rte_eal_pci_probe_one_driver(dr, dev);=0A= > + break;=0A= > + case RTE_EAL_INVOKE_TYPE_CLOSE:=0A= > + rc =3D rte_eal_pci_close_one_driver(dr, dev);=0A= > + break;=0A= > + default:=0A= > + return -1;=0A= > + }=0A= > if (rc < 0)=0A= > /* negative value is an error */=0A= > return -1;=0A= > @@ -123,6 +131,66 @@ pci_probe_all_drivers(struct rte_pci_device *dev)=0A= > return 1;=0A= > }=0A= > =0A= > +#ifdef ENABLE_HOTPLUG=0A= > +static int=0A= > +rte_eal_pci_invoke_one(struct rte_pci_addr *addr,=0A= > + enum rte_eal_invoke_type type)=0A= > +{=0A= > + struct rte_pci_device *dev =3D NULL;=0A= > + int ret =3D 0;=0A= > +=0A= > + if ((addr =3D=3D NULL) || (type >=3D RTE_EAL_INVOKE_TYPE_MAX))=0A= > + return -1;=0A= > +=0A= > + TAILQ_FOREACH(dev, &pci_device_list, next) {=0A= > + if (eal_compare_pci_addr(&dev->addr, addr))=0A= > + continue;=0A= > +=0A= > + ret =3D pci_invoke_all_drivers(dev, type);=0A= > + if (ret < 0)=0A= > + goto invoke_err_return;=0A= > +=0A= > + if (type =3D=3D RTE_EAL_INVOKE_TYPE_CLOSE)=0A= > + goto remove_dev;=0A= > +=0A= > + return 0;=0A= > + }=0A= > +=0A= > + return -1;=0A= > +=0A= > +invoke_err_return:=0A= > + RTE_LOG(WARNING, EAL, "Requested device " PCI_PRI_FMT=0A= > + " cannot be used\n", dev->addr.domain, dev->addr.bus,=0A= > + dev->addr.devid, dev->addr.function);=0A= > + return -1;=0A= > +=0A= > +remove_dev:=0A= > + TAILQ_REMOVE(&pci_device_list, dev, next);=0A= > + return 0;=0A= > +}=0A= > +=0A= > +=0A= > +/*=0A= > + * Find the pci device specified by pci address, then invoke probe funct= ion of=0A= > + * the driver of the devive.=0A= > + */=0A= > +int=0A= > +rte_eal_pci_probe_one(struct rte_pci_addr *addr)=0A= > +{=0A= > + return rte_eal_pci_invoke_one(addr, RTE_EAL_INVOKE_TYPE_PROBE);=0A= > +}=0A= > +=0A= > +/*=0A= > + * Find the pci device specified by pci address, then invoke close funct= ion of=0A= > + * the driver of the devive.=0A= > + */=0A= > +int=0A= > +rte_eal_pci_close_one(struct rte_pci_addr *addr)=0A= > +{=0A= > + return rte_eal_pci_invoke_one(addr, RTE_EAL_INVOKE_TYPE_CLOSE);=0A= > +}=0A= > +#endif /* ENABLE_HOTPLUG */=0A= > +=0A= > /*=0A= > * Scan the content of the PCI bus, and call the devinit() function for= =0A= > * all registered drivers that have a matching entry in its id_table=0A= > @@ -148,10 +216,12 @@ rte_eal_pci_probe(void)=0A= > =0A= > /* probe all or only whitelisted devices */=0A= > if (probe_all)=0A= > - ret =3D pci_probe_all_drivers(dev);=0A= > + ret =3D pci_invoke_all_drivers(dev,=0A= > + RTE_EAL_INVOKE_TYPE_PROBE);=0A= > else if (devargs !=3D NULL &&=0A= > devargs->type =3D=3D RTE_DEVTYPE_WHITELISTED_PCI)=0A= > - ret =3D pci_probe_all_drivers(dev);=0A= > + ret =3D pci_invoke_all_drivers(dev,=0A= > + RTE_EAL_INVOKE_TYPE_PROBE);=0A= > if (ret < 0)=0A= > rte_exit(EXIT_FAILURE, "Requested device " PCI_PRI_FMT=0A= > " cannot be used\n", dev->addr.domain, dev->addr.bus,=0A= > diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/= eal_private.h=0A= > index 159cd66..1a362ab 100644=0A= > --- a/lib/librte_eal/common/eal_private.h=0A= > +++ b/lib/librte_eal/common/eal_private.h=0A= > @@ -154,6 +154,15 @@ struct rte_pci_driver;=0A= > struct rte_pci_device;=0A= > =0A= > /**=0A= > + * The invoke type.=0A= > + */=0A= > +enum rte_eal_invoke_type {=0A= > + RTE_EAL_INVOKE_TYPE_PROBE, /**< invoke probe function */=0A= > + RTE_EAL_INVOKE_TYPE_CLOSE, /**< invoke close function */=0A= > + RTE_EAL_INVOKE_TYPE_MAX /**< max value of this enum */=0A= > +};=0A= > +=0A= > +/**=0A= > * Mmap memory for single PCI device=0A= > *=0A= > * This function is private to EAL.=0A= > @@ -165,6 +174,21 @@ int rte_eal_pci_probe_one_driver(struct rte_pci_driv= er *dr,=0A= > struct rte_pci_device *dev);=0A= > =0A= > /**=0A= > + * Munmap memory for single PCI device=0A= > + *=0A= > + * This function is private to EAL.=0A= > + *=0A= > + * @param dr=0A= > + * The pointer to the pci driver structure=0A= > + * @param dev=0A= > + * The pointer to the pci device structure=0A= > + * @return=0A= > + * 0 on success, negative on error=0A= > + */=0A= > +int rte_eal_pci_close_one_driver(struct rte_pci_driver *dr,=0A= > + struct rte_pci_device *dev);=0A= > +=0A= > +/**=0A= > * Init tail queues for non-EAL library structures. This is to allow=0A= > * the rings, mempools, etc. lists to be shared among multiple processes= =0A= > *=0A= > diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/com= mon/include/rte_pci.h=0A= > index 87ca4cf..a111066 100644=0A= > --- a/lib/librte_eal/common/include/rte_pci.h=0A= > +++ b/lib/librte_eal/common/include/rte_pci.h=0A= > @@ -82,6 +82,7 @@ extern "C" {=0A= > #include =0A= > =0A= > #include =0A= > +#include =0A= > =0A= > TAILQ_HEAD(pci_device_list, rte_pci_device); /**< PCI devices in D-linke= d Q. */=0A= > TAILQ_HEAD(pci_driver_list, rte_pci_driver); /**< PCI drivers in D-linke= d Q. */=0A= > @@ -323,6 +324,38 @@ eal_compare_pci_addr(struct rte_pci_addr *addr, stru= ct rte_pci_addr *addr2)=0A= > */=0A= > int rte_eal_pci_probe(void);=0A= > =0A= > +#ifdef ENABLE_HOTPLUG=0A= > +/**=0A= > + * Probe the single PCI device.=0A= > + *=0A= > + * Scan the content of the PCI bus, and find the pci device specified by= pci=0A= > + * address, then call the probe() function for registered driver that ha= s a=0A= > + * matching entry in its id_table for discovered device.=0A= > + *=0A= > + * @param addr=0A= > + * The PCI Bus-Device-Function address to probe or close.=0A= > + * @return=0A= > + * - 0 on success.=0A= > + * - Negative on error.=0A= > + */=0A= > +int rte_eal_pci_probe_one(struct rte_pci_addr *addr);=0A= > +=0A= > +/**=0A= > + * Close the single PCI device.=0A= > + *=0A= > + * Scan the content of the PCI bus, and find the pci device specified by= pci=0A= > + * address, then call the close() function for registered driver that ha= s a=0A= > + * matching entry in its id_table for discovered device.=0A= > + *=0A= > + * @param addr=0A= > + * The PCI Bus-Device-Function address to probe or close.=0A= > + * @return=0A= > + * - 0 on success.=0A= > + * - Negative on error.=0A= > + */=0A= > +int rte_eal_pci_close_one(struct rte_pci_addr *addr);=0A= > +#endif /* ENABLE_HOTPLUG */=0A= > +=0A= > /**=0A= > * Dump the content of the PCI bus.=0A= > *=0A= > diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linux= app/eal/eal_pci.c=0A= > index c3b7917..831422e 100644=0A= > --- a/lib/librte_eal/linuxapp/eal/eal_pci.c=0A= > +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c=0A= > @@ -682,6 +682,75 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *= dr, struct rte_pci_device *d=0A= > return 1;=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= =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= =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=