CoreSight November 2026

coresight@lists.linaro.org

3 participants
27 discussions

Re: [PATCH] Revert "perf/aux: Allocate non-contiguous AUX pages by default"

by Leo Yan

On Tue, Nov 03, 2026 at 12:46:24PM -0800, Yabin Cui wrote: ... > In field telemetry and local ftrace on Pixel devices (prior to > 18049c8cff9c, so __GFP_NORETRY was already in place), we observed heavy > rb_alloc_aux() -> __alloc_pages_direct_compact() stalls alongside > kcompactd0 pinned on big cores scanning order-9, and this accounted for > >20% of all perceptible app kills (via lmkd) across dogfood devices > until we stopped high-order AUX allocations. Thanks for the detailed explanation. My understanding is that repeated reclaim/compaction during AUX allocation raises memory PSI and can lead to lmkd killing applications. The problem to address is therefore the cost of obtaining higher-order pages under memory fragmentation. > Initially we worked around this in userspace (simpleperf) by setting > perf_event_attr::aux_watermark = 4096 so get_order(watermark) == 0. That > was fine for CoreSight ETR (which uses its own separate scatter-gather > buffer and only copies into AUX on flush/stop), but when enabling TRBE > (and SPE) it caused hardware maintenance interrupts every 4 KiB instead > of every 2 MiB. Also, in overwrite/snapshot mode rb_alloc_aux() ignores > aux_watermark and uses ilog2(nr_pages). That is why I sent 18049c8cff9c. Agreed. Reducing watermark to 4096 also increases maintenance interrupt frequency for SPE/TRBE. We should refine buffer allocation rather than using watermark as a workaround. > ### Possible solutions > > For PMUs without PERF_PMU_CAP_AUX_NO_SG (such as SPE, TRBE, and modern > Intel PT), higher-order AUX pages are an opportunistic optimization > rather than a functional requirement -- very similar to THP allocations > on fault (GFP_TRANSHUGE_LIGHT) or opportunistic high-order slabs in SLUB > (allocate_slab()). > > Instead of reverting to GFP_KERNEL reclaim/compaction for all high-order > attempts, could we do the following? > > 1. Make opportunistic high-order AUX allocations no-reclaim: > When !(event->pmu->capabilities & PERF_PMU_CAP_AUX_NO_SG) and > order > 0, clear __GFP_RECLAIM (and add __GFP_NOMEMALLOC, matching > SLUB's allocate_slab() and GFP_TRANSHUGE_LIGHT): > > gfp_t gfp = PERF_AUX_GFP; > > if (order && !has_no_sg) > gfp = (PERF_AUX_GFP | __GFP_NOMEMALLOC) & ~__GFP_RECLAIM; > > When order falls back to 0 (or when PERF_PMU_CAP_AUX_NO_SG is set), > keep PERF_AUX_GFP so single-page allocations still reclaim normally > under memory pressure. > > That way, when free order-9 / order-4 blocks exist, SPE and TRBE get > them immediately for PMD / contiguous-PTE vmap() mappings (and pages > stay grouped together); when memory is fragmented on a long-running > device, high-order probes fail fast on the freelist without waking > kcompactd/kswapd or triggering direct compaction and reclaim. This looks reasonable for me: try to obtain larger blocks cheaply, then then retain the PERF_AUX_GFP policy for order-0 allocations. I also agree with preserving the allocation policy for PERF_PMU_CAP_AUX_NO_SG case. Could you compare the memory PSI stall deltas before and after applying this option? I think measurements from your production devices would be convinced evidence for benefits. > 2. Clamp max_order in rb_alloc_aux() after fallback: > If rb_alloc_aux_page() requested `order` and had to fall back to a > smaller order, update `max_order = page_private(page)` in > rb_alloc_aux() so subsequent iterations for the rest of the buffer do > not keep re-probing higher orders that just failed. This also can avoid reclaim repeatedly, but the side effect is subsequent iterations would no longer try high order, which have no chance to benefit from large mapping. I would prefer to evaluate option 1 first. > (Optionally, if server/benchmark workloads ever want synchronous > compaction to form huge AUX mappings even on fragmented hosts, that could > be gated behind an opt-in attr/sysfs knob similar to THP defrag modes, > with light/no-reclaim as the default.) I will follow up with Will separately about the userspace interface. > Would this approach work for your SPE/TRBE use cases? Happy to help test > on our side. If option 1 works, we can confidently revert 18049c8cff9c, as it would restore higher-order allocations for TRBE/SPE without introducing the regression you mentioned. Thanks, Leo

3 days, 19 hours

Re: [PATCH v16 4/7] qcom-tgu: Add TGU decode support

by Suzuki K Poulose

