CoreSight

coresight@lists.linaro.org

1 participants
2837 discussions

[PATCH 0/2] 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. Unfortunately the test has to be added as a separate binary like the other existing Coresight workloads which needs a bit of makefile boilerplate. Ideally it would be a builtin Perf test workload, but Perf has a lot of dependencies and generates a lot of trace when starting up which makes tracing tests very slow because all that has to be decoded. Hopefully I can find a generic way to enable Perf events at the beginning of the workload and then I can migrate everything from tools/perf/tests/shell/coresight to Perf test workloads, but that will have to be done as a separate change. Signed-off-by: James Clark <james.clark(a)linaro.org> --- James Clark (1): perf test cs-etm: Test thread attribution Leo Yan (1): perf cs-etm: Queue context packets for frontend tools/perf/tests/shell/coresight/Makefile | 1 + .../shell/coresight/context_switch_loop/.gitignore | 1 + .../shell/coresight/context_switch_loop/Makefile | 29 ++++ .../context_switch_loop/context_switch_loop.c | 83 ++++++++++ .../tests/shell/coresight/context_switch_thread.sh | 48 ++++++ tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 18 +- tools/perf/util/cs-etm.c | 181 +++++++++++++-------- tools/perf/util/cs-etm.h | 5 +- 8 files changed, 295 insertions(+), 71 deletions(-) --- base-commit: 09d355618f7ccc27ffc7fc668b2e232872962079 change-id: 20260515-james-cs-context-tracking-fix-754998bae7ed Best regards, -- James Clark <james.clark(a)linaro.org>

3 months, 1 week

Re: [PATCH v9 2/4] coresight: cti: use __reg_addr() helper for register access

by Leo Yan

