Re: [dpdk-dev] [PATCH] eal: map PCI memory resources after hugepages

["XU Liang" <liang.xu@cinfotech.cn>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 11770 bytes --]

By the way, the pci_map_resource will check the mapaddr == requested_addr. So you must provide a usable requested_addr. It's a reason I modified the pci_map_resource().------------------------------------------------------------------From:徐亮 <liang.xu@cinfotech.cn>Time:2014 Nov 10 (Mon) 18 : 04To:Burakov, Anatoly <anatoly.burakov@intel.com>, dev@dpdk.org <dev@dpdk.org>Subject:Re: [dpdk-dev] [PATCH] eal: map PCI memory resources after hugepages
Of course you can take this job. Thanks you for your help.------------------------------------------------------------------From:徐亮 <liang.xu@cinfotech.cn>Time:2014 Nov 10 (Mon) 18 : 01To:Burakov, Anatoly <anatoly.burakov@intel.com>, dev@dpdk.org <dev@dpdk.org>Cc:thomas.monjalon@6wind.com <thomas.monjalon@6wind.com>Subject:Re: [PATCH] eal: map PCI memory resources after hugepages
It is a default value when the requested_addr isn't exist, not an overide. When the pci_map_resource is called at the primary process, the requested_addr is NULL. The default value will be provided by default_map_addr.
When the pci_map_resource is called at the secondery process, the requested_addr is exist. Then everything isn't be changed.------------------------------------------------------------------From:Burakov, Anatoly <anatoly.burakov@intel.com>Time:2014 Nov 10 (Mon) 17 : 54To:徐亮 <liang.xu@cinfotech.cn>, dev@dpdk.org <dev@dpdk.org>Cc:thomas.monjalon@6wind.com <thomas.monjalon@6wind.com>Subject:RE: [PATCH] eal: map PCI memory resources after hugepages
Hi Liang

I don't think that overriding the value passed to pci_map_resource as argument is the way to go. While it results in less code, it looks weird, in my opinion at least, as I believe tracking the correctness of address being requested should be the responsibility of the caller, i.e. either UIO or VFIO code. Which is why I keep insisting that you make requested_pci_addr global to linuxapp EAL PCI section and put it into include/eal_pci_init.h. Would you mind if I made a patch for this issue based on your code?

Thanks,
Anatoly

-----Original Message-----
From: Liang Xu [mailto:liang.xu@cinfotech.cn] 
Sent: Saturday, November 8, 2014 3:32 AM
To: dev@dpdk.org
Cc: Burakov, Anatoly; thomas.monjalon@6wind.com
Subject: [PATCH] eal: map PCI memory resources after hugepages

A multiple process DPDK application must mmap hugepages and pci resources into same virtual addresses. By default the virtual addresses chosen by the primary process automatically when calling the mmap. But sometime the chosen virtual addresses isn't usable at secondary process. Such as the secondary process linked with more libraries than primary process. The library has been mapped into this virtual address. The command line parameter 'base-virtaddr' has been added for this situation. If it's configured, the hugepages will be mapped into this base address. But the virtual address of pci resources mapped still does not refer to the parameter. In that case "EAL: pci_map_resource(): cannot mmap" 
will be got.

This patch try to map pci resources after hugepages. So the error can be resolved by set base-virtaddr into free virtual address space.

Signed-off-by: Liang Xu <liang.xu@cinfotech.cn>
---
 lib/librte_eal/linuxapp/eal/eal_pci.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index ddb0535..502eef2 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -97,14 +97,42 @@ error:
 	return -1;
 }
 
