On Tue, May 21, 2013 at 01:05:37PM +0200, Peter Zijlstra wrote: > On Tue, May 21, 2013 at 10:56:30AM +0200, Robert Richter wrote: > > On 17.05.13 12:57:30, Peter Zijlstra wrote: > > > > > > So what about something like the below? > > > > See my comments below, otherwise it looks fine to me. > > I've been liberal and read an Ack there, holler if that needs be amended. > > > There is the question about core performance counters and its > > constraints on fam16h. Not sure if there are any. Cc'ing Jacob. > > Currently the code doesn't work for Fam16h afaict, so I didn't wreck > that did I?
Hi, right, Family 16h does not have perfctr_core, instead it still has the 4 legacy performance counter registers just like Family 10h, and so does not have any special constraints as 15h's perfctr_core does. Thanks! Acked-by: Jacob Shin <[email protected]> > > > > /* > > > * If core performance counter extensions exists, we must use > > > * MSR_F15H_PERF_CTL/MSR_F15H_PERF_CTR msrs. See also > > > > ... amd_pmu_addr_offset(): > > > > * If core performance counter extensions exists, we must use > > * MSR_F15H_PERF_CTL/MSR_F15H_PERF_CTR msrs. See also > > * amd_pmu_addr_offset(). > > > > Done. > > > > @@ -675,8 +670,7 @@ static int setup_perfctr_core(void) > > > x86_pmu.perfctr = MSR_F15H_PERF_CTR; > > > x86_pmu.num_counters = AMD64_NUM_COUNTERS_CORE; > > > > > > - printk(KERN_INFO "perf: AMD core performance counters detected\n"); > > > - > > > + pr_cont("core perfctr, "); > > > return 0; > > > } > > > > > > @@ -688,8 +682,8 @@ __init int amd_pmu_init(void) > > > > > > x86_pmu = amd_pmu; > > > > > > - setup_event_constraints(); > > > - setup_perfctr_core(); > > > + if (cpu_has_perfctr_core && amd_core_pmu_init()) > > > + return -ENODEV; > > > > Better return result of amd_core_pmu_init(). > > Done.. that was admittedly a tad lazy of me :-) > > --- > Subject: perf, x86: Rework AMD PMU init code > From: Peter Zijlstra <[email protected]> > Date: Fri, 17 May 2013 12:57:30 +0200 > > Josh reported that his QEMU is a bad hardware emulator and trips a > WARN in the AMD PMU init code. He requested the WARN be turned into a > pr_err() or similar. > > While there, rework the code a little. > > Reported-by: Josh Boyer <[email protected]> > Acked-by: Robert Richter <[email protected]> > Signed-off-by: Peter Zijlstra <[email protected]> > --- > arch/x86/kernel/cpu/perf_event_amd.c | 34 > +++++++++++++++++----------------- > 1 file changed, 17 insertions(+), 17 deletions(-) > > --- a/arch/x86/kernel/cpu/perf_event_amd.c > +++ b/arch/x86/kernel/cpu/perf_event_amd.c > @@ -648,48 +648,48 @@ static __initconst const struct x86_pmu > .cpu_dead = amd_pmu_cpu_dead, > }; > > -static int setup_event_constraints(void) > +__init int amd_core_pmu_init(void) > { > - if (boot_cpu_data.x86 == 0x15) > + if (!cpu_has_perfctr_core) > + return 0; > + > + switch (boot_cpu_data.x86) { > + case 0x15: > + pr_cont("Fam15h "); > x86_pmu.get_event_constraints = amd_get_event_constraints_f15h; > - return 0; > -} > + break; > > -static int setup_perfctr_core(void) > -{ > - if (!cpu_has_perfctr_core) { > - WARN(x86_pmu.get_event_constraints == > amd_get_event_constraints_f15h, > - KERN_ERR "Odd, counter constraints enabled but no core > perfctrs detected!"); > + default: > + pr_err("core perfctr but no constraints; unknown hardware!\n"); > return -ENODEV; > } > > - WARN(x86_pmu.get_event_constraints == amd_get_event_constraints, > - KERN_ERR "hw perf events core counters need constraints handler!"); > - > /* > * If core performance counter extensions exists, we must use > * MSR_F15H_PERF_CTL/MSR_F15H_PERF_CTR msrs. See also > - * x86_pmu_addr_offset(). > + * amd_pmu_addr_offset(). > */ > x86_pmu.eventsel = MSR_F15H_PERF_CTL; > x86_pmu.perfctr = MSR_F15H_PERF_CTR; > x86_pmu.num_counters = AMD64_NUM_COUNTERS_CORE; > > - printk(KERN_INFO "perf: AMD core performance counters detected\n"); > - > + pr_cont("core perfctr, "); > return 0; > } > > __init int amd_pmu_init(void) > { > + int ret; > + > /* Performance-monitoring supported from K7 and later: */ > if (boot_cpu_data.x86 < 6) > return -ENODEV; > > x86_pmu = amd_pmu; > > - setup_event_constraints(); > - setup_perfctr_core(); > + ret = amd_core_pmu_init(); > + if (ret) > + return ret; > > /* Events are common for all AMDs */ > memcpy(hw_cache_event_ids, amd_hw_cache_event_ids, > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
