Hey Leo,
On Sun, 17 Mar 2019 at 00:30, Leo Yan leo.yan@linaro.org wrote:
On Wed, Mar 06, 2019 at 03:57:35PM -0700, Mathieu Poirier wrote:
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 | 109 +++++++++++++++++- 1 file changed, 106 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c index f067f7ee08c7..2c29d6743fee 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));@@ -201,6 +204,91 @@ 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, 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;/* 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;}/** Initialise original and reload counter value to the smallest* possible value in order to get as much precision as we can.*/config->cntr_val[i] = 1;config->cntrldvr[i] = 1;Maybe this variable is defined by old code, but show use the more consistent naming like: config->cntr_ldvr?
Perhaps yes... But it would require a patch on its own, something that is at the very bottom of the priority list.
/* 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[i] = 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__);goto out;}If there ahve no available resource selector and 'goto' out tag, it will not reset the values for counter registers. Do you think for the failure case, we should clean below fields:
config->cntr_val[i] = 0; config->cntrldvr[i] = 0; config->cntr_ctrl[i] = 0;
You are correct - I will fix this.
val = 0x2 << 16 | /* Group 0b0010 - Counter and sequencers */counter << 0; /* Counter to use */When read the ETMv4 spec, the counter value are only set in bits 0 to 3, so should reserve for other fields?
This is a concern I looked into a while back. The other bits are labeled "RES0", which is defined in the TRM's glossary as:
"A reserved bit or field with Should-Be-Zero-or-Preserved (SBZP) behavior".
As such we should be fine with just writing '0' to the RES0 bits.
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 +324,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);Use this method, I can see now it only allocates counter but there have no flow to release counter; so after run for several times with perf command, will it fail to allocate any counter and resource selector?
Very well spotted. I will fix this.
/** 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 */-- 2.17.1