[v2,02/12] mldev: support PMD functions for ML device

Message ID 20230206202453.336280-3-jerinj@marvell.com (mailing list archive)
State Changes Requested, archived
Delegated to: Thomas Monjalon
Headers
Series mldev: introduce machine learning device library |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Jerin Jacob Kollanukkaran Feb. 6, 2023, 8:24 p.m. UTC
  From: Srikanth Yalavarthi <syalavarthi@marvell.com>

Added PMD functions to handle ML devices. The rte_mldev_pmd.*
files are for drivers only and should be private to DPDK, and
are not installed for application use.

Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
Signed-off-by: Jerin Jacob <jerinj@marvell.com>
---
 lib/mldev/meson.build      |   9 +++
 lib/mldev/rte_mldev.c      | 129 ++++++++++++++++++++++++++++++++
 lib/mldev/rte_mldev_core.h | 111 +++++++++++++++++++++++++++
 lib/mldev/rte_mldev_pmd.c  |  62 +++++++++++++++
 lib/mldev/rte_mldev_pmd.h  | 149 +++++++++++++++++++++++++++++++++++++
 lib/mldev/version.map      |  11 +++
 6 files changed, 471 insertions(+)
 create mode 100644 lib/mldev/rte_mldev_core.h
 create mode 100644 lib/mldev/rte_mldev_pmd.c
 create mode 100644 lib/mldev/rte_mldev_pmd.h
  

Comments

Stephen Hemminger Feb. 6, 2023, 9:04 p.m. UTC | #1
On Tue, 7 Feb 2023 01:54:43 +0530
<jerinj@marvell.com> wrote:

> +static struct rte_ml_dev ml_devices[RTE_MLDEV_MAX_DEVS];
>

This will reserve space for 64 devices, but almost all users
will only have one. Maybe a level of indirection and allocate as needed?

You could even use a single allocation for the pmd and device private
data portion.

