Skip site navigation (1)Skip section navigation (2)
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
--Signature=_Tue__29_May_2012_16_28_02_+0200_obMajNhDVGJ/mkhy
Content-Type: text/plain; charset=US-ASCII
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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=20
> >>> "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 t=
he
> >>> P-state is acquired.
> >>=20
> >> 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 (an=
d also
> correct) that here is a patch:
>=20
> 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 =3D 0; j < 100; j++){
> +			/* get the result. not assure msr=3Did */
>  			msr =3D rdmsr(MSR_AMD_10H_11H_STATUS);
>  			if(msr =3D=3D id){
>  				break;
>  			}
>  			DELAY(100);
>  		}
> -		/* get the result. not assure msr=3Did */
> -		msr =3D rdmsr(MSR_AMD_10H_11H_STATUS);
>  		HWPSTATE_DEBUG(dev, "result  P%d-state on cpu%d\n",
>  		    (int)msr, PCPU_GET(cpuid));
>  		if (msr !=3D id) {

I can confirm, that this patchs works on a "Bulldozer" CPU and on an old
Phenom II "Deneb".=20

--=20
Homepage:  www.yamagi.org
XMPP:      yamagi@yamagi.org
GnuPG/GPG: 0xEFBCCBCB

--Signature=_Tue__29_May_2012_16_28_02_+0200_obMajNhDVGJ/mkhy
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (FreeBSD)

iEYEARECAAYFAk/E3PkACgkQWTjlg++8y8s/VQCeKNdKv2O2DBlA73w4PASCaFhM
i+QAnjtnUvqSb0raeyLfxcnvouNGqym/
=xbp5
-----END PGP SIGNATURE-----

--Signature=_Tue__29_May_2012_16_28_02_+0200_obMajNhDVGJ/mkhy--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120529162802.9b809969.lists>