From: Linu Cherian <lcherian(a)marvell.com>
[ Upstream commit 6d578258b955fc8888e1bbd9a8fefe7b10065a84 ]
Coresight driver assumes sink is common across all the ETMs,
and tries to build a path between ETM and the first enabled
sink found using bus based search. This breaks sysFS usage
on implementations that has multiple per core sinks in
enabled state.
To fix this, coresight_get_enabled_sink API is updated to
do a connection based search starting from the given source,
instead of bus based search.
With sink selection using sysfs depecrated for perf interface,
provision for reset is removed as well in this API.
Signed-off-by: Linu Cherian <lcherian(a)marvell.com>
[Fixed indentation problem and removed obsolete comment]
Signed-off-by: Mathieu Poirier <mathieu.poirier(a)linaro.org>
Link: https://lore.kernel.org/r/20200916191737.4001561-15-mathieu.poirier@linaro.…
Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal(a)kernel.org>
---
drivers/hwtracing/coresight/coresight-priv.h | 3 +-
drivers/hwtracing/coresight/coresight.c | 62 +++++++++-----------
2 files changed, 29 insertions(+), 36 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index 36c943ae94d52..e383bc72143d8 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -148,7 +148,8 @@ static inline void coresight_write_reg_pair(void __iomem *addr, u64 val,
void coresight_disable_path(struct list_head *path);
int coresight_enable_path(struct list_head *path, u32 mode, void *sink_data);
struct coresight_device *coresight_get_sink(struct list_head *path);
-struct coresight_device *coresight_get_enabled_sink(bool reset);
+struct coresight_device *
+coresight_get_enabled_sink(struct coresight_device *source);
struct coresight_device *coresight_get_sink_by_id(u32 id);
struct list_head *coresight_build_path(struct coresight_device *csdev,
struct coresight_device *sink);
diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
index f3efbb3b2b4d1..f834255b013f3 100644
--- a/drivers/hwtracing/coresight/coresight.c
+++ b/drivers/hwtracing/coresight/coresight.c
@@ -540,50 +540,46 @@ struct coresight_device *coresight_get_sink(struct list_head *path)
return csdev;
}
-static int coresight_enabled_sink(struct device *dev, const void *data)
+static struct coresight_device *
+coresight_find_enabled_sink(struct coresight_device *csdev)
{
- const bool *reset = data;
- struct coresight_device *csdev = to_coresight_device(dev);
+ int i;
+ struct coresight_device *sink;
if ((csdev->type == CORESIGHT_DEV_TYPE_SINK ||
csdev->type == CORESIGHT_DEV_TYPE_LINKSINK) &&
- csdev->activated) {
- /*
- * Now that we have a handle on the sink for this session,
- * disable the sysFS "enable_sink" flag so that possible
- * concurrent perf session that wish to use another sink don't
- * trip on it. Doing so has no ramification for the current
- * session.
- */
- if (*reset)
- csdev->activated = false;
+ csdev->activated)
+ return csdev;
- return 1;
+ /*
+ * Recursively explore each port found on this element.
+ */
+ for (i = 0; i < csdev->pdata->nr_outport; i++) {
+ struct coresight_device *child_dev;
+
+ child_dev = csdev->pdata->conns[i].child_dev;
+ if (child_dev)
+ sink = coresight_find_enabled_sink(child_dev);
+ if (sink)
+ return sink;
}
- return 0;
+ return NULL;
}
/**
- * coresight_get_enabled_sink - returns the first enabled sink found on the bus
- * @deactivate: Whether the 'enable_sink' flag should be reset
+ * coresight_get_enabled_sink - returns the first enabled sink using
+ * connection based search starting from the source reference
*
- * When operated from perf the deactivate parameter should be set to 'true'.
- * That way the "enabled_sink" flag of the sink that was selected can be reset,
- * allowing for other concurrent perf sessions to choose a different sink.
- *
- * When operated from sysFS users have full control and as such the deactivate
- * parameter should be set to 'false', hence mandating users to explicitly
- * clear the flag.
+ * @source: Coresight source device reference
*/
-struct coresight_device *coresight_get_enabled_sink(bool deactivate)
+struct coresight_device *
+coresight_get_enabled_sink(struct coresight_device *source)
{
- struct device *dev = NULL;
-
- dev = bus_find_device(&coresight_bustype, NULL, &deactivate,
- coresight_enabled_sink);
+ if (!source)
+ return NULL;
- return dev ? to_coresight_device(dev) : NULL;
+ return coresight_find_enabled_sink(source);
}
static int coresight_sink_by_id(struct device *dev, const void *data)
@@ -823,11 +819,7 @@ int coresight_enable(struct coresight_device *csdev)
goto out;
}
- /*
- * Search for a valid sink for this session but don't reset the
- * "enable_sink" flag in sysFS. Users get to do that explicitly.
- */
- sink = coresight_get_enabled_sink(false);
+ sink = coresight_get_enabled_sink(csdev);
if (!sink) {
ret = -EINVAL;
goto out;
--
2.25.1
There was a report of NULL pointer dereference in ETF enable
path for perf CS mode with PID monitoring. It is almost 100%
reproducible when the process to monitor is something very
active such as chrome and with ETF as the sink and not ETR.
Currently in a bid to find the pid, the owner is dereferenced
via task_pid_nr() call in tmc_enable_etf_sink_perf() and with
owner being NULL, we get a NULL pointer dereference.
Looking at the ETR and other places in the kernel, ETF and the
ETB are the only places trying to dereference the task(owner)
in tmc_enable_etf_sink_perf() which is also called from the
sched_in path as in the call trace. Owner(task) is NULL even
in the case of ETR in tmc_enable_etr_sink_perf(), but since we
cache the PID in alloc_buffer() callback and it is done as part
of etm_setup_aux() when allocating buffer for ETR sink, we never
dereference this NULL pointer and we are safe. So lets do the
same thing with ETF and ETB and cache the PID to which the
cs_buffer belongs in alloc_buffer() callback for ETF and ETB as
done for ETR. This will also remove the unnecessary function calls
(task_pid_nr()) in tmc_enable_etr_sink_perf() and etb_enable_perf().
Easily reproducible running below:
perf record -e cs_etm/@tmc_etf0/ -N -p <pid>
Unable to handle kernel NULL pointer dereference at virtual address 0000000000000548
Mem abort info:
ESR = 0x96000006
EC = 0x25: DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
Data abort info:
ISV = 0, ISS = 0x00000006
CM = 0, WnR = 0
<snip>...
Call trace:
tmc_enable_etf_sink+0xe4/0x280
coresight_enable_path+0x168/0x1fc
etm_event_start+0x8c/0xf8
etm_event_add+0x38/0x54
event_sched_in+0x194/0x2ac
group_sched_in+0x54/0x12c
flexible_sched_in+0xd8/0x120
visit_groups_merge+0x100/0x16c
ctx_flexible_sched_in+0x50/0x74
ctx_sched_in+0xa4/0xa8
perf_event_sched_in+0x60/0x6c
perf_event_context_sched_in+0x98/0xe0
__perf_event_task_sched_in+0x5c/0xd8
finish_task_switch+0x184/0x1cc
schedule_tail+0x20/0xec
ret_from_fork+0x4/0x18
Sai Prakash Ranjan (2):
coresight: tmc-etf: Fix NULL ptr dereference in
tmc_enable_etf_sink_perf()
coresight: etb10: Fix possible NULL ptr dereference in
etb_enable_perf()
drivers/hwtracing/coresight/coresight-etb10.c | 4 +++-
drivers/hwtracing/coresight/coresight-priv.h | 2 ++
drivers/hwtracing/coresight/coresight-tmc-etf.c | 4 +++-
3 files changed, 8 insertions(+), 2 deletions(-)
base-commit: 3477326277451000bc667dfcc4fd0774c039184c
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Hi,
Just writing to check if you would be interested in acquiring the attendees
list of "Embedded Linux Conference Europe" Virtual|Oct 26th - 29th -
2020|
Each record will contain details like: Contact Name, Title, Phone Number,
Fax Number, Physical address, State/City, Company Name, Company URL, and
most importantly verified email addresses.
Let us know if you are interested and we will revert back with cost and
other deliverables.
Regards,
Laura Miller| Sales Team|
Email: laura(a)globaltradevisitors.com
"If you don't wish to receive email from us please reply back with Opt Out"
There is a bug on the systems supporting to skip power up
(qcom,skip-power-up) where setting LPOVERRIDE bit(low-power
state override behaviour) will result in CPU hangs/lockups
even on the implementations which supports it. So skip
setting the LPOVERRIDE bit for such platforms.
Fixes: 02510a5aa78d ("coresight: etm4x: Add support to skip trace unit power up")
Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan(a)codeaurora.org>
---
drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index abd706b216ac..6096d7abf80d 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -779,7 +779,7 @@ static void etm4_init_arch_data(void *info)
* LPOVERRIDE, bit[23] implementation supports
* low-power state override
*/
- if (BMVAL(etmidr5, 23, 23))
+ if (BMVAL(etmidr5, 23, 23) && (!drvdata->skip_power_up))
drvdata->lpoverride = true;
else
drvdata->lpoverride = false;
base-commit: 3477326277451000bc667dfcc4fd0774c039184c
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Hi,
I believe that you are one of the Exhibitors of "Arm DevSummit 2020".
If you are interested in acquiring the attendees list of "Arm DevSummit
2020" please reply to this email and I shall revert back with pricing,
counts.
Each record will contain details like Contact Name, Number, Email Address,
Company Name, URL, Physical Address, Phone etc.. and you basically get to
own these Opt-in contacts.
Let us know if you are interested and we will revert back with cost and
other deliverables.
Thank you and I look forward to hear from you soon.
Regards,
Holly Richard| Inside Sales, USA & Europe|
Email: <mailto:holly@reachattendee.com> holly(a)reachattendee.com
"If you don't wish to receive emails from us reply back with LEAVE OUT"
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.