[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <86abfa22-4d2e-2a4-7365-571a69e8a233@os.amperecomputing.com>
Date: Mon, 28 Oct 2024 19:06:26 -0700 (PDT)
From: Ilkka Koskinen <ilkka@...amperecomputing.com>
To: James Clark <james.clark@...aro.org>
cc: Ilkka Koskinen <ilkka@...amperecomputing.com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Namhyung Kim <namhyung@...nel.org>, Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
Adrian Hunter <adrian.hunter@...el.com>,
"Liang, Kan" <kan.liang@...ux.intel.com>,
John Garry <john.g.garry@...cle.com>, Will Deacon <will@...nel.org>,
Mike Leach <mike.leach@...aro.org>, Leo Yan <leo.yan@...ux.dev>
Subject: Re: [PATCH] perf arm-spe: Add support for SPE Data Source packet on
AmpereOne
Hi James,
On Fri, 25 Oct 2024, James Clark wrote:
> On 25/10/2024 12:30 am, Ilkka Koskinen wrote:
>> Decode SPE Data Source packets on AmpereOne. The field is IMPDEF.
>>
>> Signed-off-by: Ilkka Koskinen <ilkka@...amperecomputing.com>
>> ---
>> .../util/arm-spe-decoder/arm-spe-decoder.h | 9 +++
>> tools/perf/util/arm-spe.c | 61 +++++++++++++++++++
>> 2 files changed, 70 insertions(+)
>>
>> diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h
>> b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h
>> index 1443c28545a9..e4115b1e92b2 100644
>> --- a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h
>> +++ b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h
>> @@ -67,6 +67,15 @@ enum arm_spe_neoverse_data_source {
>> ARM_SPE_NV_DRAM = 0xe,
>> };
>> +enum arm_spe_ampereone_data_source {
>> + ARM_SPE_AMPEREONE_LOCAL_CHIP_CACHE_OR_DEVICE = 0x0,
>> + ARM_SPE_AMPEREONE_SLC = 0x3,
>> + ARM_SPE_AMPEREONE_REMOTE_CHIP_CACHE = 0x5,
>> + ARM_SPE_AMPEREONE_DDR = 0x7,
>> + ARM_SPE_AMPEREONE_L1D = 0x8,
>> + ARM_SPE_AMPEREONE_L2D = 0x9,
>> +};
>> +
>> struct arm_spe_record {
>> enum arm_spe_sample_type type;
>> int err;
>> diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
>> index 138ffc71b32d..04bd21ad7ea8 100644
>> --- a/tools/perf/util/arm-spe.c
>> +++ b/tools/perf/util/arm-spe.c
>> @@ -515,10 +515,69 @@ static void arm_spe__synth_data_source_generic(const
>> struct arm_spe_record *reco
>> data_src->mem_lvl |= PERF_MEM_LVL_REM_CCE1;
>> }
>> +static const struct midr_range ampereone_source_spe[] = {
>> + MIDR_ALL_VERSIONS(MIDR_AMPERE1A),
>> + {},
>> +};
>> +
>> +static void arm_spe__synth_data_source_ampereone(const struct
>> arm_spe_record *record,
>> + union perf_mem_data_src
>> *data_src,
>> + u64 midr)
>> +{
>> + if (!is_midr_in_range_list(midr, ampereone_source_spe)) {
>> + arm_spe__synth_data_source_generic(record, data_src);
>> + return;
>> + }
> [...]
>> static u64 arm_spe__synth_data_source(const struct arm_spe_record
>> *record, u64 midr)
>> {
>> union perf_mem_data_src data_src = { .mem_op = PERF_MEM_OP_NA };
>> bool is_neoverse = is_midr_in_range_list(midr, neoverse_spe);
>> + bool is_ampereone = (read_cpuid_implementor() == ARM_CPU_IMP_AMPERE);
>>
>
> Hi Ilkka,
>
> I think this read_cpuid_implementor() is for the device that's running
> report, rather than record. You need to use the midr that's saved into the
> file.
Uh, that didn't come to my mind at all.
> But it looks like you've done that for is_midr_in_range_list(midr,
> ampereone_source_spe) above. Is it possible to just do that and not
> read_cpuid_implementor() and then it's done the same way as neoverse and also
> works off target?
You're right, it's wrong and there's no even need for separate implementor
id check. I fix it.
Cheers, Ilkka
>
> Thanks
>
> James
>
>
Powered by blists - more mailing lists