CoreSight April 2026

coresight@lists.linaro.org

5 participants
41 discussions

[PATCH] coresight: tmc: Fix overflow when calculating is bigger than 2GiB

by Leo Yan

When specifying a 2GB AUX buffer, the ETR driver ends up allocating only a 1MB buffer instead: # echo 'file coresight-tmc-etr.c +p' > \ /sys/kernel/debug/dynamic_debug/control # perf record -e cs_etm/@tmc_etr0,timestamp=0/u -C 0 -m ,2G -- test coresight tmc_etr0: allocated buffer of size 1024KB in mode 0 The page index is an 'int' type, and shifting it by PAGE_SHIFT overflows when the resulting value exceeds 2GB. This produces a negative value, causing the driver to fall back to the minimum buffer size (1MB). Cast the page index to a wider type to accommodate large buffer sizes. Also fix a similar issue in the buffer offset calculation. Reported-by: Michiel van Tol <michiel.vantol(a)arm.com> Fixes: 99443ea19e8b ("coresight: Add generic TMC sg table framework") Fixes: eebe8dbd8630 ("coresight: tmc: Decouple the perf buffer allocation from sysfs mode") Signed-off-by: Leo Yan <leo.yan(a)arm.com> --- drivers/hwtracing/coresight/coresight-tmc-etr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index cee82e52c4ea96b035f1db71b2d9a006bfc1c51e..990bbb721e1d712d7b93f1e36087fdaf9d3baa3b 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -154,7 +154,7 @@ tmc_pages_get_offset(struct tmc_pages *tmc_pages, dma_addr_t addr) for (i = 0; i < tmc_pages->nr_pages; i++) { page_start = tmc_pages->daddrs[i]; if (addr >= page_start && addr < (page_start + PAGE_SIZE)) - return i * PAGE_SIZE + (addr - page_start); + return (long)i * PAGE_SIZE + (addr - page_start); } return -EINVAL; @@ -1381,7 +1381,7 @@ alloc_etr_buf(struct tmc_drvdata *drvdata, struct perf_event *event, node = (event->cpu == -1) ? NUMA_NO_NODE : cpu_to_node(event->cpu); /* Use the minimum limit if the required size is smaller */ - size = nr_pages << PAGE_SHIFT; + size = (ssize_t)nr_pages << PAGE_SHIFT; size = max_t(ssize_t, size, TMC_ETR_PERF_MIN_BUF_SIZE); /* --- base-commit: eebe8dbd8630f51cf70b1f68a440cd3d7f7a914d change-id: 20260217-arm_coresight_fix_big_buffer_size-a8a41298369d Best regards, -- Leo Yan <leo.yan(a)arm.com>

1 day, 13 hours

[PATCH] coresight: etm4x: Correct TRCVMIDCCTLR1 save and restore

by Leo Yan

It is a typo to use trcvmidcctlr0 to save and restore TRCVMIDCCTLR1. Use trcvmidcctlr1 instead. Fixes: f5bd523690d2 ("coresight: etm4x: Convert all register accesses") Signed-off-by: Leo Yan <leo.yan(a)arm.com> --- drivers/hwtracing/coresight/coresight-etm4x-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index d565a73f0042e3e0b21fcf9cb94009cc25834d3d..c370b6bf45ef26aff29ee71630f86eae2487069a 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -1979,7 +1979,7 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata) state->trcvmidcctlr0 = etm4x_read32(csa, TRCVMIDCCTLR0); if (drvdata->numvmidc > 4) - state->trcvmidcctlr0 = etm4x_read32(csa, TRCVMIDCCTLR1); + state->trcvmidcctlr1 = etm4x_read32(csa, TRCVMIDCCTLR1); state->trcclaimset = etm4x_read32(csa, TRCCLAIMCLR); @@ -2102,7 +2102,7 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata) etm4x_relaxed_write32(csa, state->trcvmidcctlr0, TRCVMIDCCTLR0); if (drvdata->numvmidc > 4) - etm4x_relaxed_write32(csa, state->trcvmidcctlr0, TRCVMIDCCTLR1); + etm4x_relaxed_write32(csa, state->trcvmidcctlr1, TRCVMIDCCTLR1); etm4x_relaxed_write32(csa, state->trcclaimset, TRCCLAIMSET); --- base-commit: 971f3474f8898ae8bbab19a9b547819a5e6fbcf1 change-id: 20260408-arm_cs_fix_trcvmidcctlr1_typo-3382c27d1c28 Best regards, -- Leo Yan <leo.yan(a)arm.com>

