CoreSight

coresight@lists.linaro.org

6 participants
2768 discussions

Re: [PATCH] coresight: ultrasoc-smb: Fix OOB write in smb_sync_perf_buffer()

by Suzuki K Poulose

On Thu, 04 Jun 2026 15:34:25 +0800, Junrui Luo wrote: > When the SMB sink is used as a perf AUX sink, smb_update_buffer() calls > smb_sync_perf_buffer() to copy hardware trace data into the perf AUX ring > buffer pages. It derives pg_idx = head >> PAGE_SHIFT from @head, which is > handle->head, and indexes dst_pages[pg_idx]. The pg_idx %= nr_pages > normalization is only applied after the first loop iteration. > > This leaves the initial page index underived from the buffer size, which > can result in an out-of-bounds write past dst_pages[] when head exceeds > the AUX buffer size. > > [...] Applied, thanks! [1/1] coresight: ultrasoc-smb: Fix OOB write in smb_sync_perf_buffer() https://git.kernel.org/coresight/c/98495b5a4d77 Best regards, -- Suzuki K Poulose <suzuki.poulose(a)arm.com>

1 week, 1 day

Re: [PATCH v7 05/13] coresight: etm4x: exclude ss_status from drvdata->config

by Suzuki K Poulose

Hi Levi, On 19/05/2026 16:48, Yeoreum Yun wrote: > The purpose of TRCSSCSRn register is to show status of > the corresponding Single-shot Comparator Control and input supports. > That means writable field's purpose for reset or restore from idle status > not for configuration. > > Therefore, exclude ss_status from drvdata->config and move it to drvdata. TRCSSCSRn has two different types of fields. 1. Capability 2. Status Moving the entire register from "config" to a common dangling place looks like shifting the problem to me. Could we do something like : 1. Split the fields to two. a. Store only the capability information in etmv4_caps. Bits[4:0] b. Store the Status bit in drvdata->config , as it is really matters to the session ? Bits[31:30] Add a helper to write/read the TRCSSSRn which would apply/mask the bits from drvdata->etmv4_caps->fields to the Status/Pending Suzuki > > Signed-off-by: Yeoreum Yun <yeoreum.yun(a)arm.com> > --- > .../hwtracing/coresight/coresight-etm4x-cfg.c | 1 - > .../hwtracing/coresight/coresight-etm4x-core.c | 16 +++++++++------- > .../hwtracing/coresight/coresight-etm4x-sysfs.c | 10 +++++----- > drivers/hwtracing/coresight/coresight-etm4x.h | 7 ++++++- > 4 files changed, 20 insertions(+), 14 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-cfg.c b/drivers/hwtracing/coresight/coresight-etm4x-cfg.c > index e1a59b434505..9b4947d75fde 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-cfg.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-cfg.c > @@ -86,7 +86,6 @@ static int etm4_cfg_map_reg_offset(struct etmv4_drvdata *drvdata, > off_mask = (offset & GENMASK(11, 5)); > do { > CHECKREGIDX(TRCSSCCRn(0), ss_ctrl, idx, off_mask); > - CHECKREGIDX(TRCSSCSRn(0), ss_status, idx, off_mask); > CHECKREGIDX(TRCSSPCICRn(0), ss_pe_cmp, idx, off_mask); > } while (0); > } else if ((offset >= TRCCIDCVRn(0)) && (offset <= TRCVMIDCVRn(7))) { > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index 53fbc4826628..7783c97b53e8 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -95,7 +95,7 @@ static bool etm4x_sspcicrn_present(struct etmv4_drvdata *drvdata, int n) > const struct etmv4_caps *caps = &drvdata->caps; > > return (n < caps->nr_ss_cmp) && caps->nr_pe_cmp && > - (drvdata->config.ss_status[n] & TRCSSCSRn_PC); > + (drvdata->ss_status[n] & TRCSSCSRn_PC); > } > > u64 etm4x_sysreg_read(u32 offset, bool _relaxed, bool _64bit) > @@ -571,11 +571,11 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) > etm4x_relaxed_write32(csa, config->res_ctrl[i], TRCRSCTLRn(i)); > > for (i = 0; i < caps->nr_ss_cmp; i++) { > - /* always clear status bit on restart if using single-shot */ > + /* always clear status and pending bits on restart if using single-shot */ > if (config->ss_ctrl[i] || config->ss_pe_cmp[i]) > - config->ss_status[i] &= ~TRCSSCSRn_STATUS; > + drvdata->ss_status[i] &= ~(TRCSSCSRn_STATUS | TRCSSCSRn_PENDING); > etm4x_relaxed_write32(csa, config->ss_ctrl[i], TRCSSCCRn(i)); > - etm4x_relaxed_write32(csa, config->ss_status[i], TRCSSCSRn(i)); > + etm4x_relaxed_write32(csa, drvdata->ss_status[i], TRCSSCSRn(i)); > if (etm4x_sspcicrn_present(drvdata, i)) > etm4x_relaxed_write32(csa, config->ss_pe_cmp[i], TRCSSPCICRn(i)); > } > @@ -772,6 +772,7 @@ static int etm4_parse_event_config(struct coresight_device *csdev, > /* Clear configuration from previous run */ > memset(config, 0, sizeof(struct etmv4_config)); > > + > if (attr->exclude_kernel) > config->mode = ETM_MODE_EXCL_KERN; > > @@ -1064,7 +1065,7 @@ static void etm4_disable_hw(struct etmv4_drvdata *drvdata) > > /* read the status of the single shot comparators */ > for (i = 0; i < caps->nr_ss_cmp; i++) { > - config->ss_status[i] = > + drvdata->ss_status[i] = > etm4x_relaxed_read32(csa, TRCSSCSRn(i)); > } > > @@ -1497,8 +1498,9 @@ static void etm4_init_arch_data(void *info) > */ > caps->nr_ss_cmp = FIELD_GET(TRCIDR4_NUMSSCC_MASK, etmidr4); > for (i = 0; i < caps->nr_ss_cmp; i++) { > - drvdata->config.ss_status[i] = > - etm4x_relaxed_read32(csa, TRCSSCSRn(i)); > + drvdata->ss_status[i] = etm4x_relaxed_read32(csa, TRCSSCSRn(i)); > + drvdata->ss_status[i] &= (TRCSSCSRn_PC | TRCSSCSRn_DV | > + TRCSSCSRn_DA | TRCSSCSRn_INST); > } > /* NUMCIDC, bits[27:24] number of Context ID comparators for tracing */ > caps->numcidc = FIELD_GET(TRCIDR4_NUMCIDC_MASK, etmidr4); > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > index 7de3c58a47b4..71e95d152ee6 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > @@ -1829,8 +1829,8 @@ static ssize_t sshot_ctrl_store(struct device *dev, > raw_spin_lock(&drvdata->spinlock); > idx = config->ss_idx; > config->ss_ctrl[idx] = FIELD_PREP(TRCSSCCRn_SAC_ARC_RST_MASK, val); > - /* must clear bit 31 in related status register on programming */ > - config->ss_status[idx] &= ~TRCSSCSRn_STATUS; > + /* must clear bit 31 and 30 in related status register on programming */ > + drvdata->ss_status[idx] &= ~(TRCSSCSRn_STATUS | TRCSSCSRn_PENDING); > raw_spin_unlock(&drvdata->spinlock); > return size; > } > @@ -1844,7 +1844,7 @@ static ssize_t sshot_status_show(struct device *dev, > struct etmv4_config *config = &drvdata->config; > > raw_spin_lock(&drvdata->spinlock); > - val = config->ss_status[config->ss_idx]; > + val = drvdata->ss_status[config->ss_idx]; > raw_spin_unlock(&drvdata->spinlock); > return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); > } > @@ -1879,8 +1879,8 @@ static ssize_t sshot_pe_ctrl_store(struct device *dev, > raw_spin_lock(&drvdata->spinlock); > idx = config->ss_idx; > config->ss_pe_cmp[idx] = FIELD_PREP(TRCSSPCICRn_PC_MASK, val); > - /* must clear bit 31 in related status register on programming */ > - config->ss_status[idx] &= ~TRCSSCSRn_STATUS; > + /* must clear bit 31 and 30 in related status register on programming */ > + drvdata->ss_status[idx] &= ~(TRCSSCSRn_STATUS | TRCSSCSRn_PENDING); > raw_spin_unlock(&drvdata->spinlock); > return size; > } > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h > index 6d4fbae78448..43db1eb6f8cd 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x.h > +++ b/drivers/hwtracing/coresight/coresight-etm4x.h > @@ -213,6 +213,7 @@ > #define TRCACATRn_EXLEVEL_MASK GENMASK(14, 8) > > #define TRCSSCSRn_STATUS BIT(31) > +#define TRCSSCSRn_PENDING BIT(30) > #define TRCSSCCRn_SAC_ARC_RST_MASK GENMASK(24, 0) > > #define TRCSSPCICRn_PC_MASK GENMASK(7, 0) > @@ -730,6 +731,9 @@ static inline u32 etm4_res_sel_pair(u8 res_sel_idx) > #define ETM_DEFAULT_ADDR_COMP 0 > > #define TRCSSCSRn_PC BIT(3) > +#define TRCSSCSRn_DV BIT(2) > +#define TRCSSCSRn_DA BIT(1) > +#define TRCSSCSRn_INST BIT(0) > > /* PowerDown Control Register bits */ > #define TRCPDCR_PU BIT(3) > @@ -978,7 +982,6 @@ struct etmv4_config { > u32 res_ctrl[ETM_MAX_RES_SEL]; /* TRCRSCTLRn */ > u8 ss_idx; > u32 ss_ctrl[ETM_MAX_SS_CMP]; > - u32 ss_status[ETM_MAX_SS_CMP]; > u32 ss_pe_cmp[ETM_MAX_SS_CMP]; > u8 addr_idx; > u64 addr_val[ETM_MAX_SINGLE_ADDR_CMP]; > @@ -1073,6 +1076,7 @@ struct etmv4_save_state { > * @config: structure holding configuration parameters. > * @save_state: State to be preserved across power loss > * @paused: Indicates if the trace unit is paused. > + * @ss_status: The status of the corresponding single-shot comparator. > * @arch_features: Bitmap of arch features of etmv4 devices. > */ > struct etmv4_drvdata { > @@ -1092,6 +1096,7 @@ struct etmv4_drvdata { > u64 trfcr; > struct etmv4_config config; > struct etmv4_save_state *save_state; > + u32 ss_status[ETM_MAX_SS_CMP]; > DECLARE_BITMAP(arch_features, ETM4_IMPDEF_FEATURE_MAX); > }; >

