* ath9k should perhaps use hw_random api? @ 2022-02-08 16:51 Jason A. Donenfeld 2022-02-15 15:38 ` Jason A. Donenfeld 0 siblings, 1 reply; 18+ messages in thread From: Jason A. Donenfeld @ 2022-02-08 16:51 UTC (permalink / raw) To: miaoqing; +Cc: Dominik Brodowski, Linux Crypto Mailing List, Herbert Xu Hi Miaoqing, I'm emailing you because I've noticed that ath9k's rng.c is the *only* driver in the whole of the tree that calls add_hwgenerator_randomness() directly, rather than going through Herbert's hw_random API, as every single other hardware RNG does. I'm wondering if you'd consider converting your driver into something suitable for the hw_random API (in drivers/char/hw_random/), rather than adhoc rolling your own ath9k rng kthread. Is this something you're actively maintaining and would be interested in doing? Regards, Jason ^ permalink raw reply [flat|nested] 18+ messages in thread
* ath9k should perhaps use hw_random api? 2022-02-08 16:51 ath9k should perhaps use hw_random api? Jason A. Donenfeld @ 2022-02-15 15:38 ` Jason A. Donenfeld 2022-02-15 16:28 ` [PATCH] ath9k: use hw_random API instead of directly dumping into random.c Jason A. Donenfeld 0 siblings, 1 reply; 18+ messages in thread From: Jason A. Donenfeld @ 2022-02-15 15:38 UTC (permalink / raw) To: miaoqing, Jason Cooper, Sepehrdad, Pouyan, ath9k-devel, linux-wireless, Kalle Valo, Toke Høiland-Jørgensen Cc: Dominik Brodowski, Linux Crypto Mailing List, Herbert Xu, LKML, Netdev Hi Ath9k Maintainers, I'm emailing you because I've noticed that ath9k's rng.c is the *only* driver in the whole of the tree that calls add_hwgenerator_randomness() directly, rather than going through Herbert's hw_random API, as every single other hardware RNG does. I'm wondering if you'd consider converting your driver into something suitable for the hw_random API (in drivers/char/hw_random/), rather than adhoc rolling your own ath9k rng kthread. Is this something you're actively maintaining and would be interested in doing? Regards, Jason ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] ath9k: use hw_random API instead of directly dumping into random.c 2022-02-15 15:38 ` Jason A. Donenfeld @ 2022-02-15 16:28 ` Jason A. Donenfeld 2022-02-15 22:55 ` Rui Salvaterra 2022-02-15 23:22 ` Toke Høiland-Jørgensen 0 siblings, 2 replies; 18+ messages in thread From: Jason A. Donenfeld @ 2022-02-15 16:28 UTC (permalink / raw) To: miaoqing, Jason Cooper, Sepehrdad, Pouyan, ath9k-devel, linux-wireless, Kalle Valo, Toke Høiland-Jørgensen, Dominik Brodowski, Linux Crypto Mailing List, Herbert Xu, LKML, Netdev Cc: Jason A. Donenfeld 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. This compiles, but I have no hardware or other ability to determine whether it works. I'll leave further development up to the ath9k and hw_random maintainers. Cc: Toke Høiland-Jørgensen <toke@redhat.com> Cc: Kalle Valo <kvalo@kernel.org> Cc: Dominik Brodowski <linux@dominikbrodowski.net> Cc: Herbert Xu <herbert@gondor.apana.org.au> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- drivers/net/wireless/ath/ath9k/ath9k.h | 2 +- drivers/net/wireless/ath/ath9k/rng.c | 62 +++++++++----------------- 2 files changed, 23 insertions(+), 41 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h index ef6f5ea06c1f..142f472903dc 100644 --- a/drivers/net/wireless/ath/ath9k/ath9k.h +++ b/drivers/net/wireless/ath/ath9k/ath9k.h @@ -1072,7 +1072,7 @@ struct ath_softc { #ifdef CONFIG_ATH9K_HWRNG u32 rng_last; - struct task_struct *rng_task; + struct hwrng rng_ops; #endif }; diff --git a/drivers/net/wireless/ath/ath9k/rng.c b/drivers/net/wireless/ath/ath9k/rng.c index aae2bd3cac69..369b222908ba 100644 --- a/drivers/net/wireless/ath/ath9k/rng.c +++ b/drivers/net/wireless/ath/ath9k/rng.c @@ -22,9 +22,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) @@ -72,61 +69,46 @@ 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) { + struct ath_softc *sc = container_of(rng, struct ath_softc, rng_ops); 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; - } - - fail_stats = 0; - - /* sleep until entropy bits under write_wakeup_threshold */ - add_hwgenerator_randomness((void *)rng_buf, bytes_read, - ATH9K_RNG_ENTROPY(bytes_read)); - } + u32 fail_stats = 0; - kfree(rng_buf); -out: - sc->rng_task = NULL; +retry: + bytes_read = ath9k_rng_data_read(sc, buf, max); + if (unlikely(!bytes_read) && wait) { + msleep(ath9k_rng_delay_get(++fail_stats)); + goto retry; + } - return 0; + return bytes_read; } void ath9k_rng_start(struct ath_softc *sc) { struct ath_hw *ah = sc->sc_ah; + int ret; - 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; + sc->rng_ops.name = "ath9k"; + sc->rng_ops.read = ath9k_rng_read; + sc->rng_ops.quality = 320; + + ret = devm_hwrng_register(sc->dev, &sc->rng_ops); + if (ret) + 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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] ath9k: use hw_random API instead of directly dumping into random.c 2022-02-15 16:28 ` [PATCH] ath9k: use hw_random API instead of directly dumping into random.c Jason A. Donenfeld @ 2022-02-15 22:55 ` Rui Salvaterra 2022-02-15 23:22 ` Toke Høiland-Jørgensen 1 sibling, 0 replies; 18+ messages in thread From: Rui Salvaterra @ 2022-02-15 22:55 UTC (permalink / raw) To: Jason A. Donenfeld Cc: miaoqing, Jason Cooper, Sepehrdad, Pouyan, ath9k-devel, linux-wireless, Kalle Valo, Toke Høiland-Jørgensen, Dominik Brodowski, Linux Crypto Mailing List, Herbert Xu, LKML, Netdev Hi, Jason, On Tue, 15 Feb 2022 at 22:44, Jason A. Donenfeld <Jason@zx2c4.com> 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. > > This compiles, but I have no hardware or other ability to determine > whether it works. I'll leave further development up to the ath9k > and hw_random maintainers. > > Cc: Toke Høiland-Jørgensen <toke@redhat.com> > Cc: Kalle Valo <kvalo@kernel.org> > Cc: Dominik Brodowski <linux@dominikbrodowski.net> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> [patch snipped] On my laptop, with a… 02:00.0 Network controller: Qualcomm Atheros AR9462 Wireless Network Adapter (rev 01) … I have the following… rui@arrandale:~$ cat /sys/devices/virtual/misc/hw_random/rng_available ath9k rui@arrandale:~$ cat /sys/devices/virtual/misc/hw_random/rng_current ath9k rui@arrandale:~$ … and sure enough, /dev/hwrng is created and outputs a stream of random data, as expected. I haven't done any serious randomness quality testing, but it should be the same as the one produced by the original code. I consider this patch thus Tested-by: Rui Salvaterra <rsalvaterra@gmail.com> Thanks, Rui ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ath9k: use hw_random API instead of directly dumping into random.c 2022-02-15 16:28 ` [PATCH] ath9k: use hw_random API instead of directly dumping into random.c Jason A. Donenfeld 2022-02-15 22:55 ` Rui Salvaterra @ 2022-02-15 23:22 ` Toke Høiland-Jørgensen 2022-02-15 23:52 ` Jason A. Donenfeld 1 sibling, 1 reply; 18+ messages in thread From: Toke Høiland-Jørgensen @ 2022-02-15 23:22 UTC (permalink / raw) To: Jason A. Donenfeld, miaoqing, Jason Cooper, Sepehrdad, Pouyan, ath9k-devel, linux-wireless, Kalle Valo, Dominik Brodowski, Linux Crypto Mailing List, Herbert Xu, LKML, Netdev "Jason A. Donenfeld" <Jason@zx2c4.com> writes: > 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. > > This compiles, but I have no hardware or other ability to determine > whether it works. I'll leave further development up to the ath9k > and hw_random maintainers. > > Cc: Toke Høiland-Jørgensen <toke@redhat.com> > Cc: Kalle Valo <kvalo@kernel.org> > Cc: Dominik Brodowski <linux@dominikbrodowski.net> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > --- > drivers/net/wireless/ath/ath9k/ath9k.h | 2 +- > drivers/net/wireless/ath/ath9k/rng.c | 62 +++++++++----------------- > 2 files changed, 23 insertions(+), 41 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h > index ef6f5ea06c1f..142f472903dc 100644 > --- a/drivers/net/wireless/ath/ath9k/ath9k.h > +++ b/drivers/net/wireless/ath/ath9k/ath9k.h > @@ -1072,7 +1072,7 @@ struct ath_softc { > > #ifdef CONFIG_ATH9K_HWRNG > u32 rng_last; > - struct task_struct *rng_task; > + struct hwrng rng_ops; > #endif > }; > > diff --git a/drivers/net/wireless/ath/ath9k/rng.c b/drivers/net/wireless/ath/ath9k/rng.c > index aae2bd3cac69..369b222908ba 100644 > --- a/drivers/net/wireless/ath/ath9k/rng.c > +++ b/drivers/net/wireless/ath/ath9k/rng.c > @@ -22,9 +22,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 */ So this comment says "quality: 10/32" but below you're setting "quality" as 320. No idea what the units are supposed to be, but is this right? > static DECLARE_WAIT_QUEUE_HEAD(rng_queue); > > static int ath9k_rng_data_read(struct ath_softc *sc, u32 *buf, u32 buf_size) This function takes buf as a *u32, and interprets buf_size as a number of u32s... > @@ -72,61 +69,46 @@ 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) > { > + struct ath_softc *sc = container_of(rng, struct ath_softc, rng_ops); > 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; > - } > - > - fail_stats = 0; > - > - /* sleep until entropy bits under write_wakeup_threshold */ > - add_hwgenerator_randomness((void *)rng_buf, bytes_read, > - ATH9K_RNG_ENTROPY(bytes_read)); > - } > + u32 fail_stats = 0; > > - kfree(rng_buf); > -out: > - sc->rng_task = NULL; > +retry: > + bytes_read = ath9k_rng_data_read(sc, buf, max); ... but AFAICT here you're calling it with a buffer size from hw_random that's in bytes? -Toke ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ath9k: use hw_random API instead of directly dumping into random.c 2022-02-15 23:22 ` Toke Høiland-Jørgensen @ 2022-02-15 23:52 ` Jason A. Donenfeld 2022-02-16 0:02 ` [PATCH v2] " Jason A. Donenfeld 2022-02-16 7:11 ` [PATCH] " Dominik Brodowski 0 siblings, 2 replies; 18+ messages in thread From: Jason A. Donenfeld @ 2022-02-15 23:52 UTC (permalink / raw) To: Toke Høiland-Jørgensen Cc: miaoqing, rsalvaterra, Jason Cooper, Sepehrdad, Pouyan, ath9k-devel, linux-wireless, Kalle Valo, Dominik Brodowski, Linux Crypto Mailing List, Herbert Xu, LKML, Netdev On 2/16/22, Toke Høiland-Jørgensen <toke@toke.dk> wrote: > "Jason A. Donenfeld" <Jason@zx2c4.com> writes: > >> 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. >> >> This compiles, but I have no hardware or other ability to determine >> whether it works. I'll leave further development up to the ath9k >> and hw_random maintainers. >> >> Cc: Toke Høiland-Jørgensen <toke@redhat.com> >> Cc: Kalle Valo <kvalo@kernel.org> >> Cc: Dominik Brodowski <linux@dominikbrodowski.net> >> Cc: Herbert Xu <herbert@gondor.apana.org.au> >> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> >> --- >> drivers/net/wireless/ath/ath9k/ath9k.h | 2 +- >> drivers/net/wireless/ath/ath9k/rng.c | 62 +++++++++----------------- >> 2 files changed, 23 insertions(+), 41 deletions(-) >> >> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h >> b/drivers/net/wireless/ath/ath9k/ath9k.h >> index ef6f5ea06c1f..142f472903dc 100644 >> --- a/drivers/net/wireless/ath/ath9k/ath9k.h >> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h >> @@ -1072,7 +1072,7 @@ struct ath_softc { >> >> #ifdef CONFIG_ATH9K_HWRNG >> u32 rng_last; >> - struct task_struct *rng_task; >> + struct hwrng rng_ops; >> #endif >> }; >> >> diff --git a/drivers/net/wireless/ath/ath9k/rng.c >> b/drivers/net/wireless/ath/ath9k/rng.c >> index aae2bd3cac69..369b222908ba 100644 >> --- a/drivers/net/wireless/ath/ath9k/rng.c >> +++ b/drivers/net/wireless/ath/ath9k/rng.c >> @@ -22,9 +22,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 */ > > So this comment says "quality: 10/32" but below you're setting "quality" > as 320. No idea what the units are supposed to be, but is this right? I think the unit is supposed to be how many entropic bits there are out of 1024 bits? These types of estimates are always BS, so keeping it on the lower end as before seemed right. Herbert can jump in here if he has a better idea; that's his jig. > >> static DECLARE_WAIT_QUEUE_HEAD(rng_queue); >> >> static int ath9k_rng_data_read(struct ath_softc *sc, u32 *buf, u32 >> buf_size) > > This function takes buf as a *u32, and interprets buf_size as a number > of u32s... Oh my... Nice catch. I'll send a v2 shortly. I wonder how this managed to work for Rui. > >> @@ -72,61 +69,46 @@ 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) >> { >> + struct ath_softc *sc = container_of(rng, struct ath_softc, rng_ops); >> 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; >> - } >> - >> - fail_stats = 0; >> - >> - /* sleep until entropy bits under write_wakeup_threshold */ >> - add_hwgenerator_randomness((void *)rng_buf, bytes_read, >> - ATH9K_RNG_ENTROPY(bytes_read)); >> - } >> + u32 fail_stats = 0; >> >> - kfree(rng_buf); >> -out: >> - sc->rng_task = NULL; >> +retry: >> + bytes_read = ath9k_rng_data_read(sc, buf, max); > > ... but AFAICT here you're calling it with a buffer size from hw_random > that's in bytes? V2 on its way. Rui - you may need to re-test... Jason ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2] ath9k: use hw_random API instead of directly dumping into random.c 2022-02-15 23:52 ` Jason A. Donenfeld @ 2022-02-16 0:02 ` Jason A. Donenfeld 2022-02-16 3:13 ` Florian Fainelli ` (2 more replies) 2022-02-16 7:11 ` [PATCH] " Dominik Brodowski 1 sibling, 3 replies; 18+ messages in thread From: Jason A. Donenfeld @ 2022-02-16 0:02 UTC (permalink / raw) To: miaoqing, rsalvaterra, Toke Høiland-Jørgensen, Sepehrdad, Pouyan, ath9k-devel, linux-wireless, Kalle Valo, Dominik Brodowski, Linux Crypto Mailing List, Herbert Xu, LKML, Netdev Cc: Jason A. Donenfeld, Toke Høiland-Jørgensen 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. This compiles, but I have no hardware or other ability to determine whether it works. I'll leave further development up to the ath9k and hw_random maintainers. Cc: Toke Høiland-Jørgensen <toke@redhat.com> Cc: Kalle Valo <kvalo@kernel.org> Cc: Dominik Brodowski <linux@dominikbrodowski.net> Cc: Herbert Xu <herbert@gondor.apana.org.au> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- v2 operates on whole words when possible. drivers/net/wireless/ath/ath9k/ath9k.h | 2 +- drivers/net/wireless/ath/ath9k/rng.c | 72 +++++++++++--------------- 2 files changed, 30 insertions(+), 44 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h index ef6f5ea06c1f..142f472903dc 100644 --- a/drivers/net/wireless/ath/ath9k/ath9k.h +++ b/drivers/net/wireless/ath/ath9k/ath9k.h @@ -1072,7 +1072,7 @@ struct ath_softc { #ifdef CONFIG_ATH9K_HWRNG u32 rng_last; - struct task_struct *rng_task; + struct hwrng rng_ops; #endif }; diff --git a/drivers/net/wireless/ath/ath9k/rng.c b/drivers/net/wireless/ath/ath9k/rng.c index aae2bd3cac69..a0a58f8e08d3 100644 --- a/drivers/net/wireless/ath/ath9k/rng.c +++ b/drivers/net/wireless/ath/ath9k/rng.c @@ -22,11 +22,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; @@ -72,61 +67,52 @@ 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; - } - - fail_stats = 0; - - /* sleep until entropy bits under write_wakeup_threshold */ - add_hwgenerator_randomness((void *)rng_buf, bytes_read, - ATH9K_RNG_ENTROPY(bytes_read)); + struct ath_softc *sc = container_of(rng, struct ath_softc, rng_ops); + int bytes_read = 0; + u32 fail_stats = 0, word; + +retry: + 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 & 3); + bytes_read += max & 3; + memzero_explicit(&word, sizeof(word)); + } + if (max && unlikely(!bytes_read) && wait) { + msleep(ath9k_rng_delay_get(++fail_stats)); + goto retry; } - kfree(rng_buf); -out: - sc->rng_task = NULL; - - return 0; + return bytes_read; } void ath9k_rng_start(struct ath_softc *sc) { struct ath_hw *ah = sc->sc_ah; + int ret; - 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; + sc->rng_ops.name = "ath9k"; + sc->rng_ops.read = ath9k_rng_read; + sc->rng_ops.quality = 320; + + ret = devm_hwrng_register(sc->dev, &sc->rng_ops); + if (ret) + 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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2] ath9k: use hw_random API instead of directly dumping into random.c 2022-02-16 0:02 ` [PATCH v2] " Jason A. Donenfeld @ 2022-02-16 3:13 ` Florian Fainelli 2022-02-16 10:43 ` Jason A. Donenfeld 2022-02-16 5:38 ` [PATCH v2] " Kalle Valo 2022-02-16 7:15 ` Dominik Brodowski 2 siblings, 1 reply; 18+ messages in thread From: Florian Fainelli @ 2022-02-16 3:13 UTC (permalink / raw) To: Jason A. Donenfeld, miaoqing, rsalvaterra, Toke Høiland-Jørgensen, Sepehrdad, Pouyan, ath9k-devel, linux-wireless, Kalle Valo, Dominik Brodowski, Linux Crypto Mailing List, Herbert Xu, LKML, Netdev Cc: Toke Høiland-Jørgensen On 2/15/2022 4:02 PM, Jason A. Donenfeld 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. > > This compiles, but I have no hardware or other ability to determine > whether it works. I'll leave further development up to the ath9k > and hw_random maintainers. > > Cc: Toke Høiland-Jørgensen <toke@redhat.com> > Cc: Kalle Valo <kvalo@kernel.org> > Cc: Dominik Brodowski <linux@dominikbrodowski.net> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > --- [snip] > 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; > + sc->rng_ops.name = "ath9k"; You will have to give this instance an unique name because there can be multiple ath9k adapters registered in a given system (like Wi-Fi routers), and one of the first thing hwrng_register() does is ensure that there is not an existing rng with the same name. Maybe using a combination of ath9k + dev_name() ought to be unique enough? -- Florian ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] ath9k: use hw_random API instead of directly dumping into random.c 2022-02-16 3:13 ` Florian Fainelli @ 2022-02-16 10:43 ` Jason A. Donenfeld 2022-02-16 11:33 ` [PATCH v3] " Jason A. Donenfeld 0 siblings, 1 reply; 18+ messages in thread From: Jason A. Donenfeld @ 2022-02-16 10:43 UTC (permalink / raw) To: Florian Fainelli Cc: miaoqing, Rui Salvaterra, Toke Høiland-Jørgensen, Sepehrdad, Pouyan, ath9k-devel, linux-wireless, Kalle Valo, Dominik Brodowski, Linux Crypto Mailing List, Herbert Xu, LKML, Netdev, Toke Høiland-Jørgensen Hi Florian, On Wed, Feb 16, 2022 at 4:13 AM Florian Fainelli <f.fainelli@gmail.com> wrote: > You will have to give this instance an unique name because there can be > multiple ath9k adapters registered in a given system (like Wi-Fi > routers), and one of the first thing hwrng_register() does is ensure > that there is not an existing rng with the same name. > > Maybe using a combination of ath9k + dev_name() ought to be unique enough? Good point. Will do. dev_name() probably won't cut it because of namespaces, but I can always just attach a counter. Will do that for v3. Jason ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3] ath9k: use hw_random API instead of directly dumping into random.c 2022-02-16 10:43 ` Jason A. Donenfeld @ 2022-02-16 11:33 ` Jason A. Donenfeld 2022-02-16 13:27 ` Rui Salvaterra ` (2 more replies) 0 siblings, 3 replies; 18+ messages in thread From: Jason A. Donenfeld @ 2022-02-16 11:33 UTC (permalink / raw) To: miaoqing, Rui Salvaterra, Toke Høiland-Jørgensen, Sepehrdad, Pouyan, ath9k-devel, linux-wireless, Kalle Valo, Dominik Brodowski, Linux Crypto Mailing List, Herbert Xu, LKML, Netdev, Toke Høiland-Jørgensen, Florian Fainelli Cc: Jason A. Donenfeld 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 <toke@redhat.com> Cc: Kalle Valo <kvalo@kernel.org> Cc: Rui Salvaterra <rsalvaterra@gmail.com> Cc: Dominik Brodowski <linux@dominikbrodowski.net> Cc: Herbert Xu <herbert@gondor.apana.org.au> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- 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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3] ath9k: use hw_random API instead of directly dumping into random.c 2022-02-16 11:33 ` [PATCH v3] " Jason A. Donenfeld @ 2022-02-16 13:27 ` Rui Salvaterra 2022-02-17 13:51 ` Toke Høiland-Jørgensen 2022-02-21 10:22 ` Kalle Valo 2 siblings, 0 replies; 18+ messages in thread From: Rui Salvaterra @ 2022-02-16 13:27 UTC (permalink / raw) To: Jason A. Donenfeld Cc: miaoqing, Toke Høiland-Jørgensen, Sepehrdad, Pouyan, ath9k-devel, linux-wireless, Kalle Valo, Dominik Brodowski, Linux Crypto Mailing List, Herbert Xu, LKML, Netdev, Toke Høiland-Jørgensen, Florian Fainelli Hi again, Jason, On Wed, 16 Feb 2022 at 11:33, Jason A. Donenfeld <Jason@zx2c4.com> wrote: [snipped] > 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. [snipped] Everything working as expected here, so this patch (v3) is also Tested-by: Rui Salvaterra <rsalvaterra@gmail.com> Thanks, Rui ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3] ath9k: use hw_random API instead of directly dumping into random.c 2022-02-16 11:33 ` [PATCH v3] " Jason A. Donenfeld 2022-02-16 13:27 ` Rui Salvaterra @ 2022-02-17 13:51 ` Toke Høiland-Jørgensen 2022-02-21 10:22 ` Kalle Valo 2 siblings, 0 replies; 18+ messages in thread From: Toke Høiland-Jørgensen @ 2022-02-17 13:51 UTC (permalink / raw) To: Jason A. Donenfeld, miaoqing, Rui Salvaterra, Sepehrdad, Pouyan, ath9k-devel, linux-wireless, Kalle Valo, Dominik Brodowski, Linux Crypto Mailing List, Herbert Xu, LKML, Netdev, Florian Fainelli Cc: Jason A. Donenfeld "Jason A. Donenfeld" <Jason@zx2c4.com> writes: > 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 <toke@redhat.com> > Cc: Kalle Valo <kvalo@kernel.org> > Cc: Rui Salvaterra <rsalvaterra@gmail.com> > Cc: Dominik Brodowski <linux@dominikbrodowski.net> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> Alright, LGTM. Thank you for the patch! Acked-by: Toke Høiland-Jørgensen <toke@toke.dk> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3] ath9k: use hw_random API instead of directly dumping into random.c 2022-02-16 11:33 ` [PATCH v3] " Jason A. Donenfeld 2022-02-16 13:27 ` Rui Salvaterra 2022-02-17 13:51 ` Toke Høiland-Jørgensen @ 2022-02-21 10:22 ` Kalle Valo 2022-02-22 10:01 ` Ard Biesheuvel 2 siblings, 1 reply; 18+ messages in thread From: Kalle Valo @ 2022-02-21 10:22 UTC (permalink / raw) To: Jason A. Donenfeld Cc: miaoqing, Rui Salvaterra, Toke Høiland-Jørgensen, Sepehrdad, Pouyan, ath9k-devel, linux-wireless, Dominik Brodowski, Linux Crypto Mailing List, Herbert Xu, LKML, Netdev, Toke Høiland-Jørgensen, Florian Fainelli, Jason A. Donenfeld "Jason A. Donenfeld" <Jason@zx2c4.com> 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 <toke@redhat.com> > Cc: Kalle Valo <kvalo@kernel.org> > Cc: Rui Salvaterra <rsalvaterra@gmail.com> > Cc: Dominik Brodowski <linux@dominikbrodowski.net> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > Tested-by: Rui Salvaterra <rsalvaterra@gmail.com> > Acked-by: Toke Høiland-Jørgensen <toke@toke.dk> > Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> Patch applied to ath-next branch of ath.git, thanks. fcd09c90c3c5 ath9k: use hw_random API instead of directly dumping into random.c -- https://patchwork.kernel.org/project/linux-wireless/patch/20220216113323.53332-1-Jason@zx2c4.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3] ath9k: use hw_random API instead of directly dumping into random.c 2022-02-21 10:22 ` Kalle Valo @ 2022-02-22 10:01 ` Ard Biesheuvel 2022-02-22 10:13 ` Jason A. Donenfeld 0 siblings, 1 reply; 18+ messages in thread From: Ard Biesheuvel @ 2022-02-22 10:01 UTC (permalink / raw) To: Kalle Valo Cc: Jason A. Donenfeld, miaoqing, Rui Salvaterra, Toke Høiland-Jørgensen, Sepehrdad, Pouyan, ath9k-devel, linux-wireless, Dominik Brodowski, Linux Crypto Mailing List, Herbert Xu, LKML, Netdev, Toke Høiland-Jørgensen, Florian Fainelli On Mon, 21 Feb 2022 at 11:57, Kalle Valo <kvalo@kernel.org> wrote: > > "Jason A. Donenfeld" <Jason@zx2c4.com> 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 <toke@redhat.com> > > Cc: Kalle Valo <kvalo@kernel.org> > > Cc: Rui Salvaterra <rsalvaterra@gmail.com> > > Cc: Dominik Brodowski <linux@dominikbrodowski.net> > > Cc: Herbert Xu <herbert@gondor.apana.org.au> > > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > > Tested-by: Rui Salvaterra <rsalvaterra@gmail.com> > > Acked-by: Toke Høiland-Jørgensen <toke@toke.dk> > > Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> > > 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? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3] ath9k: use hw_random API instead of directly dumping into random.c 2022-02-22 10:01 ` Ard Biesheuvel @ 2022-02-22 10:13 ` Jason A. Donenfeld 0 siblings, 0 replies; 18+ messages in thread From: Jason A. Donenfeld @ 2022-02-22 10:13 UTC (permalink / raw) To: Ard Biesheuvel Cc: Kalle Valo, miaoqing, Rui Salvaterra, Toke Høiland-Jørgensen, Sepehrdad, Pouyan, ath9k-devel, linux-wireless, Dominik Brodowski, Linux Crypto Mailing List, Herbert Xu, LKML, Netdev, Toke Høiland-Jørgensen, Florian Fainelli On 2/22/22, Ard Biesheuvel <ardb@kernel.org> wrote: > On Mon, 21 Feb 2022 at 11:57, Kalle Valo <kvalo@kernel.org> wrote: >> >> "Jason A. Donenfeld" <Jason@zx2c4.com> 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 <toke@redhat.com> >> > Cc: Kalle Valo <kvalo@kernel.org> >> > Cc: Rui Salvaterra <rsalvaterra@gmail.com> >> > Cc: Dominik Brodowski <linux@dominikbrodowski.net> >> > Cc: Herbert Xu <herbert@gondor.apana.org.au> >> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> >> > Tested-by: Rui Salvaterra <rsalvaterra@gmail.com> >> > Acked-by: Toke Høiland-Jørgensen <toke@toke.dk> >> > Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> >> >> 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? > I really have no idea how this thing works, and I tried hard to change as little as possible in converting it to the API. You may want to send some follow-up patches if you have hardware to experiment with. One consideration does leap out, which is that in my experience wifi cards use a lot less power when they're set "down", as though a decent amount of hardware is being switched off. I think this ath9k rng call might be using the ADC to gather samples of ether from somewhere. I imagine this gets shutdown too as part of that dame circuitry. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] ath9k: use hw_random API instead of directly dumping into random.c 2022-02-16 0:02 ` [PATCH v2] " Jason A. Donenfeld 2022-02-16 3:13 ` Florian Fainelli @ 2022-02-16 5:38 ` Kalle Valo 2022-02-16 7:15 ` Dominik Brodowski 2 siblings, 0 replies; 18+ messages in thread From: Kalle Valo @ 2022-02-16 5:38 UTC (permalink / raw) To: Jason A. Donenfeld Cc: miaoqing, rsalvaterra, Toke Høiland-Jørgensen, Sepehrdad, Pouyan, ath9k-devel, linux-wireless, Dominik Brodowski, Linux Crypto Mailing List, Herbert Xu, LKML, Netdev, Toke Høiland-Jørgensen "Jason A. Donenfeld" <Jason@zx2c4.com> writes: > 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. > > This compiles, but I have no hardware or other ability to determine > whether it works. I'll leave further development up to the ath9k > and hw_random maintainers. > > Cc: Toke Høiland-Jørgensen <toke@redhat.com> > Cc: Kalle Valo <kvalo@kernel.org> > Cc: Dominik Brodowski <linux@dominikbrodowski.net> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> [...] > +retry: > + 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 & 3); > + bytes_read += max & 3; > + memzero_explicit(&word, sizeof(word)); > + } > + if (max && unlikely(!bytes_read) && wait) { > + msleep(ath9k_rng_delay_get(++fail_stats)); > + goto retry; > } Wouldn't a while loop be cleaner? With a some kind limit for the number of loops, to avoid a neverending loop. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] ath9k: use hw_random API instead of directly dumping into random.c 2022-02-16 0:02 ` [PATCH v2] " Jason A. Donenfeld 2022-02-16 3:13 ` Florian Fainelli 2022-02-16 5:38 ` [PATCH v2] " Kalle Valo @ 2022-02-16 7:15 ` Dominik Brodowski 2 siblings, 0 replies; 18+ messages in thread From: Dominik Brodowski @ 2022-02-16 7:15 UTC (permalink / raw) To: Jason A. Donenfeld Cc: miaoqing, rsalvaterra, Toke Høiland-Jørgensen, Sepehrdad, Pouyan, ath9k-devel, linux-wireless, Kalle Valo, Linux Crypto Mailing List, Herbert Xu, LKML, Netdev, Toke Høiland-Jørgensen Am Wed, Feb 16, 2022 at 01:02:30AM +0100 schrieb Jason A. Donenfeld: > 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. > > This compiles, but I have no hardware or other ability to determine > whether it works. I'll leave further development up to the ath9k > and hw_random maintainers. > > Cc: Toke Høiland-Jørgensen <toke@redhat.com> > Cc: Kalle Valo <kvalo@kernel.org> > Cc: Dominik Brodowski <linux@dominikbrodowski.net> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > --- > v2 operates on whole words when possible. > > drivers/net/wireless/ath/ath9k/ath9k.h | 2 +- > drivers/net/wireless/ath/ath9k/rng.c | 72 +++++++++++--------------- > 2 files changed, 30 insertions(+), 44 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h > index ef6f5ea06c1f..142f472903dc 100644 > --- a/drivers/net/wireless/ath/ath9k/ath9k.h > +++ b/drivers/net/wireless/ath/ath9k/ath9k.h > @@ -1072,7 +1072,7 @@ struct ath_softc { > > #ifdef CONFIG_ATH9K_HWRNG > u32 rng_last; > - struct task_struct *rng_task; > + struct hwrng rng_ops; > #endif > }; > > diff --git a/drivers/net/wireless/ath/ath9k/rng.c b/drivers/net/wireless/ath/ath9k/rng.c > index aae2bd3cac69..a0a58f8e08d3 100644 > --- a/drivers/net/wireless/ath/ath9k/rng.c > +++ b/drivers/net/wireless/ath/ath9k/rng.c > @@ -22,11 +22,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; > @@ -72,61 +67,52 @@ 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; > - } > - > - fail_stats = 0; > - > - /* sleep until entropy bits under write_wakeup_threshold */ > - add_hwgenerator_randomness((void *)rng_buf, bytes_read, > - ATH9K_RNG_ENTROPY(bytes_read)); > + struct ath_softc *sc = container_of(rng, struct ath_softc, rng_ops); > + int bytes_read = 0; > + u32 fail_stats = 0, word; > + > +retry: > + 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 & 3); > + bytes_read += max & 3; > + memzero_explicit(&word, sizeof(word)); > + } > + if (max && unlikely(!bytes_read) && wait) { > + msleep(ath9k_rng_delay_get(++fail_stats)); > + goto retry; > } Potentially, this waits forever, if wait is set and no data is returned. Instead, it should return to the main kthread loop every once in a while. Thanks, Dominik ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ath9k: use hw_random API instead of directly dumping into random.c 2022-02-15 23:52 ` Jason A. Donenfeld 2022-02-16 0:02 ` [PATCH v2] " Jason A. Donenfeld @ 2022-02-16 7:11 ` Dominik Brodowski 1 sibling, 0 replies; 18+ messages in thread From: Dominik Brodowski @ 2022-02-16 7:11 UTC (permalink / raw) To: Jason A. Donenfeld Cc: Toke Høiland-Jørgensen, miaoqing, rsalvaterra, Jason Cooper, Sepehrdad, Pouyan, ath9k-devel, linux-wireless, Kalle Valo, Linux Crypto Mailing List, Herbert Xu, LKML, Netdev Am Wed, Feb 16, 2022 at 12:52:15AM +0100 schrieb Jason A. Donenfeld: > On 2/16/22, Toke Høiland-Jørgensen <toke@toke.dk> wrote: > >> @@ -22,9 +22,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 */ > > > > So this comment says "quality: 10/32" but below you're setting "quality" > > as 320. No idea what the units are supposed to be, but is this right? > > I think the unit is supposed to be how many entropic bits there are > out of 1024 bits? These types of estimates are always BS, so keeping > it on the lower end as before seemed right. Herbert can jump in here > if he has a better idea; that's his jig. 10/32 = 320/1024, so that change is correct. Dominik ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2022-02-22 10:13 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-02-08 16:51 ath9k should perhaps use hw_random api? Jason A. Donenfeld 2022-02-15 15:38 ` Jason A. Donenfeld 2022-02-15 16:28 ` [PATCH] ath9k: use hw_random API instead of directly dumping into random.c Jason A. Donenfeld 2022-02-15 22:55 ` Rui Salvaterra 2022-02-15 23:22 ` Toke Høiland-Jørgensen 2022-02-15 23:52 ` Jason A. Donenfeld 2022-02-16 0:02 ` [PATCH v2] " Jason A. Donenfeld 2022-02-16 3:13 ` Florian Fainelli 2022-02-16 10:43 ` Jason A. Donenfeld 2022-02-16 11:33 ` [PATCH v3] " Jason A. Donenfeld 2022-02-16 13:27 ` Rui Salvaterra 2022-02-17 13:51 ` Toke Høiland-Jørgensen 2022-02-21 10:22 ` Kalle Valo 2022-02-22 10:01 ` Ard Biesheuvel 2022-02-22 10:13 ` Jason A. Donenfeld 2022-02-16 5:38 ` [PATCH v2] " Kalle Valo 2022-02-16 7:15 ` Dominik Brodowski 2022-02-16 7:11 ` [PATCH] " Dominik Brodowski
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.