Leverage devres to get rid of code storing global context as well as init_done flag. Original code also has a circular deallocation dependency where unregister_algs() -> caam_rng_exit() -> caam_jr_free() chain would only happen if all of JRs were freed. Fix this by moving caam_rng_exit() outside of unregister_algs() and doing it specifically for JR that instantiated HWRNG. Signed-off-by: Andrey Smirnov <andrew.smirnov@xxxxxxxxx> Cc: Chris Healy <cphealy@xxxxxxxxx> Cc: Lucas Stach <l.stach@xxxxxxxxxxxxxx> Cc: Horia Geantă <horia.geanta@xxxxxxx> Cc: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> Cc: Iuliana Prodan <iuliana.prodan@xxxxxxx> Cc: linux-crypto@xxxxxxxxxxxxxxx Cc: linux-kernel@xxxxxxxxxxxxxxx Cc: linux-imx@xxxxxxx --- drivers/crypto/caam/caamrng.c | 67 +++++++++++++++++------------------ drivers/crypto/caam/intern.h | 7 ++-- drivers/crypto/caam/jr.c | 13 ++++--- 3 files changed, 44 insertions(+), 43 deletions(-) diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c index 69a02ac5de54..753625f2b2c0 100644 --- a/drivers/crypto/caam/caamrng.c +++ b/drivers/crypto/caam/caamrng.c @@ -70,6 +70,7 @@ struct buf_data { /* rng per-device context */ struct caam_rng_ctx { + struct hwrng rng; struct device *jrdev; dma_addr_t sh_desc_dma; u32 sh_desc[DESC_RNG_LEN]; @@ -78,13 +79,10 @@ struct caam_rng_ctx { struct buf_data bufs[2]; }; -static struct caam_rng_ctx *rng_ctx; - -/* - * Variable used to avoid double free of resources in case - * algorithm registration was unsuccessful - */ -static bool init_done; +static struct caam_rng_ctx *to_caam_rng_ctx(struct hwrng *r) +{ + return (struct caam_rng_ctx *)r->priv; +} static inline void rng_unmap_buf(struct device *jrdev, struct buf_data *bd) { @@ -143,7 +141,7 @@ static inline int submit_job(struct caam_rng_ctx *ctx, int to_current) static int caam_read(struct hwrng *rng, void *data, size_t max, bool wait) { - struct caam_rng_ctx *ctx = rng_ctx; + struct caam_rng_ctx *ctx = to_caam_rng_ctx(rng); struct buf_data *bd = &ctx->bufs[ctx->current_buf]; int next_buf_idx, copied_idx; int err; @@ -246,17 +244,18 @@ static inline int rng_create_job_desc(struct caam_rng_ctx *ctx, int buf_id) static void caam_cleanup(struct hwrng *rng) { + struct caam_rng_ctx *ctx = to_caam_rng_ctx(rng); int i; struct buf_data *bd; for (i = 0; i < 2; i++) { - bd = &rng_ctx->bufs[i]; + bd = &ctx->bufs[i]; if (atomic_read(&bd->empty) == BUF_PENDING) wait_for_completion(&bd->filled); } - rng_unmap_ctx(rng_ctx); - caam_jr_free(rng_ctx->jrdev); + rng_unmap_ctx(ctx); + caam_jr_free(ctx->jrdev); } static int caam_init_buf(struct caam_rng_ctx *ctx, int buf_id) @@ -277,7 +276,7 @@ static int caam_init_buf(struct caam_rng_ctx *ctx, int buf_id) static int caam_init(struct hwrng *rng) { - struct caam_rng_ctx *ctx = rng_ctx; + struct caam_rng_ctx *ctx = to_caam_rng_ctx(rng); int err; ctx->jrdev = caam_jr_alloc(); @@ -309,28 +308,19 @@ static int caam_init(struct hwrng *rng) return err; } -static struct hwrng caam_rng = { - .name = "rng-caam", - .init = caam_init, - .cleanup = caam_cleanup, - .read = caam_read, -}; +int caam_rng_init(struct device *ctrldev); -void caam_rng_exit(void) +void caam_rng_exit(struct device *ctrldev) { - if (!init_done) - return; - - hwrng_unregister(&caam_rng); - kfree(rng_ctx); + devres_release_group(ctrldev, caam_rng_init); } int caam_rng_init(struct device *ctrldev) { + struct caam_rng_ctx *ctx; u32 rng_inst; struct caam_drv_private *priv = dev_get_drvdata(ctrldev); - int err; - init_done = false; + int ret; /* Check for an instantiated RNG before registration */ if (priv->era < 10) @@ -342,18 +332,27 @@ int caam_rng_init(struct device *ctrldev) if (!rng_inst) return 0; - rng_ctx = kmalloc(sizeof(*rng_ctx), GFP_DMA | GFP_KERNEL); - if (!rng_ctx) + if (!devres_open_group(ctrldev, caam_rng_init, GFP_KERNEL)) + return -ENOMEM; + + ctx = devm_kzalloc(ctrldev, sizeof(*ctx), GFP_DMA | GFP_KERNEL); + if (!ctx) return -ENOMEM; + ctx->rng.name = "rng-caam"; + ctx->rng.init = caam_init; + ctx->rng.cleanup = caam_cleanup; + ctx->rng.read = caam_read; + ctx->rng.priv = (unsigned long)ctx; + dev_info(ctrldev, "registering rng-caam\n"); - err = hwrng_register(&caam_rng); - if (!err) { - init_done = true; - return err; + ret = devm_hwrng_register(ctrldev, &ctx->rng); + if (ret) { + caam_rng_exit(ctrldev); + return ret; } - kfree(rng_ctx); - return err; + devres_close_group(ctrldev, caam_rng_init); + return 0; } diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h index 230ea88184f5..402d6a362e8c 100644 --- a/drivers/crypto/caam/intern.h +++ b/drivers/crypto/caam/intern.h @@ -47,6 +47,7 @@ struct caam_drv_private_jr { struct caam_job_ring __iomem *rregs; /* JobR's register space */ struct tasklet_struct irqtask; int irq; /* One per queue */ + bool hwrng; /* Number of scatterlist crypt transforms active on the JobR */ atomic_t tfm_count ____cacheline_aligned; @@ -163,7 +164,7 @@ static inline void caam_pkc_exit(void) #ifdef CONFIG_CRYPTO_DEV_FSL_CAAM_RNG_API int caam_rng_init(struct device *dev); -void caam_rng_exit(void); +void caam_rng_exit(struct device *dev); #else @@ -172,9 +173,7 @@ static inline int caam_rng_init(struct device *dev) return 0; } -static inline void caam_rng_exit(void) -{ -} +static inline void caam_rng_exit(struct device *dev) {} #endif /* CONFIG_CRYPTO_DEV_FSL_CAAM_RNG_API */ diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c index 88aff2aefd5d..4af22e7ceb4f 100644 --- a/drivers/crypto/caam/jr.c +++ b/drivers/crypto/caam/jr.c @@ -27,7 +27,8 @@ static struct jr_driver_data driver_data; static DEFINE_MUTEX(algs_lock); static unsigned int active_devs; -static void register_algs(struct device *dev) +static void register_algs(struct caam_drv_private_jr *jrpriv, + struct device *dev) { mutex_lock(&algs_lock); @@ -37,7 +38,7 @@ static void register_algs(struct device *dev) caam_algapi_init(dev); caam_algapi_hash_init(dev); caam_pkc_init(dev); - caam_rng_init(dev); + jrpriv->hwrng = !caam_rng_init(dev); caam_qi_algapi_init(dev); algs_unlock: @@ -53,7 +54,6 @@ static void unregister_algs(void) caam_qi_algapi_exit(); - caam_rng_exit(); caam_pkc_exit(); caam_algapi_hash_exit(); caam_algapi_exit(); @@ -135,6 +135,9 @@ static int caam_jr_remove(struct platform_device *pdev) jrdev = &pdev->dev; jrpriv = dev_get_drvdata(jrdev); + if (jrpriv->hwrng) + caam_rng_exit(jrdev->parent); + /* * Return EBUSY if job ring already allocated. */ @@ -514,7 +517,7 @@ static int caam_jr_probe(struct platform_device *pdev) int error; jrdev = &pdev->dev; - jrpriv = devm_kmalloc(jrdev, sizeof(*jrpriv), GFP_KERNEL); + jrpriv = devm_kzalloc(jrdev, sizeof(*jrpriv), GFP_KERNEL); if (!jrpriv) return -ENOMEM; @@ -590,7 +593,7 @@ static int caam_jr_probe(struct platform_device *pdev) atomic_set(&jrpriv->tfm_count, 0); - register_algs(jrdev->parent); + register_algs(jrpriv, jrdev->parent); return 0; } -- 2.21.0