> + */
> +struct rte_ml_dev_data {
> +	/** Unique identifier name. */
> +	char name[RTE_ML_STR_MAX];


Why is name first, it is the least used field. Might want it to be last
for cache locality.

> +	/** Reserved for future fields */
> +	uint64_t reserved[3];

Reserved fields have been a problem in the past.
Why do this? Are thy just available pad elements to be cache line size?

And why bother being cache aligned for an info struct?
  
Thomas Monjalon Feb. 6, 2023, 10:17 p.m. UTC | #2
06/02/2023 22:04, Stephen Hemminger:
> On Tue, 7 Feb 2023 01:54:43 +0530
> <jerinj@marvell.com> wrote:
> 
> > +static struct rte_ml_dev ml_devices[RTE_MLDEV_MAX_DEVS];
> >
> 
> This will reserve space for 64 devices, but almost all users
> will only have one. Maybe a level of indirection and allocate as needed?
> 
> You could even use a single allocation for the pmd and device private
> data portion.

I like what we did for GPU class: rte_gpu_init(size_t dev_max)
If not called by the app, it is called automatically with a default size.
So you can have a small default and there is no compilation settings.
  
Jerin Jacob Feb. 7, 2023, 5:16 a.m. UTC | #3
On Tue, Feb 7, 2023 at 2:34 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Tue, 7 Feb 2023 01:54:43 +0530
> <jerinj@marvell.com> wrote:
>
> > +static struct rte_ml_dev ml_devices[RTE_MLDEV_MAX_DEVS];
> >
>
> This will reserve space for 64 devices, but almost all users
> will only have one. Maybe a level of indirection and allocate as needed?

As Thomas suggested, I will add something similar to rte_gpu_init()

>
> You could even use a single allocation for the pmd and device private
> data portion.
>
> > + */
> > +struct rte_ml_dev_data {
> > +     /** Unique identifier name. */
> > +     char name[RTE_ML_STR_MAX];
>
>
> Why is name first, it is the least used field. Might want it to be last
> for cache locality.

It is slowpath, does not matter. But there is no harm in moving end of
it. I will move to end of the structure.


>
> > +     /** Reserved for future fields */
> > +     uint64_t reserved[3];
>
> Reserved fields have been a problem in the past.

Will remove it.

> Why do this? Are thy just available pad elements to be cache line size?

Yes.

>
> And why bother being cache aligned for an info struct?

We can remove it. Will removing reserved and cache aligned.

>
  

Patch

diff --git a/lib/mldev/meson.build b/lib/mldev/meson.build
index e378cfca30..5c99532c1a 100644
--- a/lib/mldev/meson.build
+++ b/lib/mldev/meson.build
@@ -2,6 +2,7 @@ 
 # Copyright (c) 2022 Marvell.
 
 sources = files(
+        'rte_mldev_pmd.c',
         'rte_mldev.c',
 )
 
@@ -9,6 +10,14 @@  headers = files(
         'rte_mldev.h',
 )
 
+indirect_headers += files(
+        'rte_mldev_core.h',
+)
+
+driver_sdk_headers += files(
+        'rte_mldev_pmd.h',
+)
+
 deps += ['mempool']
 
 if get_option('buildtype').contains('debug')
diff --git a/lib/mldev/rte_mldev.c b/lib/mldev/rte_mldev.c
index 70aad4c44b..06396de680 100644
--- a/lib/mldev/rte_mldev.c
+++ b/lib/mldev/rte_mldev.c
@@ -4,5 +4,134 @@ 
 
 #include <rte_log.h>
 #include <rte_mldev.h>
+#include <rte_mldev_pmd.h>
+
+static struct rte_ml_dev ml_devices[RTE_MLDEV_MAX_DEVS];
+
+static struct rte_ml_dev_global ml_dev_globals = {
+	.devs = ml_devices, .data = {NULL}, .nb_devs = 0, .max_devs = RTE_MLDEV_MAX_DEVS};
+
+struct rte_ml_dev *
+rte_ml_dev_pmd_get_dev(int16_t dev_id)
+{
+	return &ml_dev_globals.devs[dev_id];
+}
+
+struct rte_ml_dev *
+rte_ml_dev_pmd_get_named_dev(const char *name)
+{
+	struct rte_ml_dev *dev;
+	int16_t dev_id;
+
+	if (name == NULL)
+		return NULL;
+
+	for (dev_id = 0; dev_id < RTE_MLDEV_MAX_DEVS; dev_id++) {
+		dev = rte_ml_dev_pmd_get_dev(dev_id);
+		if ((dev->attached == ML_DEV_ATTACHED) && (strcmp(dev->data->name, name) == 0))
+			return dev;
+	}
+
+	return NULL;
+}
+
+struct rte_ml_dev *
+rte_ml_dev_pmd_allocate(const char *name, uint8_t socket_id)
+{
+	char mz_name[RTE_MEMZONE_NAMESIZE];
+	const struct rte_memzone *mz;
+	struct rte_ml_dev *dev;
+	int16_t dev_id;
+
+	if (rte_ml_dev_pmd_get_named_dev(name) != NULL) {
+		RTE_MLDEV_LOG(ERR, "ML device with name %s already allocated!", name);
+		return NULL;
+	}
+
+	/* Get a free device ID */
+	for (dev_id = 0; dev_id < RTE_MLDEV_MAX_DEVS; dev_id++) {
+		dev = rte_ml_dev_pmd_get_dev(dev_id);
+		if (dev->attached == ML_DEV_DETACHED)
+			break;
+	}
+
+	if (dev_id == RTE_MLDEV_MAX_DEVS) {
+		RTE_MLDEV_LOG(ERR, "Reached maximum number of ML devices");
+		return NULL;
+	}
+
+	if (dev->data == NULL) {
+		/* Reserve memzone name */
+		sprintf(mz_name, "rte_ml_dev_data_%d", dev_id);
+		if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+			mz = rte_memzone_reserve(mz_name, sizeof(struct rte_ml_dev_data), socket_id,
+						 0);
+			RTE_MLDEV_LOG(DEBUG, "PRIMARY: reserved memzone for %s (%p)", mz_name, mz);
+		} else {
+			mz = rte_memzone_lookup(mz_name);
+			RTE_MLDEV_LOG(DEBUG, "SECONDARY: looked up memzone for %s (%p)", mz_name,
+				      mz);
+		}
+
+		if (mz == NULL)
+			return NULL;
+
+		ml_dev_globals.data[dev_id] = mz->addr;
+		if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+			memset(ml_dev_globals.data[dev_id], 0, sizeof(struct rte_ml_dev_data));
+
+		dev->data = ml_dev_globals.data[dev_id];
+		if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+			strlcpy(dev->data->name, name, RTE_ML_STR_MAX);
+			dev->data->dev_id = dev_id;
+			dev->data->socket_id = socket_id;
+			dev->data->dev_started = 0;
+			RTE_MLDEV_LOG(DEBUG, "PRIMARY: init mldev data");
+		}
+
+		RTE_MLDEV_LOG(DEBUG, "Data for %s: dev_id %d, socket %u", dev->data->name,
+			      dev->data->dev_id, dev->data->socket_id);
+
+		dev->attached = ML_DEV_ATTACHED;
+		ml_dev_globals.nb_devs++;
+	}
+
+	return dev;
+}
+
+int
+rte_ml_dev_pmd_release(struct rte_ml_dev *dev)
+{
+	char mz_name[RTE_MEMZONE_NAMESIZE];
+	const struct rte_memzone *mz;
+	int16_t dev_id;
+	int ret = 0;
+
+	if (dev == NULL)
+		return -EINVAL;
+
+	dev_id = dev->data->dev_id;
+
+	/* Memzone lookup */
+	sprintf(mz_name, "rte_ml_dev_data_%d", dev_id);
+	mz = rte_memzone_lookup(mz_name);
+	if (mz == NULL)
+		return -ENOMEM;
+
+	RTE_ASSERT(ml_dev_globals.data[dev_id] == mz->addr);
+	ml_dev_globals.data[dev_id] = NULL;
+
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		RTE_MLDEV_LOG(DEBUG, "PRIMARY: free memzone of %s (%p)", mz_name, mz);
+		ret = rte_memzone_free(mz);
+	} else {
+		RTE_MLDEV_LOG(DEBUG, "SECONDARY: don't free memzone of %s (%p)", mz_name, mz);
+	}
+
+	dev->attached = ML_DEV_DETACHED;
+	ml_dev_globals.nb_devs--;
+
+	return ret;
+}
 
 RTE_LOG_REGISTER_DEFAULT(rte_ml_dev_logtype, INFO);
