Random bit generator (RBG) driver for RPi and RPi2.
ClosedPublic
Actions

Authored by stevek on Jun 18 2016, 10:34 AM.

Details

Reviewers

markm
delphij
adrian

Group Reviewers

ARM
secteam

Commits

rS331889: MFC r303035 by markm:
rS303035: Random bit generator (RBG) driver for RPi and RPi2.

Summary

This driver supports the following methods to trigger gathering random bits from the hardware:

interrupt when the FIFO is full (default) fed into the harvest queue
callout (when BCM2835_RNG_USE_CALLOUT is defined) every second if hz is less than 100, otherwise hz / 100, feeding the random bits into the harvest queue

If the kernel is booted with verbose enabled, the contents of the registers will be dumped after the RBG is started during the attach routine.

Test Plan

Built RPI2 kernel and booted on board. Tested the different methods to feed the harvest queue (callout, interrupt) and the interrupt driven approach seems best. However, keeping the other method for people to be able to experiment with.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

stevek updated this revision to Diff 17681.Jun 18 2016, 10:34 AM

stevek retitled this revision from to Random bit generator (RBG) driver for RPi and RPi2..

stevek updated this object.

stevek edited the test plan for this revision. (Show Details)

stevek added a reviewer: ARM.

stevek set the repository for this revision to rS FreeBSD src repository - subversion.

stevek added subscribers: jtl, sjg.

Herald added subscribers: andrew, imp. · View Herald TranscriptJun 18 2016, 10:34 AM

stevek updated this object.Jun 18 2016, 10:34 AM

stevek added a reviewer: markm.Jun 18 2016, 10:39 AM

markm requested changes to this revision.Jun 18 2016, 11:25 AM

markm edited edge metadata.

markm added inline comments.

sys/arm/broadcom/bcm2835/bcm2835_rng.c
54	This mode looks like it may be a liability? Should the callout/interrupt-using method not be the default?
73	There is a mix of tabs and spaces in these defines giving a ragged indentation. Please check style(9) and use only one form for uniformity.
113	This define looks a little spare here.
127	An inline function gives slightly better type-safety and better readability, not to mention consistency with the functions below.
165	Is this right? The general pattern suggests the constant should be 0x1. Also, can the whole block not be done with some arithmetic, rather than having a bunch of nearly identical constant strings?
247	It might be a good idea to make this tweekable, like a sysctl, instead of a hard constant. A panic is quite harsh, so I agree with the #ifdef defaulting to "off".
256	Yuk :-) A delay here s a no-no. The interrupt/callout method needs to be the default/only method, and this delay needs to go.
426	Can this also be a sysctl?
436	Yuk. I know there is precedent for goto's in the BSD kernel, but I'd prefer not to see any new ones. (Preference not insistence).
sys/arm/broadcom/bcm2835/files.bcm283x
13	"optional random" not "standard" please.

This revision now requires changes to proceed.Jun 18 2016, 11:25 AM

Thanks for this, BTW :-)

markm added a reviewer: delphij.Jun 18 2016, 11:27 AM

stevek added inline comments.Jun 18 2016, 4:50 PM

sys/arm/broadcom/bcm2835/bcm2835_rng.c
54	Yes, this bled in from my testing.
256	Okay, I'll just remove the source registration pieces. Makes sense to just axe them since the RBG is not able to keep up with requests as a pure entropy source.
426	There is a per-device sysctl. See farther below in this function.

I'm not sure what Phabricator is doing with the indentation in bcm2835_rng.c, but it is properly tab indented and the web interface is mutilating the formatting output for the #defines.

The changes in this diff are as follows:
Clean up formatting of some #define(s) to match style(9).
Remove entropy source registration (the RBG cannot keep up, so no sense trying that mode.)
Make the stats incrementing for underruns a static inline function.
Try to detect stalls and reset the device if it happens. If the device stalls twice in one harvest attempt, just disable the device rather than causing potential issues with the rest of the system.
Make the RNG optional, enabling only with the random device is configured.
Add sysctl and tunable for setting stall count.