On Sat, 26 May 2012 12:34:25 +0300 Andriy Gapon <[email protected]> wrote:
> >>> if we decide so, then I think that we could still keep the things
> >>> "simple". As we currently use the "wholesale" approach (all CPUs are
> >>> set to the same P-state regardless of topology), then we could first
> >>> make a pass of writing the MSR on all processors with a new P-state
> >>> value and then make another pass of checking via MSR C001_0063 that the
> >>> P-state is acquired.
> >>
> >> No, I believe checking MSRC001_0071[18:16] is much simpler if it works.
> >> And it does not break current cpufreq(4) design principles.
One potential problem with MSRC001_0071[18:16] is, that it's on older
CPUs supported by hwpstate it's the same as C001_0063. Only on newer
models with "turbo" it containts the actual "hardware p-state". So
additional logic would be required:
1. Set new p-state
2. Check CPUID for support of "hardware p-states"
3.1 If yes, read MSRC001_0071[18:16] and convert the "hardware p-state"
into a "software p-state"
3.2 If not, just read MSRC001_0071[18:16]
4. Compare read (and converted) p-state to the requested p-state
I don't think that it's worth this additional efford. The solution
suggest by Andriy Gapon is trivial, works fine and is supported by
all CPUs supported by hwpstate.
> I believe the approach that I suggested to be so trivial to implement (and
> also
> correct) that here is a patch:
>
> diff --git a/sys/x86/cpufreq/hwpstate.c b/sys/x86/cpufreq/hwpstate.c
> index 40e1943..9c17a41 100644
> --- a/sys/x86/cpufreq/hwpstate.c
> +++ b/sys/x86/cpufreq/hwpstate.c
> @@ -186,16 +186,21 @@ hwpstate_goto_pstate(device_t dev, int pstate)
> id, PCPU_GET(cpuid));
> /* Go To Px-state */
> wrmsr(MSR_AMD_10H_11H_CONTROL, id);
> + }
> + CPU_FOREACH(i) {
> + /* Bind to each cpu. */
> + thread_lock(curthread);
> + sched_bind(curthread, i);
> + thread_unlock(curthread);
> /* wait loop (100*100 usec is enough ?) */
> for(j = 0; j < 100; j++){
> + /* get the result. not assure msr=id */
> msr = rdmsr(MSR_AMD_10H_11H_STATUS);
> if(msr == id){
> break;
> }
> DELAY(100);
> }
> - /* get the result. not assure msr=id */
> - msr = rdmsr(MSR_AMD_10H_11H_STATUS);
> HWPSTATE_DEBUG(dev, "result P%d-state on cpu%d\n",
> (int)msr, PCPU_GET(cpuid));
> if (msr != id) {
I can confirm, that this patchs works on a "Bulldozer" CPU and on an old
Phenom II "Deneb".
--
Homepage: www.yamagi.org
XMPP: [email protected]
GnuPG/GPG: 0xEFBCCBCB
pgpt74TfFnBSR.pgp
Description: PGP signature
