linux-kernel - Re: [PATCH 6/6] perf test: Add pmu-events test

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] [day] [month] [year] [list]

Date:   Fri, 13 Mar 2020 11:08:48 +0000
From:   John Garry <john.garry@...wei.com>
To:     Jiri Olsa <jolsa@...hat.com>
CC:     <peterz@...radead.org>, <mingo@...hat.com>, <acme@...nel.org>,
        <mark.rutland@....com>, <alexander.shishkin@...ux.intel.com>,
        <namhyung@...nel.org>, <will@...nel.org>, <ak@...ux.intel.com>,
        <linuxarm@...wei.com>, <linux-kernel@...r.kernel.org>,
        <james.clark@....com>, <qiangqing.zhang@....com>
Subject: Re: [PATCH 6/6] perf test: Add pmu-events test

Hi jirka,

>>>
>>>     - so we actualy have the parsed json events in C structs and we can go
>>>       through them and check it contains fields with strings that we expect
>>
>> No, we use pme_test_cpu[] to generate the event aliases for a PMU, and
>> verify that the aliases are as expected.
>>
>>>
>>>     - you go through all detected pmus and check if the tests events we
>>>       generated are matching some of the events from these pmus,
>>
>> Not exactly.
>>
>>>       and that's where I'm lost ;-) why?
>>
>> So consider the "cpu" HW PMU. During normal operation, we create the event
>> aliases for this PMU in pmu_lookup()->pmu_add_cpu_aliases(). This step looks
>> up a map of cpu events for that CPUID, and then creates the event aliases
>> for that PMU from that map.
>>
>> I want the test to recreate this and verify that the events from the test
>> JSONs will have event aliases created properly.
> 
> aah ok, my first objective was to have some way to test pmu-events
> changes we plan to do and their affect to generated pmu-event.c

Since the format of the test events table is slightly different to 
regular event tables, I wasn't sure on the value there. But I could add it.

For that, I could modify how we generate pmu-events.c as to not have a 
separate special test table, but add the test events as a special entry 
in pmu_events_map[], with some fake test cpuid. That could be better if 
we want to verify the generated test event table.

> 
> you want to test the code paths after that.. perfect
> 
>>
>> So in the test when we scan the PMUs and find "cpu" HW PMU, we create a test
>> PMU with the same name, create the event aliases from pme_test_cpu[] for
>> that test PMU, and then verify that the event aliases created are as
>> expected. Then the test PMU is deleted.
>>
>> So overall the test covers:
>> a. jevents code to generate the struct pmu_event []
>> b. util/pmu.c code to create the event aliases for a given PMU
>>
>> Note: the test does not (yet) cover matching of events declared in the HW
>> PMU sysfs folder. I'm talking about these, for example:
> 
> ok
> 

I'm going to look at what I can do here. I just worry it will make 
things more arch specific and make the test brittle.

>>
>> $ ls /sys/bus/event_source/devices/cpu/events/
>> branch-instructions  cache-references  el-abort     el-start ref-cycles
>> ...
>>
>>>
>>>>
>>>>>
>>>>> or as I'm thinking about that now, would it be enough
>>>>> to check pme_test_cpu array to have string that we
>>>>> expect?
>>>>
>>>> Right, I might change this.
>>>>
>>>> So currently we iterate the PMU aliases to ensure that we have a matching
>>>> event in pme_test_cpu[]. It may be better to iterate the events in
>>>> pme_test_cpu[] to ensure that we have an alias.
>>>
>>> that's what I described above.. I dont understand the connection/value
>>> of this tests
>>>
>>>>
>>>> The problem here is uncore PMUs. They have the "Unit" field, which is used
>>>> for matching the PMU. So we cannot ensure test events from uncore.json will
>>>> always have an event alias created per PMU. But maybe I could use
>>>> pmu_uncore_alias_match() to check if the test event matches in this case.
>>>
>>> hum I guess I don't follow all the details.. but some more explanation
>>> of the test would be great
>>
>> Let's just concentrate on core PMU ATM :)

So just going back to the uncore events now, since they have the "Unit" 
property to match to specific uncore PMUs, we need to test them slightly 
differently to core PMUs. But I have improved the testing for that now.

Thanks,
John