[kvm-ppc-devel] [PATCH] [v2] Add idle wait support for 44x platforms

Segher Boessenkool segher at kernel.crashing.org
Sat Apr 5 05:47:04 EST 2008


>> +	msr_save = mfmsr();
>> +	/* set wait state MSR */
>> +	mtmsr(msr_save|MSR_WE|MSR_EE|MSR_CE);
>
> Did we decide to drop MSR_DE?
>
>> +	/* return to initial state */
>> +	mtmsr(msr_save);
>
> It may be my paranoia but I'm pretty sure you need the isync() after
> _both_ mtmsr()s
> Certainly can't hurt.

It hurts because it spreads the paranoia.  Cargo cult.

Please add a comment that explains _why_ you need a context 
synchronising
instruction here.  The POWER ISA 2.05 says that the requirements for
changing the WE bit are implementation dependent, both before and after
the mtmsr, so the safe thing to do would be to use the "big hammer" 
isync
both before and after each of these mtmsrs.  A sync before entering wait
state might be needed as well, dunno.  Either way, comment please :-)


Segher




More information about the Linuxppc-dev mailing list