[v5,2/3] common/qat: decouple pmds from the common code

Message ID 20240301155251.5101-2-arkadiuszx.kusztal@intel.com (mailing list archive)
State Accepted, archived
Delegated to: akhil goyal
Headers
Series [v5,1/3] common/qat: isolate parser arguments configuration |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Arkadiusz Kusztal March 1, 2024, 3:52 p.m. UTC
  Service specific functions were moved to services
files. Weak symbols for device create/destroy were removed,
named private devs were replaced by an opaque array.

Signed-off-by: Arkadiusz Kusztal <arkadiuszx.kusztal@intel.com>
---
 drivers/common/qat/qat_device.c     | 112 ++++++++++--------------------------
 drivers/common/qat/qat_device.h     |  47 ++++-----------
 drivers/compress/qat/qat_comp_pmd.c |  34 +++++------
 drivers/compress/qat/qat_comp_pmd.h |   7 ---
 drivers/crypto/qat/qat_asym.c       |  20 ++++---
 drivers/crypto/qat/qat_sym.c        |  19 +++---
 6 files changed, 83 insertions(+), 156 deletions(-)
  

Comments

Brian Dooley March 1, 2024, 4:08 p.m. UTC | #1
> -----Original Message-----
> From: Arkadiusz Kusztal <arkadiuszx.kusztal@intel.com>
> Sent: Friday, March 1, 2024 3:53 PM
> To: dev@dpdk.org
> Cc: gakhil@marvell.com; Power, Ciara <ciara.power@intel.com>; Kusztal,
> ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Subject: [PATCH v5 2/3] common/qat: decouple pmds from the common
> code
> 
> Service specific functions were moved to services files. Weak symbols for
> device create/destroy were removed, named private devs were replaced by an
> opaque array.
> 
> Signed-off-by: Arkadiusz Kusztal <arkadiuszx.kusztal@intel.com>
> ---
>  drivers/common/qat/qat_device.c     | 112 ++++++++++-------------------------
> -
>  drivers/common/qat/qat_device.h     |  47 ++++-----------
>  drivers/compress/qat/qat_comp_pmd.c |  34 +++++------
>  drivers/compress/qat/qat_comp_pmd.h |   7 ---
>  drivers/crypto/qat/qat_asym.c       |  20 ++++---
>  drivers/crypto/qat/qat_sym.c        |  19 +++---
>  6 files changed, 83 insertions(+), 156 deletions(-)
> 
<snip>

Acked-by: Brian Dooley <brian.dooley@intel.com>
  

Patch

diff --git a/drivers/common/qat/qat_device.c b/drivers/common/qat/qat_device.c
index e1f1060535..a27252ea4d 100644
--- a/drivers/common/qat/qat_device.c
+++ b/drivers/common/qat/qat_device.c
@@ -26,6 +26,8 @@ 
 struct qat_gen_hw_data qat_gen_config[QAT_N_GENS];
 struct qat_dev_hw_spec_funcs *qat_dev_hw_spec[QAT_N_GENS];
 
+struct qat_service qat_service[QAT_MAX_SERVICES];
+
 /* per-process array of device data */
 struct qat_device_info qat_pci_devs[RTE_PMD_QAT_MAX_PCI_DEVICES];
 static int qat_nb_pci_devices;
@@ -120,7 +122,7 @@  qat_pci_find_free_device_index(void)
 		return dev_id;
 }
 
-struct qat_pci_device *
+static struct qat_pci_device *
 qat_get_qat_dev_from_pci_dev(struct rte_pci_device *pci_dev)
 {
 	char name[QAT_DEV_NAME_MAX_LEN];
@@ -308,7 +310,8 @@  qat_pci_device_allocate(struct rte_pci_device *pci_dev)
 		return NULL;
 	}
 
-	qat_dev_size = sizeof(struct qat_pci_device) + extra_size;
+	qat_dev_size = sizeof(struct qat_pci_device) + sizeof(void *) *
+				QAT_MAX_SERVICES + extra_size;
 	qat_dev_mz = rte_memzone_reserve(name, qat_dev_size,
 		rte_socket_id(), 0);
 
