[PATCH 1/2] coresight: etm4x: Report whether the PMU counts external output 1
Amir Ayupov
aaupov at fb.com
Sun Oct 11 22:23:36 PDT 2026
On Thu, 8 Oct 2026 09:38:52 +0100 James Clark <james.clark at linaro.org> wrote:
Hi James,
Please see answers inline:
>
>
> On 07/10/2026 17:24, Leo Yan wrote:
> > Hi Amir,
> >
> > Thanks for the patch. I have a few initial comments below. We may have
> > further feedback after our internal review.
> >
> > On Wed, Sep 30, 2026 at 11:37:43PM -0700, Amir Ayupov wrote:
> >
> > [...]
> >
> >> #define CS_CFG_MATCH_CLASS_SRC_ALL 0x0001 /* match any source */
> >> #define CS_CFG_MATCH_CLASS_SRC_ETM4 0x0002 /* match any ETMv4 device */
> >> +/* ETM external output 1 is countable by the PMU as TRCEXTOUT1 */
> >> +#define CS_CFG_MATCH_CAP_PMU_EXTOUT1 0x0004
> >
> > Do we need to tie this capability to TRCEXTOUT1? For example, Neoverse
> > V2 exposes TRCEXTOUT0 through TRCEXTOUT3 as PMU events.
> >
>
> Shouldn't we also use TRCEXTOUT0 instead of 1? Isn't 1 for devices that
> have two external outputs, but some devices might only have 1 output so
> only have TRCEXTOUT0?
>
IIUC we can't because output 0 is driven by event 0, which stops tracing:
CoreSight Base System Architecture, RMHKBV (second bullet)
> For at least one of the events implemented in the trace unit, when this event occurs, any trace buffer which
> captures trace from this trace unit must be able to detect the event has occurred, and must be able to use this
> event to stop trace capture. One of the following mechanisms must be used:
> • Use of the trigger trace source ID value 0x7D as defined in [12], and use of the ATB Trigger Enable
> function in the trace unit.
> • Use of a dedicated mechanism when Event 0 occurs in the trace unit.
> >> /* flags defining device instance matching - used in config match desc data. */
> >> #define CS_CFG_MATCH_INST_ANY 0x80000000 /* any instance of a class */
> >> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-cfg.c b/drivers/hwtracing/coresight/coresight-etm4x-cfg.c
> >> index e1a59b4345052..2847d2d7f7bed 100644
> >> --- a/drivers/hwtracing/coresight/coresight-etm4x-cfg.c
> >> +++ b/drivers/hwtracing/coresight/coresight-etm4x-cfg.c
> >> @@ -174,9 +174,14 @@ static int etm4_cfg_load_feature(struct coresight_device *csdev,
> >>
> >> int etm4_cscfg_register(struct coresight_device *csdev)
> >> {
> >> + struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> >> struct cscfg_csdev_feat_ops ops;
> >> + u32 match_flags = CS_CFG_ETM4_MATCH_FLAGS;
> >>
> >> ops.load_feat = &etm4_cfg_load_feature;
> >>
> >> - return cscfg_register_csdev(csdev, CS_CFG_ETM4_MATCH_FLAGS, &ops);
> >> + if (drvdata->pmu_extout1)
> >> + match_flags |= CS_CFG_MATCH_CAP_PMU_EXTOUT1;
> >> +
> >> + return cscfg_register_csdev(csdev, match_flags, &ops);
> >
> > The current matching logic succeeds if a device and feature share any
> > flag bit. If the feature sets both CS_CFG_MATCH_CLASS_SRC_ETM4 and
> > CS_CFG_MATCH_CAP_PMU_EXTOUT1, it will still load on an ETM4 device that
> > lacks the capability.
> >
> > Could we check the capability separately when loading the feature,
> > perhaps in cscfg_load_feat_csdev(), and skip it on unsupported devices?
> >
> >> +static bool etm4_pmu_has_extout1(struct etmv4_drvdata *drvdata)
> >> +{
> >> + int pmuver = read_pmuver();
> >> +
> >> + if (!is_midr_in_range_list(etm4_pmu_extout1_cpus))
> >> + return false;
> >
> > Based on specific CPU variant, we should already have identified
> > TRCEXTOUT has supported.
> >
> > So either we only base on MIDR list or we can figure out a reliable
> > way to detect the feature dynamically.
> >
>
> At least with ETE the ARM says:
>
> D4.6.12 External Outputs
>
> SRBKWBThe TRCIDR0.NUMEVENT field shows how many ETEEvents are
> for the particular implementation
>
> 0x4011, TRCEXTOUT1, Trace unit external output 1
>
>
> D14 PMU Event Descriptions
>
> The counter counts each event signaled by the trace unit on external
> event 1.
>
> It is IMPLEMENTATION DEFINED whether this event is available as an
> external input to the ETE.
>
> PMCEID0_EL0[49] reads as 1 if this event is implemented and 0
> otherwise.
>
> The number of outputs and the PMU event are both discoverable. It
> specifically says that only the external input is implementation
> defined, implying that if it's available it's always connected as an output.
>
> We could leave the MIDR list to only support errata when the external
> output isn't connected to the PMU event.
>
>
>
I searched for TRCEXTOUT in errata notices and couldn't find any, so I'm
dropping the list in v2. Feel free to add it in the future.
> >> +
> >> + /* PMCEID0_EL0[63:32] describe events 0x4000-0x401f from PMUv3p1 */
> >> + if (!pmuv3_implemented(pmuver) || pmuver < ID_AA64DFR0_EL1_PMUVer_V3P1)
> >> + return false;
> >> + if (!(read_pmceid0() & BIT_ULL(32 + ARMV8_PMUV3_PERFCTR_TRCEXTOUT1 -
> >> + ARMV8_PMUV3_EXT_COMMON_EVENT_BASE)))
> >> + return false;
> >> +
> >> + /* nr_event is TRCIDR0.NUMEVENT, the number of events minus one */
> >> + return drvdata->nr_event >= 1;
> >> +}
> >
> > [...]
> >
> >> @@ -1116,6 +1116,13 @@ int cscfg_csdev_enable_active_config(struct coresight_device *csdev,
> >>
> >> if (err)
> >> cscfg_config_desc_put(config_desc);
> >> + } else {
> >> + /*
> >> + * The configuration is active but was not loaded on this
> >> + * device, for example because the device lacks a capability
> >> + * its features require. Fail rather than trace without it.
> >> + */
> >> + err = -EINVAL;
> >> }
> >
> > This fixes a pre-existing issue. It is worther to put it in a separate
> > patch with fixes tag:
> >
> > An early return would also avoid the normal enable path's indentation:
> >
> > if (!config_csdev_active)
> > return -EINVAL;
> >
> > err = cscfg_csdev_enable_config(config_csdev_active, preset);
> > ...
> >
> > Thanks,
> > Leo
Sent using hkml (https://github.com/sjp38/hackermail)
More information about the linux-arm-kernel
mailing list