Hi,
On Mon, 10 Mar 2025 at 09:04, Jie Gan <quic_jiegan(a)quicinc.com> wrote:
>
> The new functions calculate and return the offset to the write pointer of
> the ETR buffer based on whether the memory mode is SG, flat or reserved.
> The functions have the RWP offset can directly read data from ETR buffer,
> enabling the transfer of data to any required location.
>
> Signed-off-by: Jie Gan <quic_jiegan(a)quicinc.com>
> ---
> .../hwtracing/coresight/coresight-tmc-etr.c | 40 +++++++++++++++++++
> drivers/hwtracing/coresight/coresight-tmc.h | 1 +
> 2 files changed, 41 insertions(+)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index eda7cdad0e2b..ec636ab1fd75 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -267,6 +267,46 @@ void tmc_free_sg_table(struct tmc_sg_table *sg_table)
> }
> EXPORT_SYMBOL_GPL(tmc_free_sg_table);
>
> +static long tmc_flat_resrv_get_rwp_offset(struct tmc_drvdata *drvdata)
> +{
> + dma_addr_t paddr = drvdata->sysfs_buf->hwaddr;
> + u64 rwp;
> +
It is not valid to read RWP if the TMC is running. It must be in the
stopped or disabled state - see the specifications for TMC /ETR
It is likely that CSUNLOCK / CSLOCK are needed here too, along with
the spinlock that protects drvdata
See the code in coresight_tmc_etr.c :-
e.g. in
tmc_update_etr_buffer()
...
<take spinlock>
...
CS_UNLOCK(drvdata->base);
tmc_flush_and_stop(drvdata); // this ensures tmc is stopped and
flushed to memory - essential to ensure full formatted frame is in
memory.
tmc_sync_etr_buf(drvdata); // this function reads rwp.
CS_LOCK(drvdata->base);
<release spinlokc>
This type of program flow is common to both sysfs and perf handling of
TMC buffers.
> + rwp = tmc_read_rwp(drvdata);
> + return rwp - paddr;
> +}
> +
> +static long tmc_sg_get_rwp_offset(struct tmc_drvdata *drvdata)
> +{
> + struct etr_buf *etr_buf = drvdata->sysfs_buf;
> + struct etr_sg_table *etr_table = etr_buf->private;
> + struct tmc_sg_table *table = etr_table->sg_table;
> + long w_offset;
> + u64 rwp;
> +
Same comments as above
> + rwp = tmc_read_rwp(drvdata);
> + w_offset = tmc_sg_get_data_page_offset(table, rwp);
> +
> + return w_offset;
> +}
> +
> +/*
> + * Retrieve the offset to the write pointer of the ETR buffer based on whether
> + * the memory mode is SG, flat or reserved.
> + */
> +long tmc_get_rwp_offset(struct tmc_drvdata *drvdata)
> +{
> + struct etr_buf *etr_buf = drvdata->sysfs_buf;
> +
As this is an exported function, please ensure that the inputs are
valid - check the pointers
Code to ensure TMC is flushed and stopped could be inserted here.
Regards
Mike
> + if (etr_buf->mode == ETR_MODE_ETR_SG)
> + return tmc_sg_get_rwp_offset(drvdata);
> + else if (etr_buf->mode == ETR_MODE_FLAT || etr_buf->mode == ETR_MODE_RESRV)
> + return tmc_flat_resrv_get_rwp_offset(drvdata);
> + else
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(tmc_get_rwp_offset);
> +
> /*
> * Alloc pages for the table. Since this will be used by the device,
> * allocate the pages closer to the device (i.e, dev_to_node(dev)
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
> index b48bc9a01cc0..baedb4dcfc3f 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.h
> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> @@ -442,5 +442,6 @@ void tmc_etr_remove_catu_ops(void);
> struct etr_buf *tmc_etr_get_buffer(struct coresight_device *csdev,
> enum cs_mode mode, void *data);
> extern const struct attribute_group coresight_etr_group;
> +long tmc_get_rwp_offset(struct tmc_drvdata *drvdata);
>
> #endif
> --
> 2.34.1
>
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
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
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 and 06 offers an extra feature for
updating buffer on AUX pause occasion, which can mitigate the trace data
lose issue.
Patch 07 documents the AUX pause usages with Arm CoreSight. The last
patch syncs headers between user space and the kernel.
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 v1:
- Added validation function pointers in pause and resume APIs (Mike).
Leo Yan (8):
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: etm: Add an attribute for updating buffer
coresight: perf: Update buffer on AUX pause
Documentation: coresight: Document AUX pause and resume
perf cs-etm: Sync kernel coresight-pmu.h header
.../trace/coresight/coresight-perf.rst | 50 ++++++
drivers/hwtracing/coresight/coresight-core.c | 20 +++
.../hwtracing/coresight/coresight-etm-perf.c | 94 +++++++++-
.../hwtracing/coresight/coresight-etm-perf.h | 2 +
.../coresight/coresight-etm4x-core.c | 166 ++++++++++++------
drivers/hwtracing/coresight/coresight-etm4x.h | 2 +
drivers/hwtracing/coresight/coresight-priv.h | 2 +
include/linux/coresight-pmu.h | 1 +
include/linux/coresight.h | 4 +
tools/include/linux/coresight-pmu.h | 1 +
10 files changed, 289 insertions(+), 53 deletions(-)
--
2.34.1
On Thu, Mar 06, 2025 at 04:39:27PM +0800, Yuanfang Zhang wrote:
[...]
> >> +static ssize_t flush_req_store(struct device *dev,
> >> + struct device_attribute *attr,
> >> + const char *buf,
> >> + size_t size)
> >> +{
...
> >> + reg = readl_relaxed(drvdata->base + TRACE_NOC_CTRL);
> >> + reg = reg | TRACE_NOC_CTRL_FLUSHREQ;
> >> + writel_relaxed(reg, drvdata->base + TRACE_NOC_CTRL);
> >
> > How can userspace determine when to trigger a flush?
> It can be triggered under any circumstances.
> >
> > Generally, a driver kicks off a flush operation for a hardware before
> > reading data from buffer or when disable a link path. I don't know the
> > hardware mechanism of TNOC, but seems to me, it does not make sense to
> > let the userspace to trigger a hardware flush, given the userspace has
> > no knowledge for device's state.
>
> TNOC supports the aforementioned flush operation, and it also adds this
> flush functionality, allowing users to set the flush themselves.
I am still not convinced for providing knobs to allow userspace to
directly control hardware.
A low level driver should have sufficient information to know when and
how it triggers a flush. E.g., CoreSight ETF (coresight-tmc-etf.c) can
act as a link, in this case, it calls the tmc_flush_and_stop() function
to flush its buffer when it is stopped. A flushing is triggered when a
session is terminated (either is a perf session or a Sysfs session).
Why not TNOC driver do the flushing same as other drivers? It can flush
the data before a hardware link is to be disabled. I don't think flush
operations are required at any time.
Seems to me, exposing APIs to userspace for flushing operations also
will introduce potential security risk. A malicious software might
attack system with triggering tons of flushing in short time.
> > Furthermore, based on my understanding for patch 02 and 03, the working
> > flow is also concerned me. IIUC, you want to use the driver to create
> > a linkage and then use userspace program to poll state and trigger
> > flushing. Could you explain why use this way for managing the device?
> >
> TNOC support flush just like other links. This interface simply provides
> customers with an additional option to trigger the flush.
This is not true for Arm CoreSight components. My understanding is Arm
CoreSight drivers never provides an API to userspace to manually trigger
flush operations.
Thanks,
Leo
On Mon, 10 Mar 2025 18:27:24 +0800, Jie Gan wrote:
> The coresight_etm_get_trace_id function is a global function. The
> verification process for 'csdev' is required prior to its usage.
>
>
Applied, thanks!
[1/1] coresight: add verification process for coresight_etm_get_trace_id
https://git.kernel.org/coresight/c/ab37128a
Best regards,
--
Suzuki K Poulose <suzuki.poulose(a)arm.com>
On Thu, 06 Mar 2025 12:11:01 +0000, Yeoreum Yun wrote:
> In some coresight drivers, drvdata->spinlock can be held during __schedule()
> by perf_event_task_sched_out()/in().
>
> Since drvdata->spinlock type is spinlock_t and
> perf_event_task_sched_out()/in() is called after acquiring rq_lock,
> which is raw_spinlock_t (an unsleepable lock),
> this poses an issue in PREEMPT_RT kernel where spinlock_t is sleepable.
>
> [...]
Applied, thanks!
[1/9] coresight: change coresight_device lock type to raw_spinlock_t
https://git.kernel.org/coresight/c/26f060c1
[2/9] coresight-etm4x: change etmv4_drvdata spinlock type to raw_spinlock_t
https://git.kernel.org/coresight/c/743c5a97
[3/9] coresight: change coresight_trace_id_map's lock type to raw_spinlock_t
https://git.kernel.org/coresight/c/4cf364ca
[4/9] coresight-cti: change cti_drvdata spinlock's type to raw_spinlock_t
https://git.kernel.org/coresight/c/e3044065
[5/9] coresight-etb10: change etb_drvdata spinlock's type to raw_spinlock_t
https://git.kernel.org/coresight/c/6b80c0ab
[6/9] coresight-funnel: change funnel_drvdata spinlock's type to raw_spinlock_t
https://git.kernel.org/coresight/c/56eb02f0
[7/9] coresight-replicator: change replicator_drvdata spinlock's type to raw_spinlock_t
https://git.kernel.org/coresight/c/982d0a0e
[8/9] coresight-tmc: change tmc_drvdata spinlock's type to raw_spinlock_t
https://git.kernel.org/coresight/c/db11f75b
[9/9] coresight/ultrasoc: change smb_drv_data spinlock's type to raw_spinlock_t
https://git.kernel.org/coresight/c/d11eb31d
Best regards,
--
Suzuki K Poulose <suzuki.poulose(a)arm.com>
On Thu, Mar 06, 2025 at 04:22:20PM +0800, Yuanfang Zhang wrote:
[...]
> >> +static int trace_noc_init_default_data(struct trace_noc_drvdata *drvdata)
> >> +{
> >> + int atid;
> >> +
> >> + atid = coresight_trace_id_get_system_id();
> >> + if (atid < 0)
> >> + return atid;
> >> +
> >> + drvdata->atid = atid;
> >> +
> >> + drvdata->freq_type = FREQ_TS;
> >
> > I don't see anywhere uses FREQ. Please remove the unused definitions
> > and related code.
>
> it is used in trace_noc_enable_hw().
I understood some macros and definitions are used by seqential patches.
A good practice is code should be added only when they are used. This
can allow every patch in neat way and easier for review.
Thanks,
Leo
> >
> >> + drvdata->flag_type = FLAG;
> >
> > FLAG_TS is not used in the driver as well. Remove it.
> it is used in trace_noc_enable_hw().
On 10/03/2025 02:23, Jie Gan wrote:
> The coresight_etm_get_trace_id function is a global function. The
> verification process for 'csdev' is required prior to its usage.
>
> Fixes: c367a89dec26 ("Coresight: Add trace_id function to retrieving the trace ID")
> Signed-off-by: Jie Gan <quic_jiegan(a)quicinc.com>
> ---
> drivers/hwtracing/coresight/coresight-core.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index bd0a7edd38c9..5a7cd2376e2d 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -1616,9 +1616,12 @@ EXPORT_SYMBOL_GPL(coresight_remove_driver);
> int coresight_etm_get_trace_id(struct coresight_device *csdev, enum cs_mode mode,
> struct coresight_device *sink)
> {
> - int trace_id;
> - int cpu = source_ops(csdev)->cpu_id(csdev);
> + int cpu, trace_id;
> +
> + if (csdev->type != CORESIGHT_DEV_TYPE_SOURCE && !source_ops(csdev)->cpu_id)
That must be :
csdev->type != CORESIGHT_DEV_TYPE_SOURCE || !source_ops(csdev)->cpu_id)
Suzuki
> + return -EINVAL;
>
> + cpu = source_ops(csdev)->cpu_id(csdev);
> switch (mode) {
> case CS_MODE_SYSFS:
> trace_id = coresight_trace_id_get_cpu_id(cpu);