[v1,1/7] crypro/qat: qat driver refactor skeleton

Message ID 20211026172518.20183-2-kai.ji@intel.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series drivers/qat: QAT symmetric crypto datapatch rework |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-testing warning apply patch failure

Commit Message

Ji, Kai Oct. 26, 2021, 5:25 p.m. UTC
  Add-in enqueue/dequeue op burst and build-request skeleton functions
for qat crypto driver refactor.

Signed-off-by: Kai Ji <kai.ji@intel.com>
---
 drivers/common/qat/qat_qp.c          | 16 +++++++++++++
 drivers/common/qat/qat_qp.h          | 22 +++++++++++++++++
 drivers/crypto/qat/qat_asym.c        | 35 ++++++++++++++++++++++++++++
 drivers/crypto/qat/qat_asym.h        | 15 ++++++++++++
 drivers/crypto/qat/qat_sym.c         | 25 ++++++++++++++++++++
 drivers/crypto/qat/qat_sym.h         | 16 +++++++++++++
 drivers/crypto/qat/qat_sym_session.h |  6 +++++
 7 files changed, 135 insertions(+)
  

Comments

Fan Zhang Oct. 29, 2021, 1:58 p.m. UTC | #1
Hi Kai,

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Kai Ji
> Sent: Tuesday, October 26, 2021 6:25 PM
> To: dev@dpdk.org
> Cc: Ji, Kai <kai.ji@intel.com>
> Subject: [dpdk-dev] [dpdk-dev v1 1/7] crypro/qat: qat driver refactor
> skeleton
> 
> Add-in enqueue/dequeue op burst and build-request skeleton functions
> for qat crypto driver refactor.
> 
> Signed-off-by: Kai Ji <kai.ji@intel.com>
> ---

...

> +/**
> + * @brief Function prototype to build a QAT request.
> + * @param opaque: an opaque data may be used to store context may be
> useful
> + *    between 2 enqueue operations.
> + **/
The comments for the function prototype can be polished to make it clearer.
Also the formatting shall be inline with the rest of dpdk code. Such as

/**
 * <brief introduction to the function prototype>
 *
 * @param x
 *   Description to the param x.
 * @return
 *   - 0 if the crypto request is built successfully.
*    - error code otherwise.
 */


> +typedef int (*qat_op_build_request_t)(void *in_op, uint8_t *out_msg,
> +		void *op_cookie, uint64_t *opaque, enum qat_device_gen
> dev_gen);
> +
> +typedef int (*qat_op_dequeue_t)(void **op, uint8_t *resp, void
> *op_cookie,
> +		uint64_t *dequeue_err_count __rte_unused);

The above function prototype worth some description too?

