Hi Mike,
I am from the Android toolchain team. We are interested in using the ETM
strobe feature in Android kernels.
We noticed your patch "coresight: etmv4: Adds initial complex config with
ETM4 strobe feature." in
https://lists.linaro.org/pipermail/coresight/2020-June/004097.html.
We tested it on Pixel devices with Qualcomm chips. It works pretty well.
And I have a few questions:
1. The param setting of window and period seems doesn't take effect. Not
sure if it is a bug or something I misunderstand.
The only place I found moving feat->params to feat->regs is in
cs_cfg_reset_feat(). But that function will also reset feat->params to
default values in feat_desc->params.
So if I write a value to /sys/.../window, it goes to feat->params, but
can't change value in feat->regs.
I tried a work-around by changing cs_cfg_param_store():
* static ssize_t cs_cfg_param_store(struct device *dev, {*
* ... spin_lock(param->feat->dev_spinlock);*
* if (!kstrtoull(buf, 0, &val)) {*
* param->current_value = val; for (i = 0; i
< feat->nr_regs; i++) { reg_desc =
&feat_desc->regs[i];*
* flags = reg_desc->flags;*
* reg = &feat->regs[i];*
* if (flags & CS_CFG_REG_VAL_PARAM) {*
* if (param ==
&feat->params[reg_desc->value.val32]) {*
* coresight_cfg_init_reg_param(reg,
flags, param); }*
* }*
* }*
* }*
* spin_unlock(param->feat->dev_spinlock);*
* }*
2. Is my understanding below correct?
If I set the window param to 1000 and the period param to 10000, then etm
will trace the last 1000 cpu cycles of every 10^7 cpu cycles?
And since the strobe window/period counters are saved when disabling etm,
the counting should continue from the saved value the next time I enable
etm. And I should still
get etm data even if the traced thread is scheduled off cpu before
reaching 10^7 cpu cycles each time.
3. I want to +1 Mathieu's suggestion:
*Perf sessions need to be able to choose which feature they want to use rather
than defaulting to a system wide configuration.*
We use perf sessions to trace ETM data in Android. It's better if we
can configure ETM strobe params in the perf interface (like from
perf_event_attr or ioctl). And It's better for each perf session to
count separately.
4. The last update on the patch is Jun 24. And it seems the patch
hasn't been landed in any branch yet. So what's the future plan?
We need to wait until it lands to some upstream branches to cherry
pick it to Android kernels. So I want to know when it will be ready.
Thanks,
Yabin
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.
CoreSight ETMv4.4 introduced system instructions for accessing
the ETM. This also implies that they may not be on the amba bus.
Right now all the CoreSight components are accessed via memory
map. Also, we have some common routines in coresight generic
code driver (e.g, CS_LOCK, claim/disclaim), which assume the
mmio. In order to preserve the generic algorithms at a single
place and to allow dynamic switch for ETMs, this series introduces
an abstraction layer for accessing a coresight device. It is
designed such that the mmio access are fast tracked (i.e, without
an indirect function call).
This will also help us to get rid of the driver+attribute specific
sysfs show/store routines and replace them with a single routine
to access a given register offset (which can be embedded in the
dev_ext_attribute). This is not currently implemented in the series,
but can be achieved.
Further we switch the generic routines to work with the abstraction.
With this in place, we refactor the etm4x code a bit to allow for
supporting the system instructions with very little new code. The
changes also switch to using the system instructions by default
even when we may have an MMIO.
We use TRCDEVARCH for the detection of the ETM component, which
is a standard register as per CoreSight architecture, rather than
the etm specific id register TRCIDR3. This is for making sure
that we are able to detect the ETM via system instructions accurately,
when the the trace unit could be anything (etm or a custom trace unit).
The series has been mildly tested on a model. I would really
appreciate any testing on real hardware.
Applies on coresight/next
Changes since V1:
- Flip the switch for iomem from no_iomem to io_mem in csdev_access.
- Split patches for claim/disclaim and CS_LOCK/UNLOCK conversions.
- Move device access initialisation for etm4x to the target CPU
- Cleanup secure exception level mask handling.
- Switch to use TRCDEVARCH for ETM component discovery. This
is for making
- Check the availability of OS/Software Locks before using them.
Suzuki K Poulose (19):
coresight: Introduce device access abstraction
coresight: tpiu: Prepare for using coresight device access abstraction
coresight: Convert coresight_timeout to use access abstraction
coresight: Convert claim/disclaim operations to use access wrappers
coresight: Use device access layer for Software lock/unlock operations
coresight: etm4x: Always read the registers on the host CPU
coresight: etm4x: Convert all register accesses
coresight: etm4x: Add commentary on the registers
coresight: etm4x: Add sysreg access helpers
coresight: etm4x: Define DEVARCH register fields
coresight: etm4x: Check for OS and Software Lock
coresight: etm4x: Cleanup secure exception level masks
coresight: etm4x: Clean up exception level masks
coresight: etm4x: Detect access early on the target CPU
coresight: etm4x: Use TRCDEVARCH for component discovery
coresight: etm4x: Detect system instructions support
coresight: etm4x: Refactor probing routine
coresight: etm4x: Add support for sysreg only devices
dts: bindings: coresight: ETMv4.4 system register access only units
.../devicetree/bindings/arm/coresight.txt | 6 +-
drivers/hwtracing/coresight/coresight-catu.c | 24 +-
.../hwtracing/coresight/coresight-cpu-debug.c | 22 +-
.../hwtracing/coresight/coresight-cti-sysfs.c | 5 +-
drivers/hwtracing/coresight/coresight-cti.c | 34 +-
drivers/hwtracing/coresight/coresight-etb10.c | 29 +-
.../coresight/coresight-etm3x-sysfs.c | 10 +-
drivers/hwtracing/coresight/coresight-etm3x.c | 35 +-
.../coresight/coresight-etm4x-sysfs.c | 44 +-
drivers/hwtracing/coresight/coresight-etm4x.c | 716 +++++++++++-------
drivers/hwtracing/coresight/coresight-etm4x.h | 440 ++++++++++-
.../hwtracing/coresight/coresight-funnel.c | 22 +-
drivers/hwtracing/coresight/coresight-priv.h | 9 +-
.../coresight/coresight-replicator.c | 31 +-
drivers/hwtracing/coresight/coresight-stm.c | 50 +-
.../hwtracing/coresight/coresight-tmc-etf.c | 38 +-
.../hwtracing/coresight/coresight-tmc-etr.c | 20 +-
drivers/hwtracing/coresight/coresight-tmc.c | 16 +-
drivers/hwtracing/coresight/coresight-tpiu.c | 32 +-
drivers/hwtracing/coresight/coresight.c | 130 +++-
include/linux/coresight.h | 230 +++++-
21 files changed, 1449 insertions(+), 494 deletions(-)
--
2.24.1
This 2 patch series provides fixes to ETF null pointer dereference crash
and TRCVMIDCCTLR1 register save and restore fix.
Patch 1 is an RFC since I am not sure of the fix provided since it looks
more like a band-aid than the actual fix.
Changes in v2:
* Remove extra fixes tag (Suzuki)
Sai Prakash Ranjan (2):
coresight: tmc-etf: Fix NULL pointer dereference in
tmc_enable_etf_sink_perf()
coresight: etm4x: Fix save and restore of TRCVMIDCCTLR1 register
drivers/hwtracing/coresight/coresight-etm4x-core.c | 4 ++--
drivers/hwtracing/coresight/coresight-tmc-etf.c | 3 +++
2 files changed, 5 insertions(+), 2 deletions(-)
base-commit: e209e73bee253afe969410150248f0c300c13d84
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
From: Leo Yan <leo.yan(a)linaro.org>
[ Upstream commit c9f5baa136777b2c982f6f7a90c9da69a88be148 ]
When 'etm->instructions_sample_period' is less than
'tidq->period_instructions', the function cs_etm__sample() cannot handle
this case properly with its logic.
Let's see below flow as an example:
- If we set itrace option '--itrace=i4', then function cs_etm__sample()
has variables with initialized values:
tidq->period_instructions = 0
etm->instructions_sample_period = 4
- When the first packet is coming:
packet->instr_count = 10; the number of instructions executed in this
packet is 10, thus update period_instructions as below:
tidq->period_instructions = 0 + 10 = 10
instrs_over = 10 - 4 = 6
offset = 10 - 6 - 1 = 3
tidq->period_instructions = instrs_over = 6
- When the second packet is coming:
packet->instr_count = 10; in the second pass, assume 10 instructions
in the trace sample again:
tidq->period_instructions = 6 + 10 = 16
instrs_over = 16 - 4 = 12
offset = 10 - 12 - 1 = -3 -> the negative value
tidq->period_instructions = instrs_over = 12
So after handle these two packets, there have below issues:
The first issue is that cs_etm__instr_addr() returns the address within
the current trace sample of the instruction related to offset, so the
offset is supposed to be always unsigned value. But in fact, function
cs_etm__sample() might calculate a negative offset value (in handling
the second packet, the offset is -3) and pass to cs_etm__instr_addr()
with u64 type with a big positive integer.
The second issue is it only synthesizes 2 samples for sample period = 4.
In theory, every packet has 10 instructions so the two packets have
total 20 instructions, 20 instructions should generate 5 samples
(4 x 5 = 20). This is because cs_etm__sample() only calls once
cs_etm__synth_instruction_sample() to generate instruction sample per
range packet.
This patch fixes the logic in function cs_etm__sample(); the basic
idea for handling coming packet is:
- To synthesize the first instruction sample, it combines the left
instructions from the previous packet and the head of the new
packet; then generate continuous samples with sample period;
- At the tail of the new packet, if it has the rest instructions,
these instructions will be left for the sequential sample.
Suggested-by: Mike Leach <mike.leach(a)linaro.org>
Signed-off-by: Leo Yan <leo.yan(a)linaro.org>
Reviewed-by: Mathieu Poirier <mathieu.poirier(a)linaro.org>
Reviewed-by: Mike Leach <mike.leach(a)linaro.org>
Cc: Alexander Shishkin <alexander.shishkin(a)linux.intel.com>
Cc: Jiri Olsa <jolsa(a)redhat.com>
Cc: Mark Rutland <mark.rutland(a)arm.com>
Cc: Namhyung Kim <namhyung(a)kernel.org>
Cc: Peter Zijlstra <peterz(a)infradead.org>
Cc: Robert Walker <robert.walker(a)arm.com>
Cc: Suzuki Poulouse <suzuki.poulose(a)arm.com>
Cc: coresight ml <coresight(a)lists.linaro.org>
Cc: linux-arm-kernel(a)lists.infradead.org
Link: http://lore.kernel.org/lkml/20200219021811.20067-4-leo.yan@linaro.org
Signed-off-by: Arnaldo Carvalho de Melo <acme(a)redhat.com>
Signed-off-by: Sasha Levin <sashal(a)kernel.org>
---
tools/perf/util/cs-etm.c | 87 ++++++++++++++++++++++++++++++++--------
1 file changed, 70 insertions(+), 17 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 38298cbb07524..451eee24165ee 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1359,9 +1359,12 @@ static int cs_etm__sample(struct cs_etm_queue *etmq,
struct cs_etm_auxtrace *etm = etmq->etm;
int ret;
u8 trace_chan_id = tidq->trace_chan_id;
- u64 instrs_executed = tidq->packet->instr_count;
+ u64 instrs_prev;
- tidq->period_instructions += instrs_executed;
+ /* Get instructions remainder from previous packet */
+ instrs_prev = tidq->period_instructions;
+
+ tidq->period_instructions += tidq->packet->instr_count;
/*
* Record a branch when the last instruction in
@@ -1379,26 +1382,76 @@ static int cs_etm__sample(struct cs_etm_queue *etmq,
* TODO: allow period to be defined in cycles and clock time
*/
- /* Get number of instructions executed after the sample point */
- u64 instrs_over = tidq->period_instructions -
- etm->instructions_sample_period;
+ /*
+ * Below diagram demonstrates the instruction samples
+ * generation flows:
+ *
+ * Instrs Instrs Instrs Instrs
+ * Sample(n) Sample(n+1) Sample(n+2) Sample(n+3)
+ * | | | |
+ * V V V V
+ * --------------------------------------------------
+ * ^ ^
+ * | |
+ * Period Period
+ * instructions(Pi) instructions(Pi')
+ *
+ * | |
+ * \---------------- -----------------/
+ * V
+ * tidq->packet->instr_count
+ *
+ * Instrs Sample(n...) are the synthesised samples occurring
+ * every etm->instructions_sample_period instructions - as
+ * defined on the perf command line. Sample(n) is being the
+ * last sample before the current etm packet, n+1 to n+3
+ * samples are generated from the current etm packet.
+ *
+ * tidq->packet->instr_count represents the number of
+ * instructions in the current etm packet.
+ *
+ * Period instructions (Pi) contains the the number of
+ * instructions executed after the sample point(n) from the
+ * previous etm packet. This will always be less than
+ * etm->instructions_sample_period.
+ *
+ * When generate new samples, it combines with two parts
+ * instructions, one is the tail of the old packet and another
+ * is the head of the new coming packet, to generate
+ * sample(n+1); sample(n+2) and sample(n+3) consume the
+ * instructions with sample period. After sample(n+3), the rest
+ * instructions will be used by later packet and it is assigned
+ * to tidq->period_instructions for next round calculation.
+ */
/*
- * Calculate the address of the sampled instruction (-1 as
- * sample is reported as though instruction has just been
- * executed, but PC has not advanced to next instruction)
+ * Get the initial offset into the current packet instructions;
+ * entry conditions ensure that instrs_prev is less than
+ * etm->instructions_sample_period.
*/
- u64 offset = (instrs_executed - instrs_over - 1);
- u64 addr = cs_etm__instr_addr(etmq, trace_chan_id,
- tidq->packet, offset);
+ u64 offset = etm->instructions_sample_period - instrs_prev;
+ u64 addr;
- ret = cs_etm__synth_instruction_sample(
- etmq, tidq, addr, etm->instructions_sample_period);
- if (ret)
- return ret;
+ while (tidq->period_instructions >=
+ etm->instructions_sample_period) {
+ /*
+ * Calculate the address of the sampled instruction (-1
+ * as sample is reported as though instruction has just
+ * been executed, but PC has not advanced to next
+ * instruction)
+ */
+ addr = cs_etm__instr_addr(etmq, trace_chan_id,
+ tidq->packet, offset - 1);
+ ret = cs_etm__synth_instruction_sample(
+ etmq, tidq, addr,
+ etm->instructions_sample_period);
+ if (ret)
+ return ret;
- /* Carry remaining instructions into next sample period */
- tidq->period_instructions = instrs_over;
+ offset += etm->instructions_sample_period;
+ tidq->period_instructions -=
+ etm->instructions_sample_period;
+ }
}
if (etm->sample_branches) {
--
2.25.1
This 2 patch series provides fixes to ETF null pointer dereference crash
and TRCVMIDCCTLR1 register save and restore fix.
Patch 1 is an RFC since I am not sure of the fix provided since it looks
more like a band-aid than the actual fix.
Sai Prakash Ranjan (2):
coresight: tmc-etf: Fix NULL pointer dereference in
tmc_enable_etf_sink_perf()
coresight: etm4x: Fix save and restore of TRCVMIDCCTLR1 register
drivers/hwtracing/coresight/coresight-etm4x-core.c | 4 ++--
drivers/hwtracing/coresight/coresight-tmc-etf.c | 3 +++
2 files changed, 5 insertions(+), 2 deletions(-)
base-commit: e209e73bee253afe969410150248f0c300c13d84
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Hello guys,
There was an issue reported where the ETM timestamp values would
freeze or be 0 sometimes when using perf mode of tracing. This was
observed only with perf mode and not sysfs but was reproducible
with both ETR and ETF as sinks.
After some debugging, I found that it's mostly to do with CPU
idle states. For perf mode, I never see entering CPU_PM_ENTER
or EXIT(since the check is mode) and so etm4_cpu_save() and
restore() are not called and I believe the contexts are not
properly saved and restored across low power states leading
to these bad timestamp values. For sysfs tracing, I see save
and restore called for respective ETMs when CPUs enter low
power states.
For testing, I would run perf record and report commands in a loop.
perf record -e cs_etm/@tmc_etf0/ -N -o perf.data -- some_test
perf report -D -i perf.data | grep Timestamp | head
On other shell, I would try to suspend the system:
echo freeze/mem > /sys/power/state
After waking up from suspend, perf commands would continue to
run and reports would continue to show either 0 or same
timestamps for some period of time.
Even the above test does not always reproduce the issue but
the observation that save and restore is not called is always
consistent with perf cs mode.
Any idea as to what might be missing for perf mode or if it is
expected or some limitation?
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
Allow to build coresight as modules. This gives developers the feasibility to
test their code without reboot.
This series is based on below two series.
- "coresight: allow to build components as modules"
https://lkml.org/lkml/2018/6/5/989
- "coresight: make drivers modular"
https://lkml.org/lkml/2020/1/17/468
Change from v10:
Rebase to latest coresight-next
Protect etmdrvdata[] by modifying it on relevant CPU for ETM3. (Mathieu and Suzuki)
-It's not tested.
Refine the alignment in coresight makefile. (Mathieu)
Add reason for cti_remove_conn_xrefs change. (Mathieu)
Set owner for static funnel driver. (Suzuki)
Change from v9:
Fix warning reported by kernel test robot <lkp(a)intel.com>
Rebase to coresight-next
Change from v8:
Protect etmdrvdata[] by modifying it on relevant CPU (Mathieu and Suzuki)
Grab the device before allocating memory for the node (Mathieu)
Add author of coresight core driver (Mathieu)
Change from v7:
Depends on below change for ETM hotplug (Sai)
https://lore.kernel.org/linux-arm-kernel/20200729051310.18436-1-saiprakash.…
Add mutex lock to protect etmdrvdata[] (Suzuki)
Add helper function coresight_get_ref() (Suzuki)
Reorg replicator and funnel change. Use two patches to support these
two drivers. (Suzuki)
Add fix tag to "coresight: etm: perf: Fix warning caused by etm_setup_aux
failure" (Suzuki)
Update author of "coresight: cti: Fix bug clearing sysfs links on callback"
Change from v6:
Correct module description for CATU (Mike)
Check ect_ret equals 0 and set ect_enabled flag (Mike)
Add Tested-by and Reviewed-by from Mike
Change from v5:
Add below CTI clean up change from Mike into series
-https://lists.linaro.org/pipermail/coresight/2020-July/004349.html
Increase module reference count when enabling CTI device (Mike)
Change from v4:
Fix error handling in coresight_grab_devicei() (Greg)
Add coresight: cti: Fix remove sysfs link error from Mike
-https://lists.linaro.org/pipermail/coresight/2020-July/004275.html
Move cti_remove_conn_xrefs() into cti_remove() (Mike)
Align patch subject to coresight: <component>: <description> (Mike)
Change from v3:
Rebase to coresight-next (Mike and Mathieu)
Reorder try_get_module() (Suzuki)
Clean up etmdrvdata[] in device remote path (Mike)
Move cti_remove_conn_xrefs to cti_remove (Mike)
Change from v2:
Rebase to 5.8-rc5. Export coresight_add_sysfs_link and
coresight_remove_sysfs_link
Fix one cut and paste error on MODULE_DESCRIPTION of CTI
Change from v1:
Use try_module_get() to avoid module to be unloaded when device is used
in active trace session. (Mathieu P)
Change from above two series.
This series adds the support to dynamically remove module when the device in
that module is enabled and used by some trace path. It disables all trace
paths with that device and release the trace path.
Kim Phillips (8):
coresight: use IS_ENABLED for CONFIGs that may be modules
coresight: etm3x: allow etm3x to be built as a module
coresight: etm4x: allow etm4x to be built as a module
coresight: etb: allow etb to be built as a module
coresight: tpiu: allow tpiu to be built as a module
coresight: tmc: allow tmc to be built as a module
coresight: allow funnel driver to be built as module
coresight: allow replicator driver to be built as module
Mian Yousaf Kaukab (2):
coresight: export global symbols
coresight: tmc-etr: add function to register catu ops
Mike Leach (2):
coresight: cti: Fix remove sysfs link error
coresight: cti: Fix bug clearing sysfs links on callback
Tingwei Zhang (12):
coresight: cpu_debug: add module name in Kconfig
coresight: cpu_debug: define MODULE_DEVICE_TABLE
coresight: add coresight prefix to barrier_pkt
coresight: add try_get_module() in coresight_grab_device()
coresight: stm: allow to build coresight-stm as a module
coresight: etm: perf: Fix warning caused by etm_setup_aux failure
coresight: cti: add function to register cti associate ops
coresight: cti: don't disable ect device if it's not enabled
coresight: cti: increase reference count when enabling cti
coresight: cti: allow cti to be built as a module
coresight: catu: allow catu drivers to be built as modules
coresight: allow the coresight core driver to be built as a module
drivers/hwtracing/coresight/Kconfig | 54 ++++--
drivers/hwtracing/coresight/Makefile | 26 +--
drivers/hwtracing/coresight/coresight-catu.c | 37 ++++-
drivers/hwtracing/coresight/coresight-catu.h | 2 -
.../{coresight.c => coresight-core.c} | 154 +++++++++++++++---
.../hwtracing/coresight/coresight-cpu-debug.c | 2 +
.../{coresight-cti.c => coresight-cti-core.c} | 67 ++++++--
drivers/hwtracing/coresight/coresight-etb10.c | 28 +++-
.../hwtracing/coresight/coresight-etm-perf.c | 13 +-
.../hwtracing/coresight/coresight-etm-perf.h | 5 +-
...resight-etm3x.c => coresight-etm3x-core.c} | 148 +++++++++++++----
...resight-etm4x.c => coresight-etm4x-core.c} | 84 +++++++---
.../hwtracing/coresight/coresight-funnel.c | 65 +++++++-
.../hwtracing/coresight/coresight-platform.c | 1 +
drivers/hwtracing/coresight/coresight-priv.h | 23 ++-
.../coresight/coresight-replicator.c | 65 +++++++-
drivers/hwtracing/coresight/coresight-stm.c | 20 ++-
drivers/hwtracing/coresight/coresight-sysfs.c | 2 +
.../{coresight-tmc.c => coresight-tmc-core.c} | 25 ++-
.../hwtracing/coresight/coresight-tmc-etf.c | 2 +-
.../hwtracing/coresight/coresight-tmc-etr.c | 21 ++-
drivers/hwtracing/coresight/coresight-tmc.h | 3 +
drivers/hwtracing/coresight/coresight-tpiu.c | 20 ++-
include/linux/coresight.h | 3 +-
24 files changed, 726 insertions(+), 144 deletions(-)
rename drivers/hwtracing/coresight/{coresight.c => coresight-core.c} (92%)
rename drivers/hwtracing/coresight/{coresight-cti.c => coresight-cti-core.c} (94%)
rename drivers/hwtracing/coresight/{coresight-etm3x.c => coresight-etm3x-core.c} (90%)
rename drivers/hwtracing/coresight/{coresight-etm4x.c => coresight-etm4x-core.c} (96%)
rename drivers/hwtracing/coresight/{coresight-tmc.c => coresight-tmc-core.c} (95%)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
From: Leo Yan <leo.yan(a)linaro.org>
[ Upstream commit c9f5baa136777b2c982f6f7a90c9da69a88be148 ]
When 'etm->instructions_sample_period' is less than
'tidq->period_instructions', the function cs_etm__sample() cannot handle
this case properly with its logic.
Let's see below flow as an example:
- If we set itrace option '--itrace=i4', then function cs_etm__sample()
has variables with initialized values:
tidq->period_instructions = 0
etm->instructions_sample_period = 4
- When the first packet is coming:
packet->instr_count = 10; the number of instructions executed in this
packet is 10, thus update period_instructions as below:
tidq->period_instructions = 0 + 10 = 10
instrs_over = 10 - 4 = 6
offset = 10 - 6 - 1 = 3
tidq->period_instructions = instrs_over = 6
- When the second packet is coming:
packet->instr_count = 10; in the second pass, assume 10 instructions
in the trace sample again:
tidq->period_instructions = 6 + 10 = 16
instrs_over = 16 - 4 = 12
offset = 10 - 12 - 1 = -3 -> the negative value
tidq->period_instructions = instrs_over = 12
So after handle these two packets, there have below issues:
The first issue is that cs_etm__instr_addr() returns the address within
the current trace sample of the instruction related to offset, so the
offset is supposed to be always unsigned value. But in fact, function
cs_etm__sample() might calculate a negative offset value (in handling
the second packet, the offset is -3) and pass to cs_etm__instr_addr()
with u64 type with a big positive integer.
The second issue is it only synthesizes 2 samples for sample period = 4.
In theory, every packet has 10 instructions so the two packets have
total 20 instructions, 20 instructions should generate 5 samples
(4 x 5 = 20). This is because cs_etm__sample() only calls once
cs_etm__synth_instruction_sample() to generate instruction sample per
range packet.
This patch fixes the logic in function cs_etm__sample(); the basic
idea for handling coming packet is:
- To synthesize the first instruction sample, it combines the left
instructions from the previous packet and the head of the new
packet; then generate continuous samples with sample period;
- At the tail of the new packet, if it has the rest instructions,
these instructions will be left for the sequential sample.
Suggested-by: Mike Leach <mike.leach(a)linaro.org>
Signed-off-by: Leo Yan <leo.yan(a)linaro.org>
Reviewed-by: Mathieu Poirier <mathieu.poirier(a)linaro.org>
Reviewed-by: Mike Leach <mike.leach(a)linaro.org>
Cc: Alexander Shishkin <alexander.shishkin(a)linux.intel.com>
Cc: Jiri Olsa <jolsa(a)redhat.com>
Cc: Mark Rutland <mark.rutland(a)arm.com>
Cc: Namhyung Kim <namhyung(a)kernel.org>
Cc: Peter Zijlstra <peterz(a)infradead.org>
Cc: Robert Walker <robert.walker(a)arm.com>
Cc: Suzuki Poulouse <suzuki.poulose(a)arm.com>
Cc: coresight ml <coresight(a)lists.linaro.org>
Cc: linux-arm-kernel(a)lists.infradead.org
Link: http://lore.kernel.org/lkml/20200219021811.20067-4-leo.yan@linaro.org
Signed-off-by: Arnaldo Carvalho de Melo <acme(a)redhat.com>
Signed-off-by: Sasha Levin <sashal(a)kernel.org>
---
tools/perf/util/cs-etm.c | 87 ++++++++++++++++++++++++++++++++--------
1 file changed, 70 insertions(+), 17 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 38298cbb07524..451eee24165ee 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1359,9 +1359,12 @@ static int cs_etm__sample(struct cs_etm_queue *etmq,
struct cs_etm_auxtrace *etm = etmq->etm;
int ret;
u8 trace_chan_id = tidq->trace_chan_id;
- u64 instrs_executed = tidq->packet->instr_count;
+ u64 instrs_prev;
- tidq->period_instructions += instrs_executed;
+ /* Get instructions remainder from previous packet */
+ instrs_prev = tidq->period_instructions;
+
+ tidq->period_instructions += tidq->packet->instr_count;
/*
* Record a branch when the last instruction in
@@ -1379,26 +1382,76 @@ static int cs_etm__sample(struct cs_etm_queue *etmq,
* TODO: allow period to be defined in cycles and clock time
*/
- /* Get number of instructions executed after the sample point */
- u64 instrs_over = tidq->period_instructions -
- etm->instructions_sample_period;
+ /*
+ * Below diagram demonstrates the instruction samples
+ * generation flows:
+ *
+ * Instrs Instrs Instrs Instrs
+ * Sample(n) Sample(n+1) Sample(n+2) Sample(n+3)
+ * | | | |
+ * V V V V
+ * --------------------------------------------------
+ * ^ ^
+ * | |
+ * Period Period
+ * instructions(Pi) instructions(Pi')
+ *
+ * | |
+ * \---------------- -----------------/
+ * V
+ * tidq->packet->instr_count
+ *
+ * Instrs Sample(n...) are the synthesised samples occurring
+ * every etm->instructions_sample_period instructions - as
+ * defined on the perf command line. Sample(n) is being the
+ * last sample before the current etm packet, n+1 to n+3
+ * samples are generated from the current etm packet.
+ *
+ * tidq->packet->instr_count represents the number of
+ * instructions in the current etm packet.
+ *
+ * Period instructions (Pi) contains the the number of
+ * instructions executed after the sample point(n) from the
+ * previous etm packet. This will always be less than
+ * etm->instructions_sample_period.
+ *
+ * When generate new samples, it combines with two parts
+ * instructions, one is the tail of the old packet and another
+ * is the head of the new coming packet, to generate
+ * sample(n+1); sample(n+2) and sample(n+3) consume the
+ * instructions with sample period. After sample(n+3), the rest
+ * instructions will be used by later packet and it is assigned
+ * to tidq->period_instructions for next round calculation.
+ */
/*
- * Calculate the address of the sampled instruction (-1 as
- * sample is reported as though instruction has just been
- * executed, but PC has not advanced to next instruction)
+ * Get the initial offset into the current packet instructions;
+ * entry conditions ensure that instrs_prev is less than
+ * etm->instructions_sample_period.
*/
- u64 offset = (instrs_executed - instrs_over - 1);
- u64 addr = cs_etm__instr_addr(etmq, trace_chan_id,
- tidq->packet, offset);
+ u64 offset = etm->instructions_sample_period - instrs_prev;
+ u64 addr;
- ret = cs_etm__synth_instruction_sample(
- etmq, tidq, addr, etm->instructions_sample_period);
- if (ret)
- return ret;
+ while (tidq->period_instructions >=
+ etm->instructions_sample_period) {
+ /*
+ * Calculate the address of the sampled instruction (-1
+ * as sample is reported as though instruction has just
+ * been executed, but PC has not advanced to next
+ * instruction)
+ */
+ addr = cs_etm__instr_addr(etmq, trace_chan_id,
+ tidq->packet, offset - 1);
+ ret = cs_etm__synth_instruction_sample(
+ etmq, tidq, addr,
+ etm->instructions_sample_period);
+ if (ret)
+ return ret;
- /* Carry remaining instructions into next sample period */
- tidq->period_instructions = instrs_over;
+ offset += etm->instructions_sample_period;
+ tidq->period_instructions -=
+ etm->instructions_sample_period;
+ }
}
if (etm->sample_branches) {
--
2.25.1