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

[Stefan Bader <stefan.bader@canonical.com>]

[-- Attachment #1.1: Type: text/plain, Size: 5974 bytes --]

On 07.03.2013 15:17, Konrad Rzeszutek Wilk wrote:
> On Thu, Mar 07, 2013 at 09:45:51AM +0100, Stefan Bader wrote:
>> 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:
>>>
>>>
>>> From b704d4419e9e483922382d2339bd41c245f435e1 Mon Sep 17 00:00:00 2001
>>> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>> 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@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 <konrad.wilk@oracle.com>
>>> ---
>>>  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 >=
>>>
>>
>> That would look good to me. Using Thunderbird I do not want to make any
>> statement about indentation.
> 
> I think it came out right (I hope). I presume I can stick 'Acked-by' on the
> patch from you?
> 
Sure.



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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