CoreSight January 2025

coresight@lists.linaro.org

8 participants
41 discussions

[PATCH] coresight: docs: Remove target sink from examples

by James Clark

Previously the sink had to be specified, but now it auto selects one by default. Including a sink in the examples causes issues when copy pasting the command because it might not work if that sink isn't present. Remove the sink from all the basic examples and create a new section specifically about overriding the default one. Make the text a but more concise now that it's in the advanced section, and similarly for removing the old kernel advice. Signed-off-by: James Clark <james.clark(a)linaro.org> --- Documentation/trace/coresight/coresight.rst | 41 ++++++++----------- .../userspace-api/perf_ring_buffer.rst | 4 +- 2 files changed, 18 insertions(+), 27 deletions(-) diff --git a/Documentation/trace/coresight/coresight.rst b/Documentation/trace/coresight/coresight.rst index d4f93d6a2d63..806699871b80 100644 --- a/Documentation/trace/coresight/coresight.rst +++ b/Documentation/trace/coresight/coresight.rst @@ -462,44 +462,35 @@ queried by the perf command line tool: cs_etm// [Kernel PMU event] - linaro@linaro-nano:~$ - Regardless of the number of tracers available in a system (usually equal to the amount of processor cores), the "cs_etm" PMU will be listed only once. A Coresight PMU works the same way as any other PMU, i.e the name of the PMU is -listed along with configuration options within forward slashes '/'. Since a -Coresight system will typically have more than one sink, the name of the sink to -work with needs to be specified as an event option. -On newer kernels the available sinks are listed in sysFS under +provided along with configuration options within forward slashes '/' (see +`Config option formats`_). + +Advanced Perf framework usage +----------------------------- + +Sink selection +~~~~~~~~~~~~~~ + +An appropriate sink will be selected automatically for use with Perf, but since +there will typically be more than one sink, the name of the sink to use may be +specified as a special config option prefixed with '@'. + +The available sinks are listed in sysFS under ($SYSFS)/bus/event_source/devices/cs_etm/sinks/:: root@localhost:/sys/bus/event_source/devices/cs_etm/sinks# ls tmc_etf0 tmc_etr0 tpiu0 -On older kernels, this may need to be found from the list of coresight devices, -available under ($SYSFS)/bus/coresight/devices/:: - - root:~# ls /sys/bus/coresight/devices/ - etm0 etm1 etm2 etm3 etm4 etm5 funnel0 - funnel1 funnel2 replicator0 stm0 tmc_etf0 tmc_etr0 tpiu0 root@linaro-nano:~# perf record -e cs_etm/@tmc_etr0/u --per-thread program -As mentioned above in section "Device Naming scheme", the names of the devices could -look different from what is used in the example above. One must use the device names -as it appears under the sysFS. - -The syntax within the forward slashes '/' is important. The '@' character -tells the parser that a sink is about to be specified and that this is the sink -to use for the trace session. - More information on the above and other example on how to use Coresight with the perf tools can be found in the "HOWTO.md" file of the openCSD gitHub repository [#third]_. -Advanced perf framework usage ------------------------------ - AutoFDO analysis using the perf tools ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -508,7 +499,7 @@ perf can be used to record and analyze trace of programs. Execution can be recorded using 'perf record' with the cs_etm event, specifying the name of the sink to record to, e.g:: - perf record -e cs_etm/@tmc_etr0/u --per-thread + perf record -e cs_etm//u --per-thread The 'perf report' and 'perf script' commands can be used to analyze execution, synthesizing instruction and branch events from the instruction trace. @@ -572,7 +563,7 @@ sort example is from the AutoFDO tutorial (https://gcc.gnu.org/wiki/AutoFDO/Tuto Bubble sorting array of 30000 elements 5910 ms - $ perf record -e cs_etm/@tmc_etr0/u --per-thread taskset -c 2 ./sort + $ perf record -e cs_etm//u --per-thread taskset -c 2 ./sort Bubble sorting array of 30000 elements 12543 ms [ perf record: Woken up 35 times to write data ] diff --git a/Documentation/userspace-api/perf_ring_buffer.rst b/Documentation/userspace-api/perf_ring_buffer.rst index bde9d8cbc106..dc71544532ce 100644 --- a/Documentation/userspace-api/perf_ring_buffer.rst +++ b/Documentation/userspace-api/perf_ring_buffer.rst @@ -627,7 +627,7 @@ regular ring buffer. AUX events and AUX trace data are two different things. Let's see an example:: - perf record -a -e cycles -e cs_etm/@tmc_etr0/ -- sleep 2 + perf record -a -e cycles -e cs_etm// -- sleep 2 The above command enables two events: one is the event *cycles* from PMU and another is the AUX event *cs_etm* from Arm CoreSight, both are saved @@ -766,7 +766,7 @@ only record AUX trace data at a specific time point which users are interested in. E.g. below gives an example of how to take snapshots with 1 second interval with Arm CoreSight:: - perf record -e cs_etm/@tmc_etr0/u -S -a program & + perf record -e cs_etm//u -S -a program & PERFPID=$! while true; do kill -USR2 $PERFPID -- 2.34.1

3 days, 11 hours

