[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJ9a7VgQO0_FuZj2CD0yAW10xefJNmqRC-0DxJTG9FJu6h-o6A@mail.gmail.com>
Date: Thu, 20 Nov 2025 11:21:36 +0000
From: Mike Leach <mike.leach@...aro.org>
To: James Clark <james.clark@...aro.org>
Cc: Suzuki K Poulose <suzuki.poulose@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jonathan Corbet <corbet@....net>,
Leo Yan <leo.yan@....com>, Randy Dunlap <rdunlap@...radead.org>, coresight@...ts.linaro.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-doc@...r.kernel.org
Subject: Re: [PATCH v5 10/13] coresight: Remove misleading definitions
On Tue, 18 Nov 2025 at 16:28, James Clark <james.clark@...aro.org> wrote:
>
> ETM_OPT_* definitions duplicate the PMU format attributes that have
> always been published in sysfs. Hardcoding them here makes it misleading
> as to what the 'real' PMU API is and prevents attributes from being
> rearranged in the future.
>
> ETM4_CFG_BIT_* definitions just define what the Arm Architecture is
> which is not the responsibility of the kernel to do and doesn't scale to
> other registers or versions of ETM. It's not an actual software ABI/API
> and these definitions here mislead that it is.
>
> Any tools using the first ones would be broken anyway as they won't work
> when attributes are moved, so removing them is the right thing to do and
> will prompt a fix. Tools using the second ones can trivially redefine
> them locally.
>
> Perf also has its own copy of the headers so both of these things can be
> fixed up at a later date.
>
The perf version is used to reconstruct the control registers for etm3
/ etm4 to put into the trace metadata headers in the perf.data file
for the decoder to be intitialised correctly.
perf_event_attr::config uses the ETM_OPT_* values, used directly for
etm3 since they match the bit pattern in the etm3/ptm config register,
and remapped for etm4 from ETM_OPT_* to equivalent ETM4_CFG_BIT* for
the etm4 config register.
The reason we do this re-construction - rather than read registers as
we do for the other metadata - is that the register value is not set
at the point we are recording the metadata - it does not actually get
set until the etm is enabled, later in the perf process.
On this basis it would seem that any changing of the attribute bit
ordering has potential to break perf decode. Probably safe to remove
from this version of the file though, but overall, changing bit
meanings in the underlying variable may possibly need a fix in perf
and potentially a breaking change for earlier versions of the tools.
That said, for this particular version of the header, since it appears
to no longer need the values due to earlier changes.
Reviewed-by: Mike Leach <mike.leach@...aro.org>
> Reviewed-by: Leo Yan <leo.yan@....com>
> Signed-off-by: James Clark <james.clark@...aro.org>
> ---
> include/linux/coresight-pmu.h | 24 ------------------------
> 1 file changed, 24 deletions(-)
>
> diff --git a/include/linux/coresight-pmu.h b/include/linux/coresight-pmu.h
> index 89b0ac0014b0..2e179abe472a 100644
> --- a/include/linux/coresight-pmu.h
> +++ b/include/linux/coresight-pmu.h
> @@ -21,30 +21,6 @@
> */
> #define CORESIGHT_LEGACY_CPU_TRACE_ID(cpu) (0x10 + (cpu * 2))
>
> -/*
> - * Below are the definition of bit offsets for perf option, and works as
> - * arbitrary values for all ETM versions.
> - *
> - * Most of them are orignally from ETMv3.5/PTM's ETMCR config, therefore,
> - * ETMv3.5/PTM doesn't define ETMCR config bits with prefix "ETM3_" and
> - * directly use below macros as config bits.
> - */
> -#define ETM_OPT_BRANCH_BROADCAST 8
> -#define ETM_OPT_CYCACC 12
> -#define ETM_OPT_CTXTID 14
> -#define ETM_OPT_CTXTID2 15
> -#define ETM_OPT_TS 28
> -#define ETM_OPT_RETSTK 29
> -
> -/* ETMv4 CONFIGR programming bits for the ETM OPTs */
> -#define ETM4_CFG_BIT_BB 3
> -#define ETM4_CFG_BIT_CYCACC 4
> -#define ETM4_CFG_BIT_CTXTID 6
> -#define ETM4_CFG_BIT_VMID 7
> -#define ETM4_CFG_BIT_TS 11
> -#define ETM4_CFG_BIT_RETSTK 12
> -#define ETM4_CFG_BIT_VMID_OPT 15
> -
> /*
> * Interpretation of the PERF_RECORD_AUX_OUTPUT_HW_ID payload.
> * Used to associate a CPU with the CoreSight Trace ID.
>
> --
> 2.34.1
>
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
Powered by blists - more mailing lists