Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 20 Oct 1995 08:43:10 +0200
From:      Mark Murray <mark@grondar.za>
To:        Bruce Evans <bde@zeta.org.au>
Cc:        hackers@FreeBSD.org
Subject:   Re: Creating a /dev/random 
Message-ID:  <199510200643.IAA01491@grumble.grondar.za>

next in thread | raw e-mail | index | archive | help
> > 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.

There is a problem with this. The interrupt handler is called with a unit
number, which is non-unique. There is no way to get back to the original
interrupt handler.

(you suggested:)
> I think you will need to hook selected interrupt handlers to get dynamic
> interrupt information.  INTR() is too static.  The following would almost
> work:
> 
> void init_interrupt_randomness(void) {
> 	for (each irq of interest) {
>		prev_intr_handler[irq] = intr_handler[irq];
>		prev_intr_unit[irq] = intr_unit[irq];
>		disable_intr();
>		intr_handler[irq] = add_interrupt_randomness;
>		intr_unit[irq] = irq;
> 		enable_intr();
>	 }
> }
> 
> void add_interrupt_randomness(int irq) { <-- units, not irq's
> 	static u_char busy;
> 
> 	disable_intr();
> 	if (busy)
> 		enable_intr();
> 	else {
> 		busy = TRUE;
> 		enable_intr();
> 		do_things(irq);
> 		busy = FALSE;
> 	}
> 	(*prev_intr_handler[irq])(prev_intr_unit[irq]); <=== won't work
                                                         as units are mostly
                                                         small numbers
> }

> > 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:

Great! Thanks..

[I like the example - "except for this list of bugs, this is the route
to follow" ;-)]

> >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'.

I would rather make it a permanent fixture. When random.c is compiled
the object file is about 5k, not much for the bloatists to complain
about ;-). If software developers like Netscrape could count on
this device being present, then they would use it. If it is optional,
it may as well be not there. These are Ts'o's sentiments as well.
Right now there are #ifdef DEVRANDOM constructs around the code,
which I would like to remove when it is stable.

> >diff -udr --exclude=compile sys.ORG/i386/i386/machdep.c sys/i386/i386/machde
p.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.

is this the wrong place to put this header? It is a short file containing
necessary function prototypes.

> >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.

Will do.

M
--
Mark Murray
46 Harvey Rd, Claremont, Cape Town 7700, South Africa
+27 21 61-3768 GMT+0200
Finger mark@grumble.grondar.za for PGP key



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199510200643.IAA01491>