Messages in this thread Patch in this message | | | Subject | Re: [PATCH] x86/perf: Add hardware performance events support for Zhaoxin CPU. | From | CodyYao-oc <> | Date | Thu, 16 Apr 2020 14:26:32 +0800 |
| |
On 2020/4/15 下午6:23, Peter Zijlstra wrote: > On Mon, Apr 13, 2020 at 11:14:29AM +0800, CodyYao-oc wrote: >> Zhaoxin CPU has provided facilities for monitoring performance >> via PMU(Performance Monitor Unit), but the functionality is unused so far. >> Therefore, add support for zhaoxin pmu to make performance related >> hardware events available. >> >> Signed-off-by: CodyYao-oc <CodyYao-oc@zhaoxin.com> >> Reported-by: kbuild test robot <lkp@intel.com> > > What's that reported-by thing? Did the robot complain you didn't have a > PMU implementation? > Hi Peter, it's a warning message about uninitialized variable, paste the log below, sorry for miss it. Futhermore, it will disappear base on your newly modified patch. Thanks!
[All warnings (new ones prefixed by >>):
>> arch/x86/events/zhaoxin/core.c:362:6: warning: variable 'is_zxc' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] if (boot_cpu_data.x86 == 0x06 && ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ arch/x86/events/zhaoxin/core.c:369:6: note: uninitialized use occurs here if (is_zxc) ^~~~~~ arch/x86/events/zhaoxin/core.c:362:2: note: remove the 'if' if its condition is always true if (boot_cpu_data.x86 == 0x06 && ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> arch/x86/events/zhaoxin/core.c:362:6: warning: variable 'is_zxc' is used uninitialized whenever '&&' condition is false [-Wsometimes-uninitialized] if (boot_cpu_data.x86 == 0x06 && ^~~~~~~~~~~~~~~~~~~~~~~~~ arch/x86/events/zhaoxin/core.c:369:6: note: uninitialized use occurs here if (is_zxc) ^~~~~~ arch/x86/events/zhaoxin/core.c:362:6: note: remove the '&&' if its condition is always true if (boot_cpu_data.x86 == 0x06 && ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ arch/x86/events/zhaoxin/core.c:352:13: note: initialize the variable 'is_zxc' to silence this warning bool is_zxc; ^ = 0 2 warnings generated.]
Reported-by: kbuild test rebot<lkp@intel.com>
> Anyway, I've made the below changes to the patch. > > --- > --- a/arch/x86/events/perf_event.h > +++ b/arch/x86/events/perf_event.h > @@ -618,6 +618,7 @@ struct x86_pmu { > > /* PMI handler bits */ > unsigned int late_ack :1, > + enabled_ack :1, > counter_freezing :1; > /* > * sysfs attrs > --- a/arch/x86/events/zhaoxin/core.c > +++ b/arch/x86/events/zhaoxin/core.c > @@ -357,10 +357,9 @@ static int zhaoxin_pmu_handle_irq(struct > { > struct perf_sample_data data; > struct cpu_hw_events *cpuc; > - int bit; > - u64 status; > - bool is_zxc = false; > int handled = 0; > + u64 status; > + int bit; > > cpuc = this_cpu_ptr(&cpu_hw_events); > apic_write(APIC_LVTPC, APIC_DM_NMI); > @@ -369,14 +368,8 @@ static int zhaoxin_pmu_handle_irq(struct > if (!status) > goto done; > > - if (boot_cpu_data.x86 == 0x06 && > - (boot_cpu_data.x86_model == 0x0f || > - boot_cpu_data.x86_model == 0x19)) > - is_zxc = true; > again: > - > - /*Clearing status works only if the global control is enable on zxc.*/ > - if (is_zxc) > + if (x86_pmu.enabled_ack) > zxc_pmu_ack_status(status); > else > zhaoxin_pmu_ack_status(status); > @@ -504,12 +497,10 @@ static __init void zhaoxin_arch_events_q > int bit; > > /* disable event that reported as not presend by cpuid */ > - for_each_set_bit(bit, x86_pmu.events_mask, > - ARRAY_SIZE(zx_arch_events_map)) { > - > + for_each_set_bit(bit, x86_pmu.events_mask, ARRAY_SIZE(zx_arch_events_map)) { > zx_pmon_event_map[zx_arch_events_map[bit].id] = 0; > pr_warn("CPUID marked event: \'%s\' unavailable\n", > - zx_arch_events_map[bit].name); > + zx_arch_events_map[bit].name); > } > } > > @@ -534,12 +525,12 @@ __init int zhaoxin_pmu_init(void) > return -ENODEV; > > version = eax.split.version_id; > - if (version == 2) { > - x86_pmu = zhaoxin_pmu; > - pr_info("Version check pass!\n"); > - } else > + if (version != 2) > return -ENODEV; > > + x86_pmu = zhaoxin_pmu; > + pr_info("Version check pass!\n"); > + > x86_pmu.version = version; > x86_pmu.num_counters = eax.split.num_counters; > x86_pmu.cntval_bits = eax.split.bit_width; > @@ -552,11 +543,13 @@ __init int zhaoxin_pmu_init(void) > > switch (boot_cpu_data.x86) { > case 0x06: > - if (boot_cpu_data.x86_model == 0x0f || > - boot_cpu_data.x86_model == 0x19) { > + if (boot_cpu_data.x86_model == 0x0f || boot_cpu_data.x86_model == 0x19) { > > x86_pmu.max_period = x86_pmu.cntval_mask >> 1; > > + /* Clearing status works only if the global control is enable on zxc. */ > + x86_pmu.enabled_ack = 1; > + > x86_pmu.event_constraints = zxc_event_constraints; > zx_pmon_event_map[PERF_COUNT_HW_INSTRUCTIONS] = 0; > zx_pmon_event_map[PERF_COUNT_HW_CACHE_REFERENCES] = 0; > @@ -564,40 +557,37 @@ __init int zhaoxin_pmu_init(void) > zx_pmon_event_map[PERF_COUNT_HW_BUS_CYCLES] = 0; > > pr_cont("ZXC events, "); > - } else > - return -ENODEV; > - break; > + break; > + } > + return -ENODEV; > + > case 0x07: > zx_pmon_event_map[PERF_COUNT_HW_STALLED_CYCLES_FRONTEND] = > - X86_CONFIG(.event = 0x01, .umask = 0x01, .inv = 0x01, .cmask = 0x01); > + X86_CONFIG(.event = 0x01, .umask = 0x01, .inv = 0x01, .cmask = 0x01); > > zx_pmon_event_map[PERF_COUNT_HW_STALLED_CYCLES_BACKEND] = > - X86_CONFIG(.event = 0x0f, .umask = 0x04, .inv = 0, .cmask = 0); > + X86_CONFIG(.event = 0x0f, .umask = 0x04, .inv = 0, .cmask = 0); > > switch (boot_cpu_data.x86_model) { > case 0x1b: > memcpy(hw_cache_event_ids, zxd_hw_cache_event_ids, > - sizeof(hw_cache_event_ids)); > + sizeof(hw_cache_event_ids)); > > x86_pmu.event_constraints = zxd_event_constraints; > > - zx_pmon_event_map[PERF_COUNT_HW_BRANCH_INSTRUCTIONS] > - = 0x0700; > - zx_pmon_event_map[PERF_COUNT_HW_BRANCH_MISSES] > - = 0x0709; > + zx_pmon_event_map[PERF_COUNT_HW_BRANCH_INSTRUCTIONS] = 0x0700; > + zx_pmon_event_map[PERF_COUNT_HW_BRANCH_MISSES] = 0x0709; > > pr_cont("ZXD events, "); > break; > case 0x3b: > memcpy(hw_cache_event_ids, zxe_hw_cache_event_ids, > - sizeof(hw_cache_event_ids)); > + sizeof(hw_cache_event_ids)); > > x86_pmu.event_constraints = zxd_event_constraints; > > - zx_pmon_event_map[PERF_COUNT_HW_BRANCH_INSTRUCTIONS] > - = 0x0028; > - zx_pmon_event_map[PERF_COUNT_HW_BRANCH_MISSES] > - = 0x0029; > + zx_pmon_event_map[PERF_COUNT_HW_BRANCH_INSTRUCTIONS] = 0x0028; > + zx_pmon_event_map[PERF_COUNT_HW_BRANCH_MISSES] = 0x0029; > > pr_cont("ZXE events, "); > break; > @@ -605,13 +595,13 @@ __init int zhaoxin_pmu_init(void) > return -ENODEV; > } > break; > + > default: > return -ENODEV; > } > > x86_pmu.intel_ctrl = (1 << (x86_pmu.num_counters)) - 1; > - x86_pmu.intel_ctrl |= > - ((1LL << x86_pmu.num_counters_fixed)-1) << INTEL_PMC_IDX_FIXED; > + x86_pmu.intel_ctrl |= ((1LL << x86_pmu.num_counters_fixed)-1) << INTEL_PMC_IDX_FIXED; > > if (x86_pmu.event_constraints) { > for_each_event_constraint(c, x86_pmu.event_constraints) { > --- a/arch/x86/kernel/cpu/perfctr-watchdog.c > +++ b/arch/x86/kernel/cpu/perfctr-watchdog.c > @@ -63,6 +63,7 @@ static inline unsigned int nmi_perfctr_m > case 15: > return msr - MSR_P4_BPU_PERFCTR0; > } > + fallthrough; > case X86_VENDOR_ZHAOXIN: > case X86_VENDOR_CENTAUR: > return msr - MSR_ARCH_PERFMON_PERFCTR0; > @@ -95,6 +96,7 @@ static inline unsigned int nmi_evntsel_m > case 15: > return msr - MSR_P4_BSU_ESCR0; > } > + fallthrough; > case X86_VENDOR_ZHAOXIN: > case X86_VENDOR_CENTAUR: > return msr - MSR_ARCH_PERFMON_EVENTSEL0; > . >
| |