> +
> +#define QAT_BUILD_REQUEST_MAX_OPAQUE_SIZE	2
> +
>  struct qat_qp {
>  	void			*mmap_bar_addr;
>  	struct qat_queue	tx_q;
> @@ -44,6 +57,7 @@ struct qat_qp {
>  	struct rte_mempool *op_cookie_pool;
>  	void **op_cookies;
>  	uint32_t nb_descriptors;
> +	uint64_t opaque[QAT_BUILD_REQUEST_MAX_OPAQUE_SIZE];
>  	enum qat_device_gen qat_dev_gen;
>  	enum qat_service_type service_type;
>  	struct qat_pci_device *qat_dev;
> @@ -77,6 +91,14 @@ struct qat_qp_config {
>  	const char *service_str;
>  };
> 
> +uint16_t
> +refactor_qat_enqueue_op_burst(void *qp, qat_op_build_request_t
> op_build_request,
> +		void **ops, uint16_t nb_ops);
> +
> +uint16_t
> +refactor_qat_dequeue_op_burst(void *qp, void **ops,
> +		qat_op_dequeue_t qat_dequeue_process_response,
> uint16_t nb_ops);
> +
>  uint16_t
>  qat_enqueue_op_burst(void *qp, void **ops, uint16_t nb_ops);
> 
> diff --git a/drivers/crypto/qat/qat_asym.c b/drivers/crypto/qat/qat_asym.c
> index 85973812a8..d5b4c66d68 100644
> --- a/drivers/crypto/qat/qat_asym.c
> +++ b/drivers/crypto/qat/qat_asym.c
> @@ -456,6 +456,41 @@ qat_asym_fill_arrays(struct rte_crypto_asym_op
> *asym_op,
>  	return 0;
>  }
> 
> +static __rte_always_inline int
> +refactor_qat_asym_build_request(__rte_unused void *in_op,
> +		__rte_unused uint8_t *out_msg, __rte_unused void
> *op_cookie,
> +		__rte_unused uint64_t *opaque,
> +		__rte_unused enum qat_device_gen dev_gen)
> +{
> +	return 0;
> +}
> +
> +int
> +refactor_qat_asym_process_response(__rte_unused void **op,
> +		__rte_unused uint8_t *resp,
> +		__rte_unused void *op_cookie,
> +		__rte_unused uint64_t *dequeue_err_count)
> +{
> +	return 0;
> +}

I assume the functions refactor_xxx() is removed in future patches right?
Maybe adding some comments to help reviewing.

> +
> +uint16_t
> +qat_asym_crypto_enqueue_op_burst(void *qp, struct rte_crypto_op
> **ops,
> +		uint16_t nb_ops)
> +{
> +	return refactor_qat_enqueue_op_burst(qp,
> +					refactor_qat_asym_build_request,
> +					(void **)ops, nb_ops);
> +}
> +
> +uint16_t
> +qat_asym_crypto_dequeue_op_burst(void *qp, struct rte_crypto_op
> **ops,
> +		uint16_t nb_ops)
> +{
> +	return refactor_qat_dequeue_op_burst(qp, (void **)ops,
> +				refactor_qat_asym_process_response,
> nb_ops);
> +}
> +
>  int
>  qat_asym_build_request(void *in_op,
>  			uint8_t *out_msg,
> diff --git a/drivers/crypto/qat/qat_asym.h b/drivers/crypto/qat/qat_asym.h
> index 308b6b2e0b..50c2641eba 100644
> --- a/drivers/crypto/qat/qat_asym.h
> +++ b/drivers/crypto/qat/qat_asym.h
> @@ -92,4 +92,19 @@ void
>  qat_asym_process_response(void __rte_unused **op, uint8_t *resp,
>  		void *op_cookie);
> 
> +int
> +refactor_qat_asym_process_response(__rte_unused void **op,
> +		__rte_unused uint8_t *resp,
> +		__rte_unused void *op_cookie,
> +		__rte_unused uint64_t *dequeue_err_count);
> +
> +uint16_t
> +qat_asym_crypto_enqueue_op_burst(void *qp, struct rte_crypto_op
> **ops,
> +		uint16_t nb_ops);
> +
> +
> +uint16_t
> +qat_asym_crypto_dequeue_op_burst(void *qp, struct rte_crypto_op
> **ops,
> +		uint16_t nb_ops);
> +
>  #endif /* _QAT_ASYM_H_ */
> diff --git a/drivers/crypto/qat/qat_sym.c b/drivers/crypto/qat/qat_sym.c
> index 93b257522b..a92874cd27 100644
> --- a/drivers/crypto/qat/qat_sym.c
> +++ b/drivers/crypto/qat/qat_sym.c
> @@ -210,6 +210,31 @@ handle_spc_gmac(struct qat_sym_session *ctx,
> struct rte_crypto_op *op,
>  			ICP_QAT_FW_LA_NO_PROTO);
>  }
> 
> +static __rte_always_inline int
> +refactor_qat_sym_build_request(__rte_unused void *in_op,
> +		__rte_unused uint8_t *out_msg, __rte_unused	void
> *op_cookie,
> +		__rte_unused uint64_t *opaque,
> +		__rte_unused enum qat_device_gen dev_gen)
> +{
> +	return 0;
> +}
> +
> +uint16_t
> +refactor_qat_sym_enqueue_burst(void *qp, struct rte_crypto_op **ops,
> +		uint16_t nb_ops)
> +{
> +	return refactor_qat_enqueue_op_burst(qp,
> refactor_qat_sym_build_request,
> +			(void **)ops, nb_ops);
> +}
> +

Same as above, some comments would be helpful and shall remove altogether in
later patches.

> +uint16_t
> +refactor_qat_sym_dequeue_burst(void *qp, struct rte_crypto_op **ops,
> +		uint16_t nb_ops)
> +{
> +	return refactor_qat_dequeue_op_burst(qp, (void **)ops,
> +				refactor_qat_sym_process_response,
> nb_ops);
> +}
> +
>  int
>  qat_sym_build_request(void *in_op, uint8_t *out_msg,
>  		void *op_cookie, enum qat_device_gen qat_dev_gen)
> diff --git a/drivers/crypto/qat/qat_sym.h b/drivers/crypto/qat/qat_sym.h
> index e3ec7f0de4..17b2c871bd 100644
> --- a/drivers/crypto/qat/qat_sym.h
> +++ b/drivers/crypto/qat/qat_sym.h
> @@ -54,6 +54,22 @@ struct qat_sym_op_cookie {
>  	} opt;
>  };
> 
> +static __rte_always_inline int
> +refactor_qat_sym_process_response(__rte_unused void **op,
> +		__rte_unused uint8_t *resp, __rte_unused void *op_cookie,
> +		__rte_unused uint64_t *dequeue_err_count)
> +{
> +	return 0;
> +}
> +
> +uint16_t
> +refactor_qat_sym_enqueue_burst(void *qp, struct rte_crypto_op **ops,
> +		uint16_t nb_ops);
> +
> +uint16_t
> +refactor_qat_sym_dequeue_burst(void *qp, struct rte_crypto_op **ops,
> +		uint16_t nb_ops);
> +
>  int
>  qat_sym_build_request(void *in_op, uint8_t *out_msg,
>  		void *op_cookie, enum qat_device_gen qat_dev_gen);
> diff --git a/drivers/crypto/qat/qat_sym_session.h
> b/drivers/crypto/qat/qat_sym_session.h
> index 6ebc176729..73493ec864 100644
> --- a/drivers/crypto/qat/qat_sym_session.h
> +++ b/drivers/crypto/qat/qat_sym_session.h
> @@ -55,6 +55,11 @@
>  #define QAT_SESSION_IS_SLICE_SET(flags, flag)	\
>  	(!!((flags) & (flag)))
> 
> +struct qat_sym_session;
> +
> +typedef int (*qat_sym_build_request_t)(void *in_op, struct
> qat_sym_session *ctx,
> +		uint8_t *out_msg, void *op_cookie);
> +
>  enum qat_sym_proto_flag {
>  	QAT_CRYPTO_PROTO_FLAG_NONE = 0,
>  	QAT_CRYPTO_PROTO_FLAG_CCM = 1,
> @@ -107,6 +112,7 @@ struct qat_sym_session {
>  	/* Some generations need different setup of counter */
>  	uint32_t slice_types;
>  	enum qat_sym_proto_flag qat_proto_flag;
> +	qat_sym_build_request_t build_request[2];
>  };
> 
>  int
> --
> 2.17.1

All comments are towards the comments - as of the rest
Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
  

Patch

diff --git a/drivers/common/qat/qat_qp.c b/drivers/common/qat/qat_qp.c
index cde421eb77..0fda890075 100644
--- a/drivers/common/qat/qat_qp.c
+++ b/drivers/common/qat/qat_qp.c
@@ -549,6 +549,22 @@  adf_modulo(uint32_t data, uint32_t modulo_mask)
 	return data & modulo_mask;
 }
 
+uint16_t
+refactor_qat_enqueue_op_burst(__rte_unused void *qp,
+		__rte_unused qat_op_build_request_t op_build_request,
+		__rte_unused void **ops, __rte_unused uint16_t nb_ops)
+{
+	return 0;
+}
+
+uint16_t
+refactor_qat_dequeue_op_burst(__rte_unused void *qp, __rte_unused void **ops,
+		__rte_unused qat_op_dequeue_t qat_dequeue_process_response,
+		__rte_unused uint16_t nb_ops)
+{
+	return 0;
+}
+
 uint16_t
 qat_enqueue_op_burst(void *qp, void **ops, uint16_t nb_ops)
 {
diff --git a/drivers/common/qat/qat_qp.h b/drivers/common/qat/qat_qp.h
index deafb407b3..c5f115310c 100644
--- a/drivers/common/qat/qat_qp.h
+++ b/drivers/common/qat/qat_qp.h
@@ -36,6 +36,19 @@  struct qat_queue {
 	/* number of responses processed since last CSR head write */
 };
 
+/**
+ * @brief Function prototype to build a QAT request.
+ * @param opaque: an opaque data may be used to store context may be useful
+ *    between 2 enqueue operations.
+ **/
+typedef int (*qat_op_build_request_t)(void *in_op, uint8_t *out_msg,
+		void *op_cookie, uint64_t *opaque, enum qat_device_gen dev_gen);
+
+typedef int (*qat_op_dequeue_t)(void **op, uint8_t *resp, void *op_cookie,
+		uint64_t *dequeue_err_count __rte_unused);
+
+#define QAT_BUILD_REQUEST_MAX_OPAQUE_SIZE	2
+
 struct qat_qp {
 	void			*mmap_bar_addr;
 	struct qat_queue	tx_q;
@@ -44,6 +57,7 @@  struct qat_qp {
 	struct rte_mempool *op_cookie_pool;
 	void **op_cookies;
 	uint32_t nb_descriptors;
+	uint64_t opaque[QAT_BUILD_REQUEST_MAX_OPAQUE_SIZE];
 	enum qat_device_gen qat_dev_gen;
 	enum qat_service_type service_type;
 	struct qat_pci_device *qat_dev;
@@ -77,6 +91,14 @@  struct qat_qp_config {
 	const char *service_str;
 };
 
+uint16_t
+refactor_qat_enqueue_op_burst(void *qp, qat_op_build_request_t op_build_request,
+		void **ops, uint16_t nb_ops);
+
+uint16_t
+refactor_qat_dequeue_op_burst(void *qp, void **ops,
+		qat_op_dequeue_t qat_dequeue_process_response, uint16_t nb_ops);
+
 uint16_t
 qat_enqueue_op_burst(void *qp, void **ops, uint16_t nb_ops);
 
diff --git a/drivers/crypto/qat/qat_asym.c b/drivers/crypto/qat/qat_asym.c
index 85973812a8..d5b4c66d68 100644
--- a/drivers/crypto/qat/qat_asym.c
+++ b/drivers/crypto/qat/qat_asym.c
@@ -456,6 +456,41 @@  qat_asym_fill_arrays(struct rte_crypto_asym_op *asym_op,
 	return 0;
 }
 
+static __rte_always_inline int
+refactor_qat_asym_build_request(__rte_unused void *in_op,
+		__rte_unused uint8_t *out_msg, __rte_unused void *op_cookie,
+		__rte_unused uint64_t *opaque,
+		__rte_unused enum qat_device_gen dev_gen)
+{
+	return 0;
+}
+
+int
+refactor_qat_asym_process_response(__rte_unused void **op,
+		__rte_unused uint8_t *resp,
+		__rte_unused void *op_cookie,
+		__rte_unused uint64_t *dequeue_err_count)
+{
+	return 0;
+}
+
+uint16_t
+qat_asym_crypto_enqueue_op_burst(void *qp, struct rte_crypto_op **ops,
+		uint16_t nb_ops)
+{
+	return refactor_qat_enqueue_op_burst(qp,
+					refactor_qat_asym_build_request,
+					(void **)ops, nb_ops);
+}
+
+uint16_t
+qat_asym_crypto_dequeue_op_burst(void *qp, struct rte_crypto_op **ops,
+		uint16_t nb_ops)
+{
+	return refactor_qat_dequeue_op_burst(qp, (void **)ops,
+				refactor_qat_asym_process_response, nb_ops);
+}
+
 int
 qat_asym_build_request(void *in_op,
 			uint8_t *out_msg,
diff --git a/drivers/crypto/qat/qat_asym.h b/drivers/crypto/qat/qat_asym.h
index 308b6b2e0b..50c2641eba 100644
--- a/drivers/crypto/qat/qat_asym.h
+++ b/drivers/crypto/qat/qat_asym.h
@@ -92,4 +92,19 @@  void
 qat_asym_process_response(void __rte_unused **op, uint8_t *resp,
 		void *op_cookie);
 
+int
+refactor_qat_asym_process_response(__rte_unused void **op,
+		__rte_unused uint8_t *resp,
+		__rte_unused void *op_cookie,
+		__rte_unused uint64_t *dequeue_err_count);
+
+uint16_t
+qat_asym_crypto_enqueue_op_burst(void *qp, struct rte_crypto_op **ops,
+		uint16_t nb_ops);
+
+
+uint16_t
+qat_asym_crypto_dequeue_op_burst(void *qp, struct rte_crypto_op **ops,
+		uint16_t nb_ops);
+
 #endif /* _QAT_ASYM_H_ */
diff --git a/drivers/crypto/qat/qat_sym.c b/drivers/crypto/qat/qat_sym.c
index 93b257522b..a92874cd27 100644
--- a/drivers/crypto/qat/qat_sym.c
+++ b/drivers/crypto/qat/qat_sym.c
@@ -210,6 +210,31 @@  handle_spc_gmac(struct qat_sym_session *ctx, struct rte_crypto_op *op,
 			ICP_QAT_FW_LA_NO_PROTO);
 }
 
+static __rte_always_inline int
+refactor_qat_sym_build_request(__rte_unused void *in_op,
+		__rte_unused uint8_t *out_msg, __rte_unused	void *op_cookie,
+		__rte_unused uint64_t *opaque,
+		__rte_unused enum qat_device_gen dev_gen)
+{
+	return 0;
+}
+
+uint16_t
+refactor_qat_sym_enqueue_burst(void *qp, struct rte_crypto_op **ops,
+		uint16_t nb_ops)
+{
+	return refactor_qat_enqueue_op_burst(qp, refactor_qat_sym_build_request,
+			(void **)ops, nb_ops);
+}
+
+uint16_t
+refactor_qat_sym_dequeue_burst(void *qp, struct rte_crypto_op **ops,
+		uint16_t nb_ops)
+{
+	return refactor_qat_dequeue_op_burst(qp, (void **)ops,
+				refactor_qat_sym_process_response, nb_ops);
+}
+
 int
 qat_sym_build_request(void *in_op, uint8_t *out_msg,
 		void *op_cookie, enum qat_device_gen qat_dev_gen)
diff --git a/drivers/crypto/qat/qat_sym.h b/drivers/crypto/qat/qat_sym.h
index e3ec7f0de4..17b2c871bd 100644
--- a/drivers/crypto/qat/qat_sym.h
+++ b/drivers/crypto/qat/qat_sym.h
@@ -54,6 +54,22 @@  struct qat_sym_op_cookie {
 	} opt;
 };
 
+static __rte_always_inline int
+refactor_qat_sym_process_response(__rte_unused void **op,
+		__rte_unused uint8_t *resp, __rte_unused void *op_cookie,
+		__rte_unused uint64_t *dequeue_err_count)
+{
+	return 0;
+}
+
+uint16_t
+refactor_qat_sym_enqueue_burst(void *qp, struct rte_crypto_op **ops,
+		uint16_t nb_ops);
+
+uint16_t
+refactor_qat_sym_dequeue_burst(void *qp, struct rte_crypto_op **ops,
+		uint16_t nb_ops);
+
 int
 qat_sym_build_request(void *in_op, uint8_t *out_msg,
 		void *op_cookie, enum qat_device_gen qat_dev_gen);
diff --git a/drivers/crypto/qat/qat_sym_session.h b/drivers/crypto/qat/qat_sym_session.h
index 6ebc176729..73493ec864 100644
--- a/drivers/crypto/qat/qat_sym_session.h
+++ b/drivers/crypto/qat/qat_sym_session.h
@@ -55,6 +55,11 @@ 
 #define QAT_SESSION_IS_SLICE_SET(flags, flag)	\
 	(!!((flags) & (flag)))
 
+struct qat_sym_session;
+
+typedef int (*qat_sym_build_request_t)(void *in_op, struct qat_sym_session *ctx,
+		uint8_t *out_msg, void *op_cookie);
+
 enum qat_sym_proto_flag {
 	QAT_CRYPTO_PROTO_FLAG_NONE = 0,
 	QAT_CRYPTO_PROTO_FLAG_CCM = 1,
@@ -107,6 +112,7 @@  struct qat_sym_session {
 	/* Some generations need different setup of counter */
 	uint32_t slice_types;
 	enum qat_sym_proto_flag qat_proto_flag;
+	qat_sym_build_request_t build_request[2];
 };
 
 int