Re: [dpdk-dev] [PATCH v8 11/13] crypto/dpaa2_sec: add crypto operation support
Hi Hemant,
> -Original Message-
> From: Hemant Agrawal [mailto:hemant.agra...@nxp.com]
> Sent: Wednesday, April 19, 2017 6:48 PM
> To: De Lara Guarch, Pablo; Akhil Goyal; dev@dpdk.org
> Cc: Doherty, Declan; Mcnamara, John
> Subject: RE: [dpdk-dev] [PATCH v8 11/13] crypto/dpaa2_sec: add crypto
> operation support
>
> Hi Pablo,
>
> > -Original Message-
> > From: De Lara Guarch, Pablo [mailto:pablo.de.lara.gua...@intel.com]
> > > -Original Message-
> > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of
> > > akhil.go...@nxp.com
> > > Sent: Wednesday, April 19, 2017 4:38 PM
> > > To: dev@dpdk.org
> > > Cc: Doherty, Declan; Mcnamara, John; hemant.agra...@nxp.com
> > > Subject: [dpdk-dev] [PATCH v8 11/13] crypto/dpaa2_sec: add crypto
> > > operation support
> > >
> > > From: Akhil Goyal
> > >
> > > Signed-off-by: Akhil Goyal
> > > Signed-off-by: Hemant Agrawal
> > > ---
> > > drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 1236
> > > +++
> > > drivers/crypto/dpaa2_sec/dpaa2_sec_priv.h | 143
> > > 2 files changed, 1379 insertions(+)
> > >
> > > diff --git a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> > > b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> > > index e0e8cfb..7c497c0 100644
> > > --- a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> > > +++ b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> >
> > ...
> >
> > > +/** Clear the memory of session so it doesn't leave key material
> > > +behind */ static void dpaa2_sec_session_clear(struct rte_cryptodev
> > > +*dev __rte_unused, void
> > > *sess)
> > > +{
> > > + PMD_INIT_FUNC_TRACE();
> > > + dpaa2_sec_session *s = (dpaa2_sec_session *)sess;
> > > +
> > > + if (s) {
> > > + if (s->ctxt)
> > > + rte_free(s->ctxt);
> > > + if (&s->cipher_key)
> > > + rte_free(s->cipher_key.data);
> > > + if (&s->auth_key)
> > > + rte_free(s->auth_key.data);
> >
> > No need for these checks, rte_free can handle NULL pointers (assuming
> that the
> > structure is initialized to all 0s when created, which looks like it is
> happening
> > below).
> >
> > Unless there are other changes required (I am currently reviewing the
> patchset),
> > I can make this and the change from the other email myself, when
> applying the
> > patchset.
>
> [Hemant] No, we are not expecting other changes.
>
> If you want, I can send the new patchset or you can make the changes -
> either way is fine with us.
> (2nd is preferred 😊)
There are other issues with this patchset.
1 - There are two functions that are not being used:
/root/dpdk-next-crypto-nxp/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c:77:1:
error: unused function 'print_fd' [-Werror,-Wunused-function]
print_fd(const struct qbman_fd *fd)
^
/root/dpdk-next-crypto-nxp/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c:91:1:
error: unused function 'print_fle' [-Werror,-Wunused-function]
print_fle(const struct qbman_fle *fle)
^
2 - When enabling CONFIG_RTE_LIBRTE_DPAA2_SEC_DEBUG_RX, I see the following
errors
/root/dpdk-next-crypto-nxp/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c:554:6:
error: 'bpid_info' undeclared (first use in this function)
bpid_info[DPAA2_GET_FD_BPID(fd)].meta_data_size,
^
/root/dpdk-next-crypto-nxp/build/include/rte_log.h:334:32: note: in definition
of macro 'RTE_LOG'
RTE_LOGTYPE_ ## t, # t ": " __VA_ARGS__)
^~~
/root/dpdk-next-crypto-nxp/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c:551:2:
note: in expansion of macro 'PMD_RX_LOG'
PMD_RX_LOG(DEBUG, "fdaddr =%p bpid =%d meta =%d off =%d, len =%d",
^~
So, I think these errors deserve a v9, sorry I just spotted them.
Pablo
> >
> > Thanks,
> > Pablo
Re: [dpdk-dev] [PATCH v8 11/13] crypto/dpaa2_sec: add crypto operation support
Hi Pablo,
> -Original Message-
> From: De Lara Guarch, Pablo [mailto:pablo.de.lara.gua...@intel.com]
> > -Original Message-
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of
> > akhil.go...@nxp.com
> > Sent: Wednesday, April 19, 2017 4:38 PM
> > To: dev@dpdk.org
> > Cc: Doherty, Declan; Mcnamara, John; hemant.agra...@nxp.com
> > Subject: [dpdk-dev] [PATCH v8 11/13] crypto/dpaa2_sec: add crypto
> > operation support
> >
> > From: Akhil Goyal
> >
> > Signed-off-by: Akhil Goyal
> > Signed-off-by: Hemant Agrawal
> > ---
> > drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 1236
> > +++
> > drivers/crypto/dpaa2_sec/dpaa2_sec_priv.h | 143
> > 2 files changed, 1379 insertions(+)
> >
> > diff --git a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> > b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> > index e0e8cfb..7c497c0 100644
> > --- a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> > +++ b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
>
> ...
>
> > +/** Clear the memory of session so it doesn't leave key material
> > +behind */ static void dpaa2_sec_session_clear(struct rte_cryptodev
> > +*dev __rte_unused, void
> > *sess)
> > +{
> > + PMD_INIT_FUNC_TRACE();
> > + dpaa2_sec_session *s = (dpaa2_sec_session *)sess;
> > +
> > + if (s) {
> > + if (s->ctxt)
> > + rte_free(s->ctxt);
> > + if (&s->cipher_key)
> > + rte_free(s->cipher_key.data);
> > + if (&s->auth_key)
> > + rte_free(s->auth_key.data);
>
> No need for these checks, rte_free can handle NULL pointers (assuming that the
> structure is initialized to all 0s when created, which looks like it is
> happening
> below).
>
> Unless there are other changes required (I am currently reviewing the
> patchset),
> I can make this and the change from the other email myself, when applying the
> patchset.
[Hemant] No, we are not expecting other changes.
If you want, I can send the new patchset or you can make the changes - either
way is fine with us.
(2nd is preferred 😊)
>
> Thanks,
> Pablo
Re: [dpdk-dev] [PATCH v8 11/13] crypto/dpaa2_sec: add crypto operation support
> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of
> akhil.go...@nxp.com
> Sent: Wednesday, April 19, 2017 4:38 PM
> To: dev@dpdk.org
> Cc: Doherty, Declan; Mcnamara, John; hemant.agra...@nxp.com
> Subject: [dpdk-dev] [PATCH v8 11/13] crypto/dpaa2_sec: add crypto
> operation support
>
> From: Akhil Goyal
>
> Signed-off-by: Akhil Goyal
> Signed-off-by: Hemant Agrawal
> ---
> drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 1236
> +++
> drivers/crypto/dpaa2_sec/dpaa2_sec_priv.h | 143
> 2 files changed, 1379 insertions(+)
>
> diff --git a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> index e0e8cfb..7c497c0 100644
> --- a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> +++ b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
...
> +/** Clear the memory of session so it doesn't leave key material behind */
> +static void
> +dpaa2_sec_session_clear(struct rte_cryptodev *dev __rte_unused, void
> *sess)
> +{
> + PMD_INIT_FUNC_TRACE();
> + dpaa2_sec_session *s = (dpaa2_sec_session *)sess;
> +
> + if (s) {
> + if (s->ctxt)
> + rte_free(s->ctxt);
> + if (&s->cipher_key)
> + rte_free(s->cipher_key.data);
> + if (&s->auth_key)
> + rte_free(s->auth_key.data);
No need for these checks, rte_free can handle NULL pointers
(assuming that the structure is initialized to all 0s when created, which looks
like it is happening below).
Unless there are other changes required (I am currently reviewing the
patchset), I can make this and
the change from the other email myself, when applying the patchset.
Thanks,
Pablo
[dpdk-dev] [PATCH v8 11/13] crypto/dpaa2_sec: add crypto operation support
From: Akhil Goyal
Signed-off-by: Akhil Goyal
Signed-off-by: Hemant Agrawal
---
drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 1236 +++
drivers/crypto/dpaa2_sec/dpaa2_sec_priv.h | 143
2 files changed, 1379 insertions(+)
diff --git a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
index e0e8cfb..7c497c0 100644
--- a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
+++ b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
@@ -48,17 +48,1242 @@
#include
#include
#include
+#include
#include
#include
#include "dpaa2_sec_priv.h"
#include "dpaa2_sec_logs.h"
+/* RTA header files */
+#include
+#include
+
+/* Minimum job descriptor consists of a oneword job descriptor HEADER and
+ * a pointer to the shared descriptor
+ */
+#define MIN_JOB_DESC_SIZE (CAAM_CMD_SZ + CAAM_PTR_SZ)
#define FSL_VENDOR_ID 0x1957
#define FSL_DEVICE_ID 0x410
#define FSL_SUBSYSTEM_SEC 1
#define FSL_MC_DPSECI_DEVID 3
+#define NO_PREFETCH 0
+#define TDES_CBC_IV_LEN 8
+#define AES_CBC_IV_LEN 16
+enum rta_sec_era rta_sec_era = RTA_SEC_ERA_8;
+
+static inline void
+print_fd(const struct qbman_fd *fd)
+{
+ printf("addr_lo: %u\n", fd->simple.addr_lo);
+ printf("addr_hi: %u\n", fd->simple.addr_hi);
+ printf("len: %u\n", fd->simple.len);
+ printf("bpid: %u\n", DPAA2_GET_FD_BPID(fd));
+ printf("fi_bpid_off: %u\n", fd->simple.bpid_offset);
+ printf("frc: %u\n", fd->simple.frc);
+ printf("ctrl: %u\n", fd->simple.ctrl);
+ printf("flc_lo: %u\n", fd->simple.flc_lo);
+ printf("flc_hi: %u\n\n", fd->simple.flc_hi);
+}
+
+static inline void
+print_fle(const struct qbman_fle *fle)
+{
+ printf("addr_lo: %u\n", fle->addr_lo);
+ printf("addr_hi: %u\n", fle->addr_hi);
+ printf("len: %u\n", fle->length);
+ printf("fi_bpid_off: %u\n", fle->fin_bpid_offset);
+ printf("frc: %u\n", fle->frc);
+}
+
+static inline int
+build_authenc_fd(dpaa2_sec_session *sess,
+struct rte_crypto_op *op,
+struct qbman_fd *fd, uint16_t bpid)
+{
+ struct rte_crypto_sym_op *sym_op = op->sym;
+ struct ctxt_priv *priv = sess->ctxt;
+ struct qbman_fle *fle, *sge;
+ struct sec_flow_context *flc;
+ uint32_t auth_only_len = sym_op->auth.data.length -
+ sym_op->cipher.data.length;
+ int icv_len = sym_op->auth.digest.length;
+ uint8_t *old_icv;
+ uint32_t mem_len = (7 * sizeof(struct qbman_fle)) + icv_len;
+
+ PMD_INIT_FUNC_TRACE();
+
+ /* we are using the first FLE entry to store Mbuf.
+* Currently we donot know which FLE has the mbuf stored.
+* So while retreiving we can go back 1 FLE from the FD -ADDR
+* to get the MBUF Addr from the previous FLE.
+* We can have a better approach to use the inline Mbuf
+*/
+ fle = rte_zmalloc(NULL, mem_len, RTE_CACHE_LINE_SIZE);
+ if (!fle) {
+ RTE_LOG(ERR, PMD, "Memory alloc failed for SGE\n");
+ return -1;
+ }
+ DPAA2_SET_FLE_ADDR(fle, DPAA2_OP_VADDR_TO_IOVA(op));
+ fle = fle + 1;
+ sge = fle + 2;
+ if (likely(bpid < MAX_BPID)) {
+ DPAA2_SET_FD_BPID(fd, bpid);
+ DPAA2_SET_FLE_BPID(fle, bpid);
+ DPAA2_SET_FLE_BPID(fle + 1, bpid);
+ DPAA2_SET_FLE_BPID(sge, bpid);
+ DPAA2_SET_FLE_BPID(sge + 1, bpid);
+ DPAA2_SET_FLE_BPID(sge + 2, bpid);
+ DPAA2_SET_FLE_BPID(sge + 3, bpid);
+ } else {
+ DPAA2_SET_FD_IVP(fd);
+ DPAA2_SET_FLE_IVP(fle);
+ DPAA2_SET_FLE_IVP((fle + 1));
+ DPAA2_SET_FLE_IVP(sge);
+ DPAA2_SET_FLE_IVP((sge + 1));
+ DPAA2_SET_FLE_IVP((sge + 2));
+ DPAA2_SET_FLE_IVP((sge + 3));
+ }
+
+ /* Save the shared descriptor */
+ flc = &priv->flc_desc[0].flc;
+ /* Configure FD as a FRAME LIST */
+ DPAA2_SET_FD_ADDR(fd, DPAA2_VADDR_TO_IOVA(fle));
+ DPAA2_SET_FD_COMPOUND_FMT(fd);
+ DPAA2_SET_FD_FLC(fd, DPAA2_VADDR_TO_IOVA(flc));
+
+ PMD_TX_LOG(DEBUG, "auth_off: 0x%x/length %d, digest-len=%d\n"
+ "cipher_off: 0x%x/length %d, iv-len=%d data_off: 0x%x\n",
+ sym_op->auth.data.offset,
+ sym_op->auth.data.length,
+ sym_op->auth.digest.length,
+ sym_op->cipher.data.offset,
+ sym_op->cipher.data.length,
+ sym_op->cipher.iv.length,
+ sym_op->m_src->data_off);
+
+ /* Configure Output FLE with Scatter/Gather Entry */
+ DPAA2_SET_FLE_ADDR(fle, DPAA2_VADDR_TO_IOVA(sge));
+ if (auth_only_len)
+ DPAA2_S
