diff mbox

[v2] ath9k: export HW random number generator

Message ID 1435912110-9646-1-git-send-email-miaoqing@qca.qualcomm.com (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show

Commit Message

miaoqing pan July 3, 2015, 8:28 a.m. UTC
From: Miaoqing Pan <miaoqing@qca.qualcomm.com>

We measured the ADC-based entropy in 3 ways, Shannon entropy,
collision entropy, and directly measured min-entropy. Entropy is
in bits per 16 bit value,
   ---------------------------
   Shannon | collision | min
   ---------------------------
   12.00   | 10.80     | 9.18
   ---------------------------

Recommend: A generous safety factor be used. NIST Special Publication
800-90B (draft) requires that data used to seed a deterministic random
bit generator with N bits of strength have an estimated entropy at least
twice the block size of the underlying primitive. Given all the
uncertainties, it would be wise to collect even more.

Analysis was done by Jacobson,David(djacobso@qti.qualcomm.com).

Signed-off-by: Miaoqing Pan <miaoqing@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath9k/Kconfig  |  7 ++++
 drivers/net/wireless/ath/ath9k/Makefile |  1 +
 drivers/net/wireless/ath/ath9k/ath9k.h  | 23 ++++++++++++
 drivers/net/wireless/ath/ath9k/main.c   |  4 ++
 drivers/net/wireless/ath/ath9k/rng.c    | 66 +++++++++++++++++++++++++++++++++
 5 files changed, 101 insertions(+)
 create mode 100644 drivers/net/wireless/ath/ath9k/rng.c

Comments

Felix Fietkau July 6, 2015, 10:31 a.m. UTC | #1
On 2015-07-03 10:28, miaoqing@qti.qualcomm.com wrote:
> From: Miaoqing Pan <miaoqing@qca.qualcomm.com>
> 
> We measured the ADC-based entropy in 3 ways, Shannon entropy,
> collision entropy, and directly measured min-entropy. Entropy is
> in bits per 16 bit value,
>    ---------------------------
>    Shannon | collision | min
>    ---------------------------
>    12.00   | 10.80     | 9.18
>    ---------------------------
> 
> Recommend: A generous safety factor be used. NIST Special Publication
> 800-90B (draft) requires that data used to seed a deterministic random
> bit generator with N bits of strength have an estimated entropy at least
> twice the block size of the underlying primitive. Given all the
> uncertainties, it would be wise to collect even more.
> 
> Analysis was done by Jacobson,David(djacobso@qti.qualcomm.com).
> 
> Signed-off-by: Miaoqing Pan <miaoqing@qca.qualcomm.com>
> ---
>  drivers/net/wireless/ath/ath9k/Kconfig  |  7 ++++
>  drivers/net/wireless/ath/ath9k/Makefile |  1 +
>  drivers/net/wireless/ath/ath9k/ath9k.h  | 23 ++++++++++++
>  drivers/net/wireless/ath/ath9k/main.c   |  4 ++
>  drivers/net/wireless/ath/ath9k/rng.c    | 66 +++++++++++++++++++++++++++++++++
>  5 files changed, 101 insertions(+)
>  create mode 100644 drivers/net/wireless/ath/ath9k/rng.c
> 

> --- /dev/null
> +++ b/drivers/net/wireless/ath/ath9k/rng.c
> @@ -0,0 +1,66 @@
> +/*
> + * Copyright (c) 2015 Qualcomm Atheros, Inc.
> + *
> + * Permission to use, copy, modify, and/or distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include "ath9k.h"
> +#include "hw.h"
> +#include "ar9003_phy.h"
> +
> +static int ath9k_rng_data_read(struct hwrng *rng, u32 *data)
> +{
> +	u32 v1, v2;
> +	struct ath_softc *sc = (struct ath_softc *)rng->priv;
> +	struct ath_hw *ah = sc->sc_ah;
> +
> +	ath9k_ps_wakeup(sc);
> +
> +	v1 = REG_READ(ah, AR_PHY_TST_ADC);
> +	v2 = REG_READ(ah, AR_PHY_TST_ADC);
> +
> +	ath9k_ps_restore(sc);
> +
> +	/* wait for data ready */
> +	if (v1 && v2 && sc->rng_last != v1 && v1 != v2) {
> +		*data = (v1 & 0xffff) | (v2 << 16);
> +		sc->rng_last = v2;
> +
> +		return sizeof(u32);
> +	}
I have some doubt about this part. Doesn't the value of AR_PHY_TST_ADC
depend on the initialization of the baseband observation registers?
What guarantee is there that it's initialized to return any data that is
useful for random number generation on all chipsets?
Did you validate all relevant initval settings for this?

- Felix
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
miaoqing pan July 7, 2015, 5:07 a.m. UTC | #2
AR_PHY_TST_ADC is BB ADC sample register, it doesn't depend on other BB registers.  Get non zero data if it has the ADC values.   

Thanks,
Miaoqing
-----Original Message-----
From: Felix Fietkau [mailto:nbd@openwrt.org] 
Sent: Monday, July 06, 2015 6:32 PM
To: Pan, Miaoqing; linville@tuxdriver.com
Cc: linux-wireless@vger.kernel.org; ath9k-devel; Valo, Kalle
Subject: Re: [PATCH v2] ath9k: export HW random number generator

On 2015-07-03 10:28, miaoqing@qti.qualcomm.com wrote:
> From: Miaoqing Pan <miaoqing@qca.qualcomm.com>
> 
> We measured the ADC-based entropy in 3 ways, Shannon entropy, 
> collision entropy, and directly measured min-entropy. Entropy is in 
> bits per 16 bit value,
>    ---------------------------
>    Shannon | collision | min
>    ---------------------------
>    12.00   | 10.80     | 9.18
>    ---------------------------
> 
> Recommend: A generous safety factor be used. NIST Special Publication 
> 800-90B (draft) requires that data used to seed a deterministic random 
> bit generator with N bits of strength have an estimated entropy at 
> least twice the block size of the underlying primitive. Given all the 
> uncertainties, it would be wise to collect even more.
> 
> Analysis was done by Jacobson,David(djacobso@qti.qualcomm.com).
> 
> Signed-off-by: Miaoqing Pan <miaoqing@qca.qualcomm.com>
> ---
>  drivers/net/wireless/ath/ath9k/Kconfig  |  7 ++++  
> drivers/net/wireless/ath/ath9k/Makefile |  1 +  
> drivers/net/wireless/ath/ath9k/ath9k.h  | 23 ++++++++++++
>  drivers/net/wireless/ath/ath9k/main.c   |  4 ++
>  drivers/net/wireless/ath/ath9k/rng.c    | 66 +++++++++++++++++++++++++++++++++
>  5 files changed, 101 insertions(+)
>  create mode 100644 drivers/net/wireless/ath/ath9k/rng.c
> 

> --- /dev/null
> +++ b/drivers/net/wireless/ath/ath9k/rng.c
> @@ -0,0 +1,66 @@
> +/*
> + * Copyright (c) 2015 Qualcomm Atheros, Inc.
> + *
> + * Permission to use, copy, modify, and/or distribute this software 
> +for any
> + * purpose with or without fee is hereby granted, provided that the 
> +above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL 
> +WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE 
> +LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY 
> +DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN 
> +AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING 
> +OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include "ath9k.h"
> +#include "hw.h"
> +#include "ar9003_phy.h"
> +
> +static int ath9k_rng_data_read(struct hwrng *rng, u32 *data) {
> +	u32 v1, v2;
> +	struct ath_softc *sc = (struct ath_softc *)rng->priv;
> +	struct ath_hw *ah = sc->sc_ah;
> +
> +	ath9k_ps_wakeup(sc);
> +
> +	v1 = REG_READ(ah, AR_PHY_TST_ADC);
> +	v2 = REG_READ(ah, AR_PHY_TST_ADC);
> +
> +	ath9k_ps_restore(sc);
> +
> +	/* wait for data ready */
> +	if (v1 && v2 && sc->rng_last != v1 && v1 != v2) {
> +		*data = (v1 & 0xffff) | (v2 << 16);
> +		sc->rng_last = v2;
> +
> +		return sizeof(u32);
> +	}
I have some doubt about this part. Doesn't the value of AR_PHY_TST_ADC depend on the initialization of the baseband observation registers?
What guarantee is there that it's initialized to return any data that is useful for random number generation on all chipsets?
Did you validate all relevant initval settings for this?

- Felix
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Felix Fietkau July 7, 2015, 7:08 a.m. UTC | #3
On 2015-07-07 07:07, Pan, Miaoqing wrote:
> AR_PHY_TST_ADC is BB ADC sample register, it doesn't depend on other
> BB registers. Get non zero data if it has the ADC values.
This is completely wrong!

AR_PHY_TST_ADC is a generic observation register which can be configured
to observe multiple points in the rx path.
I did my own experiments with sampling raw I/Q values from the ADC, and
you can find my patch here:

http://git.openwrt.org/?p=openwrt.git;a=blob_plain;f=package/kernel/mac80211/patches/543-ath9k_entropy_from_adc.patch

It right now uses only the lowest bits (they are the hardest to control
from the outside).

Through extensive testing (not just by me, but also by many people in
the OpenWrt community), I found that leaving the AR_PHY_TST_ADC register
configured for raw I/Q samples causes the radio to become unstable on
AR5008-AR9002. That's why my patch currently only gathers some data at
driver init time.
By the way, on AR5008-AR9002 this register is in a different location
than AR9003, something your patch also needs to take into account.

You should also make sure that the radio is actually configured to a
channel (and active!) before gathering data.

When you rework your patch, please carefully review the documentation
for the fields rx_obs_sel and cf_bbb_obs_sel before you change the code
again.

- Felix
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
miaoqing pan July 7, 2015, 9:20 a.m. UTC | #4
Thanks Felix, I think that's why we can't get the ideal entropy.  And  I only saw the unstable issue when read the ADC values from MAC_PCU buffer after enable BB multiplexes the ADC input from radio to MAC ,  as it will defer the regular operation.

- Miaoqing

-----Original Message-----
From: Felix Fietkau [mailto:nbd@openwrt.org] 
Sent: Tuesday, July 07, 2015 3:09 PM
To: Pan, Miaoqing; linville@tuxdriver.com
Cc: linux-wireless@vger.kernel.org; ath9k-devel; Valo, Kalle
Subject: Re: [PATCH v2] ath9k: export HW random number generator

On 2015-07-07 07:07, Pan, Miaoqing wrote:
> AR_PHY_TST_ADC is BB ADC sample register, it doesn't depend on other 
> BB registers. Get non zero data if it has the ADC values.
This is completely wrong!

AR_PHY_TST_ADC is a generic observation register which can be configured to observe multiple points in the rx path.
I did my own experiments with sampling raw I/Q values from the ADC, and you can find my patch here:

http://git.openwrt.org/?p=openwrt.git;a=blob_plain;f=package/kernel/mac80211/patches/543-ath9k_entropy_from_adc.patch

It right now uses only the lowest bits (they are the hardest to control from the outside).

Through extensive testing (not just by me, but also by many people in the OpenWrt community), I found that leaving the AR_PHY_TST_ADC register configured for raw I/Q samples causes the radio to become unstable on AR5008-AR9002. That's why my patch currently only gathers some data at driver init time.
By the way, on AR5008-AR9002 this register is in a different location than AR9003, something your patch also needs to take into account.

You should also make sure that the radio is actually configured to a channel (and active!) before gathering data.

When you rework your patch, please carefully review the documentation for the fields rx_obs_sel and cf_bbb_obs_sel before you change the code again.

- Felix
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/wireless/ath/ath9k/Kconfig b/drivers/net/wireless/ath/ath9k/Kconfig
index fee0cad..bde62ec9 100644
--- a/drivers/net/wireless/ath/ath9k/Kconfig
+++ b/drivers/net/wireless/ath/ath9k/Kconfig
@@ -176,3 +176,10 @@  config ATH9K_HTC_DEBUGFS
 	depends on ATH9K_HTC && DEBUG_FS
 	---help---
 	  Say Y, if you need access to ath9k_htc's statistics.
+
+config ATH9K_HWRNG
+	bool "Random number generator support"
+	depends on ATH9K && (HW_RANDOM = y || HW_RANDOM = ATH9K)
+	default y
+	---help---
+	  Provides a hardware random number generator to the kernel.
diff --git a/drivers/net/wireless/ath/ath9k/Makefile b/drivers/net/wireless/ath/ath9k/Makefile
index ecda613..76f9dc3 100644
--- a/drivers/net/wireless/ath/ath9k/Makefile
+++ b/drivers/net/wireless/ath/ath9k/Makefile
@@ -15,6 +15,7 @@  ath9k-$(CONFIG_ATH9K_DFS_DEBUGFS) += dfs_debug.o
 ath9k-$(CONFIG_ATH9K_DFS_CERTIFIED) += dfs.o
 ath9k-$(CONFIG_ATH9K_TX99) += tx99.o
 ath9k-$(CONFIG_ATH9K_WOW) += wow.o
+ath9k-$(CONFIG_ATH9K_HWRNG) += rng.o
 
 ath9k-$(CONFIG_ATH9K_DEBUGFS) += debug.o
 
diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index a7a81b3..45596e5 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -23,6 +23,7 @@ 
 #include <linux/leds.h>
 #include <linux/completion.h>
 #include <linux/time.h>
+#include <linux/hw_random.h>
 
 #include "common.h"
 #include "debug.h"
@@ -1041,6 +1042,12 @@  struct ath_softc {
 	u32 wow_intr_before_sleep;
 	bool force_wow;
 #endif
+
+#ifdef CONFIG_ATH9K_HWRNG
+	struct hwrng rng;
+	bool rng_initialized;
+	u32 rng_last;
+#endif
 };
 
 /********/
@@ -1063,6 +1070,22 @@  static inline int ath9k_tx99_send(struct ath_softc *sc,
 }
 #endif /* CONFIG_ATH9K_TX99 */
 
+/***************************/
+/* Random Number Generator */
+/***************************/
+#ifdef CONFIG_ATH9K_HWRNG
+void ath9k_rng_register(struct ath_softc *sc);
+void ath9k_rng_unregister(struct ath_softc *sc);
+#else
+static inline void ath9k_rng_register(struct ath_softc *sc)
+{
+}
+
+static inline void ath9k_rng_unregister(struct ath_softc *sc)
+{
+}
+#endif
+
 static inline void ath_read_cachesize(struct ath_common *common, int *csz)
 {
 	common->bus_ops->read_cachesize(common, csz);
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index cfd45cb..5916ab2 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -739,6 +739,8 @@  static int ath9k_start(struct ieee80211_hw *hw)
 
 	ath9k_ps_restore(sc);
 
+	ath9k_rng_register(sc);
+
 	return 0;
 }
 
@@ -828,6 +830,8 @@  static void ath9k_stop(struct ieee80211_hw *hw)
 
 	ath9k_deinit_channel_context(sc);
 
+	ath9k_rng_unregister(sc);
+
 	mutex_lock(&sc->mutex);
 
 	ath_cancel_work(sc);
diff --git a/drivers/net/wireless/ath/ath9k/rng.c b/drivers/net/wireless/ath/ath9k/rng.c
new file mode 100644
index 0000000..e2cce31
--- /dev/null
+++ b/drivers/net/wireless/ath/ath9k/rng.c
@@ -0,0 +1,66 @@ 
+/*
+ * Copyright (c) 2015 Qualcomm Atheros, Inc.
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include "ath9k.h"
+#include "hw.h"
+#include "ar9003_phy.h"
+
+static int ath9k_rng_data_read(struct hwrng *rng, u32 *data)
+{
+	u32 v1, v2;
+	struct ath_softc *sc = (struct ath_softc *)rng->priv;
+	struct ath_hw *ah = sc->sc_ah;
+
+	ath9k_ps_wakeup(sc);
+
+	v1 = REG_READ(ah, AR_PHY_TST_ADC);
+	v2 = REG_READ(ah, AR_PHY_TST_ADC);
+
+	ath9k_ps_restore(sc);
+
+	/* wait for data ready */
+	if (v1 && v2 && sc->rng_last != v1 && v1 != v2) {
+		*data = (v1 & 0xffff) | (v2 << 16);
+		sc->rng_last = v2;
+
+		return sizeof(u32);
+	}
+
+	sc->rng_last = v2;
+
+	return 0;
+}
+
+void ath9k_rng_register(struct ath_softc *sc)
+{
+	if (WARN_ON(sc->rng_initialized))
+		return;
+
+	sc->rng.name = "ath9k";
+	sc->rng.data_read = ath9k_rng_data_read;
+	sc->rng.priv = (unsigned long)sc;
+
+	if (!hwrng_register(&sc->rng))
+		sc->rng_initialized = true;
+}
+
+void ath9k_rng_unregister(struct ath_softc *sc)
+{
+	if (sc->rng_initialized) {
+		hwrng_unregister(&sc->rng);
+		sc->rng_initialized = false;
+	}
+}