Skip to contentSkip to conversation

New Generic HW RNG (#2)

PATCH In review Michael Buesch 3 participants 13 messages May 7, 2006

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

cover New Generic HW RNG (#2)

Second try. Various fixes included. This does even compile (and work) now. :)

This patch series replaces the old non-generic Hardware Random Number Generator support by a fully generic RNG API.

This makes it possible to register additional RNGs from modules. With this patch series applied, Laptops with a bcm43xx chip (PowerBook) have a HW RNG available now.

Additionally two new RNG drivers are added for the "ixp4xx" and "omap" devices. (Written by Deepak Saxena). This patch series includes the old patches by Deepak Saxena.

The x86-rng driver is the old RNG driver ported to the new API (and cleaned up a bit).

The userspace RNG daemon can later be updated to select the RNG through /sys/class/misc/hw_random/ for convenience. For now it is sufficient to use cat and echo -n on the sysfs attributes.

Please apply to -mm for testing.

#1 lore raw

PATCH 1/6 New Generic HW RNG (#2)

Remove the old H/W RNG support.

drivers/char/Kconfig +0 -15
Index: hwrng/drivers/char/Kconfig
===================================================================
@@ -670,21 +670,6 @@
 
 	  If you're not sure, say N.
 
-config HW_RANDOM
-	tristate "Intel/AMD/VIA HW Random Number Generator support"
-	depends on (X86 || IA64) && PCI
-	---help---
-	  This driver provides kernel-side support for the Random Number
-	  Generator hardware found on Intel i8xx-based motherboards,
-	  AMD 76x-based motherboards, and Via Nehemiah CPUs.
-
-	  Provides a character driver, used to read() entropy data.
-
-	  To compile this driver as a module, choose M here: the
-	  module will be called hw_random.
-
-	  If unsure, say N.
-
 config NVRAM
 	tristate "/dev/nvram support"
 	depends on ATARI || X86 || ARM || GENERIC_NVRAM
drivers/char/Makefile +0 -1
Index: hwrng/drivers/char/Makefile
===================================================================
@@ -75,7 +75,6 @@
 obj-$(CONFIG_TOSHIBA)		+= toshiba.o
 obj-$(CONFIG_I8K)		+= i8k.o
 obj-$(CONFIG_DS1620)		+= ds1620.o
-obj-$(CONFIG_HW_RANDOM)		+= hw_random.o
 obj-$(CONFIG_FTAPE)		+= ftape/
 obj-$(CONFIG_COBALT_LCD)	+= lcd.o
 obj-$(CONFIG_PPDEV)		+= ppdev.o
drivers/char/hw_random.c deleted +0 -698

698 lines not on this page · raw message

--

#2 lore raw

PATCH 2/6 New Generic HW RNG (#2)

In reply to Michael Buesch

Add a new generic H/W RNG core.

drivers/char/Kconfig +2 -0
Index: hwrng/drivers/char/Kconfig
===================================================================
@@ -670,6 +670,8 @@
 
 	  If you're not sure, say N.
 
+source "drivers/char/hw_random/Kconfig"
+
 config NVRAM
 	tristate "/dev/nvram support"
 	depends on ATARI || X86 || ARM || GENERIC_NVRAM
drivers/char/Makefile +1 -0
Index: hwrng/drivers/char/Makefile
===================================================================
@@ -75,6 +75,7 @@
 obj-$(CONFIG_TOSHIBA)		+= toshiba.o
 obj-$(CONFIG_I8K)		+= i8k.o
 obj-$(CONFIG_DS1620)		+= ds1620.o
+obj-$(CONFIG_HW_RANDOM)		+= hw_random/
 obj-$(CONFIG_FTAPE)		+= ftape/
 obj-$(CONFIG_COBALT_LCD)	+= lcd.o
 obj-$(CONFIG_PPDEV)		+= ppdev.o
drivers/char/hw_random/Kconfig new file +11 -0
Index: hwrng/drivers/char/hw_random/Kconfig
===================================================================
@@ -0,0 +1,11 @@
+#
+# Hardware Random Number Generator (RNG) configuration
+#
+
+config HW_RANDOM
+	bool "Hardware Random Number Generator Core support"
+	default y
+	---help---
+	  Hardware Random Number Generator Core infrastructure.
+
+	  If unsure, say Y.
drivers/char/hw_random/Makefile new file +5 -0
Index: hwrng/drivers/char/hw_random/Makefile
===================================================================
@@ -0,0 +1,5 @@
+#
+# Makefile for HW Random Number Generator (RNG) device drivers.
+#
+
+obj-$(CONFIG_HW_RANDOM) += core.o
drivers/char/hw_random/core.c new file +337 -0

337 lines not on this page · raw message

include/linux/hw_random.h new file +50 -0

50 lines not on this page · raw message

--

#3 lore raw

PATCH 3/6 New Generic HW RNG (#2)

In reply to Michael Buesch

Add a driver for the x86 RNG.
This driver is ported from the old hw_random.c

drivers/char/hw_random/Kconfig +13 -0
Index: hwrng/drivers/char/hw_random/Kconfig
===================================================================
@@ -9,3 +9,16 @@
 	  Hardware Random Number Generator Core infrastructure.
 
 	  If unsure, say Y.
+
+config X86_RNG
+	tristate "Intel/AMD/VIA HW Random Number Generator support"
+	depends on HW_RANDOM && (X86 || IA64) && PCI
+	---help---
+	  This driver provides kernel-side support for the Random Number
+	  Generator hardware found on Intel i8xx-based motherboards,
+	  AMD 76x-based motherboards, and Via Nehemiah CPUs.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called x86-rng.
+
+	  If unsure, say Y.
drivers/char/hw_random/Makefile +1 -0
Index: hwrng/drivers/char/hw_random/Makefile
===================================================================
@@ -3,3 +3,4 @@
 #
 
 obj-$(CONFIG_HW_RANDOM) += core.o
+obj-$(CONFIG_X86_RNG) += x86-rng.o
drivers/char/hw_random/x86-rng.c new file +586 -0

586 lines not on this page · raw message

--

#4 lore raw
Quoted 3 lines · Michael Buesch

Add a driver for the x86 RNG.
This driver is ported from the old hw_random.c

[skip]

Quoted 1 line

+static int __init intel_init(struct hwrng *rng)

Cannot be __init anymore - now rng->init could be called at any time.

Also, there is another problem with putting this function into rng->init - if another RNG has been registered when this module is loaded, ->init will not be called during hwrng_register(), so the module load will succeed even if the chipset does not have RNG hardware.

Quoted 42 lines
drivers/char/hw_random/x86-rng.c new file +42 -0
+{
+	void __iomem *rng_mem;
+	int rc;
+	u8 hw_status;
+
+	DPRINTK ("ENTER\n");
+
+	rng_mem = ioremap (INTEL_RNG_ADDR, INTEL_RNG_ADDR_LEN);
+	if (rng_mem == NULL) {
+		printk (KERN_ERR PFX "cannot ioremap RNG Memory\n");
+		rc = -EBUSY;
+		goto err_out;
+	}
+	rng->priv = (unsigned long)rng_mem;
+
+	/* Check for Intel 82802 */
+	hw_status = intel_hwstatus (rng_mem);
+	if ((hw_status & INTEL_RNG_PRESENT) == 0) {
+		printk (KERN_ERR PFX "RNG not detected\n");
+		rc = -ENODEV;
+		goto err_out_free_map;
+	}
+
+	/* turn RNG h/w on, if it's off */
+	if ((hw_status & INTEL_RNG_ENABLED) == 0)
+		hw_status = intel_hwstatus_set (rng_mem, hw_status | INTEL_RNG_ENABLED);
+	if ((hw_status & INTEL_RNG_ENABLED) == 0) {
+		printk (KERN_ERR PFX "cannot enable RNG, aborting\n");
+		rc = -EIO;
+		goto err_out_free_map;
+	}
+
+	DPRINTK ("EXIT, returning 0\n");
+	return 0;
+
+err_out_free_map:
+	iounmap (rng_mem);
+err_out:
+	DPRINTK ("EXIT, returning %d\n", rc);
+	return rc;
+}
+

[skip]

Quoted 1 line

+static int __init amd_init(struct hwrng *rng)

Again, __init is wrong.

[skip]

Quoted 1 line

+static int __init via_init(struct hwrng *rng)

This __init is wrong too.

[skip]

#5 lore raw
Quoted 8 lines · you
Quoted 3 lines · Michael Buesch

Add a driver for the x86 RNG.
This driver is ported from the old hw_random.c

[skip]

Quoted 1 line

+static int __init intel_init(struct hwrng *rng)

Cannot be __init anymore - now rng->init could be called at any time.

Sure, will fix this.

Quoted 5 lines

Also, there is another problem with putting this function into rng->init - if another RNG has been registered when this module is loaded, ->init will not be called during hwrng_register(), so the module load will succeed even if the chipset does not have RNG hardware.

Ok, I see. The question is, are we going to hwrng_register() the intel, althought there is no device? We check for the PCI IDs. Anyway. This should be fixed.
And I think this is also a good time to split the x86 driver, so all this init-stuff is much cleaner.

Quoted 45 lines
Quoted 42 lines
drivers/char/hw_random/x86-rng.c new file +42 -0
+{
+	void __iomem *rng_mem;
+	int rc;
+	u8 hw_status;
+
+	DPRINTK ("ENTER\n");
+
+	rng_mem = ioremap (INTEL_RNG_ADDR, INTEL_RNG_ADDR_LEN);
+	if (rng_mem == NULL) {
+		printk (KERN_ERR PFX "cannot ioremap RNG Memory\n");
+		rc = -EBUSY;
+		goto err_out;
+	}
+	rng->priv = (unsigned long)rng_mem;
+
+	/* Check for Intel 82802 */
+	hw_status = intel_hwstatus (rng_mem);
+	if ((hw_status & INTEL_RNG_PRESENT) == 0) {
+		printk (KERN_ERR PFX "RNG not detected\n");
+		rc = -ENODEV;
+		goto err_out_free_map;
+	}
+
+	/* turn RNG h/w on, if it's off */
+	if ((hw_status & INTEL_RNG_ENABLED) == 0)
+		hw_status = intel_hwstatus_set (rng_mem, hw_status | INTEL_RNG_ENABLED);
+	if ((hw_status & INTEL_RNG_ENABLED) == 0) {
+		printk (KERN_ERR PFX "cannot enable RNG, aborting\n");
+		rc = -EIO;
+		goto err_out_free_map;
+	}
+
+	DPRINTK ("EXIT, returning 0\n");
+	return 0;
+
+err_out_free_map:
+	iounmap (rng_mem);
+err_out:
+	DPRINTK ("EXIT, returning %d\n", rc);
+	return rc;
+}
+

[skip]

Quoted 1 line

+static int __init amd_init(struct hwrng *rng)

Quoted 9 lines

Again, __init is wrong.

[skip]

Quoted 1 line

+static int __init via_init(struct hwrng *rng)

This __init is wrong too.

[skip]

Ah, and I found another bug in hwrng_unregister:

	current_rng = list_entry(rng_list.prev, struct hwrng, list);

current_rng->init() should be called here (if nonNULL). If that fails current_rng = NULL;

Will fix that, too.

Signature · 1 line
Greetings Michael.
#6 lore raw
Quoted 12 lines · Michael Buesch
Quoted 8 lines · you
Quoted 3 lines · Michael Buesch

Add a driver for the x86 RNG.
This driver is ported from the old hw_random.c

[skip]

Quoted 1 line

+static int __init intel_init(struct hwrng *rng)

Cannot be __init anymore - now rng->init could be called at any time.

Sure, will fix this.

Quoted 8 lines
Quoted 5 lines

Also, there is another problem with putting this function into rng->init - if another RNG has been registered when this module is loaded, ->init will not be called during hwrng_register(), so the module load will succeed even if the chipset does not have RNG hardware.

Ok, I see. The question is, are we going to hwrng_register() the intel, althought there is no device? We check for the PCI IDs.

Most Intel chipset do not really have the hardware RNG - PCI ID matches, but the check for INTEL_RNG_PRESENT bit in intel_init() fails. (In fact, I have not ever seen a board which had that RNG.)

[skip]

Quoted 4 lines

Ah, and I found another bug in hwrng_unregister:

	current_rng = list_entry(rng_list.prev, struct hwrng, list);

current_rng->init() should be called here (if nonNULL). If that fails current_rng = NULL;

All that logic in hwrng_register() and hwrng_unregister() looks overly complex. Maybe we should just register the miscdevice unconditionally, and make it return -ENODEV from open() if no RNG is registered?

#7 lore raw

PATCH 4/6 New Generic HW RNG (#2)

In reply to Michael Buesch

Add ixp4xx H/W RNG driver.
Driver written by Deepak Saxena.

drivers/char/hw_random/Kconfig +12 -0
Index: hwrng/drivers/char/hw_random/Kconfig
===================================================================
@@ -22,3 +22,15 @@
 	  module will be called x86-rng.
 
 	  If unsure, say Y.
+
+config IXP4XX_RNG
+	tristate "Intel IXP4xx NPU HW Random Number Generator support"
+	depends on HW_RANDOM && ARCH_IXP4XX
+	---help---
+	  This driver provides kernel-side support for the Random
+	  Number Generator hardware found on the Intel IXP4xx NPU.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ixp4xx-rng.
+
+	  If unsure, say Y.
drivers/char/hw_random/Makefile +1 -0
Index: hwrng/drivers/char/hw_random/Makefile
===================================================================
@@ -4,3 +4,4 @@
 
 obj-$(CONFIG_HW_RANDOM) += core.o
 obj-$(CONFIG_X86_RNG) += x86-rng.o
+obj-$(CONFIG_IXP4XX_RNG) += ixp4xx-rng.o
drivers/char/hw_random/ixp4xx-rng.c new file +73 -0

73 lines not on this page · raw message

--

#8 lore raw

PATCH 5/6 New Generic HW RNG (#2)

In reply to Michael Buesch

Add H/W RNG driver for TI OMAP CPU family.
Driver written by Deepak Saxena.

drivers/char/hw_random/Kconfig +13 -0
Index: hwrng/drivers/char/hw_random/Kconfig
===================================================================
@@ -34,3 +34,16 @@
 	  module will be called ixp4xx-rng.
 
 	  If unsure, say Y.
+
+config OMAP_RNG
+	tristate "OMAP Random Number Generator support"
+	depends on HW_RANDOM && (ARCH_OMAP16XX || ARCH_OMAP24XX)
+ 	---help---
+ 	  This driver provides kernel-side support for the Random Number
+	  Generator hardware found on OMAP16xx and OMAP24xx multimedia
+	  processors.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called omap-rng.
+
+ 	  If unsure, say Y.
drivers/char/hw_random/Makefile +1 -0
Index: hwrng/drivers/char/hw_random/Makefile
===================================================================
@@ -5,3 +5,4 @@
 obj-$(CONFIG_HW_RANDOM) += core.o
 obj-$(CONFIG_X86_RNG) += x86-rng.o
 obj-$(CONFIG_IXP4XX_RNG) += ixp4xx-rng.o
+obj-$(CONFIG_OMAP_RNG) += omap-rng.o
drivers/char/hw_random/omap-rng.c new file +208 -0

208 lines not on this page · raw message

--

#9 lore raw

PATCH 6/6 New Generic HW RNG (#2)

In reply to Michael Buesch

Add bcm43xx H/W RNG support.

drivers/net/wireless/bcm43xx/Kconfig +1 -0
Index: hwrng/drivers/net/wireless/bcm43xx/Kconfig
===================================================================
@@ -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.
drivers/net/wireless/bcm43xx/bcm43xx.h +6 -0
Index: hwrng/drivers/net/wireless/bcm43xx/bcm43xx.h
===================================================================
@@ -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;
drivers/net/wireless/bcm43xx/bcm43xx_main.c +37 -0
Index: hwrng/drivers/net/wireless/bcm43xx/bcm43xx_main.c
===================================================================
@@ -3144,6 +3144,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)
 {
@@ -3159,6 +3192,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;
@@ -3240,6 +3274,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");

--

maintainer
#10 lore raw
In reply to Michael Buesch
Quoted 2 lines · Michael Buesch

Second try. Various fixes included. This does even compile (and work) now. :)

It would be good to give the patches more descriptive names, currently they all have the same subject lines, which is not really helpful.

Quoted 3 lines

The userspace RNG daemon can later be updated to select the RNG through /sys/class/misc/hw_random/ for convenience. For now it is sufficient to use cat and echo -n on the sysfs attributes.

When you're making the behaviour of hw_random configurable, maybe you could also add an option to automatically use the current hw_random driver to feed the /dev/random entropy pool on systems where the administrator trusts its randomness.

	Arnd <><
#11 lore raw
Quoted 6 lines · you
Quoted 2 lines · Michael Buesch

Second try. Various fixes included. This does even compile (and work) now. :)

It would be good to give the patches more descriptive names, currently they all have the same subject lines, which is not really helpful.

How to do this with quilt?

Quoted 8 lines
Quoted 3 lines

The userspace RNG daemon can later be updated to select the RNG through /sys/class/misc/hw_random/ for convenience. For now it is sufficient to use cat and echo -n on the sysfs attributes.

When you're making the behaviour of hw_random configurable, maybe you could also add an option to automatically use the current hw_random driver to feed the /dev/random entropy pool on systems where the administrator trusts its randomness.

This has been discussed and it is not desired behaviour. If you want to feed /dev/random, use rngd to read from /dev/hwrng, validate the data and put it into /dev/random.

Signature · 1 line
Greetings Michael.
maintainer
#12 lore raw
Quoted 5 lines · Michael Buesch
Quoted 3 lines

It would be good to give the patches more descriptive names, currently they all have the same subject lines, which is not really helpful.

How to do this with quilt?

Change the first line in each patch header to

Subject: <what this patch does>

Quoted 3 lines

This has been discussed and it is not desired behaviour. If you want to feed /dev/random, use rngd to read from /dev/hwrng, validate the data and put it into /dev/random.

ok

	Arnd <><