Re: kernel 3.7+ cpufreq regression on AMD system running as dom0

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 18/01/13 19:38, Boris Ostrovsky wrote:
> On 01/18/2013 02:00 PM, Konrad Rzeszutek Wilk wrote:
>> Right, that information is gathered from the MSRs. I think the Xen would
>> need to do this since it can do the MSRs correctly and modify the P-states.
>>
>> So something like this in the hypervisor maybe (not even tested):
> Is there any harm in allowing dom0 read P-state registers?

Yes - the dom0 vcpu could be moved across pcpus between MSR accesses.

There is currently some hacky code for pinning the dom0 cpus right at
boot time, after which dom0 is permitted to access a few more MSRs,
which appear to be power related.

~Andrew

>
> Something along these lines:
>
> diff -r 40881d58e991 xen/arch/x86/traps.c
> --- a/xen/arch/x86/traps.c	Thu Jan 17 14:47:04 2013 -0500
> +++ b/xen/arch/x86/traps.c	Fri Jan 18 09:32:51 2013 -0500
> @@ -2535,7 +2535,7 @@ static int emulate_privileged_op(struct
>           case MSR_K8_PSTATE7:
>               if ( boot_cpu_data.x86_vendor != X86_VENDOR_AMD )
>                   goto fail;
> -            if ( !is_cpufreq_controller(v->domain) )
> +            if ( d->domain_id != 0 )
>               {
>                   regs->eax = regs->edx = 0;
>                   break;
>
>
> (It does seem to fix the bug too)
>
> -boris
>
>
>> diff --git a/xen/arch/x86/acpi/cpufreq/powernow.c b/xen/arch/x86/acpi/cpufreq/powernow.c
>> index a9b7792..54e7808 100644
>> --- a/xen/arch/x86/acpi/cpufreq/powernow.c
>> +++ b/xen/arch/x86/acpi/cpufreq/powernow.c
>> @@ -146,7 +146,40 @@ static int powernow_cpufreq_target(struct cpufreq_policy *policy,
>>
>>       return 0;
>>   }
>> +#define MSR_AMD_PSTATE_DEF_BASE     0xc0010064
>> +static void amd_fixup_frequency(struct xen_processor_px *px, int i)
>> +{
>> +	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_AMD_PSTATE_DEF_BASE + index, lo, hi);
>> +        /* Bit 63 indicates whether contents are valid */
>> +        if (!(hi & 0x80000000))
>> +            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;
>> +	}
>> +}
>> +
>> +static void amd_fixup_freq(struct processor_performance *perf)
>> +{
>>
>> +    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;
>> @@ -158,6 +191,8 @@ static int powernow_cpufreq_verify(struct cpufreq_policy *policy)
>>
>>       perf = &processor_pminfo[policy->cpu]->perf;
>>
>> +    amd_fixup_freq(perf);
>> +
>>       cpufreq_verify_within_limits(policy, 0,
>>           perf->states[perf->platform_limit].core_frequency * 1000);
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
>>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel