Good morning,
Is tracing a multi-threaded program a supported use case for perf cs-etm?
If yes, are there any flags that should be specified with perf?
Thanks,
Andrea
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
This patch series adds support for thread stack and callchain; this patch
set depends on the instruction sample fix patch set [1].
This patch set get more complex, so before divide into small groups, I'd
like to use this patch set version to include all relevant patches, hope
this can give whole context for related code change.
Briefly, this patch can be divided into three parts, which also can be
reviewed separately for every part:
Patches 01, 02 are used to fix samples for one corner case is for
accessing the branch's target address and trigger an exception.
Essentially, an extra branch sample is added to reflect this
mediate branch between the previous branch and exception entry.
Patches 03, 04, 05, 06 are coming from patch v4, which are used to
support thread stack and callchain.
Patches 07, 08, 09 are used to fixup for exception entry and exit. This
is mainly used to fix two cases, one part is to fixup the thread stack
and callchain for the case when access branch target address and trigger
exception; another part is to fixup the thread stack for instruction
emulation (and other single step cases).
This patch set has been tested on Juno-r2 after applied on perf/core
branch with latest commit 85fc95d75970 ("perf maps: Add missing unlock
to maps__insert() error case"), and this patch set is also applied on
top of instruction sample fix patch set [1].
Test for option '-F,+callindent':
# perf script -F,+callindent
main 3258 1 branches: main ffffad684d20 __libc_start_main+0xe0 (/usr/lib/aarch64-linux-gnu/libc-2.28.so)
main 3258 1 branches: lib_loop_test@plt aaaae2c4d78c main+0x18 (/root/coresight_test/main)
main 3258 1 branches: _dl_fixup ffffad811b4c _dl_runtime_resolve+0x40 (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
main 3258 1 branches: _dl_lookup_symbol_x ffffad80c078 _dl_fixup+0xb8 (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
main 3258 1 branches: do_lookup_x ffffad80849c _dl_lookup_symbol_x+0x104 (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
main 3258 1 branches: check_match ffffad807bf0 do_lookup_x+0x238 (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
main 3258 1 branches: strcmp ffffad807888 check_match+0x70 (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
main 3258 1 branches: lib_loop_test@plt aaaae2c4d78c main+0x18 (/root/coresight_test/main)
main 3258 1 branches: lib_loop_test@plt aaaae2c4d78c main+0x18 (/root/coresight_test/main)
main 3258 1 branches: lib_loop_test@plt aaaae2c4d78c main+0x18 (/root/coresight_test/main)
main 3258 1 branches: lib_loop_test@plt aaaae2c4d78c main+0x18 (/root/coresight_test/main)
[...]
Test for option '--itrace=g':
# perf script --itrace=g16l64i100
main 3258 100 instructions:
ffffad816a80 memcpy+0x70 (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
ffffad809468 _dl_new_object+0xa8 (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
ffffad801840 dl_main+0x778 (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
ffffad81384c _dl_sysdep_start+0x36c (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
ffffad800884 _dl_start_final+0xac (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
ffffad800b00 _dl_start+0x200 (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
ffffad800048 _start+0x8 (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
main 3258 100 instructions:
ffffad80952c _dl_new_object+0x16c (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
ffffad801840 dl_main+0x778 (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
ffffad81384c _dl_sysdep_start+0x36c (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
ffffad800884 _dl_start_final+0xac (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
ffffad800b00 _dl_start+0x200 (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
ffffad800048 _start+0x8 (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
main 3258 100 instructions:
ffffad8018dc dl_main+0x814 (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
ffffad81384c _dl_sysdep_start+0x36c (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
ffffad800884 _dl_start_final+0xac (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
ffffad800b00 _dl_start+0x200 (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
ffffad800048 _start+0x8 (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
main 3258 100 instructions:
ffff8000100878d0 el0_sync_handler+0x168 ([kernel.kallsyms])
ffff800010082d00 el0_sync+0x140 ([kernel.kallsyms])
ffffad801910 dl_main+0x848 (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
ffffad81384c _dl_sysdep_start+0x36c (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
ffffad800884 _dl_start_final+0xac (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
ffffad800b00 _dl_start+0x200 (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
ffffad800048 _start+0x8 (/usr/lib/aarch64-linux-gnu/ld-2.28.so)
[...]
Changes from v4:
* Addressed Mike's suggestion for performance improvement for function
cs_etm__instr_addr() for quick calculation for non T32;
* Removed the patch 'perf cs-etm: Synchronize instruction sample with
the thread stack' (Mike);
* Fixed the issue for exception is taken for branch target address
accessing, for the branch sample and stack thread handling, the
related patches are 01, 02, 07;
* Fixed the stack thread handling for instruction emulation and single
step with patches 08, 09.
Changes from v3:
* Split out separate patch set for instruction samples fixing.
* Rebased on latest perf/core branch.
Changes from v2:
* Added patch 01 to fix the unsigned variable comparison to zero
(Suzuki).
* Refined commit logs.
Changes from v1:
* Added comments for task thread handling (Mathieu).
* Split patch 02 into two patches, one is for support thread stack and
another is for callchain support (Mathieu).
* Added a new patch to support branch filter.
[1] https://lkml.org/lkml/2020/2/18/1406
Leo Yan (9):
perf cs-etm: Defer to assign exception sample flag
perf cs-etm: Reflect branch prior to exception
perf cs-etm: Refactor instruction size handling
perf cs-etm: Support thread stack
perf cs-etm: Support branch filter
perf cs-etm: Support callchain for instruction sample
perf cs-etm: Fixup exception entry for thread stack
perf thread: Add helper to get top return address
perf cs-etm: Fixup exception exit for thread stack
.../perf/util/cs-etm-decoder/cs-etm-decoder.c | 1 +
tools/perf/util/cs-etm.c | 290 ++++++++++++++++--
tools/perf/util/thread-stack.c | 10 +
tools/perf/util/thread-stack.h | 1 +
4 files changed, 268 insertions(+), 34 deletions(-)
--
2.17.1
Hi, this is an incomplete patch for an issue with EL2 kernels, and I'm looking
for feedback on how to complete it.
The background is that to support tracing multiple address spaces we get ETM to
embed the context id in the trace, and we build with CONFIG_PID_IN_CONTEXTIDR
to get the scheduler to put the thread id in CONTEXTIDR_EL1. This is a known
technique, it's what context id tracing is designed for.
The problem is when the kernel is running not at EL1 (OS level) but at EL2
(hypervisor level), which is now becoming common. With HCR_EL2.E2H set,
the kernel's writes to CONTEXTIDR_EL1 actually change a different physical
register, CONTEXTIDR_EL2. However, ETM still traces CONTEXTIDR_EL1.
So the context ids in the trace are zero, and trace cannot be reconstructed.
ETM 4.1 has an option VMIDOPT to cause CONTEXTIDR_EL2 to be output in trace,
in the VMID field replacing the value of VTTBR.VMID. So we can use that, but the
trace follower, collecting events from OpenCSD, needs to be aware it needs to
check the VMID field not the CID field. OpenCSD doesn't need to change but
perf does. TRCCONFIGR is already in the metadata, so perf consumers can check
it to see what's going on.
The patch below does the kernel and userspace side but is not complete.
The problem is that userspace perf creates the metadata copy of TRCCONFIGR
based on its request (and fills in the other id registers by reading sysfs),
but the detection of EL2/E2H happens in the kernel which adjusts TRCCONFIGR,
and it's this config which is needed for decode. I see three ways round this:
- have userspace test to see if the kernel is EL2 (somehow) and adjust the
metadata to mirror what the kernel is doing
- have the kernel pass the adjusted TRCCONFIGR back so perf can put it in the
metadata
- have the perf decoder get the thread id from whichever of VMID and
CONTEXTID is available in a PE_CONTEXT element
Obviously, the last is simplest, but it's a bodge, and means that OpenCSD
will see VMIDs when its TRCCONFIGR says it won't. It's kind of cleanest to get
the real TRCCONFIGR somehow, but how do we do that?
Al
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index a128b5063f46..96488a0cfdcf 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -353,8 +353,32 @@ static int etm4_parse_event_config(struct etmv4_drvdata *drvdata,
}
if (attr->config & BIT(ETM_OPT_CTXTID))
- /* bit[6], Context ID tracing bit */
- config->cfg |= BIT(ETM4_CFG_BIT_CTXTID);
+ {
+ /*
+ * Enable context-id tracing. The assumption is that this
+ * will work with CONFIG_PID_IN_CONTEXTIDR to trace process
+ * id changes and support decode of multiple processes.
+ * But ETM's context id trace traces physical CONTEXTIDR_EL1,
+ * while the logical CONTEXTIDR_EL1 that is written to on
+ * process switch is either physical CONTEXTIDR_EL1 or
+ * CONTEXTIDR_EL2 depending on HCR_EL2.E2H. On principle
+ * we should continue to use logical CONTEXTIDR_EL1.
+ * In order to trace physical CONTEXTIDR_EL2, we need to
+ * enable VMID tracing and use the VMIDOPT flag to trace
+ * CONTEXTIDR_EL2 rather than VTTBR.VMID in the VMID field.
+ * Trace decoders will need to inspect TRCCONFIGR and use
+ * either the CID or the VMID field from the trace packet.
+ */
+ if (!(is_kernel_in_hyp_mode() &&
+ (read_sysreg(hcr_el2) & BIT(34)) != 0)) {
+ /* bit[6], Context ID tracing bit */
+ config->cfg |= BIT(ETM4_CFG_BIT_CTXTID);
+ } else {
+ /* bits[7,15], trace CONTEXTID_EL2 in VMID field */
+ config->cfg |= (BIT(ETM4_CFG_BIT_VMID) |
+ BIT(ETM4_CFG_BIT_VMIDOPT));
+ }
+ }
/* return stack - enable if selected and supported */
if ((attr->config & BIT(ETM_OPT_RETSTK)) && drvdata->retstack)
diff --git a/include/linux/coresight-pmu.h b/include/linux/coresight-pmu.h
index b0e35eec6499..c2f47b25daab 100644
--- a/include/linux/coresight-pmu.h
+++ b/include/linux/coresight-pmu.h
@@ -19,8 +19,10 @@
/* ETMv4 CONFIGR programming bits for the ETM OPTs */
#define ETM4_CFG_BIT_CYCACC 4
#define ETM4_CFG_BIT_CTXTID 6
+#define ETM4_CFG_BIT_VMID 7
#define ETM4_CFG_BIT_TS 11
#define ETM4_CFG_BIT_RETSTK 12
+#define ETM4_CFG_BIT_VMIDOPT 15
static inline int coresight_get_trace_id(int cpu)
{
diff --git a/tools/include/linux/coresight-pmu.h b/tools/include/linux/coresight-pmu.h
index b0e35eec6499..c2f47b25daab 100644
--- a/tools/include/linux/coresight-pmu.h
+++ b/tools/include/linux/coresight-pmu.h
@@ -19,8 +19,10 @@
/* ETMv4 CONFIGR programming bits for the ETM OPTs */
#define ETM4_CFG_BIT_CYCACC 4
#define ETM4_CFG_BIT_CTXTID 6
+#define ETM4_CFG_BIT_VMID 7
#define ETM4_CFG_BIT_TS 11
#define ETM4_CFG_BIT_RETSTK 12
+#define ETM4_CFG_BIT_VMIDOPT 15
static inline int coresight_get_trace_id(int cpu)
{
diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
index cd92a99eb89d..a54cad778841 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
@@ -35,6 +35,7 @@ struct cs_etm_decoder {
dcd_tree_handle_t dcd_tree;
cs_etm_mem_cb_type mem_access;
ocsd_datapath_resp_t prev_return;
+ uint32 thread_id_in_vmid:1;
};
static u32
@@ -496,17 +497,24 @@ cs_etm_decoder__buffer_exception_ret(struct cs_etm_packet_queue *queue,
static ocsd_datapath_resp_t
cs_etm_decoder__set_tid(struct cs_etm_queue *etmq,
+ struct cs_etm_decoder *decoder,
struct cs_etm_packet_queue *packet_queue,
const ocsd_generic_trace_elem *elem,
const uint8_t trace_chan_id)
{
pid_t tid;
- /* Ignore PE_CONTEXT packets that don't have a valid contextID */
- if (!elem->context.ctxt_id_valid)
- return OCSD_RESP_CONT;
+ if (!decoder->thread_id_in_vmid) {
+ /* Ignore PE_CONTEXT packets that don't have a valid contextID */
+ if (!elem->context.ctxt_id_valid)
+ return OCSD_RESP_CONT;
+ tid = elem->context.context_id;
+ } else {
+ if (!elem->context.vmid_valid)
+ return OCSD_RESP_CONT;
+ tid = elem->context.vmid;
+ }
- tid = elem->context.context_id;
if (cs_etm__etmq_set_tid(etmq, tid, trace_chan_id))
return OCSD_RESP_FATAL_SYS_ERR;
@@ -561,7 +569,7 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer(
trace_chan_id);
break;
case OCSD_GEN_TRC_ELEM_PE_CONTEXT:
- resp = cs_etm_decoder__set_tid(etmq, packet_queue,
+ resp = cs_etm_decoder__set_tid(etmq, decoder, packet_queue,
elem, trace_chan_id);
break;
case OCSD_GEN_TRC_ELEM_ADDR_NACC:
@@ -595,11 +603,15 @@ static int cs_etm_decoder__create_etm_packet_decoder(
OCSD_BUILTIN_DCD_ETMV3 :
OCSD_BUILTIN_DCD_PTM;
trace_config = &config_etmv3;
+ decoder->thread_id_in_vmid = 0;
break;
case CS_ETM_PROTO_ETMV4i:
cs_etm_decoder__gen_etmv4_config(t_params, &trace_config_etmv4);
decoder_name = OCSD_BUILTIN_DCD_ETMV4I;
trace_config = &trace_config_etmv4;
+ /* If VMID and VMIDOPT are set, thread id is in VMID not CID */
+ decoder->thread_id_in_vmid =
+ ((trace_config_etmv4.reg.configr & 0x8080) == 0x8080);
break;
default:
return -1;
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
This patchset introduces initial concepts in CoreSight complex configuration
support - the device "feature", which is a method of programming the device
to perform a specific function.
A built-in feature is provided - the ETM strobing function, which programs the
ETM to switch trace on and off in a specific mark / space ratio to effectively
sample the program being traced. This feature is essential for the Auto-FDO
flow using CoreSight trace.
Features are declared as a data table, a set of register, resource and parameter
requirements. A feature can then be loaded into a device, when the requirements
are validated. Once loaded a feature can be enabled for a specific trace run.
A feature appears in the sysfs file for the device, as a directory of form
'name.feat', with parameters 'enable', 'description' and any input parameters
that may be used to control the operation.
For example the ETM strobing feature provided has parameters of 'window' and
'period' to control the sampling mark / space ratio. The representation in
sysfs for the ETMv4 is therefore:-
etmX/strobing.feat/
/enable
/window
/period
Future developments will introduce resource management, and allow for the
runtime loading of additional features, and the setting of features across
an entire CoreSight system.
Mike Leach (3):
coresight: etmv4: Fix resource selector constant.
coresight: etmv4: Counter values not saved on disable.
coresight: etmv4: Adds initial complex config with ETM4 strobe
feature.
drivers/hwtracing/coresight/Makefile | 7 +-
.../hwtracing/coresight/coresight-config.c | 380 ++++++++++++++++++
.../hwtracing/coresight/coresight-config.h | 156 +++++++
.../hwtracing/coresight/coresight-etm4x-cfg.c | 325 +++++++++++++++
.../hwtracing/coresight/coresight-etm4x-cfg.h | 29 ++
.../coresight/coresight-etm4x-sysfs.c | 3 +
drivers/hwtracing/coresight/coresight-etm4x.c | 24 +-
drivers/hwtracing/coresight/coresight-etm4x.h | 4 +-
drivers/hwtracing/coresight/coresight.c | 1 +
include/linux/coresight.h | 2 +
10 files changed, 925 insertions(+), 6 deletions(-)
create mode 100644 drivers/hwtracing/coresight/coresight-config.c
create mode 100644 drivers/hwtracing/coresight/coresight-config.h
create mode 100644 drivers/hwtracing/coresight/coresight-etm4x-cfg.c
create mode 100644 drivers/hwtracing/coresight/coresight-etm4x-cfg.h
--
2.17.1
Dears,
I am beginner of using opencsd-perf to study the coresight.
My testing platform is Qcom smart phone (SDM845).
I've successful build the opencsed lib and perf-opencsd (both are master
branch)
https://github.com/Linaro/OpenCSDhttps://github.com/Linaro/perf-opencsd
I can find some devices name like these, ( when executing `ls
/sys/bus/coresight/devices/`)
..., coresight-stm, coresight-tmc-etf, coresight-tmc-etr,
coresight-ssc-etm0, ...
However, when I using the commands, I got the error.
1. perfcsd record -e cs_etm/coresight-tmc-etf/ --per-thread ls
==> event syntax error: 'cs_etm/coresight-tmc-etf/' \___ unknown term
2. perfcsd record -e cs_etm/@coresight-tmc-etf/ --per-thread ls
==> event syntax error: 'cs_etm/coresight-tmc-etf/' \___ parser error
I've tried any solutions as many as possible, but I still cannot solve this
problem.
Does perf-opencsd support the android?
Can you help me?
Thank you very much.
Eric
Hi David,
I am CC'ing the coresight mailing list as it is the best way to get
coresight related support on various issues. I have also included Sai
who has worked on coresight on the dragon board 845.
Regarding autoFDO, Mike Leach recently posted a patchset [1] that
automatically configures autoFDO on a platform. You can either work
with that patchset or use a scripted solution that Al Grant and Robert
Walker have been using.
Thanks,
Mathieu
[1]. https://lists.linaro.org/pipermail/coresight/2020-June/004078.html
---------- Forwarded message ---------
From: David Cunado <David.Cunado(a)arm.com>
Date: Mon, 29 Jun 2020 at 05:14
Subject: Coresight - help
To: Mathieu Poirier <mathieu.poirier(a)linaro.org>, Dave Rodgman
<dave.rodgman(a)arm.com>
Cc: bill.fletcher(a)linaro.org <bill.fletcher(a)linaro.org>
Hi Mathieu,
James King passed me your details as you have had expensive experience
with Coresight support in many platforms.
One of our OSS teams here at Arm is working with Google to try to get
Auto FDO working on DragonBoard 845 – do you know whether Coresight is
‘usable’ on this board and if so, if there are any tips on getting it
to work?
I’ve CC’ed Dave Rodgman, the Tech Lead for the team, who can provide
additional technical details.
Thanks!
David
david cunado | open source software group | Arm
tel: +44 1223 404112 | mob: +44 7827 230 654
IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended
recipient, please notify the sender immediately and do not disclose
the contents to any other person, use it for any purpose, or store or
copy the information in any medium. Thank you.
This patchset provides an infrastructure to allow for the automatic
selection of a sink during CoreSight tracing operations.
Currently starting tracing using perf requires a sink selection on the
command line:-
sudo ./perf record -e cs_etm/@tmc_etr0/ --per-thread uname -a
After this set (and the follow-up perf change set) the infrastructure will
be able to select a default sink:-
sudo ./perf record -e cs_etm// --per-thread uname -a
This matches with the default operation provided with perf and intelpt.
Where no sink is specified at the start of a trace session, the CoreSight
system will walk the connection graph from the source ETM, to find a
suitable sink using the first encountered highest priority device.
The CoreSight infrastructure is updated to define sink sub_types to
differentiate between sinks with built in buffers (ETB / ETF) - BUFFER
type, and those that use system memory (ETR) - SYSMEM - types.
SYSMEM types are considered higher priority.
When two sinks are found of equal priority, then the closest sink to the
source in terms of connection nodes is chosen.
The automatic sink selection will also operate if an ETM is enabled using
sysfs commands, and no sink is currently enabled. A last_sink attribute is
added to trace sources that is set to the value of the sink used when a
source is enabled via sysfs. This is set in both default and user enabled
sink scenarios.
Applies to Linux 5.8-rc1
Tested on Dragonboard DB410c.
Changes since v4:
1) Added reviewed-by etc that were missing from previous sets.
2) Added last_sink attribute to source devices.
3) Added documentation patch to update docs for default sinks.
4) Moved comment fix patch into separate misc fixes set.
Mike Leach (5):
coresight: Add default sink selection to CoreSight base.
coresight: tmc: Update sink types for default selection.
coresight: etm: perf: Add default sink selection to etm perf.
coresight: sysfs: Allow select default sink on source enable.
documentation: coresight: Update CoreSight document for default sink.
Documentation/trace/coresight/coresight.rst | 48 ++--
.../hwtracing/coresight/coresight-etm-perf.c | 17 +-
drivers/hwtracing/coresight/coresight-priv.h | 2 +
drivers/hwtracing/coresight/coresight-tmc.c | 3 +-
drivers/hwtracing/coresight/coresight.c | 205 +++++++++++++++++-
include/linux/coresight.h | 6 +
6 files changed, 261 insertions(+), 20 deletions(-)
--
2.17.1
The current probe() function calls a pair of cpuhp_xxx API functions to
setup CPU hotplug handling. The hotplug lock is held for the duration of
the two calls and other CPU related code using cpus_read_lock() /
cpus_read_unlock() calls.
The problem is that on error states, goto: statements bypass the
cpus_read_unlock() call. This code has increased in complexity as the
driver has developed.
This patch introduces a pair of helper functions etm4_pm_setup_cpuslocked()
and etm4_pm_clear() which correct the issues above and group the PM code a
little better.
The two functions etm4_cpu_pm_register() and etm4_cpu_pm_unregister() are
dropped as these call cpu_pm_register_notifier() / ..unregister_notifier()
dependent on CONFIG_CPU_PM - but this define is used to nop these functions
out in the pm headers - so the wrapper functions are superfluous.
Fixes: f188b5e76aae ("coresight: etm4x: Save/restore state across CPU low power states")
Fixes: e9f5d63f84fe ("hwtracing/coresight-etm4x: Use cpuhp_setup_state_nocalls_cpuslocked()")
Fixes: 58eb457be028 ("etm4x: Convert to hotplug state machine")
Signed-off-by: Mike Leach <mike.leach(a)linaro.org>
---
drivers/hwtracing/coresight/coresight-etm4x.c | 82 ++++++++++++-------
1 file changed, 53 insertions(+), 29 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index 82fc2fab072a..2d732af8b3e7 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -1398,18 +1398,57 @@ static struct notifier_block etm4_cpu_pm_nb = {
.notifier_call = etm4_cpu_pm_notify,
};
-static int etm4_cpu_pm_register(void)
+/* Setup PM. Called with cpus locked. Deals with error conditions and counts */
+static int etm4_pm_setup_cpuslocked(void)
{
- if (IS_ENABLED(CONFIG_CPU_PM))
- return cpu_pm_register_notifier(&etm4_cpu_pm_nb);
+ int ret;
- return 0;
+ if (etm4_count++)
+ return 0;
+
+ ret = cpu_pm_register_notifier(&etm4_cpu_pm_nb);
+ if (ret)
+ goto reduce_count;
+
+ ret = cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ARM_CORESIGHT_STARTING,
+ "arm/coresight4:starting",
+ etm4_starting_cpu, etm4_dying_cpu);
+
+ if (ret)
+ goto unregister_notifier;
+
+ ret = cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ONLINE_DYN,
+ "arm/coresight4:online",
+ etm4_online_cpu, NULL);
+
+ /* HP dyn state ID returned in ret on success */
+ if (ret > 0) {
+ hp_online = ret;
+ return 0;
+ }
+
+ /* failed dyn state - remove others */
+ cpuhp_remove_state_nocalls_cpuslocked(CPUHP_AP_ARM_CORESIGHT_STARTING);
+
+unregister_notifier:
+ cpu_pm_unregister_notifier(&etm4_cpu_pm_nb);
+
+reduce_count:
+ --etm4_count;
+ return ret;
}
-static void etm4_cpu_pm_unregister(void)
+static void etm4_pm_clear(void)
{
- if (IS_ENABLED(CONFIG_CPU_PM))
- cpu_pm_unregister_notifier(&etm4_cpu_pm_nb);
+ if (--etm4_count != 0)
+ return;
+
+ cpu_pm_unregister_notifier(&etm4_cpu_pm_nb);
+ cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
+ if (hp_online) {
+ cpuhp_remove_state_nocalls(hp_online);
+ hp_online = 0;
+ }
}
static int etm4_probe(struct amba_device *adev, const struct amba_id *id)
@@ -1466,24 +1505,15 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id)
etm4_init_arch_data, drvdata, 1))
dev_err(dev, "ETM arch init failed\n");
- if (!etm4_count++) {
- cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ARM_CORESIGHT_STARTING,
- "arm/coresight4:starting",
- etm4_starting_cpu, etm4_dying_cpu);
- ret = cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ONLINE_DYN,
- "arm/coresight4:online",
- etm4_online_cpu, NULL);
- if (ret < 0)
- goto err_arch_supported;
- hp_online = ret;
+ ret = etm4_pm_setup_cpuslocked();
+ cpus_read_unlock();
- ret = etm4_cpu_pm_register();
- if (ret)
- goto err_arch_supported;
+ /* etm4_pm_setup_cpuslocked() does its own cleanup - exit on error */
+ if (ret) {
+ etmdrvdata[drvdata->cpu] = NULL;
+ return ret;
}
- cpus_read_unlock();
-
if (etm4_arch_supported(drvdata->arch) == false) {
ret = -EINVAL;
goto err_arch_supported;
@@ -1530,13 +1560,7 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id)
err_arch_supported:
etmdrvdata[drvdata->cpu] = NULL;
- if (--etm4_count == 0) {
- etm4_cpu_pm_unregister();
-
- cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
- if (hp_online)
- cpuhp_remove_state_nocalls(hp_online);
- }
+ etm4_pm_clear();
return ret;
}
--
2.17.1
The current probe() function calls a pair of cpuhp_xxx API functions to
setup CPU hotplug handling. The hotplug lock is held for the duration of
the two calls and other CPU related code using cpus_read_lock() /
cpus_read_unlock() calls.
The problem is that on error states, goto: statements bypass the
cpus_read_unlock() call. This code has increased in complexity as the
driver has developed.
This patch introduces a pair of helper functions etm4_pm_setup() and
etm4_pm_clear() which correct the issues above and group the PM code a
little better.
The two functions etm4_cpu_pm_register() and etm4_cpu_pm_unregister() are
dropped as these call cpu_pm_register_notifier() / ..unregister_notifier()
dependent on CONFIG_CPU_PM - but this define is used to nop these functions
out in the pm headers - so the wrapper functions are superfluous.
Fixes: f188b5e76aae ("coresight: etm4x: Save/restore state across CPU low power states")
Fixes: e9f5d63f84fe ("hwtracing/coresight-etm4x: Use cpuhp_setup_state_nocalls_cpuslocked()")
Fixes: 58eb457be028 ("etm4x: Convert to hotplug state machine")
Signed-off-by: Mike Leach <mike.leach(a)linaro.org>
---
drivers/hwtracing/coresight/coresight-etm4x.c | 79 ++++++++++++-------
1 file changed, 51 insertions(+), 28 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index 919b76fa49c4..4dd59de440de 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -1407,18 +1407,56 @@ static struct notifier_block etm4_cpu_pm_nb = {
.notifier_call = etm4_cpu_pm_notify,
};
-static int etm4_cpu_pm_register(void)
+/* Setup PM. Called with cpus locked. Deals with error conditions and counts */
+static int etm4_pm_setup_cpuslocked(void)
{
- if (IS_ENABLED(CONFIG_CPU_PM))
- return cpu_pm_register_notifier(&etm4_cpu_pm_nb);
+ int ret;
- return 0;
+ if (etm4_count++)
+ return 0;
+
+ ret = cpu_pm_register_notifier(&etm4_cpu_pm_nb);
+ if (ret)
+ goto reduce_count;
+
+ ret = cpuhp_setup_state_nocalls_cpuslocked(
+ CPUHP_AP_ARM_CORESIGHT_STARTING, "arm/coresight4:starting",
+ etm4_starting_cpu, etm4_dying_cpu);
+
+ if (ret)
+ goto unregister_notifier;
+
+ ret = cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ONLINE_DYN,
+ "arm/coresight4:online",
+ etm4_online_cpu, NULL);
+
+ /* HP dyn state ID returned in ret on success */
+ if (ret > 0) {
+ hp_online = ret;
+ return 0;
+ }
+
+ /* failed dyn state - remove others */
+ cpuhp_remove_state_nocalls_cpuslocked(CPUHP_AP_ARM_CORESIGHT_STARTING);
+
+unregister_notifier:
+ cpu_pm_unregister_notifier(&etm4_cpu_pm_nb);
+
+reduce_count:
+ --etm4_count;
+ return ret;
}
-static void etm4_cpu_pm_unregister(void)
+static void etm4_pm_clear(void)
{
- if (IS_ENABLED(CONFIG_CPU_PM))
+ if (--etm4_count == 0) {
cpu_pm_unregister_notifier(&etm4_cpu_pm_nb);
+ cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
+ if (hp_online) {
+ cpuhp_remove_state_nocalls(hp_online);
+ hp_online = 0;
+ }
+ }
}
static int etm4_probe(struct amba_device *adev, const struct amba_id *id)
@@ -1475,24 +1513,15 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id)
etm4_init_arch_data, drvdata, 1))
dev_err(dev, "ETM arch init failed\n");
- if (!etm4_count++) {
- cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ARM_CORESIGHT_STARTING,
- "arm/coresight4:starting",
- etm4_starting_cpu, etm4_dying_cpu);
- ret = cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ONLINE_DYN,
- "arm/coresight4:online",
- etm4_online_cpu, NULL);
- if (ret < 0)
- goto err_arch_supported;
- hp_online = ret;
+ ret = etm4_pm_setup_cpuslocked();
+ cpus_read_unlock();
- ret = etm4_cpu_pm_register();
- if (ret)
- goto err_arch_supported;
+ /* etm4_pm_setup does its own cleanup - just exit on error here */
+ if (ret) {
+ etmdrvdata[drvdata->cpu] = NULL;
+ return ret;
}
- cpus_read_unlock();
-
if (etm4_arch_supported(drvdata->arch) == false) {
ret = -EINVAL;
goto err_arch_supported;
@@ -1544,13 +1573,7 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id)
err_arch_supported:
etmdrvdata[drvdata->cpu] = NULL;
- if (--etm4_count == 0) {
- etm4_cpu_pm_unregister();
-
- cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
- if (hp_online)
- cpuhp_remove_state_nocalls(hp_online);
- }
+ etm4_pm_clear();
return ret;
}
--
2.17.1
