-----Original Message-----
From: Akhil Goyal [mailto:akhil.go...@nxp.com]
Sent: Thursday, August 24, 2017 1:01 AM
To: dev@dpdk.org; De Lara Guarch, Pablo
<pablo.de.lara.gua...@intel.com>
Cc: Doherty, Declan <declan.dohe...@intel.com>; Mcnamara, John
<john.mcnam...@intel.com>; hemant.agra...@nxp.com; Akhil Goyal
<akhil.go...@nxp.com>
Subject: [PATCH 2/4] crypto/dpaa_sec: add crypto driver for NXP DPAA
platform
Signed-off-by: Forrest Shi <xuelin....@nxp.com>
Signed-off-by: Akhil Goyal <akhil.go...@nxp.com>
Signed-off-by: Hemant Agrawal <hemant.agra...@nxp.com>
---
MAINTAINERS | 5 +
config/common_base | 8 +
config/defconfig_arm64-dpaa-linuxapp-gcc | 14 +
drivers/Makefile | 2 +-
drivers/crypto/Makefile | 2 +
drivers/crypto/dpaa_sec/Makefile | 71 +
drivers/crypto/dpaa_sec/dpaa_sec.c | 1552
++++++++++++++++++++
drivers/crypto/dpaa_sec/dpaa_sec.h | 403 +++++
drivers/crypto/dpaa_sec/dpaa_sec_log.h | 70 +
.../crypto/dpaa_sec/rte_pmd_dpaa_sec_version.map | 4 +
mk/rte.app.mk | 6 +
11 files changed, 2136 insertions(+), 1 deletion(-)
create mode 100644 drivers/crypto/dpaa_sec/Makefile
create mode 100644 drivers/crypto/dpaa_sec/dpaa_sec.c
create mode 100644 drivers/crypto/dpaa_sec/dpaa_sec.h
create mode 100644 drivers/crypto/dpaa_sec/dpaa_sec_log.h
create mode 100644
drivers/crypto/dpaa_sec/rte_pmd_dpaa_sec_version.map
diff --git a/MAINTAINERS b/MAINTAINERS
index 48afbfc..24b3b41 100644
...
+
+include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/crypto/dpaa_sec/dpaa_sec.c
b/drivers/crypto/dpaa_sec/dpaa_sec.c
new file mode 100644
index 0000000..c8f8be9
--- /dev/null
+++ b/drivers/crypto/dpaa_sec/dpaa_sec.c
@@ -0,0 +1,1552 @@
+/*-
+ * BSD LICENSE
+ *
...
+
+static inline struct dpaa_sec_op_ctx *
+dpaa_sec_alloc_ctx(dpaa_sec_session *ses)
+{
+ struct dpaa_sec_op_ctx *ctx;
+ int retval;
+
+ retval = rte_mempool_get(ses->ctx_pool, (void **)(&ctx));
+ if (!ctx || retval) {
+ PMD_TX_LOG(ERR, "Alloc sec descriptor failed!");
+ return NULL;
+ }
+ dcbz_64(&ctx->job.sg[0]);
+ dcbz_64(&ctx->job.sg[5]);
+ dcbz_64(&ctx->job.sg[9]);
+ dcbz_64(&ctx->job.sg[13]);
Are these numbers ok? First, you should define macros for them, but it looks
strange
that there is a gap of 5 between the first and the second, and the rest has a
gap of 4.
+
+ ctx->ctx_pool = ses->ctx_pool;
+
+ return ctx;
+}
+
...
+/* prepare command block of the session */
+static int
+dpaa_sec_prep_cdb(dpaa_sec_session *ses)
+{
+ struct alginfo alginfo_c = {0}, alginfo_a = {0}, alginfo = {0};
+ uint32_t shared_desc_len = 0;
+ struct sec_cdb *cdb = &ses->qp->cdb;
+ int err;
+#if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
+ int swap = false;
+#else
+ int swap = true;
+#endif
+
+ memset(cdb, 0, sizeof(struct sec_cdb));
+
+ if (is_cipher_only(ses)) {
+ caam_cipher_alg(ses, &alginfo_c);
+ if (alginfo_c.algtype == (unsigned
int)DPAA_SEC_ALG_UNSUPPORT) {
+ PMD_TX_LOG(ERR, "not supported cipher alg\n");
+ return -1;
You could use -ENOTSUP, instead of -1.
I also checked that this function is called, but the return value is not
verified,
so either check it when calling it, or change the return type to "void".
+ }
+
+ alginfo_c.key = (uint64_t)ses->cipher_key.data;
+ alginfo_c.keylen = ses->cipher_key.length;
+ alginfo_c.key_enc_flags = 0;
+ alginfo_c.key_type = RTA_DATA_IMM;
+
+ shared_desc_len = cnstr_shdsc_blkcipher(
+ cdb->sh_desc, true,
+ swap, &alginfo_c,
+ NULL,
+ ses->iv.length,
+ ses->dir);
...
+static uint16_t
+dpaa_sec_enqueue_burst(void *qp, struct rte_crypto_op **ops,
+ uint16_t nb_ops)
+{
+ /* Function to transmit the frames to given device and queuepair */
+ uint32_t loop;
+ int32_t ret;
+ struct dpaa_sec_qp *dpaa_qp = (struct dpaa_sec_qp *)qp;
+ uint16_t num_tx = 0;
+
+ if (unlikely(nb_ops == 0))
+ return 0;
+
+ if (ops[0]->sess_type != RTE_CRYPTO_OP_WITH_SESSION) {
+ PMD_TX_LOG(ERR, "sessionless crypto op not supported");
+ return 0;
+ }
Each operation is independent from the other ones, so that means that some
operations
could have a session, while others not. Shouldn't you check each operation?
+
+ /*Prepare each packet which is to be sent*/
+ for (loop = 0; loop < nb_ops; loop++) {
+ ret = dpaa_sec_enqueue_op(ops[loop], dpaa_qp);
+ if (!ret)
+ num_tx++;
+ }
+ dpaa_qp->tx_pkts += num_tx;
+ dpaa_qp->tx_errs += nb_ops - num_tx;
+
+ return num_tx;
+}
+
...
+/** Release queue pair */
+static int
+dpaa_sec_queue_pair_release(struct rte_cryptodev *dev,
+ uint16_t qp_id)
+{
+ struct dpaa_sec_dev_private *internals;
+ struct dpaa_sec_qp *qp = NULL;
+
+ PMD_INIT_FUNC_TRACE();
+
+ PMD_INIT_LOG(DEBUG, "dev =%p, queue =%d", dev, qp_id);
+
+ internals = dev->data->dev_private;
+ if (qp_id >= internals->max_nb_queue_pairs) {
+ PMD_INIT_LOG(ERR, "Max supported qpid %d",
+ internals->max_nb_queue_pairs);
+ return -1;
+ }
Better to return -EINVAL.
+
+ qp = &internals->qps[qp_id];
+ qp->internals = NULL;
+ dev->data->queue_pairs[qp_id] = NULL;
+
+ return 0;
+}
+
+/** Setup a queue pair */
+static int
+dpaa_sec_queue_pair_setup(struct rte_cryptodev *dev, uint16_t qp_id,
+ __rte_unused const struct rte_cryptodev_qp_conf
*qp_conf,
+ __rte_unused int socket_id,
+ __rte_unused struct rte_mempool *session_pool)
+{
+ struct dpaa_sec_dev_private *internals;
+ struct dpaa_sec_qp *qp = NULL;
+
+ PMD_INIT_LOG(DEBUG, "dev =%p, queue =%d, conf =%p",
+ dev, qp_id, qp_conf);
+
+ internals = dev->data->dev_private;
+ if (qp_id >= internals->max_nb_queue_pairs) {
+ PMD_INIT_LOG(ERR, "Max supported qpid %d",
+ internals->max_nb_queue_pairs);
+ return -1;
+ }
Better to return -EINVAL.
+
+ qp = &internals->qps[qp_id];
+ qp->internals = internals;
+ dev->data->queue_pairs[qp_id] = qp;
+
+ return 0;
+}
+
...
+static
+void dpaa_sec_stats_get(struct rte_cryptodev *dev __rte_unused,
+ struct rte_cryptodev_stats *stats __rte_unused)
"static void" should be in the same line. Anyway, if this function is not going
to be implemented,
then it is probably better to just remove it, so when rte_cryptodev_stats_get()
gets called,
it will return -ENOTSUP, as the PMD does not actually support this. Same for
the next function.
+{
+ PMD_INIT_FUNC_TRACE();
+ /* -ENOTSUP; */
+}
+
+static
+void dpaa_sec_stats_reset(struct rte_cryptodev *dev __rte_unused)
+{
+ PMD_INIT_FUNC_TRACE();
+ /* -ENOTSUP; */
+}
+
...
diff --git a/drivers/crypto/dpaa_sec/dpaa_sec.h
b/drivers/crypto/dpaa_sec/dpaa_sec.h
new file mode 100644
index 0000000..2677f8b
--- /dev/null
+++ b/drivers/crypto/dpaa_sec/dpaa_sec.h
@@ -0,0 +1,403 @@
...
+static const struct rte_cryptodev_capabilities dpaa_sec_capabilities[] = {
+ { /* MD5 HMAC */
+ .op = RTE_CRYPTO_OP_TYPE_SYMMETRIC,
+ {.sym = {
+ .xform_type = RTE_CRYPTO_SYM_XFORM_AUTH,
+ {.auth = {
+ .algo = RTE_CRYPTO_AUTH_MD5_HMAC,
+ .block_size = 64,
+ .key_size = {
+ .min = 1,
+ .max = 64,
+ .increment = 1
+ },
+ .digest_size = {
+ .min = 16,
+ .max = 16,
+ .increment = 0
+ },
+ .aad_size = { 0 }
No need to include aad_size here, as it is only applicable to AEAD algorithms.
I just realized that this was left after the rework done in 17.08.
Unfortunately, removing it will be an API change, so it will not be removed at
least until 18.02.
As it is not used, we should remove it from here, to avoid confusion.
+ }, }
+ }, }
+ },
...
+ { /* SHA256 HMAC */
+ .op = RTE_CRYPTO_OP_TYPE_SYMMETRIC,
+ {.sym = {
+ .xform_type = RTE_CRYPTO_SYM_XFORM_AUTH,
+ {.auth = {
+ .algo = RTE_CRYPTO_AUTH_SHA256_HMAC,
+ .block_size = 64,
+ .key_size = {
+ .min = 1,
+ .max = 64,
+ .increment = 1
+ },
+ .digest_size = {
+ .min = 32,
+ .max = 32,
+ .increment = 0
+ },
Unnecessary extra tab.
+ .aad_size = { 0 }
+ }, }
+ }, }