On 30/07/2026 09:07, Songwei Chai wrote: > From: Songwei Chai <songchai(a)qti.qualcomm.com> > > Decoding is when all the potential pieces for creating a trigger > are brought together for a given step. Example - there may be a > counter keeping track of some occurrences and a priority-group that > is being used to detect a pattern on the sense inputs. These 2 > inputs to condition_decode must be programmed, for a given step, > to establish the condition for the trigger, or movement to another > steps. > > Reviewed-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > Signed-off-by: Songwei Chai <songchai(a)qti.qualcomm.com> > --- > .../ABI/testing/sysfs-bus-amba-devices-tgu | 7 + > drivers/hwtracing/qcom/tgu.c | 182 ++++++++++++++++-- > drivers/hwtracing/qcom/tgu.h | 37 ++++ > 3 files changed, 206 insertions(+), 20 deletions(-) > > diff --git a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > index 7dd24314592d..705e75c776d4 100644 > --- a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > +++ b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > @@ -14,3 +14,10 @@ KernelVersion: 7.2 > Contact: Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>, Songwei Chai <songwei.chai(a)oss.qualcomm.com> > Description: > (RW) Set/Get the sensed signal with specific step and priority for TGU. > + > +What: /sys/bus/amba/devices/<tgu-name>/step[0:7]_condition_decode/reg[0:3] > +Date: August 2026 > +KernelVersion: 7.2 7.5 for the next version , everywhere, please. > +Contact: Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>, Songwei Chai <songwei.chai(a)oss.qualcomm.com> > +Description: > + (RW) Set/Get the decode mode with specific step for TGU. > diff --git a/drivers/hwtracing/qcom/tgu.c b/drivers/hwtracing/qcom/tgu.c > index 0e88b437f191..e83295217e7a 100644 > --- a/drivers/hwtracing/qcom/tgu.c > +++ b/drivers/hwtracing/qcom/tgu.c > @@ -19,8 +19,33 @@ static int calculate_array_location(struct tgu_drvdata *drvdata, > int step_index, int operation_index, > int reg_index) > { > - return operation_index * (drvdata->num_step) * (drvdata->num_reg) + > - step_index * (drvdata->num_reg) + reg_index; > + switch (operation_index) { > + case TGU_PRIORITY0: > + case TGU_PRIORITY1: > + case TGU_PRIORITY2: > + case TGU_PRIORITY3: > + return operation_index * (drvdata->num_step) * > + (drvdata->num_reg) + > + step_index * (drvdata->num_reg) + reg_index; > + case TGU_CONDITION_DECODE: > + return step_index * (drvdata->num_condition_decode) + > + reg_index; > + default: > + break; > + } > + > + return -EINVAL; > +} > + > +static int check_array_location(struct tgu_drvdata *drvdata, int step, > + int ops, int reg) > +{ > + int result = calculate_array_location(drvdata, step, ops, reg); > + > + if (result == -EINVAL) > + dev_err(drvdata->dev, "check array location - Fail\n"); > + > + return result; > } > > static ssize_t tgu_dataset_show(struct device *dev, > @@ -31,9 +56,11 @@ static ssize_t tgu_dataset_show(struct device *dev, > container_of(attr, struct tgu_attribute, attr); > int index; > > - index = calculate_array_location(drvdata, tgu_attr->step_index, > - tgu_attr->operation_index, > - tgu_attr->reg_num); > + index = check_array_location(drvdata, tgu_attr->step_index, > + tgu_attr->operation_index, tgu_attr->reg_num); > + > + if (index == -EINVAL) > + return index; > > /* > * Serialise against tgu_dataset_store() and tgu_enable(), which read > @@ -42,8 +69,20 @@ static ssize_t tgu_dataset_show(struct device *dev, > */ > guard(mutex)(&drvdata->lock); > > - return sysfs_emit(buf, "0x%x\n", > - drvdata->value_table->priority[index]); > + switch (tgu_attr->operation_index) { > + case TGU_PRIORITY0: > + case TGU_PRIORITY1: > + case TGU_PRIORITY2: > + case TGU_PRIORITY3: > + return sysfs_emit(buf, "0x%x\n", > + drvdata->value_table->priority[index]); > + case TGU_CONDITION_DECODE: > + return sysfs_emit(buf, "0x%x\n", > + drvdata->value_table->condition_decode[index]); > + default: > + break; > + } > + return -EINVAL; > } > > static ssize_t tgu_dataset_store(struct device *dev, > @@ -66,9 +105,27 @@ static ssize_t tgu_dataset_store(struct device *dev, > tgu_attr->operation_index, > tgu_attr->reg_num); > > - tgu_drvdata->value_table->priority[index] = val; > + if (index == -EINVAL) > + return index; > + > + switch (tgu_attr->operation_index) { > + case TGU_PRIORITY0: > + case TGU_PRIORITY1: > + case TGU_PRIORITY2: > + case TGU_PRIORITY3: > + tgu_drvdata->value_table->priority[index] = val; > + ret = size; > + break; > + case TGU_CONDITION_DECODE: > + tgu_drvdata->value_table->condition_decode[index] = val; > + ret = size; > + break; > + default: > + ret = -EINVAL; > + break; > + } > > - return size; > + return ret; > } > > static umode_t tgu_node_visible(struct kobject *kobject, > @@ -85,22 +142,41 @@ static umode_t tgu_node_visible(struct kobject *kobject, > if (tgu_attr->step_index >= drvdata->num_step) > return SYSFS_GROUP_INVISIBLE; > > - if (tgu_attr->reg_num >= drvdata->num_reg) > - return 0; > + switch (tgu_attr->operation_index) { > + case TGU_PRIORITY0: > + case TGU_PRIORITY1: > + case TGU_PRIORITY2: > + case TGU_PRIORITY3: > + if (!drvdata->num_reg) > + return SYSFS_GROUP_INVISIBLE; > + if (tgu_attr->reg_num < drvdata->num_reg) > + return attr->mode; > + break; > + case TGU_CONDITION_DECODE: > + if (!drvdata->num_condition_decode) > + return SYSFS_GROUP_INVISIBLE; > + if (tgu_attr->reg_num < drvdata->num_condition_decode) > + return attr->mode; > + break; > + default: > + break; > + } > > - return attr->mode; > + return 0; > } > > -static void tgu_write_all_hw_regs(struct tgu_drvdata *drvdata) > +static ssize_t tgu_write_all_hw_regs(struct tgu_drvdata *drvdata) > { > - int i, j, k, index; > + int i, j, k, index = 0; > > TGU_UNLOCK(drvdata->base); > for (i = 0; i < drvdata->num_step; i++) { > for (j = 0; j < MAX_PRIORITY; j++) { > for (k = 0; k < drvdata->num_reg; k++) { > - index = calculate_array_location( > + index = check_array_location( > drvdata, i, j, k); > + if (index == -EINVAL) > + goto exit; > > writel(drvdata->value_table->priority[index], > drvdata->base + > @@ -108,9 +184,23 @@ static void tgu_write_all_hw_regs(struct tgu_drvdata *drvdata) > } > } > } > + > + for (i = 0; i < drvdata->num_step; i++) { > + for (j = 0; j < drvdata->num_condition_decode; j++) { > + index = check_array_location(drvdata, i, > + TGU_CONDITION_DECODE, j); > + if (index == -EINVAL) > + goto exit; > + > + writel(drvdata->value_table->condition_decode[index], > + drvdata->base + CONDITION_DECODE_STEP(i, j)); > + } > + } > /* Enable TGU to program the triggers */ > writel(1, drvdata->base + TGU_CONTROL); > +exit: > TGU_LOCK(drvdata->base); > + return index >= 0 ? 0 : -EINVAL; > } > > static void tgu_set_reg_number(struct tgu_drvdata *drvdata) > @@ -153,10 +243,31 @@ static void tgu_set_steps(struct tgu_drvdata *drvdata) > } > } > > -static void tgu_enable(struct tgu_drvdata *drvdata) > +static int tgu_enable(struct tgu_drvdata *drvdata) > { > - tgu_write_all_hw_regs(drvdata); > + int ret; > + > + ret = tgu_write_all_hw_regs(drvdata); > + if (ret) > + return ret; > + > drvdata->enabled = true; > + return 0; > +} > + > +static void tgu_set_conditions(struct tgu_drvdata *drvdata) > +{ > + u32 devid; > + > + devid = readl(drvdata->base + TGU_DEVID); > + drvdata->num_condition_decode = TGU_DEVID_CONDITIONS(devid); > + > + if (drvdata->num_condition_decode > TGU_MAX_CONDITION_DECODE) { > + dev_warn(drvdata->dev, > + "DEVID reports %d condition decode regs, clamping to %d\n", > + drvdata->num_condition_decode, TGU_MAX_CONDITION_DECODE); > + drvdata->num_condition_decode = TGU_MAX_CONDITION_DECODE; > + } > } > > static void tgu_disable(struct tgu_drvdata *drvdata) > @@ -210,7 +321,11 @@ static ssize_t enable_tgu_store(struct device *dev, > if (ret) > return ret; > > - tgu_enable(drvdata); > + ret = tgu_enable(drvdata); > + if (ret) { > + pm_runtime_put(dev); > + return ret; > + } > } else { > if (!drvdata->enabled) > return -EINVAL; > @@ -266,6 +381,14 @@ static const struct attribute_group *tgu_attr_groups[] = { > PRIORITY_ATTRIBUTE_GROUP_INIT(7, 1), > PRIORITY_ATTRIBUTE_GROUP_INIT(7, 2), > PRIORITY_ATTRIBUTE_GROUP_INIT(7, 3), > + CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(0), > + CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(1), > + CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(2), > + CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(3), > + CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(4), > + CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(5), > + CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(6), > + CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(7), > NULL, > }; > > @@ -273,8 +396,9 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id) > { > struct device *dev = &adev->dev; > struct tgu_drvdata *drvdata; > - unsigned int *priority; > - size_t priority_size; > + unsigned int *priority, *condition; > + size_t priority_size, condition_size; > + int i, j; > int ret; > > drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); > @@ -294,6 +418,7 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id) > > tgu_set_reg_number(drvdata); > tgu_set_steps(drvdata); > + tgu_set_conditions(drvdata); > > drvdata->value_table = > devm_kzalloc(dev, sizeof(*drvdata->value_table), GFP_KERNEL); > @@ -310,6 +435,23 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id) > > drvdata->value_table->priority = priority; > > + condition_size = drvdata->num_condition_decode * drvdata->num_step; > + > + condition = devm_kcalloc(dev, condition_size, > + sizeof(*(drvdata->value_table->condition_decode)), > + GFP_KERNEL); > + if (!condition) > + return -ENOMEM; > + > + drvdata->value_table->condition_decode = condition; > + > + /* Initialize all condition decode registers to NOT */ > + for (i = 0; i < drvdata->num_step; i++) > + for (j = 0; j < drvdata->num_condition_decode; j++) > + condition[calculate_array_location( > + drvdata, i, TGU_CONDITION_DECODE, j)] = > + TGU_CONDITION_DECODE_NOT; > + > drvdata->enabled = false; > /* > * Create the sysfs groups only after the backing storage is fully > diff --git a/drivers/hwtracing/qcom/tgu.h b/drivers/hwtracing/qcom/tgu.h > index 7a3e1161c4cf..8b6a46995a5f 100644 > --- a/drivers/hwtracing/qcom/tgu.h > +++ b/drivers/hwtracing/qcom/tgu.h > @@ -19,6 +19,8 @@ > ((int)FIELD_GET(GENMASK(17, 10), devid_val)) > #define TGU_DEVID_STEPS(devid_val) \ > ((int)FIELD_GET(GENMASK(6, 3), devid_val)) > +#define TGU_DEVID_CONDITIONS(devid_val) \ > + ((int)FIELD_GET(GENMASK(2, 0), devid_val)) > #define TGU_BITS_PER_SIGNAL 4 > #define LENGTH_REGISTER 32 > > @@ -52,6 +54,7 @@ > */ > #define STEP_OFFSET 0x1D8 > #define PRIORITY_START_OFFSET 0x0074 > +#define CONDITION_DECODE_OFFSET 0x0050 > #define PRIORITY_OFFSET 0x60 > #define REG_OFFSET 0x4 > > @@ -63,6 +66,7 @@ > * > * TGU_MAX_STEPS static tables define step0..step7 > * TGU_MAX_PRIORITY_REGS STEP_PRIORITY_LIST defines reg0..reg17 > + * TGU_MAX_CONDITION_DECODE decode region 0x50..0x5F holds 4 u32 registers > * > * A device whose DEVID reports counts above these bounds is clamped, so writes > * never fall outside the ioremap()'d window or spill into an adjacent region. > @@ -70,12 +74,16 @@ > > #define TGU_MAX_STEPS 8 > #define TGU_MAX_PRIORITY_REGS 18 > +#define TGU_MAX_CONDITION_DECODE 4 > > /* Calculate compare step addresses */ > #define PRIORITY_REG_STEP(step, priority, reg)\ > (PRIORITY_START_OFFSET + PRIORITY_OFFSET * priority +\ > REG_OFFSET * reg + STEP_OFFSET * step) > > +#define CONDITION_DECODE_STEP(step, decode) \ > + (CONDITION_DECODE_OFFSET + REG_OFFSET * decode + STEP_OFFSET * step) > + > #define tgu_dataset_rw(name, step_index, type, reg_num) \ > (&((struct tgu_attribute[]){ { \ > __ATTR(name, 0644, tgu_dataset_show, tgu_dataset_store), \ > @@ -87,6 +95,8 @@ > #define STEP_PRIORITY(step_index, reg_num, priority) \ > tgu_dataset_rw(reg##reg_num, step_index, TGU_PRIORITY##priority, \ > reg_num) > +#define STEP_DECODE(step_index, reg_num) \ > + tgu_dataset_rw(reg##reg_num, step_index, TGU_CONDITION_DECODE, reg_num) > > #define STEP_PRIORITY_LIST(step_index, priority) \ > {STEP_PRIORITY(step_index, 0, priority), \ > @@ -110,6 +120,14 @@ > NULL \ > } > > +#define STEP_DECODE_LIST(n) \ > + {STEP_DECODE(n, 0), \ Please use a new line for all such defintions. > + STEP_DECODE(n, 1), \ > + STEP_DECODE(n, 2), \ > + STEP_DECODE(n, 3), \ > + NULL \ > + } > + > #define PRIORITY_ATTRIBUTE_GROUP_INIT(step, priority)\ > (&(const struct attribute_group){\ > .attrs = (struct attribute*[])STEP_PRIORITY_LIST(step, priority),\ > @@ -117,16 +135,32 @@ > .name = "step" #step "_priority" #priority \ > }) > > +#define CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(step)\ > + (&(const struct attribute_group){\ > + .attrs = (struct attribute*[])STEP_DECODE_LIST(step),\ > + .is_visible = tgu_node_visible,\ > + .name = "step" #step "_condition_decode" \ > + }) > + > enum operation_index { > TGU_PRIORITY0, > TGU_PRIORITY1, > TGU_PRIORITY2, > TGU_PRIORITY3, > + TGU_CONDITION_DECODE, > }; > > /* Maximum priority that TGU supports */ > #define MAX_PRIORITY 4 > > +/* > + * Neutral ('NOT') value for the condition decode registers. This is the > + * driver's default so that, before userspace programs a real decode, an > + * enabled step does not spuriously match. Used both when seeding the shadow > + * table at probe and when clearing it via reset_tgu. > + */ > +#define TGU_CONDITION_DECODE_NOT 0x1000000 > + > struct tgu_attribute { > struct device_attribute attr; > u32 step_index; > @@ -136,6 +170,7 @@ struct tgu_attribute { > > struct value_table { > unsigned int *priority; > + unsigned int *condition_decode; > }; > > static inline void TGU_LOCK(void __iomem *addr) > @@ -166,6 +201,7 @@ static inline void TGU_UNLOCK(void __iomem *addr) > * @value_table: Store given value based on relevant parameters > * @num_reg: Maximum number of registers > * @num_step: Maximum step size > + * @num_condition_decode: Maximum number of condition_decode > * > * This structure defines the data associated with a TGU device, > * including its base address, device pointer, the lock serialising > @@ -179,6 +215,7 @@ struct tgu_drvdata { > struct value_table *value_table; > int num_reg; > int num_step; > + int num_condition_decode; > }; > > #endif