On Thu, May 21, 2026 at 08:16:28PM +0800, Yingchao Deng wrote: [...] > +static void __iomem *__reg_addr(struct cti_drvdata *drvdata, u32 off, > + u32 index) > +{ > + return drvdata->base + off + sizeof(u32) * index; > +} > + > +#define reg_addr(drvdata, off) __reg_addr((drvdata), (off), 0) > +#define reg_index_addr(drvdata, off, i) __reg_addr((drvdata), (off), (i)) > -u32 cti_read_single_reg(struct cti_drvdata *drvdata, int offset) > +u32 cti_read_single_reg(struct cti_drvdata *drvdata, u32 off, u32 index) > { > - int val; > + u32 val; > > CS_UNLOCK(drvdata->base); > - val = readl_relaxed(drvdata->base + offset); > + val = readl_relaxed(reg_index_addr(drvdata, off, index)); > CS_LOCK(drvdata->base); > > return val; > } > > -void cti_write_single_reg(struct cti_drvdata *drvdata, int offset, u32 value) > +void cti_write_single_reg(struct cti_drvdata *drvdata, u32 off, u32 index, > + u32 value) > { > CS_UNLOCK(drvdata->base); > - writel_relaxed(value, drvdata->base + offset); > + writel_relaxed(value, reg_index_addr(drvdata, off, index)); > CS_LOCK(drvdata->base); > } I prefer to move the register helpers into coresight-cti.h and add two additional helpers: cti_read_single_reg_index() and cti_write_single_reg_index(). I also found circular dependency between coresight-cti.h and qcom-cti.h. Since qcom-cti.h only contains register definitions and a small inline function for register conversion, we can simply fold it into coresight-cti.h and dismiss the circular dependency. Based on this idea, I played around with the code a bit (see the snippet below). Would you be okay with applying this approach to patches 02/03? If have any questions, please let me know. ---8<--- --- a/drivers/hwtracing/coresight/coresight-cti.h +++ b/drivers/hwtracing/coresight/coresight-cti.h @@ -60,6 +60,31 @@ struct fwnode_handle; */ #define CTIINOUTEN_MAX 128 +/* QCOM CTI extension */ +#define QCOM_ARCHITECT 0x477 + +#define QCOM_CTIINTACK 0x020 +#define QCOM_CTIAPPSET 0x004 +#define QCOM_CTIAPPCLEAR 0x008 +#define QCOM_CTIAPPPULSE 0x00C +#define QCOM_CTIINEN 0x400 +#define QCOM_CTIOUTEN 0x800 +#define QCOM_CTITRIGINSTATUS 0x040 +#define QCOM_CTITRIGOUTSTATUS 0x060 +#define QCOM_CTICHINSTATUS 0x080 +#define QCOM_CTICHOUTSTATUS 0x084 +#define QCOM_CTIGATE 0x088 +#define QCOM_ASICCTL 0x08C +/* Integration test registers */ +#define QCOM_ITCHINACK 0xE70 +#define QCOM_ITTRIGINACK 0xE80 +#define QCOM_ITCHOUT 0xE74 +#define QCOM_ITTRIGOUT 0xEA0 +#define QCOM_ITCHOUTACK 0xE78 +#define QCOM_ITTRIGOUTACK 0xEC0 +#define QCOM_ITCHIN 0xE7C +#define QCOM_ITTRIGIN 0xEE0 + /** * Group of related trigger signals * @@ -222,9 +247,8 @@ int cti_enable(struct coresight_device *csdev, enum cs_mode mode, int cti_disable(struct coresight_device *csdev, struct coresight_path *path); void cti_write_all_hw_regs(struct cti_drvdata *drvdata); void cti_write_intack(struct device *dev, u32 ackval); -void cti_write_single_reg(struct cti_drvdata *drvdata, u32 off, u32 index, - u32 value); -u32 cti_read_single_reg(struct cti_drvdata *drvdata, u32 off, u32 index); int cti_channel_trig_op(struct device *dev, enum cti_chan_op op, enum cti_trig_dir direction, u32 channel_idx, u32 trigger_idx); @@ -237,6 +261,78 @@ struct coresight_platform_data * coresight_cti_get_platform_data(struct device *dev); const char *cti_plat_get_node_name(struct fwnode_handle *fwnode); +static inline u32 cti_qcom_reg_off(u32 offset) +{ + switch (offset) { + case CTIINTACK: return QCOM_CTIINTACK; + case CTIAPPSET: return QCOM_CTIAPPSET; + case CTIAPPCLEAR: return QCOM_CTIAPPCLEAR; + case CTIAPPPULSE: return QCOM_CTIAPPPULSE; + case CTIINEN: return QCOM_CTIINEN; + case CTIOUTEN: return QCOM_CTIOUTEN; + case CTITRIGINSTATUS: return QCOM_CTITRIGINSTATUS; + case CTITRIGOUTSTATUS: return QCOM_CTITRIGOUTSTATUS; + case CTICHINSTATUS: return QCOM_CTICHINSTATUS; + case CTICHOUTSTATUS: return QCOM_CTICHOUTSTATUS; + case CTIGATE: return QCOM_CTIGATE; + case ASICCTL: return QCOM_ASICCTL; + case ITCHINACK: return QCOM_ITCHINACK; + case ITTRIGINACK: return QCOM_ITTRIGINACK; + case ITCHOUT: return QCOM_ITCHOUT; + case ITTRIGOUT: return QCOM_ITTRIGOUT; + case ITCHOUTACK: return QCOM_ITCHOUTACK; + case ITTRIGOUTACK: return QCOM_ITTRIGOUTACK; + case ITCHIN: return QCOM_ITCHIN; + case ITTRIGIN: return QCOM_ITTRIGIN; + + default: + return offset; + } +} + +static inline void __iomem *__reg_addr(struct cti_drvdata *drvdata, + u32 off, u32 index) +{ + if (unlikely(drvdata->is_qcom_cti)) + off = cti_qcom_reg_off(off); + + return drvdata->base + off + index * sizeof(u32); +} + +#define reg_addr(drvdata, off) __reg_addr((drvdata), (off), 0) +#define reg_index_addr(drvdata, off, i) __reg_addr((drvdata), (off), (i)) + +static inline u32 cti_read_single_reg_index(struct cti_drvdata *drvdata, + u32 off, u32 index) +{ + u32 val; + + CS_UNLOCK(drvdata->base); + val = readl_relaxed(reg_index_addr(drvdata, off, index)); + CS_LOCK(drvdata->base); + + return val; +} + +static inline u32 cti_read_single_reg(struct cti_drvdata *drvdata, u32 off) +{ + return cti_read_single_reg_index(drvdata, off, 0); +} + +static inline void cti_write_single_reg_index(struct cti_drvdata *drvdata, + u32 off, u32 index, u32 value) +{ + CS_UNLOCK(drvdata->base); + writel_relaxed(value, reg_index_addr(drvdata, off, index)); + CS_LOCK(drvdata->base); +} + +static inline void cti_write_single_reg(struct cti_drvdata *drvdata, + u32 off, u32 value) +{ + cti_write_single_reg_index(drvdata, off, 0, value); +} + /* Check if a cti device is enabled */ static inline bool cti_is_active(struct cti_config *cfg) {

