Thread (41 messages) 41 messages, 4 authors, 2019-04-24

Re: [PATCH v2 03/16] coresight: etm4x: Configure tracers to emit timestamps

From: Suzuki K Poulose <suzuki.poulose@arm.com>
Date: 2019-03-26 11:51:51

On 03/25/2019 09:56 PM, Mathieu Poirier wrote:
quoted hunk ↗ jump to hunk
Configure timestamps to be emitted at regular intervals in the trace
stream to temporally correlate instructions executed on different CPUs.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
  drivers/hwtracing/coresight/coresight-etm4x.c | 115 +++++++++++++++++-
  1 file changed, 112 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index 732ae12fca9b..45c341a5aa0b 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -138,8 +138,11 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
  			       drvdata->base + TRCCNTVRn(i));
  	}
  
-	/* Resource selector pair 0 is always implemented and reserved */
-	for (i = 0; i < drvdata->nr_resource * 2; i++)
+	/*
+	 * Resource selector pair 0 is always implemented and reserved.  As
+	 * such start at 2.
+	 */
+	for (i = 2; i < drvdata->nr_resource * 2; i++)
  		writel_relaxed(config->res_ctrl[i],
  			       drvdata->base + TRCRSCTLRn(i));

To be frank, that looks like a separate fix for the existing driver,
from this series.
quoted hunk ↗ jump to hunk
  
@@ -201,6 +204,97 @@ static void etm4_enable_hw_smp_call(void *info)
  	arg->rc = etm4_enable_hw(arg->drvdata);
  }
  
+/*
+ * The goal of function etm4_config_timestamp_event() is to configure a
+ * counter that will tell the tracer to emit a timestamp packet when it
+ * reaches zero.  This is done in order to get a more fine grained idea
+ * of when instructions are executed so that they can be correlated
+ * with execution on other CPUs.
+ *
+ * To do this the counter itself is configured to self reload and
+ * TRCRSCTLR1 (always true) used to get the counter to decrement.  From
+ * there a resource selector is configured with the counter and the
+ * timestamp control register to use the resource selector to trigger the
+ * event that will insert a timestamp packet in the stream.
+ */
+static int etm4_config_timestamp_event(struct etmv4_drvdata *drvdata)
+{
+	int i, ctridx, ret = -EINVAL;
+	int counter, rselector;
+	u32 val = 0;
+	struct etmv4_config *config = &drvdata->config;
+
+	/* No point in trying if we don't have at least one counter */
+	if (!drvdata->nr_cntr)
+		goto out;
+
+	/* Find a counter that hasn't been initialised */
+	for (i = 0; i < drvdata->nr_cntr; i++)
+		if (config->cntr_val[i] == 0)
+			break;
+
+	/* Remember what counter we used */
+	counter = 1 << i;
+	ctridx = i;
+
+	/* All the counters have been configured already, bail out */
+	if (i == drvdata->nr_cntr) {
+		pr_err("%s: no available counter found\n", __func__);
+		goto out;
+	}
Should this be pr_debug ? This could be easily triggered to flood the
dmesg. Also, I think the return code could be -ENODEV or -ENOSPC rather
than -EINVAL in this case.
+
+	/*
+	 * Initialise original and reload counter value to the smallest
+	 * possible value in order to get as much precision as we can.
+	 */
+	config->cntr_val[ctridx] = 1;
+	config->cntrldvr[ctridx] = 1;
We could delay the initialisation until we find the resource selector
to avoid clearing them later.
+
+	/* Set the trace counter control register */
+	val =  0x1 << 16	|  /* Bit 16, reload counter automatically */
+	       0x0 << 7		|  /* Select single resource selector */
+	       0x1;		   /* Resource selector 1, i.e always true */
+
+	config->cntr_ctrl[ctridx] = val;
+
+	/*
+	 * Searching for an available resource selector to use, starting at
+	 * '2' since every implementation has at least 2 resource selector.
+	 * ETMIDR4 gives the number of resource selector _pairs_,
+	 * hence multiply by 2.
+	 */
+	for (i = 2; i < drvdata->nr_resource * 2; i++)
+		if (!config->res_ctrl[i])
+			break;
+
+	/* Remember what resource selector we used */
+	rselector = i;
+
+	if (i == drvdata->nr_resource * 2) {
+		pr_err("%s: no available resource selector found\n", __func__);
+
Same as above. This shouldn't be an error, may be a debug. It is a
crunch of resources, a usage error from the user and not kernel.
quoted hunk ↗ jump to hunk
+		/* Backout what we did and exit */
+		config->cntr_ctrl[ctridx] = 0;
+		config->cntrldvr[ctridx] = 0;
+		config->cntr_val[ctridx] = 0;
+		goto out;
+	}
+
+	val = 0x2 << 16		| /* Group 0b0010 - Counter and sequencers */
+	      counter << 0;	  /* Counter to use */
+
+	config->res_ctrl[i] = val;
+
+	val = 0x0 << 7		| /* Select single resource selector */
+	      rselector;	  /* Resource selector */
+
+	config->ts_ctrl = val;
+
+	ret = 0;
+out:
+	return ret;
+}
+
  static int etm4_parse_event_config(struct etmv4_drvdata *drvdata,
  				   struct perf_event *event)
  {
@@ -236,9 +330,24 @@ static int etm4_parse_event_config(struct etmv4_drvdata *drvdata,
  		/* TRM: Must program this for cycacc to work */
  		config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
  	}
-	if (attr->config & BIT(ETM_OPT_TS))
+	if (attr->config & BIT(ETM_OPT_TS)) {
+		/*
+		 * Configure timestamps to be emitted at regular intervals in
+		 * order to correlate instructions executed on different CPUs
+		 * (CPU-wide trace scenarios).
+		 */
+		ret = etm4_config_timestamp_event(drvdata);
+
+		/*
+		 * No need to go further if timestamp intervals can't
+		 * be configured.
+		 */
+		if (ret)
+			goto out;
+
  		/* bit[11], Global timestamp tracing bit */
  		config->cfg |= BIT(11);
+	}
  
  	if (attr->config & BIT(ETM_OPT_CTXTID))
  		/* bit[6], Context ID tracing bit */

Rest all looks good to me.

Suzuki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help