From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 2C7E85934 for ; Thu, 22 Jan 2015 09:12:42 +0100 (CET) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga102.jf.intel.com with ESMTP; 22 Jan 2015 00:09:41 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,447,1418112000"; d="scan'208";a="515852274" Received: from pgsmsx101.gar.corp.intel.com ([10.221.44.78]) by orsmga003.jf.intel.com with ESMTP; 22 Jan 2015 00:05:46 -0800 Received: from kmsmsx154.gar.corp.intel.com (172.21.73.14) by PGSMSX101.gar.corp.intel.com (10.221.44.78) with Microsoft SMTP Server (TLS) id 14.3.195.1; Thu, 22 Jan 2015 16:12:29 +0800 Received: from shsmsx104.ccr.corp.intel.com (10.239.110.15) by KMSMSX154.gar.corp.intel.com (172.21.73.14) with Microsoft SMTP Server (TLS) id 14.3.195.1; Thu, 22 Jan 2015 16:12:27 +0800 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.64]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.231]) with mapi id 14.03.0195.001; Thu, 22 Jan 2015 16:12:27 +0800 From: "Qiu, Michael" To: Tetsuya Mukawa Thread-Topic: [dpdk-dev] [PATCH v4 06/11] eal/linux/pci: Add functions for unmapping igb_uio resources Thread-Index: AQHQNWFAXN6+QKM0HkSGXiCxS7ko7w== Date: Thu, 22 Jan 2015 08:12:25 +0000 Message-ID: <533710CFB86FA344BFBF2D6802E60286CB882E@SHSMSX101.ccr.corp.intel.com> References: <1418106629-22227-2-git-send-email-mukawa@igel.co.j> <1421664027-17971-1-git-send-email-mukawa@igel.co.jp> <1421664027-17971-7-git-send-email-mukawa@igel.co.jp> <533710CFB86FA344BFBF2D6802E60286CB7D11@SHSMSX101.ccr.corp.intel.com> <54BF78EF.7060304@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 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v4 06/11] eal/linux/pci: Add functions for unmapping igb_uio resources 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: Thu, 22 Jan 2015 08:12:43 -0000 On 1/21/2015 6:01 PM, Tetsuya Mukawa wrote:=0A= > Hi Michael,=0A= >=0A= > On 2015/01/20 18:23, Qiu, Michael wrote:=0A= >> On 1/19/2015 6:42 PM, Tetsuya Mukawa wrote:=0A= >>> The patch adds functions for unmapping igb_uio resources. The patch is = only=0A= >>> for Linux and igb_uio environment. VFIO and BSD are not supported.=0A= >>>=0A= >>> v4:=0A= >>> - Add paramerter checking.=0A= >>> - Add header file to determine if hotplug can be enabled.=0A= >>>=0A= >>> Signed-off-by: Tetsuya Mukawa =0A= >>> ---=0A= >>> lib/librte_eal/common/Makefile | 1 +=0A= >>> lib/librte_eal/common/include/rte_dev_hotplug.h | 44 +++++++++++++++++= =0A= >>> lib/librte_eal/linuxapp/eal/eal_pci.c | 38 +++++++++++++++= =0A= >>> lib/librte_eal/linuxapp/eal/eal_pci_init.h | 8 +++=0A= >>> lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 65 +++++++++++++++++= ++++++++=0A= >>> 5 files changed, 156 insertions(+)=0A= >>> create mode 100644 lib/librte_eal/common/include/rte_dev_hotplug.h=0A= >>>=0A= >>> diff --git a/lib/librte_eal/common/Makefile b/lib/librte_eal/common/Mak= efile=0A= >>> index 52c1a5f..db7cc93 100644=0A= >>> --- a/lib/librte_eal/common/Makefile=0A= >>> +++ b/lib/librte_eal/common/Makefile=0A= >>> @@ -41,6 +41,7 @@ INC +=3D rte_eal_memconfig.h rte_malloc_heap.h=0A= >>> INC +=3D rte_hexdump.h rte_devargs.h rte_dev.h=0A= >>> INC +=3D rte_common_vect.h=0A= >>> INC +=3D rte_pci_dev_feature_defs.h rte_pci_dev_features.h=0A= >>> +INC +=3D rte_dev_hotplug.h=0A= >>> =0A= >>> ifeq ($(CONFIG_RTE_INSECURE_FUNCTION_WARNING),y)=0A= >>> INC +=3D rte_warnings.h=0A= >>> diff --git a/lib/librte_eal/common/include/rte_dev_hotplug.h b/lib/libr= te_eal/common/include/rte_dev_hotplug.h=0A= >>> new file mode 100644=0A= >>> index 0000000..b333e0f=0A= >>> --- /dev/null=0A= >>> +++ b/lib/librte_eal/common/include/rte_dev_hotplug.h=0A= >>> @@ -0,0 +1,44 @@=0A= >>> +/*-=0A= >>> + * BSD LICENSE=0A= >>> + *=0A= >>> + * Copyright(c) 2015 IGEL Co.,LTd.=0A= >>> + * All rights reserved.=0A= >>> + *=0A= >>> + * Redistribution and use in source and binary forms, with or withou= t=0A= >>> + * modification, are permitted provided that the following condition= s=0A= >>> + * are met:=0A= >>> + *=0A= >>> + * * Redistributions of source code must retain the above copyrigh= t=0A= >>> + * notice, this list of conditions and the following disclaimer.= =0A= >>> + * * Redistributions in binary form must reproduce the above copyr= ight=0A= >>> + * notice, this list of conditions and the following disclaimer = in=0A= >>> + * the documentation and/or other materials provided with the=0A= >>> + * distribution.=0A= >>> + * * Neither the name of IGEL Co.,Ltd. nor the names of its=0A= >>> + * contributors may be used to endorse or promote products deriv= ed=0A= >>> + * from this software without specific prior written permission.= =0A= >>> + *=0A= >>> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTO= RS=0A= >>> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT= =0A= >>> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS= FOR=0A= >>> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRI= GHT=0A= >>> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDEN= TAL,=0A= >>> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT= =0A= >>> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF = USE,=0A= >>> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON= ANY=0A= >>> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TO= RT=0A= >>> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE= USE=0A= >>> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMA= GE.=0A= >>> + */=0A= >>> +=0A= >>> +#ifndef _RTE_DEV_HOTPLUG_H_=0A= >>> +#define _RTE_DEV_HOTPLUG_H_=0A= >>> +=0A= >>> +/*=0A= >>> + * determine if hotplug can be enabled on the system=0A= >>> + */=0A= >>> +#if defined(RTE_LIBRTE_EAL_HOTPLUG) && defined(RTE_LIBRTE_EAL_LINUXAPP= )=0A= >> As you said, VFIO should not work with it, so does it need to add the=0A= >> vfio check here?=0A= > Could I have a advice of you?=0A= > First I guess it's the best to include "eal_vfio.h" here, and add=0A= > checking of VFIO_PRESENT macro.=0A= =0A= =0A= I have a question, will your hotplug feature support freebsd ?=0A= =0A= If not, how about to put it in "lib/librte_eal/linuxapp/eal/" ? Also =0A= include attach or detach affairs.=0A= =0A= > But it seems I cannot reach "eal_vfio.h" from this file.=0A= =0A= Yes, you can't :)=0A= =0A= > My second option is just checking RTE_EAL_VFIO macro.=0A= > But according to "eal_vfio.h", if kernel is under 3.6.0, VFIO_PRESENT=0A= =0A= Actually, in my opinion, whatever vfio or uio, only need be care in=0A= runtime.=0A= =0A= DPDK to check vfio only to add support for vfio, but this does not=0A= means the device will use vfio,=0A= =0A= So even if VFIO_PRESENT is defined, and vfio is enabled, but the device=0A= is bind to igb_uio, then your hotplug still need work, but if it bind=0A= to vfio, will not, am I right?=0A= =0A= If yes, I'm not sure if your hotplug has this ability, but it is=0A= reasonable, I think.=0A= =0A= > will not be defined even when RTE_EAL_VFIO is enabled.=0A= > So I guess simply macro checking will not work correctly.=0A= > =0A= > Anyway, here are my implementation choices so far.=0A= >=0A= > 1. Like "eal_vfio.h", check kernel version in "rte_dev_hotplug.h".=0A= > In this case, if "eal_vfio.h" is changed, "rte_edv_hotplug.h" may need=0A= > to be changed also.=0A= >=0A= > 2. Merge "eal_vfio.h" and "rte_dev_hotplug.h" definitions, and define=0A= > these in new rte header like "rte_settings.h".=0A= >=0A= > Can I have advice about it?=0A= =0A= As I said, VFIO enable or not is not related for your hotplug, only the=0A= devices managed by VFIO will affect your hotplug.=0A= =0A= If you agree, I think need discuss the details of it.=0A= =0A= Thanks,=0A= Michael=0A= > Thanks,=0A= > Tetsuya=0A= >=0A= >> Thanks,=0A= >> Michael=0A= >>> +#define ENABLE_HOTPLUG=0A= >>> +#endif /* RTE_LIBRTE_EAL_HOTPLUG & RTE_LIBRTE_EAL_LINUXAPP */=0A= >>> +=0A= >>> +#endif /* _RTE_DEV_HOTPLUG_H_ */=0A= >>> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/lin= uxapp/eal/eal_pci.c=0A= >>> index 3d2d93c..52c464c 100644=0A= >>> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c=0A= >>> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c=0A= >>> @@ -137,6 +137,25 @@ pci_map_resource(void *requested_addr, int fd, off= _t offset, size_t size)=0A= >>> return mapaddr;=0A= >>> }=0A= >>> =0A= >>> +#ifdef ENABLE_HOTPLUG=0A= >>> +/* unmap a particular resource */=0A= >>> +void=0A= >>> +pci_unmap_resource(void *requested_addr, size_t size)=0A= >>> +{=0A= >>> + if (requested_addr =3D=3D NULL)=0A= >>> + return;=0A= >>> +=0A= >>> + /* Unmap the PCI memory resource of device */=0A= >>> + if (munmap(requested_addr, size)) {=0A= >>> + RTE_LOG(ERR, EAL, "%s(): cannot munmap(%p, 0x%lx): %s\n",=0A= >>> + __func__, requested_addr, (unsigned long)size,=0A= >>> + strerror(errno));=0A= >>> + } else=0A= >>> + RTE_LOG(DEBUG, EAL, " PCI memory mapped at %p\n",=0A= >>> + requested_addr);=0A= >>> +}=0A= >>> +#endif /* ENABLE_HOTPLUG */=0A= >>> +=0A= >>> /* parse the "resource" sysfs file */=0A= >>> #define IORESOURCE_MEM 0x00000200=0A= >>> =0A= >>> @@ -510,6 +529,25 @@ pci_map_device(struct rte_pci_device *dev)=0A= >>> return 0;=0A= >>> }=0A= >>> =0A= >>> +#ifdef ENABLE_HOTPLUG=0A= >>> +static void=0A= >>> +pci_unmap_device(struct rte_pci_device *dev)=0A= >>> +{=0A= >>> + if (dev =3D=3D NULL)=0A= >>> + return;=0A= >>> +=0A= >>> + /* try unmapping the NIC resources using VFIO if it exists */=0A= >>> +#ifdef VFIO_PRESENT=0A= >>> + if (pci_vfio_is_enabled()) {=0A= >>> + RTE_LOG(ERR, EAL, "%s() doesn't support vfio yet.\n",=0A= >>> + __func__);=0A= >>> + return;=0A= >>> + }=0A= >>> +#endif=0A= >>> + pci_uio_unmap_resource(dev);=0A= >>> +}=0A= >>> +#endif /* ENABLE_HOTPLUG */=0A= >>> +=0A= >>> /*=0A= >>> * If vendor/device ID match, call the devinit() function of the=0A= >>> * driver.=0A= >>> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_init.h b/lib/librte_ea= l/linuxapp/eal/eal_pci_init.h=0A= >>> index 1070eb8..5152a0b 100644=0A= >>> --- a/lib/librte_eal/linuxapp/eal/eal_pci_init.h=0A= >>> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_init.h=0A= >>> @@ -34,6 +34,7 @@=0A= >>> #ifndef EAL_PCI_INIT_H_=0A= >>> #define EAL_PCI_INIT_H_=0A= >>> =0A= >>> +#include =0A= >>> #include "eal_vfio.h"=0A= >>> =0A= >>> struct pci_map {=0A= >>> @@ -71,6 +72,13 @@ void *pci_map_resource(void *requested_addr, int fd,= off_t offset,=0A= >>> /* map IGB_UIO resource prototype */=0A= >>> int pci_uio_map_resource(struct rte_pci_device *dev);=0A= >>> =0A= >>> +#ifdef ENABLE_HOTPLUG=0A= >>> +void pci_unmap_resource(void *requested_addr, size_t size);=0A= >>> +=0A= >>> +/* unmap IGB_UIO resource prototype */=0A= >>> +void pci_uio_unmap_resource(struct rte_pci_device *dev);=0A= >>> +#endif /* ENABLE_HOTPLUG */=0A= >>> +=0A= >>> #ifdef VFIO_PRESENT=0A= >>> =0A= >>> #define VFIO_MAX_GROUPS 64=0A= >>> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal= /linuxapp/eal/eal_pci_uio.c=0A= >>> index 1da3507..81830d1 100644=0A= >>> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c=0A= >>> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c=0A= >>> @@ -404,6 +404,71 @@ pci_uio_map_resource(struct rte_pci_device *dev)= =0A= >>> return 0;=0A= >>> }=0A= >>> =0A= >>> +#ifdef ENABLE_HOTPLUG=0A= >>> +static void=0A= >>> +pci_uio_unmap(struct mapped_pci_resource *uio_res)=0A= >>> +{=0A= >>> + int i;=0A= >>> +=0A= >>> + if (uio_res =3D=3D NULL)=0A= >>> + return;=0A= >>> +=0A= >>> + for (i =3D 0; i !=3D uio_res->nb_maps; i++)=0A= >>> + pci_unmap_resource(uio_res->maps[i].addr,=0A= >>> + (size_t)uio_res->maps[i].size);=0A= >>> +}=0A= >>> +=0A= >>> +static struct mapped_pci_resource *=0A= >>> +pci_uio_find_resource(struct rte_pci_device *dev)=0A= >>> +{=0A= >>> + struct mapped_pci_resource *uio_res;=0A= >>> +=0A= >>> + if (dev =3D=3D NULL)=0A= >>> + return NULL;=0A= >>> +=0A= >>> + TAILQ_FOREACH(uio_res, pci_res_list, next) {=0A= >>> +=0A= >>> + /* skip this element if it doesn't match our PCI address */=0A= >>> + if (!eal_compare_pci_addr(&uio_res->pci_addr, &dev->addr))=0A= >>> + return uio_res;=0A= >>> + }=0A= >>> + return NULL;=0A= >>> +}=0A= >>> +=0A= >>> +/* unmap the PCI resource of a PCI device in virtual memory */=0A= >>> +void=0A= >>> +pci_uio_unmap_resource(struct rte_pci_device *dev)=0A= >>> +{=0A= >>> + struct mapped_pci_resource *uio_res;=0A= >>> +=0A= >>> + if (dev =3D=3D NULL)=0A= >>> + return;=0A= >>> +=0A= >>> + /* find an entry for the device */=0A= >>> + uio_res =3D pci_uio_find_resource(dev);=0A= >>> + if (uio_res =3D=3D NULL)=0A= >>> + return;=0A= >>> +=0A= >>> + /* secondary processes - just free maps */=0A= >>> + if (rte_eal_process_type() !=3D RTE_PROC_PRIMARY)=0A= >>> + return pci_uio_unmap(uio_res);=0A= >>> +=0A= >>> + TAILQ_REMOVE(pci_res_list, uio_res, next);=0A= >>> +=0A= >>> + /* unmap all resources */=0A= >>> + pci_uio_unmap(uio_res);=0A= >>> +=0A= >>> + /* free uio resource */=0A= >>> + rte_free(uio_res);=0A= >>> +=0A= >>> + /* close fd if in primary process */=0A= >>> + close(dev->intr_handle.fd);=0A= >>> +=0A= >>> + dev->intr_handle.fd =3D -1;=0A= >>> + dev->intr_handle.type =3D RTE_INTR_HANDLE_UNKNOWN;=0A= >>> +}=0A= >>> +#endif /* ENABLE_HOTPLUG */=0A= >>> +=0A= >>> /*=0A= >>> * parse a sysfs file containing one integer value=0A= >>> * different to the eal version, as it needs to work with 64-bit value= s=0A= >=0A= >=0A= =0A=