1 day, 13 hours

[PATCH 0/2] coresight: ete: Always save state on power down

by James Clark

Fix PM save on ETE, which is an issue that showed up on the FVP when booted with ACPI and the newly enabled idle states. Then refactor to clarify and avoid any probe order issues. Signed-off-by: James Clark <james.clark(a)linaro.org> --- James Clark (2): coresight: ete: Always save state on power down coresight: etm4x: Refactor pm_save_enable handling drivers/hwtracing/coresight/coresight-etm4x-core.c | 55 ++++++++++++++++------ drivers/hwtracing/coresight/coresight-etm4x.h | 1 + 2 files changed, 42 insertions(+), 14 deletions(-) --- base-commit: 971f3474f8898ae8bbab19a9b547819a5e6fbcf1 change-id: 20260420-james-cs-ete-pm_save_enable-4e994e35cdac Best regards, -- James Clark <james.clark(a)linaro.org>

1 week, 4 days

Re: [PATCH v8 1/4] coresight: cti: Convert trigger usage fields to dynamic bitmaps and arrays

by Leo Yan

On Tue, Apr 28, 2026 at 10:25:11AM +0800, Yingchao Deng (Consultant) wrote: [...] > > tg->used_mask = bitmap_zalloc(nr_filter_sigs, GFP_KERNEL); > "nr_filter_sigs" is the count of entries in the DT property array, if the DT > property is: > arm,trig-filters = <22 23>; > Here nr_filter_sigs=2, so bitmap_zalloc(2) allocates only 1 unsigned long > (64 bits). set_bit(22/23, used_mask) still fits, but it's logically an OOB, > and any index >=64 would > write past the end. Thanks for explanation. It is correct for me.

2 weeks

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

by Leo Yan

