2022-02-16 11:40:50

by Jason A. Donenfeld

[permalink] [raw]
Subject: [PATCH v3] ath9k: use hw_random API instead of directly dumping into random.c

Hardware random number generators are supposed to use the hw_random
framework. This commit turns ath9k's kthread-based design into a proper
hw_random driver.

Cc: Toke Høiland-Jørgensen <[email protected]>
Cc: Kalle Valo <[email protected]>
Cc: Rui Salvaterra <[email protected]>
Cc: Dominik Brodowski <[email protected]>
Cc: Herbert Xu <[email protected]>
Signed-off-by: Jason A. Donenfeld <[email protected]>
---
Changes v2->v3:
- Use msleep_interruptable like other hwrng drivers.
- Give up after 110 tries.
- Return -EIO after giving up like other hwrng drivers.
- Use for loop for style nits.
- Append serial number for driver in case of multiple cards.

Changes v1->v2:
- Count in words rather than bytes.

drivers/net/wireless/ath/ath9k/ath9k.h | 3 +-
drivers/net/wireless/ath/ath9k/rng.c | 72 +++++++++++---------------
2 files changed, 33 insertions(+), 42 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index ef6f5ea06c1f..3ccf8cfc6b63 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -1071,8 +1071,9 @@ struct ath_softc {
#endif

#ifdef CONFIG_ATH9K_HWRNG
+ struct hwrng rng_ops;
u32 rng_last;
- struct task_struct *rng_task;
+ char rng_name[sizeof("ath9k_65535")];
#endif
};

diff --git a/drivers/net/wireless/ath/ath9k/rng.c b/drivers/net/wireless/ath/ath9k/rng.c
index f9d3d6eedd3c..cb5414265a9b 100644
--- a/drivers/net/wireless/ath/ath9k/rng.c
+++ b/drivers/net/wireless/ath/ath9k/rng.c
@@ -21,11 +21,6 @@
#include "hw.h"
#include "ar9003_phy.h"

-#define ATH9K_RNG_BUF_SIZE 320
-#define ATH9K_RNG_ENTROPY(x) (((x) * 8 * 10) >> 5) /* quality: 10/32 */
-
-static DECLARE_WAIT_QUEUE_HEAD(rng_queue);
-
static int ath9k_rng_data_read(struct ath_softc *sc, u32 *buf, u32 buf_size)
{
int i, j;
@@ -71,61 +66,56 @@ static u32 ath9k_rng_delay_get(u32 fail_stats)
return delay;
}

-static int ath9k_rng_kthread(void *data)
+static int ath9k_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
{
- int bytes_read;
- struct ath_softc *sc = data;
- u32 *rng_buf;
- u32 delay, fail_stats = 0;
-
- rng_buf = kmalloc_array(ATH9K_RNG_BUF_SIZE, sizeof(u32), GFP_KERNEL);
- if (!rng_buf)
- goto out;
-
- while (!kthread_should_stop()) {
- bytes_read = ath9k_rng_data_read(sc, rng_buf,
- ATH9K_RNG_BUF_SIZE);
- if (unlikely(!bytes_read)) {
- delay = ath9k_rng_delay_get(++fail_stats);
- wait_event_interruptible_timeout(rng_queue,
- kthread_should_stop(),
- msecs_to_jiffies(delay));
- continue;
+ struct ath_softc *sc = container_of(rng, struct ath_softc, rng_ops);
+ u32 fail_stats = 0, word;
+ int bytes_read = 0;
+
+ for (;;) {
+ if (max & ~3UL)
+ bytes_read = ath9k_rng_data_read(sc, buf, max >> 2);
+ if ((max & 3UL) && ath9k_rng_data_read(sc, &word, 1)) {
+ memcpy(buf + bytes_read, &word, max & 3UL);
+ bytes_read += max & 3UL;
+ memzero_explicit(&word, sizeof(word));
}
+ if (!wait || !max || likely(bytes_read) || fail_stats > 110)
+ break;

- fail_stats = 0;
-
- /* sleep until entropy bits under write_wakeup_threshold */
- add_hwgenerator_randomness((void *)rng_buf, bytes_read,
- ATH9K_RNG_ENTROPY(bytes_read));
+ msleep_interruptible(ath9k_rng_delay_get(++fail_stats));
}

- kfree(rng_buf);
-out:
- sc->rng_task = NULL;
-
- return 0;
+ if (wait && !bytes_read && max)
+ bytes_read = -EIO;
+ return bytes_read;
}

void ath9k_rng_start(struct ath_softc *sc)
{
+ static atomic_t serial = ATOMIC_INIT(0);
struct ath_hw *ah = sc->sc_ah;

- if (sc->rng_task)
+ if (sc->rng_ops.read)
return;

if (!AR_SREV_9300_20_OR_LATER(ah))
return;

- sc->rng_task = kthread_run(ath9k_rng_kthread, sc, "ath9k-hwrng");
- if (IS_ERR(sc->rng_task))
- sc->rng_task = NULL;
+ snprintf(sc->rng_name, sizeof(sc->rng_name), "ath9k_%u",
+ (atomic_inc_return(&serial) - 1) & U16_MAX);
+ sc->rng_ops.name = sc->rng_name;
+ sc->rng_ops.read = ath9k_rng_read;
+ sc->rng_ops.quality = 320;
+
+ if (devm_hwrng_register(sc->dev, &sc->rng_ops))
+ sc->rng_ops.read = NULL;
}

void ath9k_rng_stop(struct ath_softc *sc)
{
- if (sc->rng_task) {
- kthread_stop(sc->rng_task);
- sc->rng_task = NULL;
+ if (sc->rng_ops.read) {
+ devm_hwrng_unregister(sc->dev, &sc->rng_ops);
+ sc->rng_ops.read = NULL;
}
}
--
2.35.0


2022-02-22 10:04:51

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH v3] ath9k: use hw_random API instead of directly dumping into random.c

On Mon, 21 Feb 2022 at 11:57, Kalle Valo <[email protected]> wrote:
>
> "Jason A. Donenfeld" <[email protected]> wrote:
>
> > Hardware random number generators are supposed to use the hw_random
> > framework. This commit turns ath9k's kthread-based design into a proper
> > hw_random driver.
> >
> > Cc: Toke Høiland-Jørgensen <[email protected]>
> > Cc: Kalle Valo <[email protected]>
> > Cc: Rui Salvaterra <[email protected]>
> > Cc: Dominik Brodowski <[email protected]>
> > Cc: Herbert Xu <[email protected]>
> > Signed-off-by: Jason A. Donenfeld <[email protected]>
> > Tested-by: Rui Salvaterra <[email protected]>
> > Acked-by: Toke Høiland-Jørgensen <[email protected]>
> > Signed-off-by: Kalle Valo <[email protected]>
>
> Patch applied to ath-next branch of ath.git, thanks.
>
> fcd09c90c3c5 ath9k: use hw_random API instead of directly dumping into random.c
>

With this patch, it seems we end up registering the hw_rng every time
the link goes up, and unregister it again when the link goes down,
right?

Wouldn't it be better to split off this driver from the 802.11 link
state handling?