[PATCH] 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. Signed-off-by: James Clark <james.clark(a)linaro.org> --- drivers/hwtracing/coresight/Kconfig | 9 ++ drivers/hwtracing/coresight/Makefile | 1 + drivers/hwtracing/coresight/coresight-core.c | 1 + .../coresight/coresight-kunit-tests.c | 90 +++++++++++++++++++ 4 files changed, 101 insertions(+) create mode 100644 drivers/hwtracing/coresight/coresight-kunit-tests.c diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index 06f0a7594169..056d04bc540a 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -247,4 +247,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 4ba478211b31..c170a41b3056 100644 --- a/drivers/hwtracing/coresight/Makefile +++ b/drivers/hwtracing/coresight/Makefile @@ -51,3 +51,4 @@ coresight-cti-y := coresight-cti-core.o coresight-cti-platform.o \ coresight-cti-sysfs.o obj-$(CONFIG_ULTRASOC_SMB) += ultrasoc-smb.o obj-$(CONFIG_CORESIGHT_DUMMY) += coresight-dummy.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 ea38ecf26fcb..ce63b68d5503 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -869,6 +869,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..d022bacc8357 --- /dev/null +++ b/drivers/hwtracing/coresight/coresight-kunit-tests.c @@ -0,0 +1,90 @@ +// 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 coresight_free_test_device(struct device *dev, + struct coresight_device *csdev) +{ + devm_kfree(dev, csdev->pdata->out_conns); + devm_kfree(dev, csdev->pdata); + devm_kfree(dev, csdev); +} + +static int coresight_test_cpuid(struct coresight_device *csdev) +{ + return 0; +} + +static void test_default_sink(struct kunit *test) +{ + /* + * ETM -> ETF -> ETR -> CATU + * ^ + * | default + */ + struct device *dev = kunit_device_register(test, "coresight_kunit"); + struct coresight_device *etm = coresight_test_device(dev), + *etf = coresight_test_device(dev), + *etr = coresight_test_device(dev), + *catu = coresight_test_device(dev); + struct coresight_connection conn = {}; + struct coresight_ops_source src_ops = {.cpu_id = coresight_test_cpuid }; + struct coresight_ops etm_cs_ops = { .source_ops = &src_ops }; + + etm->type = CORESIGHT_DEV_TYPE_SOURCE; + etm->ops = &etm_cs_ops; + 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 = etm; + conn.dest_dev = etf; + coresight_add_out_conn(dev, etm->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(etm), etr); + + coresight_free_test_device(dev, etm); + coresight_free_test_device(dev, etf); + coresight_free_test_device(dev, etr); + coresight_free_test_device(dev, catu); +} + +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"); -- 2.34.1

3 days, 13 hours

[PATCH v4] coresight-etm4x: add isb() before reading the TRCSTATR

by Yuanfang Zhang

