linux-kernel - Re: [PATCH] perf/x86/intel: make anythread filter support conditional

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201022080003.GR2628@hirez.programming.kicks-ass.net>
Date:   Thu, 22 Oct 2020 10:00:03 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Stephane Eranian <eranian@...gle.com>
Cc:     linux-kernel@...r.kernel.org, ak@...ux.intel.com,
        kan.liang@...el.com, jolsa@...hat.com, mingo@...e.hu,
        irogers@...gle.com, namhyung@...nel.org
Subject: 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 <eranian@...gle.com>
> ---
>  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[] = {
> +	&format_attr_event.attr,
> +	&format_attr_umask.attr,
> +	&format_attr_edge.attr,
> +	&format_attr_pc.attr,
> +	&format_attr_inv.attr,
> +	&format_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