diff --git a/lib/mldev/rte_mldev_core.h b/lib/mldev/rte_mldev_core.h
new file mode 100644
index 0000000000..1c989a5ecf
--- /dev/null
+++ b/lib/mldev/rte_mldev_core.h
@@ -0,0 +1,111 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2022 Marvell.
+ */
+
+#ifndef _RTE_MLDEV_INTERNAL_H_
+#define _RTE_MLDEV_INTERNAL_H_
+
+/**
+ * @file
+ *
+ * MLDEV internal header
+ *
+ * This file contains MLDEV private data structures and macros.
+ *
+ * @note
+ * These APIs are for MLDEV PMDs and library only.
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <stdint.h>
+
+#include <dev_driver.h>
+#include <rte_common.h>
+#include <rte_log.h>
+#include <rte_mldev.h>
+
+/* Device state */
+#define ML_DEV_DETACHED (0)
+#define ML_DEV_ATTACHED (1)
+
+/**
+ * @internal
+ *
+ * The data part, with no function pointers, associated with each device. This structure is safe to
+ * place in shared memory to be common among different processes in a multi-process configuration.
+ */
+struct rte_ml_dev_data {
+	/** Unique identifier name. */
+	char name[RTE_ML_STR_MAX];
+
+	/** Device ID for this instance. */
+	int16_t dev_id;
+
+	/** Socket ID where memory is allocated. */
+	int16_t socket_id;
+
+	/** Device state: STOPPED(0) / STARTED(1) */
+	__extension__ uint8_t dev_started : 1;
+
+	/** Number of device queue pairs. */
+	uint16_t nb_queue_pairs;
+
+	/** Number of ML models. */
+	uint16_t nb_models;
+
+	/** Array of pointers to queue pairs. */
+	void **queue_pairs;
+
+	/** Array of pointers to ML models. */
+	void **models;
+
+	/** PMD-specific private data. */
+	void *dev_private;
+
+	/** Reserved for future fields */
+	uint64_t reserved[3];
+} __rte_cache_aligned;
+
+/**
+ * @internal
+ *
+ * The data structure associated with each ML device.
+ */
+struct rte_ml_dev {
+	/** Pointer to device data. */
+	struct rte_ml_dev_data *data;
+
+	/** Backing RTE device. */
+	struct rte_device *device;
+
+	/** Flag indicating the device is attached. */
+	__extension__ uint8_t attached : 1;
+} __rte_cache_aligned;
+
+/**
+ * @internal
+ *
+ * Global structure used for maintaining state of allocated ML devices.
+ */
+struct rte_ml_dev_global {
+	/** Device information array. */
+	struct rte_ml_dev *devs;
+
+	/** Device private data array. */
+	struct rte_ml_dev_data *data[RTE_MLDEV_MAX_DEVS];
+
+	/** Number of devices found. */
+	uint8_t nb_devs;
+
+	/** Maximum number of devices. */
+	uint8_t max_devs;
+};
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_MLDEV_INTERNAL_H_ */
diff --git a/lib/mldev/rte_mldev_pmd.c b/lib/mldev/rte_mldev_pmd.c
new file mode 100644
index 0000000000..3169e5d4fa
--- /dev/null
+++ b/lib/mldev/rte_mldev_pmd.c
@@ -0,0 +1,62 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2022 Marvell.
+ */
+
+#include <dev_driver.h>
+#include <rte_eal.h>
+#include <rte_malloc.h>
+
+#include "rte_mldev_pmd.h"
+
+struct rte_ml_dev *
+rte_ml_dev_pmd_create(const char *name, struct rte_device *device,
+		      struct rte_ml_dev_pmd_init_params *params)
+{
+	struct rte_ml_dev *dev;
+
+	RTE_MLDEV_LOG(INFO, "ML device initialisation - name: %s, socket_id: %u", name,
+		      params->socket_id);
+
+	/* Allocate device structure */
+	dev = rte_ml_dev_pmd_allocate(name, params->socket_id);
+	if (dev == NULL) {
+		RTE_MLDEV_LOG(ERR, "Failed to allocate ML device for %s", name);
+		return NULL;
+	}
+
+	/* Allocate private device structure */
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		dev->data->dev_private =
+			rte_zmalloc_socket("ml_dev_private", params->private_data_size,
+					   RTE_CACHE_LINE_SIZE, params->socket_id);
+
+		if (dev->data->dev_private == NULL) {
+			RTE_MLDEV_LOG(ERR, "Cannot allocate memory for mldev %s private data",
+				      name);
+			rte_ml_dev_pmd_release(dev);
+			return NULL;
+		}
+	}
+	dev->device = device;
+
+	return dev;
+}
+
+int
+rte_ml_dev_pmd_destroy(struct rte_ml_dev *dev)
+{
+	int ret;
+
+	RTE_MLDEV_LOG(INFO, "Releasing ML device - name: %s", dev->device->name);
+	ret = rte_ml_dev_pmd_release(dev);
+	if (ret)
+		return ret;
+
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		rte_free(dev->data->dev_private);
+
+	dev->data = NULL;
+	dev->device = NULL;
+
+	return 0;
+}
diff --git a/lib/mldev/rte_mldev_pmd.h b/lib/mldev/rte_mldev_pmd.h
new file mode 100644
index 0000000000..33544f1b80
--- /dev/null
+++ b/lib/mldev/rte_mldev_pmd.h
@@ -0,0 +1,149 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2022 Marvell.
+ */
+
+#ifndef _RTE_MLDEV_PMD_H_
+#define _RTE_MLDEV_PMD_H_
+
+/**
+ * @file
+ *
+ * RTE MLDEV PMD APIs
+ *
+ * ML Device PMD interface
+ *
+ * @note
+ * These APIs are for MLDEV PMDs only and user applications should not call them directly.
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <stdint.h>
+
+#include <rte_common.h>
+#include <rte_compat.h>
+#include <rte_mldev.h>
+#include <rte_mldev_core.h>
+
+/**
+ * @internal
+ *
+ * Initialisation parameters for ML devices.
+ */
+struct rte_ml_dev_pmd_init_params {
+	/** Socket to use for memory allocation. */
+	uint8_t socket_id;
+
+	/** Size of device private data. */
+	uint64_t private_data_size;
+};
+
+/**
+ * @internal
+ *
+ * Get the ML device pointer for the device. Assumes a valid device index.
+ *
+ * @param dev_id
+ *	Device ID value to select the device structure.
+ *
+ * @return
+ *	The rte_ml_dev pointer for the given device ID.
+ */
+__rte_internal
+struct rte_ml_dev *
+rte_ml_dev_pmd_get_dev(int16_t dev_id);
+
+/**
+ * @internal
+ *
+ * Get the rte_ml_dev structure device pointer for the named device.
+ *
+ * @param name
+ *	Device name to select the device structure.
+ *
+ * @return
+ *	The rte_ml_dev pointer for the given device ID.
+ */
+__rte_internal
+struct rte_ml_dev *
+rte_ml_dev_pmd_get_named_dev(const char *name);
+
+/**
+ * @internal
+ *
+ * Allocates a new mldev slot for an ML device and returns the pointer to that slot for use.
+ * Function for internal use by dummy drivers.
+ *
+ * @param name
+ *	Unique identifier name for each device.
+ * @param socket_id
+ *	Socket to allocate resources.
+ *
+ * @return
+ *	Slot in the rte_ml_dev_devices array for a new device.
+ */
+__rte_internal
+struct rte_ml_dev *
+rte_ml_dev_pmd_allocate(const char *name, uint8_t socket_id);
+
+/**
+ * @internal
+ *
+ * Release the specified mldev device.
+ *
+ * @param dev
+ *	ML device.
+ * @return
+ *	- 0 on success.
+ *	- < 0, error code on failure.
+ */
+__rte_internal
+int
+rte_ml_dev_pmd_release(struct rte_ml_dev *dev);
+
+/**
+ * @internal
+ *
+ * PMD assist function to provide boiler plate code for ML driver to create and allocate resources
+ * for a new ML PMD device instance.
+ *
+ * @param name
+ *	ML device name.
+ * @param device
+ *	Base device handle.
+ * @param params
+ *	PMD initialisation parameters.
+ *
+ * @return
+ *	- ML device instance on success.
+ *	- NULL on failure.
+ */
+__rte_internal
+struct rte_ml_dev *
+rte_ml_dev_pmd_create(const char *name, struct rte_device *device,
+		      struct rte_ml_dev_pmd_init_params *params);
+
+/**
+ * @internal
+ *
+ * PMD assist function to provide boiler plate code for ML driver to destroy and free resources
+ * associated with a ML PMD device instance.
+ *
+ * @param mldev
+ *	ML device instance.
+ *
+ * @return
+ *	- 0 on success.
+ *	- < 0, error code on failure.
+ */
+__rte_internal
+int
+rte_ml_dev_pmd_destroy(struct rte_ml_dev *mldev);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_MLDEV_PMD_H_ */
diff --git a/lib/mldev/version.map b/lib/mldev/version.map
index 3793380442..98a5e7d117 100644
--- a/lib/mldev/version.map
+++ b/lib/mldev/version.map
@@ -5,3 +5,14 @@  EXPERIMENTAL {
 
 	local: *;
 };
+
+INTERNAL {
+	global:
+
+	rte_ml_dev_pmd_allocate;
+	rte_ml_dev_pmd_create;
+	rte_ml_dev_pmd_destroy;
+	rte_ml_dev_pmd_get_dev;
+	rte_ml_dev_pmd_get_named_dev;
+	rte_ml_dev_pmd_release;
+};