[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] kernel 3.7+ cpufreq regression on AMD system running as dom0



On 01/21/2013 12:42 PM, Borislav Petkov wrote:
On Mon, Jan 21, 2013 at 12:22:18PM +0000, Stefan Bader wrote:
So for having the "check for sensible BIOS" in mainline I refreshed
the patch (fixed the bit test, and actually tested it this time) and
also added some hopefully sensible explanation to it (attached
below).

Should I send it to acpi lists or would that have to go via an Andre?

Maybe Rafael could pick it up?


-Stefan

 From 6e2fc8291c91339123a37162382d8b08b50867ba Mon Sep 17 00:00:00 2001
From: Stefan Bader <stefan.bader@xxxxxxxxxxxxx>
Date: Mon, 14 Jan 2013 16:17:00 +0100
Subject: [PATCH] ACPI: Check MSR valid bit before using P-state frequencies

To fix incorrect P-state frequencies which can happen on
some AMD systems f594065faf4f9067c2283a34619fc0714e79a98d
   "ACPI: Add fixups for AMD P-state figures"
introduced a quirk to obtain the correct values by reading
from AMD specific MSRs.

This did cause a regression when running a kernel using that
quirk under Xen which does (currently) not pass on the contents
of the HW but 0.

Actually this should say "does not currently pass through MSR accesses
to baremetal" or similar.

Ok, that sounds much better.


And this bit you mean is actually bit 63:

"63: PstateEn. Read-write. 1=The P-state specified by this MSR is valid.
0=The P-state specified by this MSR is not valid. The purpose of this
register is to indicate if the rest of the P-state information in the
register is valid after a reset; it controls no hardware."

in the MSRC001_00[68:64] P-State [4:0] Registers.

Darn, yes.


And this seems to cause a failure to initialize
the ondemand governour (hard to say for sure as all P-states
appear to run at the same frequency).

While this should also be fixed in the hypervisor (to allow
a guest to read that MSR), this patch is intended to work
around the issue in the meantime. In discussion 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.

Signed-off-by: Stefan Bader <stefan.bader@xxxxxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx # v3.7..
---
  drivers/acpi/processor_perflib.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
index 836bfe0..41f4bdac 100644
--- a/drivers/acpi/processor_perflib.c
+++ b/drivers/acpi/processor_perflib.c
@@ -340,6 +340,9 @@ static void amd_fixup_frequency(struct
acpi_processor_px *px, int i)
        if ((boot_cpu_data.x86 == 0x10 && boot_cpu_data.x86_model < 10)
            || boot_cpu_data.x86 == 0x11) {
                rdmsr(MSR_AMD_PSTATE_DEF_BASE + index, lo, hi);
+               /* Bit 63 indicates whether contents are valid */
+               if (!(hi & 0x80000000))

You can make this a lot more explicit:

                if (!(hi & BIT(31)))
                        return;


True, ok, so let me respin the whole thing and re-send it.

-Stefan
This way

1) you're sure you're testing the correct bit and
2) any reviewer can know on the spot which bit it is about.

+                       return;
                fid = lo & 0x3f;
                did = (lo >> 6) & 7;
                if (boot_cpu_data.x86 == 0x10)

Thanks.



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.