[PATCH v10 3/6] hwrng: starfive - move the SEU reseed out of the interrupt handler

["lianfeng.ouyang" <lianfeng.ouyang@starfivetech.com>]

From: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>

When the hardware reports an LFSR lockup (a single event upset), the
interrupt handler writes the reseed command straight into the CTRL
register. The spinlock only serialises that register write: nothing
stops it from racing with a command ->read() has just issued, nothing
waits for the reseed to complete before the next command is issued, and
a sample taken from a locked up generator is handed out as random data.
The 40us timeout used for wait == 0 also rounds up to a jiffy and so is
not what it claims to be.

Rework the command handling around a mutex that serialises complete
command sequences instead:

 - starfive_trng_cmd() checks that the hardware is idle before issuing
   a command, takes an explicit timeout, and resynchronises with the
   interrupt handler before declaring a timeout so that a late
   completion is not left behind for the next command.

 - The interrupt handler only records a lockup in trng->need_reseed and
   queues a work item which performs the reseed under the mutex.
   starfive_trng_irq_clear() records a lockup it clears for the same
   reason, so that one observed across suspend/resume is not lost.

 - ->read() reseeds before sampling when a lockup is pending, and
   retries once when the lockup is reported together with RAND_RDY.
   Non-blocking readers return no data instead and make sure the work
   is queued, so they cannot stall when the lockup was observed outside
   the interrupt handler or a previous reseed attempt failed.

 - The interrupt is requested with IRQF_NO_AUTOEN and is only enabled
   between ->init() and ->cleanup(). trng->initialized tracks that
   state: the hwrng core may call ->init() again without an intervening
   ->cleanup() when the device is deselected and quickly reselected,
   and it calls ->cleanup() after a failed ->init(), so neither may
   enable or disable the line a second time.

 - The suspend callback masks the interrupts, and asserts the reset
   while the device is not initialised; resume re-enables the
   interrupts when it is.

Fixes: c388f458bc34 ("hwrng: starfive - Add TRNG driver for StarFive SoC")
Signed-off-by: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
---
 drivers/char/hw_random/jh7110-trng.c | 352 ++++++++++++++++++++-------
 1 file changed, 261 insertions(+), 91 deletions(-)

diff --git a/drivers/char/hw_random/jh7110-trng.c b/drivers/char/hw_random/jh7110-trng.c
index 17932cc6d7aa..8e0e7865453b 100644
--- a/drivers/char/hw_random/jh7110-trng.c
+++ b/drivers/char/hw_random/jh7110-trng.c
@@ -8,18 +8,22 @@
 #include <linux/clk.h>
 #include <linux/completion.h>
 #include <linux/delay.h>
+#include <linux/devm-helpers.h>
 #include <linux/err.h>
 #include <linux/hw_random.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include <linux/kernel.h>
+#include <linux/lockdep.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/random.h>
 #include <linux/reset.h>
+#include <linux/workqueue.h>
 
 /* trng register offset */
 #define STARFIVE_CTRL			0x00
@@ -78,7 +82,14 @@
 #define STARFIVE_ISTAT_SEED_DONE	BIT(1)
 #define STARFIVE_ISTAT_LFSR_LOCKUP	BIT(4)
 
+#define STARFIVE_ISTAT_ALL		(STARFIVE_ISTAT_RAND_RDY | \
+					 STARFIVE_ISTAT_SEED_DONE | \
+					 STARFIVE_ISTAT_LFSR_LOCKUP)
+
 #define STARFIVE_RAND_LEN		sizeof(u32)
+#define STARFIVE_RANDRESEED_TIMEOUT_MS	50
+#define STARFIVE_RANDGEN_TIMEOUT_MS	20
+#define STARFIVE_RANDGEN_NOWAIT_TIMEOUT	1
 
 #define to_trng(p)			container_of(p, struct starfive_trng, rng)
 