@@ -400,7 +403,7 @@  qat_pci_device_release(struct rte_pci_device *pci_dev)
 {
 	struct qat_pci_device *qat_dev;
 	char name[QAT_DEV_NAME_MAX_LEN];
-	int busy = 0;
+	int busy = 0, i;
 
 	if (pci_dev == NULL)
 		return -EINVAL;
@@ -415,24 +418,16 @@  qat_pci_device_release(struct rte_pci_device *pci_dev)
 		/* Check that there are no service devs still on pci device */
 
 		if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
-			if (qat_dev->sym_dev != NULL) {
-				QAT_LOG(DEBUG, "QAT sym device %s is busy",
-					name);
-				busy = 1;
-			}
-			if (qat_dev->asym_dev != NULL) {
-				QAT_LOG(DEBUG, "QAT asym device %s is busy",
-					name);
-				busy = 1;
-			}
-			if (qat_dev->comp_dev != NULL) {
-				QAT_LOG(DEBUG, "QAT comp device %s is busy",
-					name);
+			for (i = 0; i < QAT_MAX_SERVICES; i++) {
+				if (qat_dev->pmd[i] == NULL)
+					continue;
+				QAT_LOG(DEBUG, "QAT %s device %s is busy",
+					qat_service[i].name, name);
 				busy = 1;
-			}
 			if (busy)
 				return -EBUSY;
 			rte_memzone_free(inst->mz);
+			}
 		}
 		memset(inst, 0, sizeof(struct qat_device_info));
 		qat_nb_pci_devices--;
