Hi Suzuki,
On Wed, 7 Oct 2020 at 10:36, Suzuki K Poulose suzuki.poulose@arm.com wrote:
Hi Mike
On 08/27/2020 03:34 PM, Mike Leach wrote:
Adds handling for the cs_etm/cs_config directory that contains loaded configurations that may be selected by the perf command line using the @name syntax.
Adds flag to mark the hash ID found as a configuration name rather than a sink name.
Signed-off-by: Mike Leach mike.leach@linaro.org
Do we need this to be controlled by "@" ?
Can we make it work like :
cat /sys/bus/event_source/devices/cs_etm/format/cs_sys_cfg config2:36
cat /sys/bus/event_source/devices/cs_etm/events/autofdo cs_sys_cfg=1,sinkid=<hash-for-autodo>
Not sure how this works on the command line?
Are you suggesting the replacing the easy to use: perf -e cs_etm/@autofdo/ ... with: perf -e cs_etm/cs_sys_cfg=1,sinkid=<some value i've got to look up and copy>/ ... or are you suggesting that the two syntax variants should be equivalent - as per the @sink / sinkid=<sink_hash> are?
Given that for a given name configuration such as 'autofdo', this will be visible to the user as 'autofdo' in configfs, will be visible to the user in /sys/bus/event_source/devices/cs_etm/cs_config as 'autofdo', requiring the use two elements, one of which is sinkid is overly complex and misleading given this is not a sink selection. One of the drivers for me doing the auto sink selection was to enable the use of configurations - including the selection of such.
Having looked at the perf command processing and yacc etc, it did not seem sensible to try to come up with a new command option given that this was easy to use and designed to convert a string token into a hash value that could be passed to the kernel drivers later.
The use of named configurations and using @ is predicated on making selection simple. There is no reason I can see for not using the @ syntax, but it may be possible to include an equivalence with a flag.sinkid=<hash> syntax, which would also have the advantage of resulting in an entry in the format directory marking bit 36 of config2 used.
Thanks and Regards
Mike
Cheers Suzuki
include/linux/coresight-pmu.h | 3 +++ tools/include/linux/coresight-pmu.h | 3 +++ tools/perf/arch/arm/util/cs-etm.c | 17 +++++++++++++++-- 3 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/include/linux/coresight-pmu.h b/include/linux/coresight-pmu.h index b0e35eec6499..9bb0fb46f576 100644 --- a/include/linux/coresight-pmu.h +++ b/include/linux/coresight-pmu.h @@ -22,6 +22,9 @@ #define ETM4_CFG_BIT_TS 11 #define ETM4_CFG_BIT_RETSTK 12
+/* Coresight config select bit - mark sink ID as a config ID */ +#define CSSYS_BIT_CONFIG_SEL 36
- 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..9bb0fb46f576 100644 --- a/tools/include/linux/coresight-pmu.h +++ b/tools/include/linux/coresight-pmu.h @@ -22,6 +22,9 @@ #define ETM4_CFG_BIT_TS 11 #define ETM4_CFG_BIT_RETSTK 12
+/* Coresight config select bit - mark sink ID as a config ID */ +#define CSSYS_BIT_CONFIG_SEL 36
- static inline int coresight_get_trace_id(int cpu) { /*
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c index cad7bf783413..4c46448396f1 100644 --- a/tools/perf/arch/arm/util/cs-etm.c +++ b/tools/perf/arch/arm/util/cs-etm.c @@ -219,6 +219,7 @@ static int cs_etm_set_sink_attr(struct perf_pmu *pmu, struct evsel_config_term *term; int ret = -EINVAL; u32 hash;
u64 config_sel = 0; if (evsel->core.attr.config2 & GENMASK(31, 0)) return 0;
@@ -228,9 +229,21 @@ static int cs_etm_set_sink_attr(struct perf_pmu *pmu, continue;
sink = term->val.str;
snprintf(path, PATH_MAX, "sinks/%s", sink);
/* re-using @val for config select - check for config first */
snprintf(path, PATH_MAX, "cs_config/%s", sink); ret = perf_pmu__scan_file(pmu, path, "%x", &hash);
if (ret != 1) {
/* not config, look for sink */
snprintf(path, PATH_MAX, "sinks/%s", sink);
ret = perf_pmu__scan_file(pmu, path, "%x", &hash);
} else {
/* is a config - mark as such */
config_sel = BIT(CSSYS_BIT_CONFIG_SEL);
}
if (ret != 1) { pr_err("failed to set sink \"%s\" on event %s with %d (%s)\n", sink, evsel__name(evsel), errno,
@@ -238,7 +251,7 @@ static int cs_etm_set_sink_attr(struct perf_pmu *pmu, return ret; }
evsel->core.attr.config2 |= hash;
evsel->core.attr.config2 |= hash | config_sel; return 0; }