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 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.