Re: [PATCH v3 00/12] perf cs-etm/arm-spe: Remove hard coded config fields

Ian Rogers
2025-12-16T21:00:45Z

On Fri, Dec 12, 2025 at 7:32 AM James Clark <james.clark@linaro.org> wrote:
>
> The specific config field that an event format attribute is in is
> consistently hard coded, even though the API is supposed to be that the
> driver publishes the config field name. To stop this pattern from being
> copy pasted and causing problems in the future, replace them all with
> calls to a new helper that returns the value that a user set.
>
> This reveals some issues in evsel__set_config_if_unset(). It doesn't
> work with sparse bitfields, which are an unused but documented feature.
> And it also only writes to the attr.config field. To fix it we need to
> start tracking user changes for all config fields and then use existing
> helper functions that support sparse bitfields. Some other refactoring
> was also required and a test was added.
>
> Signed-off-by: James Clark <james.clark@linaro.org>

Outside of some nits, for the series:
Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
> Changes in v3:
> - Fix uninitialized variable warning on GCC
> - Fix leak of evlist in test
> - Confirm no type punning issues with ubsan (Ian)
> - Link to v2: https://lore.kernel.org/r/20251208-james-perf-config-bits-v2-0-4ac0281993b0@linaro.org
>
> Changes in v2:
> - Remove macros in get_config_chgs() and some other refactoring.
> - Support sparse bitfields in evsel__set_config_if_unset().
> - Always track user changes instead of only when
> 'pmu->perf_event_attr_init_default' is set.
> - Add a test.
> - Don't bail out in cs-etm.c if any format fields are missing (Leo).
> - Rename 'guess' to 'synth' (Mike).
> - Link to v1: https://lore.kernel.org/r/20251201-james-perf-config-bits-v1-0-22ecbbf8007c@linaro.org
>
> ---
> James Clark (12):
> perf parse-events: Refactor get_config_terms() to remove macros
> perf evsel: Support sparse fields in evsel__set_config_if_unset()
> perf parse-events: Track all user changed config bits
> perf evsel: apply evsel__set_config_if_unset() to all config fields
> perf evsel: Add a helper to get the value of a config field
> perf parse-events: Always track user config changes
> perf tests: Test evsel__set_config_if_unset() and config change tracking
> perf cs-etm: Make a helper to find the Coresight evsel
> perf cs-etm: Don't use hard coded config bits when setting up ETMCR
> perf cs-etm: Don't use hard coded config bits when setting up TRCCONFIGR
> perf cs-etm: Don't hard code config attribute when configuring the event
> perf arm-spe: Don't hard code config attribute
>
> tools/perf/arch/arm/util/cs-etm.c | 193 +++++++++++++++------------
> tools/perf/arch/arm64/util/arm-spe.c | 15 ++-
> tools/perf/tests/pmu.c | 91 +++++++++++++
> tools/perf/util/evsel.c | 6 +-
> tools/perf/util/evsel.h | 2 +
> tools/perf/util/evsel_config.h | 7 +-
> tools/perf/util/parse-events.c | 248 ++++++++++++++++++++---------------
> tools/perf/util/pmu.c | 112 ++++++++++++++--
> 8 files changed, 460 insertions(+), 214 deletions(-)
> ---
> base-commit: 2eeb09fe1c5173b659929f92fee4461796ca8c14
> change-id: 20251112-james-perf-config-bits-bee7106f0f00
>
> Best regards,
> --
> James Clark <james.clark@linaro.org>
>