In our hardware design, by combining a funnel and a replicator, it
implement a hardware device with one-to-one correspondence between
output ports and input ports. The programming usage on this device
is the same as funnel. The software uses a funnel and a static
replicator to implement the driver of this device. Since original
funnels only support a single output connection and original
replicator only support a single input connection, the code needs
to be modified to support this new feature. The following is a
typical topology diagram of multi-port output mechanism.
|----------| |---------| |----------| |---------|
| TPDM 0 | | Source0 | | Source 1 | | TPDM 1 |
|----------| |---------| |----------| |---------|
| | | |
| | | |
| --------- | | |
| | | |
| | | |
| | | |
\-------------/ ---------------------- |
\ Funnel 0 / | |
----------- | ------------------------------
| | |
| | |
\------------------/
\ Funnel 1 / ----|
\--------------/ |
| |----> Combine a funnel and a
| | static replicator
/-----------------\ |
/ replicator 0 \ ----|
/---------------------\
| | |
| | |-----------|
| |---------| |
| |TPDM0 |TPDM1
| \-----------------/
| \ TPDA 0 /
| \-------------/
| |
| |
|Source0/1 |
\-------------------------------/
\ Funnel 2 /
\---------------------------/
Changes in V6:
1. Optimize the prompt content of the warning log
when the filter handle is not a trace source.
-- Suzuki K Poulose
2. Reset the filter device and fwnode if it is not
a trace source.
-- Suzuki K Poulose
Changes in V5:
1. Replace "filter-src" with "filter-source" in the
dt-binding document.
-- Suzuki K Poulose
2. Optimize the comments of the patch "coresight:
Add support for trace filtering by source" due to bad
example.
-- Suzuki K Poulose
3. Correct spelling errors in the patch "coresight:
Add support for trace filtering by source".
-- Suzuki K Poulose
4. Optimize the function "coresight_blocks_source".
-- Suzuki K Poulose
5. Add { } in the function "of_coresight_parse_endpoint".
-- Suzuki K Poulose
6. Adjust the order of the patches.
-- Suzuki K Poulose
7. Adjust the alignment in "coresight-platform.c".
-- Suzuki K Poulose
Changes in V4:
1. Use "coresight_get_source(path)" in the function
"coresight_disable_path_from" instead of explicitly
passing the source.
-- Suzuki K Poulose
2. Optimize the order of the input parameters for
"_coresight_build_path".
-- Suzuki K Poulose
3. Reuse the method "coresight_block_source" in
"_coresight_build_path".
-- Suzuki K Poulose
4. Remove the unnecessary () in "coresight_build_path".
-- Suzuki K Poulose
5. Add a helper to check if a device is SOURCE.
-- Suzuki K Poulose
6. Adjust the posistion of setting "still_orphan" in
"coresight_build_path".
-- Suzuki K Poulose
Changes in V3:
1. Rename the function "coresight_source_filter" to
"coresight_block_source". And refine this function.
-- Suzuki K Poulose
2. Rename the parameters of the function
"coresight_find_out_connection" to avoid confusion.
-- Suzuki K Poulose
3. Get the source of path in "coresight_enable_path" and
"coresight_disable_path".
-- Suzuki K Poulose
4. Fix filter source device before skip the port in
"coresight_orphan_match".
-- Suzuki K Poulose
5. Make sure the device still orphan if whter is a filter
source firmware node but the filter source device is null.
-- Suzuki K Poulose
6. Walk through the entire coresight bus and fixup the
"filter_src_dev" if the source is being removed.
-- Suzuki K Poulose
7. Refine the commit description of patch#2.
-- Suzuki K Poulose
8. Fix the warning reported by kernel test robot.
-- kernel test robot.
9. Use the source device directly if the port has a
hardcoded filter in "tpda_get_element_size".
-- Suzuki K Poulose
Changes in V2:
1. Change the reference for endpoint property in dt-binding.
-- Krzysztof Kozlowski
2. Change the property name "filter_src" to "filter-src".
-- Krzysztof Kozlowski
3. Fix the errors in running 'make dt_binding_check'.
-- Rob Herring
4. Pass in the source parameter instead of path.
-- Suzuki K Poulose
5. Reset the "filter_src_dev" if the "src" csdev is being removed.
-- Suzuki K Poulose
6. Add a warning if the "filter_src_dev" is of not the
type DEV_TYPE_SOURCE.
-- Suzuki K Poulose
7. Optimize the procedure for handling all possible cases.
-- Suzuki K Poulose
Changes in V1:
1. Add a static replicator connect to a funnel to implement the
correspondence between the output ports and the input ports on
funnels.
-- Suzuki K Poulose
2. Add filter_src_dev and filter_src_dev phandle to
"coresight_connection" struct, and populate them if there is one.
-- Suzuki K Poulose
3. To look at the phandle and then fixup/remove the filter_src
device in fixup/remove connections.
-- Suzuki K Poulose
4. When TPDA reads DSB/CMB element size, it is implemented by
looking up filter src device in the connections.
-- Suzuki K Poulose
Tao Zhang (4):
dt-bindings: arm: qcom,coresight-static-replicator: Add property for
source filtering
coresight: Add a helper to check if a device is source
coresight: Add support for trace filtering by source
coresight-tpda: Optimize the function of reading element size
.../arm/arm,coresight-static-replicator.yaml | 19 ++-
drivers/hwtracing/coresight/coresight-core.c | 113 +++++++++++++++---
.../hwtracing/coresight/coresight-platform.c | 21 ++++
drivers/hwtracing/coresight/coresight-tpda.c | 13 +-
include/linux/coresight.h | 12 +-
5 files changed, 155 insertions(+), 23 deletions(-)
--
2.17.1
From: Yuanfang Zhang <quic_yuanfang(a)quicinc.com>
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.
Fixes: ebddaad09e10 ("coresight: etm4x: Add missing single-shot control API to sysfs")
Signed-off-by: Yuanfang Zhang <quic_yuanfang(a)quicinc.com>
---
Changes in V3:
1. Remove dsb(sy) after polling TRCSTATR.
2. Add isb() after polling TRCSTATR.
---
.../hwtracing/coresight/coresight-etm4x-core.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 66d44a404ad0..c6ea00bba0cc 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -531,7 +531,6 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
* As recommended by section 4.3.7 ("Synchronization when using the
* memory-mapped interface") of ARM IHI 0064D
*/
- dsb(sy);
isb();
done:
@@ -906,10 +905,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))
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] =
--
2.34.1
Hi
On 04/12/2024 10:13, yuanfang zhang wrote:
> From: Yuanfang Zhang <quic_yuanfang(a)quicinc.com>
>
> 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.
>
Thanks for the patch. Please could you add the above in a comment
in the changes below ?
Also, add Fixes tag ? (It may go all the way back to initial support)
Otherwise looks sensible to me.
Suzuki
> Signed-off-by: Yuanfang Zhang <quic_yuanfang(a)quicinc.com>
> ---
> drivers/hwtracing/coresight/coresight-etm4x-core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 66d44a404ad0..5da2c523c30a 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -906,6 +906,7 @@ static void etm4_disable_hw(void *info)
> tsb_csync();
> etm4x_relaxed_write32(csa, control, TRCPRGCTLR);
>
> + isb();
> /* wait for TRCSTATR.PMSTABLE to go to '1' */
> if (coresight_timeout(csa, TRCSTATR, TRCSTATR_PMSTABLE_BIT, 1))
> dev_err(etm_dev,
Hi Yicong
On 02/12/2024 09:24, Yicong Yang wrote:
> From: Yicong Yang <yangyicong(a)hisilicon.com>
>
> When trying to run perf and sysfs mode simultaneously, the WARN_ON()
> in tmc_etr_enable_hw() is triggered sometimes:
>
> WARNING: CPU: 42 PID: 3911571 at drivers/hwtracing/coresight/coresight-tmc-etr.c:1060 tmc_etr_enable_hw+0xc0/0xd8 [coresight_tmc]
> [..snip..]
> Call trace:
> tmc_etr_enable_hw+0xc0/0xd8 [coresight_tmc] (P)
> tmc_enable_etr_sink+0x11c/0x250 [coresight_tmc] (L)
> tmc_enable_etr_sink+0x11c/0x250 [coresight_tmc]
> coresight_enable_path+0x1c8/0x218 [coresight]
> coresight_enable_sysfs+0xa4/0x228 [coresight]
> enable_source_store+0x58/0xa8 [coresight]
> dev_attr_store+0x20/0x40
> sysfs_kf_write+0x4c/0x68
> kernfs_fop_write_iter+0x120/0x1b8
> vfs_write+0x2c8/0x388
> ksys_write+0x74/0x108
> __arm64_sys_write+0x24/0x38
> el0_svc_common.constprop.0+0x64/0x148
> do_el0_svc+0x24/0x38
> el0_svc+0x3c/0x130
> el0t_64_sync_handler+0xc8/0xd0
> el0t_64_sync+0x1ac/0x1b0
> ---[ end trace 0000000000000000 ]---
>
> Since the enablement of sysfs mode is separeted into two critical regions,
> one for sysfs buffer allocation and another for hardware enablement, it's
> possible to race with the perf mode. Fix this by double check whether
> the perf mode's been used before enabling the hardware in sysfs mode.
Thanks for the fix. Some minor comments below.
It needs a Fixes tag.
>
> Signed-off-by: Yicong Yang <yangyicong(a)hisilicon.com>
> ---
> .../hwtracing/coresight/coresight-tmc-etr.c | 30 +++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index ad83714ca4dc..d382d95da5ff 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -1230,6 +1230,36 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev)
>
> spin_lock_irqsave(&drvdata->spinlock, flags);
>
> + /*
> + * Since the sysfs buffer allocation and the hardware enablement is not
> + * in the same critical region, it's possible to race with the perf
> + * mode:
> + * [sysfs mode] [perf mode]
> + * tmc_etr_get_sysfs_buffer()
> + * spin_lock(&drvdata->spinlock)
> + * [sysfs buffer allocation]
> + * spin_unlock(&drvdata->spinlock)
> + * spin_lock(&drvdata->spinlock)
> + * tmc_etr_enable_hw()
> + * drvdata->etr_buf = etr_perf->etr_buf
> + * spin_unlock(&drvdata->spinlock)
> + * spin_lock(&drvdata->spinlock)
> + * tmc_etr_enable_hw()
> + * WARN_ON(drvdata->etr_buf) // WARN sicne etr_buf initialized at
> + * the perf side
> + * spin_unlock(&drvdata->spinlock)
> + *
> + * So check here before continue.
> + */
> + if (coresight_get_mode(csdev) == CS_MODE_PERF) {
> + drvdata->sysfs_buf = NULL;
> + spin_unlock_irqrestore(&drvdata->spinlock, flags);
> +
> + /* Free allocated memory out side of the spinlock */
> + tmc_etr_free_sysfs_buf(sysfs_buf);
> + return -EBUSY;
> + }
With this in place, I think we should remove the check for CS_MODE_PERF
in get_etr_sysfs_buf() to avoid confusion (which I believe opened up
this race)
Suzuki
> +
> /*
> * In sysFS mode we can have multiple writers per sink. Since this
> * sink is already enabled no memory is needed and the HW need not be