From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f46.google.com (mail-pa0-f46.google.com [209.85.220.46]) by dpdk.org (Postfix) with ESMTP id 66316559C for ; Tue, 3 Feb 2015 05:07:32 +0100 (CET) Received: by mail-pa0-f46.google.com with SMTP id lj1so90810024pab.5 for ; Mon, 02 Feb 2015 20:07:31 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=LrTabgRse7q54d26zfslsWBCcJcMvdWtcPNtfUEAxwk=; b=Vo7VIxNRR82zpSZo/XifrrbC4mxzeTDwvPJWl6hHOFatOMIuw+oGHoQPesjcymWffh X7t3APvWs+r1yRTPsmY0xlqtSWm5remqJfxDz1psx32nvp6L89i1NWC6jHm5TGfnMgA6 ikJNSqczoHFIqk7LzRnOcuIOXi6plG5tvZ8yfrOFWsTX4z/U1ERzMIPMPqDm0wzj04Oj vw8krK19Wo3fw+FNb3FBLJTo8y5LmSPRIGhMsOIHv8x/QuQ2Knj/F20wm0hZRR3QakrR vr3J9+mrZQiGzgAiFM44ln7qHNyCmGFz3Vq5L5nz33wq/DuKnzjTIoJ4z28m3U48d2ns Fjtw== X-Gm-Message-State: ALoCoQm1/zY7BUybEOSe067Z2btPFXW8WZXubBDJFi5f5FPH55w9MZiFAwRPHo7qnaumK8n4dFM6 X-Received: by 10.66.157.5 with SMTP id wi5mr34070987pab.37.1422936451355; Mon, 02 Feb 2015 20:07:31 -0800 (PST) Received: from [10.16.129.101] (napt.igel.co.jp. [219.106.231.132]) by mx.google.com with ESMTPSA id t7sm532348pbs.90.2015.02.02.20.07.29 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 02 Feb 2015 20:07:30 -0800 (PST) Message-ID: <54D0497F.9080206@igel.co.jp> Date: Tue, 03 Feb 2015 13:07:27 +0900 From: Tetsuya Mukawa User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: "Qiu, Michael" , "dev@dpdk.org" 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> In-Reply-To: <533710CFB86FA344BFBF2D6802E60286CD3AE6@SHSMSX101.ccr.corp.intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable 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 04:07:33 -0000 On 2015/02/03 11:35, Qiu, Michael wrote: > On 2/1/2015 12:02 PM, Tetsuya Mukawa wrote: >> - Add rte_eal_pci_close_one_dirver() >> The function is used for closing the specified driver and device. >> - Add pci_invoke_all_drivers() >> The function is based on pci_probe_all_drivers. But it can not only >> probe but also close drivers. >> - Add pci_close_all_drivers() >> The function tries to find a driver for the specified device, and >> then close the driver. >> - Add rte_eal_pci_probe_one() and rte_eal_pci_close_one() >> The functions are used for probe and close a device. >> First the function tries to find a device that has the specfied >> PCI address. Then, probe or close the device. >> >> v5: >> - Remove RTE_EAL_INVOKE_TYPE_UNKNOWN, because it's unused. >> v4: >> - Fix paramerter checking. >> - Fix indent of 'if' statement. >> >> Signed-off-by: Tetsuya Mukawa >> --- >> lib/librte_eal/common/eal_common_pci.c | 90 ++++++++++++++++++++++++= +++++---- >> lib/librte_eal/common/eal_private.h | 24 +++++++++ >> lib/librte_eal/common/include/rte_pci.h | 33 ++++++++++++ >> lib/librte_eal/linuxapp/eal/eal_pci.c | 69 ++++++++++++++++++++++++= + >> 4 files changed, 206 insertions(+), 10 deletions(-) >> >> diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/c= ommon/eal_common_pci.c >> index a89f5c3..7c9b8c5 100644 >> --- a/lib/librte_eal/common/eal_common_pci.c >> +++ b/lib/librte_eal/common/eal_common_pci.c >> @@ -99,19 +99,27 @@ static struct rte_devargs *pci_devargs_lookup(stru= ct rte_pci_device *dev) >> return NULL; >> } >> =20 >> -/* >> - * If vendor/device ID match, call the devinit() function of all >> - * registered driver for the given device. Return -1 if initializatio= n >> - * failed, return 1 if no driver is found for this device. >> - */ >> static int >> -pci_probe_all_drivers(struct rte_pci_device *dev) >> +pci_invoke_all_drivers(struct rte_pci_device *dev, >> + enum rte_eal_invoke_type type) >> { >> struct rte_pci_driver *dr =3D NULL; >> - int rc; >> + int rc =3D 0; >> + >> + if ((dev =3D=3D NULL) || (type >=3D RTE_EAL_INVOKE_TYPE_MAX)) >> + return -1; >> =20 >> TAILQ_FOREACH(dr, &pci_driver_list, next) { >> - rc =3D rte_eal_pci_probe_one_driver(dr, dev); >> + switch (type) { >> + case RTE_EAL_INVOKE_TYPE_PROBE: >> + rc =3D rte_eal_pci_probe_one_driver(dr, dev); >> + break; >> + case RTE_EAL_INVOKE_TYPE_CLOSE: >> + rc =3D rte_eal_pci_close_one_driver(dr, dev); >> + break; >> + default: >> + return -1; >> + } >> if (rc < 0) >> /* negative value is an error */ >> return -1; >> @@ -123,6 +131,66 @@ pci_probe_all_drivers(struct rte_pci_device *dev)= >> return 1; >> } >> =20 >> +#ifdef ENABLE_HOTPLUG >> +static int >> +rte_eal_pci_invoke_one(struct rte_pci_addr *addr, >> + enum rte_eal_invoke_type type) >> +{ >> + struct rte_pci_device *dev =3D NULL; >> + int ret =3D 0; >> + >> + if ((addr =3D=3D NULL) || (type >=3D RTE_EAL_INVOKE_TYPE_MAX)) >> + return -1; >> + >> + TAILQ_FOREACH(dev, &pci_device_list, next) { >> + if (eal_compare_pci_addr(&dev->addr, addr)) >> + continue; >> + >> + ret =3D pci_invoke_all_drivers(dev, type); >> + if (ret < 0) >> + goto invoke_err_return; >> + >> + if (type =3D=3D RTE_EAL_INVOKE_TYPE_CLOSE) >> + goto remove_dev; >> + >> + return 0; >> + } >> + >> + return -1; >> + >> +invoke_err_return: >> + RTE_LOG(WARNING, EAL, "Requested device " PCI_PRI_FMT >> + " cannot be used\n", dev->addr.domain, dev->addr.bus, >> + dev->addr.devid, dev->addr.function); >> + return -1; >> + >> +remove_dev: >> + TAILQ_REMOVE(&pci_device_list, dev, next); >> + return 0; >> +} >> + >> + >> +/* >> + * Find the pci device specified by pci address, then invoke probe fu= nction of >> + * the driver of the devive. >> + */ >> +int >> +rte_eal_pci_probe_one(struct rte_pci_addr *addr) >> +{ >> + return rte_eal_pci_invoke_one(addr, RTE_EAL_INVOKE_TYPE_PROBE); >> +} >> + >> +/* >> + * Find the pci device specified by pci address, then invoke close fu= nction of >> + * the driver of the devive. >> + */ >> +int >> +rte_eal_pci_close_one(struct rte_pci_addr *addr) >> +{ >> + return rte_eal_pci_invoke_one(addr, RTE_EAL_INVOKE_TYPE_CLOSE); >> +} >> +#endif /* ENABLE_HOTPLUG */ >> + >> /* >> * Scan the content of the PCI bus, and call the devinit() function f= or >> * all registered drivers that have a matching entry in its id_table >> @@ -148,10 +216,12 @@ rte_eal_pci_probe(void) >> =20 >> /* probe all or only whitelisted devices */ >> if (probe_all) >> - ret =3D pci_probe_all_drivers(dev); >> + ret =3D pci_invoke_all_drivers(dev, >> + RTE_EAL_INVOKE_TYPE_PROBE); >> else if (devargs !=3D NULL && >> devargs->type =3D=3D RTE_DEVTYPE_WHITELISTED_PCI) >> - ret =3D pci_probe_all_drivers(dev); >> + ret =3D pci_invoke_all_drivers(dev, >> + RTE_EAL_INVOKE_TYPE_PROBE); >> if (ret < 0) >> rte_exit(EXIT_FAILURE, "Requested device " PCI_PRI_FMT >> " cannot be used\n", dev->addr.domain, dev->addr.bus, >> diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/comm= on/eal_private.h >> index 159cd66..1a362ab 100644 >> --- a/lib/librte_eal/common/eal_private.h >> +++ b/lib/librte_eal/common/eal_private.h >> @@ -154,6 +154,15 @@ struct rte_pci_driver; >> struct rte_pci_device; >> =20 >> /** >> + * The invoke type. >> + */ >> +enum rte_eal_invoke_type { >> + RTE_EAL_INVOKE_TYPE_PROBE, /**< invoke probe function */ >> + RTE_EAL_INVOKE_TYPE_CLOSE, /**< invoke close function */ >> + RTE_EAL_INVOKE_TYPE_MAX /**< max value of this enum */ >> +}; >> + >> +/** >> * Mmap memory for single PCI device >> * >> * This function is private to EAL. >> @@ -165,6 +174,21 @@ int rte_eal_pci_probe_one_driver(struct rte_pci_d= river *dr, >> struct rte_pci_device *dev); >> =20 >> /** >> + * Munmap memory for single PCI device >> + * >> + * This function is private to EAL. >> + * >> + * @param dr >> + * The pointer to the pci driver structure >> + * @param dev >> + * The pointer to the pci device structure >> + * @return >> + * 0 on success, negative on error >> + */ >> +int rte_eal_pci_close_one_driver(struct rte_pci_driver *dr, >> + struct rte_pci_device *dev); >> + >> +/** >> * Init tail queues for non-EAL library structures. This is to allow >> * the rings, mempools, etc. lists to be shared among multiple proces= ses >> * >> diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/= common/include/rte_pci.h >> index 87ca4cf..a111066 100644 >> --- a/lib/librte_eal/common/include/rte_pci.h >> +++ b/lib/librte_eal/common/include/rte_pci.h >> @@ -82,6 +82,7 @@ extern "C" { >> #include >> =20 >> #include >> +#include >> =20 >> TAILQ_HEAD(pci_device_list, rte_pci_device); /**< PCI devices in D-li= nked Q. */ >> TAILQ_HEAD(pci_driver_list, rte_pci_driver); /**< PCI drivers in D-li= nked Q. */ >> @@ -323,6 +324,38 @@ eal_compare_pci_addr(struct rte_pci_addr *addr, s= truct rte_pci_addr *addr2) >> */ >> int rte_eal_pci_probe(void); >> =20 >> +#ifdef ENABLE_HOTPLUG >> +/** >> + * Probe the single PCI device. >> + * >> + * Scan the content of the PCI bus, and find the pci device specified= by pci >> + * address, then call the probe() function for registered driver that= has a >> + * matching entry in its id_table for discovered device. >> + * >> + * @param addr >> + * The PCI Bus-Device-Function address to probe or close. >> + * @return >> + * - 0 on success. >> + * - Negative on error. >> + */ >> +int rte_eal_pci_probe_one(struct rte_pci_addr *addr); >> + >> +/** >> + * Close the single PCI device. >> + * >> + * Scan the content of the PCI bus, and find the pci device specified= by pci >> + * address, then call the close() function for registered driver that= has a >> + * matching entry in its id_table for discovered device. >> + * >> + * @param addr >> + * The PCI Bus-Device-Function address to probe or close. >> + * @return >> + * - 0 on success. >> + * - Negative on error. >> + */ >> +int rte_eal_pci_close_one(struct rte_pci_addr *addr); >> +#endif /* ENABLE_HOTPLUG */ >> + >> /** >> * Dump the content of the PCI bus. >> * >> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/li= nuxapp/eal/eal_pci.c >> index c3b7917..831422e 100644 >> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c >> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c >> @@ -682,6 +682,75 @@ rte_eal_pci_probe_one_driver(struct rte_pci_drive= r *dr, struct rte_pci_device *d >> return 1; >> } >> =20 >> +#ifdef ENABLE_HOTPLUG >> +/* >> + * If vendor/device ID match, call the devuninit() function of the >> + * driver. >> + */ >> +int >> +rte_eal_pci_close_one_driver(struct rte_pci_driver *dr, >> + struct rte_pci_device *dev) >> +{ >> + struct rte_pci_id *id_table; >> + >> + if ((dr =3D=3D NULL) || (dev =3D=3D NULL)) >> + return -EINVAL; >> + >> + for (id_table =3D dr->id_table ; id_table->vendor_id !=3D 0; id_tabl= e++) { >> + >> + /* check if device's identifiers match the driver's ones */ >> + if (id_table->vendor_id !=3D dev->id.vendor_id && >> + id_table->vendor_id !=3D PCI_ANY_ID) >> + continue; >> + if (id_table->device_id !=3D dev->id.device_id && >> + id_table->device_id !=3D PCI_ANY_ID) >> + continue; >> + if (id_table->subsystem_vendor_id !=3D >> + dev->id.subsystem_vendor_id && >> + id_table->subsystem_vendor_id !=3D PCI_ANY_ID) >> + continue; >> + if (id_table->subsystem_device_id !=3D >> + dev->id.subsystem_device_id && >> + id_table->subsystem_device_id !=3D PCI_ANY_ID) >> + continue; >> + >> + struct rte_pci_addr *loc =3D &dev->addr; >> + >> + RTE_LOG(DEBUG, EAL, >> + "PCI device "PCI_PRI_FMT" on NUMA socket %i\n", >> + loc->domain, loc->bus, loc->devid, >> + loc->function, dev->numa_node); >> + >> + RTE_LOG(DEBUG, EAL, " remove driver: %x:%x %s\n", >> + dev->id.vendor_id, dev->id.device_id, >> + dr->name); >> + >> + /* call the driver devuninit() function */ >> + if (dr->devuninit && (dr->devuninit(dr, dev) < 0)) >> + return -1; /* negative value is an error */ >> + >> + /* clear driver structure */ >> + dev->driver =3D NULL; >> + >> + if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) >> + /* unmap resources for devices that use igb_uio */ >> + pci_unmap_device(dev); > Hi, Tetsuya > > I have one question, as the code shows, in pci_unmap_device(), will > check pt_driver. > > But assume that, we are now try to detach a vfio device, after print ou= t > a error message of unsupported, the does this port workable? > > I think this port will unworkable, am I right? > > But actually, we should keep it workable. > > My suggestion is to add a check in rte_eth_dev_check_detachable() for > pci_device port. Hi Michael, I appreciate your comment. In the function called "rte_eal_dev_detach_pdev()", "rte_eth_dev_check_detachable()" has been already checked. But in the future, someone may want to reuse "rte_eal_pci_close_one_driver()". So I will add the checking like your suggestion. Thanks, Tetsuya > > Thanks > Michael > >> + >> + return 0; >> + } >> + /* return positive value if driver is not found */ >> + return 1; >> +} >> +#else /* ENABLE_HOTPLUG */ >> +int >> +rte_eal_pci_close_one_driver(struct rte_pci_driver *dr __rte_unused, >> + struct rte_pci_device *dev __rte_unused) >> +{ >> + RTE_LOG(ERR, EAL, "Hotplug support isn't enabled\n"); >> + return -1; >> +} >> +#endif /* ENABLE_HOTPLUG */ >> + >> /* Init the PCI EAL subsystem */ >> int >> rte_eal_pci_init(void)