linux-kernel - Re: [PATCH v4 04/10] coresight: etm-perf: update to handle configuration selection

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <921225a2-14a8-2a4f-4726-f61224cafa28@arm.com>
Date:   Thu, 4 Mar 2021 14:25:16 +0000
From:   Suzuki K Poulose <suzuki.poulose@....com>
To:     Mike Leach <mike.leach@...aro.org>
Cc:     linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        Coresight ML <coresight@...ts.linaro.org>,
        Mathieu Poirier <mathieu.poirier@...aro.org>,
        "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
        Yabin Cui <yabinc@...gle.com>,
        Jonathan Corbet <corbet@....net>, Leo Yan <leo.yan@...aro.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Tingwei Zhang <tingwei@...eaurora.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 04/10] coresight: etm-perf: update to handle
 configuration selection

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@....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@...aro.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