Re: [dpdk-dev] [PATCH] pci: keep API compatibility with mmap values

[Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>]

My apologies for not examining mmap() usage thoroughly enough when designing
these internal APIs. I support your suggestion to move PCI mapping functions
to bus driver after proper deprecation.

On Fri, 10 Jul 2020 13:53:24 +0200, Thomas Monjalon wrote:
[snip]
> 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
>  };
>  
>  /**
> diff --git a/lib/librte_eal/windows/include/sys/mman.h b/lib/librte_eal/windows/include/sys/mman.h
> new file mode 100644
> index 0000000000..0b4b10df1f
> --- /dev/null
> +++ b/lib/librte_eal/windows/include/sys/mman.h
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright 2020 Mellanox Technologies, Ltd
> + */
> +
> +/*
> + * The syscall mmap does not exist on Windows,
> + * but this error code is used in a badly defined DPDK API for PCI mapping.
> + */
> +#define MAP_FAILED ((void *) -1)

I see why we can't help exposing <sys/mman.h> in <rte_eal_paging.h>, but I'm
against adding this stub header. Why not make #include <sys/mman.h>
conditional and define MAP_FAIELD in <rte_os.h> for Windows?

(While networking headers we have define things that are common to all
platforms, just expressed in incompatible ways, memory management is
inherently OS-specific.)

-- 
Dmitry Kozlyuk