1 week, 1 day

Re: [PATCH] perf cs-etm: stamp pid/tid/EL on each buffered packet to fix cross-pid attribution

by Leo Yan

Hi Amir, On Wed, Jun 03, 2026 at 01:10:17PM -0700, Amir Ayupov wrote: > Hi James, > > Thank you for picking it up. > > I tested the v2 patch series and it looks good. There was a minor > difference in 2/39 tested perf data files: the number of brstack samples > differs by one, however, there was no loss of binary profile. The resulting > BOLT profile converted from the perf script output was identical, so I'm OK > with v2 patch as-is. Sorry jumping in as brstack is mentioned. Now branch stack is maintained per-CPU wise, it mixes up branch stack cross threads. The patch 03 in the series [1] refactors branch stack per-thread by using common code. Hope this can benefit a bit the profiling data quality and in case you are interested in. Thanks, Leo [1] https://lore.kernel.org/linux-perf-users/20260526-b4-arm_cs_callchain_suppo…

1 week, 1 day

[PATCH v3 00/19] perf cs-etm: Queue context packets for frontend

by James Clark

Fix thread tracking when decoding Coresight trace and add a new test for it. The new test is added as a Perf test workload instead of a custom binary with its own build system, but this requires a new feature in Perf test to pass in control pipes which can enable and disable events. This scopes the recording to just the workload and helps to reduce the amount of data recorded in tracing tests. With this new feature we can re-write all of the Coresight tests to make use of it and remove the remaining binaries which fixes the following issues: * They didn't work in out of source builds * A lot of the tests unnecessarily required root and didn't skip without it * They were mainly qualitative tests which didn't look for specific behavior Most importantly, the long build and runtime has been reduced. On a Radxa Orion O6, unroll_loop_thread.c took 37s to compile which is longer than the entire Perf build. Now the build time is negligible and the before and after test runtimes for all the Coresight tests are: | N1SDP | Orion O6 ----------------------------------- Before | 4m 0s | 14m 49s After | 26s | 56s ----------------------------------- Signed-off-by: James Clark <james.clark(a)linaro.org> --- Changes in v3: - Minor sashiko comments - Close some more pipes - Fix warning messages - Error handling improvements - Pass packet into cs_etm__synth_instruction_sample() - Fixup stale comment (Leo) - Link to v2: https://lore.kernel.org/r/20260602-james-cs-context-tracking-fix-v2-0-85b5c… Changes in v2: - Add --workload-ctl option to Perf test - Re-write all the Coresight tests and speed them up - Pass packet to memory access function so frontend can use either the previous or current packet's EL - Link to v1: https://lore.kernel.org/r/20260526-james-cs-context-tracking-fix-v1-0-ebd60… --- James Clark (19): perf cs-etm: Queue context packets for frontend perf test: Add workload-ctl option perf test: Add a workload that forces context switches perf test cs-etm: Test process attribution perf test: Add deterministic workload perf test cs-etm: Replace unroll loop thread with deterministic decode test perf test cs-etm: Remove asm_pure_loop test perf test cs-etm: Replace memcpy test with raw dump stress test perf test: Add named_threads workload perf test cs-etm: Test decoding for concurrent threads test perf test cs-etm: Remove duplicate branch tests perf test cs-etm: Skip if not root perf test cs-etm: Reduce snapshot size perf test cs-etm: Speed up basic test perf test cs-etm: Remove unused Coresight workloads perf test cs-etm: Make disassembly test use kcore perf test cs-etm: Add all branch instructions to test perf test cs-etm: Speed up disassembly test perf test cs-etm: Move existing tests to coresight folder Documentation/trace/coresight/coresight-perf.rst | 78 +------ MAINTAINERS | 2 - tools/perf/Documentation/perf-test.txt | 18 +- tools/perf/Makefile.perf | 14 +- tools/perf/scripts/python/arm-cs-trace-disasm.py | 20 +- tools/perf/tests/builtin-test.c | 187 +++++++++++++++- tools/perf/tests/shell/coresight/Makefile | 29 --- .../perf/tests/shell/coresight/Makefile.miniconfig | 14 -- tools/perf/tests/shell/coresight/asm_pure_loop.sh | 22 -- .../tests/shell/coresight/asm_pure_loop/.gitignore | 1 - .../tests/shell/coresight/asm_pure_loop/Makefile | 34 --- .../shell/coresight/asm_pure_loop/asm_pure_loop.S | 30 --- .../tests/shell/coresight/concurrent_threads.sh | 45 ++++ .../tests/shell/coresight/context_switch_thread.sh | 69 ++++++ tools/perf/tests/shell/coresight/deterministic.sh | 71 +++++++ .../tests/shell/coresight/memcpy_thread/.gitignore | 1 - .../tests/shell/coresight/memcpy_thread/Makefile | 33 --- .../shell/coresight/memcpy_thread/memcpy_thread.c | 80 ------- .../tests/shell/coresight/memcpy_thread_16k_10.sh | 22 -- .../perf/tests/shell/coresight/raw_dump_stress.sh | 48 +++++ .../shell/{ => coresight}/test_arm_coresight.sh | 43 ++-- .../{ => coresight}/test_arm_coresight_disasm.sh | 17 +- .../tests/shell/coresight/thread_loop/.gitignore | 1 - .../tests/shell/coresight/thread_loop/Makefile | 33 --- .../shell/coresight/thread_loop/thread_loop.c | 85 -------- .../shell/coresight/thread_loop_check_tid_10.sh | 23 -- .../shell/coresight/thread_loop_check_tid_2.sh | 23 -- .../shell/coresight/unroll_loop_thread/.gitignore | 1 - .../shell/coresight/unroll_loop_thread/Makefile | 33 --- .../unroll_loop_thread/unroll_loop_thread.c | 75 ------- .../tests/shell/coresight/unroll_loop_thread_10.sh | 22 -- tools/perf/tests/shell/lib/coresight.sh | 134 ------------ tools/perf/tests/tests.h | 3 + tools/perf/tests/workloads/Build | 4 + tools/perf/tests/workloads/context_switch_loop.c | 101 +++++++++ tools/perf/tests/workloads/deterministic.c | 39 ++++ tools/perf/tests/workloads/named_threads.c | 109 ++++++++++ tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 21 +- tools/perf/util/cs-etm.c | 234 ++++++++++++--------- tools/perf/util/cs-etm.h | 8 +- 40 files changed, 889 insertions(+), 938 deletions(-) --- base-commit: 5f0ca6b80b12bab1ce06839cdffb6148bb650ff4 change-id: 20260515-james-cs-context-tracking-fix-754998bae7ed Best regards, -- James Clark <james.clark(a)linaro.org>

