From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f52.google.com (mail-pa0-f52.google.com [209.85.220.52]) by dpdk.org (Postfix) with ESMTP id 2A9D4282 for ; Wed, 21 Jan 2015 11:01:23 +0100 (CET) Received: by mail-pa0-f52.google.com with SMTP id kx10so21290920pab.11 for ; Wed, 21 Jan 2015 02:01:22 -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=LqJrZ26nI5QhhyRgtV987LQUS0f/GSXU8Eyan1gM7BM=; b=ch7O2LOvHA2ObQvgJ3a2zCsd3qgV9a2RGV3/mqSYQhY6Mog5mIXNdkX9SXi32aQRmn ATq3JFYSbq85ayAvi7z8/wAGxas4dSrSr/JOVTXCFCQIQP8AzjtgGBDrqJ8ulExNcNTj 8Zqx+bx9HibDHUIclUZ5EcfBKEUO4XZPgVLcQO5znYAi/sD6W08/pmS6HmDyIgmeZHCr noSXiSN6Oi7x3Be8RWQ5JFp3KzX3s0yliMI4Bj9dgS0/wHLVxhWzQWZkBmkchLOX51na QWgGgpRYYOKTqRQ+OSvjhHP4UD2OzI5w63W2ZPeuUsQd4hMayC2I9fzj7A+n5sQyCIRv tLig== X-Gm-Message-State: ALoCoQlXl0vbg+nMsOfIq7JQLIM8aTAJZURFpExcp7dki+6TlaUP9WIyPsSZUw44i6aDPTwqqKcz X-Received: by 10.68.171.1 with SMTP id aq1mr45752252pbc.125.1421834482299; Wed, 21 Jan 2015 02:01:22 -0800 (PST) Received: from [10.16.129.101] (napt.igel.co.jp. [219.106.231.132]) by mx.google.com with ESMTPSA id am14sm5715755pac.35.2015.01.21.02.01.20 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 21 Jan 2015 02:01:21 -0800 (PST) Message-ID: <54BF78EF.7060304@igel.co.jp> Date: Wed, 21 Jan 2015 19:01:19 +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" 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> In-Reply-To: <533710CFB86FA344BFBF2D6802E60286CB7D11@SHSMSX101.ccr.corp.intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable 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: Wed, 21 Jan 2015 10:01:24 -0000 Hi Michael, On 2015/01/20 18:23, Qiu, Michael wrote: > On 1/19/2015 6:42 PM, Tetsuya Mukawa wrote: >> The patch adds functions for unmapping igb_uio resources. The patch is= only >> for Linux and igb_uio environment. VFIO and BSD are not supported. >> >> v4: >> - Add paramerter checking. >> - Add header file to determine if hotplug can be enabled. >> >> Signed-off-by: Tetsuya Mukawa >> --- >> lib/librte_eal/common/Makefile | 1 + >> lib/librte_eal/common/include/rte_dev_hotplug.h | 44 ++++++++++++++++= + >> lib/librte_eal/linuxapp/eal/eal_pci.c | 38 +++++++++++++++ >> lib/librte_eal/linuxapp/eal/eal_pci_init.h | 8 +++ >> lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 65 ++++++++++++++++= +++++++++ >> 5 files changed, 156 insertions(+) >> create mode 100644 lib/librte_eal/common/include/rte_dev_hotplug.h >> >> diff --git a/lib/librte_eal/common/Makefile b/lib/librte_eal/common/Ma= kefile >> index 52c1a5f..db7cc93 100644 >> --- a/lib/librte_eal/common/Makefile >> +++ b/lib/librte_eal/common/Makefile >> @@ -41,6 +41,7 @@ INC +=3D rte_eal_memconfig.h rte_malloc_heap.h >> INC +=3D rte_hexdump.h rte_devargs.h rte_dev.h >> INC +=3D rte_common_vect.h >> INC +=3D rte_pci_dev_feature_defs.h rte_pci_dev_features.h >> +INC +=3D rte_dev_hotplug.h >> =20 >> ifeq ($(CONFIG_RTE_INSECURE_FUNCTION_WARNING),y) >> INC +=3D rte_warnings.h >> diff --git a/lib/librte_eal/common/include/rte_dev_hotplug.h b/lib/lib= rte_eal/common/include/rte_dev_hotplug.h >> new file mode 100644 >> index 0000000..b333e0f >> --- /dev/null >> +++ b/lib/librte_eal/common/include/rte_dev_hotplug.h >> @@ -0,0 +1,44 @@ >> +/*- >> + * BSD LICENSE >> + * >> + * Copyright(c) 2015 IGEL Co.,LTd. >> + * All rights reserved. >> + * >> + * Redistribution and use in source and binary forms, with or witho= ut >> + * modification, are permitted provided that the following conditio= ns >> + * are met: >> + * >> + * * Redistributions of source code must retain the above copyrig= ht >> + * notice, this list of conditions and the following disclaimer= =2E >> + * * Redistributions in binary form must reproduce the above copy= right >> + * notice, this list of conditions and the following disclaimer= in >> + * the documentation and/or other materials provided with the >> + * distribution. >> + * * Neither the name of IGEL Co.,Ltd. nor the names of its >> + * contributors may be used to endorse or promote products deri= ved >> + * from this software without specific prior written permission= =2E >> + * >> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUT= ORS >> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NO= T >> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNES= S FOR >> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYR= IGHT >> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDE= NTAL, >> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT= >> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF= USE, >> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND O= N ANY >> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR T= ORT >> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF TH= E USE >> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAM= AGE. >> + */ >> + >> +#ifndef _RTE_DEV_HOTPLUG_H_ >> +#define _RTE_DEV_HOTPLUG_H_ >> + >> +/* >> + * determine if hotplug can be enabled on the system >> + */ >> +#if defined(RTE_LIBRTE_EAL_HOTPLUG) && defined(RTE_LIBRTE_EAL_LINUXAP= P) > As you said, VFIO should not work with it, so does it need to add the > vfio check here? Could I have a advice of you? First I guess it's the best to include "eal_vfio.h" here, and add checking of VFIO_PRESENT macro. But it seems I cannot reach "eal_vfio.h" from this file. My second option is just checking RTE_EAL_VFIO macro. But according to "eal_vfio.h", if kernel is under 3.6.0, VFIO_PRESENT will not be defined even when RTE_EAL_VFIO is enabled. So I guess simply macro checking will not work correctly. =20 Anyway, here are my implementation choices so far. 1. Like "eal_vfio.h", check kernel version in "rte_dev_hotplug.h". In this case, if "eal_vfio.h" is changed, "rte_edv_hotplug.h" may need to be changed also. 2. Merge "eal_vfio.h" and "rte_dev_hotplug.h" definitions, and define these in new rte header like "rte_settings.h". Can I have advice about it? Thanks, Tetsuya > > Thanks, > Michael >> +#define ENABLE_HOTPLUG >> +#endif /* RTE_LIBRTE_EAL_HOTPLUG & RTE_LIBRTE_EAL_LINUXAPP */ >> + >> +#endif /* _RTE_DEV_HOTPLUG_H_ */ >> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/li= nuxapp/eal/eal_pci.c >> index 3d2d93c..52c464c 100644 >> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c >> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c >> @@ -137,6 +137,25 @@ pci_map_resource(void *requested_addr, int fd, of= f_t offset, size_t size) >> return mapaddr; >> } >> =20 >> +#ifdef ENABLE_HOTPLUG >> +/* unmap a particular resource */ >> +void >> +pci_unmap_resource(void *requested_addr, size_t size) >> +{ >> + if (requested_addr =3D=3D NULL) >> + return; >> + >> + /* Unmap the PCI memory resource of device */ >> + if (munmap(requested_addr, size)) { >> + RTE_LOG(ERR, EAL, "%s(): cannot munmap(%p, 0x%lx): %s\n", >> + __func__, requested_addr, (unsigned long)size, >> + strerror(errno)); >> + } else >> + RTE_LOG(DEBUG, EAL, " PCI memory mapped at %p\n", >> + requested_addr); >> +} >> +#endif /* ENABLE_HOTPLUG */ >> + >> /* parse the "resource" sysfs file */ >> #define IORESOURCE_MEM 0x00000200 >> =20 >> @@ -510,6 +529,25 @@ pci_map_device(struct rte_pci_device *dev) >> return 0; >> } >> =20 >> +#ifdef ENABLE_HOTPLUG >> +static void >> +pci_unmap_device(struct rte_pci_device *dev) >> +{ >> + if (dev =3D=3D NULL) >> + return; >> + >> + /* try unmapping the NIC resources using VFIO if it exists */ >> +#ifdef VFIO_PRESENT >> + if (pci_vfio_is_enabled()) { >> + RTE_LOG(ERR, EAL, "%s() doesn't support vfio yet.\n", >> + __func__); >> + return; >> + } >> +#endif >> + pci_uio_unmap_resource(dev); >> +} >> +#endif /* ENABLE_HOTPLUG */ >> + >> /* >> * If vendor/device ID match, call the devinit() function of the >> * driver. >> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_init.h b/lib/librte_e= al/linuxapp/eal/eal_pci_init.h >> index 1070eb8..5152a0b 100644 >> --- a/lib/librte_eal/linuxapp/eal/eal_pci_init.h >> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_init.h >> @@ -34,6 +34,7 @@ >> #ifndef EAL_PCI_INIT_H_ >> #define EAL_PCI_INIT_H_ >> =20 >> +#include >> #include "eal_vfio.h" >> =20 >> struct pci_map { >> @@ -71,6 +72,13 @@ void *pci_map_resource(void *requested_addr, int fd= , off_t offset, >> /* map IGB_UIO resource prototype */ >> int pci_uio_map_resource(struct rte_pci_device *dev); >> =20 >> +#ifdef ENABLE_HOTPLUG >> +void pci_unmap_resource(void *requested_addr, size_t size); >> + >> +/* unmap IGB_UIO resource prototype */ >> +void pci_uio_unmap_resource(struct rte_pci_device *dev); >> +#endif /* ENABLE_HOTPLUG */ >> + >> #ifdef VFIO_PRESENT >> =20 >> #define VFIO_MAX_GROUPS 64 >> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_ea= l/linuxapp/eal/eal_pci_uio.c >> index 1da3507..81830d1 100644 >> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c >> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c >> @@ -404,6 +404,71 @@ pci_uio_map_resource(struct rte_pci_device *dev) >> return 0; >> } >> =20 >> +#ifdef ENABLE_HOTPLUG >> +static void >> +pci_uio_unmap(struct mapped_pci_resource *uio_res) >> +{ >> + int i; >> + >> + if (uio_res =3D=3D NULL) >> + return; >> + >> + for (i =3D 0; i !=3D uio_res->nb_maps; i++) >> + pci_unmap_resource(uio_res->maps[i].addr, >> + (size_t)uio_res->maps[i].size); >> +} >> + >> +static struct mapped_pci_resource * >> +pci_uio_find_resource(struct rte_pci_device *dev) >> +{ >> + struct mapped_pci_resource *uio_res; >> + >> + if (dev =3D=3D NULL) >> + return NULL; >> + >> + TAILQ_FOREACH(uio_res, pci_res_list, next) { >> + >> + /* skip this element if it doesn't match our PCI address */ >> + if (!eal_compare_pci_addr(&uio_res->pci_addr, &dev->addr)) >> + return uio_res; >> + } >> + return NULL; >> +} >> + >> +/* unmap the PCI resource of a PCI device in virtual memory */ >> +void >> +pci_uio_unmap_resource(struct rte_pci_device *dev) >> +{ >> + struct mapped_pci_resource *uio_res; >> + >> + if (dev =3D=3D NULL) >> + return; >> + >> + /* find an entry for the device */ >> + uio_res =3D pci_uio_find_resource(dev); >> + if (uio_res =3D=3D NULL) >> + return; >> + >> + /* secondary processes - just free maps */ >> + if (rte_eal_process_type() !=3D RTE_PROC_PRIMARY) >> + return pci_uio_unmap(uio_res); >> + >> + TAILQ_REMOVE(pci_res_list, uio_res, next); >> + >> + /* unmap all resources */ >> + pci_uio_unmap(uio_res); >> + >> + /* free uio resource */ >> + rte_free(uio_res); >> + >> + /* close fd if in primary process */ >> + close(dev->intr_handle.fd); >> + >> + dev->intr_handle.fd =3D -1; >> + dev->intr_handle.type =3D RTE_INTR_HANDLE_UNKNOWN; >> +} >> +#endif /* ENABLE_HOTPLUG */ >> + >> /* >> * parse a sysfs file containing one integer value >> * different to the eal version, as it needs to work with 64-bit valu= es