[PATCH 1/2] hwrng: msm: add a spinlock and support for blocking reads

[Timur Tabi <timur@codeaurora.org>]

The hwrng.read callback includes a boolean parameter called 'wait'
which indicates whether the function should block and wait for
more data.

When 'wait' is true, the driver spins on the DATA_AVAIL bit or until
a reasonable timeout.  The timeout can occur if there is a heavy load
on reading the PRNG.

The same code also needs a spinlock to protect against race conditions.

If multiple cores hammer on the PRNG, it's possible for a race
condition to occur between reading the status register and
reading the data register.  Add a spinlock to protect against
that.

1. Core 1 reads status register, shows data is available.
2. Core 2 also reads status register, same result
3. Core 2 reads data register, depleting all entropy
4. Core 1 reads data register, which returns 0

Signed-off-by: Timur Tabi <timur@codeaurora.org>
---
 drivers/char/hw_random/msm-rng.c | 57 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 50 insertions(+), 7 deletions(-)

diff --git a/drivers/char/hw_random/msm-rng.c b/drivers/char/hw_random/msm-rng.c
index 841fee845ec9..44580588b938 100644
--- a/drivers/char/hw_random/msm-rng.c
+++ b/drivers/char/hw_random/msm-rng.c
@@ -15,9 +15,11 @@
 #include <linux/err.h>
 #include <linux/hw_random.h>
 #include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/spinlock.h>
 
 /* Device specific register offsets */
 #define PRNG_DATA_OUT		0x0000
@@ -35,10 +37,22 @@
 #define MAX_HW_FIFO_SIZE	(MAX_HW_FIFO_DEPTH * 4)
 #define WORD_SZ			4
 
+/*
+ * Normally, this would be the maximum time it takes to refill the FIFO,
+ * after a read.  Under heavy load, tests show that this delay is either
+ * below 50us or above 2200us.  The higher value is probably what happens
+ * when entropy is completely depleted.
+ *
+ * Since we don't want to wait 2ms in a spinlock, set the timeout to the
+ * lower value.  Under extreme situations, that timeout can extend to 100us.
+ */
+#define TIMEOUT		50
+
 struct msm_rng {
 	void __iomem *base;
 	struct clk *clk;
 	struct hwrng hwrng;
+	spinlock_t lock;
 };
 
 #define to_msm_rng(p)	container_of(p, struct msm_rng, hwrng)
@@ -96,11 +110,39 @@ static int msm_rng_read(struct hwrng *hwrng, void *data, size_t max, bool wait)
 
 	/* read random data from hardware */
 	do {
-		val = readl_relaxed(rng->base + PRNG_STATUS);
-		if (!(val & PRNG_STATUS_DATA_AVAIL))
-			break;
+		spin_lock(&rng->lock);
+
+		/*
+		 * First check the status bit.  If 'wait' is true, then wait
+		 * up to TIMEOUT microseconds for data to be available.
+		 */
+		if (wait) {
+			int ret;
+
+			ret = readl_poll_timeout_atomic(rng->base + PRNG_STATUS,
+				val, val & PRNG_STATUS_DATA_AVAIL, 0, TIMEOUT);
+			if (ret) {
+				/* Timed out */
+				spin_unlock(&rng->lock);
+				break;
+			}
+		} else {
+			val = readl_relaxed(rng->base + PRNG_STATUS);
+			if (!(val & PRNG_STATUS_DATA_AVAIL)) {
+				spin_unlock(&rng->lock);
+				break;
+			}
+		}
 
 		val = readl_relaxed(rng->base + PRNG_DATA_OUT);
+		spin_unlock(&rng->lock);
+
+		/*
+		 * Zero is technically a valid random number, but it's also
+		 * the value returned if the PRNG is not enabled properly.
+		 * To avoid accidentally returning all zeros, treat it as
+		 * invalid and just return what we've already read.
+		 */
 		if (!val)
 			break;
 
@@ -148,10 +190,11 @@ static int msm_rng_probe(struct platform_device *pdev)
 	if (IS_ERR(rng->clk))
 		return PTR_ERR(rng->clk);
 
-	rng->hwrng.name = KBUILD_MODNAME,
-	rng->hwrng.init = msm_rng_init,
-	rng->hwrng.cleanup = msm_rng_cleanup,
-	rng->hwrng.read = msm_rng_read,
+	rng->hwrng.name = KBUILD_MODNAME;
+	rng->hwrng.init = msm_rng_init;
+	rng->hwrng.cleanup = msm_rng_cleanup;
+	rng->hwrng.read = msm_rng_read;
+	spin_lock_init(&rng->lock);
 
 	ret = devm_hwrng_register(&pdev->dev, &rng->hwrng);
 	if (ret) {
-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.