@@ -446,17 +441,20 @@  static int
 qat_pci_dev_destroy(struct qat_pci_device *qat_pci_dev,
 		struct rte_pci_device *pci_dev)
 {
-	qat_sym_dev_destroy(qat_pci_dev);
-	qat_comp_dev_destroy(qat_pci_dev);
-	qat_asym_dev_destroy(qat_pci_dev);
+	int i;
+
+	for (i = 0; i < QAT_MAX_SERVICES; i++) {
+		if (!qat_service[i].dev_create)
+			continue;
+		qat_service[i].dev_destroy(qat_pci_dev);
+	}
 	return qat_pci_device_release(pci_dev);
 }
 
 static int qat_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 		struct rte_pci_device *pci_dev)
 {
-	int sym_ret = 0, asym_ret = 0, comp_ret = 0;
-	int num_pmds_created = 0;
+	int i, ret = 0, num_pmds_created = 0;
 	struct qat_pci_device *qat_pci_dev;
 
 	QAT_LOG(DEBUG, "Found QAT device at %02x:%02x.%x",
@@ -468,30 +466,18 @@  static int qat_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	if (qat_pci_dev == NULL)
 		return -ENODEV;
 
-	sym_ret = qat_sym_dev_create(qat_pci_dev);
-	if (sym_ret == 0) {
-		num_pmds_created++;
-	}
-	else
-		QAT_LOG(WARNING,
-				"Failed to create QAT SYM PMD on device %s",
-				qat_pci_dev->name);
-
-	comp_ret = qat_comp_dev_create(qat_pci_dev);
-	if (comp_ret == 0)
-		num_pmds_created++;
-	else
-		QAT_LOG(WARNING,
-				"Failed to create QAT COMP PMD on device %s",
-				qat_pci_dev->name);
-
-	asym_ret = qat_asym_dev_create(qat_pci_dev);
-	if (asym_ret == 0)
-		num_pmds_created++;
-	else
-		QAT_LOG(WARNING,
-				"Failed to create QAT ASYM PMD on device %s",
+	for (i = 0; i < QAT_MAX_SERVICES; i++) {
+		if (!qat_service[i].dev_create)
+			continue;
+		ret = qat_service[i].dev_create(qat_pci_dev);
+		if (ret == 0)
+			num_pmds_created++;
+		else {
+			QAT_LOG(WARNING, "Failed to create %s PMD on device %s",
+				qat_service[i].name,
 				qat_pci_dev->name);
+		}
+	}
 
 	if (num_pmds_created == 0)
 		qat_pci_dev_destroy(qat_pci_dev, pci_dev);
@@ -521,42 +507,6 @@  static struct rte_pci_driver rte_qat_pmd = {
 	.remove = qat_pci_remove
 };
 
-__rte_weak int
-qat_sym_dev_create(struct qat_pci_device *qat_pci_dev __rte_unused)
-{
-	return 0;
-}
-
-__rte_weak int
-qat_asym_dev_create(struct qat_pci_device *qat_pci_dev __rte_unused)
-{
-	return 0;
-}
-
-__rte_weak int
-qat_sym_dev_destroy(struct qat_pci_device *qat_pci_dev __rte_unused)
-{
-	return 0;
-}
-
-__rte_weak int
-qat_asym_dev_destroy(struct qat_pci_device *qat_pci_dev __rte_unused)
-{
-	return 0;
-}
-
-__rte_weak int
-qat_comp_dev_create(struct qat_pci_device *qat_pci_dev __rte_unused)
-{
-	return 0;
-}
-
-__rte_weak int
-qat_comp_dev_destroy(struct qat_pci_device *qat_pci_dev __rte_unused)
-{
-	return 0;
-}
-
 RTE_PMD_REGISTER_PCI(QAT_PCI_NAME, rte_qat_pmd);
 RTE_PMD_REGISTER_PCI_TABLE(QAT_PCI_NAME, pci_id_qat_map);
 RTE_PMD_REGISTER_KMOD_DEP(QAT_PCI_NAME, "* igb_uio | uio_pci_generic | vfio-pci");
diff --git a/drivers/common/qat/qat_device.h b/drivers/common/qat/qat_device.h
index 8d332e0d0b..9275156ef8 100644
--- a/drivers/common/qat/qat_device.h
+++ b/drivers/common/qat/qat_device.h
@@ -19,6 +19,14 @@ 
 #define MAX_QP_THRESHOLD_SIZE	32
 #define QAT_LEGACY_CAPA "qat_legacy_capa"
 
+struct qat_service {
+	const char *name;
+	int (*dev_create)(struct qat_pci_device *qat_pci_dev);
+	int (*dev_destroy)(struct qat_pci_device *qat_pci_dev);
+};
+
+extern struct qat_service qat_service[];
+
 /**
  * Function prototypes for GENx specific device operations.
  **/
@@ -104,27 +112,12 @@  struct qat_pci_device {
 	/**< QAT device generation */
 	rte_spinlock_t arb_csr_lock;
 	/**< lock to protect accesses to the arbiter CSR */
-
 	struct qat_qp *qps_in_use[QAT_MAX_SERVICES][ADF_MAX_QPS_ON_ANY_SERVICE];
 	/**< links to qps set up for each service, index same as on API */
-
-	/* Data relating to symmetric crypto service */
-	struct qat_cryptodev_private *sym_dev;
-	/**< link back to cryptodev private data */
-
 	int qat_sym_driver_id;
 	/**< Symmetric driver id used by this device */
-
-	/* Data relating to asymmetric crypto service */
-	struct qat_cryptodev_private *asym_dev;
-	/**< link back to cryptodev private data */
-
 	int qat_asym_driver_id;
 	/**< Symmetric driver id used by this device */
-
-	/* Data relating to compression service */
-	struct qat_comp_dev_private *comp_dev;
-	/**< link back to compressdev private data */
 	void *misc_bar_io_addr;
 	/**< Address of misc bar */
 	void *dev_private;
@@ -135,6 +128,8 @@  struct qat_pci_device {
 	/**< Wireless Slices supported */
 	char *command_line;
 	/**< Map of the crypto and compression slices */
+	void *pmd[QAT_MAX_SERVICES];
+	/**< link back to pmd private data */
 };
 
 struct qat_gen_hw_data {
@@ -154,26 +149,4 @@  struct qat_pf2vf_dev {
 
 extern struct qat_gen_hw_data qat_gen_config[];
 
-struct qat_pci_device *
-qat_get_qat_dev_from_pci_dev(struct rte_pci_device *pci_dev);
-
-/* declaration needed for weak functions */
-int
-qat_sym_dev_create(struct qat_pci_device *qat_pci_dev __rte_unused);
-
-int
-qat_asym_dev_create(struct qat_pci_device *qat_pci_dev);
-
-int
-qat_sym_dev_destroy(struct qat_pci_device *qat_pci_dev __rte_unused);
-
-int
-qat_asym_dev_destroy(struct qat_pci_device *qat_pci_dev __rte_unused);
-
-int
-qat_comp_dev_create(struct qat_pci_device *qat_pci_dev __rte_unused);
-
-int
-qat_comp_dev_destroy(struct qat_pci_device *qat_pci_dev __rte_unused);
-
 #endif /* _QAT_DEVICE_H_ */
diff --git a/drivers/compress/qat/qat_comp_pmd.c b/drivers/compress/qat/qat_comp_pmd.c
index 4b24929a70..c5f7c7dc67 100644
--- a/drivers/compress/qat/qat_comp_pmd.c
+++ b/drivers/compress/qat/qat_comp_pmd.c
@@ -636,22 +636,24 @@  qat_comp_pmd_dequeue_first_op_burst(void *qp, struct rte_comp_op **ops,
 {
 	uint16_t ret = qat_comp_dequeue_burst(qp, ops, nb_ops);
 	struct qat_qp *tmp_qp = (struct qat_qp *)qp;
+	struct qat_comp_dev_private *dev =
+		tmp_qp->qat_dev->pmd[QAT_SERVICE_COMPRESSION];
 
 	if (ret) {
 		if ((*ops)->debug_status ==
 				(uint64_t)ERR_CODE_QAT_COMP_WRONG_FW) {
-			tmp_qp->qat_dev->comp_dev->compressdev->enqueue_burst =
+			dev->compressdev->enqueue_burst =
 					qat_comp_pmd_enq_deq_dummy_op_burst;
-			tmp_qp->qat_dev->comp_dev->compressdev->dequeue_burst =
+			dev->compressdev->dequeue_burst =
 					qat_comp_pmd_enq_deq_dummy_op_burst;
 
-			tmp_qp->qat_dev->comp_dev->compressdev->dev_ops =
+			dev->compressdev->dev_ops =
 					&compress_qat_dummy_ops;
 			QAT_LOG(ERR,
 					"This QAT hardware doesn't support compression operation");
 
 		} else {
-			tmp_qp->qat_dev->comp_dev->compressdev->dequeue_burst =
+			dev->compressdev->dequeue_burst =
 					qat_comp_dequeue_burst;
 		}
 	}
@@ -669,7 +671,7 @@  static const struct rte_driver compdev_qat_driver = {
 	.alias = qat_comp_drv_name
 };
 
-int
+static int
 qat_comp_dev_create(struct qat_pci_device *qat_pci_dev)
 {
 	struct qat_device_info *qat_dev_instance =
@@ -779,7 +781,7 @@  qat_comp_dev_create(struct qat_pci_device *qat_pci_dev)
 			MAX_QP_THRESHOLD_SIZE :
 			atoi(cmdline);
 	}
-	qat_pci_dev->comp_dev = comp_dev;
+	qat_pci_dev->pmd[QAT_SERVICE_COMPRESSION] = comp_dev;
 
 	QAT_LOG(DEBUG,
 		    "Created QAT COMP device %s as compressdev instance %d",
@@ -787,26 +789,23 @@  qat_comp_dev_create(struct qat_pci_device *qat_pci_dev)
 	return 0;
 }
 
-int
+static int
 qat_comp_dev_destroy(struct qat_pci_device *qat_pci_dev)
 {
-	struct qat_comp_dev_private *comp_dev;
+	struct qat_comp_dev_private *dev =
+		qat_pci_dev->pmd[QAT_SERVICE_COMPRESSION];
 
 	if (qat_pci_dev == NULL)
 		return -ENODEV;
 
-	comp_dev = qat_pci_dev->comp_dev;
-	if (comp_dev == NULL)
-		return 0;
-
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
-		rte_memzone_free(qat_pci_dev->comp_dev->capa_mz);
+		rte_memzone_free(dev->capa_mz);
 
 	/* clean up any resources used by the device */
-	qat_comp_dev_close(comp_dev->compressdev);
+	qat_comp_dev_close(dev->compressdev);
 
-	rte_compressdev_pmd_destroy(comp_dev->compressdev);
-	qat_pci_dev->comp_dev = NULL;
+	rte_compressdev_pmd_destroy(dev->compressdev);
+	qat_pci_dev->pmd[QAT_SERVICE_COMPRESSION] = NULL;
 
 	return 0;
 }
@@ -814,4 +813,7 @@  qat_comp_dev_destroy(struct qat_pci_device *qat_pci_dev)
 RTE_INIT(qat_sym_init)
 {
 	qat_cmdline_defines[QAT_SERVICE_COMPRESSION] = arguments;
+	qat_service[QAT_SERVICE_COMPRESSION].name = "symmetric crypto";
+	qat_service[QAT_SERVICE_COMPRESSION].dev_create = qat_comp_dev_create;
+	qat_service[QAT_SERVICE_COMPRESSION].dev_destroy = qat_comp_dev_destroy;
 }
diff --git a/drivers/compress/qat/qat_comp_pmd.h b/drivers/compress/qat/qat_comp_pmd.h
index 1f5b0facf7..01a8983287 100644
--- a/drivers/compress/qat/qat_comp_pmd.h
+++ b/drivers/compress/qat/qat_comp_pmd.h
@@ -106,13 +106,6 @@  const struct rte_memzone *
 qat_comp_setup_inter_buffers(struct qat_comp_dev_private *comp_dev,
 		uint32_t buff_size);
 
-int
-qat_comp_dev_create(struct qat_pci_device *qat_pci_dev);
-
-int
-qat_comp_dev_destroy(struct qat_pci_device *qat_pci_dev);
-
-
 static __rte_always_inline unsigned int
 qat_comp_get_num_im_bufs_required(enum qat_device_gen gen)
 {
diff --git a/drivers/crypto/qat/qat_asym.c b/drivers/crypto/qat/qat_asym.c
index 7d01ad503b..8ec9f65156 100644
--- a/drivers/crypto/qat/qat_asym.c
+++ b/drivers/crypto/qat/qat_asym.c
@@ -1505,7 +1505,7 @@  qat_asym_init_op_cookie(void *op_cookie)
 	}
 }
 
-int
+static int
 qat_asym_dev_create(struct qat_pci_device *qat_pci_dev)
 {
 	struct qat_cryptodev_private *internals;
@@ -1614,31 +1614,32 @@  qat_asym_dev_create(struct qat_pci_device *qat_pci_dev)
 		return -1;
 	}
 
-	qat_pci_dev->asym_dev = internals;
+	qat_pci_dev->pmd[QAT_SERVICE_ASYMMETRIC] = internals;
 	internals->service_type = QAT_SERVICE_ASYMMETRIC;
 	QAT_LOG(DEBUG, "Created QAT ASYM device %s as cryptodev instance %d",
 			cryptodev->data->name, internals->dev_id);
 	return 0;
 }
 
-int
+static int
 qat_asym_dev_destroy(struct qat_pci_device *qat_pci_dev)
 {
 	struct rte_cryptodev *cryptodev;
+	struct qat_cryptodev_private *dev =
+		qat_pci_dev->pmd[QAT_SERVICE_ASYMMETRIC];
 
 	if (qat_pci_dev == NULL)
 		return -ENODEV;
-	if (qat_pci_dev->asym_dev == NULL)
+	if (dev == NULL)
 		return 0;
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
-		rte_memzone_free(qat_pci_dev->asym_dev->capa_mz);
+		rte_memzone_free(dev->capa_mz);
 
 	/* free crypto device */
-	cryptodev = rte_cryptodev_pmd_get_dev(
-			qat_pci_dev->asym_dev->dev_id);
+	cryptodev = rte_cryptodev_pmd_get_dev(dev->dev_id);
 	rte_cryptodev_pmd_destroy(cryptodev);
 	qat_pci_devs[qat_pci_dev->qat_dev_id].asym_rte_dev.name = NULL;
-	qat_pci_dev->asym_dev = NULL;
+	qat_pci_dev->pmd[QAT_SERVICE_ASYMMETRIC] = NULL;
 
 	return 0;
 }
@@ -1651,4 +1652,7 @@  RTE_PMD_REGISTER_CRYPTO_DRIVER(qat_crypto_drv,
 RTE_INIT(qat_asym_init)
 {
 	qat_cmdline_defines[QAT_SERVICE_ASYMMETRIC] = arguments;
+	qat_service[QAT_SERVICE_ASYMMETRIC].name = "asymmetric crypto";
+	qat_service[QAT_SERVICE_ASYMMETRIC].dev_create = qat_asym_dev_create;
+	qat_service[QAT_SERVICE_ASYMMETRIC].dev_destroy = qat_asym_dev_destroy;
 }
diff --git a/drivers/crypto/qat/qat_sym.c b/drivers/crypto/qat/qat_sym.c
index 2d7c737de7..efca8f3ba1 100644
--- a/drivers/crypto/qat/qat_sym.c
+++ b/drivers/crypto/qat/qat_sym.c
@@ -199,7 +199,7 @@  qat_sym_dequeue_burst_gen_lce(void *qp, struct rte_crypto_op **ops, uint16_t nb_
 	return qat_dequeue_op_burst(qp, (void **)ops, qat_sym_process_response_gen_lce, nb_ops);
 }
 
-int
+static int
 qat_sym_dev_create(struct qat_pci_device *qat_pci_dev)
 {
 	int ret = 0;
@@ -326,7 +326,7 @@  qat_sym_dev_create(struct qat_pci_device *qat_pci_dev)
 		goto error;
 	}
 	internals->service_type = QAT_SERVICE_SYMMETRIC;
-	qat_pci_dev->sym_dev = internals;
+	qat_pci_dev->pmd[QAT_SERVICE_SYMMETRIC] = internals;
 	QAT_LOG(DEBUG, "Created QAT SYM device %s as cryptodev instance %d",
 			cryptodev->data->name, internals->dev_id);
 
@@ -342,25 +342,27 @@  qat_sym_dev_create(struct qat_pci_device *qat_pci_dev)
 	return ret;
 }
 
-int
+static int
 qat_sym_dev_destroy(struct qat_pci_device *qat_pci_dev)
 {
 	struct rte_cryptodev *cryptodev;
+	struct qat_cryptodev_private *dev =
+		qat_pci_dev->pmd[QAT_SERVICE_SYMMETRIC];
 
 	if (qat_pci_dev == NULL)
 		return -ENODEV;
-	if (qat_pci_dev->sym_dev == NULL)
+	if (dev == NULL)
 		return 0;
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
-		rte_memzone_free(qat_pci_dev->sym_dev->capa_mz);
+		rte_memzone_free(dev->capa_mz);
 
 	/* free crypto device */
-	cryptodev = rte_cryptodev_pmd_get_dev(qat_pci_dev->sym_dev->dev_id);
+	cryptodev = rte_cryptodev_pmd_get_dev(dev->dev_id);
 	rte_free(cryptodev->security_ctx);
 	cryptodev->security_ctx = NULL;
 	rte_cryptodev_pmd_destroy(cryptodev);
 	qat_pci_devs[qat_pci_dev->qat_dev_id].sym_rte_dev.name = NULL;
-	qat_pci_dev->sym_dev = NULL;
+	qat_pci_dev->pmd[QAT_SERVICE_SYMMETRIC] = NULL;
 
 	return 0;
 }
@@ -421,4 +423,7 @@  RTE_PMD_REGISTER_CRYPTO_DRIVER(qat_crypto_drv,
 RTE_INIT(qat_sym_init)
 {
 	qat_cmdline_defines[QAT_SERVICE_SYMMETRIC] = arguments;
+	qat_service[QAT_SERVICE_SYMMETRIC].name = "symmetric crypto";
+	qat_service[QAT_SERVICE_SYMMETRIC].dev_create = qat_sym_dev_create;
+	qat_service[QAT_SERVICE_SYMMETRIC].dev_destroy = qat_sym_dev_destroy;
 }