@@ -95,21 +106,29 @@ enum mode {
 struct starfive_trng {
 	struct device		*dev;
 	void __iomem		*base;
+	int			irq;
 	struct clk		*hclk;
 	struct clk		*ahb;
 	struct reset_control	*rst;
 	struct hwrng		rng;
 	struct completion	random_done;
 	struct completion	reseed_done;
+	struct work_struct	work;
 	u32			mode;
 	u32			mission;
 	u32			reseed;
+	bool			need_reseed;
+	bool			initialized;
 	bool			clk_on;
 	bool			rst_deassert;
-	/* protects against concurrent write to ctrl register */
-	spinlock_t		write_lock;
+	struct mutex		lock; /* protect trng cmd seq */
 };
 
+static inline struct starfive_trng *work_to_trng(struct work_struct *work)
+{
+	return container_of(work, struct starfive_trng, work);
+}
+
 static u16 autoreq;
 module_param(autoreq, ushort, 0);
 MODULE_PARM_DESC(autoreq, "Auto-reseeding after random number requests by host reaches specified counter:\n"
@@ -122,22 +141,38 @@ MODULE_PARM_DESC(autoage, "Auto-reseeding after specified timer countdowns to 0:
 				" 0 - disable timer\n"
 				" other - reload value for internal timer");
 
-static inline int starfive_trng_wait_idle(struct starfive_trng *trng)
+static bool starfive_trng_idle(struct starfive_trng *trng, bool wait)
 {
-	u32 stat;
+	u32 stat, busy = STARFIVE_STAT_RAND_GENERATING | STARFIVE_STAT_RAND_SEEDING;
 
-	return readl_relaxed_poll_timeout(trng->base + STARFIVE_STAT, stat,
-					  !(stat & (STARFIVE_STAT_RAND_GENERATING |
-						    STARFIVE_STAT_RAND_SEEDING)),
-					  10, 100000);
+	if (!wait)
+		return !(readl_relaxed(trng->base + STARFIVE_STAT) & busy);
+
+	return !readl_relaxed_poll_timeout(trng->base + STARFIVE_STAT, stat,
+					   !(stat & busy),
+					   10, 100000);
 }
 
-static inline void starfive_trng_irq_mask_clear(struct starfive_trng *trng)
+/*
+ * Record a pending LFSR lockup and make sure it gets serviced. queue_work()
+ * is a no-op while the work is already pending, so this is safe to call
+ * from any context that notices the lockup.
+ */
+static void starfive_trng_request_reseed(struct starfive_trng *trng)
 {
-	/* clear register: ISTAT */
-	u32 data = readl(trng->base + STARFIVE_ISTAT);
+	WRITE_ONCE(trng->need_reseed, true);
+	queue_work(system_freezable_wq, &trng->work);
+}
+
+static void starfive_trng_irq_clear(struct starfive_trng *trng)
+{
+	u32 istat = readl(trng->base + STARFIVE_ISTAT);
+
+	/* Do not lose an LFSR lockup that has not been serviced yet. */
+	if (istat & STARFIVE_ISTAT_LFSR_LOCKUP)
+		starfive_trng_request_reseed(trng);
 
-	writel(data, trng->base + STARFIVE_ISTAT);
+	writel(istat, trng->base + STARFIVE_ISTAT);
 }
 
 static int starfive_trng_power_on(struct starfive_trng *trng)
@@ -194,42 +229,71 @@ static void starfive_trng_power_off_action(void *data)
 	starfive_trng_power_off(data, true);
 }
 
-static int starfive_trng_cmd(struct starfive_trng *trng, u32 cmd, bool wait)
+static int starfive_trng_cmd(struct starfive_trng *trng, u32 cmd, unsigned long timeout, bool wait)
 {
-	int wait_time = 1000;
+	struct completion *done;
 
-	/* allow up to 40 us for wait == 0 */
-	if (!wait)
-		wait_time = 40;
+	lockdep_assert_held(&trng->lock);
+
+	if (!starfive_trng_idle(trng, wait))
+		return wait ? -ETIMEDOUT : -EBUSY;
 
 	switch (cmd) {
 	case STARFIVE_CTRL_GENE_RANDNUM:
-		reinit_completion(&trng->random_done);
-		spin_lock_irq(&trng->write_lock);
-		writel(cmd, trng->base + STARFIVE_CTRL);
-		spin_unlock_irq(&trng->write_lock);
-		if (!wait_for_completion_timeout(&trng->random_done, usecs_to_jiffies(wait_time)))
-			return -ETIMEDOUT;
+		done = &trng->random_done;
 		break;
 	case STARFIVE_CTRL_EXEC_RANDRESEED:
-		reinit_completion(&trng->reseed_done);
-		spin_lock_irq(&trng->write_lock);
-		writel(cmd, trng->base + STARFIVE_CTRL);
-		spin_unlock_irq(&trng->write_lock);
-		if (!wait_for_completion_timeout(&trng->reseed_done, usecs_to_jiffies(wait_time)))
-			return -ETIMEDOUT;
+		done = &trng->reseed_done;
 		break;
 	default:
 		return -EINVAL;
 	}
 
+	reinit_completion(done);
+	writel(cmd, trng->base + STARFIVE_CTRL);
+	if (!wait_for_completion_timeout(done, timeout)) {
+		synchronize_irq(trng->irq);
+		if (!try_wait_for_completion(done))
+			return -ETIMEDOUT;
+	}
+
 	return 0;
 }
 
+static int starfive_trng_reseed(struct starfive_trng *trng)
+{
+	int ret;
+
+	lockdep_assert_held(&trng->lock);
+
+	WRITE_ONCE(trng->need_reseed, false);
+	ret = starfive_trng_cmd(trng, STARFIVE_CTRL_EXEC_RANDRESEED,
+				msecs_to_jiffies(STARFIVE_RANDRESEED_TIMEOUT_MS), true);
+	if (ret)
+		WRITE_ONCE(trng->need_reseed, true);
+
+	return ret;
+}
+
+static void starfive_trng_hw_setup(struct starfive_trng *trng)
+{
+	u32 mode;
+
+	/* setup Auto Request/Age register */
+	writel(autoage, trng->base + STARFIVE_AUTO_AGE);
+	writel(autoreq, trng->base + STARFIVE_AUTO_RQSTS);
+
+	mode = readl(trng->base + STARFIVE_MODE);
+	if (trng->mode == PRNG_128BIT)
+		mode &= ~STARFIVE_MODE_R256;
+	else
+		mode |= STARFIVE_MODE_R256;
+	writel(mode, trng->base + STARFIVE_MODE);
+}
+
 static int starfive_trng_init(struct hwrng *rng)
 {
 	struct starfive_trng *trng = to_trng(rng);
-	u32 mode, intr = 0;
 	int ret;
 
 	ret = pm_runtime_resume_and_get(trng->dev);
@@ -238,37 +302,62 @@ static int starfive_trng_init(struct hwrng *rng)
 		return ret;
 	}
 
-	/* setup Auto Request/Age register */
-	writel(autoage, trng->base + STARFIVE_AUTO_AGE);
-	writel(autoreq, trng->base + STARFIVE_AUTO_RQSTS);
+	mutex_lock(&trng->lock);
+
+	/*
+	 * The hwrng core can call init again without an intervening
+	 * cleanup: deselecting and quickly reselecting the device makes
+	 * cleanup_rng_work() skip ->cleanup() once the refcount is back up.
+	 * Only bring up the interrupt line once in that case.
+	 */
+	if (!trng->initialized) {
+		starfive_trng_irq_clear(trng);
+		starfive_trng_hw_setup(trng);
+
+		enable_irq(trng->irq);
+		writel(STARFIVE_IE_ALL, trng->base + STARFIVE_IE);
+		trng->initialized = true;
+	}
+
+	ret = starfive_trng_reseed(trng);
+	if (ret) {
+		trng->initialized = false;
+		writel(0, trng->base + STARFIVE_IE);
+		starfive_trng_irq_clear(trng);
+		disable_irq(trng->irq);
+	}
 
-	/* clear register: ISTAT */
-	starfive_trng_irq_mask_clear(trng);
+	mutex_unlock(&trng->lock);
 
-	intr |= STARFIVE_IE_ALL;
-	writel(intr, trng->base + STARFIVE_IE);
+	pm_runtime_put_autosuspend(trng->dev);
 
-	mode  = readl(trng->base + STARFIVE_MODE);
+	return ret;
+}
 
