Re: [dpdk-dev] [PATCH] gpudev: introduce memory API

["Wang, Haiyue" <haiyue.wang@intel.com>]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Thomas Monjalon
> Sent: Thursday, June 3, 2021 04:36
> To: dev@dpdk.org
> Cc: Elena Agostini <eagostini@nvidia.com>
> Subject: [dpdk-dev] [PATCH] gpudev: introduce memory API
> 
> From: Elena Agostini <eagostini@nvidia.com>
> 
> The new library gpudev is for dealing with GPU from a DPDK application
> in a vendor-agnostic way.
> 
> As a first step, the features are focused on memory management.
> A function allows to allocate memory inside the GPU,
> while another one allows to use main (CPU) memory from the GPU.
> 
> The infrastructure is prepared to welcome drivers in drivers/gpu/
> as the upcoming NVIDIA one, implementing the gpudev API.
> Other additions planned for next revisions:
>   - C implementation file
>   - guide documentation
>   - unit tests
>   - integration in testpmd to enable Rx/Tx to/from GPU memory.
> 
> The next step should focus on GPU processing task control.
> 
> Signed-off-by: Elena Agostini <eagostini@nvidia.com>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  .gitignore                           |   1 +
>  MAINTAINERS                          |   6 +
>  doc/api/doxy-api-index.md            |   1 +
>  doc/api/doxy-api.conf.in             |   1 +
>  doc/guides/conf.py                   |   8 ++
>  doc/guides/gpus/features/default.ini |  13 ++
>  doc/guides/gpus/index.rst            |  11 ++
>  doc/guides/gpus/overview.rst         |   7 +
>  doc/guides/index.rst                 |   1 +
>  doc/guides/prog_guide/gpu.rst        |   5 +
>  doc/guides/prog_guide/index.rst      |   1 +
>  drivers/gpu/meson.build              |   4 +
>  drivers/meson.build                  |   1 +
>  lib/gpudev/gpu_driver.h              |  44 +++++++
>  lib/gpudev/meson.build               |   9 ++
>  lib/gpudev/rte_gpudev.h              | 183 +++++++++++++++++++++++++++
>  lib/gpudev/version.map               |  11 ++
>  lib/meson.build                      |   1 +
>  18 files changed, 308 insertions(+)
>  create mode 100644 doc/guides/gpus/features/default.ini
>  create mode 100644 doc/guides/gpus/index.rst
>  create mode 100644 doc/guides/gpus/overview.rst
>  create mode 100644 doc/guides/prog_guide/gpu.rst
>  create mode 100644 drivers/gpu/meson.build
>  create mode 100644 lib/gpudev/gpu_driver.h
>  create mode 100644 lib/gpudev/meson.build
>  create mode 100644 lib/gpudev/rte_gpudev.h
>  create mode 100644 lib/gpudev/version.map
> 


> +#include <stdint.h>
> +
> +#include <rte_common.h>
> +
> +#include "rte_gpudev.h"
> +
> +struct rte_gpu_dev;
> +
> +typedef int (*gpu_malloc_t)(struct rte_gpu_dev *dev, size_t size, void **ptr);
> +typedef int (*gpu_free_t)(struct rte_gpu_dev *dev, void *ptr);
> +
> +struct rte_gpu_dev {
> +	/* Backing device. */
> +	struct rte_device *device;
> +	/* GPU info structure. */
> +	struct rte_gpu_info info;
> +	/* Counter of processes using the device. */
> +	uint16_t process_cnt;
> +	/* If device is currently used or not. */
> +	enum rte_gpu_state state;
> +	/* FUNCTION: Allocate memory on the GPU. */
> +	gpu_malloc_t gpu_malloc;
> +	/* FUNCTION: Allocate memory on the CPU visible from the GPU. */
> +	gpu_malloc_t gpu_malloc_visible;
> +	/* FUNCTION: Free allocated memory on the GPU. */
> +	gpu_free_t gpu_free;


I'm wondering that we can define the malloc type as:

typedef int (*gpu_malloc_t)(struct rte_gpu_dev *dev, size_t size, void **ptr,
					unsigned int flags)

#define RTE_GPU_MALLOC_F_CPU_VISIBLE 0x01u --> gpu_malloc_visible

Then only one malloc function member is needed, paired with 'gpu_free'.

> +	/* Device interrupt handle. */
> +	struct rte_intr_handle *intr_handle;
> +	/* Driver-specific private data. */
> +	void *dev_private;
> +} __rte_cache_aligned;
> +


> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Allocate a chunk of memory on the GPU.
> + *
> + * @param gpu_id
> + *   GPU ID to allocate memory.
> + * @param size
> + *   Number of bytes to allocate.
> + * @param ptr
> + *   Pointer to store the address of the allocated memory.
> + *
> + * @return
> + *   0 on success, -1 otherwise.
> + */
> +__rte_experimental
> +int rte_gpu_malloc(uint16_t gpu_id, size_t size, void **ptr);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Allocate a chunk of memory on the CPU that is visible from the GPU.
> + *
> + * @param gpu_id
> + *   Reference GPU ID.
> + * @param size
> + *   Number of bytes to allocate.
> + * @param ptr
> + *   Pointer to store the address of the allocated memory.
> + *
> + * @return
> + *   0 on success, -1 otherwise.
> + */
> +__rte_experimental
> +int rte_gpu_malloc_visible(uint16_t gpu_id, size_t size, void **ptr);

Then 'rte_gpu_malloc_visible' is no needed, and the new call is:

rte_gpu_malloc(uint16_t gpu_id, size_t size, void **ptr, RTE_GPU_MALLOC_F_CPU_VISIBLE).

Also, we can define more flags for feature extension. ;-)

> +
> +#ifdef __cplusplus
> +}
> --
> 2.31.1