From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-f67.google.com (mail-ed1-f67.google.com [209.85.208.67]) by dpdk.org (Postfix) with ESMTP id 2F5AB7EC7 for ; Thu, 1 Nov 2018 12:30:40 +0100 (CET) Received: by mail-ed1-f67.google.com with SMTP id z12-v6so10496945edp.7 for ; Thu, 01 Nov 2018 04:30:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=netronome-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=C4kShOaOHO3IazN1GBcj318oV3CYubMbIxVmpxUhcUo=; b=dOU8vLlf3u0dOGsw4ctXE10P54hOfi4OLnpS3+bWmeH+t6ym3nosAjAFXH0yYU5EKn pExajKgptw3SZcjTSTcs+HuXrXJz/24xBMi6YGwCEtnXaSOMQvaYm5PJnvtC7lQ4SXfD fRLX2TCux2XV52R8xvWkKRFS0G2+i/AZx1WR+4K1upFxmsKGG17hhBhCELhEdhVKrJP/ nWWYMhx0HwCCKnz0UhncWjbqRk6rMchOhBKAUcK6FlGeYmvCS7T6P7UI4numLqzIN2PO b0vGpuYf66hajJdzGx82uAsTGuml8F2em0xO2f2P/MMNyvNxMa4WjjBw2dlZrnLAX6L5 sZpA== 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=C4kShOaOHO3IazN1GBcj318oV3CYubMbIxVmpxUhcUo=; b=licc2n8cG/LkJ+/nsgArXnOUmemMtcbm8+ZdnZk/wWRQ6ShPEpNNY2ByosJgrl/+/Y jrlcmzKbvnIBupGG+ZRzX0XZbfGJJFjlkQaMTjaPGDjvxwD0flkXNPZ99IbK8TzOvJET WzJsXjLn+/TeGSpEVJ1eoZihtE2jUe6ZGv+joZU01T61arU8TLc2kNpffXY76ghCNV2H MsQ0GQqiCxhaqJR3gJM5Zc6r5i01hrai8a7xO0xN+NFcbxt5FLOBu6UMgl4WlIjcNlXv +Ln5GHyjJWq/OyRBkSlkxBN3NHgEWH6qcxrLQUxDRs38Ad3PpfdUSP2NFjFote74+XOa 5/LA== X-Gm-Message-State: AGRZ1gK9SHYwH6VLHonxK6TLCuGHDnFtIIhTX+lO2FabVnATTOnFTV/g kmJLiJ1nkfvMmq3Jw6NzqKZLI8j6qUcT+BcLb5iNeA== X-Google-Smtp-Source: AJdET5duAnlb7BbaHle9Shw46nIVLVVf5SHIg8scBcO/6I9W8tc0fG6euC11fygV7nV87CqqYbMXiBb1COvE/oxaOvM= X-Received: by 2002:a17:906:37da:: with SMTP id o26-v6mr3783234ejc.179.1541071839727; Thu, 01 Nov 2018 04:30:39 -0700 (PDT) MIME-Version: 1.0 References: <20181031172931.11894-1-alejandro.lucero@netronome.com> <20181031172931.11894-4-alejandro.lucero@netronome.com> <2be201f7-308f-53e1-715a-6c37b92de10b@intel.com> <1496b1e0-f3de-2a02-a5d9-3e6c65eb93d4@intel.com> In-Reply-To: <1496b1e0-f3de-2a02-a5d9-3e6c65eb93d4@intel.com> From: Alejandro Lucero Date: Thu, 1 Nov 2018 11:30:28 +0000 Message-ID: To: "Burakov, Anatoly" Cc: dev Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH 3/7] mem: add function for setting DMA mask 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: , X-List-Received-Date: Thu, 01 Nov 2018 11:30:40 -0000 On Thu, Nov 1, 2018 at 10:57 AM Burakov, Anatoly wrote: > On 01-Nov-18 10:48 AM, Alejandro Lucero wrote: > > > > > > On Thu, Nov 1, 2018 at 10:11 AM Burakov, Anatoly > > > wrote: > > > > On 31-Oct-18 5:29 PM, Alejandro Lucero wrote: > > > This patch adds the possibility of setting a dma mask to be used > > > once the memory initialization is done. > > > > > > This is currently needed when IOVA mode is set by PCI related > > > code and an x86 IOMMU hardware unit is present. Current code calls > > > rte_mem_check_dma_mask but it is wrong to do so at that point > > > because the memory has not been initialized yet. > > > > > > Signed-off-by: Alejandro Lucero > > > > > --- > > > lib/librte_eal/common/eal_common_memory.c | 10 ++++++++++ > > > lib/librte_eal/common/include/rte_memory.h | 10 ++++++++++ > > > lib/librte_eal/rte_eal_version.map | 1 + > > > 3 files changed, 21 insertions(+) > > > > > > diff --git a/lib/librte_eal/common/eal_common_memory.c > > b/lib/librte_eal/common/eal_common_memory.c > > > index e0f08f39a..24b72fcb0 100644 > > > --- a/lib/librte_eal/common/eal_common_memory.c > > > +++ b/lib/librte_eal/common/eal_common_memory.c > > > @@ -480,6 +480,16 @@ rte_mem_check_dma_mask(uint8_t maskbits) > > > return 0; > > > } > > > > > > +/* set dma mask to use when memory initialization is done */ > > > +void __rte_experimental > > > +rte_mem_set_dma_mask(uint8_t maskbits) > > > +{ > > > + struct rte_mem_config *mcfg = > > rte_eal_get_configuration()->mem_config; > > > + > > > + mcfg->dma_maskbits = mcfg->dma_maskbits == 0 ? maskbits : > > > + RTE_MIN(mcfg->dma_maskbits, maskbits); > > > +} > > > + > > > /* return the number of memory channels */ > > > unsigned rte_memory_get_nchannel(void) > > > { > > > diff --git a/lib/librte_eal/common/include/rte_memory.h > > b/lib/librte_eal/common/include/rte_memory.h > > > index ad3f3cfb0..eff028db1 100644 > > > --- a/lib/librte_eal/common/include/rte_memory.h > > > +++ b/lib/librte_eal/common/include/rte_memory.h > > > @@ -466,6 +466,16 @@ unsigned rte_memory_get_nrank(void); > > > /* check memsegs iovas are within a range based on dma mask */ > > > int __rte_experimental rte_mem_check_dma_mask(uint8_t maskbits); > > > > > > +/** > > > + * * @warning > > > + * @b EXPERIMENTAL: this API may change without prior notice > > > > I think there's a copy-paste error here (extra star and indent before > > warning). > > > > > > I will fix this. > > > > Thanks. > > > > > > > + * > > > + * Set dma mask to use once memory initialization is done. > > > + * Previous function rte_mem_check_dma_mask can not be used > > > + * safely until memory has been initialized. > > > > IMO this really requires a big bold warning saying that this function > > must only be called early during initialization, before memory is > > initialized, and not to be called in user code. > > > > > > what about adding a new EAL variable for keeping memory initialization > > status? > > It would be something like "uninit" until is changed to "done" after the > > memory initialization is completed. > > That variable would help to avoid effective calls to set the dma mask > > after initialization. > > I'm not opposed to it in principle, however, this is a slippery slope > towards having each and every subsystem storing their init status :) > > That said, while it's not a complete solution because it won't prevent > spurious calls to this function _after_ memory has initialized, there is > a variable in internal_config that you can use to deny calls to this > function after EAL init is complete (see internal_config->init_complete). > > You could also store a static variable in eal_common_memory.c, and > change it at the end of rte_eal_memory_init() call, but that does not > really complete the memory initialization, because we then go to > rte_eal_malloc_heap_init(), which actually completes the memory init. > > I would greatly prefer using internal_config->init_complete - it is > enough to forbid user code from calling it, while driver/DPDK developers > presumably know what they're doing enough to read the warning and not > try to call this after memory init :) > I agree that we should avoid new status variables as I proposed, and in this case I think using internal_config->init_complete will be enough. I will add the check in the next version. Thanks > > > > > > + */ > > > +void __rte_experimental rte_mem_set_dma_mask(uint8_t maskbits); > > > + > > > /** > > > * Drivers based on uio will not load unless physical > > > * addresses are obtainable. It is only possible to get > > > diff --git a/lib/librte_eal/rte_eal_version.map > > b/lib/librte_eal/rte_eal_version.map > > > index ef8126a97..ae24b5c73 100644 > > > --- a/lib/librte_eal/rte_eal_version.map > > > +++ b/lib/librte_eal/rte_eal_version.map > > > @@ -296,6 +296,7 @@ EXPERIMENTAL { > > > rte_devargs_remove; > > > rte_devargs_type_count; > > > rte_mem_check_dma_mask; > > > + rte_mem_set_dma_mask; > > > > Same, this needs to be alphabetic. > > > > > rte_eal_cleanup; > > > rte_fbarray_attach; > > > rte_fbarray_destroy; > > > > > > > > > -- > > Thanks, > > Anatoly > > > > > -- > Thanks, > Anatoly >