3 months, 1 week

Re: [PATCH v3 0/3] Enable CTCU and ETR devices for multiple QCOM platforms

by Suzuki K Poulose

On 29/05/2026 11:26, Jie Gan wrote: > > > On 2/27/2026 6:10 PM, Suzuki K Poulose wrote: >> Hello, >> >> >> On 04/02/2026 02:22, Jie Gan wrote: >>> The DT‑binding patch adds platform‑specific compatibles for the >>> CTCU device, and the following Qualcomm platforms are included: >>> Kaanapali >>> Pakala(sm8750) >>> Hamoa(x1e80100) >>> Glymur >> >> Given this is predominantly DTS changes, and there is very low chances >> of a conflict with the binding yaml change, I would recommend this to go >> via soc or the qcom platform tree. >> >> For the series: >> >> Acked-by: Suzuki K Poulose <suzuki.poulose(a)arm.com> > > Hi Suzuki, > > May I ask is there a chance this patch series could go through the > CoreSight tree? Like I said, it is mostly Qcom platform changes. So, I would leave it to the appropriate channel Suzuki > > Thanks a lot. > Jie > >> >> >>> >>> Since the base Coresight DT patches for the Kaanapali and Glymur >>> platforms have not yet been applied, I created DT patches only >>> for the Pakala and Hamoa platforms. I will submit the Kaanapali >>> and Glymur patches once their corresponding base Coresight DT patches >>> are merged. >>> >>> The Hamoa‑related patches were posted in a separate email, and I >>> have included them in the current patch series. >>> >>> Link to the previous Hamoa patch series: >>> https://lore.kernel.org/all/20251106-enable-etr-and-ctcu-for-hamoa- >>> v2-0-cdb3a18753aa(a)oss.qualcomm.com/ >>> >>> Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> >>> --- >>> Changes in v3: >>> - change back to the numeric compatible from hamoa to x1e80100. >>> - Link to v2: https://lore.kernel.org/r/20260203-enable-ctcu-and-etr- >>> v2-0-aacc7bd7eccb(a)oss.qualcomm.com >>> >>> Changes in v2: >>> - change back to the numeric compatible from pakala to sm8750. >>> - Link to v1: https://lore.kernel.org/r/20260203-enable-ctcu-and-etr- >>> v1-0-a5371a2ec2b8(a)oss.qualcomm.com >>> >>> --- >>> Jie Gan (3): >>> dt-binding: document QCOM platforms for CTCU device >>> arm64: dts: qcom: hamoa: enable ETR and CTCU devices >>> arm64: dts: qcom: sm8750: enable ETR and CTCU devices >>> >>> .../bindings/arm/qcom,coresight-ctcu.yaml | 4 + >>> arch/arm64/boot/dts/qcom/hamoa.dtsi | 160 ++++++++++ >>> + +++++++- >>> arch/arm64/boot/dts/qcom/sm8750.dtsi | 177 ++++++++++ >>> + ++++++++++ >>> 3 files changed, 340 insertions(+), 1 deletion(-) >>> --- >>> base-commit: 193579fe01389bc21aff0051d13f24e8ea95b47d >>> change-id: 20260203-enable-ctcu-and-etr-31f9e9d1088d >>> >>> Best regards, >> >

