Messages in this thread | | | Subject | Re: [PATCH v4 04/10] coresight: etm-perf: update to handle configuration selection | From | Suzuki K Poulose <> | Date | Thu, 4 Mar 2021 14:25:16 +0000 |
| |
On 3/4/21 2:19 PM, Mike Leach wrote: > Hi Suzuki, > > On Thu, 4 Mar 2021 at 12:13, Suzuki K Poulose <suzuki.poulose@arm.com> wrote: >> >> On 1/28/21 5:09 PM, Mike Leach wrote: >>> Loaded coresight configurations are registered in the cs_etm\cs_config sub >>> directory. This extends the etm-perf code to handle these registrations, >>> and the cs_syscfg driver to perform the registration on load. >>> >>> Signed-off-by: Mike Leach <mike.leach@linaro.org> >>> --- >>> .../hwtracing/coresight/coresight-config.h | 5 +- >>> .../hwtracing/coresight/coresight-etm-perf.c | 164 +++++++++++++++--- >>> .../hwtracing/coresight/coresight-etm-perf.h | 8 + >>> .../hwtracing/coresight/coresight-syscfg.c | 13 +- >>> 4 files changed, 166 insertions(+), 24 deletions(-) >>>
>>> +static ssize_t etm_perf_cscfg_event_show(struct device *dev, >>> + struct device_attribute *dattr, >>> + char *buf) >>> +{ >>> + struct dev_ext_attribute *ea; >>> + >>> + ea = container_of(dattr, struct dev_ext_attribute, attr); >>> + return scnprintf(buf, PAGE_SIZE, "%s\n", (const char *)(ea->var)); >>> +} >> >> "configid=0x%lx", (unsigned long)ea->var ? >> > > ea->var _is_ "configid=0x%lx" due to the way perf handles the events > sub-dir entries. >
This must be combined with the suggestion below.
>>> + >>> +static int etm_perf_add_cscfg_event(struct device *dev, struct cscfg_config_desc *cs_cfg) >>> +{ >>> + struct dev_ext_attribute *ea; >>> + unsigned long hash; >>> + int ret; >>> + struct device *pmu_dev = etm_pmu.dev; >>> + >>> + ea = devm_kzalloc(dev, sizeof(*ea), GFP_KERNEL); >>> + if (!ea) >>> + return -ENOMEM; >>> + >>> + hash = (unsigned long)cs_cfg->id_ea->var; >>> + >>> + sysfs_attr_init(&ea->attr.attr); >>> + ea->attr.attr.name = devm_kstrdup(dev, cs_cfg->name, GFP_KERNEL); >>> + if (!ea->attr.attr.name) >>> + return -ENOMEM; >>> + >>> + /* >>> + * attribute value is "configid=<hash>". >>> + * this will be what perf evaluates when the config name is used >>> + * on the command line. >>> + */ >>> + ea->var = devm_kzalloc(dev, CSCFG_EVENT_STR_SIZE, GFP_KERNEL); >>> + if (!ea->var) >>> + return -ENOMEM; >> >> Could we drop this string and use the "hash" instead ? >> > > No. My understanding is that we have added an events directory to > cs_etm, and add the configurations in there:- > > cs_etm/events/autofdo > > Now the contents of autofdo are "configid=0x<hash-value>" - where > hash-value is the hash of "autofdo". > > On the perf command line:- > > perf record -e cs_etm/autofdo/ ..... > > will result in perf parsing autofdo, looking in the events dir for > cs_etm, seeing the configid=-string, and parsing that to assign to > configid attribute - which we have allocated to config2:63:32 - this > will then appear as a value in the perf_event and we can load the > configuration when starting up the event on the ETM etc.
Sorry, I was not explicit in my comments. You could drop the string and have ea->var = hash. And the _show() could simply do
"configid=0x%lx" , hash
as mentioned above.
That would avoid another string allocation, with the same interface.
Suzuki
| |