-	switch (trng->mode) {
-	case PRNG_128BIT:
-		mode &= ~STARFIVE_MODE_R256;
-		break;
-	case PRNG_256BIT:
-		mode |= STARFIVE_MODE_R256;
-		break;
-	default:
-		mode |= STARFIVE_MODE_R256;
-		break;
+static void starfive_trng_reseed_work(struct work_struct *work)
+{
+	struct starfive_trng *trng = work_to_trng(work);
+	int ret;
+
+	ret = pm_runtime_resume_and_get(trng->dev);
+	if (ret < 0) {
+		dev_err(trng->dev, "Failed to wake device for reseed: %d\n", ret);
+		return;
 	}
 
-	writel(mode, trng->base + STARFIVE_MODE);
+	mutex_lock(&trng->lock);
 
-	ret = starfive_trng_cmd(trng, STARFIVE_CTRL_EXEC_RANDRESEED, 1);
+	if (!READ_ONCE(trng->need_reseed) || !trng->initialized)
+		goto unlock;
 
-	pm_runtime_put_autosuspend(trng->dev);
+	ret = starfive_trng_reseed(trng);
+	if (ret)
+		dev_err(trng->dev, "Reseed after LFSR lockup failed: %d\n", ret);
 
-	return ret;
+unlock:
+	mutex_unlock(&trng->lock);
+
+	pm_runtime_put_autosuspend(trng->dev);
 }
 
 static irqreturn_t starfive_trng_irq(int irq, void *priv)
@@ -276,24 +365,21 @@ static irqreturn_t starfive_trng_irq(int irq, void *priv)
 	u32 status;
 	struct starfive_trng *trng = (struct starfive_trng *)priv;
 
-	status = readl(trng->base + STARFIVE_ISTAT);
-	if (status & STARFIVE_ISTAT_RAND_RDY) {
-		writel(STARFIVE_ISTAT_RAND_RDY, trng->base + STARFIVE_ISTAT);
+	status = readl(trng->base + STARFIVE_ISTAT) & STARFIVE_ISTAT_ALL;
+	if (!status)
+		return IRQ_NONE;
+
+	writel(status, trng->base + STARFIVE_ISTAT);
+
+	/* SEU occurred, reseeding required */
+	if (status & STARFIVE_ISTAT_LFSR_LOCKUP)
+		starfive_trng_request_reseed(trng);
+
+	if (status & STARFIVE_ISTAT_RAND_RDY)
 		complete(&trng->random_done);
-	}
 
-	if (status & STARFIVE_ISTAT_SEED_DONE) {
-		writel(STARFIVE_ISTAT_SEED_DONE, trng->base + STARFIVE_ISTAT);
+	if (status & STARFIVE_ISTAT_SEED_DONE)
 		complete(&trng->reseed_done);
-	}
-
-	if (status & STARFIVE_ISTAT_LFSR_LOCKUP) {
-		writel(STARFIVE_ISTAT_LFSR_LOCKUP, trng->base + STARFIVE_ISTAT);
-		/* SEU occurred, reseeding required*/
-		spin_lock(&trng->write_lock);
-		writel(STARFIVE_CTRL_EXEC_RANDRESEED, trng->base + STARFIVE_CTRL);
-		spin_unlock(&trng->write_lock);
-	}
 
 	return IRQ_HANDLED;
 }