3 months, 1 week

Re: [PATCH] coresight: platform: defer connection counter increment until alloc succeeds

by Suzuki K Poulose

On Mon, 11 May 2026 12:19:18 +0800, Jie Gan wrote: > coresight_add_out_conn() increments nr_outconns before calling > devm_krealloc_array() and again before devm_kmalloc(). If either > allocation fails, the counter is already bumped while the corresponding > array entry is NULL or uninitialized garbage. > > coresight_add_in_conn() has the same problem with nr_inconns and > devm_krealloc_array(). > > [...] Applied, thanks! [1/1] coresight: platform: defer connection counter increment until alloc succeeds https://git.kernel.org/coresight/c/1563ae33dc4f Best regards, -- Suzuki K Poulose <suzuki.poulose(a)arm.com>

3 months, 1 week

Re: [PATCH] coresight: etb10: restore atomic_t for shared reading state

by Suzuki K Poulose

On Fri, 29 May 2026 00:52:01 +0800, Runyu Xiao wrote: > The etb10 miscdevice uses drvdata->reading as a shared exclusivity gate > for userspace buffer access. etb_open() claims that gate with > local_cmpxchg(), and etb_release() clears it with local_set(). > > That gate is shared per-device state rather than CPU-local state. A > running system can reach it whenever /dev/<etb> is opened, closed, and > reopened by different tasks while the device remains registered, so the > same drvdata->reading variable may be claimed on one CPU and later > cleared on another. > > [...] Applied, thanks! [1/1] coresight: etb10: restore atomic_t for shared reading state https://git.kernel.org/coresight/c/fa09f08ede3d Best regards, -- Suzuki K Poulose <suzuki.poulose(a)arm.com>

3 months, 1 week

Re: [PATCH] coresight: etb10: restore atomic_t for shared reading state

by James Clark

