From: Nathan Chancellor <natechancellor@gmail.com> To: Tero Kristo <t-kristo@ti.com> Cc: herbert@gondor.apana.org.au, davem@davemloft.net, linux-crypto@vger.kernel.org, j-keerthy@ti.com, linux-arm-kernel@lists.infradead.org, clang-built-linux@googlegroups.com Subject: Re: [PATCHv6 2/7] crypto: sa2ul: Add crypto driver Date: Fri, 21 Aug 2020 15:17:29 -0700 [thread overview] Message-ID: <20200821221729.GA204847@ubuntu-n2-xlarge-x86> (raw) In-Reply-To: <20200713083427.30117-3-t-kristo@ti.com> On Mon, Jul 13, 2020 at 11:34:22AM +0300, Tero Kristo wrote: > From: Keerthy <j-keerthy@ti.com> > > Adds a basic crypto driver and currently supports AES/3DES > in cbc mode for both encryption and decryption. > > Signed-off-by: Keerthy <j-keerthy@ti.com> > [t-kristo@ti.com: major re-work to fix various bugs in the driver and to > cleanup the code] > Signed-off-by: Tero Kristo <t-kristo@ti.com> > --- > drivers/crypto/Kconfig | 14 + > drivers/crypto/Makefile | 1 + > drivers/crypto/sa2ul.c | 1388 +++++++++++++++++++++++++++++++++++++++ > drivers/crypto/sa2ul.h | 380 +++++++++++ > 4 files changed, 1783 insertions(+) > create mode 100644 drivers/crypto/sa2ul.c > create mode 100644 drivers/crypto/sa2ul.h <snip> > diff --git a/drivers/crypto/sa2ul.c b/drivers/crypto/sa2ul.c > new file mode 100644 > index 000000000000..860c7435fefa > --- /dev/null > +++ b/drivers/crypto/sa2ul.c > @@ -0,0 +1,1388 @@ <snip> > +static int sa_run(struct sa_req *req) > +{ > + struct sa_rx_data *rxd; > + gfp_t gfp_flags; > + u32 cmdl[SA_MAX_CMDL_WORDS]; > + struct sa_crypto_data *pdata = dev_get_drvdata(sa_k3_dev); > + struct device *ddev; > + struct dma_chan *dma_rx; > + int sg_nents, src_nents, dst_nents; > + int mapped_src_nents, mapped_dst_nents; > + struct scatterlist *src, *dst; > + size_t pl, ml, split_size; > + struct sa_ctx_info *sa_ctx = req->enc ? &req->ctx->enc : &req->ctx->dec; > + int ret; > + struct dma_async_tx_descriptor *tx_out; > + u32 *mdptr; > + bool diff_dst; > + enum dma_data_direction dir_src; > + > + gfp_flags = req->base->flags & CRYPTO_TFM_REQ_MAY_SLEEP ? > + GFP_KERNEL : GFP_ATOMIC; > + > + rxd = kzalloc(sizeof(*rxd), gfp_flags); > + if (!rxd) > + return -ENOMEM; > + > + if (req->src != req->dst) { > + diff_dst = true; > + dir_src = DMA_TO_DEVICE; > + } else { > + diff_dst = false; > + dir_src = DMA_BIDIRECTIONAL; > + } > + > + /* > + * SA2UL has an interesting feature where the receive DMA channel > + * is selected based on the data passed to the engine. Within the > + * transition range, there is also a space where it is impossible > + * to determine where the data will end up, and this should be > + * avoided. This will be handled by the SW fallback mechanism by > + * the individual algorithm implementations. > + */ > + if (req->size >= 256) > + dma_rx = pdata->dma_rx2; > + else > + dma_rx = pdata->dma_rx1; > + > + ddev = dma_rx->device->dev; > + > + memcpy(cmdl, sa_ctx->cmdl, sa_ctx->cmdl_size); > + > + sa_update_cmdl(req, cmdl, &sa_ctx->cmdl_upd_info); > + > + if (req->type != CRYPTO_ALG_TYPE_AHASH) { > + if (req->enc) > + req->type |= > + (SA_REQ_SUBTYPE_ENC << SA_REQ_SUBTYPE_SHIFT); > + else > + req->type |= > + (SA_REQ_SUBTYPE_DEC << SA_REQ_SUBTYPE_SHIFT); > + } > + > + cmdl[sa_ctx->cmdl_size / sizeof(u32)] = req->type; > + > + /* > + * Map the packets, first we check if the data fits into a single > + * sg entry and use that if possible. If it does not fit, we check > + * if we need to do sg_split to align the scatterlist data on the > + * actual data size being processed by the crypto engine. > + */ > + src = req->src; > + sg_nents = sg_nents_for_len(src, req->size); > + > + split_size = req->size; > + > + if (sg_nents == 1 && split_size <= req->src->length) { > + src = &rxd->rx_sg; > + sg_init_table(src, 1); > + sg_set_page(src, sg_page(req->src), split_size, > + req->src->offset); > + src_nents = 1; > + dma_map_sg(ddev, src, sg_nents, dir_src); > + } else { > + mapped_src_nents = dma_map_sg(ddev, req->src, sg_nents, > + dir_src); > + ret = sg_split(req->src, mapped_src_nents, 0, 1, &split_size, > + &src, &src_nents, gfp_flags); > + if (ret) { > + src_nents = sg_nents; > + src = req->src; > + } else { > + rxd->split_src_sg = src; > + } > + } > + > + if (!diff_dst) { > + dst_nents = src_nents; > + dst = src; > + } else { > + dst_nents = sg_nents_for_len(req->dst, req->size); > + > + if (dst_nents == 1 && split_size <= req->dst->length) { > + dst = &rxd->tx_sg; > + sg_init_table(dst, 1); > + sg_set_page(dst, sg_page(req->dst), split_size, > + req->dst->offset); > + dst_nents = 1; > + dma_map_sg(ddev, dst, dst_nents, DMA_FROM_DEVICE); > + } else { > + mapped_dst_nents = dma_map_sg(ddev, req->dst, dst_nents, > + DMA_FROM_DEVICE); > + ret = sg_split(req->dst, mapped_dst_nents, 0, 1, > + &split_size, &dst, &dst_nents, > + gfp_flags); > + if (ret) { > + dst_nents = dst_nents; This causes a clang warning: drivers/crypto/sa2ul.c:1152:15: warning: explicitly assigning value of variable of type 'int' to itself [-Wself-assign] dst_nents = dst_nents; ~~~~~~~~~ ^ ~~~~~~~~~ 1 warning generated. Was the right side supposed to be something else? Otherwise, this line can be removed, right? > + dst = req->dst; > + } else { > + rxd->split_dst_sg = dst; > + } > + } > + } > + > + if (unlikely(src_nents != sg_nents)) { > + dev_warn_ratelimited(sa_k3_dev, "failed to map tx pkt\n"); > + ret = -EIO; > + goto err_cleanup; > + } > + > + rxd->tx_in = dmaengine_prep_slave_sg(dma_rx, dst, dst_nents, > + DMA_DEV_TO_MEM, > + DMA_PREP_INTERRUPT | DMA_CTRL_ACK); > + if (!rxd->tx_in) { > + dev_err(pdata->dev, "IN prep_slave_sg() failed\n"); > + ret = -EINVAL; > + goto err_cleanup; > + } > + > + rxd->req = (void *)req->base; > + rxd->enc = req->enc; > + rxd->ddev = ddev; > + rxd->src = src; > + rxd->dst = dst; > + rxd->iv_idx = req->ctx->iv_idx; > + rxd->enc_iv_size = sa_ctx->cmdl_upd_info.enc_iv.size; > + rxd->tx_in->callback = req->callback; > + rxd->tx_in->callback_param = rxd; > + > + tx_out = dmaengine_prep_slave_sg(pdata->dma_tx, src, > + src_nents, DMA_MEM_TO_DEV, > + DMA_PREP_INTERRUPT | DMA_CTRL_ACK); > + > + if (!tx_out) { > + dev_err(pdata->dev, "OUT prep_slave_sg() failed\n"); > + ret = -EINVAL; > + goto err_cleanup; > + } > + > + /* > + * Prepare metadata for DMA engine. This essentially describes the > + * crypto algorithm to be used, data sizes, different keys etc. > + */ > + mdptr = (u32 *)dmaengine_desc_get_metadata_ptr(tx_out, &pl, &ml); > + > + sa_prepare_tx_desc(mdptr, (sa_ctx->cmdl_size + (SA_PSDATA_CTX_WORDS * > + sizeof(u32))), cmdl, sizeof(sa_ctx->epib), > + sa_ctx->epib); > + > + ml = sa_ctx->cmdl_size + (SA_PSDATA_CTX_WORDS * sizeof(u32)); > + dmaengine_desc_set_metadata_len(tx_out, req->mdata_size); > + > + dmaengine_submit(tx_out); > + dmaengine_submit(rxd->tx_in); > + > + dma_async_issue_pending(dma_rx); > + dma_async_issue_pending(pdata->dma_tx); > + > + return -EINPROGRESS; > + > +err_cleanup: > + dma_unmap_sg(ddev, req->src, sg_nents, DMA_TO_DEVICE); > + kfree(rxd->split_src_sg); > + > + if (req->src != req->dst) { > + dst_nents = sg_nents_for_len(req->dst, req->size); > + dma_unmap_sg(ddev, req->dst, dst_nents, DMA_FROM_DEVICE); > + kfree(rxd->split_dst_sg); > + } > + > + kfree(rxd); > + > + return ret; > +} Cheers, Nathan
WARNING: multiple messages have this Message-ID (diff)
From: Nathan Chancellor <natechancellor@gmail.com> To: Tero Kristo <t-kristo@ti.com> Cc: herbert@gondor.apana.org.au, j-keerthy@ti.com, clang-built-linux@googlegroups.com, linux-crypto@vger.kernel.org, davem@davemloft.net, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCHv6 2/7] crypto: sa2ul: Add crypto driver Date: Fri, 21 Aug 2020 15:17:29 -0700 [thread overview] Message-ID: <20200821221729.GA204847@ubuntu-n2-xlarge-x86> (raw) In-Reply-To: <20200713083427.30117-3-t-kristo@ti.com> On Mon, Jul 13, 2020 at 11:34:22AM +0300, Tero Kristo wrote: > From: Keerthy <j-keerthy@ti.com> > > Adds a basic crypto driver and currently supports AES/3DES > in cbc mode for both encryption and decryption. > > Signed-off-by: Keerthy <j-keerthy@ti.com> > [t-kristo@ti.com: major re-work to fix various bugs in the driver and to > cleanup the code] > Signed-off-by: Tero Kristo <t-kristo@ti.com> > --- > drivers/crypto/Kconfig | 14 + > drivers/crypto/Makefile | 1 + > drivers/crypto/sa2ul.c | 1388 +++++++++++++++++++++++++++++++++++++++ > drivers/crypto/sa2ul.h | 380 +++++++++++ > 4 files changed, 1783 insertions(+) > create mode 100644 drivers/crypto/sa2ul.c > create mode 100644 drivers/crypto/sa2ul.h <snip> > diff --git a/drivers/crypto/sa2ul.c b/drivers/crypto/sa2ul.c > new file mode 100644 > index 000000000000..860c7435fefa > --- /dev/null > +++ b/drivers/crypto/sa2ul.c > @@ -0,0 +1,1388 @@ <snip> > +static int sa_run(struct sa_req *req) > +{ > + struct sa_rx_data *rxd; > + gfp_t gfp_flags; > + u32 cmdl[SA_MAX_CMDL_WORDS]; > + struct sa_crypto_data *pdata = dev_get_drvdata(sa_k3_dev); > + struct device *ddev; > + struct dma_chan *dma_rx; > + int sg_nents, src_nents, dst_nents; > + int mapped_src_nents, mapped_dst_nents; > + struct scatterlist *src, *dst; > + size_t pl, ml, split_size; > + struct sa_ctx_info *sa_ctx = req->enc ? &req->ctx->enc : &req->ctx->dec; > + int ret; > + struct dma_async_tx_descriptor *tx_out; > + u32 *mdptr; > + bool diff_dst; > + enum dma_data_direction dir_src; > + > + gfp_flags = req->base->flags & CRYPTO_TFM_REQ_MAY_SLEEP ? > + GFP_KERNEL : GFP_ATOMIC; > + > + rxd = kzalloc(sizeof(*rxd), gfp_flags); > + if (!rxd) > + return -ENOMEM; > + > + if (req->src != req->dst) { > + diff_dst = true; > + dir_src = DMA_TO_DEVICE; > + } else { > + diff_dst = false; > + dir_src = DMA_BIDIRECTIONAL; > + } > + > + /* > + * SA2UL has an interesting feature where the receive DMA channel > + * is selected based on the data passed to the engine. Within the > + * transition range, there is also a space where it is impossible > + * to determine where the data will end up, and this should be > + * avoided. This will be handled by the SW fallback mechanism by > + * the individual algorithm implementations. > + */ > + if (req->size >= 256) > + dma_rx = pdata->dma_rx2; > + else > + dma_rx = pdata->dma_rx1; > + > + ddev = dma_rx->device->dev; > + > + memcpy(cmdl, sa_ctx->cmdl, sa_ctx->cmdl_size); > + > + sa_update_cmdl(req, cmdl, &sa_ctx->cmdl_upd_info); > + > + if (req->type != CRYPTO_ALG_TYPE_AHASH) { > + if (req->enc) > + req->type |= > + (SA_REQ_SUBTYPE_ENC << SA_REQ_SUBTYPE_SHIFT); > + else > + req->type |= > + (SA_REQ_SUBTYPE_DEC << SA_REQ_SUBTYPE_SHIFT); > + } > + > + cmdl[sa_ctx->cmdl_size / sizeof(u32)] = req->type; > + > + /* > + * Map the packets, first we check if the data fits into a single > + * sg entry and use that if possible. If it does not fit, we check > + * if we need to do sg_split to align the scatterlist data on the > + * actual data size being processed by the crypto engine. > + */ > + src = req->src; > + sg_nents = sg_nents_for_len(src, req->size); > + > + split_size = req->size; > + > + if (sg_nents == 1 && split_size <= req->src->length) { > + src = &rxd->rx_sg; > + sg_init_table(src, 1); > + sg_set_page(src, sg_page(req->src), split_size, > + req->src->offset); > + src_nents = 1; > + dma_map_sg(ddev, src, sg_nents, dir_src); > + } else { > + mapped_src_nents = dma_map_sg(ddev, req->src, sg_nents, > + dir_src); > + ret = sg_split(req->src, mapped_src_nents, 0, 1, &split_size, > + &src, &src_nents, gfp_flags); > + if (ret) { > + src_nents = sg_nents; > + src = req->src; > + } else { > + rxd->split_src_sg = src; > + } > + } > + > + if (!diff_dst) { > + dst_nents = src_nents; > + dst = src; > + } else { > + dst_nents = sg_nents_for_len(req->dst, req->size); > + > + if (dst_nents == 1 && split_size <= req->dst->length) { > + dst = &rxd->tx_sg; > + sg_init_table(dst, 1); > + sg_set_page(dst, sg_page(req->dst), split_size, > + req->dst->offset); > + dst_nents = 1; > + dma_map_sg(ddev, dst, dst_nents, DMA_FROM_DEVICE); > + } else { > + mapped_dst_nents = dma_map_sg(ddev, req->dst, dst_nents, > + DMA_FROM_DEVICE); > + ret = sg_split(req->dst, mapped_dst_nents, 0, 1, > + &split_size, &dst, &dst_nents, > + gfp_flags); > + if (ret) { > + dst_nents = dst_nents; This causes a clang warning: drivers/crypto/sa2ul.c:1152:15: warning: explicitly assigning value of variable of type 'int' to itself [-Wself-assign] dst_nents = dst_nents; ~~~~~~~~~ ^ ~~~~~~~~~ 1 warning generated. Was the right side supposed to be something else? Otherwise, this line can be removed, right? > + dst = req->dst; > + } else { > + rxd->split_dst_sg = dst; > + } > + } > + } > + > + if (unlikely(src_nents != sg_nents)) { > + dev_warn_ratelimited(sa_k3_dev, "failed to map tx pkt\n"); > + ret = -EIO; > + goto err_cleanup; > + } > + > + rxd->tx_in = dmaengine_prep_slave_sg(dma_rx, dst, dst_nents, > + DMA_DEV_TO_MEM, > + DMA_PREP_INTERRUPT | DMA_CTRL_ACK); > + if (!rxd->tx_in) { > + dev_err(pdata->dev, "IN prep_slave_sg() failed\n"); > + ret = -EINVAL; > + goto err_cleanup; > + } > + > + rxd->req = (void *)req->base; > + rxd->enc = req->enc; > + rxd->ddev = ddev; > + rxd->src = src; > + rxd->dst = dst; > + rxd->iv_idx = req->ctx->iv_idx; > + rxd->enc_iv_size = sa_ctx->cmdl_upd_info.enc_iv.size; > + rxd->tx_in->callback = req->callback; > + rxd->tx_in->callback_param = rxd; > + > + tx_out = dmaengine_prep_slave_sg(pdata->dma_tx, src, > + src_nents, DMA_MEM_TO_DEV, > + DMA_PREP_INTERRUPT | DMA_CTRL_ACK); > + > + if (!tx_out) { > + dev_err(pdata->dev, "OUT prep_slave_sg() failed\n"); > + ret = -EINVAL; > + goto err_cleanup; > + } > + > + /* > + * Prepare metadata for DMA engine. This essentially describes the > + * crypto algorithm to be used, data sizes, different keys etc. > + */ > + mdptr = (u32 *)dmaengine_desc_get_metadata_ptr(tx_out, &pl, &ml); > + > + sa_prepare_tx_desc(mdptr, (sa_ctx->cmdl_size + (SA_PSDATA_CTX_WORDS * > + sizeof(u32))), cmdl, sizeof(sa_ctx->epib), > + sa_ctx->epib); > + > + ml = sa_ctx->cmdl_size + (SA_PSDATA_CTX_WORDS * sizeof(u32)); > + dmaengine_desc_set_metadata_len(tx_out, req->mdata_size); > + > + dmaengine_submit(tx_out); > + dmaengine_submit(rxd->tx_in); > + > + dma_async_issue_pending(dma_rx); > + dma_async_issue_pending(pdata->dma_tx); > + > + return -EINPROGRESS; > + > +err_cleanup: > + dma_unmap_sg(ddev, req->src, sg_nents, DMA_TO_DEVICE); > + kfree(rxd->split_src_sg); > + > + if (req->src != req->dst) { > + dst_nents = sg_nents_for_len(req->dst, req->size); > + dma_unmap_sg(ddev, req->dst, dst_nents, DMA_FROM_DEVICE); > + kfree(rxd->split_dst_sg); > + } > + > + kfree(rxd); > + > + return ret; > +} Cheers, Nathan _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-08-21 22:17 UTC|newest] Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-07-13 8:34 [PATCHv6 0/7] crypto: add driver for TI K3 SA2UL Tero Kristo 2020-07-13 8:34 ` Tero Kristo 2020-07-13 8:34 ` [PATCHv6 1/7] dt-bindings: crypto: Add TI SA2UL crypto accelerator documentation Tero Kristo 2020-07-13 8:34 ` Tero Kristo 2020-07-31 17:59 ` Rob Herring 2020-07-31 17:59 ` Rob Herring 2020-07-13 8:34 ` [PATCHv6 2/7] crypto: sa2ul: Add crypto driver Tero Kristo 2020-07-13 8:34 ` Tero Kristo 2020-08-21 22:17 ` Nathan Chancellor [this message] 2020-08-21 22:17 ` Nathan Chancellor 2020-08-24 12:30 ` Tero Kristo 2020-08-24 12:30 ` Tero Kristo 2020-07-13 8:34 ` [PATCHv6 3/7] crypto: sa2ul: add sha1/sha256/sha512 support Tero Kristo 2020-07-13 8:34 ` Tero Kristo 2020-07-13 8:34 ` [PATCHv6 4/7] crypto: sa2ul: Add AEAD algorithm support Tero Kristo 2020-07-13 8:34 ` Tero Kristo 2020-07-13 8:34 ` [PATCHv6 5/7] crypto: sa2ul: add device links to child devices Tero Kristo 2020-07-13 8:34 ` Tero Kristo 2020-07-13 8:34 ` [PATCHv6 6/7] arm64: dts: ti: k3-am6: Add crypto accelarator node Tero Kristo 2020-07-13 8:34 ` Tero Kristo 2020-07-13 8:34 ` [PATCHv6 7/7] arm64: dts: ti: k3-j721e-main: Add crypto accelerator node Tero Kristo 2020-07-13 8:34 ` Tero Kristo 2020-07-23 7:56 ` [PATCHv6 0/7] crypto: add driver for TI K3 SA2UL Herbert Xu 2020-07-23 7:56 ` Herbert Xu
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200821221729.GA204847@ubuntu-n2-xlarge-x86 \ --to=natechancellor@gmail.com \ --cc=clang-built-linux@googlegroups.com \ --cc=davem@davemloft.net \ --cc=herbert@gondor.apana.org.au \ --cc=j-keerthy@ti.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-crypto@vger.kernel.org \ --cc=t-kristo@ti.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.