From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751110AbdJRQmz (ORCPT ); Wed, 18 Oct 2017 12:42:55 -0400 Received: from szxga04-in.huawei.com ([45.249.212.190]:8946 "EHLO szxga04-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750926AbdJRQmx (ORCPT ); Wed, 18 Oct 2017 12:42:53 -0400 Subject: Re: [PATCH] perf vendor events arm64: Add hip08 implementation defined PMU core events To: Will Deacon References: <1508223699-39274-1-git-send-email-zhangshaokun@hisilicon.com> <20171017125926.GE14051@arm.com> <20171018104910.GB11669@arm.com> CC: Shaokun Zhang , Ganapatrao Kulkarni , , , , Peter Zijlstra , Ingo Molnar , "Arnaldo Carvalho de Melo" , Alexander Shishkin From: John Garry Message-ID: <49db001c-14c5-74ea-0d60-ccd5d46b0bfe@huawei.com> Date: Wed, 18 Oct 2017 17:37:03 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <20171018104910.GB11669@arm.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.202.227.238] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A010204.59E78489.0022,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: c57947254eea56ddc0daa984864de9d4 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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: >>> Hi Shaokun, >>> >>> Thanks for the patch. One comment below. >>> >>> On Tue, Oct 17, 2017 at 03:01:39PM +0800, Shaokun Zhang wrote: >>>> This is a short list of useful implementation defined PMU events of >>>> hip08, other supported events are not listed in this JSON file. >>>> >>>> This patch is dependent on Cavium's patch-v9 (Add support for >>>> ThunderX2 pmu events using json files), Link: >>>> https://www.spinics.net/lists/arm-kernel/msg611895.html >>>> >>>> Signed-off-by: Shaokun Zhang >>>> Cc: Peter Zijlstra >>>> Cc: Ingo Molnar >>>> Cc: Arnaldo Carvalho de Melo >>>> Cc: Alexander Shishkin >>>> Cc: Will Deacon >>>> Cc: Ganapatrao Kulkarni >>>> Cc: John Garry >>>> --- >>>> .../arch/arm64/hisilicon/hip08-imp-def.json | 176 +++++++++++++++++++++ >>>> tools/perf/pmu-events/arch/arm64/mapfile.csv | 1 + >>>> 2 files changed, 177 insertions(+) >>>> create mode 100644 tools/perf/pmu-events/arch/arm64/hisilicon/hip08-imp-def.json >>>> >>>> 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", }, ] #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. John > > Will > > . >