Received-SPF: pass (google.com: domain of linux-kernel+bounces-78539-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
Date: Fri, 23 Feb 2024 14:49:16 +0000
From: Marc Zyngier <maz@kernel.org>
To: Mark Rutland <mark.rutland@arm.com>
Cc: linux-kernel@vger.kernel.org, acme@redhat.com, irogers@google.com,
 james.clark@arm.com, john.g.garry@oracle.com, leo.yan@linaro.org,
 linux-arm-kernel@lists.infradead.org, linux-perf-users@vger.kernel.org,
 marcan@marcan.st, mike.leach@linaro.org, namhyung@kernel.org,
 suzuki.poulose@arm.com, tmricht@linux.ibm.com, will@kernel.org
Subject: Re: [PATCH v2] perf print-events: make is_event_supported() more
 robust
In-Reply-To: <20240126145605.1005472-1-mark.rutland@arm.com>
References: <20240126145605.1005472-1-mark.rutland@arm.com>
User-Agent: Roundcube Webmail/1.4.15
Message-ID: <8bc9dcbab11aa25115d0d278d353188e@kernel.org>
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit

On 2024-01-26 14:56, Mark Rutland wrote:
> Currently the perf tool doesn't detect support for extended event types
> on Apple M1/M2 systems, and will not auto-expand plain PERF_EVENT_TYPE
> hardware events into per-PMU events. This is due to the detection of
> extended event types not handling mandatory filters required by the
> M1/M2 PMU driver.
> 
> PMU drivers and the core perf_events code can require that
> perf_event_attr::exclude_* filters are configured in a specific way and
> may reject certain configurations of filters, for example:
> 
> (a) Many PMUs lack support for any event filtering, and require all
>     perf_event_attr::exclude_* bits to be clear. This includes Alpha's
>     CPU PMU, and ARM CPU PMUs prior to the introduction of PMUv2 in
>     ARMv7,
> 
> (b) When /proc/sys/kernel/perf_event_paranoid >= 2, the perf core
>     requires that perf_event_attr::exclude_kernel is set.
> 
> (c) The Apple M1/M2 PMU requires that perf_event_attr::exclude_guest is
>     set as the hardware PMU does not count while a guest is running 
> (but
>     might be extended in future to do so).
> 
> In is_event_supported(), we try to account for cases (a) and (b), first
> attempting to open an event without any filters, and if this fails,
> retrying with perf_event_attr::exclude_kernel set. We do not account 
> for
> case (c), or any other filters that drivers could theoretically require
> to be set.
> 
> Thus is_event_supported() will fail to detect support for any events
> targeting an Apple M1/M2 PMU, even where events would be supported with
> perf_event_attr:::exclude_guest set.
> 
> Since commit:
> 
>   82fe2e45cdb00de4 ("perf pmus: Check if we can encode the PMU number
> in perf_event_attr.type")
> 
> ... we use is_event_supported() to detect support for extended types,
> with the PMU ID encoded into the perf_event_attr::type. As above, on an
> Apple M1/M2 system this will always fail to detect that the event is
> supported, and consequently we fail to detect support for extended 
> types
> even when these are supported, as they have been since commit:
> 
>   5c816728651ae425 ("arm_pmu: Add PERF_PMU_CAP_EXTENDED_HW_TYPE 
> capability")
> 
> Due to this, the perf tool will not automatically expand plain
> PERF_TYPE_HARDWARE events into per-PMU events, even when all the
> necessary kernel support is present.
> 
> This patch updates is_event_supported() to additionally try opening
> events with perf_event_attr::exclude_guest set, allowing support for
> events to be detected on Apple M1/M2 systems. I believe that this is
> sufficient for all contemporary CPU PMU drivers, though in future it 
> may
> be necessary to check for other combinations of filter bits.
> 
> I've deliberately changed the check to not expect a specific error code
> for missing filters, as today ;the kernel may return a number of
> different error codes for missing filters (e.g. -EACCESS, -EINVAL, or
> -EOPNOTSUPP) depending on why and where the filter configuration is
> rejected, and retrying for any error is more robust.
> 
> Note that this does not remove the need for commit:
> 
>   a24d9d9dc096fc0d ("perf parse-events: Make legacy events lower
> priority than sysfs/JSON")
> 
> ... which is still necessary so that named-pmu/event/ events work on
> kernels without extended type support, even if the event name happens 
> to
> be the same as a PERF_EVENT_TYPE_HARDWARE event (e.g. as is the case 
> for
> the M1/M2 PMU's 'cycles' and 'instructions' events).
> 
> Fixes: 82fe2e45cdb00de4 ("perf pmus: Check if we can encode the PMU
> number in perf_event_attr.type")
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Tested-by: Ian Rogers <irogers@google.com>
> Tested-by: James Clark <james.clark@arm.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Hector Martin <marcan@marcan.st>
> Cc: Ian Rogers <irogers@google.com>
> Cc: James Clark <james.clark@arm.com>
> Cc: John Garry <john.g.garry@oracle.com>
> Cc: Leo Yan <leo.yan@linaro.org>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Mike Leach <mike.leach@linaro.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Thomas Richter <tmricht@linux.ibm.com>
> Cc: Will Deacon <will@kernel.org>

Tested-by: Marc Zyngier <maz@kernel.org>

         M.
-- 
Jazz is not dead. It just smells funny...