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?
> > /*
> > * 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/
