From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
by inbox.dpdk.org (Postfix) with ESMTP id 3B746A052A;
Fri, 10 Jul 2020 17:39:11 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
by dpdk.org (Postfix) with ESMTP id B2D691DAAB;
Fri, 10 Jul 2020 17:39:10 +0200 (CEST)
Received: from mga02.intel.com (mga02.intel.com [134.134.136.20])
by dpdk.org (Postfix) with ESMTP id 450681DAAA
for <dev@dpdk.org>; Fri, 10 Jul 2020 17:39:09 +0200 (CEST)
IronPort-SDR: ugo6JBFds6Rxy7aWOHxm5cARM7CvTOOhNHBCUQqbUMebHZzKRjWYCXIuvuw4fuwdifhGREdaOg
0OBn6/hmNC9A==
X-IronPort-AV: E=McAfee;i="6000,8403,9678"; a="136425630"
X-IronPort-AV: E=Sophos;i="5.75,336,1589266800"; d="scan'208";a="136425630"
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from orsmga006.jf.intel.com ([10.7.209.51])
by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
10 Jul 2020 08:39:08 -0700
IronPort-SDR: E25RqNkSDDS/Z6zyHaugS8hL3H0aixzu+472Zxvp20mXrhJGUOoazc/0TKjCzQXODTqclVZWIm
4f0HK9h0LpMw==
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.75,336,1589266800"; d="scan'208";a="284539199"
Received: from aburakov-mobl.ger.corp.intel.com (HELO [10.213.1.123])
([10.213.1.123])
by orsmga006.jf.intel.com with ESMTP; 10 Jul 2020 08:39:03 -0700
To: Thomas Monjalon <thomas@monjalon.net>, dev@dpdk.org
Cc: 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
References: <20200710115324.3902559-1-thomas@monjalon.net>
From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
Message-ID: <3151e427-77af-80c0-e53b-4e107bb1a40c@intel.com>
Date: Fri, 10 Jul 2020 16:39:02 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101
Thunderbird/68.9.0
MIME-Version: 1.0
In-Reply-To: <20200710115324.3902559-1-thomas@monjalon.net>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
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.
>
> Bugzilla ID: 503
> Fixes: 2fd3567e5425 ("pci: use OS generic memory mapping functions")
> Cc: talshn@mellanox.com
>
> Reported-by: David Marchand <david.marchand@redhat.com>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
<snip>
> /* unmap addrs correctly mapped */
> for (j = 0; j < i; j++)
> pci_unmap_resource(
> diff --git a/lib/librte_eal/include/rte_eal_paging.h b/lib/librte_eal/include/rte_eal_paging.h
> index ed98e70e9e..680a7f2505 100644
> --- a/lib/librte_eal/include/rte_eal_paging.h
> +++ b/lib/librte_eal/include/rte_eal_paging.h
> @@ -3,6 +3,7 @@
> */
>
> #include <stdint.h>
> +#include <sys/mman.h>
>
> #include <rte_compat.h>
>
> @@ -22,6 +23,7 @@ enum rte_mem_prot {
>
> /** 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
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.
--
Thanks,
Anatoly