Messages in this thread | | | Date | Wed, 18 Oct 2017 18:04:04 +0100 | From | Will Deacon <> | Subject | Re: [PATCH] perf vendor events arm64: Add hip08 implementation defined PMU core events |
| |
Hi John,
On Wed, Oct 18, 2017 at 05:37:03PM +0100, John Garry wrote: > On 18/10/2017 11:49, Will Deacon wrote: > >On Wed, Oct 18, 2017 at 10:25:39AM +0100, John Garry wrote: > >>On 17/10/2017 13:59, Will Deacon wrote: > >>>On Tue, Oct 17, 2017 at 03:01:39PM +0800, Shaokun Zhang wrote: > >>>>diff --git a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08-imp-def.json b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08-imp-def.json > >>>>new file mode 100644 > >>>>index 0000000..6bb31da > >>>>--- /dev/null > >>>>+++ b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08-imp-def.json > >>>>@@ -0,0 +1,176 @@ > >>>>+[ > >>>>+ { > >>>>+ "PublicDescription": "Attributable Level 1 data cache access, read", > >>>>+ "EventCode": "0x40", > >>>>+ "EventName": "L1D_CACHE_RD", > >>>>+ "BriefDescription": "L1D cache access, read", > >>>>+ }, > >>>>+ { > >>>>+ "PublicDescription": "Attributable Level 1 data cache access, write", > >>>>+ "EventCode": "0x41", > >>>>+ "EventName": "L1D_CACHE_WR", > >>>>+ "BriefDescription": "L1D cache access, write", > >>>>+ }, > >>> > >>>So these are the same as the events in cavium/thunderx2-imp-def.json and > >>>should be factored out. In fact, ARM recommends event numbers for 0x40-0xBF, > >>>so the best thing would be to have those defined in their own file, then > >>>have a way for the various CPU-specific .json files to pick and chose the > >>>events they need from there. > >> > >>Right, this seems reasonable. Just need to check on feasible. > >> > >>In terms of coordinating this work, shall we do it? Will arm64+ThunderX > >>support be accepted as is? > > > >Yes, that would be my preference if you don't mind. > > > Cool. > > I am looking at this topic now. But I am doubting the folder structure > again. > > Firstly, we still have this comment in the README: > All the topic JSON files for a CPU model/family should be in a separate > sub directory. > > Now, when thunderx3 or hip09 comes along, I assume that their jsons will > similarly go into cavium and hisilcon folders, respectively. So, for > example, we add thunderx3 json, like this: > arm64/cavium/thunderx3-imp-def.json > [ > { > "PublicDescription": "foo", > "EventCode": "0x40", > "EventName": "bar", > "BriefDescription": "sieve", > }, > ]
Hang on, event 0x40 is one of the common ones, so that should be somewhere else like arm64/armv8-common.json.
> #Family-model,Version,Filename,EventType > 0x00000000420f5160,v1,cavium,core > 0x00000000420f5161,v1,cavium,core > > Then we have generated pmu_events.c, like this: > #include "../../pmu-events/pmu-events.h" > struct pmu_event pme_cavium[] = { > { > .name = "bar", > .event = "event=0x40", > .desc = "sieve", > .topic = "thunderx3 imp def", > .long_desc = "foo", > }, > { > .name = "l1d_cache_rd", > .event = "event=0x40", > .desc = "L1D cache read", > .topic = "thunderx2 imp def", > .long_desc = "Attributable Level 1 data cache access, read", > }, > { > .name = "l1d_cache_wr", > .event = "event=0x41", > .desc = "L1D cache write", > .topic = "thunderx2 imp def", > .long_desc = "Attributable Level 1 data cache access, write ", > }, > > [ ... ] > > { > .name = 0, > .event = 0, > .desc = 0, > }, > }; > struct pmu_events_map pmu_events_map[] = { > { > .cpuid = "0x00000000420f5160", > .version = "v1", > .type = "core", > .table = pme_cavium > }, > { > .cpuid = "0x00000000420f5161", > .version = "v1", > .type = "core", > .table = pme_cavium > }, > { > .cpuid = 0, > .version = 0, > .type = 0, > .table = 0, > }, > }; > > It doesn't look right, espcially since we have conflicting definitions for > event 0x40. We really should have table per cpu.
There should be one definition of event 0x40 and it should use the wording from the ARM ARM:
0x0040 , L1D_CACHE_RD, Attributable Level 1 data cache access, read This event is similar to Level 1 data cache access, L1D_CACHE, but the counter counts only memory-read operations that access at least the Level 1 data or unified cache.
I think that the tricky bit is working out which subset of the armv8-common.json file applies to a given CPU, but I was hoping you'd have some ideas about that, even if it's an additional build step when building perf to generate the final JSON files
Will
| |