Messages in this thread Patch in this message | | | From | Mark Davies <> | Subject | Re: [PATCH] perf,x86,p6: Add userspace RDPMC quirk for P6 | Date | Thu, 06 Feb 2014 22:15:23 +0000 |
| |
That fixed it. Booted patched 3.4.78 with CONFIG_APM=y with no problem.
dmesg contains the "Userspace RDPMC support disabled due to a CPU erratum" message
"cat /sys/devices/cpu/rdpmc" gives "0"
"echo 1 >/sys/devices/cpu/rdpmc" gives "bash: echo: write error: Unknown error 524"
Thanks for fixing it. How I missed the return statements in intel_pmu_init I don't know, probably got a bit over excited when I spotted what the problem was.
Mark Davies
On Thu, Feb 06 2014, Peter Zijlstra wrote:
> On Thu, Feb 06, 2014 at 07:41:37PM +0000, Mark Davies wrote: >> Peter >> >> As I reported the problem I thought I better contribute by testing the >> fix. >> >> I took the patch below and managed to change the 3.4.78 code to match >> and tried to boot my Pentium Pro system but it hung as before. After >> some investigation I discovered the quirk is not being run, in fact no >> quirks were being run in init_hw_perf_events as x86_pmu.quirks was NULL. >> >> The problem as far as I can see is that the quirk is added to x86_pmu in >> p6_pmu_init but then x86_pmu is reassigned at the end of that function >> (x86_pmu = p6_pmu) resetting x86_pmu.quirks to NULL. > > Right so far. > >> x86_pmu looks like >> it is reassigned again in intel_pmu_init to either core_pmu or intel_pmu >> after the call to p6_pmu_init so I am not sure which x86_pmu.quirks >> init_hw_perf_events tries to use. > > Note how intel_pmu_init() does: return p6_pmu_init(), so it will never > get to the x86_pmu assignment for the ARCH_PERFMON drivers. > > Yep, thanks for testing and you're (mostly) quite right. > > We can do the copy early though, if we return -ENODEV nothing will end > up using the x86_pmu anyway, so it doesn't matter what lives in there. > > --- > Subject: perf,x86,p6: Add userspace RDPMC quirk for PPro > From: Peter Zijlstra <peterz@infradead.org> > Date: Wed, 5 Feb 2014 20:48:51 +0100 > > PPro machines can die hard when PCE gets enabled due to a CPU erratum. > The safe way it so disable it by default and keep it disabled. > > See erratum 26 in: > > http://download.intel.com/design/archives/processors/pro/docs/24268935.pdf > > Cc: Alan Cox <alan@lxorguk.ukuu.org.uk> > Cc: Ingo Molnar <mingo@kernel.org> > Cc: Stephane Eranian <eranian@google.com> > Cc: Vince Weaver <vince@deater.net> > Reported-by: Mark Davies <junk@eslaf.co.uk> > Signed-off-by: Peter Zijlstra <peterz@infradead.org> > --- > arch/x86/kernel/cpu/perf_event.c | 6 +++- > arch/x86/kernel/cpu/perf_event.h | 1 > arch/x86/kernel/cpu/perf_event_p6.c | 48 ++++++++++++++++++++++++------------ > 3 files changed, 39 insertions(+), 16 deletions(-) > > --- a/arch/x86/kernel/cpu/perf_event.c > +++ b/arch/x86/kernel/cpu/perf_event.c > @@ -1521,6 +1521,8 @@ static int __init init_hw_perf_events(vo > > pr_cont("%s PMU driver.\n", x86_pmu.name); > > + x86_pmu.attr_rdpmc = 1; /* enable userspace RDPMC usage by default */ > + > for (quirk = x86_pmu.quirks; quirk; quirk = quirk->next) > quirk->func(); > > @@ -1534,7 +1536,6 @@ static int __init init_hw_perf_events(vo > __EVENT_CONSTRAINT(0, (1ULL << x86_pmu.num_counters) - 1, > 0, x86_pmu.num_counters, 0, 0); > > - x86_pmu.attr_rdpmc = 1; /* enable userspace RDPMC usage by default */ > x86_pmu_format_group.attrs = x86_pmu.format_attrs; > > if (x86_pmu.event_attrs) > @@ -1820,6 +1821,9 @@ static ssize_t set_attr_rdpmc(struct dev > if (ret) > return ret; > > + if (x86_pmu.attr_rdpmc_broken) > + return -ENOTSUPP; > + > if (!!val != !!x86_pmu.attr_rdpmc) { > x86_pmu.attr_rdpmc = !!val; > smp_call_function(change_rdpmc, (void *)val, 1); > --- a/arch/x86/kernel/cpu/perf_event.h > +++ b/arch/x86/kernel/cpu/perf_event.h > @@ -409,6 +409,7 @@ struct x86_pmu { > /* > * sysfs attrs > */ > + int attr_rdpmc_broken; > int attr_rdpmc; > struct attribute **format_attrs; > struct attribute **event_attrs; > --- a/arch/x86/kernel/cpu/perf_event_p6.c > +++ b/arch/x86/kernel/cpu/perf_event_p6.c > @@ -231,31 +231,49 @@ static __initconst const struct x86_pmu > > }; > > +static __init void p6_pmu_rdpmc_quirk(void) > +{ > + if (boot_cpu_data.x86_mask < 9) { > + /* > + * PPro erratum 26; fixed in stepping 9 and above. > + */ > + pr_warn("Userspace RDPMC support disabled due to a CPU erratum\n"); > + x86_pmu.attr_rdpmc_broken = 1; > + x86_pmu.attr_rdpmc = 0; > + } > +} > + > __init int p6_pmu_init(void) > { > + x86_pmu = p6_pmu; > + > switch (boot_cpu_data.x86_model) { > - case 1: > - case 3: /* Pentium Pro */ > - case 5: > - case 6: /* Pentium II */ > - case 7: > - case 8: > - case 11: /* Pentium III */ > - case 9: > - case 13: > - /* Pentium M */ > + case 1: /* Pentium Pro */ > + x86_add_quirk(p6_pmu_rdpmc_quirk); > + break; > + > + case 3: /* Pentium II - Klamath */ > + case 5: /* Pentium II - Deschutes */ > + case 6: /* Pentium II - Mendocino */ > break; > + > + case 7: /* Pentium III - Katmai */ > + case 8: /* Pentium III - Coppermine */ > + case 10: /* Pentium III Xeon */ > + case 11: /* Pentium III - Tualatin */ > + break; > + > + case 9: /* Pentium M - Banias */ > + case 13: /* Pentium M - Dothan */ > + break; > + > default: > - pr_cont("unsupported p6 CPU model %d ", > - boot_cpu_data.x86_model); > + pr_cont("unsupported p6 CPU model %d ", boot_cpu_data.x86_model); > return -ENODEV; > } > > - x86_pmu = p6_pmu; > - > memcpy(hw_cache_event_ids, p6_hw_cache_event_ids, > sizeof(hw_cache_event_ids)); > > - > return 0; > }
| |