1 week, 2 days

Re: [PATCH v7 09/13] coresight: etm4x: missing cscfg_csdev_disable_active_config() in perf enable

by Leo Yan

On Tue, May 19, 2026 at 04:48:08PM +0100, Yeoreum Yun wrote: > In the perf enable path, there are missing cases where > cscfg_csdev_disable_active_config() is not called: > > - Branch broadcast is selected but not supported by the hardware > - etm4_enable_hw() fails > > This can lead to a leak of config_desc->active_cnt. > Fix this by properly calling cscfg_csdev_disable_active_config() > in these error paths. > > Fixes: 810ac401db1f ("coresight: etm4x: Add complex configuration handlers to etmv4") > Signed-off-by: Yeoreum Yun <yeoreum.yun(a)arm.com> > --- > .../hwtracing/coresight/coresight-etm4x-core.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index f0a9f2ef4b27..0889937811cb 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -899,6 +899,8 @@ static int etm4_parse_event_config(struct coresight_device *csdev, > * Missing BB support could cause silent decode errors > * so fail to open if it's not supported. > */ > + if (cfg_hash) > + cscfg_csdev_disable_active_config(csdev); > ret = -EINVAL; > goto out; > } else { > @@ -915,6 +917,7 @@ static int etm4_enable_perf(struct coresight_device *csdev, > struct coresight_path *path) > { > struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + struct perf_event_attr *attr = &event->attr; > int ret; > > if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id())) > @@ -926,7 +929,7 @@ static int etm4_enable_perf(struct coresight_device *csdev, > /* Configure the tracer based on the session's specifics */ > ret = etm4_parse_event_config(csdev, event); > if (ret) > - goto out; > + goto err; > > drvdata->trcid = path->trace_id; > > @@ -935,16 +938,19 @@ static int etm4_enable_perf(struct coresight_device *csdev, > > /* And enable it */ > ret = etm4_enable_hw(drvdata); > - > -out: > - /* Failed to start tracer; roll back to DISABLED mode */ > if (ret) { > - coresight_set_mode(csdev, CS_MODE_DISABLED); > - return ret; > + if (ATTR_CFG_GET_FLD(attr, configid)) > + cscfg_csdev_disable_active_config(csdev); > + goto err; > } > > csdev->path = path; > return 0; > + > +err: > + /* Failed to start tracer; roll back to DISABLED mode */ > + coresight_set_mode(csdev, CS_MODE_DISABLED); > + return ret; > } Seems to me, it is not readable if spread error handling into both child and parent functions. I would like we can stick to use the same function for resource allocation and rollback for failures. How about below change (I did not verify it yet): diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 0889937811cb..8347efb2069b 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -794,8 +794,7 @@ static int etm4_parse_event_config(struct coresight_device *csdev, .ATTR_CFG_FLD_timestamp_CFG = U64_MAX, }; struct perf_event_attr *attr = &event->attr; - unsigned long cfg_hash; - int preset, cc_threshold; + int cc_threshold; u8 ts_level; /* Clear configuration from previous run */ @@ -882,16 +881,6 @@ static int etm4_parse_event_config(struct coresight_device *csdev, /* bit[12], Return stack enable bit */ config->cfg |= TRCCONFIGR_RS; - /* - * Set any selected configuration and preset. A zero configid means no - * configuration active, preset = 0 means no preset selected. - */ - cfg_hash = ATTR_CFG_GET_FLD(attr, configid); - if (cfg_hash) { - preset = ATTR_CFG_GET_FLD(attr, preset); - ret = cscfg_csdev_enable_active_config(csdev, cfg_hash, preset); - } - /* branch broadcast - enable if selected and supported */ if (ATTR_CFG_GET_FLD(attr, branch_broadcast)) { if (!caps->trcbb) { @@ -899,8 +888,6 @@ static int etm4_parse_event_config(struct coresight_device *csdev, * Missing BB support could cause silent decode errors * so fail to open if it's not supported. */ - if (cfg_hash) - cscfg_csdev_disable_active_config(csdev); ret = -EINVAL; goto out; } else { @@ -918,7 +905,8 @@ static int etm4_enable_perf(struct coresight_device *csdev, { struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); struct perf_event_attr *attr = &event->attr; - int ret; + unsigned long cfg_hash; + int preset, ret; if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id())) return -EINVAL; @@ -931,6 +919,18 @@ static int etm4_enable_perf(struct coresight_device *csdev, if (ret) goto err; + /* + * Set any selected configuration and preset. A zero configid means no + * configuration active, preset = 0 means no preset selected. + */ + cfg_hash = ATTR_CFG_GET_FLD(attr, configid); + if (cfg_hash) { + preset = ATTR_CFG_GET_FLD(attr, preset); + ret = cscfg_csdev_enable_active_config(csdev, cfg_hash, preset); + if (ret) + goto err; + } + drvdata->trcid = path->trace_id; /* Populate pause state */ @@ -938,15 +938,15 @@ static int etm4_enable_perf(struct coresight_device *csdev, /* And enable it */ ret = etm4_enable_hw(drvdata); - if (ret) { - if (ATTR_CFG_GET_FLD(attr, configid)) - cscfg_csdev_disable_active_config(csdev); - goto err; - } + if (ret) + goto err_enable_hw; csdev->path = path; return 0; +err_enable_hw: + if (cfg_hash) + cscfg_csdev_disable_active_config(csdev); err: /* Failed to start tracer; roll back to DISABLED mode */ coresight_set_mode(csdev, CS_MODE_DISABLED);

1 week, 4 days

Re: [PATCH v7 08/13] coresight: etm4x: fix inconsistencies with sysfs configuration

by Leo Yan

On Thu, May 28, 2026 at 03:26:57PM +0100, Yeoreum Yun wrote: [...] > > > @@ -47,7 +47,7 @@ static int etm4_cfg_map_reg_offset(struct etmv4_drvdata *drvdata, > > > struct cscfg_regval_csdev *reg_csdev, u32 offset) > > > { > > > int err = -EINVAL, idx; > > > - struct etmv4_config *drvcfg = &drvdata->config; > > > + struct etmv4_config *drvcfg = &drvdata->active_config; > > > > Wouldn't it make more sense to keep using drvdata->config for cfgfs? > > This would avoid cfgfs updating the active configuration while a session > > is enabled. > > > > My understanding is after refactoring cfgfs, we will never expose > > active_config or config anymore so this should not an issue. Before > > that, maybe we just keep to use config so can avoid mess. > > I don't think so. since the cfgfs's config is updated right before > enable and in this patchset, It's applied after "take the mode". > > If we do it into drvdta->config, then contention would be increased > with the access of sysfs and that nullifies almost this patch purpose > and because of we use "one config" this makes a corruption with perf > and sysfs. Here have two different contention: the contention between sysfs and cfgfs, and the contention between cfgfs and a runtime config (active config). My suggestion is to avoid the later contention, which is the main idea in this series that avoid the runtime config is modified in the middle of a session. That said, I have no strong opinion. > > > static void etm4_enable_sysfs_smp_call(void *info) > > > { > > > struct etm4_enable_arg *arg = info; > > > + struct etmv4_drvdata *drvdata; > > > struct coresight_device *csdev; > > > + unsigned long cfg_hash; > > > + int preset; > > > > > > if (WARN_ON(!arg)) > > > return; > > > > > > - csdev = arg->drvdata->csdev; > > > + drvdata = arg->drvdata; > > > + csdev = drvdata->csdev; > > > if (!coresight_take_mode(csdev, CS_MODE_SYSFS)) { > > > /* Someone is already using the tracer */ > > > arg->rc = -EBUSY; > > > return; > > > } > > > > > > - arg->rc = etm4_enable_hw(arg->drvdata); > > > + /* enable any config activated by configfs */ > > > + cscfg_config_sysfs_get_active_cfg(&cfg_hash, &preset); > > > > > > - /* The tracer didn't start */ > > > + drvdata->active_config = arg->config; > > > > Can move this down to just before drvdata->trcid assignment so cscfg > > operations can be put together? > > No. Copy the arg->config must be before cscfg_csdev_enable_active_config(). > If that's after, it overrides the "preset" and this is different from > the former behavior. Please copy config first, then do cscfg stuffs. > > > + arg->rc = etm4_enable_hw(drvdata); > > > if (arg->rc) { > > > - coresight_set_mode(csdev, CS_MODE_DISABLED); > > > - return; > > > + cscfg_csdev_disable_active_config(csdev); > > > + goto err; > > > > Add a new goto tag (like err_enable_hw) for the disable_active_config > > rollback? > > This is only place where cscfg_csdev_disable_active_config(). > Why do we need to goto for cleanup where there is no duplication? It is about to use a central place for handling errors. > > > @@ -1449,7 +1458,7 @@ static void etm4_init_arch_data(void *info) > > > > > > /* EXLEVEL_S, bits[19:16] Secure state instruction tracing */ > > > caps->s_ex_level = FIELD_GET(TRCIDR3_EXLEVEL_S_MASK, etmidr3); > > > - drvdata->config.s_ex_level = caps->s_ex_level; > > > + config->s_ex_level = caps->s_ex_level; > > > > config->s_ex_level is redundant and not really a configuration item. > > We should use caps->s_ex_level instead of config->s_ex_level wherever > > it is used. > > Agree, but this includes it should change the omse function > declations to pass "caps" argument: > - etm4_set_comparator_filter() > - etm4_set_start_stop_filter() > > If that's okay, I'll respin with it. This is fine for me. Thanks, Leo

1 week, 4 days

Re: [PATCH v7 03/13] coresight: etm4x: introduce ETM_MAX_SEQ_TRANSITIONS

by Suzuki K Poulose

On 19/05/2026 16:48, Yeoreum Yun wrote: > According to IHI006H Embedded Trace Macrocell Architecture > Specification [0], n could be 0-2 for TCRSEQEVR<n> when > TCRIDR5.NUMSEQSTATE is 0b100. > > Therefore, introduce ETM_MAX_SEQ_TRANSITIONS macro and apply this > in TCRSEQEVR<n> relevant fields. > > Link: https://developer.arm.com/documentation/ihi0064/latest/ [0] > Suggestedby: Leo Yan <leo.yan(a)arm.com> > Signed-off-by: Yeoreum Yun <yeoreum.yun(a)arm.com> > --- > drivers/hwtracing/coresight/coresight-etm4x-cfg.c | 2 +- > drivers/hwtracing/coresight/coresight-etm4x.h | 5 +++-- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-cfg.c b/drivers/hwtracing/coresight/coresight-etm4x-cfg.c > index c302072b293a..e1a59b434505 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-cfg.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-cfg.c > @@ -76,7 +76,7 @@ static int etm4_cfg_map_reg_offset(struct etmv4_drvdata *drvdata, > } else if ((offset & GENMASK(11, 4)) == TRCSEQEVRn(0)) { > /* sequencer state control registers */ > idx = (offset & GENMASK(3, 0)) / 4; > - if (idx < ETM_MAX_SEQ_STATES) { > + if (idx < ETM_MAX_SEQ_TRANSITIONS) { > reg_csdev->driver_regval = &drvcfg->seq_ctrl[idx]; > err = 0; > } > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h > index 89d81ce4e04e..60e08ab085c5 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x.h > +++ b/drivers/hwtracing/coresight/coresight-etm4x.h > @@ -614,6 +614,7 @@ static inline u32 etm4_res_sel_pair(u8 res_sel_idx) > #define ETM_MAX_NR_PE 8 > #define ETMv4_MAX_CNTR 4 > #define ETM_MAX_SEQ_STATES 4 > +#define ETM_MAX_SEQ_TRANSITIONS (ETM_MAX_SEQ_STATES - 1) Similar to the other comment on the patch, please don't tie this to SEQ_STATES. Simpl define. #define ETM_MAX_SEQ_CTRLS 3 Suzuki > #define ETM_MAX_EXT_INP_SEL 4 > #define ETM_MAX_EXT_INP 256 > #define ETM_MAX_EXT_OUT 4 > @@ -877,7 +878,7 @@ struct etmv4_config { > u32 vipcssctlr; > u8 seq_idx; > u8 syncfreq; > - u32 seq_ctrl[ETM_MAX_SEQ_STATES]; > + u32 seq_ctrl[ETM_MAX_SEQ_TRANSITIONS]; > u32 seq_rst; > u32 seq_state; > u8 cntr_idx; > @@ -928,7 +929,7 @@ struct etmv4_save_state { > u32 trcvissctlr; > u32 trcvipcssctlr; > > - u32 trcseqevr[ETM_MAX_SEQ_STATES]; > + u32 trcseqevr[ETM_MAX_SEQ_TRANSITIONS]; > u32 trcseqrstevr; > u32 trcseqstr; > u32 trcextinselr;

1 week, 4 days

Re: [PATCH v9 4/4] coresight: cti: expose banked sysfs registers for Qualcomm extended CTI

by Leo Yan

On Mon, Jun 01, 2026 at 10:08:55AM +0800, Yingchao Deng (Consultant) wrote: [...] > I noticed that among the new sysfs knobs in this series, the > integration test registers (ittrigin[1-3], ittrigout[1-3], > ittrigoutack[1-3], ittriginack[1-3]) have no existing documentation > for their base (index-0) counterparts, which were added long before > this series. Ah, thanks for pointing out the missed piece! > Two options for patch 5/5: > > (a) Document only the knobs whose base versions are already > documented (triginstatus[1-3] and trigoutstatus[1-3]). > > (b) Document everything in patch 5/5 — both the new indexed > variants and the previously missing base IT register entries. Please use option (b). Could you fold the code piece below into patch 5? Note, we cannot merge ittrigin and ittrigin[1-3] into a single entry because they were introduced in different kernel versions. @Mike, if any chance you could give a review on this? ---8<--- --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti @@ -134,6 +134,60 @@ KernelVersion: 5.7 Contact: Mike Leach or Mathieu Poirier Description: (Read) read current status of output trigger signals. +What: /sys/bus/coresight/devices/<cti-name>/regs/itctrl +Date: March 2020 +KernelVersion 5.7 +Contact: coresight(a)lists.linaro.org +Description: (RW) Control integration mode. + +What: /sys/bus/coresight/devices/<cti-name>/regs/itchin +Date: March 2020 +KernelVersion 5.7 +Contact: coresight(a)lists.linaro.org +Description: (Read) Read the values of the CTCHIN inputs. + +What: /sys/bus/coresight/devices/<cti-name>/regs/itchinack +Date: March 2020 +KernelVersion 5.7 +Contact: coresight(a)lists.linaro.org +Description: (Write) Write the value of the CTCHINACK input. + +What: /sys/bus/coresight/devices/<cti-name>/regs/ittrigin +Date: March 2020 +KernelVersion 5.7 +Contact: coresight(a)lists.linaro.org +Description: (Read) Read the values of the CTTRIGIN inputs. + +What: /sys/bus/coresight/devices/<cti-name>/regs/ittriginack +Date: March 2020 +KernelVersion 5.7 +Contact: coresight(a)lists.linaro.org +Description: (Write) Write the value of the CTTRIGINACK input. + +What: /sys/bus/coresight/devices/<cti-name>/regs/itchout +Date: March 2020 +KernelVersion 5.7 +Contact: coresight(a)lists.linaro.org +Description: (RW) Read or write the value of the CTCHOUT outputs. + +What: /sys/bus/coresight/devices/<cti-name>/regs/itchoutack +Date: March 2020 +KernelVersion 5.7 +Contact: coresight(a)lists.linaro.org +Description: (Read) Read the value of the CTCHOUTACK input. + +What: /sys/bus/coresight/devices/<cti-name>/regs/ittrigout +Date: March 2020 +KernelVersion 5.7 +Contact: coresight(a)lists.linaro.org +Description: (RW) Read or write the value of the CTTRIGOUT outputs. + +What: /sys/bus/coresight/devices/<cti-name>/regs/ittrigoutack +Date: March 2020 +KernelVersion 5.7 +Contact: coresight(a)lists.linaro.org +Description: (Read) Read the value of the CTTRIGOUTACK input. + What: /sys/bus/coresight/devices/<cti-name>/channels/trigin_attach Date: March 2020 KernelVersion: 5.7

1 week, 4 days

Re: [PATCH v7 02/13] coresight: etm4x: fix underflow for nrseqstate

by Suzuki K Poulose

On 19/05/2026 16:48, Yeoreum Yun wrote: > TCRSEQEVR<n> is implemented only when TCRIDR5.NUMSEQSTATE is 0b100, > in which case n ranges from 0 to 2; otherwise, TCRIDR5.NUMSEQSTATE is 0b000. > > Therefore, drvdata->nrseqstate should be checked before entering the loop. for (i = 0; i < drvdata->nrseqstate - 1; i++) Wouldn't that check cover the case ? (provided i is signed int ?) Suzuki > > Link: https://developer.arm.com/documentation/ihi0064/latest/ [0] > Fixes: 2e1cdfe184b5 ("coresight-etm4x: Adding CoreSight ETM4x driver") > Reviewed-by: Leo Yan <leo.yan(a)arm.com> > Signed-off-by: Yeoreum Yun <yeoreum.yun(a)arm.com> > --- > .../hwtracing/coresight/coresight-etm4x-core.c | 18 ++++++++++-------- > .../coresight/coresight-etm4x-sysfs.c | 2 ++ > 2 files changed, 12 insertions(+), 8 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index 1e3b0344dc00..94b9385e964a 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -542,9 +542,11 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) > etm4x_relaxed_write32(csa, config->vissctlr, TRCVISSCTLR); > if (drvdata->nr_pe_cmp) > etm4x_relaxed_write32(csa, config->vipcssctlr, TRCVIPCSSCTLR); > - for (i = 0; i < drvdata->nrseqstate - 1; i++) > - etm4x_relaxed_write32(csa, config->seq_ctrl[i], TRCSEQEVRn(i)); > + > if (drvdata->nrseqstate) { > + for (i = 0; i < drvdata->nrseqstate - 1; i++) > + etm4x_relaxed_write32(csa, config->seq_ctrl[i], TRCSEQEVRn(i)); > + > etm4x_relaxed_write32(csa, config->seq_rst, TRCSEQRSTEVR); > etm4x_relaxed_write32(csa, config->seq_state, TRCSEQSTR); > } > @@ -1896,10 +1898,10 @@ static int etm4_cpu_save(struct coresight_device *csdev) > if (drvdata->nr_pe_cmp) > state->trcvipcssctlr = etm4x_read32(csa, TRCVIPCSSCTLR); > > - for (i = 0; i < drvdata->nrseqstate - 1; i++) > - state->trcseqevr[i] = etm4x_read32(csa, TRCSEQEVRn(i)); > - > if (drvdata->nrseqstate) { > + for (i = 0; i < drvdata->nrseqstate - 1; i++) > + state->trcseqevr[i] = etm4x_read32(csa, TRCSEQEVRn(i)); > + > state->trcseqrstevr = etm4x_read32(csa, TRCSEQRSTEVR); > state->trcseqstr = etm4x_read32(csa, TRCSEQSTR); > } > @@ -2009,10 +2011,10 @@ static void etm4_cpu_restore(struct coresight_device *csdev) > if (drvdata->nr_pe_cmp) > etm4x_relaxed_write32(csa, state->trcvipcssctlr, TRCVIPCSSCTLR); > > - for (i = 0; i < drvdata->nrseqstate - 1; i++) > - etm4x_relaxed_write32(csa, state->trcseqevr[i], TRCSEQEVRn(i)); > - > if (drvdata->nrseqstate) { > + for (i = 0; i < drvdata->nrseqstate - 1; i++) > + etm4x_relaxed_write32(csa, state->trcseqevr[i], TRCSEQEVRn(i)); > + > etm4x_relaxed_write32(csa, state->trcseqrstevr, TRCSEQRSTEVR); > etm4x_relaxed_write32(csa, state->trcseqstr, TRCSEQSTR); > } > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > index e9eeea6240d5..0e1ad175aa1e 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > @@ -1395,6 +1395,8 @@ static ssize_t seq_idx_store(struct device *dev, > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etmv4_config *config = &drvdata->config; > > + if (!drvdata->nrseqstate) > + return -ENOTSUPP; > if (kstrtoul(buf, 16, &val)) > return -EINVAL; > if (val >= drvdata->nrseqstate - 1)

1 week, 4 days

Re: [PATCH v7 02/13] coresight: etm4x: fix underflow for nrseqstate

by Suzuki K Poulose

On 19/05/2026 16:48, Yeoreum Yun wrote: > TCRSEQEVR<n> is implemented only when TCRIDR5.NUMSEQSTATE is 0b100, > in which case n ranges from 0 to 2; otherwise, TCRIDR5.NUMSEQSTATE is 0b000. > > Therefore, drvdata->nrseqstate should be checked before entering the loop. s/TCR/TRC/g ^^^ and every other patch in the series. Seem like you have been playing with the MMU code ;-) > > Link: https://developer.arm.com/documentation/ihi0064/latest/ [0] > Fixes: 2e1cdfe184b5 ("coresight-etm4x: Adding CoreSight ETM4x driver") > Reviewed-by: Leo Yan <leo.yan(a)arm.com> > Signed-off-by: Yeoreum Yun <yeoreum.yun(a)arm.com> > --- > .../hwtracing/coresight/coresight-etm4x-core.c | 18 ++++++++++-------- > .../coresight/coresight-etm4x-sysfs.c | 2 ++ > 2 files changed, 12 insertions(+), 8 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index 1e3b0344dc00..94b9385e964a 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -542,9 +542,11 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) > etm4x_relaxed_write32(csa, config->vissctlr, TRCVISSCTLR); > if (drvdata->nr_pe_cmp) > etm4x_relaxed_write32(csa, config->vipcssctlr, TRCVIPCSSCTLR); > - for (i = 0; i < drvdata->nrseqstate - 1; i++) > - etm4x_relaxed_write32(csa, config->seq_ctrl[i], TRCSEQEVRn(i)); > + > if (drvdata->nrseqstate) { > + for (i = 0; i < drvdata->nrseqstate - 1; i++) > + etm4x_relaxed_write32(csa, config->seq_ctrl[i], TRCSEQEVRn(i)); Looking at this again, I think we are contemplating two different values from a single variable (blame the architecture). Instead, we should: 1. Disconnect number of TRCSEQEVRn from drvdata->nrseqstate, and add nr_seq_ctrls to drvdata and initialise this at probe. for (i = 0; i < drvdata->nr_seq_ctrls; i++) Suzuki > + > etm4x_relaxed_write32(csa, config->seq_rst, TRCSEQRSTEVR); > etm4x_relaxed_write32(csa, config->seq_state, TRCSEQSTR); > } > @@ -1896,10 +1898,10 @@ static int etm4_cpu_save(struct coresight_device *csdev) > if (drvdata->nr_pe_cmp) > state->trcvipcssctlr = etm4x_read32(csa, TRCVIPCSSCTLR); > > - for (i = 0; i < drvdata->nrseqstate - 1; i++) > - state->trcseqevr[i] = etm4x_read32(csa, TRCSEQEVRn(i)); > - > if (drvdata->nrseqstate) { > + for (i = 0; i < drvdata->nrseqstate - 1; i++) > + state->trcseqevr[i] = etm4x_read32(csa, TRCSEQEVRn(i)); > + > state->trcseqrstevr = etm4x_read32(csa, TRCSEQRSTEVR); > state->trcseqstr = etm4x_read32(csa, TRCSEQSTR); > } > @@ -2009,10 +2011,10 @@ static void etm4_cpu_restore(struct coresight_device *csdev) > if (drvdata->nr_pe_cmp) > etm4x_relaxed_write32(csa, state->trcvipcssctlr, TRCVIPCSSCTLR); > > - for (i = 0; i < drvdata->nrseqstate - 1; i++) > - etm4x_relaxed_write32(csa, state->trcseqevr[i], TRCSEQEVRn(i)); > - > if (drvdata->nrseqstate) { > + for (i = 0; i < drvdata->nrseqstate - 1; i++) > + etm4x_relaxed_write32(csa, state->trcseqevr[i], TRCSEQEVRn(i)); > + > etm4x_relaxed_write32(csa, state->trcseqrstevr, TRCSEQRSTEVR); > etm4x_relaxed_write32(csa, state->trcseqstr, TRCSEQSTR); > } > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > index e9eeea6240d5..0e1ad175aa1e 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > @@ -1395,6 +1395,8 @@ static ssize_t seq_idx_store(struct device *dev, > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etmv4_config *config = &drvdata->config; > > + if (!drvdata->nrseqstate) > + return -ENOTSUPP; > if (kstrtoul(buf, 16, &val)) > return -EINVAL; > if (val >= drvdata->nrseqstate - 1)

1 week, 4 days

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight