Messages in this thread | | | Date | Mon, 15 Aug 2022 20:48:36 +0800 | Subject | Re: [PATCH v2 1/7] perf/x86/core: Update x86_pmu.pebs_capable for ICELAKE_{X,D} | From | Like Xu <> |
| |
On 15/8/2022 7:51 pm, Peter Zijlstra wrote: > On Mon, Aug 15, 2022 at 05:43:34PM +0800, Like Xu wrote: > >>> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c >>> index 2db93498ff71..b42c1beb9924 100644 >>> --- a/arch/x86/events/intel/core.c >>> +++ b/arch/x86/events/intel/core.c >>> @@ -5933,7 +5933,6 @@ __init int intel_pmu_init(void) >>> x86_pmu.pebs_aliases = NULL; >>> x86_pmu.pebs_prec_dist = true; >>> x86_pmu.lbr_pt_coexist = true; >>> - x86_pmu.pebs_capable = ~0ULL; >>> x86_pmu.flags |= PMU_FL_HAS_RSP_1; >>> x86_pmu.flags |= PMU_FL_PEBS_ALL; >>> x86_pmu.get_event_constraints = glp_get_event_constraints; >>> @@ -6291,7 +6290,6 @@ __init int intel_pmu_init(void) >>> x86_pmu.pebs_aliases = NULL; >>> x86_pmu.pebs_prec_dist = true; >>> x86_pmu.pebs_block = true; >>> - x86_pmu.pebs_capable = ~0ULL; >>> x86_pmu.flags |= PMU_FL_HAS_RSP_1; >>> x86_pmu.flags |= PMU_FL_NO_HT_SHARING; >>> x86_pmu.flags |= PMU_FL_PEBS_ALL; >>> @@ -6337,7 +6335,6 @@ __init int intel_pmu_init(void) >>> x86_pmu.pebs_aliases = NULL; >>> x86_pmu.pebs_prec_dist = true; >>> x86_pmu.pebs_block = true; >>> - x86_pmu.pebs_capable = ~0ULL; >>> x86_pmu.flags |= PMU_FL_HAS_RSP_1; >>> x86_pmu.flags |= PMU_FL_NO_HT_SHARING; >>> x86_pmu.flags |= PMU_FL_PEBS_ALL; >>> diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c >>> index ba60427caa6d..e2da643632b9 100644 >>> --- a/arch/x86/events/intel/ds.c >>> +++ b/arch/x86/events/intel/ds.c >>> @@ -2258,6 +2258,7 @@ void __init intel_ds_init(void) >>> x86_pmu.drain_pebs = intel_pmu_drain_pebs_icl; >>> x86_pmu.pebs_record_size = sizeof(struct pebs_basic); >>> if (x86_pmu.intel_cap.pebs_baseline) { >>> + x86_pmu.pebs_capable = ~0ULL; >> >> The two features of "Extended PEBS (about pebs_capable)" and "Adaptive PEBS >> (about pebs_baseline)" >> are orthogonal, although the two are often supported together. > > The SDM explicitly states that PEBS Baseline implies Extended PEBS. See > 3-19.8 (April 22 edition).
Indeed, "IA32_PERF_CAPABILITIES.PEBS_BASELINE [bit 14]: If set, the following is true - Extended PEBS is supported; - Adaptive PEBS is supported;"
> > The question is if there is hardware that has Extended PEBS but doesn't > have Baseline; and I simply don't know and was hoping Kan could find > out.
In the SDM world, we actually have three PEBS sub-features:
[July 2017] 18.5.4.1 Extended PEBS (first on Goldmont Plus) [January 2019] 18.9.2 Adaptive PEBS (first on Tremont) [April 2021] IA32_PERF_CAPABILITIES.PEBS_BASELINE in the Table 2-2. IA-32 Architectural MSRs
Waiting Kan to confirm the real world.
> > That said; the above patch can be further improved by also removing the > PMU_FL_PEBS_ALL lines, which is already set by intel_ds_init(). > > In general though; the point is, we shouldn't be doing the FMS table > thing for discoverable features. Having pebs_capable = ~0 and > PMU_FL_PEBS_ALL on something with BASELINE set is just wrong.
| |