2006-05-12 10:32:22

by Michael Büsch

[permalink] [raw]
Subject: [patch 9/9] Add bcm43xx HW RNG support

Signed-off-by: Michael Buesch <[email protected]>
Index: hwrng/drivers/net/wireless/bcm43xx/Kconfig
===================================================================
--- hwrng.orig/drivers/net/wireless/bcm43xx/Kconfig 2006-05-12 12:07:13.000000000 +0200
+++ hwrng/drivers/net/wireless/bcm43xx/Kconfig 2006-05-12 12:10:58.000000000 +0200
@@ -2,6 +2,7 @@
tristate "Broadcom BCM43xx wireless support"
depends on PCI && IEEE80211 && IEEE80211_SOFTMAC && NET_RADIO && EXPERIMENTAL
select FW_LOADER
+ select HW_RANDOM
---help---
This is an experimental driver for the Broadcom 43xx wireless chip,
found in the Apple Airport Extreme and various other devices.
Index: hwrng/drivers/net/wireless/bcm43xx/bcm43xx.h
===================================================================
--- hwrng.orig/drivers/net/wireless/bcm43xx/bcm43xx.h 2006-05-12 12:07:13.000000000 +0200
+++ hwrng/drivers/net/wireless/bcm43xx/bcm43xx.h 2006-05-12 12:10:58.000000000 +0200
@@ -1,6 +1,7 @@
#ifndef BCM43xx_H_
#define BCM43xx_H_

+#include <linux/hw_random.h>
#include <linux/version.h>
#include <linux/kernel.h>
#include <linux/spinlock.h>
@@ -82,6 +83,7 @@
#define BCM43xx_MMIO_TSF_1 0x634 /* core rev < 3 only */
#define BCM43xx_MMIO_TSF_2 0x636 /* core rev < 3 only */
#define BCM43xx_MMIO_TSF_3 0x638 /* core rev < 3 only */
+#define BCM43xx_MMIO_RNG 0x65A
#define BCM43xx_MMIO_POWERUP_DELAY 0x6A8

/* SPROM offsets. */
@@ -741,6 +743,10 @@
const struct firmware *initvals0;
const struct firmware *initvals1;

+ /* Random Number Generator. */
+ struct hwrng rng;
+ char rng_name[20 + 1];
+
/* Debugging stuff follows. */
#ifdef CONFIG_BCM43XX_DEBUG
struct bcm43xx_dfsentry *dfsentry;
Index: hwrng/drivers/net/wireless/bcm43xx/bcm43xx_main.c
===================================================================
--- hwrng.orig/drivers/net/wireless/bcm43xx/bcm43xx_main.c 2006-05-12 12:09:28.000000000 +0200
+++ hwrng/drivers/net/wireless/bcm43xx/bcm43xx_main.c 2006-05-12 12:10:58.000000000 +0200
@@ -3152,6 +3152,39 @@
bcm43xx_clear_keys(bcm);
}

