From: Easwar Hariharan <eahariha@linux.microsoft.com>
To: Pavitrakumar Managutte <pavitrakumarm@vayavyalabs.com>
Cc: herbert@gondor.apana.org.au, linux-crypto@vger.kernel.org,
Ruud.Derwig@synopsys.com, manjunath.hadli@vayavyalabs.com,
bhoomikak@vayavyalabs.com, shwetar <shwetar@vayavyalabs.com>
Subject: Re: [PATCH v1 1/4] Add SPAcc driver to Linux kernel
Date: Mon, 1 Apr 2024 09:34:03 -0700 [thread overview]
Message-ID: <0ca1f15a-f883-407f-8837-c2a9891637f4@linux.microsoft.com> (raw)
In-Reply-To: <CALxtO0=tvJh+h9W+1eN4xLQtwOugteABpA0QUTrgJ=f_dgeVoA@mail.gmail.com>
On 4/1/2024 12:21 AM, Pavitrakumar Managutte wrote:
> Hi Easwar,
> My comments are embedded below. Also should I wait for more comments
> from you or
> should I go ahead and push v2 with the below fixes ?
>
> Warm regards,
> PK
>
<snip>
Please see more comments below, and go ahead and push a v2. If possible, split
the series into more patches. I leave it to you to group them by file or logical
functionality, so long as each individual commit compiles neatly so as to not break
future git bisects.
On 3/28/2024 11:26 AM, Pavitrakumar M wrote:
> Signed-off-by: shwetar <shwetar@vayavyalabs.com>
> Signed-off-by: Pavitrakumar M <pavitrakumarm@vayavyalabs.com>
> Acked-by: Ruud Derwig <Ruud.Derwig@synopsys.com>
> ---
> drivers/crypto/dwc-spacc/spacc_aead.c | 1382 ++++++++++
> drivers/crypto/dwc-spacc/spacc_ahash.c | 1183 ++++++++
> drivers/crypto/dwc-spacc/spacc_core.c | 2917 ++++++++++++++++++++
> drivers/crypto/dwc-spacc/spacc_core.h | 839 ++++++
> drivers/crypto/dwc-spacc/spacc_device.c | 324 +++
> drivers/crypto/dwc-spacc/spacc_device.h | 236 ++
> drivers/crypto/dwc-spacc/spacc_hal.c | 365 +++
> drivers/crypto/dwc-spacc/spacc_hal.h | 113 +
> drivers/crypto/dwc-spacc/spacc_interrupt.c | 204 ++
> drivers/crypto/dwc-spacc/spacc_manager.c | 670 +++++
> drivers/crypto/dwc-spacc/spacc_skcipher.c | 754 +++++
> 11 files changed, 8987 insertions(+)
> create mode 100644 drivers/crypto/dwc-spacc/spacc_aead.c
> create mode 100644 drivers/crypto/dwc-spacc/spacc_ahash.c
> create mode 100644 drivers/crypto/dwc-spacc/spacc_core.c
> create mode 100644 drivers/crypto/dwc-spacc/spacc_core.h
> create mode 100644 drivers/crypto/dwc-spacc/spacc_device.c
> create mode 100644 drivers/crypto/dwc-spacc/spacc_device.h
> create mode 100644 drivers/crypto/dwc-spacc/spacc_hal.c
> create mode 100644 drivers/crypto/dwc-spacc/spacc_hal.h
> create mode 100644 drivers/crypto/dwc-spacc/spacc_interrupt.c
> create mode 100644 drivers/crypto/dwc-spacc/spacc_manager.c
> create mode 100644 drivers/crypto/dwc-spacc/spacc_skcipher.c
> diff --git a/drivers/crypto/dwc-spacc/spacc_ahash.c b/drivers/crypto/dwc-spacc/spacc_ahash.c
> new file mode 100644
> index 000000000000..53c76ee16c53
> --- /dev/null
> +++ b/drivers/crypto/dwc-spacc/spacc_ahash.c
> @@ -0,0 +1,1183 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
<snip>
> +
> +static int spacc_hash_init(struct ahash_request *req)
> +{
> + int x = 0, rc = 0;
> + struct crypto_ahash *reqtfm = crypto_ahash_reqtfm(req);
> + struct spacc_crypto_ctx *tctx = crypto_ahash_ctx(reqtfm);
> + struct spacc_crypto_reqctx *ctx = ahash_request_ctx(req);
> + const struct spacc_alg *salg = spacc_tfm_ahash(&reqtfm->base);
> + struct spacc_priv *priv = dev_get_drvdata(tctx->dev);
> +
> +
> + ctx->digest_buf = NULL;
> +
> + ctx->single_shot = 0;
> + ctx->total_nents = 0;
> + ctx->cur_part_pck = 0;
> + ctx->final_part_pck = 0;
> + ctx->rem_len = 0;
> + ctx->rem_nents = 0;
> + ctx->first_ppp_chunk = 1;
> + ctx->small_pck = 1;
> + tctx->ppp_sgl = NULL;
> +
> + if (tctx->handle < 0 || !tctx->ctx_valid) {
> + priv = NULL;
> + dev_dbg(tctx->dev, "%s: open SPAcc context\n", __func__);
> +
> + for (x = 0; x < ELP_CAPI_MAX_DEV && salg->dev[x]; x++) {
> + priv = dev_get_drvdata(salg->dev[x]);
> + tctx->dev = get_device(salg->dev[x]);
> + if (spacc_isenabled(&priv->spacc, salg->mode->id, 0)) {
> + tctx->handle = spacc_open(&priv->spacc,
> + CRYPTO_MODE_NULL,
> + salg->mode->id, -1, 0,
> + spacc_digest_cb, reqtfm);
> + }
> +
> + if (tctx->handle >= 0)
> + break;
> +
> + put_device(salg->dev[x]);
> + }
> +
> + if (tctx->handle < 0) {
> + dev_dbg(salg->dev[0], "Failed to open SPAcc context\n");
> + goto fallback;
> + }
> +
> + rc = spacc_set_operation(&priv->spacc, tctx->handle,
> + OP_ENCRYPT, ICV_HASH, IP_ICV_OFFSET,
> + 0, 0, 0);
> + if (rc < 0) {
> + spacc_close(&priv->spacc, tctx->handle);
> + dev_dbg(salg->dev[0], "Failed to open SPAcc context\n");
> + tctx->handle = -1;
> + put_device(tctx->dev);
> + goto fallback;
> + }
> + tctx->ctx_valid = true;
> + } else {
> + ;/* do nothing */
> + }
Do we need this else?
> +
> + /* alloc ppp_sgl */
> + tctx->ppp_sgl = kmalloc(sizeof(*(tctx->ppp_sgl)) * 2, GFP_KERNEL);
> + if (!tctx->ppp_sgl)
> + return -ENOMEM;
> +
> + sg_init_table(tctx->ppp_sgl, 2);
> +
> + return 0;
> +fallback:
> +
> + ctx->fb.hash_req.base = req->base;
> + ahash_request_set_tfm(&ctx->fb.hash_req, tctx->fb.hash);
> +
> + return crypto_ahash_init(&ctx->fb.hash_req);
> +}
> +
> +static int spacc_hash_final_part_pck(struct ahash_request *req)
> +{
> + struct crypto_ahash *reqtfm = crypto_ahash_reqtfm(req);
> + struct spacc_crypto_ctx *tctx = crypto_ahash_ctx(reqtfm);
> + struct spacc_crypto_reqctx *ctx = ahash_request_ctx(req);
> + struct spacc_priv *priv = dev_get_drvdata(tctx->dev);
> +
> + int rc;
> +
> + ctx->final_part_pck = 1;
> +
> + /* In all the final calls the data is same as prev update and
> + * hence we can skip this init dma part and just enQ ddt
> + * No use in calling initdata, just process remaining bytes in ppp_sgl
> + * and be done with it.
> + */
> +
> + rc = spacc_hash_init_dma(tctx->dev, req, 1);
> +
> + if (rc < 0)
> + return -ENOMEM;
> +
> + if (rc == 0) {
> + ;/* small packet */
> + }
Please cleanup these do-nothing comment code paths throughout.
> +
> + /* enqueue ddt for the remaining bytes of data, everything else
> + * would have been processed already, req->nbytes need not be
> + * processed
> + * Since this will hit only for small pkts, hence the condition
> + * ctx->rem_len-req->nbytes for the small pkt len
> + */
> + if (ctx->rem_len)
> + rc = spacc_packet_enqueue_ddt(&priv->spacc,
> + ctx->acb.new_handle, &ctx->src, &ctx->dst,
> + tctx->ppp_sgl[0].length,
> + 0, tctx->ppp_sgl[0].length, 0, 0, 0);
> + else {
> + /* zero msg handling */
> + rc = spacc_packet_enqueue_ddt(&priv->spacc,
> + ctx->acb.new_handle,
> + &ctx->src, &ctx->dst, 0, 0, 0, 0, 0, 0);
> + }
> +
> + if (rc < 0) {
> + spacc_hash_cleanup_dma(tctx->dev, req);
> + spacc_close(&priv->spacc, ctx->acb.new_handle);
> +
> + if (rc != -EBUSY) {
> + dev_err(tctx->dev, "ERR: Failed to enqueue job: %d\n", rc);
> + return rc;
> + }
> +
> + if (!(req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG))
> + return -EBUSY;
> + }
> +
> + return -EINPROGRESS;
> +}
> +
<snip>
Thanks,
Easwar
next prev parent reply other threads:[~2024-04-01 16:34 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-28 18:26 [PATCH v1 0/4] Add spacc crypto driver support Pavitrakumar M
2024-03-28 18:26 ` [PATCH v1 1/4] Add SPAcc driver to Linux kernel Pavitrakumar M
2024-03-29 17:56 ` Easwar Hariharan
2024-04-01 7:21 ` Pavitrakumar Managutte
2024-04-01 16:34 ` Easwar Hariharan [this message]
2024-04-02 5:02 ` Pavitrakumar Managutte
2024-03-28 18:26 ` [PATCH v1 2/4] Add SPACC Kconfig and Makefile Pavitrakumar M
2024-03-29 18:01 ` Easwar Hariharan
2024-03-28 18:26 ` [PATCH v1 3/4] Add SPAcc dts overlay Pavitrakumar M
2024-03-28 18:26 ` [PATCH v1 4/4] Enable Driver compilation in crypto Kconfig and Makefile file Pavitrakumar M
2024-03-30 0:40 ` kernel test robot
2024-04-05 7:08 ` [PATCH v1 0/4] Add spacc crypto driver support Herbert Xu
2024-04-11 5:31 ` Pavitrakumar Managutte
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=0ca1f15a-f883-407f-8837-c2a9891637f4@linux.microsoft.com \
--to=eahariha@linux.microsoft.com \
--cc=Ruud.Derwig@synopsys.com \
--cc=bhoomikak@vayavyalabs.com \
--cc=herbert@gondor.apana.org.au \
--cc=linux-crypto@vger.kernel.org \
--cc=manjunath.hadli@vayavyalabs.com \
--cc=pavitrakumarm@vayavyalabs.com \
--cc=shwetar@vayavyalabs.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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).