From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 93818A0471 for ; Mon, 12 Aug 2019 12:19:38 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id AB93549E0; Mon, 12 Aug 2019 12:19:37 +0200 (CEST) Received: from mail-vk1-f194.google.com (mail-vk1-f194.google.com [209.85.221.194]) by dpdk.org (Postfix) with ESMTP id CC50D378B for ; Mon, 12 Aug 2019 12:19:35 +0200 (CEST) Received: by mail-vk1-f194.google.com with SMTP id w186so12238487vkd.11 for ; Mon, 12 Aug 2019 03:19:35 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=dga2iZRDEIxswgAbVOsHeQWFdwbFkopAJ3ePAi3zR2Q=; b=Edd8L0QBEe0tphfAIRXtlwpbGMDX72jDnYSCuX2SsQ3M5FID0ahNwwOkS7x4fzsKSg Vssbsqb7olkpJDpzOr0kVNF6yYn5PCl4nI9JZknHbXXpepNsSv78uH5j4M3aYugLlBE7 OeQSPuEJsILmdfVq+iXrgjWviFiKiTgfFag+IokxZaPamfGF19Z2i3JKGZqeWymOmBKD TyE4dFtkW8/BJRygLjX86ZVBhQXdYFd+5zxdWIiDtkjXHwxI5Y+FKhjqn6S4WbSqCcte 0YzMlB6er5GsWLjwXOAlHxTB4wB2ifspx0aqnH6NaVQpu487tk5ce8A7d3v1In/3/gzM vCpQ== X-Gm-Message-State: APjAAAVuYg8ipGedJcaeiXYLrsx+ywZ8UDxadzyet4cKJZKQmCGdYLpX h93KTXnMlZK5jeBlp1rtD9KL/Gw6PLDwbfWgwmDcdw== X-Google-Smtp-Source: APXvYqyQCDMH+FpCb9ODenl9hHaRE0TMrrSA9ST1P46JKmQW0IzOBlfbIeBG0f/CFM6beSyjXa7xKC35TRWyP3ypJmE= X-Received: by 2002:a1f:5185:: with SMTP id f127mr2229740vkb.52.1565605175068; Mon, 12 Aug 2019 03:19:35 -0700 (PDT) MIME-Version: 1.0 References: <26384fad6f9afcba9d3311d1bf73e76f4577225d.1563276298.git.anatoly.burakov@intel.com> In-Reply-To: <26384fad6f9afcba9d3311d1bf73e76f4577225d.1563276298.git.anatoly.burakov@intel.com> From: David Marchand Date: Mon, 12 Aug 2019 12:19:22 +0200 Message-ID: To: Anatoly Burakov Cc: dev , John McNamara , Marko Kovacevic , Bruce Richardson Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH] eal/freebsd: add support for base virtaddr option X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Tue, Jul 16, 2019 at 1:25 PM Anatoly Burakov wrote: > > According to our docs, only Linuxapp supports base-virtaddr option. > That is, strictly speaking, not true because most of the things > that are attempting to respect base-virtaddr are in common files, > so FreeBSD already *mostly* supports this option in practice. > > This commit fixes the remaining bits to explicitly support > base-virtaddr option, and moves the arg parsing from EAL to common > options parsing code. Documentation is also updated to reflect > that all platforms now support base-virtaddr. > > Signed-off-by: Anatoly Burakov > --- > doc/guides/linux_gsg/eal_args.include.rst | 6 +++ > doc/guides/linux_gsg/linux_eal_parameters.rst | 6 --- > doc/guides/rel_notes/release_19_08.rst | 5 +++ > lib/librte_eal/common/eal_common_options.c | 38 ++++++++++++++++++ > lib/librte_eal/freebsd/eal/eal.c | 13 ++++++- > lib/librte_eal/linux/eal/eal.c | 39 ------------------- > 6 files changed, 60 insertions(+), 47 deletions(-) > > diff --git a/doc/guides/linux_gsg/eal_args.include.rst b/doc/guides/linux_gsg/eal_args.include.rst > index cf421a56e..ed8b0e35b 100644 > --- a/doc/guides/linux_gsg/eal_args.include.rst > +++ b/doc/guides/linux_gsg/eal_args.include.rst > @@ -86,6 +86,12 @@ Multiprocessing-related options > > Set the type of the current process. > > +* ``--base-virtaddr
`` > + > + Attempt to use a different starting address for all memory maps of the > + primary DPDK process. This can be helpful if secondary processes cannot > + start due to conflicts in address map. > + doc/guides/freebsd_gsg/freebsd_eal_parameters.rst:.. include:: ../linux_gsg/eal_args.include.rst Ok, a bit misleading to put in linux_gsg/, so writing this here if someone else looks at this :-) > Memory-related options > ~~~~~~~~~~~~~~~~~~~~~~ > > diff --git a/doc/guides/linux_gsg/linux_eal_parameters.rst b/doc/guides/linux_gsg/linux_eal_parameters.rst > index c63f0f49a..b2cc60e44 100644 > --- a/doc/guides/linux_gsg/linux_eal_parameters.rst > +++ b/doc/guides/linux_gsg/linux_eal_parameters.rst > @@ -49,12 +49,6 @@ Multiprocessing-related options > allows running multiple independent DPDK primary/secondary processes under > different prefixes. > > -* ``--base-virtaddr
`` > - > - Attempt to use a different starting address for all memory maps of the > - primary DPDK process. This can be helpful if secondary processes cannot > - start due to conflicts in address map. > - > Memory-related options > ~~~~~~~~~~~~~~~~~~~~~~ > > diff --git a/doc/guides/rel_notes/release_19_08.rst b/doc/guides/rel_notes/release_19_08.rst > index 4a1fd8dd8..1b58d9282 100644 > --- a/doc/guides/rel_notes/release_19_08.rst > +++ b/doc/guides/rel_notes/release_19_08.rst > @@ -56,6 +56,11 @@ New Features > Also, make sure to start the actual text at the margin. > ========================================================= > > +* **FreeBSD now supports `--base-virtaddr` EAL option.** > + > + FreeBSD version now also supports setting base virtual address for mapping > + pages and resources into its address space. > + > * **Added MCS lock.** > > MCS lock provides scalability by spinning on a CPU/thread local variable Well, obviously, this needs some rebase on 19.11-rc0 :-) > diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c > index 512d5088e..156e48e19 100644 > --- a/lib/librte_eal/common/eal_common_options.c > +++ b/lib/librte_eal/common/eal_common_options.c > @@ -20,6 +20,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -1095,6 +1096,36 @@ eal_parse_iova_mode(const char *name) > return 0; > } > > +static int > +eal_parse_base_virtaddr(const char *arg) > +{ > + char *end; > + uint64_t addr; > + > + errno = 0; > + addr = strtoull(arg, &end, 16); > + > + /* check for errors */ > + if ((errno != 0) || (arg[0] == '\0') || end == NULL || (*end != '\0')) > + return -1; > + > + /* make sure we don't exceed 32-bit boundary on 32-bit target */ > +#ifndef RTE_ARCH_64 > + if (addr >= UINTPTR_MAX) > + return -1; > +#endif > + > + /* align the addr on 16M boundary, 16MB is the minimum huge page > + * size on IBM Power architecture. If the addr is aligned to 16MB, > + * it can align to 2MB for x86. So this alignment can also be used > + * on x86 and other architectures. > + */ > + internal_config.base_virtaddr = > + RTE_PTR_ALIGN_CEIL((uintptr_t)addr, (size_t)RTE_PGSIZE_16M); > + > + return 0; > +} > + > /* caller is responsible for freeing the returned string */ > static char * > available_cores(void) > @@ -1408,6 +1439,13 @@ eal_parse_common_option(int opt, const char *optarg, > return -1; > } > break; > + case OPT_BASE_VIRTADDR_NUM: > + if (eal_parse_base_virtaddr(optarg) < 0) { > + RTE_LOG(ERR, EAL, "invalid parameter for --" > + OPT_BASE_VIRTADDR "\n"); > + return -1; > + } > + break; > > /* don't know what to do, leave this to caller */ > default: > diff --git a/lib/librte_eal/freebsd/eal/eal.c b/lib/librte_eal/freebsd/eal/eal.c > index d53f0fe69..7ebe11db6 100644 > --- a/lib/librte_eal/freebsd/eal/eal.c > +++ b/lib/librte_eal/freebsd/eal/eal.c > @@ -227,6 +227,14 @@ rte_eal_config_create(void) > if (internal_config.no_shconf) > return 0; > > + /* map the config before base address so that we don't waste a page */ > + if (internal_config.base_virtaddr != 0) > + rte_mem_cfg_addr = (void *) > + RTE_ALIGN_FLOOR(internal_config.base_virtaddr - > + sizeof(struct rte_mem_config), sysconf(_SC_PAGE_SIZE)); > + else > + rte_mem_cfg_addr = NULL; > + > if (mem_cfg_fd < 0){ > mem_cfg_fd = open(pathname, O_RDWR | O_CREAT, 0600); > if (mem_cfg_fd < 0) { > @@ -254,8 +262,9 @@ rte_eal_config_create(void) > return -1; > } > > - rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config), > - PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd, 0); > + rte_mem_cfg_addr = mmap(rte_mem_cfg_addr, > + sizeof(*rte_config.mem_config), PROT_READ | PROT_WRITE, > + MAP_SHARED, mem_cfg_fd, 0); > > if (rte_mem_cfg_addr == MAP_FAILED){ > RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config\n"); Nit: when compared to Linux implementation, the reattach step does not recommend using the --base-virtaddr in case the remmapping failed. > diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c > index 2e5499f9b..79f5d70c3 100644 > --- a/lib/librte_eal/linux/eal/eal.c > +++ b/lib/librte_eal/linux/eal/eal.c > @@ -609,35 +609,6 @@ eal_parse_socket_arg(char *strval, volatile uint64_t *socket_arg) > return 0; > } > > -static int > -eal_parse_base_virtaddr(const char *arg) > -{ > - char *end; > - uint64_t addr; > - > - errno = 0; > - addr = strtoull(arg, &end, 16); > - > - /* check for errors */ > - if ((errno != 0) || (arg[0] == '\0') || end == NULL || (*end != '\0')) > - return -1; > - > - /* make sure we don't exceed 32-bit boundary on 32-bit target */ > -#ifndef RTE_ARCH_64 > - if (addr >= UINTPTR_MAX) > - return -1; > -#endif > - > - /* align the addr on 16M boundary, 16MB is the minimum huge page > - * size on IBM Power architecture. If the addr is aligned to 16MB, > - * it can align to 2MB for x86. So this alignment can also be used > - * on x86 */ > - internal_config.base_virtaddr = > - RTE_PTR_ALIGN_CEIL((uintptr_t)addr, (size_t)RTE_PGSIZE_16M); > - > - return 0; > -} > - > static int > eal_parse_vfio_intr(const char *mode) > { > @@ -796,16 +767,6 @@ eal_parse_args(int argc, char **argv) > internal_config.force_socket_limits = 1; > break; > > - case OPT_BASE_VIRTADDR_NUM: > - if (eal_parse_base_virtaddr(optarg) < 0) { > - RTE_LOG(ERR, EAL, "invalid parameter for --" > - OPT_BASE_VIRTADDR "\n"); > - eal_usage(prgname); > - ret = -1; > - goto out; > - } > - break; > - > case OPT_VFIO_INTR_NUM: > if (eal_parse_vfio_intr(optarg) < 0) { > RTE_LOG(ERR, EAL, "invalid parameters for --" > -- > 2.17.1 Reviewed-by: David Marchand -- David Marchand