On 28/05/2026 5:52 pm, Runyu Xiao wrote: > The etb10 miscdevice uses drvdata->reading as a shared exclusivity gate > for userspace buffer access. etb_open() claims that gate with > local_cmpxchg(), and etb_release() clears it with local_set(). > > That gate is shared per-device state rather than CPU-local state. A > running system can reach it whenever /dev/<etb> is opened, closed, and > reopened by different tasks while the device remains registered, so the > same drvdata->reading variable may be claimed on one CPU and later > cleared on another. > > This code used to use atomic_t for the same gate, but commit > 27b10da8fff2 ("coresight: etb10: moving to local atomic operations") > changed it to local_t even though the access pattern remained cross-task > and cross-CPU. Restore atomic_t together with atomic_cmpxchg() and > atomic_set() so the exclusivity gate again uses a primitive intended > for shared state. > > The issue was found on Linux v6.18.21 by our static analysis tool while > scanning surviving local_t-on-shared-state sites, and then manually > reviewed against the live etb10 file-op path. > > It was runtime-validated with a reproducible QEMU no-device KCSAN PoC > that kept the same report-local contract: > > 1. use one shared struct etb_drvdata carrier and its > drvdata->reading gate; > 2. call etb_open() and etb_release() sequentially on that gate to > confirm the original claim/clear path; > 3. bind the open side to CPU0 and the release side to CPU1 for the > same gate to show cross-CPU ownership; > 4. run bound workers that repeatedly race etb_open() and > etb_release() on the same gate until KCSAN reports a target hit. > > The harness recorded: > > L1 passed open=1 release=1 > reading_after_open=1 reading_after_release=0 > L2 passed open_cpu=0 release_cpu=1 > cross_cpu_release=1 reading_after=0 open_ret=0 > > Representative KCSAN excerpt from the no-device validation run: > > BUG: KCSAN: data-race in etb_open.constprop.0.isra.0 [vuln_msv] > > write to 0xffffffffc0003810 of 4 bytes by task 216 on cpu 1: > etb_open.constprop.0.isra.0+0x38/0x80 [vuln_msv] > l3_worker_thread_fn+0x4f/0xf0 [vuln_msv] > kthread+0x17e/0x1c0 > ret_from_fork+0x22/0x30 > > read to 0xffffffffc0003810 of 4 bytes by task 215 on cpu 0: > etb_open.constprop.0.isra.0+0x18/0x80 [vuln_msv] > l3_worker_thread_fn+0x4f/0xf0 [vuln_msv] > kthread+0x17e/0x1c0 > ret_from_fork+0x22/0x30 > > value changed: 0x00000000 -> 0x00000001 > > Reported by Kernel Concurrency Sanitizer on: > CPU: 0 PID: 215 Comm: etb10_l3_a Tainted: G O 6.1.66 #2 > > This no-device harness is not a real ETB10 hardware end-to-end run, but > it preserves the same shared drvdata->reading gate and the same > etb_open()/etb_release() claim/clear contract. No real ETB10 hardware > was available for runtime testing. > > Build-tested with: > make olddefconfig > make -j"$(nproc)" drivers/hwtracing/coresight/coresight-etb10.o > > Fixes: 27b10da8fff2 ("coresight: etb10: moving to local atomic operations") > Cc: stable(a)vger.kernel.org > Signed-off-by: Runyu Xiao <runyu.xiao(a)seu.edu.cn> > --- > drivers/hwtracing/coresight/coresight-etb10.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c > index 35db1b6093d1..98269ea6f7ae 100644 > --- a/drivers/hwtracing/coresight/coresight-etb10.c > +++ b/drivers/hwtracing/coresight/coresight-etb10.c > @@ -85,7 +85,7 @@ struct etb_drvdata { > struct coresight_device *csdev; > struct miscdevice miscdev; > raw_spinlock_t spinlock; > - local_t reading; > + atomic_t reading; > pid_t pid; > u8 *buf; > u32 buffer_depth; > @@ -603,7 +603,7 @@ static int etb_open(struct inode *inode, struct file *file) > struct etb_drvdata *drvdata = container_of(file->private_data, > struct etb_drvdata, miscdev); > > - if (local_cmpxchg(&drvdata->reading, 0, 1)) > + if (atomic_cmpxchg(&drvdata->reading, 0, 1)) > return -EBUSY; > > dev_dbg(&drvdata->csdev->dev, "%s: successfully opened\n", __func__); > @@ -641,7 +641,7 @@ static int etb_release(struct inode *inode, struct file *file) > { > struct etb_drvdata *drvdata = container_of(file->private_data, > struct etb_drvdata, miscdev); > - local_set(&drvdata->reading, 0); > + atomic_set(&drvdata->reading, 0); > > dev_dbg(&drvdata->csdev->dev, "%s: released\n", __func__); > return 0; Reviewed-by: James Clark <james.clark(a)linaro.org> Semi-related to this change, etb_read() doesn't have any lock when reading drvdata->buffer_dept or drvdata->buf. It locks in etb_dump(), but then unlocks before actually calling copy_to_user(). Seems like concurrent calls to etb_read() might end up with corrupt data, although I'm not sure if that would ever happen in practice because it only allows one open file handle.

3 months, 1 week

Re: [PATCH v9 1/4] coresight: cti: Convert trigger usage fields to dynamic

by Leo Yan

