Re: [PATCH] perf/x86/intel: make anythread filter support conditional
On Thu, Oct 22, 2020 at 1:00 AM Peter Zijlstra wrote:
>
> On Wed, Oct 21, 2020 at 02:16:12PM -0700, Stephane Eranian wrote:
> > Starting with Arch Perfmon v5, the anythread filter on generic counters may
> > be
> > deprecated. The current kernel was exporting the any filter without
> > checking.
> > On Icelake, it means you could do cpu/event=0x3c,any/ even though the filter
> > does not exist. This patch corrects the problem by relying on the CPUID 0xa
> > leaf
> > function to determine if anythread is supported or not as described in the
> > Intel SDM Vol3b 18.2.5.1 AnyThread Deprecation section.
> >
> > Signed-off-by: Stephane Eranian
> > ---
> > arch/x86/events/intel/core.c | 20
> > arch/x86/events/perf_event.h | 1 +
> > arch/x86/include/asm/perf_event.h | 4 +++-
> > arch/x86/kvm/cpuid.c | 4 +++-
> > 4 files changed, 27 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> > index f1926e9f2143..65bf649048a6 100644
> > --- a/arch/x86/events/intel/core.c
> > +++ b/arch/x86/events/intel/core.c
> > @@ -4220,6 +4220,16 @@ static struct attribute *intel_arch3_formats_attr[]
> > = {
> > NULL,
> > };
> >
> > +static struct attribute *intel_arch5_formats_attr[] = {
> > + _attr_event.attr,
> > + _attr_umask.attr,
> > + _attr_edge.attr,
> > + _attr_pc.attr,
> > + _attr_inv.attr,
> > + _attr_cmask.attr,
> > + NULL,
> > +};
>
> Instead of adding yet another (which is an exact duplicate of the
> existing intel_arch_formats_attr BTW), can't we clean this up and use
> is_visible() as 'demanded' by GregKH and done by Jiri here:
>
> 3d5672735b23 ("perf/x86: Add is_visible attribute_group callback for base
> events")
> b7c9b3927337 ("perf/x86/intel: Use ->is_visible callback for default group")
> baa0c83363c7 ("perf/x86: Use the new pmu::update_attrs attribute group")
>
> And only have "any" visible for v3,v4
Sure, let me resubmit with these changes.
Re: [PATCH] perf/x86/intel: make anythread filter support conditional
On Wed, Oct 21, 2020 at 02:16:12PM -0700, Stephane Eranian wrote:
> Starting with Arch Perfmon v5, the anythread filter on generic counters may be
> deprecated. The current kernel was exporting the any filter without checking.
> On Icelake, it means you could do cpu/event=0x3c,any/ even though the filter
> does not exist. This patch corrects the problem by relying on the CPUID 0xa
> leaf
> function to determine if anythread is supported or not as described in the
> Intel SDM Vol3b 18.2.5.1 AnyThread Deprecation section.
>
> Signed-off-by: Stephane Eranian
> ---
> arch/x86/events/intel/core.c | 20
> arch/x86/events/perf_event.h | 1 +
> arch/x86/include/asm/perf_event.h | 4 +++-
> arch/x86/kvm/cpuid.c | 4 +++-
> 4 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index f1926e9f2143..65bf649048a6 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -4220,6 +4220,16 @@ static struct attribute *intel_arch3_formats_attr[] = {
> NULL,
> };
>
> +static struct attribute *intel_arch5_formats_attr[] = {
> + _attr_event.attr,
> + _attr_umask.attr,
> + _attr_edge.attr,
> + _attr_pc.attr,
> + _attr_inv.attr,
> + _attr_cmask.attr,
> + NULL,
> +};
Instead of adding yet another (which is an exact duplicate of the
existing intel_arch_formats_attr BTW), can't we clean this up and use
is_visible() as 'demanded' by GregKH and done by Jiri here:
3d5672735b23 ("perf/x86: Add is_visible attribute_group callback for base
events")
b7c9b3927337 ("perf/x86/intel: Use ->is_visible callback for default group")
baa0c83363c7 ("perf/x86: Use the new pmu::update_attrs attribute group")
And only have "any" visible for v3,v4
Re: [PATCH] perf/x86/intel: make anythread filter support conditional
On Wed, Oct 21, 2020 at 02:16:12PM -0700, Stephane Eranian wrote: > Starting with Arch Perfmon v5, the anythread filter on generic counters may be > deprecated. The current kernel was exporting the any filter without checking. > On Icelake, it means you could do cpu/event=0x3c,any/ even though the filter > does not exist. This patch corrects the problem by relying on the CPUID 0xa > leaf > function to determine if anythread is supported or not as described in the > Intel SDM Vol3b 18.2.5.1 AnyThread Deprecation section. Reviewed-by: Andi Kleen -Andi
[PATCH] perf/x86/intel: make anythread filter support conditional
Starting with Arch Perfmon v5, the anythread filter on generic counters may be
deprecated. The current kernel was exporting the any filter without checking.
On Icelake, it means you could do cpu/event=0x3c,any/ even though the filter
does not exist. This patch corrects the problem by relying on the CPUID 0xa leaf
function to determine if anythread is supported or not as described in the
Intel SDM Vol3b 18.2.5.1 AnyThread Deprecation section.
Signed-off-by: Stephane Eranian
---
arch/x86/events/intel/core.c | 20
arch/x86/events/perf_event.h | 1 +
arch/x86/include/asm/perf_event.h | 4 +++-
arch/x86/kvm/cpuid.c | 4 +++-
4 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index f1926e9f2143..65bf649048a6 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -4220,6 +4220,16 @@ static struct attribute *intel_arch3_formats_attr[] = {
NULL,
};
+static struct attribute *intel_arch5_formats_attr[] = {
+ _attr_event.attr,
+ _attr_umask.attr,
+ _attr_edge.attr,
+ _attr_pc.attr,
+ _attr_inv.attr,
+ _attr_cmask.attr,
+ NULL,
+};
+
static struct attribute *hsw_format_attr[] = {
_attr_in_tx.attr,
_attr_in_tx_cp.attr,
@@ -4987,6 +4997,12 @@ __init int intel_pmu_init(void)
x86_add_quirk(intel_arch_events_quirk); /* Install first, so it runs
last */
+ if (version >= 5) {
+ x86_pmu.intel_cap.anythread_deprecated =
edx.split.anythread_deprecated;
+ if (x86_pmu.intel_cap.anythread_deprecated)
+ pr_cont(" AnyThread deprecated, ");
+ }
+
/*
* Install the hw-cache-events table:
*/
@@ -5512,6 +5528,10 @@ __init int intel_pmu_init(void)
x86_pmu.intel_ctrl |=
((1LL << x86_pmu.num_counters_fixed)-1) << INTEL_PMC_IDX_FIXED;
+ /* AnyThread may be deprecated on arch perfmon v5 or later */
+ if (x86_pmu.intel_cap.anythread_deprecated)
+ x86_pmu.format_attrs = intel_arch5_formats_attr;
+
if (x86_pmu.event_constraints) {
/*
* event on fixed counter2 (REF_CYCLES) only works on this
diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index ee2b9b9fc2a5..906b494083a8 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -585,6 +585,7 @@ union perf_capabilities {
u64 pebs_baseline:1;
u64 perf_metrics:1;
u64 pebs_output_pt_available:1;
+ u64 anythread_deprecated:1;
};
u64 capabilities;
};
diff --git a/arch/x86/include/asm/perf_event.h
b/arch/x86/include/asm/perf_event.h
index 6960cd6d1f23..b9a7fd0a27e2 100644
--- a/arch/x86/include/asm/perf_event.h
+++ b/arch/x86/include/asm/perf_event.h
@@ -137,7 +137,9 @@ union cpuid10_edx {
struct {
unsigned int num_counters_fixed:5;
unsigned int bit_width_fixed:8;
- unsigned int reserved:19;
+ unsigned int reserved1:2;
+ unsigned int anythread_deprecated:1;
+ unsigned int reserved2:16;
} split;
unsigned int full;
};
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 7456f9ad424b..09097d430961 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -636,7 +636,9 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array
*array, u32 function)
edx.split.num_counters_fixed = min(cap.num_counters_fixed,
MAX_FIXED_COUNTERS);
edx.split.bit_width_fixed = cap.bit_width_fixed;
- edx.split.reserved = 0;
+ edx.split.anythread_deprecated = 1;
+ edx.split.reserved1 = 0;
+ edx.split.reserved2 = 0;
entry->eax = eax.full;
entry->ebx = cap.events_mask;
--
2.29.0.rc2.309.g374f81d7ae-goog
