linux-crypto.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrey Smirnov <andrew.smirnov@gmail.com>
To: linux-crypto@vger.kernel.org
Cc: "Andrey Smirnov" <andrew.smirnov@gmail.com>,
	"Chris Healy" <cphealy@gmail.com>,
	"Lucas Stach" <l.stach@pengutronix.de>,
	"Horia Geantă" <horia.geanta@nxp.com>,
	"Herbert Xu" <herbert@gondor.apana.org.au>,
	"Iuliana Prodan" <iuliana.prodan@nxp.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 11/12] crypto: caam - convert caamrng to platform device
Date: Tue,  3 Sep 2019 19:35:14 -0700	[thread overview]
Message-ID: <20190904023515.7107-12-andrew.smirnov@gmail.com> (raw)
In-Reply-To: <20190904023515.7107-1-andrew.smirnov@gmail.com>

In order to allow caam_jr_enqueue() to lock underlying JR's
device (via device_lock(), see commit that follows) we need to make
sure that no code calls caam_jr_enqueue() as a part of caam_jr_probe()
to avoid a deadlock. Unfortunately, current implementation of caamrng
code does exactly that in caam_init_buf().

Another big problem with original caamrng initialization is a
circular reference in the form of:

 1. caam_rng_init() aquires JR via caam_jr_alloc(). Freed only by
    caam_rng_exit()

 2. caam_rng_exit() is only called by unregister_algs() once last JR
    is shut down

 3. caam_jr_remove() won't call unregister_algs() for last JR
    until tfm_count reaches zero, which can only happen via
    unregister_algs() -> caam_rng_exit() call chain.

To avoid all of that, convert caamrng code to a platform device driver
and extend caam_probe() to create corresponding platform device.

Additionally, this change also allows us to remove any access to
struct caam_drv_private in caamrng.c as well as simplify resource
ownership/deallocation via devres.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Horia Geantă <horia.geanta@nxp.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Iuliana Prodan <iuliana.prodan@nxp.com>
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/caamrng.c | 102 +++++++++++++---------------------
 drivers/crypto/caam/ctrl.c    |  39 +++++++++++++
 drivers/crypto/caam/jr.c      |  23 ++++++--
 3 files changed, 97 insertions(+), 67 deletions(-)

diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c
index e8baacaabe07..f83ad34ac009 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 hwrng;
 	struct device *jrdev;
 	dma_addr_t sh_desc_dma;
 	u32 sh_desc[DESC_RNG_LEN];
@@ -78,14 +79,6 @@ 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 inline void rng_unmap_buf(struct device *jrdev, struct buf_data *bd)
 {
 	if (bd->addr)
@@ -143,7 +136,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 = (void *)rng->priv;
 	struct buf_data *bd = &ctx->bufs[ctx->current_buf];
 	int next_buf_idx, copied_idx;
 	int err;
@@ -247,15 +240,16 @@ static inline int rng_create_job_desc(struct caam_rng_ctx *ctx, int buf_id)
 static void caam_cleanup(struct hwrng *rng)
 {
 	int i;
+	struct caam_rng_ctx *ctx = (void *)rng->priv;
 	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);
+	rng_unmap_ctx(ctx);
 }
 
 static int caam_init_buf(struct caam_rng_ctx *ctx, int buf_id)
@@ -274,12 +268,10 @@ static int caam_init_buf(struct caam_rng_ctx *ctx, int buf_id)
 	return 0;
 }
 
-static int caam_init_rng(struct caam_rng_ctx *ctx, struct device *jrdev)
+static int caam_init_rng(struct caam_rng_ctx *ctx)
 {
 	int err;
 
-	ctx->jrdev = jrdev;
-
 	err = rng_create_sh_desc(ctx);
 	if (err)
 		return err;
@@ -294,65 +286,49 @@ static int caam_init_rng(struct caam_rng_ctx *ctx, struct device *jrdev)
 	return caam_init_buf(ctx, 1);
 }
 
-static struct hwrng caam_rng = {
-	.name		= "rng-caam",
-	.cleanup	= caam_cleanup,
-	.read		= caam_read,
-};
-
-void caam_rng_exit(void)
+static void caamrng_jr_free(void *data)
 {
-	if (!init_done)
-		return;
-
-	caam_jr_free(rng_ctx->jrdev);
-	hwrng_unregister(&caam_rng);
-	kfree(rng_ctx);
+	caam_jr_free(data);
 }
 
