I fiddled with this for a while trying to do it as one big asm,
but I couldn't get it to work so I'm going to put this on the back burner.

if anyone else wants to play with it, I've put a copy of the working
multi-asm code in

	ftp://ftp.netbsd.org/pub/NetBSD/misc/chs/mips-spl/

(as usual, pick the most recent diff for best results.)

-Chuck


On Wed, Mar 28, 2001 at 09:41:01AM -0800, Chris G. Demetriou wrote:
> > so assuming I'm not way off base here, the question I have is,
> > do we want spl*() inlined all the time or only optionally?
> 
> I'd say provide them as extern inlines, i.e., also have a single
> out-of-line copy, which will be used if for some reason inlining is
> disabled.  (better than having them scattered all over as would be
> done with static inlines.)  (Also, that may make life a little bit
> better in terms of a sane LKM ABI -- not clear to me that LKMs should
> really be trying to inline things like this anyway...)
> 
> 
> some comments on your asms:
> 
> (1) you probably should be marking them volatile.
> 
> (2) it may make sense to do them multiline, rather than lots of asms
> one right after the other.
> 
> > +static inline int
> > +_splraise(int newipl)
> > +{
> > +	int streg;
> > +
> > +	__asm__("mfc0 %0, $12" : "=r" (streg));
> > +	newipl &= MIPS_INT_MASK;
> > +	__asm__("mtc0 %0, $12" : : "r"	(_splmask(streg & ~newipl)));
> > +	__asm__("nop");
> > +	return streg & (MIPS_INT_MASK | MIPS_SR_INT_IE);
> > +}
> > +
> > +static inline int
> > +_spllower(int newipl)
> > +{
> > +	int streg;
> > +
> > +	__asm__("mfc0 %0, $12" : "=r" (streg));
> > +	__asm__("mtc0 %0, $12" : : "r" (_splmask((streg & ~MIPS_INT_MASK) |
> > +						 (~newipl & MIPS_INT_MASK))));
> 
> I think you probably have to be careful about hazards in places like
> this.  in particular, as i recall, on non-interlocked cpus there's a
> one cycle hazard before the result register becomes valid ... and who
> knows what the compiler's going to compile this into.
> 
> It's really probably best to translate the existing assembly directly
> into single large asm statements, which use the necessary variables
> and clobbers.
> 
> Also, there's no reason to encode the CP0 regs as hard-coded numbers
> in the assembly.  you could, for instance, use:
> 
>     __asm__ ("mtc0 %0, $%1" : : "r"(val), "i"(CP0_REGNUM));
> 
> of course, the larger ASMs get the more confusing this becomes, but on
> the other hand you're typically not whacking more than one or two cp0
> regs in a single asm...
> 
> 
> > +static inline void
> > +_setsoftintr(int setbits)
> > +{
> > +	int streg, causereg;
> > +
> > +	__asm__("mfc0 %0, $12" : "=r" (streg));
> > +	__asm__("mtc0 $0, $12");
> > +	__asm__("nop; nop");
> > +	__asm__("mfc0 %0, $13" : "=r" (causereg));
> > +	__asm__("mtc0 %0, $13" : : "r" (causereg | setbits));
> 
> i think you have a hazard issue here too.
> 
> in general, there's lots of potential for hazards in code like this.
> The two i pointed out are the two that i noticed, there are more.  (at
> least one that i noticed while writing this paragraph, that i'm not
> going to bother to point out.  You should just turn the assembly code
> into C asms.)
> 
> 
> 
> cgd