+static void *
+pci_find_max_end_va(void)
+{
+	const struct rte_memseg *seg = rte_eal_get_physmem_layout();
+	const struct rte_memseg *last = seg;
+	unsigned i = 0;
+
+	for (i = 0; i < RTE_MAX_MEMSEG; i++, seg++) {
+		if (seg->addr == NULL)
+			break;
+
+		if (seg->addr > last->addr)
+			last = seg;
+
+	}
+	return RTE_PTR_ADD(last->addr, last->len); }
+
 /* map a particular resource from a file */  void *  pci_map_resource(void *requested_addr, int fd, off_t offset, size_t size)  {
 	void *mapaddr;
 
+	/* By default the PCI memory resource will be mapped after hugepages */
+	static void *default_map_addr;
+	if (NULL == requested_addr) {
+		if (NULL == default_map_addr)
+			default_map_addr = pci_find_max_end_va();
+		mapaddr = default_map_addr;
+	} else {
+	    mapaddr = requested_addr;
+	}
+
 	/* Map the PCI memory resource of device */
-	mapaddr = mmap(requested_addr, size, PROT_READ | PROT_WRITE,
+	mapaddr = mmap(mapaddr, size, PROT_READ | PROT_WRITE,
 			MAP_SHARED, fd, offset);
 	if (mapaddr == MAP_FAILED ||
 			(requested_addr != NULL && mapaddr != requested_addr)) { @@ -114,6 +142,8 @@ pci_map_resource(void *requested_addr, int fd, off_t offset, size_t size)
 			strerror(errno), mapaddr);
 		goto fail;
 	}
+	if (NULL == requested_addr)
+		default_map_addr = RTE_PTR_ADD(mapaddr, size);
 
 	RTE_LOG(DEBUG, EAL, "  PCI memory mapped at %p\n", mapaddr);
 
--
1.9.1
From nhorman@tuxdriver.com  Mon Nov 10 12:10:38 2014
Return-Path: <nhorman@tuxdriver.com>
Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58])
 by dpdk.org (Postfix) with ESMTP id 104D56828
 for <dev@dpdk.org>; Mon, 10 Nov 2014 12:10:38 +0100 (CET)
Received: from hmsreliant.think-freely.org
 ([2001:470:8:a08:7aac:c0ff:fec2:933b] helo=localhost)
 by smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63)
 (envelope-from <nhorman@tuxdriver.com>)
 id 1Xnn14-0000fV-UC; Mon, 10 Nov 2014 06:20:21 -0500
Date: Mon, 10 Nov 2014 06:20:13 -0500
From: Neil Horman <nhorman@tuxdriver.com>
To: "Sujith Sankar (ssujith)" <ssujith@cisco.com>
Message-ID: <20141110112013.GA26613@hmsreliant.think-freely.org>
References: <1415390747-9532-1-git-send-email-ssujith@cisco.com>
 <1415390747-9532-4-git-send-email-ssujith@cisco.com>
 <20141107113449.GC25469@hmsreliant.think-freely.org>
 <D082E709.27161%ssujith@cisco.com>
 <D0868974.2733D%ssujith@cisco.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <D0868974.2733D%ssujith@cisco.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
X-Spam-Score: -2.9 (--)
X-Spam-Status: No
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH 3/7] ENIC PMD Makefile
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Mon, 10 Nov 2014 11:10:38 -0000

On Mon, Nov 10, 2014 at 09:59:45AM +0000, Sujith Sankar (ssujith) wrote:
> Neil,
>
> If I move the DPDK patch that accommodates ENIC PMD (that is the one that
> patches lib/Makefile) to the last in the series, builds between commits
> would succeed, wouldn¹t it?  Moving that to the last is anyway needed.
>
correct, yes.
Neil