As recommended by section 4.3.7 ("Synchronization when using system instructions to progrom the trace unit") of ARM IHI 0064H.b, the self-hosted trace analyzer must perform a Context synchronization event between writing to the TRCPRGCTLR and reading the TRCSTATR. Additionally, add an ISB between the each read of TRCSTATR on coresight_timeout() when using system instructions to program the trace unit. Fixes: 1ab3bb9df5e3 ("coresight: etm4x: Add necessary synchronization for sysreg access") Signed-off-by: Yuanfang Zhang <quic_yuanfang(a)quicinc.com> --- drivers/hwtracing/coresight/coresight-core.c | 20 ++++++--- drivers/hwtracing/coresight/coresight-etm4x-core.c | 48 +++++++++++++++++++--- include/linux/coresight.h | 4 ++ 3 files changed, 62 insertions(+), 10 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index ea38ecf26fcbfb383eb40a222788bf5d7976013d..c42aa9fddab9b752361b84deab6b10be045b7104 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1017,18 +1017,20 @@ static void coresight_remove_conns(struct coresight_device *csdev) } /** - * coresight_timeout - loop until a bit has changed to a specific register - * state. + * coresight_timeout_action - loop until a bit has changed to a specific register + * state, with a callback after every trial. * @csa: coresight device access for the device * @offset: Offset of the register from the base of the device. * @position: the position of the bit of interest. * @value: the value the bit should have. + * @cb: Call back after each trial. * * Return: 0 as soon as the bit has taken the desired state or -EAGAIN if * TIMEOUT_US has elapsed, which ever happens first. */ -int coresight_timeout(struct csdev_access *csa, u32 offset, - int position, int value) +int coresight_timeout_action(struct csdev_access *csa, u32 offset, + int position, int value, + coresight_timeout_cb_t cb) { int i; u32 val; @@ -1044,7 +1046,8 @@ int coresight_timeout(struct csdev_access *csa, u32 offset, if (!(val & BIT(position))) return 0; } - + if (cb) + cb(csa, offset, position, value); /* * Delay is arbitrary - the specification doesn't say how long * we are expected to wait. Extra check required to make sure @@ -1056,6 +1059,13 @@ int coresight_timeout(struct csdev_access *csa, u32 offset, return -EAGAIN; } +EXPORT_SYMBOL_GPL(coresight_timeout_action); + +int coresight_timeout(struct csdev_access *csa, u32 offset, + int position, int value) +{ + return coresight_timeout_action(csa, offset, position, value, NULL); +} EXPORT_SYMBOL_GPL(coresight_timeout); u32 coresight_relaxed_read32(struct coresight_device *csdev, u32 offset) diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index dd8c74f893dbdfe1a35d811ef2dc476b81ce07ae..b933cdc4e52fd5ecd16d3ad97cf0d3f1a3fd87e4 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -399,6 +399,29 @@ static void etm4_check_arch_features(struct etmv4_drvdata *drvdata, } #endif /* CONFIG_ETM4X_IMPDEF_FEATURE */ +static void etm4x_sys_ins_barrier(struct csdev_access *csa, u32 offset, int pos, int val) +{ + if (!csa->io_mem) + isb(); +} + +/* + * etm4x_wait_status: Poll for TRCSTATR.<pos> == <val>. While using system + * instruction to access the trace unit, each access must be separated by a + * synchronization barrier. See ARM IHI0064H.b section "4.3.7 Synchronization of + * register updates", for system instructions section, in "Notes": + * + * "In particular, whenever disabling or enabling the trace unit, a poll of + * TRCSTATR needs explicit synchronization between each read of TRCSTATR" + */ +static int etm4x_wait_status(struct csdev_access *csa, int pos, int val) +{ + if (!csa->io_mem) + return coresight_timeout_action(csa, TRCSTATR, pos, val, + etm4x_sys_ins_barrier); + return coresight_timeout(csa, TRCSTATR, pos, val); +} + static int etm4_enable_hw(struct etmv4_drvdata *drvdata) { int i, rc; @@ -430,7 +453,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) isb(); /* wait for TRCSTATR.IDLE to go up */ - if (coresight_timeout(csa, TRCSTATR, TRCSTATR_IDLE_BIT, 1)) + if (etm4x_wait_status(csa, TRCSTATR_IDLE_BIT, 1)) dev_err(etm_dev, "timeout while waiting for Idle Trace Status\n"); if (drvdata->nr_pe) @@ -523,7 +546,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) isb(); /* wait for TRCSTATR.IDLE to go back down to '0' */ - if (coresight_timeout(csa, TRCSTATR, TRCSTATR_IDLE_BIT, 0)) + if (etm4x_wait_status(csa, TRCSTATR_IDLE_BIT, 0)) dev_err(etm_dev, "timeout while waiting for Idle Trace Status\n"); @@ -906,10 +929,25 @@ static void etm4_disable_hw(void *info) tsb_csync(); etm4x_relaxed_write32(csa, control, TRCPRGCTLR); + /* + * As recommended by section 4.3.7 ("Synchronization when using system + * instructions to progrom the trace unit") of ARM IHI 0064H.b, the + * self-hosted trace analyzer must perform a Context synchronization + * event between writing to the TRCPRGCTLR and reading the TRCSTATR. + */ + if (!csa->io_mem) + isb(); + /* wait for TRCSTATR.PMSTABLE to go to '1' */ - if (coresight_timeout(csa, TRCSTATR, TRCSTATR_PMSTABLE_BIT, 1)) + if (etm4x_wait_status(csa, TRCSTATR_PMSTABLE_BIT, 1)) dev_err(etm_dev, "timeout while waiting for PM stable Trace Status\n"); + /* + * As recommended by section 4.3.7 (Synchronization of register updates) + * of ARM IHI 0064H.b. + */ + isb(); + /* read the status of the single shot comparators */ for (i = 0; i < drvdata->nr_ss_cmp; i++) { config->ss_status[i] = @@ -1711,7 +1749,7 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata) etm4_os_lock(drvdata); /* wait for TRCSTATR.PMSTABLE to go up */ - if (coresight_timeout(csa, TRCSTATR, TRCSTATR_PMSTABLE_BIT, 1)) { + if (etm4x_wait_status(csa, TRCSTATR_PMSTABLE_BIT, 1)) { dev_err(etm_dev, "timeout while waiting for PM Stable Status\n"); etm4_os_unlock(drvdata); @@ -1802,7 +1840,7 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata) state->trcpdcr = etm4x_read32(csa, TRCPDCR); /* wait for TRCSTATR.IDLE to go up */ - if (coresight_timeout(csa, TRCSTATR, TRCSTATR_IDLE_BIT, 1)) { + if (etm4x_wait_status(csa, TRCSTATR_PMSTABLE_BIT, 1)) { dev_err(etm_dev, "timeout while waiting for Idle Trace Status\n"); etm4_os_unlock(drvdata); diff --git a/include/linux/coresight.h b/include/linux/coresight.h index c1334259427850173ccc5872a6ced550a99f8e1c..f106b1025111892e9d44ac643f7409552952d4da 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -639,6 +639,10 @@ extern int coresight_enable_sysfs(struct coresight_device *csdev); extern void coresight_disable_sysfs(struct coresight_device *csdev); extern int coresight_timeout(struct csdev_access *csa, u32 offset, int position, int value); +typedef void (*coresight_timeout_cb_t) (struct csdev_access *, u32, int, int); +extern int coresight_timeout_action(struct csdev_access *csa, u32 offset, + int position, int value, + coresight_timeout_cb_t cb); extern int coresight_claim_device(struct coresight_device *csdev); extern int coresight_claim_device_unlocked(struct coresight_device *csdev); --- base-commit: fac04efc5c793dccbd07e2d59af9f90b7fc0dca4 change-id: 20250116-etm_sync-7be51cc6cbcd Best regards, -- Yuanfang Zhang <quic_yuanfang(a)quicinc.com>

1 week, 3 days

Re: CoreSight on Nvidia Jetson

by vincent.ernst＠web.de

Hi Suzuki, thanks for the reply! The CPUs of the boards I am using are all based on Arm-v8(.2), but I found the components' addresses in the manuals of the SoCs. I managed to modify the Devicetree by writing my own .dtsi file (see attachment) and finally got the CoreSight devices in /sys/devices/. However, dmesg shows the following: [ 0.000000] Booting Linux on physical CPU 0x0 [ 0.000000] Linux version 4.9.253-coresight (user@user-desktop) (gcc version 7.5.0 (Ubuntu/Linaro 7.5.0-3ubuntu1~18.04) ) #1 SMP PREEMPT Wed Jan 1 18:45:04 CET 2025 [ 0.000000] Boot CPU: AArch64 Processor [411fd071] (omitted 87 lines) [ 0.212039] DTS File Name: /home/user/Downloads/Linux_for_Tegra/source/public/kernel/kernel-4.9/arch/arm64/boot/dts/../../../../../../hardware/nvidia/platform/t210/porg/kernel-dts/tegra210-p3448-0000-p3449-0000-a02.dts [ 0.212045] DTB Build time: Jan 1 2025 16:04:45 (omitted 35 lines) [ 0.420616] DTS File Name: /home/user/Downloads/Linux_for_Tegra/source/public/kernel/kernel-4.9/arch/arm64/boot/dts/../../../../../../hardware/nvidia/platform/t210/porg/kernel-dts/tegra210-p3448-0000-p3449-0000-a02.dts [ 0.420622] DTB Build time: Jan 1 2025 16:04:45 (omitted 75 lines) [ 0.524166] OF: amba_device_add() failed (-19) for /funnel_bccplex@73001000 (omitted 367 lines) [ 1.330484] OF: graph: no port node found in /etf@72030000 [ 1.330757] OF: graph: no port node found in /etr@72050000 [ 1.330987] OF: graph: no port node found in /funnel_major@72010000 [ 1.331238] OF: graph: no port node found in /ptm0@73440000 [ 1.331451] coresight-etm4x 73440000.ptm0: CPU0: Cortex-A57 ETM v4.0 initialized [ 1.331482] OF: graph: no port node found in /ptm1@73540000 [ 1.331689] coresight-etm4x 73540000.ptm1: CPU1: Cortex-A57 ETM v4.0 initialized [ 1.331719] OF: graph: no port node found in /ptm2@73640000 [ 1.331938] coresight-etm4x 73640000.ptm2: CPU2: Cortex-A57 ETM v4.0 initialized [ 1.331944] extcon-disp-state extcon:disp-state: cable 47 state 0 [ 1.331946] Extcon AUX1(HDMI) disable [ 1.331976] OF: graph: no port node found in /ptm3@73740000 [ 1.332192] coresight-etm4x 73740000.ptm3: CPU3: Cortex-A57 ETM v4.0 initialized [ 1.332250] OF: graph: no port node found in /replicator@72040000 [ 1.332305] coresight-replicator-qcom 72040000.replicator: REPLICATOR 1.0 initialized [ 1.332350] OF: graph: no port node found in /stm@72070000 [ 1.332386] coresight-stm 72070000.stm: stm_register_device failed, probing deffered (omitted 64 lines) [ 1.411751] OF: graph: no port node found in /stm@72070000 [ 1.412025] coresight-stm 72070000.stm: STM32 initialized (omitted 212 lines) Do you have an idea what I did wrong? In the end, I want to be able to follow the steps described here: https://docs.nvidia.com/jetson/archives/l4t-archived/l4t-3275/index.html#pa… Best regards, Vincent (P.S. There was a problem sending this email a first time, but it should work now)

2 weeks, 3 days

[PATCH v13 0/8] Coresight for Kernel panic and watchdog reset

by Linu Cherian

This patch series is rebased on coresight-next-v6.12.rc4 * Patches 1 & 2 adds support for allocation of trace buffer pages from reserved RAM * Patches 3 & 4 adds support for saving metadata at the time of kernel panic * Patch 5 adds support for reading trace data captured at the time of panic * Patches 6 & 7 adds support for disabling coresight blocks at the time of panic * Patch 8: Gives the full description about this feature as part of documentation v12 is posted here, https://lore.kernel.org/linux-arm-kernel/20241129084714.3057080-1-lcherian@… Changelog from v12: * Fixed wrong buffer pointer passed to coresigh_insert_barrier_packet * tmc_read_prepare/unprepare_crashdata need to be called only once and hence removed from read path and added to tmc_probe * tmc_read_prepare_crashdata renamed to tmc_prepare_crashdata and avoid taking locks as its moved to probe function. * Introduced read status flag, "reading" specific to reserved buffer to keep the reserved buffer reading independent of the regular buffer. * open/release ops for reserved buffer has to take care only about the set/unset the "reading" status flag as the reserved buffer is prepared during the probe time itself. * Few other trivial changes Changelog from v11: Convert all commands to literal code blocks, that was missed out in v11. No other code changes. Changelog from v10: * Converted all csdev_access_* to readl functions in tmc_panic_sync_* * Added "tmc" prefix for register snapshots in struct tmc_crash_metadata * Converted dev_info to dev_dbg in panic handlers * Converted dsb to dmb in panic handlers * Fixed marking metadata as invalid when a user is trying to use the reserved buffer. Earlier this was wrongly set at the time of reading reserved trace buffer. * Moved common validation checks to is_tmc_crashdata_valid and minor code rearrangements for efficiency * Got rid of sink specific prepare/unprepare invocations * Got rid of full from struct tmc_resrv_buf * While reading crashdata, size is now calculated from metadata instead of relying on reserved buffer size populated by dtb * Minor documenation fixes Changelog from v9: * Add common helper function of_tmc_get_reserved_resource_by_name for better code reuse * Reserved buffer validity and crashdata validity has been separated to avoid interdependence * New fields added to crash metadata: version, ffcr, ffsr, mode * Version checks added for metadata validation * Special file /dev/crash_tmc_xxx would be available only when crash metadata is valid * Removed READ_CRASHDATA mode meant for special casing crashdata reads. Instead, dedicated read function added for crashdata reads from reserved buffer which is common for both ETR and ETF sinks as well. * Documentation added to Documentation/tracing/coresight/panic.rst Changelog from v8: * Added missing exit path on error in __tmc_probe. * Few whitespace fixes, checkpatch fixes. * With perf sessions honouring stop_on_flush sysfs attribute, removed redundant variable stop_on_flush_en. Changelog from v7: * Fixed breakage on perf test -vvvv "arm coresight". No issues seen with and without "resrv" buffer mode * Moved the crashdev registration into a separate function. * Removed redundant variable in tmc_etr_setup_crashdata_buf * Avoided a redundant memcpy in tmc_panic_sync_etf. * Tested kernel panic with trace session started uisng perf. Please see the title "Perf based testing" below for details. For this, stop_on_flush sysfs attribute is taken into consideration while starting perf sessions as well. Changelog from v6: * Added special device files for reading crashdata, so that read_prevboot mode flag is removed. * Added new sysfs TMC device attribute, stop_on_flush. Stop on flush trigger event is disabled by default. User need to explicitly enable this from sysfs for panic stop to work. * Address parameter for panicstop ETM configuration is chosen as kernel "panic" address by default. * Added missing tmc_wait_for_tmcready during panic handling * Few other misc code rearrangements. Changelog from v5: * Fixed issues reported by CONFIG_DEBUG_ATOMIC_SLEEP * Fixed a memory leak while reading data from /dev/tmc_etrx in READ_PREVBOOT mode * Tested reading trace data from crashdump kernel Changelog from v4: * Device tree binding - Description is made more explicit on the usage of reserved memory region - Mismatch in memory region names in dts binding and driver fixed - Removed "mem" suffix from the memory region names * Rename "struct tmc_register_snapshot" -> "struct tmc_crash_metadata", since it contains more than register snapshot. Related variables are named accordingly. * Rename struct tmc_drvdata members resrv_buf -> crash_tbuf metadata -> crash_mdata * Size field in metadata refers to RSZ register and hence indicates the size in 32 bit words. ETR metadata follows this convention, the same has been extended to ETF metadata as well. * Added crc32 for more robust metadata and tracedata validation. * Added/modified dev_dbg messages during metadata validation * Fixed a typo in patch 5 commit description Changelog from v3: * Converted the Coresight ETM driver change to a named configuration. RFC tag has been removed with this change. * Fixed yaml issues reported by "make dt_binding_check" * Added names for reserved memory regions 0 and 1 * Added prevalidation checks for metadata processing * Fixed a regression introduced in RFC v3 - TMC Status register was getting saved wrongly * Reverted memremap attribute changes from _WB to _WC to match with the dma map attributes * Introduced reserved buffer mode specific .sync op. This fixes a possible crash when reserved buffer mode was used in normal trace capture, due to unwanted dma maintenance operations. Linu Cherian (8): dt-bindings: arm: coresight-tmc: Add "memory-region" property coresight: tmc-etr: Add support to use reserved trace memory coresight: core: Add provision for panic callbacks coresight: tmc: Enable panic sync handling coresight: tmc: Add support for reading crash data coresight: tmc: Stop trace capture on FlIn coresight: config: Add preloaded configuration Documentation: coresight: Panic support .../bindings/arm/arm,coresight-tmc.yaml | 26 ++ Documentation/trace/coresight/panic.rst | 362 ++++++++++++++++++ drivers/hwtracing/coresight/Makefile | 2 +- .../coresight/coresight-cfg-preload.c | 2 + .../coresight/coresight-cfg-preload.h | 2 + .../hwtracing/coresight/coresight-cfg-pstop.c | 83 ++++ drivers/hwtracing/coresight/coresight-core.c | 42 ++ .../hwtracing/coresight/coresight-tmc-core.c | 308 ++++++++++++++- .../hwtracing/coresight/coresight-tmc-etf.c | 92 ++++- .../hwtracing/coresight/coresight-tmc-etr.c | 184 ++++++++- drivers/hwtracing/coresight/coresight-tmc.h | 105 +++++ include/linux/coresight.h | 12 + 12 files changed, 1208 insertions(+), 12 deletions(-) create mode 100644 Documentation/trace/coresight/panic.rst create mode 100644 drivers/hwtracing/coresight/coresight-cfg-pstop.c -- 2.34.1

3 weeks, 4 days

[PATCH] coresight-tmc: stop disabling tmc when flush timeout

by Yuanfang Zhang

When multiple ETMs are enabled simultaneously, the time required to complete a flush in the process of reading the TMC device node may exceed the default wait time of 100us. If the TMC capture is stopped while any ETM has not completed its flush, it can cause the corresponding CPU to hang. Fix the by checking the TMCReady bit after the flush. If TMCReady bit is set, TraceCaptEn bit can be clear; otherwise, return directly and stop the TMC read. Signed-off-by: Yuanfang Zhang <quic_yuanfang(a)quicinc.com> --- drivers/hwtracing/coresight/coresight-tmc-etf.c | 17 +++++++++++++++-- drivers/hwtracing/coresight/coresight-tmc-etr.c | 22 +++++++++++++++++----- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c index d4f641cd9de69488fe3d1c1dc9b5a9eafb55ed59..bded290c42891d782344d9a6e63ebdbed6719133 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c @@ -80,11 +80,21 @@ static void tmc_etb_dump_hw(struct tmc_drvdata *drvdata) return; } -static void __tmc_etb_disable_hw(struct tmc_drvdata *drvdata) +static int __tmc_etb_disable_hw(struct tmc_drvdata *drvdata) { + int rc; + CS_UNLOCK(drvdata->base); tmc_flush_and_stop(drvdata); + + rc = tmc_wait_for_tmcready(drvdata); + if (rc) { + dev_err(&drvdata->csdev->dev, + "Failed to disable : TMC is not ready\n"); + CS_LOCK(drvdata->base); + return rc; + } /* * When operating in sysFS mode the content of the buffer needs to be * read before the TMC is disabled. @@ -94,6 +104,7 @@ static void __tmc_etb_disable_hw(struct tmc_drvdata *drvdata) tmc_disable_hw(drvdata); CS_LOCK(drvdata->base); + return 0; } static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata) @@ -650,7 +661,9 @@ int tmc_read_prepare_etb(struct tmc_drvdata *drvdata) ret = -EINVAL; goto out; } - __tmc_etb_disable_hw(drvdata); + ret = __tmc_etb_disable_hw(drvdata); + if (ret) + goto out; } drvdata->reading = true; diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index a48bb85d0e7f44a25b813f3c828cc3d705d16012..63a1f7501562fa0b5c2fe6ea53dce4d82842bec3 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -1135,11 +1135,21 @@ static void tmc_etr_sync_sysfs_buf(struct tmc_drvdata *drvdata) } } -static void __tmc_etr_disable_hw(struct tmc_drvdata *drvdata) +static int __tmc_etr_disable_hw(struct tmc_drvdata *drvdata) { + int rc; + CS_UNLOCK(drvdata->base); tmc_flush_and_stop(drvdata); + + rc = tmc_wait_for_tmcready(drvdata); + if (rc) { + dev_err(&drvdata->csdev->dev, + "Failed to disable : TMC is not ready\n"); + CS_LOCK(drvdata->base); + return rc; + } /* * When operating in sysFS mode the content of the buffer needs to be * read before the TMC is disabled. @@ -1150,7 +1160,7 @@ static void __tmc_etr_disable_hw(struct tmc_drvdata *drvdata) tmc_disable_hw(drvdata); CS_LOCK(drvdata->base); - + return 0; } void tmc_etr_disable_hw(struct tmc_drvdata *drvdata) @@ -1779,9 +1789,11 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata) } /* Disable the TMC if we are trying to read from a running session. */ - if (coresight_get_mode(drvdata->csdev) == CS_MODE_SYSFS) - __tmc_etr_disable_hw(drvdata); - + if (coresight_get_mode(drvdata->csdev) == CS_MODE_SYSFS) { + ret = __tmc_etr_disable_hw(drvdata); + if (ret) + goto out; + } drvdata->reading = true; out: spin_unlock_irqrestore(&drvdata->spinlock, flags); --- base-commit: fac04efc5c793dccbd07e2d59af9f90b7fc0dca4 change-id: 20250103-fix_cpu_hung-b5a95179ada4 Best regards, -- Yuanfang Zhang <quic_yuanfang(a)quicinc.com>

1 month

Re: [PATCH v9 3/6] Coresight: Introduce a new struct coresight_path

by James Clark

On 24/01/2025 7:25 am, Jie Gan wrote: > Add 'struct coresight_path' to store the data that is needed by > coresight_enable_path/coresight_disable_path. The structure > will be transmitted to the helper and sink device to enable > related funcationalities. > > Signed-off-by: Jie Gan <quic_jiegan(a)quicinc.com> > --- [...] > /* > * If we still have access to the event_data via handle, > @@ -595,11 +599,11 @@ static void etm_event_stop(struct perf_event *event, int mode) > if (!csdev) > return; > > - path = etm_event_cpu_path(event_data, cpu); > - if (!path) > + cs_path = etm_event_cpu_path(event_data, cpu); > + if (!cs_path) I don't think renaming 'path' to 'cs_path' is worth the churn. It's in a lot of places in this change, but I think path is already good enough. > return; > > - sink = coresight_get_sink(path); > + sink = coresight_get_sink(cs_path->path); coresight_get_sink() is always called with cs_path->path, so we might as well make it take a whole path struct. The same with any of the other functions that operate on path like coresight_get_source().

1 month

[PATCH 0/3] coresight: Alloc trace ID after building the path

by James Clark

Proof of concept to support CTCU device. Applies to Jie's patchset in the parent email. I think this would be a good simplification, it removes some code and makes things a bit clearer, and works for both the old and new CTCU requirements. It will require merging into the parent patchset somehow as it undoes some of those changes. James Clark (3): coresight: Don't save handle in path coresight: Export coresight_get_sink() coresight: Alloc trace ID after building the path drivers/hwtracing/coresight/coresight-core.c | 107 +++++++++++++----- drivers/hwtracing/coresight/coresight-dummy.c | 9 +- drivers/hwtracing/coresight/coresight-etb10.c | 8 +- .../hwtracing/coresight/coresight-etm-perf.c | 20 ++-- drivers/hwtracing/coresight/coresight-etm.h | 1 - .../coresight/coresight-etm3x-core.c | 84 ++------------ .../coresight/coresight-etm3x-sysfs.c | 3 +- .../coresight/coresight-etm4x-core.c | 83 ++------------ .../coresight/coresight-etm4x-sysfs.c | 4 +- drivers/hwtracing/coresight/coresight-etm4x.h | 1 - drivers/hwtracing/coresight/coresight-priv.h | 17 +-- drivers/hwtracing/coresight/coresight-stm.c | 5 +- drivers/hwtracing/coresight/coresight-sysfs.c | 6 +- .../hwtracing/coresight/coresight-tmc-etf.c | 9 +- .../hwtracing/coresight/coresight-tmc-etr.c | 13 +-- drivers/hwtracing/coresight/coresight-tmc.h | 2 +- drivers/hwtracing/coresight/coresight-tpda.c | 3 +- drivers/hwtracing/coresight/coresight-tpdm.c | 3 +- drivers/hwtracing/coresight/coresight-tpiu.c | 2 +- drivers/hwtracing/coresight/coresight-trbe.c | 4 +- drivers/hwtracing/coresight/ultrasoc-smb.c | 8 +- include/linux/coresight.h | 25 +++- 22 files changed, 159 insertions(+), 258 deletions(-) -- 2.34.1

1 month

Re: [PATCH v9 5/6] Coresight: Add Coresight TMC Control Unit driver

by James Clark

On 29/01/2025 1:02 pm, Jie Gan wrote: > > > On 1/29/2025 6:35 PM, James Clark wrote: >> >> >> On 29/01/2025 12:46 am, Jie Gan wrote: >>> >>> >>> On 1/28/2025 7:55 PM, James Clark wrote: >>>> >>>> >>>> On 24/01/2025 7:25 am, Jie Gan wrote: >>>>> The Coresight TMC Control Unit hosts miscellaneous configuration >>>>> registers >>>>> which control various features related to TMC ETR sink. >>>>> >>>>> Based on the trace ID, which is programmed in the related CTCU ATID >>>>> register of a specific ETR, trace data with that trace ID gets into >>>>> the ETR buffer, while other trace data gets dropped. >>>>> >>>>> Enabling source device sets one bit of the ATID register based on >>>>> source device's trace ID. >>>>> Disabling source device resets the bit according to the source >>>>> device's trace ID. >>>>> >>>>> Signed-off-by: Jie Gan <quic_jiegan(a)quicinc.com> >>>>> --- >>>>> drivers/hwtracing/coresight/Kconfig | 12 + >>>>> drivers/hwtracing/coresight/Makefile | 1 + >>>>> drivers/hwtracing/coresight/coresight-ctcu.c | 276 ++++++++++++++ >>>>> + ++++ >>>>> drivers/hwtracing/coresight/coresight-ctcu.h | 30 ++ >>>>> include/linux/coresight.h | 3 +- >>>>> 5 files changed, 321 insertions(+), 1 deletion(-) >>>>> create mode 100644 drivers/hwtracing/coresight/coresight-ctcu.c >>>>> create mode 100644 drivers/hwtracing/coresight/coresight-ctcu.h >>>> > >>>> >>>> [...] >>>> >>>>> +/* >>>>> + * ctcu_set_etr_traceid: Retrieve the ATID offset and trace ID. >>>>> + * >>>>> + * Returns 0 indicates success. None-zero result means failure. >>>>> + */ >>>>> +static int ctcu_set_etr_traceid(struct coresight_device *csdev, >>>>> struct coresight_path *cs_path, >>>>> + bool enable) >>>>> +{ >>>>> + struct coresight_device *sink = coresight_get_sink(cs_path- >>>>> >path); >>>>> + struct ctcu_drvdata *drvdata = dev_get_drvdata(csdev- >>>>> >dev.parent); >>>>> + u8 trace_id = cs_path->trace_id; >>>>> + int port_num; >>>>> + >>>>> + if ((sink == NULL) || !IS_VALID_CS_TRACE_ID(trace_id) || >>>>> IS_ERR_OR_NULL(drvdata)) { >>>>> + dev_err(&csdev->dev, "Invalid parameters\n"); >>>>> + return -EINVAL; >>>>> + } >>>>> + >>>>> + port_num = ctcu_get_active_port(sink, csdev); >>>>> + if (port_num < 0) >>>>> + return -EINVAL; >>>>> + >>>>> + /* >>>>> + * Skip the disable session if more than one TPDM device that >>>>> + * connected to the same TPDA device has been enabled. >>>>> + */ >>>>> + if (enable) >>>>> + atomic_inc(&drvdata->traceid_refcnt[port_num][trace_id]); >>>>> + else { >>>>> + if (atomic_dec_return(&drvdata->traceid_refcnt[port_num] >>>>> [trace_id]) > 0) { >>>>> + dev_dbg(&csdev->dev, "Skip the disable session\n"); >>>>> + return 0; >>>>> + } >>>>> + } >>>>> + >>>>> + dev_dbg(&csdev->dev, "traceid is %d\n", cs_path->trace_id); >>>>> + >>>>> + return __ctcu_set_etr_traceid(csdev, trace_id, port_num, enable); >>>> >>>> Hi Jie, >>>> >>>> Using atomic_dec_return() here doesn't prevent >>>> __ctcu_set_etr_traceid() from running concurrent enable and >>>> disables. Once you pass the atomic_dec_return() a second call to >>>> enable it will mess it up. >>>> >>>> I think you need a spinlock around the whole thing and then the >>>> refcounts don't need to be atomics. >>>> >>> Hi, James >>> Thanks for comment. I may not fully tested my codes here. What I was >>> thinking is there's no way the refcnt could become a negative number >>> under current framework. So I just added spinlock in >>> __ctcu_set_etr_traceid() to ensure concurrent sessions correctly >>> manipulate the register. >>> >>> As the trace_id related to the bit of the ATID register, I think the >>> concurrent processes are working fine with spinlock around read/write >>> register. >>> >>> I may not fully got your point here. Please help me to correct it. >>> >>> Thanks, >>> Jie >>> >>> >> >> No it can't become negative, but the refcount can be a different state >> to the one that was actually written: >> >> >> CPU0 CPU1 >> ---- ---- >> ctcu_set_etr_traceid(enable) >> ctcu_set_etr_traceid(disable) >> atomic_inc() >> recount == 1 >> atomic_dec() >> recount == 0 >> >> __ctcu_set_etr_traceid(disable) >> Lock and write disable state to >> device >> >> __ctcu_set_etr_traceid(enable) >> Lock and write enable state to >> device >> >> >> As you can see this leaves the device in an enabled state but the >> refcount is 0. > Yes, you are right. I didnt consider this scenario. We definitely need > spinlock here. > >> >> This is also quite large if you use atomic types: >> >> /* refcnt for each traceid of each sink */ >> atomic_t traceid_refcnt[ATID_MAX_NUM][CORESIGHT_TRACE_ID_RES_TOP]; >> >> Presumably you can't have the refcount for each ID be higher than the >> max number of TPDMs connected? If you make the locked area a bit wider >> you don't need atomic types and also solve the above problem. So you >> could do u8, or DECLARE_BITMAP() and bitmap_read() etc to read 3 bit >> values. Or however wide it needs to be. > The original purpose of using atomic here is trying to narrow the locked > area. > > I think u8 is ok here. > u8 traceid_refcnt[ATID_MAX_NUM][CORESIGHT_TRACE_ID_RES_TOP] will cost > 224 bytes, I think it's acceptable here. > > Thanks, > Jie > Yep u8 sounds ok then

1 month, 1 week

Re: [PATCH v9 5/6] Coresight: Add Coresight TMC Control Unit driver

by James Clark

On 29/01/2025 12:46 am, Jie Gan wrote: > > > On 1/28/2025 7:55 PM, James Clark wrote: >> >> >> On 24/01/2025 7:25 am, Jie Gan wrote: >>> The Coresight TMC Control Unit hosts miscellaneous configuration >>> registers >>> which control various features related to TMC ETR sink. >>> >>> Based on the trace ID, which is programmed in the related CTCU ATID >>> register of a specific ETR, trace data with that trace ID gets into >>> the ETR buffer, while other trace data gets dropped. >>> >>> Enabling source device sets one bit of the ATID register based on >>> source device's trace ID. >>> Disabling source device resets the bit according to the source >>> device's trace ID. >>> >>> Signed-off-by: Jie Gan <quic_jiegan(a)quicinc.com> >>> --- >>> drivers/hwtracing/coresight/Kconfig | 12 + >>> drivers/hwtracing/coresight/Makefile | 1 + >>> drivers/hwtracing/coresight/coresight-ctcu.c | 276 +++++++++++++++++++ >>> drivers/hwtracing/coresight/coresight-ctcu.h | 30 ++ >>> include/linux/coresight.h | 3 +- >>> 5 files changed, 321 insertions(+), 1 deletion(-) >>> create mode 100644 drivers/hwtracing/coresight/coresight-ctcu.c >>> create mode 100644 drivers/hwtracing/coresight/coresight-ctcu.h >> > >> >> [...] >> >>> +/* >>> + * ctcu_set_etr_traceid: Retrieve the ATID offset and trace ID. >>> + * >>> + * Returns 0 indicates success. None-zero result means failure. >>> + */ >>> +static int ctcu_set_etr_traceid(struct coresight_device *csdev, >>> struct coresight_path *cs_path, >>> + bool enable) >>> +{ >>> + struct coresight_device *sink = coresight_get_sink(cs_path->path); >>> + struct ctcu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); >>> + u8 trace_id = cs_path->trace_id; >>> + int port_num; >>> + >>> + if ((sink == NULL) || !IS_VALID_CS_TRACE_ID(trace_id) || >>> IS_ERR_OR_NULL(drvdata)) { >>> + dev_err(&csdev->dev, "Invalid parameters\n"); >>> + return -EINVAL; >>> + } >>> + >>> + port_num = ctcu_get_active_port(sink, csdev); >>> + if (port_num < 0) >>> + return -EINVAL; >>> + >>> + /* >>> + * Skip the disable session if more than one TPDM device that >>> + * connected to the same TPDA device has been enabled. >>> + */ >>> + if (enable) >>> + atomic_inc(&drvdata->traceid_refcnt[port_num][trace_id]); >>> + else { >>> + if (atomic_dec_return(&drvdata->traceid_refcnt[port_num] >>> [trace_id]) > 0) { >>> + dev_dbg(&csdev->dev, "Skip the disable session\n"); >>> + return 0; >>> + } >>> + } >>> + >>> + dev_dbg(&csdev->dev, "traceid is %d\n", cs_path->trace_id); >>> + >>> + return __ctcu_set_etr_traceid(csdev, trace_id, port_num, enable); >> >> Hi Jie, >> >> Using atomic_dec_return() here doesn't prevent >> __ctcu_set_etr_traceid() from running concurrent enable and disables. >> Once you pass the atomic_dec_return() a second call to enable it will >> mess it up. >> >> I think you need a spinlock around the whole thing and then the >> refcounts don't need to be atomics. >> > Hi, James > Thanks for comment. I may not fully tested my codes here. What I was > thinking is there's no way the refcnt could become a negative number > under current framework. So I just added spinlock in > __ctcu_set_etr_traceid() to ensure concurrent sessions correctly > manipulate the register. > > As the trace_id related to the bit of the ATID register, I think the > concurrent processes are working fine with spinlock around read/write > register. > > I may not fully got your point here. Please help me to correct it. > > Thanks, > Jie > > No it can't become negative, but the refcount can be a different state to the one that was actually written: CPU0 CPU1 ---- ---- ctcu_set_etr_traceid(enable) ctcu_set_etr_traceid(disable) atomic_inc() recount == 1 atomic_dec() recount == 0 __ctcu_set_etr_traceid(disable) Lock and write disable state to device __ctcu_set_etr_traceid(enable) Lock and write enable state to device As you can see this leaves the device in an enabled state but the refcount is 0. This is also quite large if you use atomic types: /* refcnt for each traceid of each sink */ atomic_t traceid_refcnt[ATID_MAX_NUM][CORESIGHT_TRACE_ID_RES_TOP]; Presumably you can't have the refcount for each ID be higher than the max number of TPDMs connected? If you make the locked area a bit wider you don't need atomic types and also solve the above problem. So you could do u8, or DECLARE_BITMAP() and bitmap_read() etc to read 3 bit values. Or however wide it needs to be.

1 month, 1 week

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight January 2025