+static int bcm43xx_rng_read(struct hwrng *rng, u32 *data)
+{
+ struct bcm43xx_private *bcm = (struct bcm43xx_private *)rng->priv;
+ unsigned long flags;
+
+ bcm43xx_lock(bcm, flags);
+ *data = bcm43xx_read16(bcm, BCM43xx_MMIO_RNG);
+ bcm43xx_unlock(bcm, flags);
+
+ return (sizeof(u16));
+}
+
+static void bcm43xx_rng_exit(struct bcm43xx_private *bcm)
+{
+ hwrng_unregister(&bcm->rng);
+}
+
+static int bcm43xx_rng_init(struct bcm43xx_private *bcm)
+{
+ int err;
+
+ snprintf(bcm->rng_name, ARRAY_SIZE(bcm->rng_name),
+ "%s_%s", KBUILD_MODNAME, bcm->net_dev->name);
+ bcm->rng.name = bcm->rng_name;
+ bcm->rng.data_read = bcm43xx_rng_read;
+ bcm->rng.priv = (unsigned long)bcm;
+ err = hwrng_register(&bcm->rng);
+ if (err)
+ printk(KERN_ERR PFX "RNG init failed (%d)\n", err);
+
+ return err;
+}
+
/* This is the opposite of bcm43xx_init_board() */
static void bcm43xx_free_board(struct bcm43xx_private *bcm)
{
@@ -3167,6 +3200,7 @@
bcm->shutting_down = 1;
bcm43xx_unlock(bcm, flags);

+ bcm43xx_rng_exit(bcm);
for (i = 0; i < BCM43xx_MAX_80211_CORES; i++) {
if (!bcm->core_80211[i].available)
continue;
@@ -3248,6 +3282,9 @@
bcm43xx_switch_core(bcm, &bcm->core_80211[0]);
bcm43xx_mac_enable(bcm);
}
+ err = bcm43xx_rng_init(bcm);
+ if (err)
+ goto err_80211_unwind;
bcm43xx_macfilter_clear(bcm, BCM43xx_MACFILTER_ASSOC);
bcm43xx_macfilter_set(bcm, BCM43xx_MACFILTER_SELF, (u8 *)(bcm->net_dev->dev_addr));
dprintk(KERN_INFO PFX "80211 cores initialized\n");

--


2006-05-12 15:17:04

by Denis Vlasenko

[permalink] [raw]
Subject: Re: [patch 9/9] Add bcm43xx HW RNG support

On Friday 12 May 2006 13:35, Michael Buesch wrote:
> +static int bcm43xx_rng_read(struct hwrng *rng, u32 *data)
> +{
> + struct bcm43xx_private *bcm = (struct bcm43xx_private *)rng->priv;
> + unsigned long flags;
> +
> + bcm43xx_lock(bcm, flags);
> + *data = bcm43xx_read16(bcm, BCM43xx_MMIO_RNG);

You are storing random 16-bit value _and_ 16 zero bits
into 32-bit memory location. Probably not a problem for
little-endian machine (you return 2, indicating that there
are only 2 bytes of randomness), but on big endian?

Didn't you mean

*(u16*)data = bcm43xx_read16(bcm, BCM43xx_MMIO_RNG); ?

> + bcm43xx_unlock(bcm, flags);
> +
> + return (sizeof(u16));
> +}

--
vda

2006-05-12 15:21:10

by Johannes Berg

[permalink] [raw]
Subject: Re: [patch 9/9] Add bcm43xx HW RNG support

On Fri, 2006-05-12 at 18:16 +0300, Denis Vlasenko wrote:

> Didn't you mean
>
> *(u16*)data = bcm43xx_read16(bcm, BCM43xx_MMIO_RNG); ?
>
> > + bcm43xx_unlock(bcm, flags);
> > +
> > + return (sizeof(u16));
> > +}

I'm not the expert, but looking at patch 2 I'd say no, because one byte
is copied out and then the value is right-shifted, so it is always
treated as a u32 where only 'size' bytes are valid starting with the
lower bytes.

johannes


Attachments:
signature.asc (793.00 B)
This is a digitally signed message part

2006-05-12 20:44:11

by Michael Büsch

[permalink] [raw]
Subject: Re: [patch 9/9] Add bcm43xx HW RNG support

On Friday 12 May 2006 17:16, Denis Vlasenko wrote:
> On Friday 12 May 2006 13:35, Michael Buesch wrote:
> > +static int bcm43xx_rng_read(struct hwrng *rng, u32 *data)
> > +{
> > + struct bcm43xx_private *bcm = (struct bcm43xx_private *)rng->priv;
> > + unsigned long flags;
> > +
> > + bcm43xx_lock(bcm, flags);
> > + *data = bcm43xx_read16(bcm, BCM43xx_MMIO_RNG);
>
> You are storing random 16-bit value _and_ 16 zero bits
> into 32-bit memory location. Probably not a problem for
> little-endian machine (you return 2, indicating that there
> are only 2 bytes of randomness), but on big endian?
>
> Didn't you mean
>
> *(u16*)data = bcm43xx_read16(bcm, BCM43xx_MMIO_RNG); ?


Nope, the code is correct.

--
Greetings Michael.