3 days, 22 hours

Re: [PATCH v4 5/7] perf: arm_pmuv3: Add pmuv3_event_implemented()

by Suzuki K Poulose

On 26/10/2026 05:48, Amir Ayupov wrote: > Drivers outside the PMU driver may need to know whether the local CPU's > PMU implements a common event before relying on it, for example a trace > unit driver checking that the PMU counts its external outputs as > TRCEXTOUTn. > > Add pmuv3_event_implemented() to the arm64 asm/arm_pmuv3.h, next to > read_pmceid0() and read_pmceid1(). It reads PMCEID0/1 on the calling CPU: > events 0x0000-0x003f are reported in bits [31:0], and the common events > from ARMV8_PMUV3_EXT_COMMON_EVENT_BASE in bits [63:32] from PMUv3p1. Move > ARMV8_PMUV3_EXT_COMMON_EVENT_BASE next to the event numbers in > linux/perf/arm_pmuv3.h so it is available without CONFIG_ARM_PMU. > > Suggested-by: James Clark <james.clark(a)linaro.org> > Assisted-by: Devmate:Opus-5.5 > Signed-off-by: Amir Ayupov <aaupov(a)fb.com> > --- > arch/arm64/include/asm/arm_pmuv3.h | 19 +++++++++++++++++++ > include/linux/perf/arm_pmu.h | 1 - > include/linux/perf/arm_pmuv3.h | 3 +++ > 3 files changed, 22 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/include/asm/arm_pmuv3.h b/arch/arm64/include/asm/arm_pmuv3.h > index 8a777dec8d88a..acd62c4f16743 100644 > --- a/arch/arm64/include/asm/arm_pmuv3.h > +++ b/arch/arm64/include/asm/arm_pmuv3.h > @@ -173,6 +173,25 @@ static inline bool pmuv3_implemented(int pmuver) > pmuver == ID_AA64DFR0_EL1_PMUVer_NI); > } > > +/* > + * Whether this CPU's PMU implements a common event, as reported by > + * PMCEID{0,1}: events 0x0000-0x003f in bits [31:0], and from PMUv3p1 events > + * 0x4000-0x403f in bits [63:32], which read as zero before PMUv3p1. > + * Must run on the CPU whose PMU is queried. > + */ > +static inline bool pmuv3_event_implemented(u16 event) > +{ > + bool ext = event >= ARMV8_PMUV3_EXT_COMMON_EVENT_BASE; > + u16 id = ext ? event - ARMV8_PMUV3_EXT_COMMON_EVENT_BASE : event; > + u64 pmceid; > + > + if (id >= 64 || !pmuv3_implemented(read_pmuver())) > + return false; > + > + pmceid = id < 32 ? :() : read_pmceid1(); > + return pmceid & BIT_ULL(id % 32 + (ext ? 32 : 0)); > +} > + > static inline bool is_pmuv3p4(int pmuver) > { > return pmuver >= ID_AA64DFR0_EL1_PMUVer_V3P4; > diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h > index 02d2c7f45b527..792283d29d7b5 100644 > --- a/include/linux/perf/arm_pmu.h > +++ b/include/linux/perf/arm_pmu.h > @@ -124,7 +124,6 @@ struct arm_pmu { > u64 reg_brbidr; > #define ARMV8_PMUV3_MAX_COMMON_EVENTS 0x40 > DECLARE_BITMAP(pmceid_bitmap, ARMV8_PMUV3_MAX_COMMON_EVENTS); > -#define ARMV8_PMUV3_EXT_COMMON_EVENT_BASE 0x4000 > DECLARE_BITMAP(pmceid_ext_bitmap, ARMV8_PMUV3_MAX_COMMON_EVENTS); Could we not avoid this change ? Or why not move both of the definitions ? Also, I would need an ACK from the arm_pmu maintainers to accept this change via coresight tree. Cheers Suzuki > > /* Only to be used by ACPI probing code */ > diff --git a/include/linux/perf/arm_pmuv3.h b/include/linux/perf/arm_pmuv3.h > index d698efba28a27..c21f185a283d6 100644 > --- a/include/linux/perf/arm_pmuv3.h > +++ b/include/linux/perf/arm_pmuv3.h > @@ -78,6 +78,9 @@ > #define ARMV8_PMUV3_PERFCTR_STALL_SLOT_FRONTEND 0x003E > #define ARMV8_PMUV3_PERFCTR_STALL_SLOT 0x003F > > +/* Base of the common events reported in PMCEID{0,1}[63:32] from PMUv3p1 */ > +#define ARMV8_PMUV3_EXT_COMMON_EVENT_BASE 0x4000 > + > /* Statistical profiling extension microarchitectural events */ > #define ARMV8_SPE_PERFCTR_SAMPLE_POP 0x4000 > #define ARMV8_SPE_PERFCTR_SAMPLE_FEED 0x4001

3 days, 22 hours

Re: [PATCH v16 7/7] qcom-tgu: Add reset node to initialize

by Suzuki K Poulose

On 30/07/2026 09:07, Songwei Chai wrote: > Add reset node to initialize the value of > priority/condition_decode/condition_select/timer/counter nodes. > > Signed-off-by: Songwei Chai <songwei.chai(a)oss.qualcomm.com> > Reviewed-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > --- > .../ABI/testing/sysfs-bus-amba-devices-tgu | 7 ++ > drivers/hwtracing/qcom/tgu.c | 84 +++++++++++++++++++ > 2 files changed, 91 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > index a0cad2f16310..5c928a5fa033 100644 > --- a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > +++ b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > @@ -51,3 +51,10 @@ KernelVersion: 7.2 > Contact: Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>, Songwei Chai <songwei.chai(a)oss.qualcomm.com> > Description: > (RW) Set/Get the counter value with specific step for TGU. > + > +What: /sys/bus/amba/devices/<tgu-name>/reset_tgu > +Date: August 2026 > +KernelVersion: 7.2 > +Contact: Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>, Songwei Chai <songwei.chai(a)oss.qualcomm.com> > +Description: > + (Write) Write 1 to reset the dataset for TGU. > diff --git a/drivers/hwtracing/qcom/tgu.c b/drivers/hwtracing/qcom/tgu.c > index ea83273a628c..d65a68faa741 100644 > --- a/drivers/hwtracing/qcom/tgu.c > +++ b/drivers/hwtracing/qcom/tgu.c > @@ -228,6 +228,18 @@ static ssize_t tgu_write_all_hw_regs(struct tgu_drvdata *drvdata) > int i, j, k, index = 0; > > TGU_UNLOCK(drvdata->base); > + > + /* > + * Quiesce the detection engine before mutating any comparison input. > + * This function is also reached from reset_tgu_store() while the TGU is > + * already running (TGU_CONTROL == 1); rewriting the priority/condition/ > + * timer/counter registers register-by-register on a live engine could > + * feed a transient, half-updated match into CTI/interrupt consumers. > + * Programming with CONTROL == 0 and only re-enabling at the end keeps the > + * program-then-enable ordering for every caller. > + */ > + writel(0, drvdata->base + TGU_CONTROL); > + > for (i = 0; i < drvdata->num_step; i++) { > for (j = 0; j < MAX_PRIORITY; j++) { > for (k = 0; k < drvdata->num_reg; k++) { > @@ -459,8 +471,80 @@ static ssize_t enable_tgu_store(struct device *dev, > } > static DEVICE_ATTR_RW(enable_tgu); > > +static ssize_t reset_tgu_store(struct device *dev, > + struct device_attribute *attr, const char *buf, > + size_t size) > +{ > + struct tgu_drvdata *drvdata = dev_get_drvdata(dev); > + struct value_table *vt = drvdata->value_table; > + unsigned long value; > + int i, j, ret; > + > + if (kstrtoul(buf, 0, &value) || value != 1) > + return -EINVAL; > + > + /* > + * Hold the mutex across the whole sequence so it cannot race with > + * enable_tgu_store()/tgu_dataset_store(): the enabled check and the > + * hardware access stay consistent. No PM reference is taken or dropped > + * here - the shadow tables are plain memory, and the hardware is only > + * touched when the device is already resumed via the enable reference. > + */ > + guard(mutex)(&drvdata->lock); > + > + if (vt->priority) { > + size_t bytes = MAX_PRIORITY * drvdata->num_step * > + drvdata->num_reg * sizeof(unsigned int); Please could you add a macro to compute "number" of each array ? Something like : #define TGU_PRIORITY_SIZE(drvdata) (MAX_PRIORITY * (drvdata)->num_step * (drvdata)->num_reg) So that we use the same size calculation for allocation and reset ? You may scale it by sizeof(*vt->priority) > + memset(vt->priority, 0, bytes); > + } > + > + if (vt->condition_decode) { > + size_t bytes = drvdata->num_condition_decode * > + drvdata->num_step * sizeof(unsigned int); Same as above > + memset(vt->condition_decode, 0, bytes); > + > + /* Initialize all condition decode registers to NOT (0x1000000) */ then why do the memset above ? > + for (i = 0; i < drvdata->num_step; i++) > + for (j = 0; j < drvdata->num_condition_decode; j++) > + vt->condition_decode[calculate_array_location( > + drvdata, i, TGU_CONDITION_DECODE, j)] = > + TGU_CONDITION_DECODE_NOT; Then, we could simplify the above to: for (i = 0; i < TGU_CONDITION_DECODE(drvdata); i++) vt->condition_decode[i] = TGU_CONDITION_DECODE_NOT; Same for everything below. Suzuki > + } > + > + if (vt->condition_select) { > + size_t bytes = drvdata->num_condition_select * > + drvdata->num_step * sizeof(unsigned int); > + memset(vt->condition_select, 0, bytes); > + } > + > + if (vt->timer) { > + size_t bytes = drvdata->num_step * drvdata->num_timer * > + sizeof(unsigned int); > + memset(vt->timer, 0, bytes); > + } > + > + if (vt->counter) { > + size_t bytes = drvdata->num_step * drvdata->num_counter * > + sizeof(unsigned int); > + memset(vt->counter, 0, bytes); > + } > + > + /* If the TGU is running, apply the cleared configuration immediately. */ > + if (drvdata->enabled) { > + ret = tgu_write_all_hw_regs(drvdata); > + if (ret) > + return ret; > + } > + > + dev_dbg(dev, "Qualcomm-TGU reset complete\n"); > + > + return size; > +} > +static DEVICE_ATTR_WO(reset_tgu); > + > static struct attribute *tgu_common_attrs[] = { > &dev_attr_enable_tgu.attr, > + &dev_attr_reset_tgu.attr, > NULL, > }; >

3 days, 22 hours

Re: [PATCH v16 3/7] qcom-tgu: Add signal priority support

by Suzuki K Poulose

On 30/07/2026 09:07, Songwei Chai wrote: > Like circuit of a Logic analyzer, in TGU, the requirement could be > configured in each step and the trigger will be created once the > requirements are met. Add priority functionality here to sort the > signals into different priorities. The signal which is wanted could > be configured in each step's priority node, the larger number means > the higher priority and the signal with higher priority will be sensed > more preferentially. > > Reviewed-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > Signed-off-by: Songwei Chai <songwei.chai(a)oss.qualcomm.com> > --- > .../ABI/testing/sysfs-bus-amba-devices-tgu | 7 + > drivers/hwtracing/qcom/tgu.c | 189 +++++++++++++++++- > drivers/hwtracing/qcom/tgu.h | 133 ++++++++++++ > 3 files changed, 328 insertions(+), 1 deletion(-) > > diff --git a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > index 36ab64f1e301..7dd24314592d 100644 > --- a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > +++ b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > @@ -7,3 +7,10 @@ Description: > Accepts only one of the 2 values - 0 or 1. > 0 : disable TGU. > 1 : enable TGU. > + > +What: /sys/bus/amba/devices/<tgu-name>/step[0:7]_priority[0:3]/reg[0:17] > +Date: August 2026 > +KernelVersion: 7.2 > +Contact: Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>, Songwei Chai <songwei.chai(a)oss.qualcomm.com> > +Description: > + (RW) Set/Get the sensed signal with specific step and priority for TGU. > diff --git a/drivers/hwtracing/qcom/tgu.c b/drivers/hwtracing/qcom/tgu.c > index 9c7dd4a09f02..0e88b437f191 100644 > --- a/drivers/hwtracing/qcom/tgu.c > +++ b/drivers/hwtracing/qcom/tgu.c > @@ -15,14 +15,144 @@ > > #include "tgu.h" > > +static int calculate_array_location(struct tgu_drvdata *drvdata, > + int step_index, int operation_index, > + int reg_index) > +{ > + return operation_index * (drvdata->num_step) * (drvdata->num_reg) + > + step_index * (drvdata->num_reg) + reg_index; > +} > + > +static ssize_t tgu_dataset_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct tgu_drvdata *drvdata = dev_get_drvdata(dev); > + struct tgu_attribute *tgu_attr = > + container_of(attr, struct tgu_attribute, attr); > + int index; > + > + index = calculate_array_location(drvdata, tgu_attr->step_index, > + tgu_attr->operation_index, > + tgu_attr->reg_num); > + > + /* > + * Serialise against tgu_dataset_store() and tgu_enable(), which read > + * and write the same priority array, so the value is not observed > + * while being updated concurrently. > + */ > + guard(mutex)(&drvdata->lock); > + > + return sysfs_emit(buf, "0x%x\n", > + drvdata->value_table->priority[index]); > +} > + > +static ssize_t tgu_dataset_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t size) > +{ > + struct tgu_drvdata *tgu_drvdata = dev_get_drvdata(dev); > + struct tgu_attribute *tgu_attr = > + container_of(attr, struct tgu_attribute, attr); > + unsigned long val; > + int index; > + int ret; > + > + ret = kstrtoul(buf, 0, &val); > + if (ret) > + return ret; > + > + guard(mutex)(&tgu_drvdata->lock); > + index = calculate_array_location(tgu_drvdata, tgu_attr->step_index, > + tgu_attr->operation_index, > + tgu_attr->reg_num); > + > + tgu_drvdata->value_table->priority[index] = val; > + > + return size; > +} > + > +static umode_t tgu_node_visible(struct kobject *kobject, > + struct attribute *attr, > + int n) > +{ > + struct device *dev = kobj_to_dev(kobject); > + struct tgu_drvdata *drvdata = dev_get_drvdata(dev); > + struct device_attribute *dev_attr = > + container_of(attr, struct device_attribute, attr); > + struct tgu_attribute *tgu_attr = > + container_of(dev_attr, struct tgu_attribute, attr); > + > + if (tgu_attr->step_index >= drvdata->num_step) > + return SYSFS_GROUP_INVISIBLE; > + > + if (tgu_attr->reg_num >= drvdata->num_reg) > + return 0; > + > + return attr->mode; > +} > + > static void tgu_write_all_hw_regs(struct tgu_drvdata *drvdata) > { > + int i, j, k, index; > + > TGU_UNLOCK(drvdata->base); > + for (i = 0; i < drvdata->num_step; i++) { > + for (j = 0; j < MAX_PRIORITY; j++) { > + for (k = 0; k < drvdata->num_reg; k++) { > + index = calculate_array_location( > + drvdata, i, j, k); > + > + writel(drvdata->value_table->priority[index], > + drvdata->base + > + PRIORITY_REG_STEP(i, j, k)); > + } > + } > + } > /* Enable TGU to program the triggers */ > writel(1, drvdata->base + TGU_CONTROL); > TGU_LOCK(drvdata->base); > } > > +static void tgu_set_reg_number(struct tgu_drvdata *drvdata) > +{ > + int num_sense_input; > + int num_reg; > + u32 devid; > + > + devid = readl(drvdata->base + TGU_DEVID); > + > + num_sense_input = TGU_DEVID_SENSE_INPUT(devid); > + num_reg = (num_sense_input * TGU_BITS_PER_SIGNAL) / LENGTH_REGISTER; > + > + if ((num_sense_input * TGU_BITS_PER_SIGNAL) % LENGTH_REGISTER) > + num_reg++; > + > + if (num_reg > TGU_MAX_PRIORITY_REGS) { > + dev_warn(drvdata->dev, > + "DEVID reports %d priority regs, clamping to %d\n", > + num_reg, TGU_MAX_PRIORITY_REGS); > + num_reg = TGU_MAX_PRIORITY_REGS; > + } > + > + drvdata->num_reg = num_reg; > +} > + > +static void tgu_set_steps(struct tgu_drvdata *drvdata) > +{ > + u32 devid; > + > + devid = readl(drvdata->base + TGU_DEVID); > + > + drvdata->num_step = TGU_DEVID_STEPS(devid); > + > + if (drvdata->num_step > TGU_MAX_STEPS) { > + dev_warn(drvdata->dev, > + "DEVID reports %d steps, clamping to %d\n", > + drvdata->num_step, TGU_MAX_STEPS); > + drvdata->num_step = TGU_MAX_STEPS; > + } > +} Why not combine setting the num_step and num_regs in one go ? You are reading the DEVID anyways, and such macro level functional split is useless. > + > static void tgu_enable(struct tgu_drvdata *drvdata) > { > tgu_write_all_hw_regs(drvdata); > @@ -104,6 +234,38 @@ static const struct attribute_group tgu_common_grp = { > > static const struct attribute_group *tgu_attr_groups[] = { > &tgu_common_grp, > + PRIORITY_ATTRIBUTE_GROUP_INIT(0, 0), > + PRIORITY_ATTRIBUTE_GROUP_INIT(0, 1), > + PRIORITY_ATTRIBUTE_GROUP_INIT(0, 2), > + PRIORITY_ATTRIBUTE_GROUP_INIT(0, 3), > + PRIORITY_ATTRIBUTE_GROUP_INIT(1, 0), > + PRIORITY_ATTRIBUTE_GROUP_INIT(1, 1), > + PRIORITY_ATTRIBUTE_GROUP_INIT(1, 2), > + PRIORITY_ATTRIBUTE_GROUP_INIT(1, 3), > + PRIORITY_ATTRIBUTE_GROUP_INIT(2, 0), > + PRIORITY_ATTRIBUTE_GROUP_INIT(2, 1), > + PRIORITY_ATTRIBUTE_GROUP_INIT(2, 2), > + PRIORITY_ATTRIBUTE_GROUP_INIT(2, 3), > + PRIORITY_ATTRIBUTE_GROUP_INIT(3, 0), > + PRIORITY_ATTRIBUTE_GROUP_INIT(3, 1), > + PRIORITY_ATTRIBUTE_GROUP_INIT(3, 2), > + PRIORITY_ATTRIBUTE_GROUP_INIT(3, 3), > + PRIORITY_ATTRIBUTE_GROUP_INIT(4, 0), > + PRIORITY_ATTRIBUTE_GROUP_INIT(4, 1), > + PRIORITY_ATTRIBUTE_GROUP_INIT(4, 2), > + PRIORITY_ATTRIBUTE_GROUP_INIT(4, 3), > + PRIORITY_ATTRIBUTE_GROUP_INIT(5, 0), > + PRIORITY_ATTRIBUTE_GROUP_INIT(5, 1), > + PRIORITY_ATTRIBUTE_GROUP_INIT(5, 2), > + PRIORITY_ATTRIBUTE_GROUP_INIT(5, 3), > + PRIORITY_ATTRIBUTE_GROUP_INIT(6, 0), > + PRIORITY_ATTRIBUTE_GROUP_INIT(6, 1), > + PRIORITY_ATTRIBUTE_GROUP_INIT(6, 2), > + PRIORITY_ATTRIBUTE_GROUP_INIT(6, 3), > + PRIORITY_ATTRIBUTE_GROUP_INIT(7, 0), > + PRIORITY_ATTRIBUTE_GROUP_INIT(7, 1), > + PRIORITY_ATTRIBUTE_GROUP_INIT(7, 2), > + PRIORITY_ATTRIBUTE_GROUP_INIT(7, 3), > NULL, > }; > > @@ -111,6 +273,8 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id) > { > struct device *dev = &adev->dev; > struct tgu_drvdata *drvdata; > + unsigned int *priority; > + size_t priority_size; > int ret; > > drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); > @@ -128,8 +292,31 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id) > if (ret) > return ret; > > - drvdata->enabled = false; > + tgu_set_reg_number(drvdata); > + tgu_set_steps(drvdata); As mentioned above, combine both ? > + > + drvdata->value_table = > + devm_kzalloc(dev, sizeof(*drvdata->value_table), GFP_KERNEL); Could we not embed this in the drvdata without dynamic allocation ? > + if (!drvdata->value_table) > + return -ENOMEM; > > + priority_size = MAX_PRIORITY * drvdata->num_reg * drvdata->num_step; > + > + priority = devm_kcalloc(dev, priority_size, > + sizeof(*drvdata->value_table->priority), > + GFP_KERNEL); > + if (!priority) > + return -ENOMEM; > + > + drvdata->value_table->priority = priority; > + > + drvdata->enabled = false; > + /* > + * Create the sysfs groups only after the backing storage is fully > + * allocated, otherwise userspace could open a priority attribute and > + * have tgu_dataset_show() dereference an uninitialised value_table. > + * This also keeps the error paths above free of sysfs teardown. > + */ > ret = sysfs_create_groups(&dev->kobj, tgu_attr_groups); > if (ret) { > dev_err(dev, "failed to create sysfs groups: %d\n", ret); > diff --git a/drivers/hwtracing/qcom/tgu.h b/drivers/hwtracing/qcom/tgu.h > index ebba49402a20..7a3e1161c4cf 100644 > --- a/drivers/hwtracing/qcom/tgu.h > +++ b/drivers/hwtracing/qcom/tgu.h > @@ -6,10 +6,137 @@ > #ifndef _QCOM_TGU_H > #define _QCOM_TGU_H > > +#include <linux/bitfield.h> > +#include <linux/bits.h> > + > /* Register addresses */ > #define TGU_CONTROL 0x0000 > #define TGU_LAR 0xfb0 > #define TGU_UNLOCK_OFFSET 0xc5acce55 > +#define TGU_DEVID 0xfc8 > + > +#define TGU_DEVID_SENSE_INPUT(devid_val) \ > + ((int)FIELD_GET(GENMASK(17, 10), devid_val)) > +#define TGU_DEVID_STEPS(devid_val) \ > + ((int)FIELD_GET(GENMASK(6, 3), devid_val)) > +#define TGU_BITS_PER_SIGNAL 4 > +#define LENGTH_REGISTER 32 > + > +/* > + * TGU configuration space Step configuration > + * offset table space layout > + * x-------------------------x$ x-------------x$ > + * | |$ | |$ > + * | | | reserve |$ > + * | | | |$ > + * |coresight management | |-------------|base+n*0x1D8+0x1F4$ > + * | registers | |---> |priority[3] |$ > + * | | | |-------------|base+n*0x1D8+0x194$ > + * | | | |priority[2] |$ > + * |-------------------------| | |-------------|base+n*0x1D8+0x134$ > + * | | | |priority[1] |$ > + * | step[7] | | |-------------|base+n*0x1D8+0xD4$ > + * |-------------------------|->base+0x40+7*0x1D8 | |priority[0] |$ > + * | | | |-------------|base+n*0x1D8+0x74$ > + * | ... | | | condition |$ > + * | | | | select |$ > + * |-------------------------|->base+0x40+1*0x1D8 | |-------------|base+n*0x1D8+0x60$ minor nit: ^^^ s/base+0x40+1*0x1D8/step[0] + 0x1D8 and so on, for the above > + * | | | | condition |$ > + * | step[0] |--------------------> | decode |$ > + * |-------------------------|-> base+0x40 |-------------|base+n*0x1D8+0x50$ > + * | | | |$ > + * | Control and status space| |Timer/Counter|$ > + * | space | | |$ > + * x-------------------------x->base x-------------x base+n*0x1D8+0x40$ Also for each frame, you don't have to describe "base+n*..." that makes it unnecessarily crowded and complicated to read. > + * > + */ > +#define STEP_OFFSET 0x1D8 > +#define PRIORITY_START_OFFSET 0x0074 > +#define PRIORITY_OFFSET 0x60 From the diagram, it is CONDITION_SELECT ? Having two offsets named after PRIORITY is confusing. > +#define REG_OFFSET 0x4 > + > +/* > + * Upper bounds on the DEVID/DEVID2-derived per-region counts. The TGU register > + * layout is fixed by the offset macros above and mirrored by the static sysfs > + * attribute tables in tgu.c, so these are the largest counts the driver can > + * both address within the mapped window and expose to userspace: > + * > + * TGU_MAX_STEPS static tables define step0..step7 > + * TGU_MAX_PRIORITY_REGS STEP_PRIORITY_LIST defines reg0..reg17 What are these ? PRIORITY regs is 0-3 ? Do you mean each priority has 0-17 registers ? (See, the description using base+n*.. for the "step" frame is making this worse). You may move it down as a separate figure. From my maths: For pritority[0] -> 0xD4-0x74 = 0x60 => 0x18 => 24 registers! not 18! What am I missing ? > + * > + * A device whose DEVID reports counts above these bounds is clamped, so writes > + * never fall outside the ioremap()'d window or spill into an adjacent region. > + */ > + > +#define TGU_MAX_STEPS 8 > +#define TGU_MAX_PRIORITY_REGS 18 > + > +/* Calculate compare step addresses */ > +#define PRIORITY_REG_STEP(step, priority, reg)\ > + (PRIORITY_START_OFFSET + PRIORITY_OFFSET * priority +\ > + REG_OFFSET * reg + STEP_OFFSET * step) > + > +#define tgu_dataset_rw(name, step_index, type, reg_num) \ > + (&((struct tgu_attribute[]){ { \ > + __ATTR(name, 0644, tgu_dataset_show, tgu_dataset_store), \ > + step_index, \ > + type, \ > + reg_num, \ > + } })[0].attr.attr) > + > +#define STEP_PRIORITY(step_index, reg_num, priority) \ > + tgu_dataset_rw(reg##reg_num, step_index, TGU_PRIORITY##priority, \ > + reg_num) > + > +#define STEP_PRIORITY_LIST(step_index, priority) \ > + {STEP_PRIORITY(step_index, 0, priority), \ > + STEP_PRIORITY(step_index, 1, priority), \ > + STEP_PRIORITY(step_index, 2, priority), \ > + STEP_PRIORITY(step_index, 3, priority), \ > + STEP_PRIORITY(step_index, 4, priority), \ > + STEP_PRIORITY(step_index, 5, priority), \ > + STEP_PRIORITY(step_index, 6, priority), \ > + STEP_PRIORITY(step_index, 7, priority), \ > + STEP_PRIORITY(step_index, 8, priority), \ > + STEP_PRIORITY(step_index, 9, priority), \ > + STEP_PRIORITY(step_index, 10, priority), \ > + STEP_PRIORITY(step_index, 11, priority), \ > + STEP_PRIORITY(step_index, 12, priority), \ > + STEP_PRIORITY(step_index, 13, priority), \ > + STEP_PRIORITY(step_index, 14, priority), \ > + STEP_PRIORITY(step_index, 15, priority), \ > + STEP_PRIORITY(step_index, 16, priority), \ > + STEP_PRIORITY(step_index, 17, priority), \ > + NULL \ > + } > + > +#define PRIORITY_ATTRIBUTE_GROUP_INIT(step, priority)\ > + (&(const struct attribute_group){\ > + .attrs = (struct attribute*[])STEP_PRIORITY_LIST(step, priority),\ > + .is_visible = tgu_node_visible,\ > + .name = "step" #step "_priority" #priority \ > + }) Please could you move these macro magic to the .c file ? 1. Nobody else reuses them ? 2. It is really difficult to go back and forth to check what each macro means, given how complex it is. > + > +enum operation_index { why are we calling it operation_index ? > + TGU_PRIORITY0, > + TGU_PRIORITY1, > + TGU_PRIORITY2, > + TGU_PRIORITY3, > +}; > + > +/* Maximum priority that TGU supports */ > +#define MAX_PRIORITY 4 > + > +struct tgu_attribute { > + struct device_attribute attr; > + u32 step_index; > + enum operation_index operation_index; u8 priority ? > + u32 reg_num; u8 reg_num ? > +}; > + > +struct value_table { > + unsigned int *priority; > +}; > > static inline void TGU_LOCK(void __iomem *addr) > { > @@ -36,6 +163,9 @@ static inline void TGU_UNLOCK(void __iomem *addr) > * @lock: Mutex serialising enable/disable and the associated PM runtime > * reference counting against concurrent sysfs access > * @enabled: Flag indicating whether the TGU device is enabled > + * @value_table: Store given value based on relevant parameters > + * @num_reg: Maximum number of registers > + * @num_step: Maximum step size > * > * This structure defines the data associated with a TGU device, > * including its base address, device pointer, the lock serialising > @@ -46,6 +176,9 @@ struct tgu_drvdata { > struct device *dev; > struct mutex lock; > bool enabled; > + struct value_table *value_table; Why this ? why not struct value_table value_table ? Surely, we seem to be dynamically allocating the priorities etc from there. But do we need dynamic allocation for value_table field as such ? Suzuki > + int num_reg; > + int num_step; > }; > > #endif

3 days, 22 hours

Re: [PATCH v16 2/7] qcom-tgu: Add TGU driver

by Suzuki K Poulose

On 30/07/2026 09:06, Songwei Chai wrote: > Add driver to support device TGU (Trigger Generation Unit). > TGU is a Data Engine which can be utilized to sense a plurality of > signals and create a trigger into the CTI or generate interrupts to > processors. Add probe/enable/disable functions for tgu. > > Signed-off-by: Songwei Chai <songwei.chai(a)oss.qualcomm.com> > Acked-by: Konrad Dybcio <konrad.dybcio(a)oss.qualcomm.com> > Reviewed-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > --- > .../ABI/testing/sysfs-bus-amba-devices-tgu | 9 + > MAINTAINERS | 11 + > drivers/Makefile | 1 + > drivers/hwtracing/Kconfig | 2 + > drivers/hwtracing/qcom/Kconfig | 20 ++ > drivers/hwtracing/qcom/Makefile | 3 + > drivers/hwtracing/qcom/tgu.c | 192 ++++++++++++++++++ > drivers/hwtracing/qcom/tgu.h | 51 +++++ > 8 files changed, 289 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > create mode 100644 drivers/hwtracing/qcom/Kconfig > create mode 100644 drivers/hwtracing/qcom/Makefile > create mode 100644 drivers/hwtracing/qcom/tgu.c > create mode 100644 drivers/hwtracing/qcom/tgu.h > > diff --git a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > new file mode 100644 > index 000000000000..36ab64f1e301 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > @@ -0,0 +1,9 @@ > +What: /sys/bus/amba/devices/<tgu-name>/enable_tgu > +Date: August 2026 > +KernelVersion: 7.2 > +Contact: Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>, Songwei Chai <songwei.chai(a)oss.qualcomm.com> > +Description: > + (RW) Set/Get the enable/disable status of TGU > + Accepts only one of the 2 values - 0 or 1. > + 0 : disable TGU. > + 1 : enable TGU. > diff --git a/MAINTAINERS b/MAINTAINERS > index ce5c5185c773..4789321cca6d 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -22360,6 +22360,17 @@ S: Maintained > F: Documentation/tee/qtee.rst > F: drivers/tee/qcomtee/ > > +QUALCOMM HARDWARETRACING DRIVER > +M: Songwei Chai <songwei.chai(a)oss.qualcomm.com> > +M: Jie Gan <jie.gan(a)oss.qualcomm.com> > +M: Suzuki K Poulose <suzuki.poulose(a)arm.com> > +L: linux-arm-msm(a)vger.kernel.org > +S: Maintained > +T: git git://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git > +F: Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > +F: Documentation/devicetree/bindings/arm/qcom,tgu.yaml > +F: drivers/hwtracing/qcom/ > + > QUALCOMM TRUST ZONE MEMORY ALLOCATOR > M: Bartosz Golaszewski <brgl(a)kernel.org> > L: linux-arm-msm(a)vger.kernel.org > diff --git a/drivers/Makefile b/drivers/Makefile > index 0841ea851847..6a2cbe5b340f 100644 > --- a/drivers/Makefile > +++ b/drivers/Makefile > @@ -176,6 +176,7 @@ obj-$(CONFIG_RAS) += ras/ > obj-$(CONFIG_USB4) += thunderbolt/ > obj-$(CONFIG_CORESIGHT) += hwtracing/coresight/ > obj-y += hwtracing/intel_th/ > +obj-y += hwtracing/qcom/ > obj-$(CONFIG_STM) += hwtracing/stm/ > obj-$(CONFIG_HISI_PTT) += hwtracing/ptt/ > obj-y += android/ > diff --git a/drivers/hwtracing/Kconfig b/drivers/hwtracing/Kconfig > index 911ee977103c..8a640218eed8 100644 > --- a/drivers/hwtracing/Kconfig > +++ b/drivers/hwtracing/Kconfig > @@ -7,4 +7,6 @@ source "drivers/hwtracing/intel_th/Kconfig" > > source "drivers/hwtracing/ptt/Kconfig" > > +source "drivers/hwtracing/qcom/Kconfig" > + > endmenu > diff --git a/drivers/hwtracing/qcom/Kconfig b/drivers/hwtracing/qcom/Kconfig > new file mode 100644 > index 000000000000..5c94c75ffa39 > --- /dev/null > +++ b/drivers/hwtracing/qcom/Kconfig > @@ -0,0 +1,20 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +# > +# QCOM specific hwtracing drivers > +# > +menu "Qualcomm specific hwtracing drivers" > + > +config QCOM_TGU > + tristate "QCOM Trigger Generation Unit driver" > + depends on ARCH_QCOM || COMPILE_TEST > + depends on ARM_AMBA > + help > + This driver provides support for Trigger Generation Unit that is > + used to detect patterns or sequences on a given set of signals. > + TGU is used to monitor a particular bus within a given region to > + detect illegal transaction sequences or slave responses. It is also > + used to monitor a data stream to detect protocol violations and to > + provide a trigger point for centering data around a specific event > + within the trace data buffer. > + > +endmenu > diff --git a/drivers/hwtracing/qcom/Makefile b/drivers/hwtracing/qcom/Makefile > new file mode 100644 > index 000000000000..5a0a868c1ea0 > --- /dev/null > +++ b/drivers/hwtracing/qcom/Makefile > @@ -0,0 +1,3 @@ > +# SPDX-License-Identifier: GPL-2.0 > + > +obj-$(CONFIG_QCOM_TGU) += tgu.o > diff --git a/drivers/hwtracing/qcom/tgu.c b/drivers/hwtracing/qcom/tgu.c > new file mode 100644 > index 000000000000..9c7dd4a09f02 > --- /dev/null > +++ b/drivers/hwtracing/qcom/tgu.c > @@ -0,0 +1,192 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. > + */ > + > +#include <linux/amba/bus.h> > +#include <linux/device.h> > +#include <linux/err.h> > +#include <linux/io.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/of.h> > +#include <linux/pm_runtime.h> > + > +#include "tgu.h" > + > +static void tgu_write_all_hw_regs(struct tgu_drvdata *drvdata) > +{ > + TGU_UNLOCK(drvdata->base); > + /* Enable TGU to program the triggers */ > + writel(1, drvdata->base + TGU_CONTROL); > + TGU_LOCK(drvdata->base); > +} > + > +static void tgu_enable(struct tgu_drvdata *drvdata) > +{ > + tgu_write_all_hw_regs(drvdata); > + drvdata->enabled = true; > +} > + > +static void tgu_disable(struct tgu_drvdata *drvdata) > +{ > + TGU_UNLOCK(drvdata->base); > + writel(0, drvdata->base + TGU_CONTROL); > + TGU_LOCK(drvdata->base); > + > + drvdata->enabled = false; > +} > + > +static ssize_t enable_tgu_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct tgu_drvdata *drvdata = dev_get_drvdata(dev); > + bool enabled; > + > + scoped_guard(mutex, &drvdata->lock) > + enabled = drvdata->enabled; > + > + return sysfs_emit(buf, "%d\n", enabled); > +} > + > +/* enable_tgu_store - Enable or disable the Trigger Generation Unit (TGU). */ > +static ssize_t enable_tgu_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, > + size_t size) > +{ > + struct tgu_drvdata *drvdata = dev_get_drvdata(dev); > + unsigned long val; > + int ret; > + > + ret = kstrtoul(buf, 0, &val); > + if (ret || val > 1) > + return -EINVAL; > + > + /* > + * Hold the mutex across the whole check-and-act sequence: the check of > + * drvdata->enabled, the PM runtime reference counting and the state > + * update must be atomic, otherwise concurrent writers could each > + * imbalance the PM runtime usage counter. > + */ > + guard(mutex)(&drvdata->lock); > + > + if (val) { > + if (drvdata->enabled) > + return -EBUSY; > + > + ret = pm_runtime_resume_and_get(dev); > + if (ret) > + return ret; > + > + tgu_enable(drvdata); > + } else { > + if (!drvdata->enabled) > + return -EINVAL; > + > + tgu_disable(drvdata); > + pm_runtime_put(dev); > + } > + > + return size; > +} > +static DEVICE_ATTR_RW(enable_tgu); > + > +static struct attribute *tgu_common_attrs[] = { > + &dev_attr_enable_tgu.attr, > + NULL, > +}; > + > +static const struct attribute_group tgu_common_grp = { > + .attrs = tgu_common_attrs, > +}; > + > +static const struct attribute_group *tgu_attr_groups[] = { > + &tgu_common_grp, > + NULL, > +}; > + > +static int tgu_probe(struct amba_device *adev, const struct amba_id *id) > +{ > + struct device *dev = &adev->dev; > + struct tgu_drvdata *drvdata; > + int ret; > + > + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); > + if (!drvdata) > + return -ENOMEM; > + > + drvdata->dev = &adev->dev; > + dev_set_drvdata(dev, drvdata); > + > + drvdata->base = devm_ioremap_resource(dev, &adev->res); > + if (IS_ERR(drvdata->base)) > + return PTR_ERR(drvdata->base); > + > + ret = devm_mutex_init(dev, &drvdata->lock); > + if (ret) > + return ret; > + > + drvdata->enabled = false; > + > + ret = sysfs_create_groups(&dev->kobj, tgu_attr_groups); > + if (ret) { > + dev_err(dev, "failed to create sysfs groups: %d\n", ret); > + return ret; > + } > + > + pm_runtime_put(&adev->dev); > + > + return 0; > +} > + > +static void tgu_remove(struct amba_device *adev) > +{ > + struct device *dev = &adev->dev; > + struct tgu_drvdata *drvdata = dev_get_drvdata(dev); > + > + sysfs_remove_groups(&dev->kobj, tgu_attr_groups); > + > + /* > + * If the device is still enabled through sysfs, disable the hardware > + * and drop the PM runtime reference taken in enable_tgu_store() so the > + * usage counter stays balanced. Use the synchronous put: the driver core > + * disables runtime PM right after ->remove() returns, and the barrier in > + * pm_runtime_disable() would cancel a still-queued async idle/suspend, > + * leaving the runtime-PM state inconsistent. > + */ > + scoped_guard(mutex, &drvdata->lock) { > + if (drvdata->enabled) { > + tgu_disable(drvdata); > + pm_runtime_put_sync(dev); > + } > + } > +} > + > +static const struct amba_id tgu_ids[] = { > + { > + .id = 0x000f0e00, > + .mask = 0x000fffff, > + }, > + { 0, 0, NULL }, > +}; > + > +MODULE_DEVICE_TABLE(amba, tgu_ids); > + > +static struct amba_driver tgu_driver = { > + .drv = { > + .name = "qcom-tgu", > + .suppress_bind_attrs = true, > + }, > + .probe = tgu_probe, > + .remove = tgu_remove, > + .id_table = tgu_ids, > +}; > + > +module_amba_driver(tgu_driver); > + > +MODULE_AUTHOR("Songwei Chai <songwei.chai(a)oss.qualcomm.com>"); > +MODULE_AUTHOR("Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>"); > +MODULE_DESCRIPTION("Qualcomm Trigger Generation Unit driver"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/hwtracing/qcom/tgu.h b/drivers/hwtracing/qcom/tgu.h > new file mode 100644 > index 000000000000..ebba49402a20 > --- /dev/null > +++ b/drivers/hwtracing/qcom/tgu.h > @@ -0,0 +1,51 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. > + */ > + > +#ifndef _QCOM_TGU_H > +#define _QCOM_TGU_H > + > +/* Register addresses */ > +#define TGU_CONTROL 0x0000 > +#define TGU_LAR 0xfb0 > +#define TGU_UNLOCK_OFFSET 0xc5acce55 > + > +static inline void TGU_LOCK(void __iomem *addr) > +{ > + do { > + /* Wait for things to settle */ > + mb(); > + writel_relaxed(0x0, addr + TGU_LAR); > + } while (0); > +} This is simply replicating the CS_LOCK/CS_UNLOCK. I don't mind those (and relevant definitions) to be moved into include/linux/coresight.h and reused for your driver. The rest looks fine by me. Suzuki > + > +static inline void TGU_UNLOCK(void __iomem *addr) > +{ > + do { > + writel_relaxed(TGU_UNLOCK_OFFSET, addr + TGU_LAR); > + /* Make sure everyone has seen this */ > + mb(); > + } while (0); > +} > + > +/** > + * struct tgu_drvdata - Data structure for a TGU (Trigger Generation Unit) > + * @base: Memory-mapped base address of the TGU device > + * @dev: Pointer to the associated device structure > + * @lock: Mutex serialising enable/disable and the associated PM runtime > + * reference counting against concurrent sysfs access > + * @enabled: Flag indicating whether the TGU device is enabled > + * > + * This structure defines the data associated with a TGU device, > + * including its base address, device pointer, the lock serialising > + * enable/disable, and the enable status. > + */ > +struct tgu_drvdata { > + void __iomem *base; > + struct device *dev; > + struct mutex lock; > + bool enabled; > +}; > + > +#endif

4 days

Re: [PATCH v4 3/7] coresight: syscfg: Match feature capabilities separately from device class

by Suzuki K Poulose

On 26/10/2026 05:48, Amir Ayupov wrote: > A feature loads on a device when the two share any match flag. That works > for device classes, where a feature wants any one of them, but not for > capabilities a device may or may not have: a feature that sets a class > and a capability would still load on a device of that class without the > capability. > > Split the match flags into CS_CFG_MATCH_CLASS_MASK for device classes and > CS_CFG_MATCH_CAP_MASK for capabilities. A feature now loads on a device > that shares one of its classes and has all of its capabilities. Check > this once in cscfg_load_feat_csdev(), which both the feature and the > device registration paths call. No capability flags are defined yet, so > matching is unchanged. > > Assisted-by: Devmate:Opus-5.5 > Signed-off-by: Amir Ayupov <aaupov(a)fb.com> > --- > .../hwtracing/coresight/coresight-config.h | 8 ++++ > .../hwtracing/coresight/coresight-syscfg.c | 37 +++++++++++++------ > 2 files changed, 34 insertions(+), 11 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-config.h b/drivers/hwtracing/coresight/coresight-config.h > index ae6295e308131..f04e73b91d9a6 100644 > --- a/drivers/hwtracing/coresight/coresight-config.h > +++ b/drivers/hwtracing/coresight/coresight-config.h > @@ -7,6 +7,7 @@ > #ifndef _CORESIGHT_CORESIGHT_CONFIG_H > #define _CORESIGHT_CORESIGHT_CONFIG_H > > +#include <linux/bits.h> > #include <linux/cleanup.h> > #include <linux/coresight.h> > #include <linux/types.h> > @@ -30,6 +31,13 @@ > */ > #define CS_CFG_MATCH_CLASS_SRC_ALL 0x0001 /* match any source */ > #define CS_CFG_MATCH_CLASS_SRC_ETM4 0x0002 /* match any ETMv4 device */ > +#define CS_CFG_MATCH_CLASS_MASK GENMASK_U32(7, 0) > + > +/* > + * flags defining device capabilities a feature can require. A feature loads on > + * a device that matches one of its classes and has all of its capabilities. > + */ > +#define CS_CFG_MATCH_CAP_MASK GENMASK_U32(15, 8) > > /* 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-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c > index a816c22c87bc8..3ee104e94a4cd 100644 > --- a/drivers/hwtracing/coresight/coresight-syscfg.c > +++ b/drivers/hwtracing/coresight/coresight-syscfg.c > @@ -171,8 +171,22 @@ cscfg_alloc_csdev_feat(struct coresight_device *csdev, struct cscfg_feature_desc > return feat_csdev; > } > > -/* load one feature into one coresight device */ > +static bool cscfg_match_feat_csdev(u32 feat_flags, u32 csdev_flags) > +{ > + /* The device must be in one of the feature's classes */ > + if (!(feat_flags & csdev_flags & CS_CFG_MATCH_CLASS_MASK)) > + return false; > + > + /* The device must have every capability the feature requires */ > + if (feat_flags & ~csdev_flags & CS_CFG_MATCH_CAP_MASK) > + return false; Please could you address the following comment ? https://lore.kernel.org/linux-arm-kernel/82adee03-a169-4106-99f2-7624a49260… Cheers Suzuki > + > + return true; > +} > + > +/* Load the feature into the device if it matches, otherwise skip it. */ > static int cscfg_load_feat_csdev(struct coresight_device *csdev, > + u32 match_flags, > struct cscfg_feature_desc *feat_desc, > struct cscfg_csdev_feat_ops *ops) > { > @@ -183,6 +197,9 @@ static int cscfg_load_feat_csdev(struct coresight_device *csdev, > if (!ops->load_feat) > return -EINVAL; > > + if (!cscfg_match_feat_csdev(feat_desc->match_flags, match_flags)) > + return 0; > + > feat_csdev = cscfg_alloc_csdev_feat(csdev, feat_desc); > if (!feat_csdev) > return -ENOMEM; > @@ -212,11 +229,11 @@ static int cscfg_add_feat_to_csdevs(struct cscfg_feature_desc *feat_desc) > int err; > > list_for_each_entry(csdev_item, &cscfg_mgr->csdev_desc_list, item) { > - if (csdev_item->match_flags & feat_desc->match_flags) { > - err = cscfg_load_feat_csdev(csdev_item->csdev, feat_desc, &csdev_item->ops); > - if (err) > - return err; > - } > + err = cscfg_load_feat_csdev(csdev_item->csdev, > + csdev_item->match_flags, feat_desc, > + &csdev_item->ops); > + if (err) > + return err; > } > return 0; > } > @@ -739,11 +756,9 @@ static int cscfg_add_feats_csdev(struct coresight_device *csdev, > return -EINVAL; > > list_for_each_entry(feat_desc, &cscfg_mgr->feat_desc_list, item) { > - if (feat_desc->match_flags & match_flags) { > - err = cscfg_load_feat_csdev(csdev, feat_desc, ops); > - if (err) > - break; > - } > + err = cscfg_load_feat_csdev(csdev, match_flags, feat_desc, ops); > + if (err) > + break; > } > return err; > }

4 days, 1 hour

Re: [PATCH v4 6/7] coresight: etm4x: Report whether the PMU counts the external outputs

by Suzuki K Poulose

On 26/10/2026 05:48, Amir Ayupov wrote: > Some CPUs count the trace unit's external outputs as the PMU common > events TRCEXTOUT0-3 (0x4010-0x4013), which lets a configuration drive a > PMU event from trace unit events. Other CPUs, Neoverse N1 for example, > have no such events, and a feature relying on them would silently do > nothing there. > > Detect at probe, on the trace unit's CPU, whether the PMU counts every > external output: the trace unit has an external output for each of its > events (caps.nr_event), and pmuv3_event_implemented() reports whether > the PMU implements TRCEXTOUTn. Only check ETE: per its specification, an > implemented TRCEXTOUTn event counts external output n, while whether that > output is also an external input is IMPLEMENTATION DEFINED. > > Add a CS_CFG_MATCH_CAP_TRCEXTOUT capability match flag, set it on those > trace units, and show it in the feature's configfs 'matches' attribute. > With capabilities matched separately from the device class, a feature > that requires it only loads on those trace units. > > Assisted-by: Devmate:Opus-5.5 > Signed-off-by: Amir Ayupov <aaupov(a)fb.com> > --- > .../hwtracing/coresight/coresight-config.h | 2 ++ > .../hwtracing/coresight/coresight-etm4x-cfg.c | 6 ++++- > .../coresight/coresight-etm4x-core.c | 26 +++++++++++++++++++ > drivers/hwtracing/coresight/coresight-etm4x.h | 3 +++ > .../coresight/coresight-syscfg-configfs.c | 3 +++ > 5 files changed, 39 insertions(+), 1 deletion(-) > > diff --git a/drivers/hwtracing/coresight/coresight-config.h b/drivers/hwtracing/coresight/coresight-config.h > index 220bc54f8e5e1..4cf53230854d0 100644 > --- a/drivers/hwtracing/coresight/coresight-config.h > +++ b/drivers/hwtracing/coresight/coresight-config.h > @@ -38,6 +38,8 @@ > * a device that matches one of its classes and has all of its capabilities. > */ > #define CS_CFG_MATCH_CAP_MASK GENMASK_U32(15, 8) > +/* the PMU counts the trace unit's external outputs as TRCEXTOUTn */ > +#define CS_CFG_MATCH_CAP_TRCEXTOUT BIT(8) > Please could we rename this to : CS_CFG_MATCH_CAP_ETE_TRCEXTOUT as this is only applicable for ETEs ? > /* > * Limit number of presets in a configuration > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-cfg.c b/drivers/hwtracing/coresight/coresight-etm4x-cfg.c > index 52cb036bb04b9..9e4dc5950c13d 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-cfg.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-cfg.c > @@ -176,9 +176,13 @@ 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; > + if (drvdata->caps.trcextout) > + match_flags |= CS_CFG_MATCH_CAP_TRCEXTOUT; > > - return cscfg_register_csdev(csdev, CS_CFG_ETM4_MATCH_FLAGS, &ops); > + return cscfg_register_csdev(csdev, match_flags, &ops); > } > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index af017bd8a5477..cb5fbb0279b88 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -30,6 +30,7 @@ > #include <linux/uaccess.h> > #include <linux/perf_event.h> > #include <linux/perf/arm_pmu.h> > +#include <linux/perf/arm_pmuv3.h> > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > #include <linux/property.h> > @@ -1399,6 +1400,29 @@ static void etm4_fixup_wrong_ccitmin(struct etmv4_caps *caps) > } > } > > +/* > + * Whether the PMU counts each external output of this trace unit as > + * TRCEXTOUTn. The trace unit has an external output for each of its events. > + * Only ETE is checked: per its specification, an implemented TRCEXTOUTn event > + * counts external output n. > + * > + * Must run on the CPU that owns the trace unit. > + */ > +static bool etm4_pmu_counts_extout(struct etmv4_drvdata *drvdata) > +{ > + int n; > + > + if (!etm4x_is_ete(drvdata) || !drvdata->caps.nr_event) > + return false; > + > + for (n = 0; n < drvdata->caps.nr_event; n++) { > + if (!pmuv3_event_implemented(ARMV8_PMUV3_PERFCTR_TRCEXTOUT0 + n)) > + return false; > + } > + > + return true; > +} > + > static void etm4_init_arch_data(void *info) > { > u32 etmidr0; > @@ -1562,6 +1586,8 @@ static void etm4_init_arch_data(void *info) > /* NUMCNTR, bits[30:28] number of counters available for tracing */ > caps->nr_cntr = FIELD_GET(TRCIDR5_NUMCNTR_MASK, etmidr5); > > + caps->trcextout = etm4_pmu_counts_extout(drvdata); > + > coresight_clear_self_claim_tag_unlocked(csa); > etm4_cs_lock(drvdata, csa); > cpu_detect_trace_filtering(drvdata); > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h > index 5417280cdb274..33994ad4b2641 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x.h > +++ b/drivers/hwtracing/coresight/coresight-etm4x.h > @@ -868,6 +868,8 @@ enum etm_impdef_type { > * @lpoverride: If the implementation can support low-power state over. > * @skip_power_up: Indicates if an implementation can skip powering up > * the trace unit. > + * @trcextout: The trace unit is an ETE whose PMU counts its external > + * outputs as TRCEXTOUTn. We don't seem to be using this after the CSFG flag initialisation, in which case could we directly call etm4_pmu_counts_extout() from etm4_cscfg_register() ? Suzuki > */ > struct etmv4_caps { > u8 nr_pe; > @@ -907,6 +909,7 @@ struct etmv4_caps { > bool atbtrig : 1; > bool lpoverride : 1; > bool skip_power_up : 1; > + bool trcextout : 1; > }; > > /** > diff --git a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c > index 2b40e556be872..7b3800138f625 100644 > --- a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c > +++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c > @@ -250,6 +250,9 @@ static ssize_t cscfg_feat_matches_show(struct config_item *item, char *page) > if (match_flags & CS_CFG_MATCH_CLASS_SRC_ETM4) > used += scnprintf(page + used, PAGE_SIZE - used, "SRC_ETMV4 "); > > + if (match_flags & CS_CFG_MATCH_CAP_TRCEXTOUT) > + used += scnprintf(page + used, PAGE_SIZE - used, "TRCEXTOUT "); > + > used += scnprintf(page + used, PAGE_SIZE - used, "\n"); > return used; > }

4 days, 1 hour

[PATCH v4 00/19] perf cs-etm: Per-thread mode fixes and snapshot wrap support

by James Clark

This series depends on: https://lore.kernel.org/linux-perf-users/20261030-perf_cs_etm_fix_non_taken… Fix a few issues with per-thread mode: - On nVHE, everything looks like guest trace instead of host - Tracing multiple threads didn't really work After adding a tests for those, it turns out there were some snapshot bugs not picked up by the other snapshot tests. That showed that the different snapshot searches for Intel BTS and Arm SPE were inconsistent and I think all tracers can benefit from using the same pointer fixup logic, so that gets refactored into the common auxtrace.c. Then SPE can just share the Intel BTS search as it was obviously copied from it originally. IntelPT keeps the more advanced duplicate data search, but in the future that could probably be made the common one and all tracers would benefit from using it (except maybe BTS if it always has a very small buffer, but I doubt the overhead of the duplicate search would be an issue). For now this is more of a refactor rather than behavioral change so don't do that yet. Signed-off-by: James Clark <james.clark(a)linaro.org> --- Changes in v4: - Change test snapshot size to 128K for 64K machines - Put patchset depencency in cover letter text - Link to v3: https://patch.msgid.link/20261102-james-cs-unformatted-per-thread-fix-v3-0-… Changes in v3: - Keep the find_snapshot() callback instead of adding snapshot_has_wrapped() and monotonic_snapshot_head boolean (Adrian) - Add auxtrace_record__default_find_snapshot() for the simple search for BTS and SPE (Adrian) - Add auxtrace_record__snapshot_fixup() for any snapshot search implementation to use to update the pointers (Adrian) - Reword UINT64_MAX timestamp comment (Leo) - Fix disassembly test after rebasing onto exception sample fixes - Only take 'perf_guest' into account if there is no PIDFMT_CTXTID2 - Improve decoding of old files by hard coding queue 0 for per-thread - Link to v2: https://patch.msgid.link/20260821-james-cs-unformatted-per-thread-fix-v2-0-… Changes in v2: - Remove stale "may be reset to -1 in per-thread mode" comment - Set CPU when queueing auxtrace fragments otherwise auxtrace_queues__add_event() overwrites the queue's CPU - Link to v1: https://patch.msgid.link/20260811-james-cs-unformatted-per-thread-fix-v1-0-… --- James Clark (19): perf cs-etm: Fix nVHE per-thread decoding perf cs-etm: Warn for invalid timestamp option perf cs-etm: Turn on context packet timestamps in per-thread mode perf cs-etm: Use per-CPU queues for per-thread mode perf test: cs-etm: Increase window size in disassembly test perf cs-etm: Don't drop previous target for kernel samples perf python: Expose sample instruction length perf cs-etm: Use instruction length in disassembly script perf cs-etm: Increase default timestamp generation period perf auxtrace: Turn Intel BTS snapshot search into a generic one perf arm-spe: Use generic snapshot search perf auxtrace: intel-pt: Use common snapshot pointer fixup perf cs-etm: Queue partial AUX records perf cs-etm: Don't print missing buffers in snapshot mode perf auxtrace: cs-etm: Capture wrapped snapshots perf test: Allow infinite named_thread loops perf test: cs-etm: Add test for per-thread mode perf test: cs-etm: Test multiple per-thread threads perf test: cs-etm: Increase snapshot size tools/perf/Documentation/perf-test.txt | 2 +- tools/perf/arch/arm/util/cs-etm.c | 34 ++++- tools/perf/arch/arm64/util/arm-spe.c | 147 +-------------------- tools/perf/arch/x86/util/intel-bts.c | 115 +--------------- tools/perf/arch/x86/util/intel-pt.c | 21 +-- tools/perf/python/arm-cs-trace-disasm.py | 22 +-- tools/perf/python/perf.pyi | 1 + .../perf/tests/shell/coresight/per-thread-multi.sh | 76 +++++++++++ tools/perf/tests/shell/coresight/per-thread.sh | 48 +++++++ .../perf/tests/shell/coresight/raw_dump_stress.sh | 5 - .../shell/coresight/test_arm_coresight_disasm.sh | 8 +- tools/perf/tests/workloads/named_threads.c | 7 +- tools/perf/util/auxtrace.c | 145 +++++++++++++++++++- tools/perf/util/auxtrace.h | 18 +++ tools/perf/util/cs-etm.c | 113 +++++++++++----- tools/perf/util/python.c | 1 + 16 files changed, 420 insertions(+), 343 deletions(-) --- base-commit: 28ef93607ad52acc8f8be25199711153ba11a0c2 change-id: 20260605-james-cs-unformatted-per-thread-fix-50e723aa7f0e prerequisite-message-id: <20261030-perf_cs_etm_fix_non_taken-v3-0-7629b52ab047(a)arm.com> prerequisite-patch-id: 4ef889ad751e344ae47c1d75a2a2e6cb2eadf6e1 prerequisite-patch-id: 636b39730334cbc5a76642c070e704ffe178e7be prerequisite-patch-id: fb58f6daebdf5a194f8aa56ccc4022f932a5a6e5 prerequisite-patch-id: d240c10c6824f8ae6c3f10a1da01226a957f05fb prerequisite-patch-id: 17b291db261e32dcb0d6f1ab2ed0adb7e0411813 prerequisite-patch-id: 45ae1d523c9e5c8c573309f1dab107a598f3d4f8 prerequisite-patch-id: 39a3eb25b3a45f4481760c25fa270773bcfd94f5 prerequisite-patch-id: 5723a15636c2a28ec40eb6b5c9836d162e971786 prerequisite-patch-id: d18054b586fc38bc832ead185e8c1fbb0ce46f39 prerequisite-patch-id: 6735b192189d7945613458abce3da871cb79b35d prerequisite-patch-id: 387cb2c6181bef8f45a8fb167e8ef86e4a54ff69 prerequisite-patch-id: 50243a8d1739141aa24aa371a8452d4d2d078a99 prerequisite-patch-id: 2ae6f1e7707ba0c71a0678331b02d738fb25cb3c prerequisite-patch-id: 1fa0a90cfbc904c4429c1427409125ac44b27aed Best regards, -- James Clark <james.clark(a)linaro.org>

4 days, 1 hour

Re: [PATCH v4 0/7] coresight: Pulse an ETM external output for a PMU event

by Leo Yan

On Sun, Oct 25, 2026 at 10:48:20PM -0700, Amir Ayupov wrote: > Add an ETMv4 configuration, pmu_pulse, that drives trace unit external > output 1 on every traced cycle, so a PMU event counting it (TRCEXTOUT1) > can end a fixed-length ETM window, for example with aux-action=pause. > The feature only loads on an ETE whose PMU counts its external outputs. > > Patch 1 is Leo's fix for TRCIDR0.NUMEVENT decoding, which patch 6 relies > on for the number of external outputs. Maybe it is worth to update patch 03 for Suzuki's comment: https://lore.kernel.org/linux-arm-kernel/82adee03-a169-4106-99f2-7624a49260… This series looks good to me and I have verified it: Tested-by: Leo Yan <leo.yan(a)arm.com>

4 days, 1 hour

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight November 2026