| Message ID | 20230313135646.2077707-1-jiajie.ho@starfivetech.com |
|---|---|
| Headers | show |
| Series | crypto: starfive - Add drivers for crypto engine | expand |
> -----Original Message----- > From: Jia Jie Ho <jiajie.ho@starfivetech.com> > Sent: 13 March, 2023 9:57 PM > To: Herbert Xu <herbert@gondor.apana.org.au>; David S . Miller > <davem@davemloft.net>; Rob Herring <robh+dt@kernel.org>; Krzysztof > Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Emil Renner Berthing > <kernel@esmil.dk>; Conor Dooley <conor.dooley@microchip.com> > Cc: linux-crypto@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; linux-riscv@lists.infradead.org > Subject: [PATCH v3 0/4] crypto: starfive - Add drivers for crypto engine > > This patch series adds kernel driver support for StarFive JH7110 crypto engine. > The first patch adds Documentations for the device and Patch 2 adds device > probe and DMA init for the module. Patch 3 adds crypto and DMA dts node > for VisionFive 2 board. Patch 4 adds hash/hmac support to the module. > > Patch 3 needs to be applied on top of: > https://patchwork.kernel.org/project/linux- > riscv/patch/20230221024645.127922-18-hal.feng@starfivetech.com/ > https://patchwork.kernel.org/project/linux- > riscv/cover/20230120024445.244345-1-xingyu.wu@starfivetech.com/ > > Changes v2->v3: > - Only implement digest and use fallback for other ops (Herbert) > - Use interrupt instead of polling for hash complete (Herbert) > - Remove manual data copy from out-of-bound memory location as it will > be handled by DMA API. (Christoph & Herbert) > > Changes v1->v2: > - Fixed yaml filename and format (Krzysztof) > - Removed unnecessary property names in yaml (Krzysztof) > - Moved of_device_id table close to usage (Krzysztof) > - Use dev_err_probe for error returns (Krzysztof) > - Dropped redundant readl and writel wrappers (Krzysztof) > - Updated commit signed offs (Conor) > - Dropped redundant node in dts, module set to on in dtsi (Conor) > > Jia Jie Ho (4): > dt-bindings: crypto: Add StarFive crypto module > crypto: starfive - Add crypto engine support > riscv: dts: starfive: Add crypto and DMA node for VisionFive 2 > crypto: starfive - Add hash and HMAC support > > .../crypto/starfive,jh7110-crypto.yaml | 70 ++ > MAINTAINERS | 7 + > arch/riscv/boot/dts/starfive/jh7110.dtsi | 28 + > drivers/crypto/Kconfig | 1 + > drivers/crypto/Makefile | 1 + > drivers/crypto/starfive/Kconfig | 21 + > drivers/crypto/starfive/Makefile | 4 + > drivers/crypto/starfive/jh7110-cryp.c | 239 ++++ > drivers/crypto/starfive/jh7110-cryp.h | 134 +++ > drivers/crypto/starfive/jh7110-hash.c | 1041 +++++++++++++++++ > 10 files changed, 1546 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/crypto/starfive,jh7110-crypto.yaml > create mode 100644 drivers/crypto/starfive/Kconfig create mode 100644 > drivers/crypto/starfive/Makefile create mode 100644 > drivers/crypto/starfive/jh7110-cryp.c > create mode 100644 drivers/crypto/starfive/jh7110-cryp.h > create mode 100644 drivers/crypto/starfive/jh7110-hash.c > > -- > 2.25.1 Hi Herbert/David, Could you please help review this patch series? Thanks in advance. Best regards, Jia Jie
On Mon, Mar 13, 2023 at 09:56:46PM +0800, Jia Jie Ho wrote: > > +static void starfive_hash_dma_callback(void *param) > +{ > + struct starfive_cryp_dev *cryp = param; > + > + complete(&cryp->tx_comp); > +} Please get rid of tx_comp and do the rest of the processing here. Thanks,
On Mon, Mar 13, 2023 at 09:56:46PM +0800, Jia Jie Ho wrote: > > +static int starfive_hash_copy_sgs(struct starfive_cryp_request_ctx *rctx) > +{ > + void *buf_in; > + int pages, total_in; > + > + if (!starfive_hash_check_io_aligned(rctx)) { > + rctx->sgs_copied = 0; > + return 0; > + } > + > + total_in = ALIGN(rctx->total, rctx->blksize); > + pages = total_in ? get_order(total_in) : 1; > + buf_in = (void *)__get_free_pages(GFP_ATOMIC, pages); Please don't allocate the whole thing because it could be unlimited in size (and triggered from user-space by untrusted users too). If you have to copy, then just allocate a single page and copy that, hash, and then repeat until it's all done. Cheers,
On Fri, Mar 24, 2023 at 05:48:45PM +0800, Herbert Xu wrote: > > Please don't allocate the whole thing because it could be unlimited > in size (and triggered from user-space by untrusted users too). > > If you have to copy, then just allocate a single page and copy > that, hash, and then repeat until it's all done. Nevermind, your hardware doesn't support hashing piece-meal. So just use the fallback if it's not aligned. Cheers,
On Mon, Mar 13, 2023 at 09:56:46PM +0800, Jia Jie Ho wrote: > > +static int starfive_hash_update(struct ahash_request *req) > +{ > + struct starfive_cryp_request_ctx *rctx = ahash_request_ctx(req); > + struct crypto_ahash *tfm = crypto_ahash_reqtfm(req); > + struct starfive_cryp_ctx *ctx = crypto_ahash_ctx(tfm); > + > + ahash_request_set_tfm(&rctx->ahash_fbk_req, ctx->ahash_fbk); > + rctx->ahash_fbk_req.base.flags = req->base.flags > + & CRYPTO_TFM_REQ_MAY_SLEEP; > + rctx->ahash_fbk_req.nbytes = req->nbytes; > + rctx->ahash_fbk_req.src = req->src; Please don't modify the fields directly, use the ahash_request_set_* helpers. Since you allocated an async fallback, you should set the callbacks too. Thanks,
> On Mon, Mar 13, 2023 at 09:56:46PM +0800, Jia Jie Ho wrote: > > > > +static void starfive_hash_dma_callback(void *param) { > > + struct starfive_cryp_dev *cryp = param; > > + > > + complete(&cryp->tx_comp); > > +} > > Please get rid of tx_comp and do the rest of the processing here. Hi Herbert, Thanks for reviewing the patch. I'll update this in next version. Best regards, Jia Jie