-int caam_rng_init(struct device *ctrldev)
+static int caamrng_probe(struct platform_device *pdev)
 {
-	struct device *dev;
-	u32 rng_inst;
-	struct caam_drv_private *priv = dev_get_drvdata(ctrldev);
+	struct device *dev = &pdev->dev;
+	struct caam_rng_ctx *ctx;
 	int err;
-	init_done = false;
-
-	/* Check for an instantiated RNG before registration */
-	if (priv->era < 10)
-		rng_inst = (rd_reg32(&priv->ctrl->perfmon.cha_num_ls) &
-			    CHA_ID_LS_RNG_MASK) >> CHA_ID_LS_RNG_SHIFT;
-	else
-		rng_inst = rd_reg32(&priv->ctrl->vreg.rng) & CHA_VER_NUM_MASK;
 
-	if (!rng_inst)
-		return 0;
+	ctx = devm_kmalloc(dev, sizeof(*ctx), GFP_DMA | GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
 
-	dev = caam_jr_alloc();
-	if (IS_ERR(dev)) {
-		pr_err("Job Ring Device allocation for transform failed\n");
-		return PTR_ERR(dev);
-	}
-	rng_ctx = kmalloc(sizeof(*rng_ctx), GFP_DMA | GFP_KERNEL);
-	if (!rng_ctx) {
-		err = -ENOMEM;
-		goto free_caam_alloc;
+	ctx->jrdev = caam_jr_alloc();
+	err = PTR_ERR_OR_ZERO(ctx->jrdev);
+	if (err) {
+		dev_err(dev, "Job Ring Device allocation for transform failed\n");
+		return err;
 	}
-	err = caam_init_rng(rng_ctx, dev);
-	if (err)
-		goto free_rng_ctx;
 
-	dev_info(dev, "registering rng-caam\n");
+	err = devm_add_action_or_reset(dev, caamrng_jr_free, ctx->jrdev);
+	if (err)
+		return err;
 
-	err = hwrng_register(&caam_rng);
-	if (!err) {
-		init_done = true;
+	err = caam_init_rng(ctx);
+	if (err)
 		return err;
-	}
 
-free_rng_ctx:
-	kfree(rng_ctx);
-free_caam_alloc:
-	caam_jr_free(dev);
-	return err;
+	ctx->hwrng.cleanup = caam_cleanup;
+	ctx->hwrng.read    = caam_read;
+	ctx->hwrng.name    = "rng-caam";
+	ctx->hwrng.priv    = (unsigned long)ctx;
+
+	dev_info(dev, "registering rng-caam\n");
+
+	return devm_hwrng_register(dev, &ctx->hwrng);
 }
+
+struct platform_driver caamrng_driver = {
+	.probe = caamrng_probe,
+	.driver = {
+		.name = "caamrng",
+	},
+};
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index d101c28d3d1f..ce3d5817c443 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -557,6 +557,26 @@ static void caam_remove_debugfs(void *root)
 }
 #endif
 
+static void caam_platform_device_unregister(void *data)
+{
+	platform_device_unregister(data);
+}
+
+static int caam_platform_device_register(struct device *dev, const char *name)
+{
+	struct platform_device *pdev;
+	int ret;
+
+	pdev = platform_device_register_simple(name,  -1, NULL, 0);
+	ret = PTR_ERR_OR_ZERO(pdev);
+	if (ret)
+		return ret;
+
+	return devm_add_action_or_reset(dev,
+					caam_platform_device_unregister,
+					pdev);
+}
+
 /* Probe routine for CAAM top (controller) level */
 static int caam_probe(struct platform_device *pdev)
 {
@@ -907,6 +927,25 @@ static int caam_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	if (IS_ENABLED(CONFIG_CRYPTO_DEV_FSL_CAAM_RNG_API)) {
+		u32 rng_inst;
+
+		/* Check for an instantiated RNG before registration */
+		if (ctrlpriv->era < 10)
+			rng_inst = (rd_reg32(&ctrl->perfmon.cha_num_ls) &
+				    CHA_ID_LS_RNG_MASK) >> CHA_ID_LS_RNG_SHIFT;
+		else
+			rng_inst = rd_reg32(&ctrl->vreg.rng) &
+				CHA_VER_NUM_MASK;
+
+
+		if (rng_inst) {
+			ret = caam_platform_device_register(dev, "caamrng");
+			if (ret)
+				return ret;
+		}
+	}
+
 	return 0;
 }
 
diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index d11956bc358f..47b389cb1c62 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -37,7 +37,6 @@ static void register_algs(struct device *dev)
 	caam_algapi_init(dev);
 	caam_algapi_hash_init(dev);
 	caam_pkc_init(dev);
-	caam_rng_init(dev);
 	caam_qi_algapi_init(dev);
 
 algs_unlock:
@@ -53,7 +52,6 @@ static void unregister_algs(void)
 
 	caam_qi_algapi_exit();
 
-	caam_rng_exit();
 	caam_pkc_exit();
 	caam_algapi_hash_exit();
 	caam_algapi_exit();
@@ -287,7 +285,7 @@ struct device *caam_jr_alloc(void)
 
 	if (list_empty(&driver_data.jr_list)) {
 		spin_unlock(&driver_data.jr_alloc_lock);
-		return ERR_PTR(-ENODEV);
+		return ERR_PTR(-EPROBE_DEFER);
 	}
 
 	list_for_each_entry(jrpriv, &driver_data.jr_list, list_node) {
@@ -587,15 +585,32 @@ static struct platform_driver caam_jr_driver = {
 	.remove      = caam_jr_remove,
 };
 
+extern struct platform_driver caamrng_driver;
+
 static int __init jr_driver_init(void)
 {
+	int ret;
+
 	spin_lock_init(&driver_data.jr_alloc_lock);
 	INIT_LIST_HEAD(&driver_data.jr_list);
-	return platform_driver_register(&caam_jr_driver);
+	ret = platform_driver_register(&caam_jr_driver);
+	if (ret)
+		return ret;
+
+	if (IS_ENABLED(CONFIG_CRYPTO_DEV_FSL_CAAM_RNG_API)) {
+		ret = platform_driver_register(&caamrng_driver);
+		if (ret) {
+			platform_driver_unregister(&caam_jr_driver);
+			return ret;
+		}
+	}
+
+	return 0;
 }
 
 static void __exit jr_driver_exit(void)
 {
+	platform_driver_unregister(&caamrng_driver);
 	platform_driver_unregister(&caam_jr_driver);
 }
 
-- 
2.21.0


  parent reply	other threads:[~2019-09-04  2:36 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-04  2:35 [PATCH 00/12] CAAM bugfixes, small improvements Andrey Smirnov
2019-09-04  2:35 ` [PATCH 01/12] crypto: caam - make sure clocks are enabled first Andrey Smirnov
2019-09-06 11:18   ` Horia Geanta
2019-09-09  7:21     ` Herbert Xu
2019-09-09  7:22       ` Herbert Xu
2019-09-04  2:35 ` [PATCH 02/12] crypto: caam - use devres to unmap JR's registers Andrey Smirnov
2019-09-04  2:43   ` Fabio Estevam
2019-09-04  2:55     ` Andrey Smirnov
2019-09-09 13:01   ` Horia Geanta
2019-09-04  2:35 ` [PATCH 03/12] crypto: caam - check irq_of_parse_and_map for errors Andrey Smirnov
2019-09-06 12:29   ` Horia Geanta
2019-09-04  2:35 ` [PATCH 04/12] crypto: caam - dispose of IRQ mapping only after IRQ is freed Andrey Smirnov
2019-09-06 12:26   ` Horia Geanta
2019-09-09  7:46   ` crypto: caam - Cast to long first before pointer conversion Herbert Xu
2019-09-09 11:06     ` Horia Geanta
2019-09-09 13:55     ` [v2 PATCH] " Herbert Xu
2019-09-04  2:35 ` [PATCH 05/12] crypto: caam - use devres to unmap memory Andrey Smirnov
2019-09-09 13:20   ` Horia Geanta
2019-09-04  2:35 ` [PATCH 06/12] crypto: caam - use devres to remove debugfs Andrey Smirnov
2019-09-09 13:25   ` Horia Geanta
2019-09-04  2:35 ` [PATCH 07/12] crypto: caam - use devres to de-initialize the RNG Andrey Smirnov
2019-09-09 15:39   ` Horia Geanta
2019-09-18  6:06     ` Andrey Smirnov
2019-09-04  2:35 ` [PATCH 08/12] crypto: caam - use devres to de-initialize QI Andrey Smirnov
2019-09-20 15:10   ` Horia Geanta
2019-09-04  2:35 ` [PATCH 09/12] crypto: caam - user devres to populate platform devices Andrey Smirnov
2019-09-20 15:29   ` Horia Geanta
2019-09-04  2:35 ` [PATCH 10/12] crypto: caam - populate platform devices last Andrey Smirnov
2019-09-20 15:35   ` Horia Geanta
2019-09-04  2:35 ` Andrey Smirnov [this message]
2019-09-11  9:35   ` [PATCH] crypto: caam - use the same jr for rng init/exit Horia Geanta
2019-09-18  6:01     ` Andrey Smirnov
2019-09-20 15:50       ` Horia Geanta
2019-09-04  2:35 ` [PATCH 12/12] crypto: caam - change JR device ownership scheme Andrey Smirnov
2019-09-13 19:16   ` Leonard Crestez
2019-09-18  3:13     ` Andrey Smirnov
2019-09-19 11:19       ` Horia Geanta
2019-09-19 13:45         ` Herbert Xu
2019-09-09  7:53 ` [PATCH 00/12] CAAM bugfixes, small improvements Herbert Xu
2019-09-09 12:07   ` Horia Geanta
2019-09-09 12:52     ` Herbert Xu
2019-09-09 13:26       ` Horia Geanta
2019-09-09 13:52         ` 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=20190904023515.7107-12-andrew.smirnov@gmail.com \
    --to=andrew.smirnov@gmail.com \
    --cc=cphealy@gmail.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=horia.geanta@nxp.com \
    --cc=iuliana.prodan@nxp.com \
    --cc=l.stach@pengutronix.de \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /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).