CoreSight

coresight@lists.linaro.org

6 participants
2467 discussions

Re: [PATCH] coresight: trbe: Use scope-based resource management in arm_trbe_alloc_buffer()

by Anshuman Khandual

On 09/09/25 5:58 PM, Markus Elfring wrote: > From: Markus Elfring <elfring(a)users.sourceforge.net> > Date: Tue, 9 Sep 2025 14:20:06 +0200 > > Scope-based resource management became supported for some > programming interfaces by contributions of Peter Zijlstra on 2023-05-26. > See also the commit 54da6a0924311c7cf5015533991e44fb8eb12773 ("locking: ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ SHA ID here just needs first 12 digits not the entire string. > Introduce __cleanup() based infrastructure"). > > * Thus use the attribute “__free(kfree)”. > > * Reduce the scopes for the local variables “buf” and “pglist”. What is that required ? > > * Omit four kfree() calls accordingly. Right. The commit message should be re-written with little more context from arm_trbe_alloc_buffer(), after describing the new scope-based resource management. > > Signed-off-by: Markus Elfring <elfring(a)users.sourceforge.net> > --- > drivers/hwtracing/coresight/coresight-trbe.c | 21 ++++++++------------ > 1 file changed, 8 insertions(+), 13 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c > index 8f9bbef71f23..1b0d58bf8613 100644 > --- a/drivers/hwtracing/coresight/coresight-trbe.c > +++ b/drivers/hwtracing/coresight/coresight-trbe.c > @@ -733,8 +733,6 @@ static void *arm_trbe_alloc_buffer(struct coresight_device *csdev, > struct perf_event *event, void **pages, > int nr_pages, bool snapshot) > { > - struct trbe_buf *buf; > - struct page **pglist; > int i; > > /* > @@ -746,32 +744,29 @@ static void *arm_trbe_alloc_buffer(struct coresight_device *csdev, > if (nr_pages < 2) > return NULL; > > - buf = kzalloc_node(sizeof(*buf), GFP_KERNEL, trbe_alloc_node(event)); > + struct trbe_buf *buf __free(kfree) = kzalloc_node(sizeof(*buf), > + GFP_KERNEL, > + trbe_alloc_node(event)); > if (!buf) > return NULL; > > - pglist = kcalloc(nr_pages, sizeof(*pglist), GFP_KERNEL); > - if (!pglist) { > - kfree(buf); > + struct page **pglist __free(kfree) = kcalloc(nr_pages, sizeof(*pglist), GFP_KERNEL); > + if (!pglist) > return NULL; > - } > > for (i = 0; i < nr_pages; i++) > pglist[i] = virt_to_page(pages[i]); > > buf->trbe_base = (unsigned long)vmap(pglist, nr_pages, VM_MAP, PAGE_KERNEL); > - if (!buf->trbe_base) { > - kfree(pglist); > - kfree(buf); > + if (!buf->trbe_base) > return NULL; > - } > + > buf->trbe_limit = buf->trbe_base + nr_pages * PAGE_SIZE; > buf->trbe_write = buf->trbe_base; > buf->snapshot = snapshot; > buf->nr_pages = nr_pages; > buf->pages = pages; > - kfree(pglist); > - return buf; > + return_ptr(buf); > } > > static void arm_trbe_free_buffer(void *config) Seems like a good idea though. But please keep the local variable declaration in the current place which is bit cleaner and reduces code churn as well. diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 7a226316c48f..7babba1a9afb 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -733,8 +733,8 @@ static void *arm_trbe_alloc_buffer(struct coresight_device *csdev, struct perf_event *event, void **pages, int nr_pages, bool snapshot) { - struct trbe_buf *buf; - struct page **pglist; + struct trbe_buf *buf __free(kfree); + struct page **pglist __free(kfree); int i; /* @@ -751,27 +751,22 @@ static void *arm_trbe_alloc_buffer(struct coresight_device *csdev, return NULL; pglist = kcalloc(nr_pages, sizeof(*pglist), GFP_KERNEL); - if (!pglist) { - kfree(buf); + if (!pglist) return NULL; - } for (i = 0; i < nr_pages; i++) pglist[i] = virt_to_page(pages[i]); buf->trbe_base = (unsigned long)vmap(pglist, nr_pages, VM_MAP, PAGE_KERNEL); - if (!buf->trbe_base) { - kfree(pglist); - kfree(buf); + if (!buf->trbe_base) return NULL; - } + buf->trbe_limit = buf->trbe_base + nr_pages * PAGE_SIZE; buf->trbe_write = buf->trbe_base; buf->snapshot = snapshot; buf->nr_pages = nr_pages; buf->pages = pages; - kfree(pglist); - return buf; + return_ptr(buf); } static void arm_trbe_free_buffer(void *config)

18 hours, 7 minutes

Re: [PATCH v2] coresight: Fix possible deadlock in coresight_panic_cb

by Leo Yan

Hi Sean, On Thu, Sep 11, 2025 at 11:33:15AM -0400, Sean Anderson wrote: > coresight_panic_cb is called with interrupts disabled during panics. > However, bus_for_each_dev calls bus_to_subsys which takes > bus_kset->list_lock without disabling IRQs. This will cause a deadlock > if a panic occurs while one of the other coresight functions that uses > bus_for_each_dev is running. The decription is a bit misleading. Even when IRQ is disabled, if an exception happens, a CPU still can be trapped for handling kernel panic. > Maintain a separate list of coresight devices to access during a panic. Rather than maintaining a separate list and introducing a new spinlock, I would argue if we can simply register panic notifier in TMC ETR and ETF drviers (see tmc_panic_sync_etr() and tmc_panic_sync_etf()). If there is no dependency between CoreSight modules in panic sync flow, it is not necessary to maintain list (and lock) for these modules. I have not involved in panic patches before, so I would like to know the maintainers' opinion. Thanks, Leo

18 hours, 59 minutes

[PATCH v1 0/5] coresight: Fix device registration and unregistration

by Leo Yan

This series is to fix device registration and unregistration. The first patch addresses the resource is not released properly for a failure case during a device registration. The second patch is to use mutex to protect unregistration flow. The last three patches are for refactoring. Patch 03 explicitly uses the parent device handler. Patch 04 separates the success and failure flows for code readable and easier maintenance. Patch 05 improves the error handling by invoking specific functions for resource cleanup. Leo Yan (5): coresight: Correct sink ID map allocation failure handling coresight: Protect unregistration with mutex coresight: Explicitly use the parent device handler coresight: Separate failure and success flows coresight: Refine error handling for device registration drivers/hwtracing/coresight/coresight-core.c | 67 +++++++++++--------- 1 file changed, 37 insertions(+), 30 deletions(-) -- 2.34.1

1 day, 18 hours

[PATCH v3 0/3] Coresight TMC-ETR some bugfixes and cleanups

by Junhao He

This patchset builds upon Yicong's previous patches [1]. Introducing fix two race issues found by using TMC-ETR and CATU, Two cleanups found when debugging the issues. [1] https://lore.kernel.org/linux-arm-kernel/20241202092419.11777-1-yangyicong@… --- Changes in v3: - Patches 1: Additional comment for tmc_drvdata::etr_mode. Update comment for tmc_drvdata::reading with Jonathan's Tag. - Patches 2: Replace scoped_guard with guard with Jonathan's Tag. - Patches 2: Fix spinlock to raw_spinlock, and refactor this code based on Leo's suggested solution. - Patches 3: change the size's type to ssize_t and use max_t to simplify the code with Leo's Tag. Link: https://lore.kernel.org/linux-arm-kernel/20250620075412.952934-1-hejunhao3@… Changes in v2: - Updated the commit of patch2. - Rebase to v6.16-rc1 Junhao He (1): coresight: tmc: refactor the tmc-etr mode setting to avoid race conditions Yicong Yang (2): coresight: tmc: Add missing doc including reading and etr_mode of struct tmc_drvdata coresight: tmc: Decouple the perf buffer allocation from sysfs mode .../hwtracing/coresight/coresight-tmc-etr.c | 110 ++++++++---------- drivers/hwtracing/coresight/coresight-tmc.h | 2 + 2 files changed, 53 insertions(+), 59 deletions(-) -- 2.33.0

1 day, 18 hours

Re: [PATCH] coresight: Fix possible deadlock in coresight_panic_cb

by Suzuki K Poulose

Hi Sean On 28/08/2025 19:17, Sean Anderson wrote: > coresight_panic_cb is called with interrupts disabled during panics. > However, bus_for_each_dev calls bus_to_subsys which takes > bus_kset->list_lock without disabling IRQs. This will cause a deadlock > if a panic occurs while one of the other coresight functions that uses > bus_for_each_dev is running. > > Maintain a separate list of coresight devices to access during a panic. Thanks for the patch. I have a minor comment. > > Fixes: 46006ceb5d02 ("coresight: core: Add provision for panic callbacks") > Signed-off-by: Sean Anderson <sean.anderson(a)linux.dev> > --- > > drivers/hwtracing/coresight/coresight-core.c | 40 ++++++++++---------- > include/linux/coresight.h | 1 + > 2 files changed, 22 insertions(+), 19 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c > index fa758cc21827..1f1bf0e2bf92 100644 > --- a/drivers/hwtracing/coresight/coresight-core.c > +++ b/drivers/hwtracing/coresight/coresight-core.c > @@ -1315,6 +1315,9 @@ void coresight_release_platform_data(struct coresight_device *csdev, > coresight_remove_conns_sysfs_group(csdev); > } > > +static DEFINE_SPINLOCK(csdev_lock); > +static LIST_HEAD(csdev_list); May be add a comment here to mention why we maintain this list ? > + > struct coresight_device *coresight_register(struct coresight_desc *desc) > { > int ret; > @@ -1374,11 +1377,16 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) > goto out_unlock; > } > > + scoped_guard(spinlock_irq, &csdev_lock) > + list_add(&csdev->csdev_list, &csdev_list); > + > if (csdev->type == CORESIGHT_DEV_TYPE_SINK || > csdev->type == CORESIGHT_DEV_TYPE_LINKSINK) { > ret = etm_perf_add_symlink_sink(csdev); > > if (ret) { > + scoped_guard(spinlock_irq, &csdev_lock) > + list_del(&csdev->csdev_list); Could this be moved to coresight_device_release(), which will be called when the device gets unregistered ? That way, you don't need it here and in coresight_unregister() too. Rest looks good to me Suzuki > device_unregister(&csdev->dev); > /* > * As with the above, all resources are free'd > @@ -1427,6 +1435,8 @@ void coresight_unregister(struct coresight_device *csdev) > coresight_remove_conns(csdev); > coresight_clear_default_sink(csdev); > coresight_release_platform_data(csdev, csdev->dev.parent, csdev->pdata); > + scoped_guard(spinlock_irq, &csdev_lock) > + list_del(&csdev->csdev_list); > device_unregister(&csdev->dev); > } > EXPORT_SYMBOL_GPL(coresight_unregister); > @@ -1563,28 +1573,20 @@ const struct bus_type coresight_bustype = { > .name = "coresight", > }; > > -static int coresight_panic_sync(struct device *dev, void *data) > -{ > - int mode; > - struct coresight_device *csdev; > - > - /* Run through panic sync handlers for all enabled devices */ > - csdev = container_of(dev, struct coresight_device, dev); > - mode = coresight_get_mode(csdev); > - > - if ((mode == CS_MODE_SYSFS) || (mode == CS_MODE_PERF)) { > - if (panic_ops(csdev)) > - panic_ops(csdev)->sync(csdev); > - } > - > - return 0; > -} > - > static int coresight_panic_cb(struct notifier_block *self, > unsigned long v, void *p) > { > - bus_for_each_dev(&coresight_bustype, NULL, NULL, > - coresight_panic_sync); > + struct coresight_device *csdev; > + > + guard(spinlock)(&csdev_lock); > + list_for_each_entry(csdev, &csdev_list, csdev_list) { > + /* Run through panic sync handlers for all enabled devices */ > + int mode = coresight_get_mode(csdev); > + > + if ((mode == CS_MODE_SYSFS || mode == CS_MODE_PERF) && > + panic_ops(csdev)) > + panic_ops(csdev)->sync(csdev); > + } > > return 0; > } > diff --git a/include/linux/coresight.h b/include/linux/coresight.h > index 4ac65c68bbf4..a5e62ebd03b5 100644 > --- a/include/linux/coresight.h > +++ b/include/linux/coresight.h > @@ -302,6 +302,7 @@ struct coresight_device { > /* system configuration and feature lists */ > struct list_head feature_csdev_list; > struct list_head config_csdev_list; > + struct list_head csdev_list; > raw_spinlock_t cscfg_csdev_lock; > void *active_cscfg_ctxt; > };

2 days, 11 hours

[PATCH v4 0/6] Refactoring finding CPU phandles in DT

by Alireza Sanaee

This series refactor the way CPU IDs are retrieved from the device tree. Usually, there is a for loop that goes over every single CPU that can be avoided. This also reduces the amount of NULL pointer checks in drivers. I have abstracted away that loop and introduced a new function (of_cpu_node_to_id) for this. This patchset is a subset of [1], where I removed content and patches relevant to hyper-threaded cores for DT. Based on the discussion, the code refactor is still useful, hence this patchset. [1] https://lore.kernel.org/all/20250512080715.82-1-alireza.sanaee@huawei.co Changes since v3: - Rebased to d69eb204c255c35abd9e8cb621484e8074c75eaa Changes since v2: - Addressed Jonathan Cameron's comments. - Added reviewed-by tags. - Added a new commit where I suggest simplfing the code in arch_topology.c to remove the use of cpu_node on failure path. - Improve documentation. - Caught a bug in patch 1. - Commit message fixed for patch 2. Changes since v1: - Rebased on top of the latest mainline. - Addressed Krzysztof Kozlowski's comments -- Hopefully :-) - Addressed Jonathan Cameron's comments. Alireza Sanaee (6): of: add infra for finding CPU id from phandle arch_topology: drop the use of cpu_node in the pr_info arch_topology: update CPU map to use of_cpu_phandle_to_id coresight: cti: Use of_cpu_phandle_to_id for grabbing CPU id coresight: Use of_cpu_phandle_to_id for grabbing CPU ID perf/arm-dsu: refactor cpu id retrieval via new API of_cpu_phandle_to_id drivers/base/arch_topology.c | 22 ++++----- .../coresight/coresight-cti-platform.c | 13 +---- .../hwtracing/coresight/coresight-platform.c | 15 +----- drivers/of/cpu.c | 48 +++++++++++++++++++ drivers/perf/arm_dsu_pmu.c | 7 +-- include/linux/of.h | 9 ++++ 6 files changed, 71 insertions(+), 43 deletions(-) -- 2.43.0

2 days, 12 hours

Re: [PATCH v2] coresight: tpda: fix the logic to setup the element size

by Suzuki K Poulose

On Sat, 06 Sep 2025 07:53:04 +0800, Jie Gan wrote: > Some TPDM devices support both CMB and DSB datasets, requiring > the system to enable the port with both corresponding element sizes. > > Currently, the logic treats tpdm_read_element_size as successful if > the CMB element size is retrieved correctly, regardless of whether > the DSB element size is obtained. This behavior causes issues > when parsing data from TPDM devices that depend on both element sizes. > > [...] Applied, thanks! [1/1] coresight: tpda: fix the logic to setup the element size https://git.kernel.org/coresight/c/fd6974c19eb3 Best regards, -- Suzuki K Poulose <suzuki.poulose(a)arm.com>

2 days, 12 hours

Re: [PATCH -next] coresight: Fix incorrect handling for return value of devm_kzalloc

by James Clark

On 08/09/2025 1:20 pm, Lin Yujun wrote: > The return value of devm_kzalloc could be an null pointer, > use "!desc.pdata" to fix incorrect handling return value > of devm_kzalloc. > > Fixes: 4277f035d227 ("coresight: trbe: Add a representative coresight_platform_data for TRBE") > Signed-off-by: Lin Yujun <linyujun809(a)h-partners.com> > --- > drivers/hwtracing/coresight/coresight-trbe.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c > index 8267dd1a2130..caf873adfc3a 100644 > --- a/drivers/hwtracing/coresight/coresight-trbe.c > +++ b/drivers/hwtracing/coresight/coresight-trbe.c > @@ -1277,11 +1277,11 @@ static void arm_trbe_register_coresight_cpu(struct trbe_drvdata *drvdata, int cp > * ends up failing. Instead let's allocate a dummy zeroed > * coresight_platform_data structure and assign that back > * into the device for that purpose. > */ > desc.pdata = devm_kzalloc(dev, sizeof(*desc.pdata), GFP_KERNEL); > - if (IS_ERR(desc.pdata)) > + if (!desc.pdata) > goto cpu_clear; > > desc.type = CORESIGHT_DEV_TYPE_SINK; > desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_PERCPU_SYSMEM; > desc.ops = &arm_trbe_cs_ops; Reviewed-by: James Clark <james.clark(a)linaro.org>

4 days, 15 hours

[PATCH v4 0/6] Refactoring finding CPU phandles in DT

by Alireza Sanaee

1 week

[PATCH v2] coresight: trbe: Return NULL pointer for allocation failures

by Leo Yan

When the TRBE driver fails to allocate a buffer, it currently returns the error code "-ENOMEM". However, the caller etm_setup_aux() only checks for a NULL pointer, so it misses the error. As a result, the driver continues and eventually causes a kernel panic. Fix this by returning a NULL pointer from arm_trbe_alloc_buffer() on allocation failures. This allows that the callers can properly handle the failure. Fixes: 3fbf7f011f24 ("coresight: sink: Add TRBE driver") Reported-by: Tamas Zsoldos <tamas.zsoldos(a)arm.com> Signed-off-by: Leo Yan <leo.yan(a)arm.com> --- Changes in v2: - Fix TRBE driver instead of changing coresight-etm-perf.c. - Link to v1: https://lore.kernel.org/r/20250904-cs_etm_auxsetup_fix_error_handling-v1-1-… --- drivers/hwtracing/coresight/coresight-trbe.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 10f3fb401edf6a00b24b38cdaa7c2865e7a191ac..8f9bbef71f236b327d35a288689df9b0dd8ff3f4 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -748,12 +748,12 @@ static void *arm_trbe_alloc_buffer(struct coresight_device *csdev, buf = kzalloc_node(sizeof(*buf), GFP_KERNEL, trbe_alloc_node(event)); if (!buf) - return ERR_PTR(-ENOMEM); + return NULL; pglist = kcalloc(nr_pages, sizeof(*pglist), GFP_KERNEL); if (!pglist) { kfree(buf); - return ERR_PTR(-ENOMEM); + return NULL; } for (i = 0; i < nr_pages; i++) @@ -763,7 +763,7 @@ static void *arm_trbe_alloc_buffer(struct coresight_device *csdev, if (!buf->trbe_base) { kfree(pglist); kfree(buf); - return ERR_PTR(-ENOMEM); + return NULL; } buf->trbe_limit = buf->trbe_base + nr_pages * PAGE_SIZE; buf->trbe_write = buf->trbe_base; --- base-commit: fa71e9cb4cfa59abb196229667ec84929bdc18fe change-id: 20250904-cs_etm_auxsetup_fix_error_handling-cb7e07ed9adf Best regards, -- Leo Yan <leo.yan(a)arm.com>

1 week

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight