From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH] ACPI: Add fixups for AMD P-state figures. Date: Tue, 05 Mar 2013 17:12:56 -0500 Message-ID: <51366DE8.50204@oracle.com> References: <1362512728-28770-1-git-send-email-konrad.wilk@oracle.com> <51365401.4050205@oracle.com> <20130305213319.GA8235@phenom.dumpdata.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20130305213319.GA8235@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: xen-devel@lists.xensource.com, bp@suse.de, stefan.bader@canonical.com List-Id: xen-devel@lists.xenproject.org On 03/05/2013 04:33 PM, 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: > > From 3b7584f0c3c91d073bd760a038d0091b3bf5a19b 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. > > 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: boris.ostrovsky@oracle.com > [v1: Indent, #define, and email changes per Boris's review] > Signed-off-by: Konrad Rzeszutek Wilk > --- > xen/arch/x86/acpi/cpufreq/powernow.c | 38 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > > diff --git a/xen/arch/x86/acpi/cpufreq/powernow.c b/xen/arch/x86/acpi/cpufreq/powernow.c > index a9b7792..5037c30 100644 > --- a/xen/arch/x86/acpi/cpufreq/powernow.c > +++ b/xen/arch/x86/acpi/cpufreq/powernow.c > @@ -147,6 +147,42 @@ static int powernow_cpufreq_target(struct cpufreq_policy *policy, > return 0; > } > > +static void amd_fixup_frequency(struct xen_processor_px *px) > +{ > + u32 hi, lo, fid, did; > + int index = px->control & 0x00000007; > + > + if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD) > + return; > + > + if ((boot_cpu_data.x86 == 0x10 && boot_cpu_data.x86_model < 10) > + || boot_cpu_data.x86 == 0x11) { > + 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 = lo & 0x3f; > + did = (lo >> 6) & 7; > + if (boot_cpu_data.x86 == 0x10) > + px->core_frequency = (100 * (fid + 0x10)) >> did; > + else > + px->core_frequency = (100 * (fid + 8)) >> did; > + } > +} There is still something wrong with indentation, I think it's tabs vs. spaces. Everything inside if clause (except rdmsr) is indented by spaces but the rest uses tabs. -boris