CoreSight ETM4x devices could be accessed either via MMIO (handled via
amba_driver) or CPU system instructions (handled via platform driver). But
this has the following issues :
- Each new CPU comes up with its own PID and thus we need to keep on
adding the "known" PIDs to get it working with AMBA driver. While
the ETM4 architecture (and CoreSight architecture) defines way to
identify a device as ETM4. Thus older kernels won't be able to
"discover" a newer CPU, unless we add the PIDs.
- With ACPI, the ETM4x devices have the same HID to identify the device
irrespective of the mode of access. This creates a problem where two
different drivers (both AMBA based driver and platform driver) would
hook into the "HID" and could conflict. e.g., if AMBA driver gets
hold of a non-MMIO device, the probe fails. If we have single driver
hooked into the given "HID", we could handle them seamlessly,
irrespective of the mode of access.
- CoreSight is heavily dependent on the runtime power management. With
ACPI, amba_driver doesn't get us anywhere with handling the power
and thus one need to always turn the power ON to use them. Moving to
platform driver gives us the power management for free.
Due to all of the above, we are moving the MMIO based etm4x devices to be
supported via platform driver. The series makes the existing platform
driver generic to handle both type of the access modes. Although existing
AMBA driver would still continue to support DT based etm4x MMIO devices.
The series applies on 6.3-rc4.
Changes in V2:
- Enables ACPI etm4x device support in the existing platform driver
- Dropped last two patches from the series
- Dropped redundant 'devarch' checking from is_etm4x_device()
- Renamed updated is_etm4x_device() as is_etm4x_devtype()
- Fixed arguments in fallback stub for etm4_check_arch_features()
- Tagged etm4_dev_pm_ops with etm4_platform_driver
- Updated the comment for coresight_get_enable_apb_pclk() helper
- Updated the comment for new 'pclk' element in struct etm4_drvdata
- Dropped the clock when devm_ioremap_resource() fails
- Convert IS_ERR() into a direct pointer check in etm4_remove_platform_dev()
- Dropped "arm,coresight-etm4x" compatible property from etm4_match[]
Changes in V1:
https://lore.kernel.org/all/20230317030501.1811905-1-anshuman.khandual@arm.…
Cc: Steve Clevenger <scclevenger(a)os.amperecomputing.com>
Cc: Rob Herring <robh+dt(a)kernel.org>
Cc: Frank Rowand <frowand.list(a)gmail.com>
Cc: Russell King (Oracle) <linux(a)armlinux.org.uk>
Cc: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael(a)kernel.org>
Cc: Len Brown <lenb(a)kernel.org>
Cc: Sudeep Holla <sudeep.holla(a)arm.com>
Cc: Lorenzo Pieralisi <lpieralisi(a)kernel.org>
Cc: Mathieu Poirier <mathieu.poirier(a)linaro.org>
Cc: Suzuki K Poulose <suzuki.poulose(a)arm.com>
Cc: Mike Leach <mike.leach(a)linaro.org>
Cc: Leo Yan <leo.yan(a)linaro.org>
Cc: devicetree(a)vger.kernel.org
Cc: linux-acpi(a)vger.kernel.org
Cc: coresight(a)lists.linaro.org
Cc: linux-arm-kernel(a)lists.infradead.org
Cc: linux-kernel(a)vger.kernel.org
Anshuman Khandual (4):
coresight: etm4x: Allocate and device assign 'struct etmv4_drvdata' earlier
coresight: etm4x: Drop iomem 'base' argument from etm4_probe()
coresight: etm4x: Drop pid argument from etm4_probe()
coresight: etm4x: Change etm4_platform_driver driver for MMIO devices
Suzuki Poulose (1):
coresight: etm4x: Add ACPI support in platform driver
drivers/acpi/acpi_amba.c | 1 -
.../coresight/coresight-etm4x-core.c | 114 ++++++++++++++----
drivers/hwtracing/coresight/coresight-etm4x.h | 4 +
include/linux/coresight.h | 59 +++++++++
4 files changed, 154 insertions(+), 24 deletions(-)
--
2.25.1
Hi Greg
Please find a couple of fixes for v6.4 for the coresight self-hosted tracing
subsystem.
Kindly pull.
Suzuki
The following changes since commit ac9a78681b921877518763ba0e89202254349d1b:
Linux 6.4-rc1 (2023-05-07 13:34:35 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git tags/coresight-fixes-v6.4
for you to fetch changes up to 04ac7f98b92181179ea84439642493f3826d04a2:
coresight: perf: Release Coresight path when alloc trace id failed (2023-05-11 11:18:21 +0100)
----------------------------------------------------------------
coresight: Fixes for v6.4
Couple of fixes for coresight self-hosted tracing subsystem for v6.4.
These include :
- Fix signedness bug in tmc_etr_buf_insert_barrier_packet()
- Fix memory leak with failing to release the path if trace-id allocation fails
Signed-off-by: Suzuki K Poulose <suzuki.poulose(a)arm.com>
----------------------------------------------------------------
Dan Carpenter (1):
coresight: Fix signedness bug in tmc_etr_buf_insert_barrier_packet()
Ruidong Tian (1):
coresight: perf: Release Coresight path when alloc trace id failed
drivers/hwtracing/coresight/coresight-etm-perf.c | 1 +
drivers/hwtracing/coresight/coresight-tmc-etr.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
Pre 5.11 kernels don't support 'contextid1' and 'contextid2' so
validation would be skipped. By adding an additional check for
'contextid', old kernels will still have validation done even though
contextid would either be contextid1 or contextid2.
Additionally now that it's possible to override options, an existing bug
in the validation is revealed. 'val' is overwritten by the contextid1
validation, and re-used for contextid2 validation causing it to always
fail. '!val || val != 0x4' is the same as 'val != 0x4' because 0 is also
!= 4, so that expression can be simplified and the temp variable not
overwritten.
Fixes: 35c51f83dd1e ("perf cs-etm: Validate options after applying them")
Reviewed-by: Leo Yan <leo.yan(a)linaro.org>
Link: https://lore.kernel.org/all/20230501073452.GA4660@leoy-yangtze.lan
Signed-off-by: James Clark <james.clark(a)arm.com>
---
tools/perf/arch/arm/util/cs-etm.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index 77cb03e6ff87..9ca040bfb1aa 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -78,9 +78,9 @@ static int cs_etm_validate_context_id(struct auxtrace_record *itr,
char path[PATH_MAX];
int err;
u32 val;
- u64 contextid =
- evsel->core.attr.config &
- (perf_pmu__format_bits(&cs_etm_pmu->format, "contextid1") |
+ u64 contextid = evsel->core.attr.config &
+ (perf_pmu__format_bits(&cs_etm_pmu->format, "contextid") |
+ perf_pmu__format_bits(&cs_etm_pmu->format, "contextid1") |
perf_pmu__format_bits(&cs_etm_pmu->format, "contextid2"));
if (!contextid)
@@ -114,8 +114,7 @@ static int cs_etm_validate_context_id(struct auxtrace_record *itr,
* 0b00100 Maximum of 32-bit Context ID size.
* All other values are reserved.
*/
- val = BMVAL(val, 5, 9);
- if (!val || val != 0x4) {
+ if (BMVAL(val, 5, 9) != 0x4) {
pr_err("%s: CONTEXTIDR_EL1 isn't supported, disable with %s/contextid1=0/\n",
CORESIGHT_ETM_PMU_NAME, CORESIGHT_ETM_PMU_NAME);
return -EINVAL;
--
2.34.1
Hi Ian,
On Mon, May 08, 2023 at 02:31:28PM -0700, Ian Rogers wrote:
> Change "../cs-etm.h" to just "cs-etm.h" as ../cs-etm.h doesn't exist.
>
> Signed-off-by: Ian Rogers <irogers(a)google.com>
> ---
> tools/perf/arch/arm/util/pmu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/arch/arm/util/pmu.c b/tools/perf/arch/arm/util/pmu.c
> index 860a8b42b4b5..46154c226fbb 100644
> --- a/tools/perf/arch/arm/util/pmu.c
> +++ b/tools/perf/arch/arm/util/pmu.c
> @@ -12,7 +12,7 @@
> #include "arm-spe.h"
> #include "hisi-ptt.h"
> #include "../../../util/pmu.h"
> -#include "../cs-etm.h"
This code is a bit tricky. Here it deliberately uses "../cs-etm.h" to
avoid including the header file in the same folder, alternatively, the
compiler option "-I/home/leoy/Work/linux/tools/perf/util/include" will
lead the C file to include "tools/perf/util/include/../cs-etm.h".
> +#include "cs-etm.h"
So with this change, perf will fail to build actually :)
We need to change to:
#include "../../../util/cs-etm.h"
With above change:
Reviewed-by: Leo Yan <leo.yan(a)linaro.org>
Thanks for code improvement!
> struct perf_event_attr
> *perf_pmu__get_default_config(struct perf_pmu *pmu __maybe_unused)
> --
> 2.40.1.521.gf1e218fcd8-goog
>
The first commit contains a fix for a recently introduced regression,
but was always a shortcoming in the Coresight code anyway.
The following commits are a tidyup in preparation for the last commit,
which is a fairly major change to the decode logic that's also
indirectly related to the regression so I thought it would be good time
to fix that now.
Applies to perf/core (9be6ab181b7b)
James Clark (7):
perf: cs-etm: Fix timeless decode mode detection
perf tools: Add util function for overriding user set config values
perf: cs-etm: Don't test full_auxtrace because it's always set
perf: cs-etm: Validate options after applying them
perf: cs-etm: Allow user to override timestamp and contextid settings
perf: cs-etm: Use bool type for boolean values
perf: cs-etm: Add separate decode paths for timeless and per-thread
modes
tools/perf/arch/arm/util/cs-etm.c | 223 +++++++++---------
tools/perf/arch/arm/util/pmu.c | 2 +
tools/perf/arch/arm64/util/arm-spe.c | 26 +-
tools/perf/arch/x86/util/intel-pt.c | 22 +-
tools/perf/tests/shell/test_arm_coresight.sh | 24 ++
.../perf/util/cs-etm-decoder/cs-etm-decoder.h | 8 +-
tools/perf/util/cs-etm.c | 200 +++++++++++-----
tools/perf/util/cs-etm.h | 6 +-
tools/perf/util/evsel.c | 29 +++
tools/perf/util/evsel.h | 3 +
10 files changed, 325 insertions(+), 218 deletions(-)
--
2.34.1
Changes since v4:
* Update commit message on patch 1
* Drop previous change that added lockdep checks to coresight_add_helper()
because they are already in mutex_lock(). But keep extra comment.
* Check for allocation failure in coresight_add_out_conn()
------------------
Changes since v3:
* Put connection loss fix at the beginning so that it can be backported
* Replace coresight_find_link_{x}() with coresight_find_out_connection()
* Reorder coresight_enable_source() arguments for consistency
* Add source and destination reference counts so that two link devices
connected together don't clash
* Add coresight_is_helper()
* Fix overwriting csdev bug in coresight_orphan_match()
* Don't clear conns[i]->dest_fwnode in coresight_remove_conns() in case
it's used again
* Use dev instead of adev->dev for devmem allocation in
acpi_coresight_parse_graph() so that it's consistent with DT mode and
doesn't cause a warning on free.
* Rename coresight_add_helper_mutex() -> coresight_add_helper()
* Ensure coresight_mutex isn't already held in coresight_add_helper()
* Return new connection from coresight_add_out_conn()
* Comment and formatting improvements
------------------
Changes since v2:
* Make out_conns array contiguous instead of sparse which simplifies
filling and using it. New connections are always added to the end
* Store pointers to individual connection objects so that they can be
shared between inputs and outputs
* Fix an existing bug where connection info was lost when unloading a
device
* Simplify connection fixup functions. Now the orphan mechanism is used
for inputs in the same way as outputs to guarantee that all
connections have both an input and an output set
* Use input connections to disconnect devices on unload instead of
iterating through them all
* Make refcount a property of the connection rather than use it's own
array based on the number of inputs and outputs
* Fix a bug in v2 where helpers attached to the source device weren't
disabled because coresight-etm-perf.c was making a raw call to
disable rather than using a helper.
* Change names of connection members to make direction explicit now
that the connection is shared between input and outputs
------------------
Changes since v1:
* Don't dereference handle in tmc_etr_get_buffer() when not in perf mode.
* Fix some W=1 warnings
* Add a commit to rename child/output in terms of local/remote
-------------------
Currently there is a refcount leak in CTI when using system wide mode
or tracing multithreaded applications. See the last commit for a
reproducer. This prevents the module from being unloaded.
Historically there have been a few issues and fixes attempted around
here which have resulted in some extra logic and a member to keep
track of CTI being enabled 'struct coresight_device->ect_enabled'.
The fix in commit 665c157e0204 ("coresight: cti: Fix hang in
cti_disable_hw()") was also related to CTI having its own
enable/disable path which came later than other devices.
If we make CTI a helper device and enable helper devices adjacent to
the path we get very similar enable/disable behavior to now, but with
more reuse of the existing reference counting logic in the coresight
core code. This also affects CATU which can have a little bit of
its hard coded enable/disable code removed.
Enabling CATU on the generic path does require that input connections
are tracked so that it can get its associated ETR buffer.
Applies to coresight/next (197b6b60ae7b) but also requires the
realloc_array patch here [1].
Also available in full here [2].
[1]: https://lore.kernel.org/linux-arm-kernel/20230306152723.3090195-1-james.cla…
[2]: https://gitlab.arm.com/linux-arm/linux-jc/-/tree/james-cs-cti-module-refcou…
James Clark (13):
coresight: Fix loss of connection info when a module is unloaded
coresight: Use enum type for cs_mode wherever possible
coresight: Change name of pdata->conns
coresight: Rename nr_outports to nr_outconns
coresight: Rename connection members to make the direction explicit
coresight: Dynamically add connections
coresight: Store pointers to connections rather than an array of them
coresight: Simplify connection fixup mechanism
coresight: Store in-connections as well as out-connections
coresight: Make refcount a property of the connection
coresight: Refactor out buffer allocation function for ETR
coresight: Enable and disable helper devices adjacent to the path
coresight: Fix CTI module refcount leak by making it a helper device
drivers/hwtracing/coresight/coresight-catu.c | 21 +-
drivers/hwtracing/coresight/coresight-core.c | 593 ++++++++++--------
.../hwtracing/coresight/coresight-cti-core.c | 52 +-
.../hwtracing/coresight/coresight-cti-sysfs.c | 4 +-
drivers/hwtracing/coresight/coresight-cti.h | 4 +-
drivers/hwtracing/coresight/coresight-etb10.c | 13 +-
.../hwtracing/coresight/coresight-etm-perf.c | 4 +-
.../coresight/coresight-etm3x-core.c | 6 +-
.../coresight/coresight-etm4x-core.c | 6 +-
.../hwtracing/coresight/coresight-funnel.c | 26 +-
.../hwtracing/coresight/coresight-platform.c | 269 ++++----
drivers/hwtracing/coresight/coresight-priv.h | 17 +-
.../coresight/coresight-replicator.c | 23 +-
drivers/hwtracing/coresight/coresight-stm.c | 6 +-
drivers/hwtracing/coresight/coresight-sysfs.c | 17 +-
.../hwtracing/coresight/coresight-tmc-etf.c | 26 +-
.../hwtracing/coresight/coresight-tmc-etr.c | 110 ++--
drivers/hwtracing/coresight/coresight-tmc.h | 2 +
drivers/hwtracing/coresight/coresight-tpda.c | 23 +-
drivers/hwtracing/coresight/coresight-tpdm.c | 4 +-
drivers/hwtracing/coresight/coresight-tpiu.c | 7 +-
drivers/hwtracing/coresight/coresight-trbe.c | 3 +-
drivers/hwtracing/coresight/ultrasoc-smb.c | 11 +-
drivers/hwtracing/coresight/ultrasoc-smb.h | 2 +-
include/linux/coresight.h | 127 ++--
25 files changed, 708 insertions(+), 668 deletions(-)
--
2.34.1
Em Mon, Apr 24, 2023 at 06:36:14PM +0300, Adrian Hunter escreveu:
> On 24/04/23 16:47, James Clark wrote:
> > There is some duplicated code to only override config values if they
> > haven't already been set by the user so make a util function for this.
> >
> > Signed-off-by: James Clark <james.clark(a)arm.com>
>
> One minor comment, nevertheless:
>
> Acked-by: Adrian Hunter <adrian.hunter(a)intel.com>
I just moved to evsel__set_config_if_unset() to util/pmu.c, next to
some other evsel__ functions to not break the python.so binding, before
I was getting:
[acme@quaco perf-tools-next]$ perf test -v python
Couldn't bump rlimit(MEMLOCK), failures may take place when creating BPF maps, etc
19: 'import perf' in python :
--- start ---
test child forked, pid 500086
python usage test: "echo "import sys ; sys.path.append('/tmp/build/perf-tools-next/python'); import perf" | '/usr/bin/python3' "
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ImportError: /tmp/build/perf-tools-next/python/perf.cpython-311-x86_64-linux-gnu.so: undefined symbol: perf_pmu__format_bits
test child finished with -1
---- end ----
'import perf' in python: FAILED!
[acme@quaco perf-tools-next]$
Please run 'perf test' and 'make -C tools/perf build-test' prior to
sending pull requests,
Thanks, applied.
- Arnaldo