Re: [Xen-devel] [PATCH] ACPI: Add fixups for AMD P-state figures.

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

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@xxxxxxxxxxxxx
> > >>> CC: bp@xxxxxxx
> > >>> CC: borislav.ostrovsky@xxxxxxxxxx
> > >>
> > >> boris.ostrovsky@xxxxxxxxxx
> > > 
> > > Whoops!
> > > 
> > > Here is an updated version:


>From b704d4419e9e483922382d2339bd41c245f435e1 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Date: Tue, 5 Mar 2013 14:40:52 -0500
Subject: [PATCH] ACPI: Add fixups for AMD P-state figures.

In the Linux kernel, these two git commits:

- f594065faf4f9067c2283a34619fc0714e79a98d
  ACPI: Add fixups for AMD P-state figures
- 9855d8ce41a7801548a05d844db2f46c3e810166
  ACPI: Check MSR valid bit before using P-state frequencies

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..)

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..)

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.

I've tested it under Dell Inc. PowerEdge T105 /0RR825, BIOS 1.3.2
08/20/2008 where this quirk can be observed (x86 == 0x10, model == 2).
Also on other AMD (x86 == 0x12, A8-3850; x86 = 0x14, AMD E-350) to
make sure the quirk is not applied there.

CC: stefan.bader@xxxxxxxxxxxxx
CC: bp@xxxxxxx
CC: boris.ostrovsky@xxxxxxxxxx
[v1: Indent, #define, and email changes per Boris's review]
[v2: Redid the x86+model check, updated description]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
---
 xen/arch/x86/acpi/cpufreq/powernow.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/xen/arch/x86/acpi/cpufreq/powernow.c 
b/xen/arch/x86/acpi/cpufreq/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;
 }
 
+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 == 0x10 && boot_cpu_data.x86_model < 10) &&
+        !(boot_cpu_data.x86 == 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 = lo & 0x3f;
+    did = (lo >> 6) & 7;
+    if (boot_cpu_data.x86 == 0x10)
+        px->core_frequency = (100 * (fid + 16)) >> did;
+    else
+        px->core_frequency = (100 * (fid + 8)) >> did;
+}
+
+static void amd_fixup_freq(struct processor_performance *perf)
+{
+
+    unsigned int i;
+
+    for (i = 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)
 
     policy->governor = cpufreq_opt_governor ? : CPUFREQ_DEFAULT_GOVERNOR;
 
+    amd_fixup_freq(perf);
+
     /* table init */
     for (i = 0; i < perf->state_count && i <= max_hw_pstate; i++) {
         if (i > 0 && perf->states[i].core_frequency >=
-- 
1.8.0.2


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel