Messages in this thread Patch in this message | | | Date | Mon, 06 Jan 2014 10:40:39 -0800 | From | Dirk Brandewie <> | Subject | Re: intel_pstate divide error with v3.13-rc4-256-gb7000ad |
| |
On 01/06/2014 03:37 AM, Rafael J. Wysocki wrote: > On Monday, January 06, 2014 12:20:32 PM Paolo Bonzini wrote: >> Il 04/01/2014 22:38, Rafael J. Wysocki ha scritto: >>> On Saturday, January 04, 2014 07:48:13 PM Gleb Natapov wrote: >>>> On Sat, Jan 04, 2014 at 06:38:59PM +0100, Paolo Bonzini wrote: >>>>> Il 04/01/2014 15:38, Rafael J. Wysocki ha scritto: >>>>>> Well, it's just a sanity check and it makes the problem go away for the reporter. >>>>>> >>>>>>>> Your patch is welcome but perhaps it should have a WARN_ON too. >>>>>> It has been pulled in already, so the WARN_ON() can only be added via a separate >>>>>> patch now. Would you like to prepare that patch? >>>>> >>>>> Yes, I'll add it together with the CPUID check. I'll send the patch so >>>>> that it can get into 3.14. >>>>> >>>> CPUID check, while correct, will sweep the problem under the rug. Current >>>> Linux logic should detect non working pstate in KVM. We should look into >>>> why this is not happening for nested. >>> >>> I agree. It's better not to use CPUID for that in my opinion. >> >> Among hypervisors, RHEL5's Xen is probably one of the oldest in actual >> use with new hardware and new kernels, and the CPUID bit has been fixed >> in 2011. Older versions wouldn't run new kernels due to other CPUID >> bits not being cleared properly in VMs. >> >> Is there real hardware that has the CPUID bit set and non-working >> pstate? If there's no such real hardware, CPUID is what the SDM says >> you should use to detect presence of the APERF/MPERF msrs. > > OK > >> Having extra safety checks is fine on top of what the SDM says, but IMO >> they should be WARN_ONs. Otherwise you are sweeping bugs under the rug >> just as much. > > As I said I'm not against adding WARN_ON()s there. :-) >
The patch below adds a feature check for APERF/MPERF. With this patch you should NOT see "Intel P-state driver initializing." in dmesg for KVM.
commit 4279f36818bd3ac42f077de114b17eb27d81d482 Author: Dirk Brandewie <dirk.j.brandewie@intel.com> Date: Mon Jan 6 10:19:38 2014 -0800
intel_pstate: Add X86_FEATURE_APERFMPERF to cpu match parameters.
KVM environments do not support APERF/MPERF MSRs. intel_pstate cannot operate without these registers.
The previous validity checks in intel_pstate_msrs_not_valid() are insufficent in nested KVMs.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1046317
Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com> --- drivers/cpufreq/intel_pstate.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c index 0f63f5d..fe91dad 100644 --- a/drivers/cpufreq/intel_pstate.c +++ b/drivers/cpufreq/intel_pstate.c @@ -619,7 +619,8 @@ static void intel_pstate_timer_func(unsigned long __data) }
#define ICPU(model, policy) \ - { X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, (unsigned long)&policy } + { X86_VENDOR_INTEL, 6, model, X86_FEATURE_APERFMPERF,\ + (unsigned long)&policy }
static const struct x86_cpu_id intel_pstate_cpu_ids[] = { ICPU(0x2a, core_params),
| |