Messages in this thread | | | Subject | Re: [PATCH 2/3] perf/x86/intel: Add more Icelake CPUIDs | From | "Liang, Kan" <> | Date | Mon, 3 Jun 2019 12:14:49 -0400 |
| |
On 6/3/2019 11:47 AM, Peter Zijlstra wrote: > On Mon, Jun 03, 2019 at 06:41:21AM -0700, kan.liang@linux.intel.com wrote: >> @@ -4962,7 +4965,9 @@ __init int intel_pmu_init(void) >> x86_pmu.cpu_events = get_icl_events_attrs(); >> x86_pmu.rtm_abort_event = X86_CONFIG(.event=0xca, .umask=0x02); >> x86_pmu.lbr_pt_coexist = true; >> - intel_pmu_pebs_data_source_skl(false); >> + intel_pmu_pebs_data_source_skl( >> + (boot_cpu_data.x86_model == INTEL_FAM6_ICELAKE_X) || >> + (boot_cpu_data.x86_model == INTEL_FAM6_ICELAKE_XEON_D)); > > That's pretty sad, a model switch inside a model switch :/ > >> pr_cont("Icelake events, "); >> name = "icelake"; >> break; > > Would something like so not be nicer?
Yes, it looks better. Thanks.
Should I combine your patch with mine, and send out V2? Or are you prefer to add your patch on top of this patch set?
Thanks, Kan
> > --- > --- a/arch/x86/events/intel/core.c > +++ b/arch/x86/events/intel/core.c > @@ -4485,6 +4485,7 @@ __init int intel_pmu_init(void) > struct event_constraint *c; > unsigned int unused; > struct extra_reg *er; > + bool pmem = false; > int version, i; > char *name; > > @@ -4936,9 +4937,10 @@ __init int intel_pmu_init(void) > name = "knights-landing"; > break; > > + case INTEL_FAM6_SKYLAKE_X: > + pmem = true; > case INTEL_FAM6_SKYLAKE_MOBILE: > case INTEL_FAM6_SKYLAKE_DESKTOP: > - case INTEL_FAM6_SKYLAKE_X: > case INTEL_FAM6_KABYLAKE_MOBILE: > case INTEL_FAM6_KABYLAKE_DESKTOP: > x86_add_quirk(intel_pebs_isolation_quirk); > @@ -4970,8 +4972,7 @@ __init int intel_pmu_init(void) > td_attr = hsw_events_attrs; > mem_attr = hsw_mem_events_attrs; > tsx_attr = hsw_tsx_events_attrs; > - intel_pmu_pebs_data_source_skl( > - boot_cpu_data.x86_model == INTEL_FAM6_SKYLAKE_X); > + intel_pmu_pebs_data_source_skl(pmem); > > if (boot_cpu_has(X86_FEATURE_TSX_FORCE_ABORT)) { > x86_pmu.flags |= PMU_FL_TFA; > @@ -4985,7 +4986,11 @@ __init int intel_pmu_init(void) > name = "skylake"; > break; > > + case INTEL_FAM6_ICELAKE_X: > + case INTEL_FAM6_ICELAKE_XEON_D: > + pmem = true; > case INTEL_FAM6_ICELAKE_MOBILE: > + case INTEL_FAM6_ICELAKE_DESKTOP: > x86_pmu.late_ack = true; > memcpy(hw_cache_event_ids, skl_hw_cache_event_ids, sizeof(hw_cache_event_ids)); > memcpy(hw_cache_extra_regs, skl_hw_cache_extra_regs, sizeof(hw_cache_extra_regs)); > @@ -5009,7 +5014,7 @@ __init int intel_pmu_init(void) > tsx_attr = icl_tsx_events_attrs; > x86_pmu.rtm_abort_event = X86_CONFIG(.event=0xca, .umask=0x02); > x86_pmu.lbr_pt_coexist = true; > - intel_pmu_pebs_data_source_skl(false); > + intel_pmu_pebs_data_source_skl(pmem); > pr_cont("Icelake events, "); > name = "icelake"; > break; >
| |