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