Date: Fri, 20 Oct 1995 14:05:36 +1000 From: Bruce Evans <bde@zeta.org.au> To: bde@zeta.org.au, mark@grondar.za Cc: hackers@FreeBSD.org Subject: Re: Creating a /dev/random Message-ID: <199510200405.OAA14265@godzilla.zeta.org.au>
next in thread | raw e-mail | index | archive | help
>Here is how far I have gotten with creating a /dev/random. >Please have a look at the following patches, and let me know what you >think. >... > o I thought it best to place my interrupt hooks where I did, instead > of where you suggested, as I decided I wanted to be as close to > the interrupt code as I could get. Also the `unit' you suggested I > use to fiddle the irq passing would not work - it did not have the > right range of values (it would typically obly be 0, wth the occaisional > 1 or 2) My method avoids extra overhead for the interrupts that aren't hooked. I said to replace the `unit' number by the interrupt number in intr_unit[] and load the original unit number in the dispatch function before calling the interrupt handler. > o I have yet to write some kind of IOCTL to set this interrupt selector. > (Would an ioctl be the right way? Is there some precedent I could > follow? Yes. spkr.c: spkrioctl() would be a good example if it was written right. Correct the following stylistic and real bugs in it to get a good example: 1) the formatting is nonstandard. 2) after it has decided that it doesn't handle minor(dev), it should return ENODEV, not ENXIO. You will have to add a memioctl() function and return ENODEV from it for all except the new /dev/random minor. 3) it should use switch(cmd) instead of a bunch of `if (cmd == ...)' statements. 4) the copyin() of the data is bogus and may fail. In BSD, the size of the data is encoded in the command value and the copyin() is done at a higher level. The copyin() of the driver does a copy of kernel data to kernel data and may fail if copyin() only works for user to kernel copies. The i386 copyin() probably works for kernel to kernel copies too. 5) after it has decided that it doesn't handle the command, it should return ENOTTY, not EINVAl. EINVAL is for appropriate for invalid data in a supported command. The interface could use a bitmap of the interrupts to select, but this would be unportable (see recent mail about select()) and speed isn't important (unlike for select()). It should probably use the number of one interrupt, a select command and a deselect command. >diff -udr --exclude=compile sys.ORG/i386/conf/files.i386 sys/i386/conf/files.i386 > ... >+i386/isa/random.c standard Should be optional. Perhaps `optional random device-driver'. >diff -udr --exclude=compile sys.ORG/i386/i386/machdep.c sys/i386/i386/machdep.c >--- sys.ORG/i386/i386/machdep.c Thu Oct 12 12:34:03 1995 >+++ sys/i386/i386/machdep.c Thu Oct 19 20:44:13 1995 >@@ -126,6 +126,7 @@ > #include <i386/isa/isa.h> > #include <i386/isa/isa_device.h> > #include <i386/isa/rtc.h> >+#include <i386/isa/random.h> Does it really have isa dependencies? The other isa includes in machdep.c are bogus too. >diff -udr --exclude=compile sys.ORG/i386/isa/vector.s sys/i386/isa/vector.s >... >@@ -195,6 +205,8 @@ > outb %al,$icu+1 ; \ > sti ; /* XXX _doreti repeats the cli/sti */ \ > MEXITCOUNT ; \ >+ /* Add this interrupt to the pool of entropy */ \ >+ ADDENTROPY(irq_num) ; \ > /* We could usually avoid the following jmp by inlining some of */ \ > /* _doreti, but it's probably better to use less cache. */ \ > jmp _doreti ; \ The addition should be before the MEXITCOUNT for correct profiling (in my version of profiling, MEXITCOUNT is non-null and must be placed immediately before all jmps). It should probably be even earlier, immediately after the call to the interrupt handler, so that it is called while interrupts for the device are still masked in the ICU. Placing it later doesn't improve latency, because device interrupts are still masked in software. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199510200405.OAA14265>