Date: Tue, 29 May 2012 16:28:02 +0200 From: Yamagi Burmeister <lists@yamagi.org> To: avg@FreeBSD.org Cc: sbruno@FreeBSD.org, freebsd-stable@FreeBSD.org, seanbru@yahoo-inc.com, jkim@FreeBSD.org Subject: Re: [stable 9] broken hwpstate calls Message-ID: <20120529162802.9b809969.lists@yamagi.org> In-Reply-To: <4FC0A3A1.80200@FreeBSD.org> References: <1337319129.2915.4.camel@powernoodle-l7> <4FB6765A.2050307@FreeBSD.org> <1337710214.2916.8.camel@powernoodle-l7.corp.yahoo.com> <20120525163653.b61a08e2.lists@yamagi.org> <4FBFA9A9.7020806@FreeBSD.org> <4FBFBD39.7000105@FreeBSD.org> <4FBFDFFB.9020501@FreeBSD.org> <4FBFE624.1020208@FreeBSD.org> <20120526090233.f638c1d2.lists@yamagi.org> <4FC0A3A1.80200@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
On Sat, 26 May 2012 12:34:25 +0300
Andriy Gapon <avg@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@yamagi.org
GnuPG/GPG: 0xEFBCCBCB
[-- Attachment #2 --]
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (FreeBSD)
iEYEARECAAYFAk/E3PkACgkQWTjlg++8y8s/VQCeKNdKv2O2DBlA73w4PASCaFhM
i+QAnjtnUvqSb0raeyLfxcnvouNGqym/
=xbp5
-----END PGP SIGNATURE-----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120529162802.9b809969.lists>