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

[Segher Boessenkool <segher@kernel.crashing.org>]

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


-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
kvm-ppc-devel mailing list
kvm-ppc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-ppc-devel