> Thanks,
> -Sujith
>
> On 07/11/14 9:16 pm, "Sujith Sankar (ssujith)" <ssujith@cisco.com> wrote:
>
> >Hi Neil,
> >
> >Thanks for the comments.  I shall work on the modifications that you have
> >suggested and get back with V2.
> >
> >Regards,
> >-Sujith
> >
> >On 07/11/14 5:04 pm, "Neil Horman" <nhorman@tuxdriver.com> wrote:
> >
> >>On Sat, Nov 08, 2014 at 01:35:43AM +0530, Sujith Sankar wrote:
> >>> Signed-off-by: Sujith Sankar <ssujith@cisco.com>
> >>> ---
> >>>  lib/librte_pmd_enic/Makefile | 66
> >>>++++++++++++++++++++++++++++++++++++++++++++
> >>>  1 file changed, 66 insertions(+)
> >>>  create mode 100644 lib/librte_pmd_enic/Makefile
> >>>
> >>> diff --git a/lib/librte_pmd_enic/Makefile
> >>>b/lib/librte_pmd_enic/Makefile
> >>> new file mode 100644
> >>> index 0000000..7605a8f
> >>> --- /dev/null
> >>> +++ b/lib/librte_pmd_enic/Makefile
> >>> @@ -0,0 +1,66 @@
> >>> +#   BSD LICENSE
> >>> +#
> >>> +#   Copyright(c) 2010-2013 Intel Corporation. All rights reserved.
> >>> +#   All rights reserved.
> >>> +#
> >>> +#   Redistribution and use in source and binary forms, with or without
> >>> +#   modification, are permitted provided that the following conditions
> >>> +#   are met:
> >>> +#
> >>> +#     * Redistributions of source code must retain the above copyright
> >>> +#       notice, this list of conditions and the following disclaimer.
> >>> +#     * Redistributions in binary form must reproduce the above
> >>>copyright
> >>> +#       notice, this list of conditions and the following disclaimer
> >>>in
> >>> +#       the documentation and/or other materials provided with the
> >>> +#       distribution.
> >>> +#     * Neither the name of Intel Corporation nor the names of its
> >>> +#       contributors may be used to endorse or promote products
> >>>derived
> >>> +#       from this software without specific prior written permission.
> >>> +#
> >>> +#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> >>>CONTRIBUTORS
> >>> +#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> >>> +#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
> >>>FOR
> >>> +#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> >>>COPYRIGHT
> >>> +#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> >>>INCIDENTAL,
> >>> +#   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 ON
> >>>ANY
> >>> +#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> >>>TORT
> >>> +#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
> >>>USE
> >>> +#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> >>>DAMAGE.
> >>> +#
> >>> +
> >>> +include $(RTE_SDK)/mk/rte.vars.mk
> >>> +
> >>> +#
> >>> +# library name
> >>> +#
> >>> +LIB = librte_pmd_enic.a
> >>> +
> >>> +CFLAGS += -I$(RTE_SDK)/lib/librte_hash/
> >>> +CFLAGS += -O3 -Wno-deprecated
> >>> +
> >>> +VPATH += $(RTE_SDK)/lib/librte_pmd_enic/src
> >>> +
> >>> +#
> >>> +# all source are stored in SRCS-y
> >>> +#
> >>> +SRCS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += enic_main.c
> >>> +SRCS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += enic_clsf.c
> >>> +SRCS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += vnic_cq.c
> >>> +SRCS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += vnic_wq.c
> >>> +SRCS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += vnic_dev.c
> >>> +SRCS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += vnic_intr.c
> >>> +SRCS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += vnic_rq.c
> >>> +SRCS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += enic_etherdev.c
> >>> +SRCS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += enic_res.c
> >>> +SRCS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += vnic_rss.c
> >>> +
> >>> +
> >>> +# this lib depends upon:
> >>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += lib/librte_eal
> >>>lib/librte_ether
> >>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += lib/librte_mempool
> >>>lib/librte_mbuf
> >>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += lib/librte_net
> >>>lib/librte_malloc
> >>> +
> >>> +include $(RTE_SDK)/mk/rte.lib.mk
> >>> +
> >>> --
> >>> 1.9.1
> >>>
> >>>
> >>
> >>Make this the last patch in your series, and merge it with the chunk from
> >>the
> >>last patch that adds the enic directory to the lib/Makefile, so that a
> >>bisect
> >>will build between these commits.
> >>
> >>Neil
> >>
> >
>
>