[stable 9] broken hwpstate calls

[Yamagi Burmeister]

On Sat, 26 May 2012 12:34:25 +0300
Andriy Gapon <avg at FreeBSD.org> 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:      yamagi at yamagi.org
GnuPG/GPG: 0xEFBCCBCB
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-stable/attachments/20120529/f4826c1c/attachment.pgp