lkml.org 
[lkml]   [2021]   [Mar]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v4 04/10] coresight: etm-perf: update to handle configuration selection
    From
    Date
    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

    \
     
     \ /
      Last update: 2021-03-04 15:28    [W:2.859 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site