On Thu, May 21, 2026 at 08:16:27PM +0800, Yingchao Deng wrote: [...] > @@ -231,6 +254,8 @@ struct cti_trig_con *cti_allocate_trig_con(struct device *dev, int in_sigs, > { > struct cti_trig_con *tc = NULL; > struct cti_trig_grp *in = NULL, *out = NULL; > + struct cti_drvdata *drvdata = dev_get_drvdata(dev); > + int n_trigs = drvdata->config.nr_trig_max; I don't mind it allocates bitmask with nr_trig_max, but AI review suggests that when in_sigs / out_sigs bigger than nr_trig_max, it might access memory out-of-boundary (see cti_plat_read_trig_group()). It is good to add a check: if (in_sigs > n_trigs || out_sigs > n_trigs) { dev_err(dev, "trigger signal is out of range: in=%d out=%d nr_max=%d\n", in_sigs, out_sigs, n_trigs\n"); return NULL; } With this: Reviewed-by: Leo Yan <leo.yan(a)arm.com> BTW, I have given my review tag on v8, please remember to update patches with review / ack tags.

3 months, 1 week

Re: [PATCH v9 3/4] coresight: cti: add Qualcomm extended CTI identification and quirks

by Leo Yan

On Thu, May 21, 2026 at 08:16:29PM +0800, Yingchao Deng wrote: [...] > Qualcomm implements an extended variant of the ARM CoreSight CTI with a > different register layout and vendor-specific behavior. While the > programming model remains largely compatible, the register offsets differ > from the standard ARM CTI and require explicit handling. I cannot apply this patch successfuly. Please rebase on the latest coresight-next branch. > @@ -726,6 +734,22 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id) > > raw_spin_lock_init(&drvdata->spinlock); > > + devarch = readl_relaxed(drvdata->base + CORESIGHT_DEVARCH); > + if (CTI_DEVARCH_ARCHITECT(devarch) == ARCHITECT_QCOM) { > + drvdata->is_qcom_cti = true; > + /* > + * QCOM CTI does not implement Claimtag functionality as > + * per CoreSight specification, but its CLAIMSET register > + * is incorrectly initialized to 0xF. This can mislead > + * tools or drivers into thinking the component is claimed. > + * > + * Reset CLAIMSET to 0 to reflect that no claims are active. > + */ > + CS_UNLOCK(drvdata->base); > + writel_relaxed(0, drvdata->base + CORESIGHT_CLAIMSET); > + CS_LOCK(drvdata->base); Sorry I missed this piece before. Can you move this quirk into firmware? I don't think the CTI driver should clear the external claim bit as this totally break the protocol defined in PSCI. A clean way would clear the bits in firmware and then CTI driver can use the CLAIM registers. Or, another option is to create several helpers to bypass claim operations for Qcom CTI: static void cti_clear_self_claim_tag(cti_drvdata *drvdata, struct csdev_access *csa) { if (drvdata->is_qcom_cti) return; coresight_clear_self_claim_tag(csa); } static int cti_claim_device(cti_drvdata *drvdata) { if (drvdata->is_qcom_cti) return 0; return coresight_claim_device(drvdata->csdev); } static void cti_unclaim_device_unlocked(cti_drvdata *drvdata) { if (drvdata->is_qcom_cti) return; return coresight_disclaim_device_unlocked(drvdata->csdev); } Otherwise, this patch is fine for me. Thanks, Leo

3 months, 1 week

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

by Leo Yan

On Tue, May 19, 2026 at 04:48:07PM +0100, Yeoreum Yun wrote: [...] > --- a/drivers/hwtracing/coresight/coresight-etm4x-cfg.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-cfg.c > @@ -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. > u32 off_mask; > > if (((offset >= TRCEVENTCTL0R) && (offset <= TRCVIPCSSCTLR)) || > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index 9b477b36b432..f0a9f2ef4b27 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -243,6 +243,7 @@ void etm4_release_trace_id(struct etmv4_drvdata *drvdata) > struct etm4_enable_arg { > struct etmv4_drvdata *drvdata; > struct coresight_path *path; > + struct etmv4_config config; > int rc; > }; > > @@ -268,10 +269,11 @@ static void etm4x_prohibit_trace(struct etmv4_drvdata *drvdata) > static u64 etm4x_get_kern_user_filter(struct etmv4_drvdata *drvdata) > { > u64 trfcr = drvdata->trfcr; > + struct etmv4_config *config = &drvdata->active_config; > > - if (drvdata->config.mode & ETM_MODE_EXCL_KERN) > + if (config->mode & ETM_MODE_EXCL_KERN) > trfcr &= ~TRFCR_EL1_ExTRE; > - if (drvdata->config.mode & ETM_MODE_EXCL_USER) > + if (config->mode & ETM_MODE_EXCL_USER) > trfcr &= ~TRFCR_EL1_E0TRE; > > return trfcr; > @@ -279,7 +281,7 @@ static u64 etm4x_get_kern_user_filter(struct etmv4_drvdata *drvdata) > > /* > * etm4x_allow_trace - Allow CPU tracing in the respective ELs, > - * as configured by the drvdata->config.mode for the current > + * as configured by the drvdata->active_config.mode for the current > * session. Even though we have TRCVICTLR bits to filter the > * trace in the ELs, it doesn't prevent the ETM from generating > * a packet (e.g, TraceInfo) that might contain the addresses from > @@ -290,12 +292,13 @@ static u64 etm4x_get_kern_user_filter(struct etmv4_drvdata *drvdata) > static void etm4x_allow_trace(struct etmv4_drvdata *drvdata) > { > u64 trfcr, guest_trfcr; > + struct etmv4_config *config = &drvdata->active_config; > > /* If the CPU doesn't support FEAT_TRF, nothing to do */ > if (!drvdata->trfcr) > return; > > - if (drvdata->config.mode & ETM_MODE_EXCL_HOST) > + if (config->mode & ETM_MODE_EXCL_HOST) > trfcr = drvdata->trfcr & ~(TRFCR_EL1_ExTRE | TRFCR_EL1_E0TRE); > else > trfcr = etm4x_get_kern_user_filter(drvdata); > @@ -303,7 +306,7 @@ static void etm4x_allow_trace(struct etmv4_drvdata *drvdata) > write_trfcr(trfcr); > > /* Set filters for guests and pass to KVM */ > - if (drvdata->config.mode & ETM_MODE_EXCL_GUEST) > + if (config->mode & ETM_MODE_EXCL_GUEST) > guest_trfcr = drvdata->trfcr & ~(TRFCR_EL1_ExTRE | TRFCR_EL1_E0TRE); > else > guest_trfcr = etm4x_get_kern_user_filter(drvdata); > @@ -497,7 +500,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) > { > int i, rc; > const struct etmv4_caps *caps = &drvdata->caps; > - struct etmv4_config *config = &drvdata->config; > + struct etmv4_config *config = &drvdata->active_config; > struct coresight_device *csdev = drvdata->csdev; > struct device *etm_dev = &csdev->dev; > struct csdev_access *csa = &csdev->access; > @@ -618,27 +621,53 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) > 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? > + > + if (cfg_hash) { > + arg->rc = cscfg_csdev_enable_active_config(csdev, > + cfg_hash, > + preset); > + if (arg->rc) > + goto err; > + } > + > + drvdata->trcid = arg->path->trace_id; > + > + /* Tracer will never be paused in sysfs mode */ > + drvdata->paused = false; > + > + 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? > } > > + drvdata->sticky_enable = true; > csdev->path = arg->path; > + > + return; > +err: > + /* The tracer didn't start */ > + coresight_set_mode(csdev, CS_MODE_DISABLED); > } > > /* > @@ -676,7 +705,7 @@ static int etm4_config_timestamp_event(struct etmv4_drvdata *drvdata, > int ctridx; > int rselector; > const struct etmv4_caps *caps = &drvdata->caps; > - struct etmv4_config *config = &drvdata->config; > + struct etmv4_config *config = &drvdata->active_config; > > /* No point in trying if we don't have at least one counter */ > if (!caps->nr_cntr) > @@ -760,7 +789,7 @@ static int etm4_parse_event_config(struct coresight_device *csdev, > int ret = 0; > struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > const struct etmv4_caps *caps = &drvdata->caps; > - struct etmv4_config *config = &drvdata->config; > + struct etmv4_config *config = &drvdata->active_config; > struct perf_event_attr max_timestamp = { > .ATTR_CFG_FLD_timestamp_CFG = U64_MAX, > }; > @@ -922,25 +951,7 @@ static int etm4_enable_sysfs(struct coresight_device *csdev, struct coresight_pa > { > struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > struct etm4_enable_arg arg = { }; > - unsigned long cfg_hash; > - int ret, preset; > - > - /* enable any config activated by configfs */ > - cscfg_config_sysfs_get_active_cfg(&cfg_hash, &preset); > - if (cfg_hash) { > - ret = cscfg_csdev_enable_active_config(csdev, cfg_hash, preset); > - if (ret) { > - etm4_release_trace_id(drvdata); > - return ret; > - } > - } > - > - raw_spin_lock(&drvdata->spinlock); > - > - drvdata->trcid = path->trace_id; > - > - /* Tracer will never be paused in sysfs mode */ > - drvdata->paused = false; > + int ret; > > /* > * Executing etm4_enable_hw on the cpu whose ETM is being enabled > @@ -948,20 +959,20 @@ static int etm4_enable_sysfs(struct coresight_device *csdev, struct coresight_pa > */ > arg.drvdata = drvdata; > arg.path = path; > + > + raw_spin_lock(&drvdata->spinlock); > + arg.config = drvdata->config; > + raw_spin_unlock(&drvdata->spinlock); > + > ret = smp_call_function_single(drvdata->cpu, > etm4_enable_sysfs_smp_call, &arg, 1); > if (!ret) > ret = arg.rc; > if (!ret) > - drvdata->sticky_enable = true; > - > - if (ret) > + dev_dbg(&csdev->dev, "ETM tracing enabled\n"); > + else > etm4_release_trace_id(drvdata); > > - raw_spin_unlock(&drvdata->spinlock); > - > - if (!ret) > - dev_dbg(&csdev->dev, "ETM tracing enabled\n"); > return ret; > } > > @@ -1048,7 +1059,7 @@ static void etm4_disable_hw(struct etmv4_drvdata *drvdata) > { > u32 control; > const struct etmv4_caps *caps = &drvdata->caps; > - struct etmv4_config *config = &drvdata->config; > + struct etmv4_config *config = &drvdata->active_config; > struct coresight_device *csdev = drvdata->csdev; > struct csdev_access *csa = &csdev->access; > int i; > @@ -1091,6 +1102,8 @@ static void etm4_disable_sysfs_smp_call(void *info) > etm4_disable_hw(drvdata); > > drvdata->csdev->path = NULL; > + cscfg_csdev_disable_active_config(drvdata->csdev); Move cscfg_csdev_disable_active_config() just after etm4_disable_hw() so the sequence is consistent with perf mode. [...] > @@ -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. Thanks, Leo

3 months, 1 week

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

by Leo Yan

On Tue, May 19, 2026 at 04:48:04PM +0100, Yeoreum Yun wrote: [...] > @@ -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)); > > + Unexpected new line? > @@ -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); Since etm4_enable_hw() clears the TRCSSCSRn_STATUS and TRCSSCSRn_PENDING bits every time, here is no need to clear the status bits during probe. In the future, we may want to preserve the status within a session and clear it only when starting a new session. Clearing the status bits here still cannot handle stale status across multiple sessions, so we can defer this improvement for later. Thanks, Leo

3 months, 1 week

← Newer
1
...
7
8
9
10
11
12
13
...
284
Older →

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight