CoreSight March 2025

coresight@lists.linaro.org

8 participants
57 discussions

[PATCH v4] coresight: Add a KUnit test for coresight_find_default_sink()

by James Clark

Add a test to confirm that default sink selection skips over an ETF and returns an ETR even if it's further away. This also makes it easier to add new unit tests in the future. Reviewed-by: Leo Yan <leo.yan(a)arm.com> Signed-off-by: James Clark <james.clark(a)linaro.org> --- Changes in v4: - Rename etm to src now that it's not CORESIGHT_DEV_SUBTYPE_SOURCE_PROC - Remove the now empty src_ops too - Fix a rebase mistake in the Makefile that removed CTCU - Link to v3: https://lore.kernel.org/r/20250312-james-cs-kunit-test-v3-1-dcfb69730161@li… Changes in v3: - Use CORESIGHT_DEV_SUBTYPE_SOURCE_BUS type instead of the default (CORESIGHT_DEV_SUBTYPE_SOURCE_PROC) so that the test still works even when TRBE sinks are registered. This also removes the need for the fake CPU ID callback. - Link to v2: https://lore.kernel.org/r/20250305-james-cs-kunit-test-v2-1-83ba682b976c@li… Changes in v2: - Let devm free everything rather than doing individual kfrees: "Like with managed drivers, KUnit-managed fake devices are automatically cleaned up when the test finishes, but can be manually cleaned up early with kunit_device_unregister()." - Link to v1: https://lore.kernel.org/r/20250225164639.522741-1-james.clark@linaro.org --- drivers/hwtracing/coresight/Kconfig | 9 +++ drivers/hwtracing/coresight/Makefile | 1 + drivers/hwtracing/coresight/coresight-core.c | 1 + .../hwtracing/coresight/coresight-kunit-tests.c | 74 ++++++++++++++++++++++ 4 files changed, 85 insertions(+) diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index ecd7086a5b83..f064e3d172b3 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -259,4 +259,13 @@ config CORESIGHT_DUMMY To compile this driver as a module, choose M here: the module will be called coresight-dummy. + +config CORESIGHT_KUNIT_TESTS + tristate "Enable Coresight unit tests" + depends on KUNIT + default KUNIT_ALL_TESTS + help + Enable Coresight unit tests. Only useful for development and not + intended for production. + endif diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile index 8e62c3150aeb..4e6ea5b05b01 100644 --- a/drivers/hwtracing/coresight/Makefile +++ b/drivers/hwtracing/coresight/Makefile @@ -53,3 +53,4 @@ obj-$(CONFIG_ULTRASOC_SMB) += ultrasoc-smb.o obj-$(CONFIG_CORESIGHT_DUMMY) += coresight-dummy.o obj-$(CONFIG_CORESIGHT_CTCU) += coresight-ctcu.o coresight-ctcu-y := coresight-ctcu-core.o +obj-$(CONFIG_CORESIGHT_KUNIT_TESTS) += coresight-kunit-tests.o diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index fb43ef6a3b1f..47af75ba7a00 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -959,6 +959,7 @@ coresight_find_default_sink(struct coresight_device *csdev) } return csdev->def_sink; } +EXPORT_SYMBOL_GPL(coresight_find_default_sink); static int coresight_remove_sink_ref(struct device *dev, void *data) { diff --git a/drivers/hwtracing/coresight/coresight-kunit-tests.c b/drivers/hwtracing/coresight/coresight-kunit-tests.c new file mode 100644 index 000000000000..c8f361767c45 --- /dev/null +++ b/drivers/hwtracing/coresight/coresight-kunit-tests.c @@ -0,0 +1,74 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <kunit/test.h> +#include <kunit/device.h> +#include <linux/coresight.h> + +#include "coresight-priv.h" + +static struct coresight_device *coresight_test_device(struct device *dev) +{ + struct coresight_device *csdev = devm_kcalloc(dev, 1, + sizeof(struct coresight_device), + GFP_KERNEL); + csdev->pdata = devm_kcalloc(dev, 1, + sizeof(struct coresight_platform_data), + GFP_KERNEL); + return csdev; +} + +static void test_default_sink(struct kunit *test) +{ + /* + * Source -> ETF -> ETR -> CATU + * ^ + * | default + */ + struct device *dev = kunit_device_register(test, "coresight_kunit"); + struct coresight_device *src = coresight_test_device(dev), + *etf = coresight_test_device(dev), + *etr = coresight_test_device(dev), + *catu = coresight_test_device(dev); + struct coresight_connection conn = {}; + + src->type = CORESIGHT_DEV_TYPE_SOURCE; + /* + * Don't use CORESIGHT_DEV_SUBTYPE_SOURCE_PROC, that would always return + * a TRBE sink if one is registered. + */ + src->subtype.source_subtype = CORESIGHT_DEV_SUBTYPE_SOURCE_BUS; + etf->type = CORESIGHT_DEV_TYPE_LINKSINK; + etf->subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER; + etr->type = CORESIGHT_DEV_TYPE_SINK; + etr->subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM; + catu->type = CORESIGHT_DEV_TYPE_HELPER; + + conn.src_dev = src; + conn.dest_dev = etf; + coresight_add_out_conn(dev, src->pdata, &conn); + + conn.src_dev = etf; + conn.dest_dev = etr; + coresight_add_out_conn(dev, etf->pdata, &conn); + + conn.src_dev = etr; + conn.dest_dev = catu; + coresight_add_out_conn(dev, etr->pdata, &conn); + + KUNIT_ASSERT_PTR_EQ(test, coresight_find_default_sink(src), etr); +} + +static struct kunit_case coresight_testcases[] = { + KUNIT_CASE(test_default_sink), + {} +}; + +static struct kunit_suite coresight_test_suite = { + .name = "coresight_test_suite", + .test_cases = coresight_testcases, +}; + +kunit_test_suites(&coresight_test_suite); +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("James Clark <james.clark(a)linaro.org>"); +MODULE_DESCRIPTION("Arm CoreSight KUnit tests"); --- base-commit: 3eadce8308bc8d808fd9e3a9d211c84215087451 change-id: 20250305-james-cs-kunit-test-3af1df2401e6 Best regards, -- James Clark <james.clark(a)linaro.org>

1 week, 2 days

[PATCH v4 0/7] coresight: Clear self hosted claim tag on probe

by James Clark

I've gotten stuck a few times with unusable Coresight after a warm boot due to lingering claim tags, especially when testing the Coresight panic patchsets. This change does some tidy ups, adds some debug messages and clears the self hosted claim tag on probe. The last two commits are unrelated tidyups but they touch some of the same functions so to avoid extra conflicts I'm including them here. This gets as far as fixing the claim tag issue, but there is some other state not being cleared on probe that results in the following error. This can be fixed up as a later change: coresight tmc_etf0: timeout while waiting for TMC to be Ready coresight tmc_etf0: Failed to enable : TMC is not ready Changes in v4: - Add _unlocked() suffix for consistency - s/cs_access/csdev_access/ - Link to v3: https://lore.kernel.org/r/20250320-james-coresight-claim-tags-v3-0-d3145c15… Changes in v3: - Collapse rename and locked/unlocked addition commits of coresight_clear_self_claim_tag() so we don't change the name twice. - Make coresight_clear_self_claim_tag() a bit more generic by only doing UNLOCK for MMIO devices (although there is no use of this right now) - Link to v2: https://lore.kernel.org/r/20250318-james-coresight-claim-tags-v2-0-e9c8a9cd… Changes in v2: * Revert most of the interface changes, just call coresight_clear_self_claim_tag() directly. This is possible because we're not doing the read first, so it has fewer knock on effects. * Split out the change to add struct cs_access to etm3x * Add another warning for racing with external debugger -- 2.34.1 --- James Clark (7): coresight: Convert tag clear function to take a struct csdev_access coresight: Only check bottom two claim bits coresight: Add claim tag warnings and debug messages coresight: etm3x: Convert raw base pointer to struct coresight access coresight: Clear self hosted claim tag on probe coresight: Remove inlines from static function definitions coresight: Remove extern from function declarations drivers/hwtracing/coresight/coresight-catu.c | 12 +-- drivers/hwtracing/coresight/coresight-core.c | 87 ++++++++++++++-------- drivers/hwtracing/coresight/coresight-cti-core.c | 2 + drivers/hwtracing/coresight/coresight-etb10.c | 4 +- drivers/hwtracing/coresight/coresight-etm.h | 6 +- drivers/hwtracing/coresight/coresight-etm3x-core.c | 28 +++---- .../hwtracing/coresight/coresight-etm3x-sysfs.c | 8 +- drivers/hwtracing/coresight/coresight-etm4x-core.c | 10 ++- .../hwtracing/coresight/coresight-etm4x-sysfs.c | 4 +- drivers/hwtracing/coresight/coresight-funnel.c | 1 + drivers/hwtracing/coresight/coresight-platform.c | 26 +++---- drivers/hwtracing/coresight/coresight-priv.h | 20 ++--- drivers/hwtracing/coresight/coresight-replicator.c | 3 +- drivers/hwtracing/coresight/coresight-stm.c | 6 +- .../coresight/coresight-syscfg-configfs.c | 2 +- drivers/hwtracing/coresight/coresight-tmc-core.c | 9 ++- drivers/hwtracing/coresight/coresight-tmc-etr.c | 16 ++-- drivers/hwtracing/coresight/coresight-trbe.c | 18 ++--- include/linux/coresight.h | 40 +++++----- 19 files changed, 168 insertions(+), 134 deletions(-) --- base-commit: 5442d22da7dbff3ba8c6720fc6f23ea4934d402d change-id: 20250317-james-coresight-claim-tags-ae1461f1f5e0 Best regards, -- James Clark <james.clark(a)linaro.org>

1 week, 3 days

[PATCH] coresight: Fixes device's owner field for registered using coresight_init_driver()

by Junhao He

The coresight_init_driver() of the coresight-core module is called from the sub coresgiht device (such as tmc/stm/funnle/...) module. It calls amba_driver_register() and Platform_driver_register(), which are macro functions that use the coresight-core's module to initialize the caller's owner field. Therefore, when the sub coresight device calls coresight_init_driver(), an incorrect THIS_MODULE value is captured. The sub coesgiht modules can be removed while their callbacks are running, resulting in a general protection failure. Add module parameter to coresight_init_driver() so can be called with the module of the callback. Fixes: 075b7cd7ad7d ("coresight: Add helpers registering/removing both AMBA and platform drivers") Signed-off-by: Junhao He <hejunhao3(a)huawei.com> --- drivers/hwtracing/coresight/coresight-catu.c | 2 +- drivers/hwtracing/coresight/coresight-core.c | 6 +++--- drivers/hwtracing/coresight/coresight-cpu-debug.c | 3 ++- drivers/hwtracing/coresight/coresight-funnel.c | 3 ++- drivers/hwtracing/coresight/coresight-replicator.c | 3 ++- drivers/hwtracing/coresight/coresight-stm.c | 2 +- drivers/hwtracing/coresight/coresight-tmc-core.c | 2 +- drivers/hwtracing/coresight/coresight-tpiu.c | 2 +- include/linux/coresight.h | 2 +- 9 files changed, 14 insertions(+), 11 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c index bfea880d6dfb..337668f9cfd4 100644 --- a/drivers/hwtracing/coresight/coresight-catu.c +++ b/drivers/hwtracing/coresight/coresight-catu.c @@ -702,7 +702,7 @@ static int __init catu_init(void) { int ret; - ret = coresight_init_driver("catu", &catu_driver, &catu_platform_driver); + ret = coresight_init_driver("catu", &catu_driver, &catu_platform_driver, THIS_MODULE); tmc_etr_set_catu_ops(&etr_catu_buf_ops); return ret; } diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 9fc6f6b863e0..c546a417836c 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1399,17 +1399,17 @@ module_init(coresight_init); module_exit(coresight_exit); int coresight_init_driver(const char *drv, struct amba_driver *amba_drv, - struct platform_driver *pdev_drv) + struct platform_driver *pdev_drv, struct module *owner) { int ret; - ret = amba_driver_register(amba_drv); + ret = __amba_driver_register(amba_drv, owner); if (ret) { pr_err("%s: error registering AMBA driver\n", drv); return ret; } - ret = platform_driver_register(pdev_drv); + ret = __platform_driver_register(pdev_drv, owner); if (!ret) return 0; diff --git a/drivers/hwtracing/coresight/coresight-cpu-debug.c b/drivers/hwtracing/coresight/coresight-cpu-debug.c index 75962dae9aa1..cc599c5ef4b2 100644 --- a/drivers/hwtracing/coresight/coresight-cpu-debug.c +++ b/drivers/hwtracing/coresight/coresight-cpu-debug.c @@ -774,7 +774,8 @@ static struct platform_driver debug_platform_driver = { static int __init debug_init(void) { - return coresight_init_driver("debug", &debug_driver, &debug_platform_driver); + return coresight_init_driver("debug", &debug_driver, &debug_platform_driver, + THIS_MODULE); } static void __exit debug_exit(void) diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c index 5a819c8970fb..8f451b051ddc 100644 --- a/drivers/hwtracing/coresight/coresight-funnel.c +++ b/drivers/hwtracing/coresight/coresight-funnel.c @@ -433,7 +433,8 @@ static struct amba_driver dynamic_funnel_driver = { static int __init funnel_init(void) { - return coresight_init_driver("funnel", &dynamic_funnel_driver, &funnel_driver); + return coresight_init_driver("funnel", &dynamic_funnel_driver, &funnel_driver, + THIS_MODULE); } static void __exit funnel_exit(void) diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c index 3e55be9c8418..f7607c72857c 100644 --- a/drivers/hwtracing/coresight/coresight-replicator.c +++ b/drivers/hwtracing/coresight/coresight-replicator.c @@ -438,7 +438,8 @@ static struct amba_driver dynamic_replicator_driver = { static int __init replicator_init(void) { - return coresight_init_driver("replicator", &dynamic_replicator_driver, &replicator_driver); + return coresight_init_driver("replicator", &dynamic_replicator_driver, &replicator_driver, + THIS_MODULE); } static void __exit replicator_exit(void) diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c index 117dbb484543..403eea8f95d4 100644 --- a/drivers/hwtracing/coresight/coresight-stm.c +++ b/drivers/hwtracing/coresight/coresight-stm.c @@ -1046,7 +1046,7 @@ static struct platform_driver stm_platform_driver = { static int __init stm_init(void) { - return coresight_init_driver("stm", &stm_driver, &stm_platform_driver); + return coresight_init_driver("stm", &stm_driver, &stm_platform_driver, THIS_MODULE); } static void __exit stm_exit(void) diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c index b54562f392f3..e31e36635394 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-core.c +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c @@ -742,7 +742,7 @@ static struct platform_driver tmc_platform_driver = { static int __init tmc_init(void) { - return coresight_init_driver("tmc", &tmc_driver, &tmc_platform_driver); + return coresight_init_driver("tmc", &tmc_driver, &tmc_platform_driver, THIS_MODULE); } static void __exit tmc_exit(void) diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c index b048e146fbb1..f9ecd05cbe5c 100644 --- a/drivers/hwtracing/coresight/coresight-tpiu.c +++ b/drivers/hwtracing/coresight/coresight-tpiu.c @@ -318,7 +318,7 @@ static struct platform_driver tpiu_platform_driver = { static int __init tpiu_init(void) { - return coresight_init_driver("tpiu", &tpiu_driver, &tpiu_platform_driver); + return coresight_init_driver("tpiu", &tpiu_driver, &tpiu_platform_driver, THIS_MODULE); } static void __exit tpiu_exit(void) diff --git a/include/linux/coresight.h b/include/linux/coresight.h index f09ace92176e..e6c26952ddc2 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -660,7 +660,7 @@ coresight_find_output_type(struct coresight_platform_data *pdata, union coresight_dev_subtype subtype); int coresight_init_driver(const char *drv, struct amba_driver *amba_drv, - struct platform_driver *pdev_drv); + struct platform_driver *pdev_drv, struct module *owner); void coresight_remove_driver(struct amba_driver *amba_drv, struct platform_driver *pdev_drv); -- 2.33.0

1 week, 4 days

[PATCH v1 0/9] coresight: Fix and improve clock usage

by Leo Yan

This series fixes and improves clock usage in the Arm CoreSight drivers. Based on the DT binding documents, the trace clock (atclk) is defined in some CoreSight modules, but the corresponding drivers to support it are absent. In most cases, the issue is hidden because atclk is shared by multiple CoreSight modules and is enabled anyway by other drivers. The first three patches address this issue. The programming clock (pclk) management in CoreSight drivers does not use the devm_XXX() variant APIs, so the drivers needs to manually disable and release clocks for errors and for normal module exit. However, the drivers miss to disable clocks during module exit. Another issue is pclk might be enabled twice in init phase - once by AMBA bus driver, and again by CoreSight drivers. Patches 04 and 05 fix pclk issues. The atclk may also not be disabled in CoreSight drivers during module exit. This is fixed in patch 06. Patches 07 to 09 refactor the clock related code. Patch 07 makes the clock enabling sequence consistent. Patch 08 removes redundant condition checks and adds error handling in runtime PM. Patch 09 consolidats the clock initialization into a central place. This series is verified on Arm64 Hikey960 platform. Leo Yan (9): coresight: tmc: Support atclk coresight: catu: Support atclk coresight: etm4x: Support atclk coresight: Disable programming clock properly coresight: Avoid enable programming clock duplicately coresight: Disable trace bus clock properly coresight: Make clock sequence consistent coresight: Refactor runtime PM coresight: Consolidate clock enabling drivers/hwtracing/coresight/coresight-catu.c | 53 ++++++++++++++++----------------- drivers/hwtracing/coresight/coresight-catu.h | 1 + drivers/hwtracing/coresight/coresight-cpu-debug.c | 41 +++++++++----------------- drivers/hwtracing/coresight/coresight-ctcu-core.c | 24 +++++---------- drivers/hwtracing/coresight/coresight-etb10.c | 18 ++++-------- drivers/hwtracing/coresight/coresight-etm3x-core.c | 17 ++++------- drivers/hwtracing/coresight/coresight-etm4x-core.c | 32 ++++++++++---------- drivers/hwtracing/coresight/coresight-etm4x.h | 4 ++- drivers/hwtracing/coresight/coresight-funnel.c | 66 +++++++++++++++--------------------------- drivers/hwtracing/coresight/coresight-replicator.c | 63 ++++++++++++++-------------------------- drivers/hwtracing/coresight/coresight-stm.c | 34 +++++++++------------- drivers/hwtracing/coresight/coresight-tmc-core.c | 48 +++++++++++++++--------------- drivers/hwtracing/coresight/coresight-tmc.h | 2 ++ drivers/hwtracing/coresight/coresight-tpiu.c | 36 ++++++++++------------- include/linux/coresight.h | 47 ++++++++++++++++++------------ 15 files changed, 206 insertions(+), 280 deletions(-) -- 2.34.1

2 weeks, 4 days

[PATCH v2 0/5] coresight: Add Coresight Trace NOC driver

by Yuanfang Zhang

The Trace NoC is an integration hierarchy which is a replacement of Dragonlink configuration. It brings together debug component like TPDA, funnel and interconnect Trace Noc which collects trace from subsystems and transfers to QDSS sink. Compared to DL, it has the following advantages: 1. Reduce wires between subsystems. 2. Continue cleaning the infrastructure. 3. Reduce Data overhead by transporting raw data from source to target. +--------------+ +-------------+ | SDCC5 TPDM | | SDCC5 TPDM | +--------------+ +-------------+ | | | | +----------|-------------------+ | | v | | | +----v----+ Dragon Link | v | |DLNT TPDA| North | +----------------------+ | +---------+ | | TRACE NOC AG | | | | | | | v-------------+ | +----------------------+ | | | | | +------v-----+ | | | | DLNT Funnel| | | | +------------+ | | | | | | +-------------------|----------+ | <-----+ | | | | | v v +----------------+ +---------------+ | QDSS | | QDSS | +----------------+ +---------------+ Signed-off-by: Yuanfang Zhang <quic_yuanfang(a)quicinc.com> --- --- Changes in v2: 1. Modified the format of DT binging file. 2. Fix compile warnings. - Link to v1: https://lore.kernel.org/r/20250221-trace-noc-driver-v1-0-0a23fc643217@quici… --- Yuanfang Zhang (5): dt-bindings: arm: Add Coresight device Trace NOC definition coresight: add coresight Trace NOC driver coresight-tnoc: add nodes to configure flush coresight-tnoc: add node to configure flag type coresight-tnoc: add nodes to configure freq packet .../bindings/arm/qcom,coresight-tnoc.yaml | 116 ++++++ drivers/hwtracing/coresight/Kconfig | 13 + drivers/hwtracing/coresight/Makefile | 1 + drivers/hwtracing/coresight/coresight-tnoc.c | 400 +++++++++++++++++++++ drivers/hwtracing/coresight/coresight-tnoc.h | 57 +++ 5 files changed, 587 insertions(+) --- base-commit: 92514ef226f511f2ca1fb1b8752966097518edc0 change-id: 20250212-trace-noc-driver-9e75d78114fa Best regards, -- Yuanfang Zhang <quic_yuanfang(a)quicinc.com>

1 month

[PATCH v3 0/6] Arm CoreSight: Support AUX pause and resume

by Leo Yan

This series is to enable AUX pause and resume on Arm CoreSight. The first patch extracts the trace unit controlling operations to two functions. These two functions will be used by AUX pause and resume. Patches 02 and 03 change the ETMv4 driver to prepare callback functions for AUX pause and resume. Patch 04 changes the ETM perf layer to support AUX pause and resume in a perf session. The patches 05 updates buffer on AUX pause occasion, which can mitigate the trace data lose issue. Patch 07 documents the AUX pause usages with Arm CoreSight. This patch set has been verified on the Hikey960 board and TC platform. The previous one uses ETR and the later uses TRBE as sink. It is suggested to disable CPUIdle (add `nohlt` option in Linux command line) when verifying this series. ETM and funnel drivers are found issues during CPU suspend and resume which will be addressed separately. Changes from v2: - Rebased on CoreSight next branch. - Dropped the uAPI 'update_buf_on_pause' and updated document respectively (Suzuki). - Renamed ETM callbacks to .pause_perf() and .resume_perf() (Suzuki). - Minor improvement for error handling in the AUX resume flow. Changes from v1: - Added validation function pointers in pause and resume APIs (Mike). Leo Yan (6): coresight: etm4x: Extract the trace unit controlling coresight: Introduce pause and resume APIs for source coresight: etm4x: Hook pause and resume callbacks coresight: perf: Support AUX trace pause and resume coresight: perf: Update buffer on AUX pause Documentation: coresight: Document AUX pause and resume .../trace/coresight/coresight-perf.rst | 31 ++++ drivers/hwtracing/coresight/coresight-core.c | 22 +++ .../hwtracing/coresight/coresight-etm-perf.c | 84 +++++++++- .../coresight/coresight-etm4x-core.c | 143 +++++++++++++----- drivers/hwtracing/coresight/coresight-etm4x.h | 2 + drivers/hwtracing/coresight/coresight-priv.h | 2 + include/linux/coresight.h | 4 + 7 files changed, 246 insertions(+), 42 deletions(-) -- 2.34.1

1 month, 1 week

Re: [PATCH] coresight: catu: Introduce refcount and spinlock for enabling/disabling

by James Clark

On 28/03/2025 10:38 pm, Yabin Cui wrote: > When tracing ETM data on multiple CPUs concurrently via the > perf interface, the CATU device is shared across different CPU > paths. This can lead to race conditions when multiple CPUs attempt > to enable or disable the CATU device simultaneously. > > To address these race conditions, this patch introduces the > following changes: > > 1. The enable and disable operations for the CATU device are not > reentrant. Therefore, a spinlock is added to ensure that only > one CPU can enable or disable a given CATU device at any point > in time. > > 2. A reference counter is used to manage the enable/disable state > of the CATU device. The device is enabled when the first CPU > requires it and is only disabled when the last CPU finishes > using it. This ensures the device remains active as long as at > least one CPU needs it. > > Signed-off-by: Yabin Cui <yabinc(a)google.com> > --- > drivers/hwtracing/coresight/coresight-catu.c | 29 ++++++++++++++------ > drivers/hwtracing/coresight/coresight-catu.h | 1 + > 2 files changed, 22 insertions(+), 8 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c > index 275cc0d9f505..834a7ffbbdbc 100644 > --- a/drivers/hwtracing/coresight/coresight-catu.c > +++ b/drivers/hwtracing/coresight/coresight-catu.c > @@ -458,12 +458,19 @@ static int catu_enable_hw(struct catu_drvdata *drvdata, enum cs_mode cs_mode, > static int catu_enable(struct coresight_device *csdev, enum cs_mode mode, > void *data) > { > - int rc; > + int rc = 0; > + unsigned long flags; > struct catu_drvdata *catu_drvdata = csdev_to_catu_drvdata(csdev); > > - CS_UNLOCK(catu_drvdata->base); > - rc = catu_enable_hw(catu_drvdata, mode, data); > - CS_LOCK(catu_drvdata->base); > + spin_lock_irqsave(&catu_drvdata->spinlock, flags); Hi Yabin, This needs to be a raw_spinlock since [1]. Also you might as well use the new guard() thing to save someone find-and-replacing it later. But I'm wondering if this is accurate. The ETR's refcount is dependent on the pid of the owner of the trace session: /* Do not proceed if this device is associated with another session */ if (drvdata->pid != -1 && drvdata->pid != pid) { rc = -EBUSY; goto unlock_out; } /* * No HW configuration is needed if the sink is already in * use for this session. */ if (drvdata->pid == pid) { csdev->refcnt++; goto unlock_out; } If the helpers get enabled first, could this mean that CATU gets associated with a different session than the ETR? Maybe not, but it would be easier to understand if the core code handled the refcounting and locking for linked devices. [1]: https://lore.kernel.org/all/20250306121110.1647948-3-yeoreum.yun@arm.com/ Thanks James > + if (csdev->refcnt == 0) { > + CS_UNLOCK(catu_drvdata->base); > + rc = catu_enable_hw(catu_drvdata, mode, data); > + CS_LOCK(catu_drvdata->base); > + } > + if (!rc) > + csdev->refcnt++; > + spin_unlock_irqrestore(&catu_drvdata->spinlock, flags); > return rc; > } > > @@ -486,12 +493,17 @@ static int catu_disable_hw(struct catu_drvdata *drvdata) > > static int catu_disable(struct coresight_device *csdev, void *__unused) > { > - int rc; > + int rc = 0; > + unsigned long flags; > struct catu_drvdata *catu_drvdata = csdev_to_catu_drvdata(csdev); > > - CS_UNLOCK(catu_drvdata->base); > - rc = catu_disable_hw(catu_drvdata); > - CS_LOCK(catu_drvdata->base); > + spin_lock_irqsave(&catu_drvdata->spinlock, flags); > + if (--csdev->refcnt == 0) { > + CS_UNLOCK(catu_drvdata->base); > + rc = catu_disable_hw(catu_drvdata); > + CS_LOCK(catu_drvdata->base); > + } > + spin_unlock_irqrestore(&catu_drvdata->spinlock, flags); > return rc; > } > > @@ -550,6 +562,7 @@ static int __catu_probe(struct device *dev, struct resource *res) > dev->platform_data = pdata; > > drvdata->base = base; > + spin_lock_init(&drvdata->spinlock); > catu_desc.access = CSDEV_ACCESS_IOMEM(base); > catu_desc.pdata = pdata; > catu_desc.dev = dev; > diff --git a/drivers/hwtracing/coresight/coresight-catu.h b/drivers/hwtracing/coresight/coresight-catu.h > index 141feac1c14b..663282ec6381 100644 > --- a/drivers/hwtracing/coresight/coresight-catu.h > +++ b/drivers/hwtracing/coresight/coresight-catu.h > @@ -65,6 +65,7 @@ struct catu_drvdata { > void __iomem *base; > struct coresight_device *csdev; > int irq; > + spinlock_t spinlock; > }; > > #define CATU_REG32(name, offset) \

1 month, 1 week

Re: [PATCH v1 1/1] coresight: cti: Replace inclusion by struct fwnode_handle forward declaration

by James Clark

On 31/03/2025 8:14 am, Andy Shevchenko wrote: > The fwnode.h is not supposed to be used by the drivers as it > has the definitions for the core parts for different device > property provider implementations. Drop it. > > Since the code wants to use the pointer to the struct fwnode_handle > the forward declaration is provided. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko(a)linux.intel.com> > --- > drivers/hwtracing/coresight/coresight-cti.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/hwtracing/coresight/coresight-cti.h b/drivers/hwtracing/coresight/coresight-cti.h > index 16e310e7e9d4..8362a47c939c 100644 > --- a/drivers/hwtracing/coresight/coresight-cti.h > +++ b/drivers/hwtracing/coresight/coresight-cti.h > @@ -9,7 +9,6 @@ > > #include <linux/coresight.h> > #include <linux/device.h> > -#include <linux/fwnode.h> > #include <linux/list.h> > #include <linux/spinlock.h> > #include <linux/sysfs.h> > @@ -17,6 +16,8 @@ > > #include "coresight-priv.h" > > +struct fwnode_handle; > + > /* > * Device registers > * 0x000 - 0x144: CTI programming and status Reviewed-by: James Clark <james.clark(a)linaro.org>

1 month, 1 week

[PATCH V3 0/2] Fix Coresight instruction synthesis logic

by Tanmay Jagdale

When we use perf to catpure Coresight trace and generate instruction trace using 'perf script', we get the following output: # perf record -e cs_etm/@tmc_etr0/ -C 9 taskset -c 9 sleep 1 # perf script --itrace=i1ns --ns -Fcomm,tid,pid,time,cpu,event,ip,sym,addr,symoff,flags,callindent .. perf 9024/9024 [009] 2690.650470551: instructions: call 0 ffffb305591aed54 coresight_timeout+0x28 perf 9024/9024 [009] 2690.650470551: instructions: call 0 ffffb305591aed58 coresight_timeout+0x2c perf 9024/9024 [009] 2690.650470551: instructions: call 0 ffffb305591aed5c coresight_timeout+0x30 perf 9024/9024 [009] 2690.650470551: instructions: call 0 ffffb305591aed60 coresight_timeout+0x34 perf 9024/9024 [009] 2690.650470551: instructions: jmp 0 ffffb305591aed7c coresight_timeout+0x50 perf 9024/9024 [009] 2690.650470551: instructions: jmp 0 ffffb305591aed80 coresight_timeout+0x54 perf 9024/9024 [009] 2690.650470551: instructions: jmp 0 ffffb305591aed84 coresight_timeout+0x58 perf 9024/9024 [009] 2690.650470552: instructions: jcc 0 ffffb305591aede4 coresight_timeout+0xb8 perf 9024/9024 [009] 2690.650470552: instructions: jcc 0 ffffb305591aede8 coresight_timeout+0xbc perf 9024/9024 [009] 2690.650470552: instructions: jcc 0 ffffb305591aedec coresight_timeout+0xc0 perf 9024/9024 [009] 2690.650470552: instructions: jcc 0 ffffb305591aedf0 coresight_timeout+0xc4 perf 9024/9024 [009] 2690.650470557: instructions: call 0 ffffb305591bccec ete_sysreg_read+0x0 perf 9024/9024 [009] 2690.650470557: instructions: call 0 ffffb305591bccf0 ete_sysreg_read+0x4 perf 9024/9024 [009] 2690.650470557: instructions: call 0 ffffb305591bccf4 ete_sysreg_read+0x8 perf 9024/9024 [009] 2690.650470557: instructions: call 0 ffffb305591bccf8 ete_sysreg_read+0xc perf 9024/9024 [009] 2690.650470557: instructions: call 0 ffffb305591bccfc ete_sysreg_read+0x10 perf 9024/9024 [009] 2690.650470557: instructions: call 0 ffffb305591bcd00 ete_sysreg_read+0x14 This output has the following issues: 1. Non-branch instructions have mnemonics of branch instructions (Column 6) 2. Branch target address is missing (Column 7) This patch fixes these issues by changing the logic of instruction syntehsis for the Coresight trace queues. Output after applying the patch: ... perf 6111/6111 [008] 457.332794461: instructions: 0 ffffb305591aed54 coresight_timeout+0x28 perf 6111/6111 [008] 457.332794461: instructions: 0 ffffb305591aed58 coresight_timeout+0x2c perf 6111/6111 [008] 457.332794461: instructions: 0 ffffb305591aed5c coresight_timeout+0x30 perf 6111/6111 [008] 457.332794461: instructions: jmp ffffb305591aed7c ffffb305591aed60 coresight_timeout+0x34 perf 6111/6111 [008] 457.332794461: instructions: 0 ffffb305591aed7c coresight_timeout+0x50 perf 6111/6111 [008] 457.332794461: instructions: 0 ffffb305591aed80 coresight_timeout+0x54 perf 6111/6111 [008] 457.332794461: instructions: jcc ffffb305591aede4 ffffb305591aed84 coresight_timeout+0x58 perf 6111/6111 [008] 457.332794462: instructions: 0 ffffb305591aede4 coresight_timeout+0xb8 perf 6111/6111 [008] 457.332794462: instructions: 0 ffffb305591aede8 coresight_timeout+0xbc perf 6111/6111 [008] 457.332794462: instructions: 0 ffffb305591aedec coresight_timeout+0xc0 perf 6111/6111 [008] 457.332794462: instructions: call ffffb305591bccec ffffb305591aedf0 coresight_timeout+0xc4 perf 6111/6111 [008] 457.332794462: instructions: 0 ffffb305591bccec ete_sysreg_read+0x0 perf 6111/6111 [008] 457.332794462: instructions: 0 ffffb305591bccf0 ete_sysreg_read+0x4 perf 6111/6111 [008] 457.332794462: instructions: 0 ffffb305591bccf4 ete_sysreg_read+0x8 perf 6111/6111 [008] 457.332794462: instructions: 0 ffffb305591bccf8 ete_sysreg_read+0xc perf 6111/6111 [008] 457.332794462: instructions: 0 ffffb305591bccfc ete_sysreg_read+0x10 perf 6111/6111 [008] 457.332794462: instructions: 0 ffffb305591bcd00 ete_sysreg_read+0x14 Changes in V3 - Rebased to latest coresight-next branch - Added Reviewed-by tags from V2 [2] Changes in V2 - Updated commit message of Patch 1 - As discussed in the previous version [1], there were differences in instruction trace output before and after the patch. The timestamps for the instructions were not in sync. Added a patch 2 which fixes this issue. Changes in V1 - https://lkml.org/lkml/2023/6/23/912 [1] https://lkml.org/lkml/2023/6/28/506 [2] https://lore.kernel.org/lkml/b2c02eb9-0940-4770-a4b7-22d2af8078db@arm.com/ Tanmay Jagdale (2): perf: cs-etm: Fixes in instruction sample synthesis perf: cs-etm: Store previous timestamp in packet queue tools/perf/util/cs-etm.c | 49 +++++++++++++++++++++++++++++++--------- tools/perf/util/cs-etm.h | 1 + 2 files changed, 39 insertions(+), 11 deletions(-) -- 2.43.0

1 month, 2 weeks

Re: [PATCH v4] coresight: prevent deactivate active config while enabling the config

by Leo Yan

Hi Levi, On Mon, Mar 24, 2025 at 07:17:40PM +0000, Yeoreum Yun wrote: > While enable active config via cscfg_csdev_enable_active_config(), > active config could be deactivated via configfs' sysfs interface. > This could make UAF issue in below scenario: > > CPU0 CPU1 > (sysfs enable) load module > cscfg_load_config_sets() > activate config. // sysfs > (sys_active_cnt == 1) > ... > cscfg_csdev_enable_active_config() > lock(csdev->cscfg_csdev_lock) > // here load config activate by CPU1 > unlock(csdev->cscfg_csdev_lock) > > deactivate config // sysfs > (sys_activec_cnt == 0) > cscfg_unload_config_sets() > unload module > > // access to config_desc which freed > // while unloading module. > cfs_csdev_enable_config I am not sure if this flow can happen. CoreSight configfs feature is integrated into the CoreSight core layer, and the other CoreSight modules are dependent on it. For example, if the ETM4x module is not removed, the kernel module management will natually prevent the CoreSight core module from being removed. > To address this, use cscfg_config_desc's active_cnt as a reference count > which will be holded when > - activate the config. > - enable the activated config. > and put the module reference when config_active_cnt == 0. > > Signed-off-by: Yeoreum Yun <yeoreum.yun(a)arm.com> > --- > Since v3: > - Remove enable arguments in cscfg_config_desc_get() (from Mike). > - https://lore.kernel.org/all/20250109171956.3535294-1-yeoreum.yun@arm.com/ > --- > .../hwtracing/coresight/coresight-config.h | 2 +- > .../coresight/coresight-etm4x-core.c | 3 ++ > .../hwtracing/coresight/coresight-syscfg.c | 52 +++++++++++++------ > 3 files changed, 41 insertions(+), 16 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-config.h b/drivers/hwtracing/coresight/coresight-config.h > index b9ebc9fcfb7f..90fd937d3bd8 100644 > --- a/drivers/hwtracing/coresight/coresight-config.h > +++ b/drivers/hwtracing/coresight/coresight-config.h > @@ -228,7 +228,7 @@ struct cscfg_feature_csdev { > * @feats_csdev:references to the device features to enable. > */ > struct cscfg_config_csdev { > - const struct cscfg_config_desc *config_desc; > + struct cscfg_config_desc *config_desc; > struct coresight_device *csdev; > bool enabled; > struct list_head node; > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index e5972f16abff..ef96028fa56b 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -1020,6 +1020,9 @@ static void etm4_disable_sysfs(struct coresight_device *csdev) > smp_call_function_single(drvdata->cpu, etm4_disable_hw, drvdata, 1); > > raw_spin_unlock(&drvdata->spinlock); > + > + cscfg_csdev_disable_active_config(csdev); > + In general, we need to split changes into several patches if each addresses a different issue. From my understanding, the change above is to fix missing to disable config when disable Sysfs mode. If so, could we use a seperate patch for this change? > cpus_read_unlock(); > > /* > diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c > index a70c1454b410..6d8c212ad434 100644 > --- a/drivers/hwtracing/coresight/coresight-syscfg.c > +++ b/drivers/hwtracing/coresight/coresight-syscfg.c > @@ -391,14 +391,17 @@ static void cscfg_owner_put(struct cscfg_load_owner_info *owner_info) > static void cscfg_remove_owned_csdev_configs(struct coresight_device *csdev, void *load_owner) > { > struct cscfg_config_csdev *config_csdev, *tmp; > + unsigned long flags; > > if (list_empty(&csdev->config_csdev_list)) > return; > > + raw_spin_lock_irqsave(&csdev->cscfg_csdev_lock, flags); I think we should use spinlock to guard the condition checking list_empty(). Here the race condition is the 'config_csdev_list' list and configurations on the list. For atomicity, we should use lock to protect any operations on the list (read, add, delete, etc). A side topic, as here it adds locks for protecting 'config_csdev_list', I am wandering why we do not do the same thing for 'feature_csdev_list' (See cscfg_remove_owned_csdev_features() and cscfg_get_feat_csdev()). > list_for_each_entry_safe(config_csdev, tmp, &csdev->config_csdev_list, node) { > if (config_csdev->config_desc->load_owner == load_owner) > list_del(&config_csdev->node); > } > + raw_spin_unlock_irqrestore(&csdev->cscfg_csdev_lock, flags); > } > > static void cscfg_remove_owned_csdev_features(struct coresight_device *csdev, void *load_owner) > @@ -867,6 +870,25 @@ void cscfg_csdev_reset_feats(struct coresight_device *csdev) > } > EXPORT_SYMBOL_GPL(cscfg_csdev_reset_feats); > > +static bool cscfg_config_desc_get(struct cscfg_config_desc *config_desc) > +{ > + if (!atomic_fetch_inc(&config_desc->active_cnt)) { > + /* must ensure that config cannot be unloaded in use */ > + if (unlikely(cscfg_owner_get(config_desc->load_owner))) { > + atomic_dec(&config_desc->active_cnt); > + return false; > + } > + } > + > + return true; > +} > + > +static void cscfg_config_desc_put(struct cscfg_config_desc *config_desc) > +{ > + if (!atomic_dec_return(&config_desc->active_cnt)) > + cscfg_owner_put(config_desc->load_owner); > +} > + > /* > * This activate configuration for either perf or sysfs. Perf can have multiple > * active configs, selected per event, sysfs is limited to one. > @@ -890,22 +912,17 @@ static int _cscfg_activate_config(unsigned long cfg_hash) > if (config_desc->available == false) > return -EBUSY; > > - /* must ensure that config cannot be unloaded in use */ > - err = cscfg_owner_get(config_desc->load_owner); > - if (err) > + if (!cscfg_config_desc_get(config_desc)) { > + err = -EINVAL; > break; > + } > + > /* > * increment the global active count - control changes to > * active configurations > */ > atomic_inc(&cscfg_mgr->sys_active_cnt); Seems to me, it is more reasonable to use 'sys_active_cnt' to acquire the module reference instead of 'config_desc->active_cnt'. The reason is 'sys_active_cnt' is a global counter. > - /* > - * mark the descriptor as active so enable config on a > - * device instance will use it > - */ > - atomic_inc(&config_desc->active_cnt); > - > err = 0; > dev_dbg(cscfg_device(), "Activate config %s.\n", config_desc->name); > break; > @@ -920,9 +937,8 @@ static void _cscfg_deactivate_config(unsigned long cfg_hash) > > list_for_each_entry(config_desc, &cscfg_mgr->config_desc_list, item) { > if ((unsigned long)config_desc->event_ea->var == cfg_hash) { > - atomic_dec(&config_desc->active_cnt); > atomic_dec(&cscfg_mgr->sys_active_cnt); > - cscfg_owner_put(config_desc->load_owner); > + cscfg_config_desc_put(config_desc); > dev_dbg(cscfg_device(), "Deactivate config %s.\n", config_desc->name); > break; > } > @@ -1047,7 +1063,7 @@ int cscfg_csdev_enable_active_config(struct coresight_device *csdev, > unsigned long cfg_hash, int preset) > { > struct cscfg_config_csdev *config_csdev_active = NULL, *config_csdev_item; > - const struct cscfg_config_desc *config_desc; > + struct cscfg_config_desc *config_desc; > unsigned long flags; > int err = 0; > > @@ -1062,8 +1078,8 @@ int cscfg_csdev_enable_active_config(struct coresight_device *csdev, > raw_spin_lock_irqsave(&csdev->cscfg_csdev_lock, flags); > list_for_each_entry(config_csdev_item, &csdev->config_csdev_list, node) { > config_desc = config_csdev_item->config_desc; > - if ((atomic_read(&config_desc->active_cnt)) && > - ((unsigned long)config_desc->event_ea->var == cfg_hash)) { > + if (((unsigned long)config_desc->event_ea->var == cfg_hash) && > + cscfg_config_desc_get(config_desc)) { This seems to me not right. Why a config descriptor is get in multiple places? One time getting a config descriptor is in _cscfg_activate_config(), another is at here. To be honest, I am not clear what is the difference between 'activate' config and 'enable active' config. Literally, I think we only need to acquire the config at its creating phase (maybe match to activate config?). > config_csdev_active = config_csdev_item; > csdev->active_cscfg_ctxt = (void *)config_csdev_active; > break; > @@ -1097,7 +1113,11 @@ int cscfg_csdev_enable_active_config(struct coresight_device *csdev, > err = -EBUSY; > raw_spin_unlock_irqrestore(&csdev->cscfg_csdev_lock, flags); > } > + > + if (err) > + cscfg_config_desc_put(config_desc); > } > + > return err; > } > EXPORT_SYMBOL_GPL(cscfg_csdev_enable_active_config); > @@ -1136,8 +1156,10 @@ void cscfg_csdev_disable_active_config(struct coresight_device *csdev) > raw_spin_unlock_irqrestore(&csdev->cscfg_csdev_lock, flags); > > /* true if there was an enabled active config */ > - if (config_csdev) > + if (config_csdev) { > cscfg_csdev_disable_config(config_csdev); > + cscfg_config_desc_put(config_csdev->config_desc); > + } > } > EXPORT_SYMBOL_GPL(cscfg_csdev_disable_active_config); > > -- > LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7} >

1 month, 2 weeks

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight March 2025