From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Bader Subject: Re: [PATCH] ACPI: Add fixups for AMD P-state figures. Date: Thu, 07 Mar 2013 09:45:51 +0100 Message-ID: <513853BF.4030502@canonical.com> References: <1362512728-28770-1-git-send-email-konrad.wilk@oracle.com> <51365401.4050205@oracle.com> <20130305213319.GA8235@phenom.dumpdata.com> <513710D1.2010803@canonical.com> <20130306155112.GA13118@phenom.dumpdata.com> <20130306213759.GA10587@phenom.dumpdata.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6794636740631495780==" Return-path: In-Reply-To: <20130306213759.GA10587@phenom.dumpdata.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Konrad Rzeszutek Wilk Cc: Boris Ostrovsky , xen-devel@lists.xensource.com, bp@suse.de List-Id: xen-devel@lists.xenproject.org This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --===============6794636740631495780== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="------------enig5073EBB442BE6212491A2E01" This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig5073EBB442BE6212491A2E01 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 06.03.2013 22:37, Konrad Rzeszutek Wilk wrote: > On Wed, Mar 06, 2013 at 10:51:12AM -0500, Konrad Rzeszutek Wilk wrote: >> On Wed, Mar 06, 2013 at 10:48:01AM +0100, Stefan Bader wrote: >>> On 05.03.2013 22:33, Konrad Rzeszutek Wilk wrote: >>>> On Tue, Mar 05, 2013 at 03:22:25PM -0500, Boris Ostrovsky wrote: >>>>> On 03/05/2013 02:45 PM, Konrad Rzeszutek Wilk wrote: >>>>>> This a copy-n-paste from two Linux git commits: >>>>>> >>>>>> - f594065faf4f9067c2283a34619fc0714e79a98d >>>>>> ACPI: Add fixups for AMD P-state figures >>>>>> - 9855d8ce41a7801548a05d844db2f46c3e810166 >>>>>> ACPI: Check MSR valid bit before using P-state frequencies >>>>>> >>>>>> The issue is that "some AMD systems may round the frequencies in >>>>>> ACPI tables to 100MHz boundaries. We canobtain the real >>>>>> frequencies from MSRs, so add a quirk to fix these frequencies up >>>>>> on AMD systems." (from f594065..) >>>>>> >>>>>> In discussion (around 9855d8..) "it turned out that indeed real >>>>>> HW/BIOSes may choose to not set the valid bit and thus mark the >>>>>> P-state as invalid. So this could be considered a fix for broken >>>>>> BIOSes that also works around the issue on Xen." (from 9855d8..) >>>>>> >>>>>> I've tested it under Dell Inc. PowerEdge T105 /0RR825, BIOS 1.3.2 >>>>>> 08/20/2008 where this quirk can indeed be observed. >>>>>> >>>>>> CC: stefan.bader@canonical.com >>>>>> CC: bp@suse.de >>>>>> CC: borislav.ostrovsky@oracle.com >>>>> >>>>> boris.ostrovsky@oracle.com >>>> >>>> Whoops! >>>> >>>> Here is an updated version: >=20 >=20 > From b704d4419e9e483922382d2339bd41c245f435e1 Mon Sep 17 00:00:00 2001 > From: Konrad Rzeszutek Wilk > Date: Tue, 5 Mar 2013 14:40:52 -0500 > Subject: [PATCH] ACPI: Add fixups for AMD P-state figures. >=20 > In the Linux kernel, these two git commits: >=20 > - f594065faf4f9067c2283a34619fc0714e79a98d > ACPI: Add fixups for AMD P-state figures > - 9855d8ce41a7801548a05d844db2f46c3e810166 > ACPI: Check MSR valid bit before using P-state frequencies >=20 > Try to fix the the issue that "some AMD systems may round the > frequencies in ACPI tables to 100MHz boundaries. We can obtain the real= > frequencies from MSRs, so add a quirk to fix these frequencies up > on AMD systems." (from f594065..) >=20 > In discussion (around 9855d8..) "it turned out that indeed real > HW/BIOSes may choose to not set the valid bit and thus mark the > P-state as invalid. So this could be considered a fix for broken > BIOSes." (from 9855d8..) >=20 > which is great for Linux. Unfortunatly the Linux kernel, when > it tries to do the RDMSR under Xen it fails to get the right > value (it gets zero) as Xen traps it and returns zero. Hence > when dom0 uploads the P-states they will be unmodified and > we should take care of updating the frequencies with the right > values. >=20 > I've tested it under Dell Inc. PowerEdge T105 /0RR825, BIOS 1.3.2 > 08/20/2008 where this quirk can be observed (x86 =3D=3D 0x10, model =3D= =3D 2). > Also on other AMD (x86 =3D=3D 0x12, A8-3850; x86 =3D 0x14, AMD E-350) t= o > make sure the quirk is not applied there. >=20 > CC: stefan.bader@canonical.com > CC: bp@suse.de > CC: boris.ostrovsky@oracle.com > [v1: Indent, #define, and email changes per Boris's review] > [v2: Redid the x86+model check, updated description] > Signed-off-by: Konrad Rzeszutek Wilk > --- > xen/arch/x86/acpi/cpufreq/powernow.c | 36 ++++++++++++++++++++++++++++= ++++++++ > 1 file changed, 36 insertions(+) >=20 > diff --git a/xen/arch/x86/acpi/cpufreq/powernow.c b/xen/arch/x86/acpi/c= pufreq/powernow.c > index a9b7792..8c96fe0 100644 > --- a/xen/arch/x86/acpi/cpufreq/powernow.c > +++ b/xen/arch/x86/acpi/cpufreq/powernow.c > @@ -147,6 +147,40 @@ static int powernow_cpufreq_target(struct cpufreq_= policy *policy, > return 0; > } > =20 > +static void amd_fixup_frequency(struct xen_processor_px *px) > +{ > + u32 hi, lo, fid, did; > + int index =3D px->control & 0x00000007; > + > + if (!(boot_cpu_data.x86 =3D=3D 0x10 && boot_cpu_data.x86_model < 1= 0) && > + !(boot_cpu_data.x86 =3D=3D 0x11)) > + return; > + > + rdmsr(MSR_PSTATE_DEF_BASE + index, lo, hi); > + /* > + * MSR C001_0064+: > + * Bit 63: PstateEn. Read-write. If set, the P-state is valid. > + */ > + if (!(hi & (1UL << 31))) > + return; > + > + fid =3D lo & 0x3f; > + did =3D (lo >> 6) & 7; > + if (boot_cpu_data.x86 =3D=3D 0x10) > + px->core_frequency =3D (100 * (fid + 16)) >> did; > + else > + px->core_frequency =3D (100 * (fid + 8)) >> did; > +} > + > +static void amd_fixup_freq(struct processor_performance *perf) > +{ > + > + unsigned int i; > + > + for (i =3D 0; i < perf->state_count; i++) > + amd_fixup_frequency(&perf->states[i]); > + > +} > static int powernow_cpufreq_verify(struct cpufreq_policy *policy) > { > struct acpi_cpufreq_data *data; > @@ -253,6 +287,8 @@ static int powernow_cpufreq_cpu_init(struct cpufreq= _policy *policy) > =20 > policy->governor =3D cpufreq_opt_governor ? : CPUFREQ_DEFAULT_GOVE= RNOR; > =20 > + amd_fixup_freq(perf); > + > /* table init */ > for (i =3D 0; i < perf->state_count && i <=3D max_hw_pstate; i++) = { > if (i > 0 && perf->states[i].core_frequency >=3D >=20 That would look good to me. Using Thunderbird I do not want to make any statement about indentation. -Stefan --------------enig5073EBB442BE6212491A2E01 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with undefined - http://www.enigmail.net/ iQIcBAEBCgAGBQJROFPIAAoJEOhnXe7L7s6jsnMQAI8Q6/IBEmSScW5kWCNXRRHg KkWHtqxIsDzcIC8+uaNz5Vc2/VOqjumrziJUd/Roim9k5p/ap+JSTWP0GjyIDrnQ o+KbNMIMsIIucyUEq1CpeBVesVo9n8RoRfvxBiGlKByzWZ7Rdm5tzlZADDtYg7uL ZPIdGQhGo4eqbozgGbkpf4KnRLJF4SeVz3nJ/ERac2LUQGgMcY8sPO31sShywxpl 7y064WhATBnPUtT9VdA97YcabtEUN/83irxFv/q6KvAHtGfYYjW8Epd4TzniX0Z5 l3dnjZ3Th8FwA52hG/m05MYIY7csNEINI8j1f+VJ3JW9WndcDziBZH6e6QBb1znI i/YGSp4cXGPzVlpcGtLSwuebBXMpnUt/V9ySXFvM6ekrrRe0cCDL8K/SYIL1d/Vt ZH14i8/tGLv7vyA1jcedg2nRASNfStCeOHon1ZJlLJIM0p69mLg1NsbShsd252MS W1UTQouC6PuTG5bn7kt+JNxo94kZ4Byz8gWRavDjXugW5kQgtiEI5hPXSLr4clMa dxRZbX0BRXC2NTO4yE1CsgRsGbaBHtKgkLGE3EsWoshsUkKY0/iJp7BVGkBz1cf8 PDdb6KC2VMT8CnGy7zPtcPKlV/Uq6zMnVn7pYMN2zRneYo8tpQK0MmDq+H61Pj/O I7H8fc4cTDZukKR0bOET =KOs5 -----END PGP SIGNATURE----- --------------enig5073EBB442BE6212491A2E01-- --===============6794636740631495780== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============6794636740631495780==--