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 B6E77A052A; Fri, 10 Jul 2020 18:17:41 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 0D7681D965; Fri, 10 Jul 2020 18:17:41 +0200 (CEST) Received: from new3-smtp.messagingengine.com (new3-smtp.messagingengine.com [66.111.4.229]) by dpdk.org (Postfix) with ESMTP id D90B61D960 for ; Fri, 10 Jul 2020 18:17:38 +0200 (CEST) Received: from compute7.internal (compute7.nyi.internal [10.202.2.47]) by mailnew.nyi.internal (Postfix) with ESMTP id 397AE580531; Fri, 10 Jul 2020 12:17:38 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Fri, 10 Jul 2020 12:17:38 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=fm1; bh= ojz6KqwkQJKKTKs8AzLo7PxguK+LsxjlKFPX4BarjaY=; b=uiKicrUFUV+/NXu3 FAdikoabtXky4XTk4M0FagPKIX/+b8QvLoSn6BBqJUofDwJESutySxe73AAiqxCS 8z+o1I2VvSE3ZNxTdO8LOKOPGSO21MAKqXFwIgqUVB3WjXvdTzMsUfLgRwt9us6d HTqfxoVMU39QDpMsfw1aqSMbm9/COjVxwJQl8JiLlaZjffcfsJPGIZl3vv6IubJp zfyEmmdNJg1SRRXjgfVwQXsOvXlLJxB8P2EKBwe4IWEb9nLyjGBD+syL9AS/e1MN H0NXZqDoDVLsT/iUItkPuEYCa5KARkr9FrJImpWRXetYRJpZsT+bm5Wri01P+oAc /XIsTA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; bh=ojz6KqwkQJKKTKs8AzLo7PxguK+LsxjlKFPX4Barj aY=; b=U6Qwto3izigWAIa6lQca6YH/ljvWZpNN8xV3ajmSytpjKFJhiixN1xFil hNL2UgbaOxhmKhCqkFw8tkyh1v/OmlSL1KJVqGIYng18KKHHBaPzxA+B9Gzb3HVc I1Wxj1EfFBf1iPmuslZTfeErvTJXkkiQ+YZDNTiBrA+/kSFqfk79gpFSVLXjYLc/ WhRn6HSm9Ycfbe6aKkRPHEfi+T2ecB7Ty2d5u6efzO1xUv7BhA4vHOx/GFg8VSEq fkctyzNVMO4dD66xfPZp8XYXyLXtnGbPhBa6WHC6C7lUon2WQJLbCLGmYoSwZFxv 3ihj5g3XsHy160MylO92BzpkTso0Q== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduiedrvddugddutdefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepudeggfdvfeduffdtfeeglefghfeukefgfffhueejtdetuedtjeeu ieeivdffgeehnecukfhppeejjedrudefgedrvddtfedrudekgeenucevlhhushhtvghruf hiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehthhhomhgrshesmhhonhhjrghl ohhnrdhnvght X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id EFE1630600A3; Fri, 10 Jul 2020 12:17:35 -0400 (EDT) From: Thomas Monjalon To: "Burakov, Anatoly" Cc: dev@dpdk.org, david.marchand@redhat.com, ferruh.yigit@intel.com, grive@u256.net, alvinx.zhang@intel.com, beilei.xing@intel.com, jia.guo@intel.com, bruce.richardson@intel.com, dmitry.kozliuk@gmail.com, navasile@linux.microsoft.com, dmitrym@microsoft.com, pallavi.kadam@intel.com, talshn@mellanox.com Date: Fri, 10 Jul 2020 18:17:34 +0200 Message-ID: <2414408.smBOq31esu@thomas> In-Reply-To: <3151e427-77af-80c0-e53b-4e107bb1a40c@intel.com> References: <20200710115324.3902559-1-thomas@monjalon.net> <3151e427-77af-80c0-e53b-4e107bb1a40c@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH] pci: keep API compatibility with mmap values 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" 10/07/2020 17:39, Burakov, Anatoly: > On 10-Jul-20 12:53 PM, Thomas Monjalon wrote: > > The function pci_map_resource() returns MAP_FAILED in case of error. > > When replacing the call to mmap() by rte_mem_map(), > > the error code became NULL, breaking the API. > > This function is probably not used outside of DPDK, > > but it is still a problem for two reasons: > > - the deprecation process was not followed > > - the Linux function pci_vfio_mmap_bar() is broken for i40e > > > > The error code is reverted to the Unix value MAP_FAILED. > > Windows needs to define this special value (-1 as in Unix). > > After proper deprecation process, the API could be changed again > > if really needed. > > > > Because of the switch from mmap() to rte_mem_map(), > > another part of the API was changed: "int additional_flags" > > are defined as "additional flags for the mapping range" > > without mentioning it was directly used in mmap(). > > Currently it is directly used in rte_mem_map(), > > that's why the values rte_map_flags must be mapped (sic) on the mmap ones > > in case of Unix OS. > > > > These are side effects of a badly defined API using Unix values. [...] > > /** Additional flags for memory mapping. */ > > enum rte_map_flags { > > +#ifdef RTE_EXEC_ENV_WINDOWS > > /** Changes to the mapped memory are visible to other processes. */ > > RTE_MAP_SHARED = 1 << 0, > > /** Mapping is not backed by a regular file. */ > > @@ -35,6 +37,12 @@ enum rte_map_flags { > > * it is not required to do so, thus mapping with this flag may fail. > > */ > > RTE_MAP_FORCE_ADDRESS = 1 << 3 > > +#else /* map mmap flags because they are exposed in pci_map_resource() API */ > > + RTE_MAP_SHARED = MAP_SHARED, > > + RTE_MAP_ANONYMOUS = MAP_ANONYMOUS, > > + RTE_MAP_PRIVATE = MAP_PRIVATE, > > + RTE_MAP_FORCE_ADDRESS = MAP_FIXED, > > +#endif > > I'm probably missing something, but why is this needed? Doesn't Yes you missed reading the commit log :) Or maybe it is not written clearly enough. Will try to rephrase. > rte_mem_map() automatically translate these flags into proper ones? > pci_map_resource() will call rte_mem_map(), and that will translate > these flags into their Unix equivalents. The problem is that we have an API which is taking mmap flags as input. "int additional_flags" is a parameter of the function, and are supposed to be mmap flags. But it is not stated clearly. When Windows will use this function, it won't use mmap flags but RTE_MAP_*. So we must accept both. That's why the best is to make values the same. In 20.11, we could change the API, make clear that only RTE_MAP_* is accepted, and remove this workaround. Or even better, remove pci_map_resource from the PCI lib, and implement it in the PCI bus driver. pci_map_resource() function is a bad designed API