On Sun, Apr 26, 2026 at 05:44:41PM +0800, Yingchao Deng wrote: > Qualcomm extended CTI implements banked trigger status and integration > registers, where each bank covers 32 triggers. Multiple instances of > these registers are required to expose the full trigger space. > > Add static sysfs entries for the banked CTI registers and control their > visibility based on the underlying hardware configuration. Numbered > sysfs nodes are hidden on standard ARM CTIs, preserving the existing ABI. > On Qualcomm CTIs, only banked registers backed by hardware are exposed, > with the number of visible banks derived from nr_trig_max. > > This ensures that userspace only sees registers that are actually > implemented, while maintaining compatibility with existing CTI tooling. > > Signed-off-by: Yingchao Deng <yingchao.deng(a)oss.qualcomm.com> > --- > drivers/hwtracing/coresight/coresight-cti-sysfs.c | 58 +++++++++++++++++++++++ > 1 file changed, 58 insertions(+) > > diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c > index 8b70e7e38ea3..046757e4e9b6 100644 > --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c > +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c > @@ -512,18 +512,36 @@ static struct attribute *coresight_cti_regs_attrs[] = { > &dev_attr_appclear.attr, > &dev_attr_apppulse.attr, > coresight_cti_reg(triginstatus, CTITRIGINSTATUS), > + coresight_cti_reg(triginstatus1, CTI_REG_SET_NR_CONST(CTITRIGINSTATUS, 1)), How about extend the cs_off_attribute struct: struct cs_off_attribute { struct device_attribute attr; u32 off; u32 index; }; // by default, the index is 0 #define coresight_cti_reg(name, offset) \ (&((struct cs_off_attribute[]) { \ { \ __ATTR(name, 0444, coresight_cti_reg_show, NULL), \ offset \ 0 \ } \ })[0].attr.attr) // For the register with index #define coresight_cti_reg_index(name, offset, index) \ (&((struct cs_off_attribute[]) { \ { \ __ATTR(name, 0444, coresight_cti_reg_show, NULL), \ offset \ index \ } \ })[0].attr.attr) coresight_cti_reg_index(triginstatus1, CTITRIGINSTATUS, 1), > + coresight_cti_reg(triginstatus2, CTI_REG_SET_NR_CONST(CTITRIGINSTATUS, 2)), > + coresight_cti_reg(triginstatus3, CTI_REG_SET_NR_CONST(CTITRIGINSTATUS, 3)), > coresight_cti_reg(trigoutstatus, CTITRIGOUTSTATUS), > + coresight_cti_reg(trigoutstatus1, CTI_REG_SET_NR_CONST(CTITRIGOUTSTATUS, 1)), > + coresight_cti_reg(trigoutstatus2, CTI_REG_SET_NR_CONST(CTITRIGOUTSTATUS, 2)), > + coresight_cti_reg(trigoutstatus3, CTI_REG_SET_NR_CONST(CTITRIGOUTSTATUS, 3)), > coresight_cti_reg(chinstatus, CTICHINSTATUS), > coresight_cti_reg(choutstatus, CTICHOUTSTATUS), > #ifdef CONFIG_CORESIGHT_CTI_INTEGRATION_REGS > coresight_cti_reg_rw(itctrl, CORESIGHT_ITCTRL), > coresight_cti_reg(ittrigin, ITTRIGIN), > + coresight_cti_reg(ittrigin1, CTI_REG_SET_NR_CONST(ITTRIGIN, 1)), > + coresight_cti_reg(ittrigin2, CTI_REG_SET_NR_CONST(ITTRIGIN, 2)), > + coresight_cti_reg(ittrigin3, CTI_REG_SET_NR_CONST(ITTRIGIN, 3)), > coresight_cti_reg(itchin, ITCHIN), > coresight_cti_reg_rw(ittrigout, ITTRIGOUT), > + coresight_cti_reg_rw(ittrigout1, CTI_REG_SET_NR_CONST(ITTRIGOUT, 1)), > + coresight_cti_reg_rw(ittrigout2, CTI_REG_SET_NR_CONST(ITTRIGOUT, 2)), > + coresight_cti_reg_rw(ittrigout3, CTI_REG_SET_NR_CONST(ITTRIGOUT, 3)), > coresight_cti_reg_rw(itchout, ITCHOUT), > coresight_cti_reg(itchoutack, ITCHOUTACK), > coresight_cti_reg(ittrigoutack, ITTRIGOUTACK), > + coresight_cti_reg(ittrigoutack1, CTI_REG_SET_NR_CONST(ITTRIGOUTACK, 1)), > + coresight_cti_reg(ittrigoutack2, CTI_REG_SET_NR_CONST(ITTRIGOUTACK, 2)), > + coresight_cti_reg(ittrigoutack3, CTI_REG_SET_NR_CONST(ITTRIGOUTACK, 3)), > coresight_cti_reg_wo(ittriginack, ITTRIGINACK), > + coresight_cti_reg_wo(ittriginack1, CTI_REG_SET_NR_CONST(ITTRIGINACK, 1)), > + coresight_cti_reg_wo(ittriginack2, CTI_REG_SET_NR_CONST(ITTRIGINACK, 2)), > + coresight_cti_reg_wo(ittriginack3, CTI_REG_SET_NR_CONST(ITTRIGINACK, 3)), > coresight_cti_reg_wo(itchinack, ITCHINACK), > #endif > NULL, > @@ -534,10 +552,50 @@ static umode_t coresight_cti_regs_is_visible(struct kobject *kobj, > { > struct device *dev = kobj_to_dev(kobj); > struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent); > + static const char * const qcom_suffix_registers[] = { > + "triginstatus", > + "trigoutstatus", > +#ifdef CONFIG_CORESIGHT_CTI_INTEGRATION_REGS > + "ittrigin", > + "ittrigout", > + "ittriginack", > + "ittrigoutack", > +#endif > + }; > + int i, nr, max_bank; > + size_t len; > > if (attr == &dev_attr_asicctl.attr && !drvdata->config.asicctl_impl) > return 0; > > + /* > + * Banked regs are exposed as <qcom_suffix_registers><nr> (nr = 1..3). > + * - Hide them on standard CTIs. > + * - On QCOM CTIs, hide suffixes beyond the number of banks implied > + * by nr_trig_max (32 triggers per bank). > + */ > + for (i = 0; i < ARRAY_SIZE(qcom_suffix_registers); i++) { This can be general for a register with index? Like: for (i = 0; i < ARRAY_SIZE(registers_with_index); i++) { > + len = strlen(qcom_suffix_registers[i]); > + > + if (strncmp(attr->name, qcom_suffix_registers[i], len)) > + continue; > + > + if (kstrtoint(attr->name + len, 10, &nr)) > + continue; > + > + if (!drvdata->is_qcom_cti) > + return 0; > + > + if (nr < 1 || nr > 3) > + return 0; > + > + max_bank = DIV_ROUND_UP(drvdata->config.nr_trig_max, 32) - 1; > + if (nr > max_bank) > + return 0; Directly check the attr's index here? struct cs_off_attribute *cti_attr = container_of(attr, struct cs_off_attribute, attr); max_bank = DIV_ROUND_UP(drvdata->config.nr_trig_max, 32); if (cti_attr->index >= max_bank) return 0; Thanks, Leo

2 weeks, 1 day

Re: [PATCH v8 2/4] coresight: cti: encode trigger register index in register offsets

by Leo Yan

On Sun, Apr 26, 2026 at 05:44:39PM +0800, Yingchao Deng wrote: > Introduce a small encoding to carry the register index together with the > base offset in a single u32, and use a common helper to compute the final > MMIO address. This refactors register access to be based on the encoded > (reg, nr) pair, reducing duplicated arithmetic and making it easier to > support variants that bank or relocate trigger-indexed registers. > > Signed-off-by: Yingchao Deng <yingchao.deng(a)oss.qualcomm.com> > --- > drivers/hwtracing/coresight/coresight-cti-core.c | 31 +++++++++++++++-------- > drivers/hwtracing/coresight/coresight-cti-sysfs.c | 4 +-- > drivers/hwtracing/coresight/coresight-cti.h | 16 ++++++++++-- > 3 files changed, 36 insertions(+), 15 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c > index 4e7d12bd2d3e..c4cbeb64365b 100644 > --- a/drivers/hwtracing/coresight/coresight-cti-core.c > +++ b/drivers/hwtracing/coresight/coresight-cti-core.c > @@ -42,6 +42,14 @@ static DEFINE_MUTEX(ect_mutex); > #define csdev_to_cti_drvdata(csdev) \ > dev_get_drvdata(csdev->dev.parent) > > +static void __iomem *cti_reg_addr(struct cti_drvdata *drvdata, int reg) > +{ > + u32 offset = CTI_REG_CLR_NR(reg); > + u32 nr = CTI_REG_GET_NR(reg); > + > + return drvdata->base + offset + sizeof(u32) * nr; > +} Could you try below change, which is more straightforward? static void __iomem *__reg_addr(struct cti_drvdata *drvdata, int off, int index) { return drvdata->base + offset + 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)) > + > /* write set of regs to hardware - call with spinlock claimed */ > void cti_write_all_hw_regs(struct cti_drvdata *drvdata) > { > @@ -55,16 +63,17 @@ void cti_write_all_hw_regs(struct cti_drvdata *drvdata) > > /* write the CTI trigger registers */ > for (i = 0; i < config->nr_trig_max; i++) { > - writel_relaxed(config->ctiinen[i], drvdata->base + CTIINEN(i)); > + writel_relaxed(config->ctiinen[i], > + cti_reg_addr(drvdata, CTI_REG_SET_NR(CTIINEN, i))); writel_relaxed(config->ctiinen[i], reg_index_addr(drvdata, CTIINEN, i)); > writel_relaxed(config->ctiouten[i], > - drvdata->base + CTIOUTEN(i)); > + cti_reg_addr(drvdata, CTI_REG_SET_NR(CTIOUTEN, i))); writel_relaxed(config->ctiouten[i], reg_index_addr(drvdata, CTIOUTEN, i)); [...] > +/* > + * Encode CTI register offset and register index in one u32: > + * - bits[0:11] : base register offset (0x000 to 0xFFF) > + * - bits[24:31] : register index (nr) > + */ > +#define CTI_REG_NR_MASK GENMASK(31, 24) > +#define CTI_REG_GET_NR(reg) FIELD_GET(CTI_REG_NR_MASK, (reg)) > +#define CTI_REG_SET_NR_CONST(reg, nr) ((reg) | FIELD_PREP_CONST(CTI_REG_NR_MASK, (nr))) > +#define CTI_REG_SET_NR(reg, nr) ((reg) | FIELD_PREP(CTI_REG_NR_MASK, (nr))) > +#define CTI_REG_CLR_NR(reg) ((reg) & (~CTI_REG_NR_MASK)) I know this might come from my suggestion, and it is also will be heavily used in patch 04. We can have strightforward way to implement this, please drop these macros. I will reply in patch 04 separately. Sorry my review might cause extra effort. Thanks, Leo

2 weeks, 1 day

Re: [PATCH v8 1/4] coresight: cti: Convert trigger usage fields to dynamic bitmaps and arrays

by Leo Yan

On Sun, Apr 26, 2026 at 05:44:38PM +0800, Yingchao Deng wrote: [...] > @@ -316,23 +316,33 @@ static int cti_plat_process_filter_sigs(struct cti_drvdata *drvdata, > { > struct cti_trig_grp *tg = NULL; > int err = 0, nr_filter_sigs; > + int nr_trigs = drvdata->config.nr_trig_max; > > nr_filter_sigs = cti_plat_count_sig_elements(fwnode, > CTI_DT_FILTER_OUT_SIGS); > if (nr_filter_sigs == 0) > return 0; > > - if (nr_filter_sigs > drvdata->config.nr_trig_max) > + if (nr_filter_sigs > nr_trigs) > return -EINVAL; > > tg = kzalloc_obj(*tg); > if (!tg) > return -ENOMEM; > > + tg->used_mask = bitmap_zalloc(nr_trigs, GFP_KERNEL); Here would be: tg->used_mask = bitmap_zalloc(nr_filter_sigs, GFP_KERNEL); > + if (!tg->used_mask) { > + kfree(tg); > + return -ENOMEM; > + } > + It is likely this will have merge conflict with the new patch [1]. You might need to rebase this patch on the top of [1]. We need to give [1] priority as it is a fix. [1] https://lore.kernel.org/linux-arm-kernel/20260426-nr_sigs-v1-1-3b9df99dab97… Otherwise, LGTM: Reviewed-by: Leo Yan <leo.yan(a)arm.com>

2 weeks, 1 day

Re: [PATCH] coresight: cti: Fix DT filter signals silently ignored

by Leo Yan

On Sun, Apr 26, 2026 at 05:59:34PM +0800, Yingchao Deng wrote: > In cti_plat_process_filter_sigs(), after allocating a temporary > cti_trig_grp struct via kzalloc_obj(), the code never assigns tg->nr_sigs > = nr_filter_sigs. Since kzalloc zero-initialises the struct, tg->nr_sigs > remains 0. cti_plat_read_trig_group() guards with: > if (!tgrp->nr_sigs) > return 0; > > so it returns immediately without reading any signal indices from DT. > > Fix by assigning tg->nr_sigs before calling cti_plat_read_trig_group(). > > Fixes: a5614770ab97 ("coresight: cti: Add device tree support for custom CTI") > Signed-off-by: Yingchao Deng <yingchao.deng(a)oss.qualcomm.com> > --- > drivers/hwtracing/coresight/coresight-cti-platform.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/hwtracing/coresight/coresight-cti-platform.c b/drivers/hwtracing/coresight/coresight-cti-platform.c > index 4eff96f48594..d6d5388705c3 100644 > --- a/drivers/hwtracing/coresight/coresight-cti-platform.c > +++ b/drivers/hwtracing/coresight/coresight-cti-platform.c > @@ -329,6 +329,7 @@ static int cti_plat_process_filter_sigs(struct cti_drvdata *drvdata, > if (!tg) > return -ENOMEM; > > + tg->nr_sigs = nr_filter_sigs; > err = cti_plat_read_trig_group(tg, fwnode, CTI_DT_FILTER_OUT_SIGS); I checked the naming, seems tg->nr_sigs is the right field to store the number. LGTM: Reviewed-by: Leo Yan <leo.yan(a)arm.com>

2 weeks, 1 day

Re: [PATCH v5 04/12] coresight: etm4x: exclude ss_status from drvdata->config

by Mike Leach

Hi, On 4/21/26 11:30, Yeoreum Yun wrote: >> Hi Mike, >> >>> Hi, >>> >>> This register [bit 31] indicates if a single shot comparator has matched. So >>> read-back provides information to the user post run to determine which if >>> any of the comparators set in this way has actually matched. >> >> Okay. so after disable sysfs session, to check former session >> check whether comprator has matched. >> >>> >>> Moreover, the specification states "Software must reset this bit to 0 to >>> re-enable single-shot control" and "Reset state is unknown. STATUS must be >>> written to set an initial state...." >>> >>> Therefore this register must be written as part of any configuration so >>> should be available in the drvdata->config for both read and write, >> >> But I don't think this is the reason for locate ss_status into "config" >> since its write purpose is not to configure but the "clear" former bit. >> That's why I think it's enough to clear when the new sysfs session starts. >> > > IOW, I think it's better to remove ss_status from configfs item > and > - add field ss_cmp in etm4_cpas > - add another field ss_status under "etm4_drvdata" to show "PENDING > and STATUS" bits to sysfs after finishing session. > > Is is valid for you? > No. Why two different locations for a single register read? If I have the ETMv4 hardware manual I am going to look for a something that is recognizable as being related to the single shot comparator status register(s). So in sysfs I would expect to see all the bits from the register, displayed, without masking off the STATUS and PENDING bits as happens now. In the code I would expect to see a single location with a sensible name - ss_cmp doesn't really correlate terribly well with TRCSSCSR. If you do not like the original ss_status, then ss_cmp_status may actually be better, ss_cmp could be either the ss comparator status or control register. Regards Mike >> -- >> Sincerely, >> Yeoreum Yun >> > > -- > Sincerely, > Yeoreum Yun

3 weeks

Re: [PATCH v5 07/12] coresight: etm4x: fix inconsistencies with sysfs configuration

by Leo Yan

On Tue, Apr 21, 2026 at 12:14:20PM +0100, Yeoreum Yun wrote: [...] > > > - There is a risk of corrupting drvdata->config if a perf session enables > > > tracing while cscfg_csdev_disable_active_config() is being handled in > > > etm4_disable_sysfs(). > > > > Similiar to above, cscfg_csdev_disable_active_config() is not > > protected in etm4_disable_sysfs(). > > This is not true. > at the time of etm4_disable_sysfs() "mode" is already taken > (whether sysfs or perf). In this situation, it's enough to > call cscfg_csdev_disable_active_config() before chaning > mode to DISABLED. To be clear, I am trying to understand issue _before_ your patch. Without this patch, cscfg_csdev_disable_active_config() is not protected by the mode. > > > struct etm4_enable_arg { > > > struct etmv4_drvdata *drvdata; > > > + struct etmv4_config config; > > > > We don't need this. We can defer to get drvdata->config in SMP call. > > This is not true ane make a situation more complex. > If we get config in SMP call, that would be a problem when some user is > trying to modify config. > > IOW, while user modifying config via sysfs, and SMP call happens, > it makes a dead lock. so for this if we try to grab the lock in SMP call, > we should change every sysfs raw_spin_lock() into raw_spin_lock_irqsave(). > > Instead of this it would be much clear and simpler to get config in here > rather than to add some latencies via sysfs. Thanks for info. If so, it is fine for me to add "config". > > > @@ -1386,6 +1401,7 @@ static void etm4_init_arch_data(void *info) > > > drvdata = dev_get_drvdata(init_arg->dev); > > > caps = &drvdata->caps; > > > csa = init_arg->csa; > > > + config = &drvdata->active_config; > > > > Should we init drvdata->config instead so make it has sane values ? > > > > In other words, drvdata->active_config are always set at the runtime, > > so don't need to init it at all, right? > > No. at least when the initialise, I think we should fill the its > contesnt with the "etm4_set_default()". > > That's why the consequence call etm4_set_default() called with > active_config and config is coped with the default configutation. I'm concerned that some config fields may be reused across sessions. For example, a sysfs session copies drvdata->config into drvdata->active_config, and later a perf session may reuse stale values. The same issue can happen in the reverse direction. A clean approach would be to treat drvdata->active_config as per-session state: 1) clear drvdata->active_config at session start 2) apply the session-specific config 2.1) sysfs: use drvdata->config 2.2) perf: use event config 3) then apply configfs configuration So we should clear drvdata->active_config at the start of each session and rebuild it with the correct configuration. Thanks, Leo

3 weeks

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight April 2026