@@ -301,15 +387,36 @@ static irqreturn_t starfive_trng_irq(int irq, void *priv)
 static void starfive_trng_cleanup(struct hwrng *rng)
 {
 	struct starfive_trng *trng = to_trng(rng);
+	bool initialized;
 	int ret;
 
 	ret = pm_runtime_resume_and_get(trng->dev);
-	if (ret < 0) {
+	if (ret < 0)
 		dev_err(trng->dev, "Failed to wake device for cleanup: %d\n", ret);
-		return;
+
+	mutex_lock(&trng->lock);
+	initialized = trng->initialized;
+	trng->initialized = false;
+	if (ret >= 0 && initialized) {
+		writel(0, trng->base + STARFIVE_IE);
+		starfive_trng_irq_clear(trng);
 	}
+	mutex_unlock(&trng->lock);
+
+	/*
+	 * cleanup also runs after a failed init, which has already undone
+	 * its enable_irq(). Only disable the line if init left it enabled.
+	 */
+	if (initialized)
+		disable_irq(trng->irq);
+	cancel_work_sync(&trng->work);
 
+	if (ret < 0)
+		return;
+
+	mutex_lock(&trng->lock);
 	writel(STARFIVE_CTRL_EXEC_NOP, trng->base + STARFIVE_CTRL);
+	mutex_unlock(&trng->lock);
 
 	pm_runtime_put_sync_suspend(trng->dev);
 }
@@ -317,7 +424,8 @@ static void starfive_trng_cleanup(struct hwrng *rng)
 static int starfive_trng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
 {
 	struct starfive_trng *trng = to_trng(rng);
-	int ret;
+	unsigned long timeout;
+	int ret, retry = 1;
 
 	ret = pm_runtime_resume_and_get(trng->dev);
 	if (ret < 0) {
@@ -325,35 +433,75 @@ static int starfive_trng_read(struct hwrng *rng, void *buf, size_t max, bool wai
 		return ret;
 	}
 
-	if (trng->mode == PRNG_256BIT)
-		max = min_t(size_t, max, (STARFIVE_RAND_LEN * 8));
-	else
-		max = min_t(size_t, max, (STARFIVE_RAND_LEN * 4));
-
 	if (wait) {
-		ret = starfive_trng_wait_idle(trng);
-		if (ret) {
-			ret = -ETIMEDOUT;
+		mutex_lock(&trng->lock);
+		timeout = msecs_to_jiffies(STARFIVE_RANDGEN_TIMEOUT_MS);
+	} else {
+		/* allow up to 1 jiffy for wait == 0 */
+		if (!mutex_trylock(&trng->lock)) {
+			ret = 0;
 			goto out_put;
 		}
+		timeout = STARFIVE_RANDGEN_NOWAIT_TIMEOUT;
 	}
 
-	ret = starfive_trng_cmd(trng, STARFIVE_CTRL_GENE_RANDNUM, wait);
-	if (ret)
-		goto out_put;
+	for (;;) {
+		/*
+		 * A pending LFSR lockup taints the next sample. Blocking
+		 * callers reseed here; non-blocking callers return no data
+		 * and leave it to the reseed work. Re-queue it explicitly:
+		 * the lockup may have been seen by starfive_trng_irq_clear()
+		 * or a previous reseed attempt may have failed.
+		 */
+		if (READ_ONCE(trng->need_reseed)) {
+			if (!wait) {
+				starfive_trng_request_reseed(trng);
+				ret = 0;
+				goto out_unlock;
+			}
+			ret = starfive_trng_reseed(trng);
+			if (ret)
+				goto out_unlock;
+		}
+
+		ret = starfive_trng_cmd(trng, STARFIVE_CTRL_GENE_RANDNUM, timeout, wait);
+		if (ret) {
+			if (!wait && (ret == -EBUSY || ret == -ETIMEDOUT))
+				ret = 0;
+			goto out_unlock;
+		}
+
+		/* Lockup reported together with RAND_RDY: reseed and retry once */
+		if (!READ_ONCE(trng->need_reseed))
+			break;
+
+		if (!wait || !retry--) {
+			ret = 0;
+			goto out_unlock;
+		}
+	}
+
+	if (trng->mode == PRNG_256BIT)
+		max = min_t(size_t, max, (STARFIVE_RAND_LEN * 8));
+	else
+		max = min_t(size_t, max, (STARFIVE_RAND_LEN * 4));
 
 	memcpy_fromio(buf, trng->base + STARFIVE_RAND0, max);
+
 	ret = max;
 
+out_unlock:
+	mutex_unlock(&trng->lock);
+
 out_put:
 	pm_runtime_put_autosuspend(trng->dev);
+
 	return ret;
 }
 
 static int starfive_trng_probe(struct platform_device *pdev)
 {
 	int ret;
-	int irq;
 	struct starfive_trng *trng;
 
 	trng = devm_kzalloc(&pdev->dev, sizeof(*trng), GFP_KERNEL);
@@ -368,16 +516,14 @@ static int starfive_trng_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(trng->base),
 				     "Error remapping memory for platform device.\n");
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
+	trng->irq = platform_get_irq(pdev, 0);
+	if (trng->irq < 0)
+		return trng->irq;
 
 	init_completion(&trng->random_done);
 	init_completion(&trng->reseed_done);
-	spin_lock_init(&trng->write_lock);
 
-	ret = devm_request_irq(&pdev->dev, irq, starfive_trng_irq, 0, pdev->name,
-			       (void *)trng);
+	ret = devm_mutex_init(&pdev->dev, &trng->lock);
 	if (ret)
 		return ret;
 
@@ -420,6 +566,15 @@ static int starfive_trng_probe(struct platform_device *pdev)
 	if (ret)
 		return dev_err_probe(&pdev->dev, ret, "Failed to enable runtime PM\n");
 
+	ret = devm_work_autocancel(&pdev->dev, &trng->work, starfive_trng_reseed_work);
+	if (ret)
+		return ret;
+
+	ret = devm_request_irq(&pdev->dev, trng->irq, starfive_trng_irq, IRQF_NO_AUTOEN,
+			       pdev->name, (void *)trng);
+	if (ret)
+		return ret;
+
 	ret = devm_hwrng_register(&pdev->dev, &trng->rng);
 	if (ret)
 		return dev_err_probe(&pdev->dev, ret, "Failed to register hwrng\n");
@@ -432,8 +587,16 @@ static int starfive_trng_probe(struct platform_device *pdev)
 static int starfive_trng_suspend(struct device *dev)
 {
 	struct starfive_trng *trng = dev_get_drvdata(dev);
+	bool assert_rst;
+
+	mutex_lock(&trng->lock);
+	assert_rst = !trng->initialized;
+	writel(0, trng->base + STARFIVE_IE);
+	starfive_trng_irq_clear(trng);
+	mutex_unlock(&trng->lock);
 
-	starfive_trng_power_off(trng, false);
+	synchronize_irq(trng->irq);
+	starfive_trng_power_off(trng, assert_rst);
 
 	return 0;
 }
@@ -444,10 +607,17 @@ static int starfive_trng_resume(struct device *dev)
 	int ret;
 
 	ret = starfive_trng_power_on(trng);
-	if (ret)
+	if (ret) {
 		dev_err(dev, "Failed to power on: %d\n", ret);
+		return ret;
+	}
 
-	return ret;
+	if (trng->initialized) {
+		starfive_trng_irq_clear(trng);
+		writel(STARFIVE_IE_ALL, trng->base + STARFIVE_IE);
+	}
+
+	return 0;
 }
 
 static const struct of_device_id trng_dt_ids[] __maybe_unused = {
-- 
2.43.0